Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVCB/HTTPS RFC updates, ECH config tweaks #2183

Merged
merged 10 commits into from Apr 18, 2024

Commits on Apr 17, 2024

  1. ci: offer a branch push pattern for easy testing

    Pushing branches named "$WHATEVER_dev" will result in CI being run. This
    is helpful for those working on a fork that want a quick way to test CI
    for their branch before opening a PR.
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    0190b08 View commit details
    Browse the repository at this point in the history
  2. svcb: update docs to ref RFC 9460

    Since initial support for SVCB/HTTPS RRs landed in hickory-dns, RFC
    9460[0] was published:
    
      Service Binding and Parameter Specification via the DNS (SVCB and HTTPS
      Resource Records)
    
    This is the definitive reference for SVCB and HTTPS RRs and previous
    references to `draft-ietf-dnsop-svcb-https-XX` need to be updated.
    
    Thankfully, it seems as though the implementation did not change
    meaningfully from draft-03 and so this commit can largely just update
    documentation references and copied quotations to match RFC 9460.
    
    One minor change is worth mentioning: the Encrypted Client Hello (ECH)
    aspects of the draft were removed pre-publication and the RFC9460 IANA
    registry includes a "reserved" allocation for the `"ech"` key, but no
    details on its use. These details are now located in a separate draft,
    draft-ietf-tls-svcb-ech-01[1].
    
    Since the code in `svcb.rs` also concerned itself with ECH it now
    references draft-ietf-tls-svcb-ech-01 where the ECH specific usage of
    service parameter is under specification. Notably the new draft and RFC
    9460 both use `"ech"` for the service parameter key for encrypted client
    hello configs. Hickory-dns is currently using `"echconfig"`, but this
    will be fixed in a follow-up commit to keep this one documentation only.
    
    [0]: https://datatracker.ietf.org/doc/html/rfc9460
    [1]: draft-ietf-tls-svcb-ech-01
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    745bf13 View commit details
    Browse the repository at this point in the history
  3. proto: correct ECH service parameter key

    Previously `"echconfig"` was being used as the encrypted client hello
    (ECH) service parameter key for SVCB/HTTPS RRs.
    
    In RFC960 the parameter key is specified in the intial IANA registry
    contents as `"ech"`[0].
    
    This commit updates the two relevant parts of hickory (and corresponding
    test data) to use the up-to-date parameter key.
    
    This is a breaking change, however given the very low adoption of ECH,
    and the use of the correct `"ech"` key in popular test servers, it
    doesn't seem worth trying to maintain backwards compatibility with
    earlier draft RFC values.
    
    [0]: https://datatracker.ietf.org/doc/html/rfc9460#section-14.3.2
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    381a084 View commit details
    Browse the repository at this point in the history
  4. rdata_parsers: reorder svcb parse helpers

    Prev the ECH config parsing was placed in between parsing ipv4 and ipv6
    hints. This commit reorders so that `parse_ech_config()` is after
    `parse_ipv4_hint()` and `parse_ipv6_hint()`.
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    606dd36 View commit details
    Browse the repository at this point in the history
  5. proto: rename ECH SVCB types

    Previously the hickory-dns representation for ECH configurations in
    SVCB/HTTPS records were named `EchConfig` and stored/exposed
    a non-standard encoding of the config data, with the TLS-encoded length
    prefix stripped.
    
    In practice (and perhaps made clearer by draft-ietf-tls-svcb-ech-01[0]
    vs earlier texts), the value in wire-encoded form is "an ECHConfigList"
    as specified in Section 4 of draft-ietf-tls-esni-18[1] in TLS
    presentation language as:
    
    ```
    ECHConfig ECHConfigList<1..2^16-1>;
    ```
    
    To make it clearer that it's a _list_ of `ECHConfig` values in the
    `ech=` SVCB/HTTPS key, this commit renames the types to emphasize their
    listy-ness.
    
    [0]: https://datatracker.ietf.org/doc/html/draft-ietf-tls-svcb-ech-01
    [1]: https://datatracker.ietf.org/doc/html/draft-ietf-tls-esni-18#section-4
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    658b02e View commit details
    Browse the repository at this point in the history
  6. rdata: store wire-encoded form of ECH configs

    Previously the hickory-dns representation of ECH configs found in
    SVCB/HTTPS records held and exposed its own non-standard representation
    of the encoded ECH configs. Notably, it stripped the TLS-encoded list
    length prefix from the remaining data. Similarly, it's presentation
    format was the BASE64 encoding of this non-standard form.
    
    Downstream consumers are likely to want the wire-encoding format
    unmodified, because ECH is of most use to TLS libraries where they will
    have already implemented a generic TLS-encoded list decoder that expects
    the length prefix. In practice, popular tools like `dig`
    are also encoding the presentation format BASE64 of the data in DNS for
    some popular test servers with the prefix included.
    
    This commit updates hickory-dns's representation to not do the
    pre-processing it was before. This is trivial for a consumer to do if
    they need it, and avoids having to restore it manually in order to use
    other pre-existing TLS encoder/decoders with the value from hickory-dns.
    
    Again, since ECH adoption is in very early days it doesn't seem
    worthwhile to try and come up with a backwards compatible interface for
    those that need the old behaviour. It should be straightforward to
    remove the length prefix manually if required.
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    4d7435c View commit details
    Browse the repository at this point in the history
  7. rdata_parsers: support quoted SVCB parameter values

    The presentation format for RFC9460 SVCB/HTTPS RR types allows for
    parameter values to be quoted. The code for processing parameter values
    in hickory-dns had a comment indicating quotes should have been
    stripped, but this wasn't occurring in practice.
    
    This commit updates the parsing logic to perform the mentioned quote
    stripping, and updates the unit tests with fresh data found from doing
    `HTTPS` lookups for `google.com` and `crypto.cloudflare.com`. Notably
    both of these show quoted strings in `dig`'s presentation format output
    and so were sufficient to tickle the bug fixed above.
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    7022955 View commit details
    Browse the repository at this point in the history
  8. rdata_parsers: add RFC 9460 test vectors

    See https://datatracker.ietf.org/doc/html/rfc9460#appendix-D
    
    Tests that we can parse the presentation format for each of the positive
    test vectors.
    
    Possible follow-up work:
      * Add the negative failure tests
      * Add coverage for wire format matching expected
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    2cc190a View commit details
    Browse the repository at this point in the history
  9. proto: fix parse of arbitrary keys in pres. syntax

    Previously the `FromStr` impl for `SvcParamKey` had support for parsing
    the "arbitrary key" presentation syntax where a key can be specified
    "keyNNNNN", where NNNNN is the numeric value of the key type without
    leading zeros. The existing code would pull out the numeric component
    into a `u16` and then use the `TryFrom<u16>` impl for `SvcParamKey` to
    get the key.
    
    However, the `TryFrom<u16>` impl for `SvcParamKey` was using the IANA
    service parameter keys registry to map from u16s to `SvcParamKey`.
    Values 0..6 are mapped to the known key entries. The reserved range
    (65280-65534) was mapped to `SvcParamKey::Key`, and 65535 was mapped to
    `SvcParamKey::Key65535`. This makes sense when mapping an arbitrary u16,
    but when we are parsing a "keyNNNNN" presentation syntax item, we want
    to represent it as `Key(NNNNN)`, no matter if it is/isn't a registered
    key.
    
    This commit fixes this behaviour, constructing a `SvcParamKey::Key()`
    entry when parsing the arbitrary key presentation syntax, avoiding
    `TryFrom<u16>`.
    
    With this change in place the two arbitrary key test vectors can be
    included in the svcb test vector unit test.
    
    [0] https://datatracker.ietf.org/doc/html/rfc9460#name-initial-contents
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    832a8be View commit details
    Browse the repository at this point in the history
  10. rdata_parsers: fix handling of escaped list delim

    The RFC 9460 presentation syntax allows escaping the ',' list separator
    in a `SvcParamValue` by writing `\,`. This commit updates `parse_list`
    to handle this case.
    
    Once this is done, one of the RFC 9460 test vectors using this feature
    can be added to the unit tests. We don't yet support the more
    complicated escaped comma using an escaped backslash for delimiter
    escape so one remaining test vector for this is left out.
    cpu committed Apr 17, 2024
    Configuration menu
    Copy the full SHA
    1f846a1 View commit details
    Browse the repository at this point in the history