MikeBishop / dns-alt-svc

Draft for listing Alt-Svc records in the DNS
Other
71 stars 26 forks source link

Why allow multiple values for the same parameter at all? #95

Closed davidben closed 4 years ago

davidben commented 4 years ago

It seems the only parameters which allow this is ipv4hint and ipv6hint, which is not necessary because the value serialization supports multiple addresses (https://github.com/MikeBishop/dns-alt-svc/pull/94).

Why do we need this? I think we should only allow one value per parameter.

This means duplicate values can be rejected generically in the parser. Right now the spec says individual parameters silently ignore all but the first value. Silently ignoring things isn't great. Other protocols like Roughtime further require keys be sorted, which makes it easy for the parser to reject duplicates generically.

This also avoids questions when designing a new parameter as to whether to use multiple parameter values or to encode multiple structures in the value. We should have one way to do things, so let's use the latter.

enygren commented 4 years ago

The other argument would be to always do the former (multiple parameter values) in the wireformat and to strongly discourage things from having values that encode multiple structures in the value. Presentation format multi-value parameters could be syntactic sugar and convert to to multiple parameters on the wire

enygren commented 4 years ago

Agreed that we should be consistent here.

Presumably ESNIConfig is another use-case?

davidben commented 4 years ago

I believe HTTPSSVC's ESNI story, as currently specified, does not allow for multiple ESNIConfigs. It doesn't say anything and thus gets the default. This is indeed a mistake, but we're fixing it by doing the multiple values at the TLS level, which lets us handle it consistently at all sources of ESNI config. (See #88.)

It's also generally more compact to handle multiplicity within the value. Each HTTPSSVC value costs four bytes of framing per value. If the value is self-delimiting (true for addresses and ESNIConfig), you have a one-time four byte cost from HTTPSSVC, then each additional value costs no extra framing. If the value is not self-delimiting, you need length prefixes internally, costing two bytes per value. That's two bytes extra at one value, breaking even at two values, and a win beyond that.

(The difference comes from not repeating the parameter name each time.)

davidben commented 4 years ago

@dmcardle FYI

enygren commented 4 years ago

Your argument seems reasonable. We should make this clear and deterministic especially at the wireformat layer (eg, must only use the first instance of a parameter, provisioning systems should warn on duplicates, etc).

Allowing syntactic sugar at the presentation layer (eg, concatenation of parameters that are duplicated?) might be helpful usability-wise or could create many more problems and instead we should recommend that provisioning systems prevent this (or at least warn).

bemasc commented 4 years ago

I agree with this proposal, but it's going to collide heavily with #97, so I'd prefer to get that settled before updating the text. I have updated #97 to make transport names easier to delimit.

dmcardle commented 4 years ago

Could we revisit this? Given the current state of #97, it looks like it is expecting that multiple values are allowed, e.g. transport=foo transport=bar no-default-transport.

Equivalently, the wire format could simply encode a length-prefixed sequence of protocol IDs, eliminating the need for repeated SvcParamKeys. A slight annoyance is that the protocol IDs don't have fixed lengths. We could (1) invent uint16 values in our registry, (2) add length prefixes to each protocol ID, or (3) use null terminators.

davidben commented 4 years ago

I think I've lost track of what "this" is and what is being revisited where. :-) I think the encoding should be:

Re protocol IDs not having fixed lengths, just add length prefixes as needed. That's all a multi-valued parameter is doing anyway. (I don't think we should use NUL-termination. That sort of thing is prone to injection problems. See also all the problems when things get stuck into C-style strings.)

(I still need to review #97, but I doubt a vector<transport> would work anyway given that you need a port number. If it ends up being a vector<tuple<transport, port>> or a different design, that further suggests a key-specific serialization. A vector<tuple<transport, port>> needs a length prefix on the transport, not on the overall tuple.)

bemasc commented 4 years ago

I think we pretty much have consensus on this encoding.

Since #97 is a major semantic change, I'd like to get it finalized and merged before we make this syntax change.

Regarding ports, the current plan is to have a single fixed port number for each SvcFieldValue, even if it has multiple transports. This port number applies to all transports. Endpoints with different port numbers will only be representable as separate RRs within the RRSet. I'm not aware of a use case that would motivate putting multiple port numbers in one RR.

enygren commented 4 years ago

This has been incorporated.