Re: [lamps] Paul Wouters' Discuss on draft-ietf-lamps-cmp-updates-21: (with DISCUSS and COMMENT)

[Paul Wouters <[e-mail]>]

On Thu, Jun 23, 2022 at 12:26 PM Brockhaus, Hendrik <
[e-mail]> wrote:

> Dear Paul
>
> Many thanks for reviewing the CMP Updates draft and providing valuable
> feedback.
> Please find below the response of the authors. If you have any feedback,
> please let us know.
> I will try to submit the updates draft as soon as possible.
>

Thanks for the all changes. Most of it is very good. I think we are getting
very close!



> I am looking forward on the telechat next Thursday :-)
>

So say we all :)

> #1:

> >              This is a
> >              very sensitive service and therefore needs specific
> >              authorization.  This authorization is with the CA
> >              certificate itself.  Alternatively, the CA MAY delegate the
> >              authorization by placing the id-kp-cmKGA extended key usage
> >              in the certificate used to authenticate the origin of the
> >              generated private key or the delegation MAY be determined
> >              through local configuration of the end entity.
> >
> > These two MAYs are related, you MUST do one or the other. The text as it
> > can be interpreted to not perform either MAYs.
>
> Also including Carl's comment:
> > >
> > > [CW] I recognize this is a late comment and that there are no words
> > > to borrow in either RFC6402 or RFC6960, but adding a security
> > > consideration that highlights the need to authorize certificate
> > > requests including these new EKUs (not just id-
> > > kp-cmKGA) may be worthwhile. This would mirror the "therefore needs
> > > specific authorization" in the above snip but apply to the act of
> > > requesting delegation via the EKUs.
>
> I rephrased the sentence and remove the 'or'.
>
> New text:
>    This is a
>    very sensitive service and therefore needs specific
>    authorization.  This authorization is with the CA
>    certificate itself.  The CA MAY delegate the
>    authorization by placing the id-kp-cmKGA extended key usage
>    in the certificate used to authenticate the origin of the
>    generated private key. Alternatively, the authorization MAY be
> determined
>    through local configuration of the end entity.
>

I still dont think this addresses my point. Perhaps if you change "therefor
needs specific authorization" to "MUST use specific authorization", then
the next two MAY's are clear to be in the "either this or that" case.
Following both MAY's as "do not" would then violate the above MUST.


> In response to Carl's comment I propose adding a security consideration.
> This
> inserts a new Section after Section 2.24 with the following text. I also
> propose
> adding a sentence regarding validity periods based on the text in #2.
>
> 2.25.  Add Section 8.7 - Authorizing requests for certificates with
> specific EKUs
>
>    The following subsection addresses the security considerations to follow
>    when authorizing requests for certificates containing specific EKUs.
>
>    Insert this section after new Section 8.6:
>
>    8.7.  Authorizing requests for certificates with specific EKUs
>
>    When a CA issues a certificate containing extended key usage extensions
> as
>    defined in Section 4.5, this expresses delegation of an authorization
> that
>    originally is only with the CA certificate itself. Such delegation is a
> very sensitive
>    action in a PKI and therefore special care must be taken when approving
> such
>    certificate requests to ensure that only legitimate entities receive a
> certificate
>    containing such an EKU.
>
>    Certificates containing any of the above EKUs SHOULD NOT use the
> 'indefinite'
>    notAfter date 99991231235959Z. Recommendations on server certificate
> validity
>    by CA-Browser Forum may be used as guidance.
>

Why not MUST NOT?

Would it make sense to add something like "At the time of writing, the
CA-Browser Forum advise is to use XXXXXX".
(although I think it would increase the risk of people not checking to see
if that advise has been updated)


>
> >
> > #2
> >
> >    Such validity periods SHOULD
> >    NOT be used for protection of CMP messages and key generation.
> >    Certificates containing one of the above EKUs SHOULD NOT use
> >    indefinite expiration date.
> >
> > This leaves a rather unspecified part on the implementer. What time
> period
> > is too much? Clearly something between a few seconds and indefinite, but
> what
> > is it? Can this document make a recommendation ?
>
> Rephrased, new text:
>    Such validity periods SHOULD
>    NOT be used for protection of CMP messages and of centrally generated
> keys.
>    Certificates containing any of the above EKUs SHOULD NOT use the
> 'indefinite'
>    notAfter date 99991231235959Z. Recommendations on server certificate
> validity by
>    CA-Browser Forum may be used as guidance.
>
> I will also add an informative reference for CA-Browser Forum Baseline
> Requirements.
>

Sounds good.


> >
> > #3
> >
> > Throughout the document, Section references for the to-be-patched RFC are
> > turned into links for this RFC, eg in the text "Replace Section 5.1.3.4 -
> > Multiple Protection" in Section 2.6 where the section title has a bad
> link but
> > the section body has the right link. Please verify all of these
> references and
> > fix where needed.
>
> I see your point when I look at datatracker
> https://datatracker.ietf.org/doc/html/draft-ietf-lamps-cmp-updates#section-2.6
> and I am sorry for such wrong references.
> When I look at
> https://www.ietf.org/archive/id/draft-ietf-lamps-cmp-updates-21.html#name-replace-section-5134-multip
> the layout seems OK.
> The issue also occurred with the PDF version on datatracker, but it is not
> there with the
> pdf I produced with xml2rfc.
> In the xml document I only used <xref ... /> for references to sections of
> this document.
> References in the text that are to be inserted in the to-be-patched
> documents are not
> provided as xref in the xml but laid out like this by the tool chain. I do
> not know how to
> circumvent this. My hope is that the final version provided by the
> RFC-Editors will not
> contain such issues. I will definitely check this in Auth48.
>

You can report a github issue to the tools team? Perhaps at
https://github.com/ietf-tools/xml2rfc/issues


> If anyone has any guidance what to do in advance to prevent this, please
> let me know.
>

Not me, but now that you are aware I'm sure it will get resolved before
publication :)



>
> >
> > #4
> >
> >       It MAY
> >       include the original PKIMessage from the EE in the generalInfo
> >       field of PKIHeader of a nested message (to accommodate, for
> >       example, cases in which the CA wishes to check POP or other
> >       information on the original EE message).
> >
> > If a CA wishes to do so, it would REQUIRE this original PKIMessage.
> Would it
> > not be better to say "It MUST include the original PKIMessage" ? It
> seems also
> > generally better to send the originals along with the modification so
> that the
> > next step can (optionally!) authenticate the previous step. Otherwise,
> there is
> > a lot of implied trust that should be modeled in the Security
> Considerations.
>
> You are right, if the CA wants to check the POP, it is required to provide
> the
> original message in the PKIHeader. Generally speaking, providing the
> original
> message is optional when the POP is set to RAVerified. Using RAVerified",
> the
> respective RA indicates that it already verified the POP.
> I rephrased the example to indicate that in case the CA policy requires to
> again
> check the POP at CA level, the original message must be provided in the
> PKIHeader
> by the RA.
>
> New text:
>    It MAY
>    include the original PKIMessage from the EE in the generalInfo
>    field of PKIHeader of a nested message. For example, this is
>    REQUIRED to accommodate cases in which the CA wishes to
>    check POP or other information on the original EE message.
>

This still has the same issue. The original PKIMessage is still MAY, which
prevents the subsequent REQUIRED from always working.
The text we need should say something like (if I understood you correctly
above):  "MUST be included if not RAVerified and otherwise MAY be omitted".


> >
> > #5
> >
> > In Section 2.20, it talks abot updating 4210's Section 7. It suggests
> removing
> > the first 3 paragraphs with replacement text. However, the text removed
> > describes the behaviour in a version agnostic way that I think is more
> clear
> > than the replacement text.
> >
> >       If the client does not accept EnvelopedData, but EncryptedValue,
> >       then it MUST use cmp2000.
> >
> > Why not cmp1999? Because EncryptedValue is valid for cmp1999 RFC2510 as
> well?
> > Are we assuming cmp1999 is completely dead and no longer deployed?
> >
> > In general I would clarify section 2.20 better. More clearly subdivide
> client
> > and server, and leave a version of the text in Section 7 before section
> 7.1
> > intact.
> >
> > Also, it seems the into in the original Section 7 really covers the
> protocol
> > behaviour. I am not sure why there are subsections with specific version
> > numbers in 4210 nor do I understand why this has to be patched to an even
> > more
> > elaborate versioning, and mentioning EncryptedValue vs
> EncryptedEnvelope. It
> > seems the section 7 overview covers all behaviour already.
>
> We had to introduce cmp2021 to indicate those cases where the ASN.1 syntax
> changed. These are:
> (a) EncryptedKey is used instead of EncryptedValue to facilitate using
> EnvelopedData
> (b) the new hashAlg field is used in CertStatus structures of certConf
> messages In all
>      other cases new syntax is indicated using OIDs and no new CMP version
> is required.
>
> The version negotiation defined in RFC4210 is most general and rather
> straightforward but has the drawback that if a client already supports
> cmp2021
> while a server does not, an extra round trip just for version negotiation
> is required,
> even though for most messages cmp2000 is still sufficient. For backward
> compatibility
> while minimizing the need for an extra round trip just for version
> negotiation, cmp2021
> should only be used in cases where the new ASN.1 syntax is actually needed.
>
> There are cases where the client needs to indicate capability of
> understanding
> cmp2021 also without using the new syntax in the request message to tell
> the server
> that it is capable understanding the new syntax, e.g., when the clients
> request the
> server to generate the key pair centrally.
>
> There are two cases where the client should use cmp2021 in a request
> message even
> without actually using the new ASN.1 syntax in this request message. This
> is needed to
> tell the server that it can understand the cmp2021 ASN.1 syntax. These are:
> (a) the client requests central key pair generation and expects the server
> to respond
>      using an EnvelopedData structure for conveying the private key
> (b) the client sends a certificate request and expects the server to
> respond with a
>      certificate encrypted using an EnvelopedData structure (for indirect
> proof-of-possession).
>
> Therefore, the version handling of cmp2021 is a bit complex. Following
> your guidance we
> managed to simplify the text of Section 7 dramatically. We finally only
> extended the
> second paragraph and added a note. For ease of reading I add the complete
> text of Section 7.
> Sections 7.1 stays unchanged.
>
> New text:
> 7.  Version Negotiation
>
>    This section defines the version negotiation used to support older
>    protocols between client and servers.
>
>    If a client knows the protocol version(s) supported by the server
>    (e.g., from a previous PKIMessage exchange or via some out-of-band
>    means), then it MUST send a PKIMessage with the highest version
>    supported by both it and the server.  If a client does not know what
>    version(s) the server supports, then it MUST send a PKIMessage using
>    the highest version it supports, with the following exception. Version
>    cmp2021 SHOULD only be used if cmp2021 syntax is needed for the
>    request being sent or for the expected response.
>
>    Note: Using cmp2000 as the default pvno is done to avoid extra message
>    exchanges for version negotiation and to foster compatibility with
>    cmp2000 implementations. Version cmp2021 syntax is only needed if a
>    message exchange uses hashAlg (in CertStatus) or EnvelopedData.
>
>    If a server receives a message with a version that it supports, then
>    the version of the response message MUST be the same as the received
>    version.  If a server receives a message with a version higher or
>    lower than it supports, then it MUST send back an ErrorMsg with the
>    unsupportedVersion bit set (in the failureInfo field of the
>    pKIStatusInfo).  If the received version is higher than the highest
>    supported version, then the version in the error message MUST be the
>    highest version the server supports; if the received version is lower
>    than the lowest supported version then the version in the error
>    message MUST be the lowest version the server supports.
>
>    If a client gets back an ErrorMsgContent with the unsupportedVersion
>    bit set and a version it supports, then it MAY retry the request with
>    that version.
>
>
This looks good. It does not indicate that anyone supporting cmp2021 also
must support cmp2020,
but I think that's okay because the difference is only this one specific
thing where it is talked about.


