[TLS] Review of draft-ietf-tls-rfc4366-bis-04

[Simon Josefsson <simon at josefsson.org>]

"Joseph Salowey (jsalowey)" <jsalowey at cisco.com> writes:

> This is a working group last call for comments on
> draft-ietf-tls-rfc4366-bis-04 prior to sending this document on to the
> IESG for publication as a proposed standard RFC.  The document and
> revision history can be found here:
>
> http://tools.ietf.org/html/draft-ietf-tls-rfc4366-bis-04
>
>
> Please send any comments to the list by May 15, 2009.  

Hi.  The document looks good in general, however I have two comments:

1)

RFC 4366 contains:

      struct {
          opaque url<1..2^16-1>;
          Boolean hash_present;
          select (hash_present) {
              case false: struct {};
              case true: SHA1Hash;
          } hash;
      } URLAndOptionalHash;

The draft changes the struct to:

      struct {
          opaque url<1..2^16-1>;
          opaque SHA1Hash[20];
      } URLAndHash;

An RFC 4366 implementation that receives a RFC4366bis struct will reject
the encoding as corrupt, and possibly refuse the connection.

Likewise, an RFC 4366bis implementations will reject a RFC4366 struct.

It is possible to implement a parser that will accept both structs, but
this is not discussed nor suggested by the document.

I suggest that a padding byte is inserted as follows:

      struct {
          opaque url<1..2^16-1>;
          opaque padding<1>;
          opaque SHA1Hash[20];
      } URLAndHash;

And some text is added:

   The "padding" byte MUST be 0x01.  It is present to make the structure
   backwards compatible.

TLS 1.2 also changed old structs, let's not make the same mistake again.

2)

There is an RFC Errata for RFC 4366:

http://rfc-editor.org/errata_search.php?eid=111

The issues are all either irrelevant or fixed, except for (2).  As far
as I can tell, the concern is valid.  The text needs to be modified to
replace "the server name" with "any of the server names".  The WG could
also consider Alfred's suggestion that clients needs indication of which
name the server accepted, but I'm not sure this is important.

/Simon