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. Document: draft-ietf-sidr-bgpsec-pki-profiles-18 Reviewer: Dale R. Worley Review Date: 12 Dec 2016 IETF LC End Date: 19 Dec 2016 IESG Telechat date: unknown Summary: This draft is basically ready for publication, but has nits that should be fixed before publication. Some of these items appear to be technical, but I suspect that the intended meanings are clear to people well-versed in PKI and are known to work. However, they are unclear to a naive reader. 2. Describing Resources in Certificates The RIR, in turn, issues a CA certificate to an Internet Service Providers (ISP). s/Providers/Provider/ The CA also generate. The CA also generates Certificate Revocation Lists (CRLs). The first "The CA also generate." is extraneous. 3.1 BGPsec Router Certificate Fields A BGPsec Router Certificate is a valid X.509 public key certificate, consistent with the PKIX profile [RFC5280], containing the fields listed in this section. This profile is also based on [RFC6487] and only the differences between this profile and the profile in [RFC6487] are specified below. I suspect this paragraph is going to cause implementers some trouble. What, exactly, are the constraints on the BGPsec Router Certificate? It looks like the certificate must conform to: - X.509 - RFC 5280 - RFC 6487 as modified by "below" and I see that RFC 6487 requires that certificates conform to RFC 5280. So it seems that the first two items are directly required by this document and transitively required by RFC 6487. I suggest changing the first two items to be required only by transitivity, only mentioning X.509 and RFC 5280 as explanatory: A BGPsec Router Certificate is consistent with the profile in [RFC6487] as modified by the specifications in this section. As such, it must be a valid X.509 public key certificate and consistent with the PKIX profile [RFC5280]. Also, "below" is vague. I suspect you mean "The differences between this profile and the profile in [RFC6487] are specified in this section." 3.1.1.1. Subject However, each certificate issued by an individual CA MUST contain a Subject name that is unique within that context. What is "that context"? Do you mean: However, the certificates issued by an individual CA MUST contain unique Subject names. However that has difficulties when it comes time for the CA to issue new certificates with later validity times. Why there is a constraint based on "issued by an individual CA" is not clear, given that there is no constraint regarding which CA issues certificates to which routers. Merely aggregating the work of multiple CAs into one CA could require changing the subject names in the next revision of issued certificates, whereas splitting the work of one CA into multiple CAs would loosen this requirement. In addition, the definition of "an individual CA" is not clear. Is there really an operational requirement for this uniqueness constraint? More to the point, is the combination of common name (i.e. AS number) and serial number (router ID) required to be globally unique or not? That seems to be the only question that can be operationally significant. I suspect that someone well-versed in PKI knows these answers, but for the naive, what is required and why seems confusing. 3.2. BGPsec Router Certificate Request Profile o The SubjectPublicKeyInfo and PublicKey fields are specified in [ID.sidr-bgpsec-algs]. There is no "PublicKey field" discussed in ID.sidr-bgpsec-algs. Is "subjectPublicKey" intended? If so, "subjectPublicKey" seems to be a sub-field of SubjectPublicKeyInfo (per ID.sidr-bgpsec-algs section 3.1), which is also listed here, so it is not clear why it is mentioned individually here. 3.3. BGPsec Router Certificate Validation The validation procedure used for BGPsec Router Certificates is identical to the validation procedure described in Section 7 of [RFC6487] (and any RFC that updates this procedure), but using the constraints applied come from this specification. I assume you mean "and any RFC that updates the procedure of [RFC6487]". In that case, I think that "that procedure" would be required, but "the procedure of [RFC6487]" would eliminate any ambiguity. "but using the constraints applied come from this specification" is unclear. step 3: "the certificate contains all the field that must be present" This doesn't match the text in RFC 6487, despite claiming to be quoted: s/the certificate/The certificate/ s/all the field/all fields/ o BGPsec Router Certificate MUST include the "Subject Public Key Info" described in [ID.sidr-bgpsec-algs] as it updates [ID.sidr- rfc6485bis]. There is no "Subject Public Key Info" in ID.sidr-bgpsec-algs. Is "subjectPublicKeyInfo" intended? The construction "[ID.sidr-bgpsec-algs] as it updates [ID.sidr-rfc6485bis]" is awkward. Is "[ID.sidr-rfc6485bis] as updated by [ID.sidr-bgpsec-algs]" intended? If the latter construction is used, it is well-defined, though it means that the actual place to look for the description of "subjectPublicKeyInfo" is in [ID.sidr-bgpsec-algs]. Better, though, is to ask, What will this look like when the RFCs are published? Will [ID.sidr-bgpsec-algs] and [ID.sidr-rfc6485bis] be separate RFCs? If so, the desired format of "subjectPublicKeyInfo" will be described in one or the other of the RFCs, but (it seems) you could name just one of them. (It seems to me that if you have one draft modifying another draft, you should combine them into one draft, or move the modifying text into the draft it modifies.) NOTE: The cryptographic algorithms used by BGPsec routers are found in [ID.sidr-bgpsec-algs]. Currently, the algorithms specified in [ID.sidr-bgpsec-algs] and [ID.sidr-rfc6485bis] are different. BGPsec RPs will need to support algorithms that are used to validate BGPsec signatures as well as the algorithms that are needed to validate signatures on BGPsec certificates, RPKI CA certificates, and RPKI CRLs. I assume that there are two dichotomies: [ID.sidr-bgpsec-algs] vs. [ID.sidr-rfc6485bis] {the algorithms that are used to validate BGPsec signatures} vs. {the algorithms that are needed to validate signatures on BGPsec certificates, RPKI CA certificates, and RPKI CRLs} It would be easier on the reader if it was clear how these paired. E.g., NOTE: BGPsec RPs will need to support the algorithms in [ID.sidr-bgpsec-algs], which are used to validate BGPsec signatures, as well as the algorithms in [ID.sidr-rfc6485bis], which are needed to validate signatures on BGPsec certificates, RPKI CA certificates, and RPKI CRLs. or vice-versa. 4. Design Notes Note that this behavior is similar to the CA including the AS Resource Identifier Delegation extension in issued BGPsec Router Certificates despite the fact it is not present in the request. This text is indented as if it is a continuation of the immediately preceding bullet point. I can't tell for sure, but it seems to me that the text is actually a continuation of the paragraph containing the bulleted list, and so should be out-dented by two spaces from where it is now. 6. Security Considerations A BGPsec Router Certificate will fail RPKI validation, as defined in [RFC6487], because the algorithm suite is different. Different from what? Also, "algorithm suite" doesn't appear to be a defined datum in certificates; at least, it's not mentioned anywhere else in this document. Consequently, a RP needs to identify the EKU to determine the appropriate Validation constraint. I *think* this means that a RP needs to examine the EKU value to determine the algorithms that are used for [something]. (What does it mean to "identify" the EKU?) Also, this paragraph discusses "the certificate will fail validation as defined in 6487", and then talks about "the appropriate validation constraint", but it doesn't state that "the appropriate validation constraint" modifies the process in 6487. I suspect that the EKU value determines an algorithm suite (based on some unstated correspondence), which is then used to replace the algorithm suite demanded by some part of the 6487 process, and then after that, the certificate will pass the modified process. But the text doesn't assert that the certificate has to pass the modified process. I suspect that the intended meaning of this paragraph is obvious to anyone well-versed in PKI, but I don't think the words actually say that meaning. Hash functions [ID.sidr-bgpsec-algs] are used when generating the two key identifiers extension included in BGPsec certificates. I suspect s/key identifiers extension/key identifier extensions/. However as noted in [RFC6818], collision resistance is not a required property of one-way hash functions when used to generate key identifiers. Regardless, hash collisions are possible and if detected an operator should be alerted. The fact that "an operator should be alerted" suggests that if a hash collision happens it will cause an operational problem of some sort. What that problem is should be described and some bound stated for the amount of ensuing trouble. Conversely, if no operational problem can arise, then there is no reason to alert an operator. 7. IANA Considerations No IANA allocations are request of IANA, ... This should be "No IANA allocations are requested of IANA", or probably better "No allocations are requested of IANA". 9.2. Informative References Appendix A. ASN.1 Module id-kp OBJECT IDENTIFIER ::= { iso(1) identified-organization(3) dod(6) internet(1) security(5) mechanisms(5) kp(3) } Is this correct? I believe this should be id-kp OBJECT IDENTIFIER ::= { iso(1) identified-organization(3) dod(6) internet(1) security(5) mechanisms(5) pkix(7) kp(3) } RFC 3029 has id-kp-dvcs OBJECT IDENTIFIER ::= {iso(1) identified-organization(3) dod(6) internet(1) security(5) mechanisms(5) pkix(7) kp(3) 10} RFC 6494 has send-kp OBJECT IDENTIFIER ::= { iso(1) identified-organization(3) dod(6) internet(1) security(5) mechanisms(5) pkix(7) kp(3) } The IANA registry "smi-numbers" shows 1.3.6.1.5.5.7.3 as "Extended key purpose identifiers" with ...10 as "id-kp-dvcs". The IANA registry shows 1.3.6.1.5.5.3 as "PEM-Based IDUP Mechanism". Dale