[TLS] Re: [Technical Errata Reported] RFC8446 (8411)

[David Benjamin <[e-mail]>]

Yeah, there were a few rounds of people picking the worse-for-readers
convention and it was wrong then too. :-)

At the least, picking the other convention in one place is not an *error*
because they both describe the exact same protocol. I.e. we should not
declare this as an *error* in RFC 8446. Of course, it's not great to be
inconsistent, so rfc8446bis should change something. As the whole point of
specifications is to clearly describe a protocol and promote interop, I
think the choice is clear and it is not this one.

On Mon, May 12, 2025 at 4:59 AM Alicja Kario <[e-mail]> wrote:

> Specifying tight bound is already how TLS 1.3 is defined:
>
> CipherSuite cipher_suites<2..2^16-2>;
> SignatureScheme supported_signature_algorithms<2..2^16-2>;
> struct {
> ...
> Extension extensions<0..2^16-2>;
> } NewSessionTicket;
>
>
> On Friday, 9 May 2025 19:22:03 CEST, David Benjamin wrote:
> > FWIW, I don't think this is an error in the specification per
> > se. Both 2^16-2 and 2^16-1 to describe the exact same
> > structure, precisely because a length of 2^16-1 is impossible.
> > One isn't inherently more correct than the other. The question
> > is which is clearer, the loose bound or the tight bound.
> >
> > Indeed I would argue that the loose bound, 2^16-1, is a
> > _better_ way to describe the structure than the tight bound,
> > 2^16-2. (I would also argue that a minimum length of 1 is a
> > better way to describe it than 2, but that's less crucial.) The
> > bounds here communicate two things to the TLS implementor:
> >
> > 1. What size of length prefix to use, which is
> > implicitly determined by the upper bound
> > 2. What checks to apply on the length you've parsed out
> >
> > The key thing to keep in mind is that the size of the length
> > prefix already bounds the lengths (here 0..2^16-1), so any
> > checks you write from (2) are in addition to the bounds you get
> > from the length prefix. When the written bounds are equal to the
> > implicit bounds, you don't need to write a check. Where either
> > upper or lower bound are different (e.g. opaque
> > legacy_session_id<0..32>), you need to write an extra check.
> >
> > That is, you would parse opaque legacy_session_id<0..32> like this:
> >
> >   legacy_session_id = client_hello.read_u8_length_prefix()
> >   if len(legacy_session_id) > 32:
> >     raise DecodeError()
> >
> > But you would parse opaque extension_data<0..2^16-1> like this:
> >
> >   extension_data = extension.read_u16_length_prefix()
> >   # No need to check additional bounds because u16 is already <0..2^16-1>
> >
> > Now, the NamedGroup field is a u16-prefixed list of at least
> > one NamedGroup. We actually just want to parse it like this:
> >
> >   named_group_list = extension_data.read_u16_length_prefix()
> >   if len(named_group_list) == 0:
> >     raise DecodeError()
> >
> > But when we write it as NamedGroup named_group_list<2..2^16-2>,
> > it *looks like* you have to parse it like:
> >
> >   named_group_list = extension_data.read_u16_length_prefix()
> >   if len(named_group_list) < 2 or len(named_group_list) > 2**16 - 2:
> >     raise DecodeError()
> >
> > To know that these do the same thing you have to observe that:
> > 1. It is not possible to have a NamedGroup of length less than 2
> > 2. NamedGroup is always exactly two bytes, so it is not
> > possible to have a NamedGroup list of length 2^16-1.
> >
> > This is extra work for the implementer and makes it more likely
> > that they'll misunderstand it. Or perhaps they'll get used to
> > the TLSWG writing things in this weird way and miss a case when
> > they *do* need to enforce an upper bound. This is particularly
> > fraught because TLS structures are variable-length. Here is a
> > contrived example to demonstrate the silliness of this
> > convention:
> >
> > enum { len1000(1},  len30001(2) } Type;
> > struct TwoPossibleLengths {
> >     Type type;
> >     select (type) {
> >        case len1000: opaque padding[999];
> >        case len30001: opaque padding[30000];
> >     }
> > };
> >
> > struct {
> >   TwoPossibleLengths list<1000..65002>;
> > } Confusing;
> >
> > If you look at this field, you might think that this is
> > actually imposing some limit that is tighter than its length
> > prefix, and that you need to check more things. In reality, it's
> > not. It's fairly clear that the minimum length of
> > a TwoPossibleLengths is 1000. It's much less obvious that the
> > maximum length of a series of TwoPossibleLengths, constrained to
> > be under 2^16-1, is 65002. So I think this is a far, far better
> > way to write this same structure.
> >
> > struct {
> >   TwoPossibleLengths list<1..2^16-1>;
> > } MuchMuchClearer;
> >
> > It describes the exact same structure, but now it's obvious the
> > intended parse was that you check the length is non-zero and
> > leave the upper-bound set based on its length prefix. Now,
> > obviously this is a contrived example and that instance of
> > knapsack is quite a bit harder than the NamedGroup instance. But
> > I think it demonstrates why the simpler loose bound convention
> > is better for readers of the specification than the one that
> > relies on the reader to do more math.
> >
> > David
> >
> >
> > On Thu, May 8, 2025 at 8:54 PM Sean Turner <[e-mail]> wrote:
> > Paul,
> >
> > You can marked this one as “verified" if you want. I submitted
> > a PR to fix this in -rfc8446bis; see:
> > https://github.com/tlswg/tls13-spec/pull/1380
> >
> > spt
> >
> > On May 8, 2025, at 4:05 AM, RFC Errata System
> > <[e-mail]> wrote:
> >
> > The following errata report has been submitted for RFC8446,
> > "The Transport Layer Security (TLS) Protocol Version 1.3".
> >
> > --------------------------------------
> > You may review the report below and at:
> > https://www.rfc-editor.org/errata/eid8411
> >
> > --------------------------------------
> > Type: Technical
> > Reported by: Albin Johansson <[e-mail]>
> >
> > Section: 4.2.7
> >
> > Original Text
> > -------------
> > struct {
> >    NamedGroup named_group_list<2..2^16-1>;
> > } NamedGroupList;
> >
> > Corrected Text
> > --------------
> > struct {
> >    NamedGroup named_group_list<2..2^16-2>;
> > } NamedGroupList;
> >
> > Notes
> > -----
> > The specified maximum legal length of the named_group_list
> > vector in the NamedGroupList structure is 2^16-1 bytes. This is
> > invalid because NamedGroup is an enum that occupies two bytes,
> > but 2^16-1 is not an exact multiple of the element size (2
> > bytes), as required in Section 3.4. It appears that the intended
> > upper bound should be 2^16-2 bytes instead.
> >
> > Instructions:
> > -------------
> > This erratum is currently posted as "Reported". (If it is spam, it
> > will be removed shortly by the RFC Production Center.) Please
> > use "Reply All" to discuss whether it should be verified or
> > rejected. When a decision is reached, the verifying party
> > will log in to change the status and edit the report, if necessary.
> >
> > --------------------------------------
> > RFC8446 (draft-ietf-tls-tls13-28)
> > --------------------------------------
> > Title               : The Transport Layer Security (TLS)
> > Protocol Version 1.3
> > Publication Date    : August 2018
> > Author(s)           : E. Rescorla
> > Category            : PROPOSED STANDARD
> > Source              : Transport Layer Security
> > Stream              : IETF
> > Verifying Party     : IESG
> >
>
> --
> Regards,
> Alicja Kario
> Principal Quality Engineer, RHEL Crypto team
> Web: www.cz.redhat.com
> Red Hat Czech s.r.o., Purkyňova 115, 612 00, Brno, Czech Republic
>
>