Re: [TLS] WG: New Version Notification for draft-bruckert-brainpool-for-tls13-00.txt

[Hubert Kario <[e-mail]>]

On Tuesday, 4 September 2018 15:32:19 CEST Eric Rescorla wrote:
> On Tue, Sep 4, 2018 at 5:46 AM, Hubert Kario <[e-mail]> wrote:
> > On Monday, 3 September 2018 21:26:06 CEST Eric Rescorla wrote:
> > > On Mon, Sep 3, 2018 at 12:19 PM, Hubert Kario <[e-mail]> wrote:
> > > > On Monday, 3 September 2018 17:30:15 CEST Eric Rescorla wrote:
> > > > > On Mon, Sep 3, 2018 at 8:20 AM, Hubert Kario <[e-mail]>
> > 
> > wrote:
> > > > > > On Monday, 3 September 2018 17:15:24 CEST Eric Rescorla wrote:
> > > > > > > On Mon, Sep 3, 2018 at 7:28 AM, Hubert Kario <[e-mail]>
> > > > 
> > > > wrote:
> > > > > > > > On Monday, 3 September 2018 16:01:22 CEST Eric Rescorla wrote:
> > > > > > > > > On Mon, Sep 3, 2018 at 4:18 AM, Hubert Kario <
> > 
> > [e-mail]>
> > 
> > > > > > > > > wrote:
> > > > > > > > not
> > > > > > > > abort connection, so I still think it will create less
> > 
> > confusion
> > 
> > > > > > > > to
> > > > > > > > re-allow
> > > > > > > > them than to re-assign new codepoints
> > > > > > > 
> > > > > > > The issue is that it's not possible to distinguish a
> > 
> > non-compliant
> > 
> > > > TLS
> > > > 
> > > > > > 1.3
> > > > > > 
> > > > > > > implementation which is inappropriately sending these code
> > > > > > > points
> > > > 
> > > > from
> > > > 
> > > > > > > one which actually supports Brainpool with TLS 1.3. Using new
> > 
> > code
> > 
> > > > > > > points makes this clear.
> > > > > > 
> > > > > > and why having that distinction is that important?
> > > > > 
> > > > > Because otherwise you are risking interop problems:
> > > > > 
> > > > > 1. A stack which supports TLS 1.2 and TLS 1.3 but only supports
> > > > > Brainpool
> > > > > for TLS 1.2 (the only kind you can write at this point), and
> > > > 
> > > > inappropriately
> > > > 
> > > > > advertises the Brainpool curves in violation of the MUST above.
> > > > > 2. A stack which supports TLS 1.2 and TLS 1.3 and supports Brainpool
> > 
> > for
> > 
> > > > > both (assuming that we adopt your proposal and reactivate these code
> > > > > points).
> > > > > 
> > > > > If stack 2 receives a CH from stack 1 and responds by selecting a
> > > > 
> > > > Brainpool
> > > > 
> > > > > curve, then there will be an interop issue when it sends an HRR [0]
> > > > > selecting
> > > > > the Brainpool curve.
> > > > > 
> > > > > -Ekr
> > > > > 
> > > > > [0] I'm assuming that the client doesn't offer a Brainpool KeyShare.
> > > > 
> > > > ah, yes, missed this case. That does taint all those codepoints for
> > > > TLS
> > > > 1.3
> > > > 
> > > > but while the server may abort the connection upon receiving them in
> > 
> > TLS
> > 
> > > > 1.3
> > > > CH (as it is violation of the MUST clause), I don't think it actually
> > > > should
> > > > abort it...
> > > > 
> > > > For one, and I think we can agree on that, is the server MUST ignore
> > 
> > them
> > 
> > > > if
> > > > it doesn't support them in TLS 1.2.
> > > 
> > > I don't think I agree with this. Why would that be the case?
> > 
> > because when servers don't do it we have "TLS version intolerance", "TLS
> > extension intolerance" and so on.
> > 
> > From RFC 5246:
> >    If the list contains cipher
> >    suites the server does not recognize, support, or wish to use, the
> >    server MUST ignore those cipher suites, and process the remaining
> >    ones as usual.
> > 
> > and:
> >    the rules specified in [TLSEXT]
> >    require servers to ignore extensions they do not understand.
> > 
> > Not to mention the most explicit statement on this topic, from RFC 8446:
> >    Future extensions or additions to the protocol may define new values.
> >    Implementations need to be able to parse and ignore unknown values
> >    unless the definition of the field states otherwise.
> > 
> > In general, if the servers weren't required to ignore values they don't
> > recognise, we would never be able to extend the value lists (be it
> > SupportedGroups, or SignatureSchemes/SignatureAlgorithms).
> 
> This isn't an unknown value. It's a specifically prohibited value. Those
> are different cases.

yes, that's why I said "doesn't support them in TLS 1.2.". I meant there the 
protocol in general, not a specific implementation (i.e. a server that 
implements TLS 1.2 as the highest version)

> > > > Given that TLS 1.3 server usually implement both TLS 1.2 and TLS 1.3,
> > > > having
> > > > code that does ignore them in TLS 1.2 and doesn't ignore them in TLS
> > 
> > 1.3
> > 
> > > > is
> > > > only inviting bugs.
> > > 
> > > We already have other special case code that enforces such rules. For
> > > instance,
> > > 
> > > compression:
> > >       For every TLS 1.3 ClientHello, this vector
> > >       MUST contain exactly one byte, set to zero, which corresponds to
> > >       the "null" compression method in prior versions of TLS.  If a
> > >       TLS 1.3 ClientHello is received with any other value in this
> > >       field, the server MUST abort the handshake with an
> > >       "illegal_parameter" alert.  Note that TLS 1.3 servers might
> > >       receive TLS 1.2 or prior ClientHellos which contain other
> > >       compression methods and (if negotiating such a prior version) MUST
> > >       follow the procedures for the appropriate prior version of TLS.
> > 
> > the difference is that we we don't want people to use compression in TLS
> > 1.2,
> > while brainpool in TLS 1.2 is just not recommended, not provably dangerous
> > even when implemented correctly
> 
> That's reading stuff into the spec that's not there. They're both
> prohibited from
> being offered by TLS 1.3 clients.

Yes, it's not there, and I already agreed that it's acceptable for a server to 
abort when it sees such behaviour. But the reason why a behaviour (any 
behaviour) is prohibited influences the urgency of the fix (or presence of a 
check).
-- 
Regards,
Hubert Kario
Senior Quality Engineer, QE BaseOS Security team
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkyňova 115, 612 00  Brno, Czech Republic