Dale,
Thanks for the review. Responses inline. And, assuming Steve agrees I’ll
submit a version that incorporates these and other changes before the IESG does
its eval.
spt
On Dec 13, 2016, at 16:45, Dale Worley <worley(_at_)ariadne(_dot_)com> wrote:
Reviewer: Dale Worley
Review result: Ready with Nits
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.
Alvaro caught these too in his AD review so I’ve got them in the editor's copy
I’m working with.
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.”
It’s basically profile of a profile of a profile plus some stuff. I’m happy to
adopt your suggestion modulo s/must be/is.
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.
I think this is just a case of a missing “CA” in front of the word “context” so
tweaking it to: “…. that is unique to that CA context”. The certs only need to
be unique on a per CA basis the subject name does not need to be unique across
the whole of the RPKI. The combination of issuer+subject+serial # plus all the
parent certs provides the uniqueness.
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.
I think this was hold over from the CRMF request description; “and PublicKey”
can be dropped.
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.
Sure - how about:
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 that procedure), but using the
constraints applied come from this specification
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/
fair enough ;)
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?
Yes
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].
It’s a tangled web the SIDR drafts have weaved :) The latter.
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.
They are separate as ID.sidr-rfc6485bis is now RFC 7935. I think I agree we
should just refer to the bgpsec-pki-algs draft - it’s clearer.
(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}
You got it.
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.
It’s funny we say the RPKI and BGPsec algs are different at least twice. I’m
happy to change the wording here to make this paragraph easier to digest.
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.
Yep it’s a note for the para before the bullets so I fixed the ident.
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.
An algorithm suite refers to the signature and hashing algorithm used. We
could make it more explicit and change it to: … because the cryptographic
algorithms used to are different.
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?)
The RP needs to find it in the cert and then know that the value means do this
validation routine not the other one. I think identify works here because I’d
say all of the previous sentence is associating, which is a meaning of the word
identify.
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.
The validation constraints are found in s3.3 we could put a forward pointer in
here, but I think that might be overkill.
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/.
sigh - yep
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.
Key identifiers are used to build certification paths which is then used to
validate the signature. i.e., build up validate down. As an attacker, I can
insert a certificate with a collision that might get picked up and used when
validating the BGPsec signatures; it’ll obviously fail because the math won’t
work so this is really a DoS. How about something like the following:
Regardless, hash collisions are unlikely, but they are possible
and if detected an operator should be alerted. A subject key
identifier collision might cause the incorrect certificate to be
selected from the cache, resulting in a failed signature validation
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”.
Alvaro had a similar comment on the IANA considerations and he suggested the
first option.
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) }
This is great catch! And, yes the pkix(7) is missing.