[TLS] Review of draft-ietf-tls-openpgp-keys-08

The document looks mostly OK, but I think some details need 
clarification/polishing before we send this to the IESG.

Somewhat substantive comments:

1) Section 3.3: The TLS Certificate message defined in RFC4346 
   contains a list of X.509 certificates, while this section allows
   only a single PGPKey structure. IMHO we should keep this aligned
   with RFC4346, even though usually one key is enough. In other 
   words, something like this:

      struct {
          PGPKeyDescriptorType descriptorType;
          select (descriptorType) {
              case key_fingerprint: opaque fingerprint<16..20>; 
              case key:             opaque key<0..2^24-1>; 
          }
      } PGPCert;

      struct {
          PGPCert certificate_list<0..2^24-1>;
      } Certificate;

2) Section 3.5: "then a Certificate that contains an empty PGPKey
   should be sent": If the change suggested above is made, a 
   Certificate message that contains one zero-length PGPKey is 
   not the same thing as a Certificate message that contains zero 
   certificates (and it's the latter that should be sent).

3) Section 3.4: I don't think there is any need to define a new
   CertificateRequest message (the "certificate_authorities" part
   doesn't make much sense for OpenPGP, but RFC4346 clarified that
   it can be empty; and I believe many TLS 1.0 implementations also
   did this). So I'd suggest rewriting this section as follows:

   "If the certificate request message is sent, and the negotiated
   certificate type is OpenPGP, the certificate_authorities list 
   MUST be empty."

4) I'd suggest moving whole Section 2 to later in the document 
   (e.g., after Security Considerations), renaming it to "IANA
   Considerations", and rewriting it something like this:

   "This document defines a new TLS extension, "cert_type", assigned
   a value of TBD-BY-IANA (the value 7 is suggested) from the TLS
   ExtensionType registry defined in [RFC4366].

   This document establishes a new registry, to be maintained by
   IANA, for TLS certificate types. Initially, the registry contains
   the values 0 (X.509) and 1 (OpenPGP). Values from the range 2-223
   are assigned via IETF Consensus [RFC2434]. Values from the range
   224-255 are reserved for Private Use [RFC2434]."

5) Section 3.1: "...extension SHOULD be omitted if the client only
   supports X.509 certificates" and "Servers that only support X.509
   certificates MAY omit... ": Unnecessary options are a recipe for
   interoperability problems; unless there are good reasons to keep
   these, I'd suggest changing these to MUSTs.

6) Section 4: I'd suggest merging this section (only four lines)
   with Section 3.1. Also, the text should apply to all ciphersuites
   using the RSA/DHE_DSS/DHE_RSA key exchange methods, not just
   those in RFC 2246. (And with this change, the text about
   exportable ciphersuites is no longer necessary.)

Purely editorial comments:

7) Overall: I believe nowadays the RFC Editor recommends using 
   symbolic citations [RFC2119] instead of numeric ones [1].

8) Section 1, 2nd paragraph: extra comma before "provide"

9) Sections 3.6..3.8: I'd suggest removing these.  

10) Section 3.3: "The process of fingerprint generation is described 
   in OpenPGP [2]": I'd suggest adding a more specific pointer:
   Section 11.2 of [RFC2440].

11) Section 6.1: RFC 3546 is obsoleted by RFC 4366.

12) Section 6.1: Should include RFC 2119 and RFC 2434 here.

13) Section 6.2: Reference [5] is never cited in the text (and
   IMHO could be removed)

Best regards,
Pasi

_______________________________________________
TLS mailing list
TLS@lists.ietf.org
https://www1.ietf.org/mailman/listinfo/tls