[sidr] AD Review of draft-ietf-sidr-rpki-validation-reconsidered-07

["Alvaro Retana (aretana)" <[e-mail]>]

Dear authors:

I just finished reading this document.  My review is predicated on the assumption that the intent of the WG is to define an additional validation process, and not amend/change/update/deprecate the existing one…yet, which is why there are not only process changes specified, but also new OIDs.

As written, with Updates and text “to be used in place of” existing specifications, the document achieves the deprecation of the current (old) mechanism: simply because procedurally you are replacing the existing/old text with the new one…and the old one would then not be anymore.  The result then is not coherent with the assumption above, and we will need to rewrite (at least from an intent point of view) the document before progressing.

I think that the easiest way forward would be for this document to say something like: “a new validation mechanism is specified, which uses these new OIDs – the specifications defined in RFCA, RFCB, etc.. apply, except for the following exceptions/changes.”  You should then be able to use most of the substantive text already written.

Please see below for specific comments – including more on why this document should not Update the existing RFCs.

Thanks!

Alvaro.



Major:

M1. Section 4.2.1. (Changes to RFC6484).  This document requests an assignment of a new OID (id-cp-ipAddr-asNumber-v2); both the reference and the policy to be followed are contained here, and not in RFC6484.  IOW, this document shouldn’t Update RFC6484.  Instead, it should request the new OID and simply reference RFC6484 when pointing back to id-cp-ipAddr-asNumber.


M2. Section 4.2.2. (Changes to RFC3779).  Same comment: this document requests the assignment of new OIDs….  Note that if the changes in Section 4.2.2.1. (OID for id-pe-ipAddrBlocks-v2) and Section 4.2.2.3. (OID for id-pe-autonomousSysIds-v2) were to be used in place of the text in RFC3779, then id-pe-ipAddrBlocks and id-pe-autonomousSysIds would end up being deprecated.

M2.1. [minor] The syntax for id-pe-ipAddrBlocks-v2 and id-pe-autonomousSysIds-v2 (4.2.2.2 and 4.2.2.4) use the “v1” names.

M2.2. For Sections 4.2.2.5 and 4.2.2.6, I think that what you want to say is (something like): “when the OIDs defined in this document are used, the procedures in RFC3779 are followed, except for the certificate path validation (section 2.3), which is specified in Section 4.2.4.4 of this document, and…”  Again, if the text was to replace what is currently in RFC3779, then those procedures would be deprecated.

M2.3. Same comment for Section 4.2.2.7: amending the specification in RFC3779 results in loosing what is defined there.


M3. Section 4.2.4. (Changes to RFC6487).  Same comments as above: replacing the text (which is what an Update does), would result in the mechanism specified in RFC6487 to be the same as what is specified in this document…which means that the option in the first paragraph (“…MUST issue certificates either as specified in [RFC6487] or with all the amendments specified in the following sections.) would result in the same thing.  As with Section 4.2.2 (above), I think you really should specify what the exceptions are for the extensions defined in this document (and not Update RFC6487).


M4. Section 4.2.4.4. (Amended Resource Certificate Path Validation).

M4.1. [Comment about RFC6487] This document correctly mentions the NotBefore/NotAfter values (which should really be notBefore/notAfter), instead of From/To (in RFC6487).  It seems to me that this is an error in the original text.  Can you please file an Errata against RFC6487?

M4.2. s/Certificate x contains all the extensions that MUST be present/Certificate x contains all the required extensions    The “MUST” is not normative in this context.

M4.3. [minor] s/MUST be satisfy the constraints/MUST satisfy the constraints

M4.4. “If IP Address Delegation extension is present, it is replaced with the intersection of the values from that extension and the current value of the VRS-IP.”  What is replaced, the IP Address Delegation extension or the VRS-IP?  If the extension, what does it mean to replace the extension in the certificate?   Note that the text about the AS Identifier Delegation extension is as confusing.

M4.5. “…these values MAY be stored along with the certificate, to facilitate incremental validation…”  This document (and RFC6487) don’t say anything about where to store anything – I don’t think we need that normative “MAY”.   s/MAY/may

M4.6. This text is not needed: “If certificate x uses the original version of [RFC6487], then certificate x MUST be rejected.”

M5. Section 4.2.5. (Changes to RFC6482).  Same comment about not needing an Update/replacing the text/…


M6. Section 5. (Deployment Considerations).  The timeline in Table 1 shouldn’t appear in this specification.  Migration is an operational issue that the community should coordinate separately (and not by specifying a normative timeline in an RFC).  What should be included here are any other migration considerations that should be observed when making the transition – if there’s anything beyond what you already wrote.


M7. Section 6. (Security Considerations). Please point to the security considerations in RFC3779, RFC6482, RFC6484 and RFC6487 as applying here too.  It might be beneficial to include a short discussion of why not immediately rejecting the over-claiming certificates is not a security issue


M8. IANA Considerations.   TBD4/TBD5 are not for IPAddrAndASCertExtn-*, but for the new id-mod-ip-addr-and-as-ident-*.

M9. References
- Please add a reference to RFC2119 and the corresponding boilerplate.
- I don’t think that the reference to RFC6268 needs to me Normative.
- We don’t need references to RFC3849, RFC5398 or RFC5737.
- RFC3280 was Obsoleted by RFC5280



Minor:

P1.  The example in Section 3 is meant to be a continuation from the example in Section 2 (“Certificate 2 from the previous example was re-issued by TA to CA1 and the prefix 198.51.100.0/24 was removed.  However, CA1 failed to re-issue a new Certificate 3 to CA2.”); but Certificate 3 is not the same as the one shown in Section 2.  I think the example still gets the over-claiming point across, but it is “wrong” in that both Certificate 2 and Certificate 3 (not just Certificate 2) would have to change for the problem to happen.

P2, Also in the example in Section 3.  It might be beneficial for the reader if the EE certificate was also marked as invalid; the text says so, but the list of certificates doesn’t.

P3. In several places “[this document]” is used – if the intent is to refer to a specific Section, please indicate which.  If not, then you should get rid of the brackets ([]).

P4. In 4.3: “ROA1 is considered valid because the prefix matches the Verified Resource Set on the embedded EE certificate, as required by RFC 6482.”  The VRS is introduced in this document, not RFC6482.



Nits:

N1. “This document proposes…” and “The changes proposed here…”.  We’re past the proposal phase: “This document specifies…”

N2. s/ maintaining Verified Resource Set/ maintaining a Verified Resource Set

N3. s/ the loss of on IP address prefix/ the loss of one IP address prefix

N4. s/ the 1988 ASN.1 modules for which we provided an update in Section 4.2.2.7 remain the normative version/ the 1988 ASN.1 modules in Section 4.2.2.7 remain the normative version

N5. s/ Furthermore we wish to note that operators MAY issue separate BGPsec Router Certificates for different ASNs/ Operators MAY issue separate BGPsec Router Certificates for different ASNs