Re: [Sidrops] Francesca Palombini's No Objection on draft-ietf-sidrops-6486bis-09: (with COMMENT)

[Job Snijders <[e-mail]>]

Dear Francesca Palombini,

On Wed, Feb 02, 2022 at 05:06:54AM -0800, Francesca Palombini via Datatracker wrote:
> # Major
> 
> 1. -----
> 
>    objects from a successful fetch.  This implies that the RP MUST not
> 
> FP: This should be "MUST NOT" (rather than "MUST not")

You are right, a small oversight on proper capitalisation. I've updated
this for -10:

https://github.com/job/draft-ietf-sidrops-6486bis/commit/27928105481a16ef097ae69681950135ba23349f

> # 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)

The above has been addressed through
https://mailarchive.ietf.org/arch/msg/sidrops/EYxcUtZLF7go2UBnOoOXqwn6bRY/

> 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).

To resolve this comment I propose to add the following sentence to that
paragraph:

ADD:
    If the purported "new" Manifest contains an equal or lower manifestNumber than previously-validated Manifests, the RP SHOULD use locally cached versions of objects, as described in <xref target="sect-6.6"/>.

> 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.

To resolve this comment I propose to add the following sentence to that
paragraph:

ADD:
    If this field in a purported "new" Manifest is smaller (less recent) than previously-validated Manifests, the RP SHOULD use locally cached versions of objects, as described in <xref target="sect-6.6"/>.

> 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.)

I am not entirely sure what the question or comment is in the above
paragraph about section 5.1 - can you elaborate?

> 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)?

The RPKI is a globally deployed production system; this means the RPKI
operator community to be careful about what it can mandate from RPs
versus what it can mandate from CAs. Putting in a stronger mandate was
discussed in the working group: For example, I observed some time window
alignment issues in the wild. Detailed discussion in this thread
https://mailarchive.ietf.org/arch/msg/sidrops/VNG77j05I2JXOwv4qkSy-DCgRNE/

Aligning the Manifests eContent nextUpdate with the Manifest's EE
certificate's "Not After" with the associated CRL's "nextUpdate"
optimizes operations (it thwarts needless CRL growth); but deviation
from this recommendation for alignment does not negatively impact
cryptographic robustness. Hence, I believe a softer 'SHOULD' is
appropriate.

Does my elaboration help?

> 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.

I believe this implementation detail is useful: in this context the
ASN.1 Type "GeneralizedTime" is used, which allows specification of an
timezone offset. There are some seemingly thorny complications
related to Validity Time windows as described in
https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.5
Therefor I believe it is useful to remind implementers to normalize
towards Zulu.

> # Nits
> 
> 6. -----
> 
>   *  all published signed objects that are verifiable using EE
> 
> FP: Please expand all acronyms (EE, OID) on first use

Thanks! EE already was expanded, but I added capitaliation to improve
readability; I added expansion for OID. Did I miss any others?

> 7. -----
> 
>    termed a "one-time-use" EE certificate [RFC6487]
> 
> FP: missing period

thanks, added.

An overview of the changes related to this feedback can be found here:
https://github.com/job/draft-ietf-sidrops-6486bis/commit/124c15569a991083cf8b8ca3ac249d98c49d41ef

The changeset in patch(1) format is also available below.

Kind regards,

Job

commit 27928105481a16ef097ae69681950135ba23349f
Author: Job Snijders <[e-mail]>
Date:   Thu Feb 3 15:22:38 2022 +0000

    MUST not -> MUST NOT
    
    Feedback from Francesca Palombini
    
    https://mailarchive.ietf.org/arch/msg/sidrops/chrDRb2twlBi4zwOw6qG__SBYnk/

diff --git draft-ietf-sidrops-6486bis.xml draft-ietf-sidrops-6486bis.xml
index ed7de68..bd07ae4 100644
--- draft-ietf-sidrops-6486bis.xml
+++ draft-ietf-sidrops-6486bis.xml
@@ -565,7 +565,7 @@
 
 	<t>
         Termination of processing means that the RP SHOULD continue to use cached versions of the objects associated with this CA instance, until such time as they become stale or they can be replaced by objects from a successful fetch.
