Subject: Re: [TLS] tls-flags: abort on malformed extension

From: Yoav Nir <[email-masked]>
Date: Thu, 7 Oct 2021 21:37:22 +0300

Since I prefer to have the discussion in a single place, I’m copying below a comment by David Benjamin from GitHub:

> On 28 Aug 2021, at 23:36, Yoav Nir <[email-masked]> wrote:
> 
> Hi.
> 
> To address Michael StJohns comment from 19-July, I submitted PR #12:
> 
> https://github.com/tlswg/tls-flags/pull/12 <https://github.com/tlswg/tls-flags/pull/12>
> 
> What is says is that any implementation receiving a malformed tls_flags extensions should abort the handshake. The text provides a list (which I hope is comprehensive) of all the ways this specific extension can be malformed. 
> 
> Please comment here or on the PR is this makes sense to everybody.

My proposed text:

>    An implementation that receives an invalid tls_flags extension MUST 
>    terminate the TLS handshake with a fatal illegal_parameter alert. 
>    Such invalid tls_flags extensions include: 
>     * zero-length extension
>     * Multiple tls_flags extensions for the same message
>     * A flag set in the tls_flags extension of the wrong message, as 
>       specified in the document for that flag     

David’s comment about the zero-length extension:
> If we want this to be invalid, we can put it in the TLS presentation language. Change FlagExtensions to:
> 
>       struct {
>          opaque flags<1..255>;
>       } FlagExtensions;

David’s comment about the multiple extensions:
> RFC8446 already prohibits this on the sender. We don't do a good job of defining the rules for the receiver, but that should probably be done uniformly across all extensions, rather than just in this one

David’s comment about the flag on the wrong message:
> This seems unimplementable. If I receive a message with a flag I don't recognize, I have no idea whether the flag is allowed in the message or not. Yet this text says I MUST enforce this rule. (There's probably some corresponding wording in RFC8446 we can borrow.)
> 
> Nit: It also seems better to phrase this in terms of the registry, rather than the document. We might have multiple documents for the flag if we have to update it.

OK, so now my response:

I agree with the first and second comments. About the third, what I meant was that a supported flag that is supposed to appear only in CH appears instead and CR, or more likely, a flag that should appear in EE apears in SH instead.

But I think the best way to resolve the issue is to remove the bullet point list and the last sentence before them, IOW: remove the examples.

If this is agreeable to everyone, I will modify it in my PR.

Yoav

---

From: Hubert Kario <[email-masked]>
Date: Mon, 11 Oct 2021 13:12:14 +0200

On Thursday, 7 October 2021 20:37:22 CEST, Yoav Nir wrote:
>
> OK, so now my response:
>
> I agree with the first and second comments. About the third, 
> what I meant was that a supported flag that is supposed to 
> appear only in CH appears instead and CR, or more likely, a flag 
> that should appear in EE apears in SH instead.
>
> But I think the best way to resolve the issue is to remove the 
> bullet point list and the last sentence before them, IOW: remove 
> the examples.

Clients aborting on unrecognised flags in SH or EE is expected, that's what
already happens for normal extensions, but yes, it's too strong for CH. 
Maybe
also NST.

-- 
Regards,
Hubert Kario
Senior Quality Engineer, QE BaseOS Security team
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno, Czech Republic

---

From: Ilari Liusvaara <[email-masked]>
Date: Mon, 11 Oct 2021 15:06:46 +0300

On Mon, Oct 11, 2021 at 01:12:14PM +0200, Hubert Kario wrote:
> > I agree with the first and second comments. About the third, what I
> > meant was that a supported flag that is supposed to appear only in CH
> > appears instead and CR, or more likely, a flag that should appear in EE
> > apears in SH instead.
> > 
> > But I think the best way to resolve the issue is to remove the bullet
> > point list and the last sentence before them, IOW: remove the examples.
> 

I presume flags should mirror how extensions behave (being essentially
compact encoding for zero-length extension), and TLS 1.3 specifies that
known unexpected extension in any message, including CH, causes abort
with illegal_parameter alert (Reference: RFC 8446, Section 4.2).

For unknown extensions, the rule is to ignore any unknown extension in
CH, CR and NST, and abort with unsupported_extension on any unknown
extension in SH, EE and CT (Reference: RFC 8446, Section 4.2).

And there already exists an extension that can trigger the first rule
in CH, namely oid_filters. So an implementation that supports that
extension is supposed to abort if it receives the extension in CH.

-Ilari

---

From: Yoav Nir <[email-masked]>
Date: Wed, 20 Oct 2021 23:11:59 +0300

Hi.

I updated the PR.  If there are no further objections, I will commit and submit a new version in time for the submission deadline.

Yoav

> On 7 Oct 2021, at 21:37, Yoav Nir <[email-masked]> wrote:
> 
>> On 28 Aug 2021, at 23:36, Yoav Nir <[email-masked] <mailto:[email-masked]>> wrote:
>> 
>> Hi.
>>

---

From: Sean Turner <[email-masked]>
Date: Thu, 21 Oct 2021 12:40:10 -0400

Yoav,

Thanks for moving this along.

spt

> On Oct 20, 2021, at 16:11, Yoav Nir <[email-masked]> wrote:
> 
> Hi.
> 
>>> https://github.com/tlswg/tls-flags/pull/12
>>> 
> _______________________________________________
> TLS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls