Last Call Review of draft-ietf-sidr-rpki-manifests-
review-ietf-sidr-rpki-manifests-secdir-lc-williams-2011-04-30-00

Request Review of draft-ietf-sidr-rpki-manifests
Requested rev. no specific revision (document currently at 16)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2011-04-26
Requested 2011-03-11
Authors Matt Lepinski, Stephen Kent, Geoff Huston, Rob Austein
Draft last updated 2011-04-30
Completed reviews Secdir Last Call review of -?? by Nicolás Williams
Assignment Reviewer Nicolás Williams
State Completed
Review review-ietf-sidr-rpki-manifests-secdir-lc-williams-2011-04-30
Review completed: 2011-04-30

Review
review-ietf-sidr-rpki-manifests-secdir-lc-williams-2011-04-30

[Resend, from the correct address.]

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the IESG.
These comments were written primarily for the benefit of the security
area directors. Document editors and WG chairs should treat these
comments just like any other last call comments.

This document is almost ready for publication. ÂMy comments are below.
A summary of my comments is:

Â- Text on versioning of the manifest ASN.1 would be useful.
Â- Handling of boundary conditions on manifest validation could use some
Âimprovement, such as regarding clock skew. ÂSee specific comments
Âbelow.
Â- Some "SHOULDs" could use some additional commentary regarding when
Âan implementation might act otherwise.
Â- Nits (typos, out of date references).

Nico


> 2. ÂManifest Scope
>
> Â Â[...]
> Â ÂWhere an EE certificate is placed in the Cryptographic Message Syntax
> Â Â(CMS) wrapper of a published RPKI signed object
> Â Â[ID.sidr-signed-object] there is no requirement to separately publish
> Â Âthe EE certificate in the CA's repository publication point.
> Â Â[...]

Any guidance on when to place EE certs in CMS wrappers and when not to?

BTW, the reference to ID.sidr-signed-object needs to be updated (the
current version is -03).

> 3. ÂManifest Signing
>
> Â ÂA CA's manifest is verified using an EE certificate The
                           ^
Missing period.
s/EE certificate The/EE certificate. ÂThe/

> Â ÂSubjectInfoAccess (SIA) field of this EE certificate contains the
> Â Âaccess method OID of id-ad-signedObject.
> Â Â[...]
>
> 4.2. ÂeContent
>
> Â ÂThe content of a Manifest is defined as follows:
>
> Â Â ÂManifest ::= SEQUENCE {
>    version   [0] INTEGER DEFAULT 0,
> Â Â Â manifestNumber ÂINTEGER (0..MAX),
>    thisUpdate   ÂGeneralizedTime,
>    nextUpdate   ÂGeneralizedTime,
>    fileHashAlg   OBJECT IDENTIFIER,
>    fileList    ÂSEQUENCE SIZE (0..MAX) OF FileAndHash
> Â Â Â }
>
> Â Â FileAndHash ::= Â Â SEQUENCE {
>    file      ÂIA5String,
>    hash      ÂBIT STRING
> Â Â Â }

IA5String? ÂNot UTF8String? ÂWhat goes into file naming?

> 4.2.1. ÂManifest
>
> Â ÂThe data elements of the Manifest structure are defined as follows:
>
> Â Âversion:
> Â Â Â The version number of this version of the manifest specification
> Â Â Â MUST be 0.

Some text on how versioning is intended to be used would be nice.
Specifically, how might extensions be added? ÂOr perhaps extensibility
here is seen as unnecessary?

If extensibility is inteded to be done by turning the version field of
the above SEQUENCE into a CHOICE then say so -- implementors with
sufficiently capable ASN.1 compilers and runtimes may prefer to modify
the above to use an extensible CHOICE.

In general I would much prefer that we make use of ASN.1's explicit
extensibility features (namely, the extensibility marker in SEQUENCEs,
SETs, and CHOICEs) and/or typed-holes as appropriate.