> >
> > #6
> > Section 3.4 "patches" the IANA Considerations. I'd rather we didn't do
> this and
> > add a clear new IANA Considerations section with clear complete
> instructions to
> > IANA as to what changes to make, but I understand perhaps why to do this
> from
> > a
> > readability point of view. But at the very least leave a note to the RFC
> Editor
> > to confirm all IANA Actions for this document are summarized in this
> document's
> > IANA Considerations.
> >
> >     < TBD: The temporary registration of cmp URI suffix must be updated
> >    from provisional to permanent. >
> >
> > IANA will do this when the document goes from draft to RFC. So this
> comment
> > can
> > safely be removed.
> >
> >    < TBD: A new protocol registry group "Certificate Management Protocol
> >    (CMP)" (at https://www.iana.org/assignments/cmp) and an initial entry
> >    'p' must be registered. >
> >
> > Same here.
> >
> > Section 4 IANA Considerations should contain a copy of the "patch"
> instructions
> > as a clear instruction to IANA so they can make the changes without them
> > needing to "patch" the old RFC to obtain the instructions. Even if this
> sounds
> > redundant in this document.
>
> I will rephrase the text in Section 4 IANA Consideration and copy the
> instructions provided in Section 2.25 and Section 3.4.
> I will also remove the todos from the draft.
>
>
Thanks.



> >
> >
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> >
> >    As a next step, the LAMPS Working Group will consider providing
> >    RFC4210bis and RFC6712bis documents in order to offer the reader
> >    self-contained updated documents for CMP.
> >
> > I don't think these kind of "instructions to the WG or IETF" should
> appear
> > in the document. But also, I would really like to see a commitment from
> the WG
> > that these bis documents will be created. But again, such a response
> should not
> > be stated within the document.
>
> Thank you for the clarification. I will remove this sentence from the
> document.
>

Thanks. Note that I added 4210bis and 6712bis to make notes list for lamps
to not lose track :)



>
> >
> >    The following updates are made in [thisRFC]:
> >
> > I assume thisRFC refers to the current document being reviewed. But if
> that
> > text is replaced it would still be confusing. Maybe use: "[thisRFC]
> (this RFC)"
> > where [thisRFC] is replaced for the actual RFC and "(this RFC)" is
> literal text.
>
> I like this proposal and change to the literal text 'this RFC' or 'this
> document' in
> full text sections. In the ASN.1 modules the replacement with the RFC
> number is
> still needed because the may be used also outside the document.
>

Looks good.



>
> >
> >    a PKI management entity such
> >    as an RA MAY forward that message adding its own protection (which
> >    MAY be a MAC or a signature, depending on the information and
> >    certificates shared between the RA and the CA)
> >
> > More confusing use of MAY. Was the intension to say "which MUST be a MAC
> or a
> > signature" ? If I am wrong, it perhaps need to say "MAY be a MAC or a
> signature
> > or nothing". But that seems unlikely to me. The first MAY also wouldn't
> need
> > all caps.
>
> This text portion mainly originates from RFC4210, but I see your point.
>
> New text:
>   a PKI management entity such
>   as an RA MAY forward that message adding its own protection (which
>   is a MAC or a signature, depending on the information and
>   certificates shared between the RA and the CA)
>

Looks good.


