[TLS] AD review of draft-ietf-tls-grease-02

[Benjamin Kaduk <[e-mail]>]

In general this looks to be in good shape, though we do have a remaining
TODO item to rebase past the registry changes from RFCs 8446/9447.

I see the shepherd writeup says "we hear it is easy to publish RFCs" and
note with chagrin that I am not keeping up my end of the bargain.

My specific section-by-section comments are below.

Thanks!

-Ben


Section 1

   The TLS protocol [RFC8446] includes several points of extensibility,
   including the list of cipher suites and the list of extensions.  The
   values in these lists identify implementation capabilities.  TLS

We could probably make this text a little more precise (for one, there's
not a single list of extensions since many messages can carry
extensions).  So, maybe "the list of cipher suites and several lists of
extensions" and "The values transmitted in these lists"?

Section 2

Can we add an editorial note that values prefaced with "{TBD}" are
suggested values but subject to change prior to final allocation by
IANA?

   Future versions of TLS or DTLS [RFC6347] MUST NOT use any of the
   above values as versions.

Process-wise, this feels like an attempt to Update: the (D)TLS core
specs, which we can't do in an Informational document.  So it would
probably be better to say something "The values thusly allocated are no
longer available for use as version numbers by (D)TLS implemnetations".
Things are made somewhat awkward by there not being a registry of
protocol versions, sadly...

Section 3.1

Are there any of these for which we want to say "the client MUST NOT
advertise a list consisting solely of GREASE values"?  It would probably
be fine to do this for, say, key_share, but not for, say, cipher_suites.
But perhaps the reader will be smart enough to figure out what works
without prodding from us...

   Clients MUST reject GREASE values when negotiated by the server.
   Specifically, the client MUST fail the connection if a GREASE value
   appears any in the following:

I did not attempt to audit the (omitted) list for completeness; the
first sentence should cover any situations that are not specifically
listed, right?

Section 3.2

   When processing a ClientHello, servers MUST NOT treat GREASE values
   differently from any unknown value.  Servers MUST NOT negotiate any
   GREASE value when offered in a ClientHello.  Servers MUST correctly
   ignore unknown values in a ClientHello and attempt to negotiate with
   one of the remaining parameters.

Similarly to the above, we might consider adding a parenthetical noting
that there may not be any remaining valid parameters, and that's not
necessarily fatal.

   Note that these requirements are restatements or corollaries of
   existing server requirements in TLS.

(side note) Some future reviewers might complain about using normative
language to duplicate exisiting requirements from other documents; in
this case, I don't mind, myself.

Section 4.1

   o  A server MAY select one or more GREASE extension values and
      advertise corresponding extensions with varying length and
      contents.

nit: I don't think "corresponding" is quite the right word; maybe
"advertise those extensions"?

   o  A server MAY select one or more GREASE signature algorithm values
      and advertise them in the "signature_algorithms" extension.

I'm not necessarily expecting any action based on this comment, but I
note that status_request, signed_certificate_timestamp,
certificate_authorities, oid_filters, and signature_algorithms_cert are
also currently defined for CertificateRequest but we do not call out any
extension-specific greasing for them.  Of that list, only
signature_algorithms_cert seems like it might be calling out for special
handling, to me...

Section 4.2

   When processing a CertificateRequest or NewSessionTicket, clients
   MUST NOT treat GREASE values differently from any unknown value.
   Clients MUST NOT negotiate any GREASE value when offered by the
   server.  Clients MUST correctly ignore unknown values offered by the
   server and attempt to negotiate with one of the remaining parameters.

(following the theme) I don't remember any cases where the client can
succeed if the list becomes empty after pruning unknown values ... if we
are deciding that we want to say anything on this topic at all.

Section 5

   Implementations advertising GREASE values SHOULD select them at
   random.  This is intended to encourage implementations to ignore all
   unknown values rather than any individual value.  Implementations
   MUST honor protocol specifications when sending GREASE values.  For
   instance, implementations sending multiple GREASE values as
   extensions MUST NOT send the same GREASE value twice.

Feel free to tell me that I'm being internally inconsistent, but in this
case "MUST NOT send the same GREASE value twice" does not seem like a
good place to use normative language to restate an existing requirement.
So I'd rather see lowercase "must not" and possibly a section reference
to 8446 § 4.2 ("[t]here MUST NOT be more than one extension of the same
type in a given extension block.").

Section 6
   [[TODO: Update IANA considerations for TLS 1.3 and rebase over draft-
   ietf-tls-iana-registry-updates.]]

Can the shepherd please work with the author to make the needed changes?

IIRC the main change for TLS 1.3 is the "TLS 1.3" column for
extensiontype values.

Since this document is Informational, we have to be Recommended "N" for
everything.

Thanks for the note about the specific values listed being just
suggestions.