Last Call Review of draft-wallace-est-alt-challenge-04
review-wallace-est-alt-challenge-04-genart-lc-davies-2016-02-17-00

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-wallace-est-alt-challenge-04.txt
Reviewer: Elwyn Davies
Review Date: 2016/02/17
IETF LC End Date: 2016/03/07
IESG Telechat date: (if known) -



Summary: Almost ready.  There is one very minor issue and a number of 


nits. IDNITS identifies a downref (RFC 5912) but this is already in the 


downref registry (re RFC 5913) so no further action is needed, I believe.




Major issues:
None.

Minor issues:
s4, last sentence:



EST clients that include the
    "estIdentityLinking" attribute SHOULD NOT also include the
    "challengePassword" attribute.


I think this ought to be MUST NOT.  In any case, the behaviour of EST 


servers that receive a request with both challengePassword and 


estIdentityLinking defined should be more carefully specified. 


Similarly, I assume that the EST server should not respond to a 


/csrattrs request by specifying both of these attributes




Nits/editorial comments:


Abstract: s/PKCS #9 challengePassword attribute/challengePassword 


attribute defined in PKCS (Public-Key Cryptography Standards) #9 (RFC 2985)/




s1: s/PKCS/PKCS (Public-Key Cryptography Standards)/

s1: s/One-Time Password/one-time password/ (two places)



s1, para 1: Good to associate the acronym EST with its expansion here as 


well as in abstract: s/Enrollment over Secure Transport/Enrollment over 


Secure Transport (EST)/




s1: Expand SCEP on first (only) use (Simple Certificate Enrolment Protocol)



s1: Need to expand TLS on first use (apparently not a well-known 


acronym!  However HTTP is.)






s1, para 2: It would be useful to provide a reference for Certificate 


Signing Requests, as well as clarifying that CSR is the relevant acronym 


- one possibility might be "Certificate Signing Request (CSR; See, for 


example, CertificationRequestInfo in [RFC2986])".




s1, last para: s/is not intended to/are not intended to/

s3.1: s/upper bound/maximum length/ seems clearer to me.

s3.1, para 1: s/of the values/of the value/

s3.2, section title: Shouldn't this section be titled either:
     Alternative to PKCS #9 Challenge Password Attribute
or
     Revocation Challenge Attribute
?

s3.2, para 1, sentence 2: s/revocation Challenge/revocationChallenge/

s4: A reference for the definition of /csrattrs would be useful. Suggest:
OLD:
in the /csrattrs.
NEW:


in the HTTP control URI path /csrattrs (see Sections 3.2.2 and 4.5 of 


[RFC7030]).






s5: I fear that you ought to provide the full titles of RFC 4226 and RFC 


6238 to expand HOTP and TOTP ( or just omit the acronyms, since they are 


in the reference expansions.)






s6: Probably ought to reference RFC 7107 as the registry defining RFC  


(it isn't referenced otherwise).






s6: Ought to have an RFC editor note to replace the TBDx references in 


s3 and Appendix A.






s7: IMO RFC 2985 and RFC 7030 are normative.  RFC 7107 could also be 


considered normative.




s7.2: AFAICS RFC 7299 is not referenced.

Non-issue:


s7.1: IDNITS complains that RFC 5912 is a downref.  However, RFC 5912 is 


already in the downref registry (re RFC 5913) and so should not need 


further downref approval I believe.