-        This implies that the RP MUST not try to acquire and validate subordinate signed objects, e.g., subordinate CA certificates, until the next interval when the RP is scheduled to fetch and process data for this CA instance.
+        This implies that the RP MUST NOT try to acquire and validate subordinate signed objects, e.g., subordinate CA certificates, until the next interval when the RP is scheduled to fetch and process data for this CA instance.
         </t>
 
 	</section>

commit 124c15569a991083cf8b8ca3ac249d98c49d41ef
Author: Job Snijders <[e-mail]>
Date:   Thu Feb 3 15:38:59 2022 +0000

    Add a sentence about what to do when the manifestNumber verification shows a lower or equal manifestNumber compared to previously-validated Manifests
    Add a sentence about what to do when the manifest validity window is older compared to previously-validated Manifests
    Expand acronyms
    Add missing period.
    
    Feedback from Francesca Palombini
    
    https://mailarchive.ietf.org/arch/msg/sidrops/chrDRb2twlBi4zwOw6qG__SBYnk/

diff --git draft-ietf-sidrops-6486bis.xml draft-ietf-sidrops-6486bis.xml
index d8b545c..3a5df83 100644
--- draft-ietf-sidrops-6486bis.xml
+++ draft-ietf-sidrops-6486bis.xml
@@ -97,7 +97,7 @@
    Authority (CA) publication points in repositories (directories
    containing files that are subordinate certificates and Certificate
    Revocation Lists (CRLs) issued by this CA and other signed objects
-   that are verified by end-entity (EE) certificates issued by this CA).</t>
+   that are verified by End-Entity (EE) certificates issued by this CA).</t>
 
 	<t>
    Manifests are modeled on CRLs, as the issues involved in detecting
@@ -153,11 +153,11 @@
 	<section title="Manifest Signing" anchor="sect-3"><t>
    A CA's manifest is verified using an EE certificate.  The
    SubjectInfoAccess (SIA) field of this EE certificate contains the
-   access method OID of id-ad-signedObject.</t>
+   access method Object Identifier (OID) of id-ad-signedObject.</t>
 
 	<t>
    The CA MUST sign only one manifest with each generated private key, and MUST generate a new key pair for each new version of the manifest.
-   This form of use of the associated EE certificate is termed a "one-time-use" EE certificate <xref target="RFC6487"/></t>
+   This form of use of the associated EE certificate is termed a "one-time-use" EE certificate <xref target="RFC6487"/>.</t>
 
 	</section>
 
@@ -236,6 +236,7 @@
       Conforming manifest issuers MUST NOT use number values longer than 20 octets.
       The issuer MUST increase the value of this field monotonically for each newly-generated Manifest.
       Each RP MUST verify that a purported "new" Manifest contains a higher manifestNumber than previously-validated Manifests.
+      If the purported "new" Manifest contains an equal or lower manifestNumber than previously-validated Manifests, the RP SHOULD use locally cached versions of objects, as described in <xref target="sect-6.6"/>.
 	</t>
 
 	<t hangText="thisUpdate:">
@@ -244,6 +245,7 @@
       This field has the same format constraints as specified in <xref target="RFC5280"/> for the CRL field of the same name.
       The issuer MUST ensure that the value of this field is more recent than any previously-generated Manifest.
       Each RP MUST verify that this field value is greater (more recent) than the most recent Manifest it has validated.
+      If this field in a purported "new" Manifest is smaller (less recent) than previously-validated Manifests, the RP SHOULD use locally cached versions of objects, as described in <xref target="sect-6.6"/>.
 	</t>
 
 	<t hangText="nextUpdate:">
@@ -617,7 +619,7 @@
 	<section title="Acknowledgements" anchor="sect-10"><t>
    The authors would like to acknowledge the contributions from George Michelson and Randy Bush in the preparation of the manifest specification.
    Additionally, the authors would like to thank Mark Reynolds and Christopher Small for assistance in clarifying manifest validation and RP behavior.
-   The authors also wish to thank Tim Bruijnzeels, Job Snijders, Oleg Muravskiy, Sean Turner, Adianto Wibisono, and Murray Kucherawy for their helpful review of this document.</t>
+   The authors also wish to thank Tim Bruijnzeels, Job Snijders, Oleg Muravskiy, Sean Turner, Adianto Wibisono, Murray Kucherawy, and Francesca Palombini for their helpful review of this document.</t>
 	</section>
 
 	</middle>