letsencrypt / pebble

A miniature version of Boulder, Pebble is a small RFC 8555 ACME test server not suited for a production certificate authority.
Mozilla Public License 2.0
619 stars 149 forks source link

Certificate request subject commonName domain is not taken into account #304

Open bruncsak opened 4 years ago

bruncsak commented 4 years ago

RFC8555 tells:

   The CSR MUST indicate the exact same
   set of requested identifiers as the initial newOrder request.
   Identifiers of type "dns" MUST appear either in the commonName
   portion of the requested subject name or in an extensionRequest
   attribute [RFC2985] requesting a subjectAltName extension, or both.

When the subject of the CSR has CN=abc.dom.org and the subjectAltName has only DNS:xyz.dom.org, by the RFC an order must be created with both abc.dom.org and xyz.dom.org as identifiers. After making the order ready, the submitted CSR is not accepted with the following error:

{
   "type": "urn:ietf:params:acme:error:unauthorized",
   "detail": "Order includes different number of DNSnames identifiers than CSR specifies",
   "status": 403
}

For reference see: https://community.letsencrypt.org/t/the-way-domain-name-in-the-subject-of-the-certificate-request-treated/116107

felixfontein commented 4 years ago

The RFC does not say that domains from commonName must be accepted by the CA. In fact, commonName is deprecated (see for example https://security.stackexchange.com/questions/69156/using-commonname-for-dns-vs-subjaltname). This has also been discussed both here and in the Let's Encrypt community forum.

bruncsak commented 4 years ago

Hi @felixfontein, Thanks for the info, it is really useful! So, at the end, is there any plan to make pebble conform to RFC8555 in that respect, or will it stay nonconforming as today?

felixfontein commented 4 years ago

@bruncsak as I see it, Pebble does conform to RFC8555 in that respect (as do Boulder and BuyPass).

bruncsak commented 4 years ago

Sorry, I do not want to enter into debate on that, but the behavior is different between pebble and letsencrypt/buypass. See the earlier mentioned reference: https://community.letsencrypt.org/t/the-way-domain-name-in-the-subject-of-the-certificate-request-treated/116107 For the order, containing the same set of identifiers letsencrypt's and buypass's staging server issues the certificate, while pebble is not (for the same CSR). So how do you interpret the section I quoted from the RFC? My interpretation is the following: I create an order with two identifiers: abc.dom.org and xyz.dom.org. When the order state is ready, I am permitted to submit a CSR to get a certificate where (I translate the text):

   The CSR MUST indicate the exact same
   set of requested identifiers as the initial newOrder request (abc.dom.org + xyz.dom.org).
   Identifiers of type "dns" MUST appear either in the commonName
   portion of the requested subject name (abc.dom.org) or in an extensionRequest
   attribute [RFC2985] requesting a subjectAltName extension (xyz.dom.org), or both (does not apply here).
jsha commented 4 years ago

Hi @bruncsak,

Thanks for reporting! And @felixfontein thanks for digging out the link to the previous Pebble issue. The Pebble behavior is indeed intentional. In the early days of Let's Encrypt we chose to accept CSRs that only specify names in the CN without specifying them in the SAN. That was probably a mistake, but a minor one, and correcting it now would introduce compatibility problems with little benefit.

Our general goal with Pebble is to be a bit stricter about things, not to match all behavior of other implementations like Boulder and Buypass. So we're planning to keep the current behavior. If you'd like to maximize compatibility between the three implementations, I think putting all names in the SAN should work.

bruncsak commented 4 years ago

Thanks for the reply and clarification @jsha!

Is there a RFC8555 divergences document for pebble, similarly, as boulder has the following: https://github.com/letsencrypt/boulder/blob/master/docs/acme-divergences.md ?

If you'd like to maximize compatibility between the three implementations, I think putting all names in the SAN should work.

As I am writing code of an ACME client, I would like to be generic enough to handle all possible cases. There is an option where the CSR is not generated by the client, but received as input, so it is not possible to change the content of the CSR. Most likely I will add specific code to better match the pebble's behavior.

When boulder receives a CSR where the subject is empty, it adds one domain as commonName. Unlike buypass, it happily issues the certificate with empty subject. This certificate looks really weird looking at for example with firefox certificate viewer: you have to click on a blank line to see the details of the certificate. Really funny!

bruncsak commented 4 years ago

So, I implemented a workaround in my client: https://github.com/bruncsak/ght-acme.sh/commit/0f27ce73a1517be771785f89bac6995cff836103

felixfontein commented 4 years ago

@bruncsak I still don't understand why you think that RFC8555 mandates that commonName is supported. The part you quote from the RFC only says that every DNS identifier specified in newOrder must appear in commonName, SAN or both. It does not say that the CA has to accept CSRs which use commonName at all.

@jsha what's your interpretation of the RFC?

bruncsak commented 4 years ago

Oh, I see what you mean. All ACME servers are compatible with RFC8555, but different additional restriction on the CSR are not excluded which leads to incompatible behavior. So my code is correct and needed, just the comments are misleading.

bruncsak commented 4 years ago

So here is the patch to my client, fixing the comments: https://github.com/bruncsak/ght-acme.sh/commit/3d46cf6aa3377a113bd36ec95cdce509e3533b50

felixfontein commented 4 years ago

@bruncsak looks much better IMO :)

jsha commented 4 years ago

The above looks good, thanks for hashing it out! It sounds like maybe a useful addition to Pebble would be for it to have a more explicit error in this case. For instance, "The CSR's Subject commonName contains a domain name that is not in the subjectAlternativeNames of the CSR."

What do you think?

bruncsak commented 4 years ago

Today pebble accepts domain name in the Subject commonName, but it totally disregards its value, like it would not be there. Your proposed change would be more than cosmetics; before it is normal condition (certificate issued) then it becomes error condition, refusing the certificate request.

felixfontein commented 4 years ago

All improved error messages are cosmetic, but still very helpful. In this case I would definitely prefer having "The CSR's Subject commonName contains a domain name that is not in the subjectAlternativeNames of the CSR." instead of the current "Order includes different number of DNSnames identifiers than CSR specifies". This saves client developers quite some time to find out what the problem actually is.

bruncsak commented 4 years ago

Yes, that could be good to have a specific error message for that case. What about the case where one identifier is in the order, and this identifier is in the CSR's subjectAlternativeNames, and there is a different identifier in the Subject commonName . Would the CSR still be accepted as before?