> Â ÂmanifestNumber:
> Â Â Â This field is an integer that is incremented each time a new
> Â Â Â manifest is issued for a given publication point. ÂThis field
> Â Â Â allows an RP to detect gaps in a sequence of published manifests.
>
> Â Â Â As the manifest is modeled on the CRL specification, the
> Â Â Â ManifestNumber is analogous to the CRLNumber, and the guidance in
> Â Â Â [RFC5280] for CRLNumber values is appropriate as to the range of
> Â Â Â number values that can be used for the manifestNumber. ÂManifest
> Â Â Â numbers can be expected to contain long integers. ÂManifest
> Â Â Â verifiers MUST be able to handle number values up to 20 octets.
> Â Â Â Conforming Manifest issuers MUST NOT use number values longer than
> Â Â Â 20 octets

Why not write that MAX value explicitly in the constraint for this
field? Â(Because it's a fairly long number?)

> 5.1. ÂManifest Generation Procedure
>
> Â Â6. ÂIn the case of a key pair that is to be used only once, in
> Â Â Â Âconjunction with a "one-time-use" EE certificate, the private key
> Â Â Â Âassociated with this key pair SHOULD now be destroyed.

Any reason not to make this SHOULD a MUST? ÂAny guidance as to when one
might not heed this SHOULD?

> 5.2. ÂConsiderations for Manifest Generation
>
> Â ÂA new manifest MUST be issued on or before the nextUpdate time.

Well, a new manifest must be published on or before the nextUpdate time.
Since RPs clocks will have some skew, new manifests should really be
published some time ahead of the nextUpdate time. ÂA few seconds or
minutes will do. ÂSee comments on section 6.2.

What happens if an authority fails to publish a new manifest in a timely
fashion? ÂThis would surely be an important operational consideration...

> Â ÂWhen a CA entity is performing a key rollover, the entity MAY chose

s/chose/choose/

> Â Âto have two CAs instances simultaneously publishing into the same

s/CAs instances/CA instances/

> Â Ârepository publication point. ÂIn this case there will be one
> Â Âmanifest associated with each active CA instance that is publishing
> Â Âinto the common repository publication point (directory).

> 6.1. ÂTests for Determining Manifest State
>
> Â ÂFor a given publication point, the RP SHOULD perform the following
> Â Âtests to determine the manifest state of the publication point:
>
> Â Â1. ÂFor each CA using this publication point, select the CA's current
> Â Â Â Âmanifest (The "current" manifest is the manifest issued by this
> Â Â Â ÂCA having highest manifestNumber among all valid manifests, and
> Â Â Â Âwhere manifest validity is defined in Section 4.4).
>
> Â Â Â ÂIf the publication point does not contain a valid manifest, see
> Â Â Â ÂSection 6.2. ÂLacking a valid manifest, the following tests
> Â Â Â Âcannot be performed.
>
> Â Â2. ÂTo verify completeness, an RP MAY check that every file at each
> Â Â Â Âpublication point appears in one and only one current manifest,
> Â Â Â Âand that every file listed in a current manifest that is
> Â Â Â Âpublished at the same publication point as the manifest.

See comment on (4) below.

> Â Â3. ÂCheck that the current time (translated to UTC) is between
> Â Â Â ÂthisUpdate and nextUpdate.
>
> Â Â Â ÂIf the current time does not lie within this interval then see
> Â Â Â ÂSection 6.4, but still continue with the following tests.

This appears to be in conflict with (1) above. ÂThe manifest can't be
valid if the current time does not fall in the manifest's validity
period, so what's the point in continuing? ÂI suppose I'll find out when
I get to section 6.4!

> Â Â4. ÂVerify that listed hash value of every file listed in each
> Â Â Â Âmanifest matches the value obtained by hashing the file at the
> Â Â Â Âpublication point.
>
> Â Â Â ÂIf the computed hash value of a file listed on the manifest does
> Â Â Â Ânot match the hash value contained in the manifest, then see
> Â Â Â ÂSection 6.6.

Will the RP need to check every file? ÂWhy not just those that are of
interest?

> Â ÂFor each signed object, if all of the following conditions hold:
>
> Â Â Â Â[...]
>
> Â Âthen the RP can conclude that no attack against the repository system
> Â Âhas compromised the given signed object, and the signed object MUST
> Â Âbe treated as valid.

No scope for local policy exemptions to the above MUST?

> 6.2. ÂMissing Manifests
>
> Â ÂThe absence of a current manifest at a publication point could occur
> Â Âdue to an error by the publisher or due to (malicious or accidental)
> Â Âdeletion or corruption of all valid manifests.
>
> Â ÂWhen no valid manifest is available, there is no protection against
> Â Âattacks that delete signed objects or replay old versions of signed
> Â Âobjects. ÂAll signed objects at the publication point, and all
> Â Âdescendant objects that are validated using a certificate at this
> Â Âpublication point SHOULD be viewed as suspect, but MAY be used by the
> Â ÂRP, as per local policy.

I wonder if we shouldn't have a latestNextUpdate field specifying a time
past which RPs MUST NOT accept expired manifests. ÂAlternatively, local
policy ought to specify how old an expired manifest may be accepted,
with RECOMMENDED guidance as to what that maximum age should be.

Additionally, CA operators should get guidance to publish new manifests
somewhat sooner than the expiration of current manifests being replaced
so as to have some time to cope with operations failures during manifest
generation and publication.

> Â ÂThe primary risk in using signed objects at this publication point is
> Â Âthat a superseded (but not stale) CRL would cause an RP to improperly
> Â Âaccept a revoked certificate as valid (and thus rely upon signed
> Â Âobjects that are validated using that certificate). ÂThis risk is
> Â Âsomewhat mitigated if the CRL for this publication point has a short
> Â Âtime between thisUpdate and nextUpdate (and the current time is
> Â Âwithin this interval). ÂThe risk in discarding signed objects at this
> Â Âpublication point is that an RP may incorrectly discard a large
> Â Ânumber of valid objects. ÂThis gives significant power to an
> Â Âadversary that is able to delete a manifest at the publication point.

I.e., there's a trade-off between DoS and more severe attacks. ÂHowever,
we can't protect against DoS attacks here anyways, so might as well give
guidance in preference of protecting against the other attacks.

Additionally, guidance to interleave new manifest publication such that
there's enough time to cope with operations failures and DoS attacks
should help.

> Â ÂRegardless of whether signed objects from this publication are deemed
> Â Âfit for use by an RP, this situation SHOULD result in a warning to
> Â Âthe effect that: "No manifest is available for <pub point name>, and
> Â Âthus there may have been undetected deletions or replay substitutions
> Â Âfrom the publication point."

I imagine this isn't a MUST because of log squelching considerations.
Right?

> Â ÂIn the case where an RP has access to a local cache of previously
> Â Âissued manifests that are valid, the RP MAY use the most recently
> Â Âpreviously issued valid manifests for this RPKI repository
> Â Âpublication collection in this case for each entity that publishes at
> Â Âhis publication point.

Subject to the same considerations, surely.

Any advice as to when to poll for new manifests ahead the current,
cached manifest's nextUpdate?

> 6.4. ÂStale Manifests

My comments on section 6.2 apply here as well.

> Â ÂNote that there is also the potential for the current time to be
> Â Âbefore the thisUpdate time for the manifest. ÂThis case could be due
> Â Âto publisher error, or a local clock error, and in such a case this
> Â Âsituation SHOULD result in a warning to the effect that: "A manifest
> Â Âfound at <pub point name> has an incorrect thisUpdate field. ÂThis
> Â Âcould be due to publisher error, or a local clock error, and
> Â Âprocessing for this publication point will continue using this
> Â Âotherwise valid manifest."

This can also happen due to having a slow clock on the RP or a fast
clock at the CA, or both. ÂClock skew is hardly an error, since there
will always be some skew, even if only on the order of nanoseconds in
the case of good hardware setup with good time distribution mechanisms
and good hardware time sources. ÂA bit more text (any!) regarding clock
skew would be useful.

> 8. ÂSecurity Considerations
>
...
> Â Âthe manifest structure .

s/ \././