Subject: [sidr] AD Review of draft-ietf-sidr-delta-protocol-04
From: "Alvaro Retana (aretana)" <[email-masked]>
To: "[email-masked]" <[email-masked]>
Cc: Chris Morrow <[email-masked]>, "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>
Date: Tue, 20 Dec 2016 13:15:36 +0000
List-Id: Secure Interdomain Routing <sidr.ietf.org>

Dear authors:

Hi!  I just finished reading this document.

I have several comments (please see below); I marked many of them as “Major”, some because of the use of Normative language, but my main concern is that I think error conditions in the protocol are underspecified (see M7, M8, M10, below).  Along the same lines, I think that an Operations Considerations section would be of benefit (see M8 below); you might also want to take a look at RFC5706 (Guidelines for Considering Operations and Management of New Protocols and Protocol Extensions).

I would like to see the error conditions comments addressed before moving this document forward to IETF Last Call.

Thanks!!

Alvaro.

Major:

M1.  I assume that the id-ad-rpkiNotify value was assigned through the early allocation process.  Instead of pointing at the registry in Section 3.2, point at the IANA Consideration Section --- and there, please remind IANA of the early allocation and request the update.

M2. Section 3.2. (Certificate Authority Use): “Relying Parties that do not support this delta protocol MUST NOT reject a CA certificate merely because it has an SIA extension containing this new kind of AccessDescription.”  By definition, an RP that has never even considered this document will not support the delta protocol – IOW, trying to specify the behavior of RPs that may have never even seen this document makes no sense to me, and can’t be enforced.  What is the current behavior when an RP receives the extra SIA extension?

M3. Section 3.3.2. (Publishing Updates): “Whenever the repository server receives updates from a CA it SHOULD generate new snapshot and delta files.  However, if a publication server services a large number of CAs it MAY choose to combine updates from multiple CAs.  If a publication server combines updates in this way, it MUST NOT postpone publishing for longer than one minute.”  The “MUST NOT” at the end (making it mandatory to publish) doesn’t work with the “SHOULD” at the beginning, which has no time constraint and opens to the door to a situation where no publishing is done.  Suggestion:
NEW>
  Whenever the repository server receives updates from a CA it MUST
   generate new snapshot and delta.  If a publication
   server services a large number of CAs it MAY choose to combine
   updates from multiple CAs, but it MUST NOT postpone publishing for longer than one
   minute.

M4. From 3.3.2. (Publishing Updates): “The update notification file SHOULD be kept small…older delta files that…will result in total size of deltas exceeding the size of the snapshot, MUST be excluded.”  Here the “SHOULD” and the “MUST” are in contradiction: you either do it always (MUST) or there may be cases when you don’t (SHOULD).  s/SHOULD/should

M5. Section 3.4.1. (Processing the Update Notification File) says that “a Relying Party (RP)…SHOULD prefer to use this protocol as follows.”  I think you really want to be explicit: s/SHOULD prefer to use/SHOULD use

M6. 3.4.1. (Processing the Update Notification File): “The RP SHOULD download the update notification file, unless an update notification file was already downloaded and processed from the same location in this validation run.”  Is there any other reason for the file not to be downloaded?  Why would the RP decide not to (“SHOULD”)?  If there are no other reasons, then why isn’t the “SHOULD” a “MUST”?

M7. 3.4.1. (Processing the Update Notification File): “MUST issue an operator error”  What is an “operator error” and how do you issue one?  I didn’t see an error definition in the schema.

M8. 3.4.1. (Processing the Update Notification File): “If neither update notification file and one snapshot file or delta files could be processed…SHOULD use an alternate repository retrieval mechanism if it is available.”  This “SHOULD” doesn’t define anything normative with respect to the Delta Protocol.  I think that this document would benefit from a short Operation Considerations section; a place to indicate expectations of the operators, potential issues, the fact (as expressed in this piece of text) that an alternate mechanism should be enabled (or at least considered), other considerations related to logging errors for the operator (see above), etc.

M9. There are some places where non-normative and normative (RFC2119) language is used that may cause confusion.  For example, in 3.4.2: “should perform the following actions”, the actions contain a set of “MUSTs” and “SHOULDs”.  Please find an alternate way to write the lead-in header to avoid confusion.  Suggestion: s/it should perform the following actions/it performs the following actions.  [Note that the same construct if used in several different places…]

M10. In several places (related to verification, but in other places as well), “the file MUST be rejected” is used.  What does this mean exactly?  Are there actions that should be taken as a result?  An example of why I’m asking: an RP will download and process delta files based on information in a notification file – if a delta file is not well-formed (for example), then the validation will fail and the RP MUST reject it – there are multiple reasons why the file may not be well-formed, but the bottom line is that whatever information was there that the repository pointed to in the notification message is lost (?) – does this mean that the RP is now out of sync?  Should the RP now try and re-sync using the snapshot file?  What if that is also rejected?  Maybe the notification file was somehow corrupted and pointed at the wrong file(s)…should the relationship with the repository be reset/restarted?   It is not clear at all what should be done, or what the implications are of these error conditions; it just looks like we reject and go on. [See comment M8 above: it looks like only when “neither update notification file and one snapshot file or delta files could be processed” that we would abandon using RRDP.]

M11. From 3.4.3. (Processing Delta Files): “it is RECOMMENDED that a RP uses additional strategies to determine if an object is still relevant for validation before removing it from its local storage”.  Ok, like what?

M12. 3.5.1.2. (Update Notification File): “…the repository server MUST ensure that this file is not cached for longer than 1 minute.  An exception to this rule…”  Using “MUST” implies no exceptions.  s/MUST/SHOULD    s/then no/than no

M13. Why isn’t an IETF namespace [RFC3688] used in the XML schema?  I would strongly suggest that you use one and request it in the IANA Considerations Section.  Unlike the publication protocol, this document specifies version 1 – which of course doesn’t mean there isn’t a longer history behind it, so I’m open to keeping a non-IETF namespace if that is the case.

M14. 3.5.2.3: “Note that the publish element is defined in the publication protocol [I-D.ietf-sidr-publication]”  But the example doesn’t correspond to the definition in I-D.ietf-sidr-publication, even if it does comply with the schema in this document.  Should the publish element behave the same way as specified in I-D.ietf-sidr-publication?  It is good that the RRDP design goes back to the Publication Protocol, but if it doesn’t use the exact definition from there (including the schema), then the differences should be highlighted here.

M15. 3.5.3.3: “Note that the publish and withdraw elements are defined in the publication protocol [I-D.ietf-sidr-publication]”  Same comment as above.

M16. Section 5. (Security Considerations): “RRDP replaces the use of rsync by HTTPS…”. Given the statement in 3.4.1 about using “an alternate repository retrieval mechanism” and the rest of the text in this section, I would assume that the intent is not really to “replace the use of rsync” (with an update to RFC6480).   Maybe I’m reading too much into the phrase above, but please clarify.

Minor:

P1. In 3.3.1. (Initialisation): “Note that this snapshot file MAY contain zero publish elements at this point if no objects have been submitted for publication yet.”  This text is not describing an option: s/MAY/may

P2. Please expand RRDP in first use.

P3. Section 3.4.4. (Polling the Update Notification File) says: “A detailed description of the validation process itself is out of scope of this document.”  Isn’t that what is described in 3.4.1 and 3.5.1.3??

P4. “version 4 UUID”  Please provide a reference.

P5. 3.5.1.3: “The serial attribute must be…”  This is the only place where RFC2119 language is not used in this section.  Any special reason?

P6. 3.5.1.3: “If delta elements are included they MUST form a contiguous sequence of serial numbers…up to the serial number mentioned in the notification element.”  The example in this section lists the serial numbers in “reverse order” (3, 2) – is there an order requirement?  The text seems to imply it, but it could go either way.

P7. The following references should be Informative: IANA-AD-NUMBERS, RFC6481, RFC6486, RFC6488 should be made Informative.

Nits:

N1. In both 3.3.1/3.3.2, the notes about “format and caching concerns” would work better if they are part of the previous paragraph.

N2. s/are no no longer/are no longer