>
> >
> >       A client not supporting cmp2021 will not be able to handle this
> >       situation and will fail or reject the certificate.
> >
> > I don't understand the "or" here. Doesn't it fail because it rejects the
> > certificate? Can it reject a certificate and not fail ?
>
> This text in not part of the Section 7 anymore.
>
> >
> >    When a CA acts as a CMP endpoint, it should not use the same private
> >    key for issuing certificates and for protecting CMP responses, to
> >    reduce the number of usages of the key to the minimum required.
> >
> > What is "the key". It makes me read "CA" as the "CA certificate/key
> pair" but
> > since there can be multiple keys, I should read "CA" as the Certificate
> Agency
> > in general, not the single CA cert. Perhaps use "to reduce the different
> type
> > of usages of a particular key to a minimum" ? (Are we worried about
> different
> > types of usage, or actual just number of signing operations?)
>
> I am worried about both, the number of times the private key of the CA
> signing
> certificate is used as well as exposing it to other SW systems, e.g., for
> CMP
> protection or TLS handshake.
>
> New text:
>    A CA should not reuse its certificate signing key for other purposes
>    such as protecting CMP responses and TLS connections. This way, exposure
>   to other parts of the system  and the number of uses of this particularly
>   critical key is reduced to a minimum.
>

Looks good.


> >
> >    a CA
> >    certificate for use as a trust anchor, it MUST properly authenticate
> >    the message sender without already trusting any of the CA
> >    certificates given in the message.
> >
> > If the EE already trusts CA X, and CA X is included in the message, the
> EE is
> > suddenly not allowed to use its preconfigured CA? Maybe rewrite to say
> "it MUST
> > properly authenticate the message sender with existing trust anchors
> without
> > requiring new trust anchors included in the message".
>
> Thank you for this proposal.
>
> New text:
>    a CA
>    certificate for use as a trust anchor, it MUST properly authenticate
>    the message sender with existing trust anchors without
>    requiring new trust anchors included in the message.
>

Looks good.



> >
> >    -- *  that the contents of "thisMessage" MUST be encoded either as
> >    -- *  "EnvelopedData" or "EncryptedValue" (only for backward
> >    -- *  compatibility) and then wrapped in a BIT STRING.  This
> >    -- *  allows the necessary conveyance and protection of the
> >    -- *  private key while maintaining bits-on-the-wire compatibility
> >    -- *  with RFC 4211 [RFC4211].
> >
> > As EnvelopedData is only added in "this document" I think the last line
> > should be modified to say: with RFC 4211 [RFC4211] and [thisRFC]
> > (note, using TBDx instead of thisRFC might be helpfull to the RFC Editor)
>
> RFC4211 defines EncryptedKey already introduced the flexibility for using
> EnvelopedData
> instead of EncryptedValue. By some reason, it was not used in RFC4210 back
> then.
> If you dislike the referring to RFC4211 we could also refer to RFC4210 and
> [TBDx] instead.
>
> New text:
>    -- *  that the contents of "thisMessage" MUST be encoded either as
>    -- *  "EnvelopedData" or "EncryptedValue" (only for backward
>    -- *  compatibility) and then wrapped in a BIT STRING.  This
>    -- *  allows the necessary conveyance and protection of the
>    -- *  private key while maintaining bits-on-the-wire compatibility
>    -- *  with RFC4210 and [TBDx].
>

Looks good.



>
> >
> >        -- AlgId for a One-Way Function (SHA-1 recommended)
> >
> > Can the part about recommending SHA-1 be removed, or does this document
> > really want to state SHA-1 is still recommended. I guess this might be
> > okay since it is describing the 1988 ASN.1 Module ?
>
> I will remove this as SHA-1 is not listed as recommend hash algorithm in
> CMP Algorithms.
>

Even better!


Remainder of nits fixes all look good too.

So I just have the #1 and #4 as needing a bit more discussion or
clarification.

Paul



