Manifests for the Resource Public Key Infrastructure (RPKI)
draft-ietf-sidrops-6486bis-11

Thank you for the work on this document

I have an easy to fix point that should be address before publication (although it probably does not rise to the level of DISCUSS). I also have a number of minor comments (answers will be appreciated) and nits.

Francesca

# Major

1. -----

   objects from a successful fetch.  This implies that the RP MUST not

FP: This should be "MUST NOT" (rather than "MUST not")

# Minor

2. -----

FP: as noted by the shepherd, there are some references to fix:

  * RFC 6485 needs to be replaced with RFC 7935

The following references need to be removed as they are not used.

  == Unused Reference: 'RFC6482' 
  == Unused Reference: 'RFC6493' 
  == Unused Reference: 'RFC8209' 
  == Unused Reference: 'RFC8488'  (this was even last called as downref)

3. -----

      monotonically for each newly-generated Manifest.  Each RP MUST
      verify that a purported "new" Manifest contains a higher
      manifestNumber than previously-validated Manifests.

FP: This should contain a sentence about what the RP has to do if verification fails (I expect ignore the "new" Manifest). Same comment for the thisUpdate field:

      Manifest.  Each RP MUST verify that this field value is greater
      (more recent) than the most recent Manifest it has validated.

And for the text in Section 5.1:

       Note: An RP MUST verify all mandated syntactic constraints, i.e.,
       constraints imposed on a CA via a "MUST".

       publication point that is described by the manifest.  (RPs are
       required to verify the CMS signature.)

4. ------

      nextUpdate time.  Each manifest encompasses a CRL, and the
      nextUpdate field of the manifest SHOULD match that of the CRL's
      nextUpdate field, as the manifest will be re-issued when a new CRL

FP: What are the exception cases where it is expected not to match (i.e. what is the reason for the SHOULD here)?

5. -----

   The RP MUST check that the current time (translated to UTC) is
   between thisUpdate and nextUpdate.  If the current time lies within

FP: I am not sure the parenthesis needs to be there - this seems like an unnecessary implementation detail.

# Nits

6. -----

  *  all published signed objects that are verifiable using EE

FP: Please expand all acronyms (EE, OID) on first use

7. -----

   termed a "one-time-use" EE certificate [RFC6487]

FP: missing period

The response to question 7 of the shepherd writeup doesn't really answer the question being asked.  Also, please tidy up the stuff identified in the answer to question 11.

Section 4.2.1:

* "... this field is more recent any previously-generated ..." -- s/recent/recent than/

Appendix B:

* Since this is obsoleting, not updating, RFC 6486, I think this is more like "Changes Since RFC 6486".

I support Ben Kaduk's DISCUSS position.

** Section 8.  The Section 4.2.1 guidance that “Each RP MUST verify that a purported "new" Manifest contains a higher manifestNumber than previously-validated Manifests” is crucial to prevent a replay-like attack.  What is the recovery procedure if an RP loses that state (i.e., “forgets” what the last manifestNumber was for a given publication point)?  Should it be TOFU (trust whatever is the first value it downloads next)?

** Editorial
-- Section 1.  In the spirit of inclusive language consider replacing “man-in-the-middle”.  For example, s/man-in-the-middle attack on the retrieval/an on-path attack during the retrieval/

-- Section 4.2.1.  Editorial.  In the thisUpdate field, s/is recent any/is more recent than any/

Thanks for the efforts on this specification. It was a good read.

After reading the shepherd's writeup I was expecting an update to 6486 but the document actually obsoletes 6486. Something happened between when the shepherd writeup was written and when this doc went for publication, not captured. Not a big deal I guess, would have been better to get this captured.

Nits: 

   Section 4.2.1 : "The issuer MUST ensure that the value of this field is more recent any previously-generated Manifest", missing "than" ?

Thank you for the work put into this document. 

Please find below one non-blocking COMMENT points.

Special thanks to Chris Morrow for the shepherd's write-up including the section about the WG consensus (even if I would have appreciated a justification for the PS status). 

I hope that this helps to improve the document,

Regards,

-éric

-- Abstract --
In "then the RP can detect "stale" (valid) data", is "valid" really the right word to use ? I would naively expect "invalid". Or is it just an indication that the data *was* valid and is stale? The use of "(.*)" in the abstract was more to explain the previous word and this use is different and could confuse the reader.

I support Ben's DISCUSS.

Clearing DISCUSS on expectation of the additional change discussed with the
authors.  Thanks for being responsive to all the comments and working to
get the right thing documented!

Using lowercase "not" together with an uppercase RFC2119 keyword is not
acceptable usage. Found: "MUST not"

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term "man"; alternatives might be "individual", "people", "person".

-------------------------------------------------------------------------------
All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 5.2. , paragraph 5, nit:
> ion point to effect this operation. Thus the term "fetch" refers to an operat
>                                     ^^^^
A comma may be missing after the conjunctive/linking adverb "Thus".

Section 11.1. , paragraph 11, nit:
> Appendix B. Changes to RFC 6486 In 2019 it came to light that multiple Relyin
>                                    ^^^^
A comma is probably missing here.

Uncited references: [RFC6493], [RFC6482], [RFC8209], and [RFC8488].

Reference [RFC6485] to RFC6485, which was obsoleted by RFC7935 (this may be on
purpose).

Thanks for this document, just a couple of minor comments:

1) Section 4.2.1. Manifest

   nextUpdate:
      This field contains the time at which the next scheduled manifest
      will be issued.  The value of nextUpdate MUST be later than the
      value of thisUpdate.  The specification of the GeneralizedTime
      value is the same as required for the thisUpdate field.

      If the authority alters any of the items that it has published in
      the repository publication point, then the authority MUST issue a
      new manifest.  Even if no changes are made to objects at a
      publication point, a new manifest MUST be issued before the
      nextUpdate time.  Each manifest encompasses a CRL, and the
      nextUpdate field of the manifest SHOULD match that of the CRL's
      nextUpdate field, as the manifest will be re-issued when a new CRL
      is published.  When a new manifest is issued before the time
      specified in nextUpdate of the current manifest, the CA MUST also
      issue a new CRL that revokes the EE certificate corresponding to
      the old manifest.

Although this last sentence is not wrong, am I right in thinking that this sentence isn't specific to when the manifest is issued, i.e., isn't it the case that that CA MUST issue a new CRL whenever a new manifest is issues for any reason (e.g., as per 5.1, step 2)?  If so, perhaps this sentence could be tweaked to make that clearer.

2) 5.2.  Considerations for Manifest Generation

   A new manifest MUST be issued and published before the nextUpdate
   time.

Should any guidance be given about how far before the nextUpdate time a new manifest should be issued.  E.g., is publishing right at the nextUpdate time (e.g., 1 millisecond before) sufficient , or does it make sense to publish it a bit earlier than the nextUpdate time?

Nits:

4.1.  eContentType

   The eContentType for a manifest is defined as id-ct-rpkiManifest and
   has the numerical value of 1.2.840.113549.1.9.16.1.26.

Would numerical object identifier, or numerical OID, be better than numerical value here?

4.2.  eContent

I would have preferred for "file" to be "filename" in the structure, but I presume that this can't be changed at this stage anyway ...

Regards,
Rob