Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

Remove proactive issuance & csr-first new-order. #342

Merged
merged 7 commits into from
Nov 28, 2017

Conversation

cpu
Copy link
Collaborator

@cpu cpu commented Sep 26, 2017

This commit implements a first go at the replacement of proactive issuance
and the "CSR first" new-order flow proposed on the ACME mailing list

Mentions of proactive issuance are removed. The CSR field of the order
object is removed. A new "finalizeURL" field is added to the order
object. A new "identifiers" field is added to the order object. Issuance
process is updated to describe submitting identifiers in the new-order
request and POSTing a CSR to the order's finalizeURL.

This commit implements the proposed replacement of proactive issuance
and the "CSR first" new-order flow proposed on the ACME mailing list[0].

Mentions of proactive issuance are removed. The CSR field of the order
object is removed. A new "finalizeURL" field is added to the order
object. A new "identifiers" field is added to the order object. Issuance
process is updated to describe submitting identifiers in the new-order
request and POSTing a CSR to the order's finalizeURL.

[0] https://mailarchive.ietf.org/arch/msg/acme/DIjJEB06J5cFyuOlGPVcY2I51vg
Daniel added 4 commits October 12, 2017 16:43
This commit:
* updates the Action,Request,Response table for the
  finalization process.
* updates the example order object with a certificate URL to be status
  "valid" (since a cert has been issued)
* clarifies the finalization language w.r.t errors
* updates the registry of order fields to capture the finalizeURL
cpu pushed a commit to letsencrypt/pebble that referenced this pull request Nov 6, 2017
This commit implements ietf-wg-acme/acme#342
- replacing proactive issuance and CSR as part of new-order with an
explicit order finalization step that delivers the CSR.

This is largely a port of the work done to add order finalization to the
WIP ACMEv2 support in Boulder:
letsencrypt/boulder#3169

I haven't tested this end-to-end yet - There are likely bugs lurking :-)
shred added a commit to shred/acme4j that referenced this pull request Nov 9, 2017
Replaces the "CSR first" new-order flow, see ietf-wg-acme/acme#342
If a client observes a change in the contents of the "authorizations" array,
then it SHOULD consider the order invalid.
Identifiers in a new-order request MAY contain wildcard identifiers, even if
the resulting authorizations returned by the server do not.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you need to be a little more precise about defining the wildcard identifier for each type of identifier (i.e., the leftmost label can be '*' for DNS)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinthomson Thanks for the feedback. I took a shot at clarifying with 58eec44 Open to further refinement!

Should there be an RFC reference for the requirements placed on wildcard DNS identifiers? If so what's the best RFC to cite?

I thought that RFC5280 might specify the "only one wildcard character, only as the entire left-most DNS label" behaviour but it explicitly doesn't address the semantics of wildcard SAN names. RFC6125 has a section on checking wildcard certificates but it seems a little loosey-goosey and the last MAY is (to my understanding) not something browsers do.

Copy link
Collaborator

@jsha jsha Nov 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done some digging on this, and it's a mess. RFC 5280 says dNSName SANs must be in RFC 1034 "preferred name syntax." Preferred name syntax is letter-digit-hyphen, no "*" allowed. Inserting "*"'s became common practice (against 5280), as documented in RFC 6125.

Section 7.2 of RFC 6125 describes the situation for issuers, versus 6.4.3, which you link to and which describes the situation for relying parties:

This document states that the wildcard character '*' SHOULD NOT be
included in presented identifiers but MAY be checked by application
clients (mainly for the sake of backward compatibility with deployed
infrastructure).

The Baseline Requirements say:

Domain Name: The label assigned to a node in the Domain Name System.
Fully-Qualified Domain Name: A Domain Name that includes the labels of all superior nodes in the Internet Domain Name System.
Wildcard Domain Name: A Domain Name consisting of a single asterisk character followed by a single full stop character (“*.”) followed by a Fully-Qualified Domain Name.
7.1.4.2.1 Subject Alternative Name Extension
...
Wildcard FQDNs are permitted.

So, AFAICT, RFC5280 doesn't allow wildcards, then RFC6125 extra discourages them, but they are in widespread practical use. I think your current revision is very close. Here's my proposal to bring it closer to the BR definition:

Any identifier of type "dns" in a new-order request MAY have a wildcard domain
name as its value. A wildcard domain name consists of a single asterisk
character followed by a single full stop character (“*.”) followed by a
domain name as defined for use in the Subject Alternative Name Extension by RFC
5280.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done some digging on this, and it's a mess.

Classic.

So, AFAICT, RFC5280 doesn't allow wildcards, then RFC6125 extra discourages them, but they are in widespread practical use. I think your current revision is very close. Here's my proposal to bring it closer to the BR definition

Thanks! That reads much better. I stole your wording for f451f2c and preserved the "Your wildcard shouldn't be in the returned authorizations" part by more closely matching the MUST NOT from 7.1.4.

Daniel added 2 commits November 16, 2017 11:29
The spec should be more precise about the fact that a wildcard
identifier is specific to DNS type identifiers and must follow the
conventions of the web PKI w.r.t number & placement of wildcard
characters.
This commit incorporates feedback from @jsha (thanks!) refining the
wildcard identifier value clarification.
@richsalz
Copy link
Member

richsalz commented Nov 21, 2017 via email

@cpu
Copy link
Collaborator Author

cpu commented Nov 21, 2017

A wildcard is not a dnsName name. It happens to use the same encoding, perhaps unfortunately.

@richsalz Can you point out which line of the diff you're referring to? I don't see where in f451f2c we call a wildcard a dnsName.

@richsalz
Copy link
Member

richsalz commented Nov 21, 2017 via email

@cpu
Copy link
Collaborator Author

cpu commented Nov 21, 2017

I was referring to the RFC’s.

Aha. So no changes being requested for this PR 👍

@bifurcation
Copy link
Contributor

There's been a bunch of list discussion on this, and a pretty clear consensus call at IETF 100. Merging based on the chairs call of consensus.

@bifurcation bifurcation merged commit 418ad40 into ietf-wg-acme:master Nov 28, 2017
shred added a commit to shred/acme4j that referenced this pull request Dec 5, 2017
Replaces the "CSR first" new-order flow, see ietf-wg-acme/acme#342
jsha pushed a commit to letsencrypt/pebble that referenced this pull request Dec 5, 2017
This commit implements ietf-wg-acme/acme#342 - replacing proactive issuance and CSR as part of new-order with an explicit order finalization step that delivers the CSR.

This is largely a port of the work done to add order finalization to the WIP ACMEv2 support in Boulder:
letsencrypt/boulder#3169 

I haven't tested this end-to-end yet - There are likely bugs lurking :-)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants