Subject: [Last-Call] Secdir last call review of draft-ietf-acme-star-delegation-06

From: Russ Housley via Datatracker <[email-masked]>
Date: Sun, 14 Mar 2021 15:01:43 -0700

Reviewer: Russ Housley
Review result: Not Ready

I 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 authors, document editors, and WG chairs should
treat these comments just like any other IETF Last Call comments.

Document: draft-ietf-acme-star-delegation-06
Reviewer: Russ Housley
Review Date: 2021-03-14
IETF LC End Date: 2021-03-22
IESG Telechat date: unknown

Summary: Not Ready

Major Concerns:

Abstract: It says: "...  party access to a certificate associated with
said identifier."  This is odd wording, and it is incorrect.  The party
needs access to the private key that corresponds to the public key in
the certificate, and the certificate needs to contain the subject for
"said identifier".  Clearly, all of that should not go in the Abstract,
but what does appear in the Abstract needs to be technically accurate.

Section 1 says: "...   name matches the authority ...".  I find this
description confusing.  I think it would be more clear to say that the
cache server needs to present a certificate whose subject name matches
the domain name of the URL that is requested.  The current wording is
very easy to confuse name of the Certification Authority.

Section 1 says:

   While the primary use case we address is delegation of STAR
   certificates, the mechanism proposed here accommodates any
   certificate managed with the ACME protocol.  See Section 2.4 for
   details.

This is not much of a hint that long-term certificates are supported
in addition to STAR certificates.  Further, a hint about the handling
of revocation is appropriate here.  Support for long-lived certificates
is in conflict with the title of the document.  Please adjust the title
of the document accordingly.

Section 2.3.2 says: "Besides, when delegation is for a STAR certificate,
..." (in four places in this section).  I find this part of the document
structure a bit confusing.  Maybe it is the lask ot adequate warning
about support for long-lived certificates.  Maybe it the the mixing of
STAR certificate and long-lived certificate processing in one section.
I suggest that separate sections be used to present STAR certificate and
long-lived certificate processing

In Section 2.3.4, the text is begging for one more sentence.  Please
say something about the fact that the STAR certificate will expire
shortly after the automatic renewal process is stopped by the IdO.

Section 2.4 is not sufficient to explain the revocation processing.
Only the NDC has the private key needed to make the ACME revocation
request, but this does not get stated in the text.  Also, it is not
clear to me how the NDC knows where to send the revocation request
since the IdO is the ACME account owner.  In addition, the phrase
"would create a self-inflicted DoS" needs more explanation.

Section 5.6 registers a string name for each extendedKeyUsage OID.
There should be a way to provide the OID in dotted decimal format
as well.  New OIDs are being assigned all the time, and some of them
may not be registered with IANA.

Section 5.6 registers a string name for each type of subjectAltName.
This include otherName, which are identified by an OID.  New OIDs are
being assigned all the time.  For example, draft-ietf-anima-autonomic-
control-plane-30 creates a new otherName.  There should be a way to
provide the the otherName OID in dotted decimal format as well.

Minor Concerns:

Abstract: Please spell out ACME, CDN, and STAR.  These are not marked
as "well known" in the RFC Editor abbreviation expansion list.

Section 1 describes [I-D.mglt-lurk-tls13] as an ongoing effort.  This
is not accurate.  The LURK BoF did not lead to a WG or an effort in an
existing WG.  I think the best way forward is to drop this reference.

Section 1.1: Please change CA to "Certification Authority".  See
Section 3 of RFC 5280.  This changes is also needed elsewhere in the
document.

Section 1.1: Please add CDNI, uCDN, dCDN, PASSPorT, CSR and FQDN
to the list of terms.

Nits:

Section 2 says: "... in this draft ...".  Please use a work that will
still be appropriate when this document becomes an RFC.

Section 2.4: s/Sec. 7.6/Section 7.6/  (and many other places)

IDnits reports:

  ** There are 3 instances of too long lines in the document, the
     longest one  being 4 characters in excess of 72.

  == There are 4 instances of lines with non-RFC6890-compliant IPv4
     addresses in the document.  If these are example addresses, they
     should be changed.

[I suspect these are not IPv4 addresses, but OIDs in dotted decimal.]

---

From: Thomas Fossati <[email-masked]>
Date: Sun, 14 Mar 2021 22:58:50 +0000

Hi Russ,

Thanks very much for your clear and thorough review!

Your comments are now tracked in the following tickets:

On 14/03/2021, 22:02, "Russ Housley via Datatracker" <[email-masked]> wrote:
> said identifier."  This is odd wording, and it is incorrect.  The
> party needs access to the private key that corresponds to the public
> key in the certificate, and the certificate needs to contain the
> subject for "said identifier".  Clearly, all of that should not go in
> the Abstract, but what does appear in the Abstract needs to be
> technically accurate.

https://github.com/yaronf/I-D/issues/139

> very easy to confuse name of the Certification Authority.

https://github.com/yaronf/I-D/issues/140

> of revocation is appropriate here.  Support for long-lived
> certificates is in conflict with the title of the document.  Please
> adjust the title of the document accordingly.

https://github.com/yaronf/I-D/issues/141

> Section 2.3.2 says: "Besides, when delegation is for a STAR
> certificate, ..." (in four places in this section).  I find this part
> of the document structure a bit confusing.  Maybe it is the lask ot
> adequate warning about support for long-lived certificates.  Maybe it
> the the mixing of STAR certificate and long-lived certificate
> processing in one section.  I suggest that separate sections be used
> to present STAR certificate and long-lived certificate processing

https://github.com/yaronf/I-D/issues/142

> shortly after the automatic renewal process is stopped by the IdO.

https://github.com/yaronf/I-D/issues/143

> "would create a self-inflicted DoS" needs more explanation.

https://github.com/yaronf/I-D/issues/144

> There should be a way to provide the OID in dotted decimal format as
> well.  New OIDs are being assigned all the time, and some of them may
> not be registered with IANA.

https://github.com/yaronf/I-D/issues/145

> provide the the otherName OID in dotted decimal format as well.

https://github.com/yaronf/I-D/issues/146

> Section 1.1: Please add CDNI, uCDN, dCDN, PASSPorT, CSR and FQDN to
> the list of terms.

https://github.com/yaronf/I-D/issues/147

> existing WG.  I think the best way forward is to drop this reference.

https://github.com/yaronf/I-D/issues/148

> Section 2.4: s/Sec. 7.6/Section 7.6/  (and many other places)

https://github.com/yaronf/I-D/issues/149

> [I suspect these are not IPv4 addresses, but OIDs in dotted decimal.]

https://github.com/yaronf/I-D/issues/150

We'll be back to you as soon as we have addressed them or for further
clarifications, if needed.

Cheers, and thanks again.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

---

From: Russ Housley <[email-masked]>
Date: Mon, 22 Mar 2021 15:30:12 -0400

Yaron and Thomas:

Comments below ...

>    https://github.com/yaronf/I-D/issues/139 <https://github.com/yaronf/I-D/issues/139>

I think it would be better to avoid the phrase "own a certificate".  Some certificate policies explicitly says that the issuer owns the certificate, and your use of "own" has nothing to do with that concept.  You are after a private key under the control of the CDN and a certificate that contains the domain name of the party performing the delegation.

>    https://github.com/yaronf/I-D/issues/140 <https://github.com/yaronf/I-D/issues/140>

The proposed working looks fine.

>    https://github.com/yaronf/I-D/issues/141 <https://github.com/yaronf/I-D/issues/141>

This is very clear.  Thanks.

>    https://github.com/yaronf/I-D/issues/142 <https://github.com/yaronf/I-D/issues/142>

The restructuring is quire difficult to review in the markdown.  I cannot be sure this addresses my comment.  That said, the word "besides" does not appear in the document, so this has probably been sorted out.

>    https://github.com/yaronf/I-D/issues/143 <https://github.com/yaronf/I-D/issues/143>

The proposed working looks fine.

>    https://github.com/yaronf/I-D/issues/144 <https://github.com/yaronf/I-D/issues/144>

As with issue 142, it is difficult to review in this form, but I think the concern has been resolved.

>    https://github.com/yaronf/I-D/issues/145 <https://github.com/yaronf/I-D/issues/145>

I see the regex in CSR-template/template-schema.cddl, but I think there should also be text in Section 5.6.

>    https://github.com/yaronf/I-D/issues/146 <https://github.com/yaronf/I-D/issues/146>

This comment is not about authorizations.  Of course, for each name form in the SubjectAltName, the ACME CA needs to confirm control over the requested name.

Why are you explicitly allowing email address with no discussion of this point?  Email address raises the same concerns as the otherName.

>    https://github.com/yaronf/I-D/issues/147 <https://github.com/yaronf/I-D/issues/147>

It is not clear to me why some were added, but others were not.

>    https://github.com/yaronf/I-D/issues/148 <https://github.com/yaronf/I-D/issues/148>

Not clear that LURK will gain acceptance, but TLS-Subcerts is about to be passed to the IESG.

>    https://github.com/yaronf/I-D/issues/149 <https://github.com/yaronf/I-D/issues/149>

Thanks for making this change.

>    https://github.com/yaronf/I-D/issues/150 <https://github.com/yaronf/I-D/issues/150>

I cannot check this in markdown format.  Please run IDnits.

Russ

---

From: Thomas Fossati <[email-masked]>
Date: Tue, 23 Mar 2021 09:58:41 +0000

Hi Russ,

A quick follow-up to try and close the issues that you could not confirm
had been addressed because of the complexity of navigating the markdown
diffs.

I have attached the output of rfcdiff, which should give you a more
comfortable view over those changes.

On 22/03/2021, 19:30, "Russ Housley" <[email-masked]> wrote:
> The restructuring is quire difficult to review in the markdown.  I
> cannot be sure this addresses my comment.  That said, the word
> "besides" does not appear in the document, so this has probably been
> sorted out.

Please see the attached rfcdiff output.

> the concern has been resolved.

Ditto.

> I cannot check this in markdown format.  Please run IDnits.

Done on my local copy and it looks good:

$ idnits draft-ietf-acme-star-delegation-latest.txt
Inspecting file draft-ietf-acme-star-delegation-latest.txt

Comment:

   Found 4 lines with non-ASCII characters.

Found 0 errors, 0 warnings, 1 comment.

cheers, t

---

From: Russ Housley <[email-masked]>
Date: Tue, 23 Mar 2021 09:26:15 -0400

Thomas:

Thanks for the diff.  The revised structure looks good to me.  Much more clear.

Russ

> On Mar 23, 2021, at 5:58 AM, Thomas Fossati <[email-masked]> wrote:
> 
> Hi Russ,
> 
> <draft-ietf-acme-star-delegation-latest-from-06.diff.html>_______________________________________________
> Acme mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/acme

---

From: Yaron Sheffer <[email-masked]>
Date: Thu, 25 Mar 2021 16:28:44 +0200

Hi Russ,

Please see the remaining open issues from your review - we have reopened the GitHub issues:

Thanks,
	Yaron

On 3/23/21, 15:26, "Russ Housley" <[email-masked]> wrote:

    Thomas:

    Russ

---

From: Russ Housley <[email-masked]>
Date: Thu, 25 Mar 2021 15:27:53 -0400

Yaron:

You will see my comments in those issues.

Russ

> On Mar 25, 2021, at 10:28 AM, Yaron Sheffer <[email-masked]> wrote:
> 
> Hi Russ,
>

---

From: Thomas Fossati <[email-masked]>
Date: Thu, 25 Mar 2021 20:02:35 +0000

Hi Russ,

> You will see my comments in those issues.

Thanks very much!

We have prepared https://github.com/yaronf/I-D/pull/167/files

Could you please review it and see if fixes your remaining concerns?

Cheers, t

---

From: Russ Housley <[email-masked]>
Date: Thu, 25 Mar 2021 16:14:58 -0400

Thomas:

As I said in GitHub, I think the Abstract could be more clear.  There are two key points. First, the certificate contains the identifier that is delegated. Second, that the third party has control of the private key, and the certificate contains the corresponding public key.

All of the other topics have been sorted out.

Russ

> On Mar 25, 2021, at 4:02 PM, Thomas Fossati <[email-masked]> wrote:
> 
> Hi Russ,
> 
> _______________________________________________
> Acme mailing list
> [email-masked]

---

From: Thomas Fossati <[email-masked]>
Date: Thu, 25 Mar 2021 20:19:34 +0000

Hi Russ,

On 25/03/2021, 20:15, "Russ Housley" <[email-masked]> wrote:
> Thomas:
>
> As I said in GitHub, I think the Abstract could be more clear.  There
> are two key points. First, the certificate contains the identifier that
> is delegated. Second, that the third party has control of the private
> key, and the certificate contains the corresponding public key.

OK, I thought I had captured the spirit of your comment, but apparently
I failed :-)  I'll give it another go.

> All of the other topics have been sorted out.

super!

t