>
> >    Note: This appendix will be deleted in the final version of the
> >    document.
> >
> > Please rewrite as an instruction to the RFC Editor and put in [ square
> braces ]
> > for clarity.
>
> Thank you for this hint.
>
> New text:
> [RFC Editor: This appendix must be deleted in the final version of the
> document.]
>
> >
> > Nits:
> >
> > It updates  RFC 4210, RFC 5912, and RFC 6712, but skimming the ToC one
> can
> > only
> > find a section on updates for 4210 and for 6712. It turns out updates to
> 5912
> > are in Appendix B. Perhaps change the title so this becomes more obvious
> in the
> > ToC?
>
> Thank you for this suggestion. I will rephrase the title of Section A.1
> and Section A.2
>
> New section titles:
> A.1. Update to RFC4210 - 1988 ASN.1 Module
> A.2. Update to RFC5912 - 2002 ASN.1 Module
>
> >
> >    Such delegation MAY also be
> >    expressed by other means, e.g., explicit configuration.
> >
> > I don't think that MAY needs to be in all caps? It is not part of the
> protocol
> > to implement, but a directive to a human operator.
>
> Agree.
>
> New text:
>    Such delegation may also be
>    expressed by other means, e.g., explicit configuration.
>
> >
> >    these OIDs MAY be re-used.
> >
> > Same here, it explains why the authors of the RFC did this, so this MAY
> seems
> > silly at best
>
> Right.
>
> New text:
>    Note: RFC 6402 section 2.10 [RFC6402] specifies OIDs for a CMC CA and
>    a CMC RA.  As the functionality of a CA and RA is not specific to
>    using CMC or CMP as the certificate management protocol, theses EKUs
>    are re-used by CMP.
>
> >
> >    5.1.3.4.  Multiple Protection
> >
> > multiple proction what? I guess Multiple Proction in a message (or in a
> > request) ?
>
> This is the original title as of RFC4210. I think renaming it is not
> required as
> this section is about adding additional protections to a CMP message.
> If no one opposes, I would keep this wording.
>
> >
> >    To indicate support for EnvelopedData the pvno cmp2021 is introduced
> >    by this document.
> >
> > The use of "this document" is super confusing due to this being a "patch
> > document". Use "has been introduced" instead?
>
> You are right. This does not makes sense here.
>
> New text:
>    To indicate support for EnvelopedData the pvno cmp2021 has been
>    introduced.
>
> >
> >    Note: To indicate support for EnvelopedData the pvno cmp2021 is
> >    introduced by this document.
> >
> > Similar issue with "this document".
>
> New text:
>    Note: To indicate support for EnvelopedData the pvno cmp2021 has
>    been introduced.  Details on the usage of different pvno values are
>    described in Section 7.
>
> >
> >    Moreover
> >
> > Maybe use "additionally" instead of Moreover?
>
> I will update the four places 'moreover' occurs with 'additionally'.
>
> >
> >     see CVE-2008-0166 [CVE-2008-0166];
> >
> > Maybe just use the link instead of doubling the name in these references
>
> I will do so.
>
> >
> > cannot be measure -> cannot be measured
>
> Thanks, done.
>
> >
> >     the security of the
> >    shared secret information should exceed the security strength of each
> >    key pair.
> >
> > I would say "of each individual key pair", so people are not confused in
> > thinking they might need to add up all the key strength bits.
>
> Thank you for this proposal
>
> New text:
>    the security of the
>    shared secret information should exceed the security strength of each
>    individual key pair.
>
> >
> >     using pollReq and pollReq messages
> >
> > That second pollReq should be pollRep.
>
> Thank you for spotting this :-)
>
> New text:
>     using pollReq and pollRep messages
>
> >
> >     No changes are made to the existing security considerations of RFC
> 6712
> >     [RFC6712].
> >
> > I think what is meant is "no security considerations updates of RFC 6712
> were
> > required".
>
> Thanks for this proposal:
>
> New text:
>     No security considerations updates of RFC 6712 [RFC6712] were required.
>
>