Subject: [websec] draft-ietf-websec-key-pinning-20 feedback

From: Julian Reschke <[email-masked]>
Date: Tue, 26 Aug 2014 10:56:26 +0200

Hi there.

Some more quick feedback, somewhat unstructured...

Throughout: please say "header field" rather than "header".

 >    The "Public-Key-Pins" and "Public-Key-Pins-Report-Only" header
 >    fields, also referred to within this specification as the PKP and
 >    PKP-RO header fields, respectively, are new response headers defined
 >    in this specification.  They are used by a server to indicate that a

s/server/origin server/ maybe?

 >    Figure 1 describes the syntax (Augmented Backus-Naur Form) of the
 >    header fields, using the grammar defined in [RFC5234] and the rules
 >    defined in Section 3.2 of [RFC7230].  The field values of both header
 >    fields conform to the same rules.
 >
 >    Public-Key-Directives = [ directive ] *( OWS ";" OWS [ directive ] )
 >
 >    directive             = simple-directive
 >                          / pin-directive
 >
 >    simple-directive      = directive-name [ "=" directive-value ]
 >    directive-name        = token
 >    directive-value       = token
 >                          / quoted-string
 >
 >    pin-directive         = "pin-" token "=" quoted-string

1) I would recommend not to special-case pin-directive here, as it makes 
the ABNF ambiguous. Just put the additional requirements into prose.

2) The value of pin-directive ought to allow token syntax as well. 
(Otherwise a conforming parser will need to special-case their parsing 
which doesn't make any sense at all).

 >        given header field.  Directives are either optional or required,
 >        as stipulated in their definitions.

"OPTIONAL or REQUIRED", I assume?

 >    Additional directives extending the semantic functionality of the
 >    header fields can be defined in other specifications.  The first such
 >    specification will need to define a reistry for such directives.

registry.

 >    According to rule 5, above, the UA MUST ignore pin-directives with

Repeats a requirement. Maybe do not use MUST here; instead say "will".

 >    tokens naming hash algorithms it does not recognize.  If the set of
 >    remaining effective pin-directives is empty, and if the host is a
 >    Known Pinned Host, the UA MUST cease to consider the host as a Known
 >    Pinned Host (the UA should fail open).  The UA should indicate to

SHOULD?

 >    UAs SHOULD make their best effort to report Pin Validation failures
 >    to the report-uri, but may fail to report in exceptional conditions.

MAY? (in general: try to avoid lowercase RFC2119 terms)

Best regards, Julian