SNI cached incorrectly for wildcard domains (w/ fix) #74

New Issue

Ghost · 2021-04-23T09:52:17Z

Ghost commented

2021-04-23 09:52:17 +00:00

When a request for e.g. x.example.com under a *.example.com + example.com certificate is made, greenlock-express will:

cache ['x.example.com'] = { secureContext: { _valid: false } }
generate correct context
cache...
- ['*.example.com'] = correctContext
- ['example.com'] = correctContext

It notably won't ever cache the correct context for x.example.com, so every following request to x.example.com will fail.

I wrote a quick fix here: a05b702e66

(No PR because I can't find contributing guidelines and I don't know what branch to PR into. Feel free to take this code or write your own solution! I think there may be deeper potential issues here to address.)

It took took me hours to find this bug! Some search terms to help anyone else encountering this:

I encountered this using greenlock-express with dns wildcard certificates
Not particularly related, but I was also using greenlock-express as a reverse proxy and a https redirect. I searched those terms quite a bit as I thought something was wrong with the proxy library!
The bug was exposed as the first few requests to subdomains succeeding, then all following ones failing
Firefox would show PR_END_OF_FILE_ERROR. Chrome would show ERR_CONNECTION_CLOSED
Inspecting the TCP traffic showed a TLS ClientHello then the TCP connection suddenly closing on both sides.
My express app received no traffic, and the internal HTTP and HTTPS servers also received no traffic since node was dropping / killing / disconnecting the request early.

When a request for e.g. `x.example.com` under a `*.example.com` + `example.com` certificate is made, greenlock-express will: 1. cache `['x.example.com'] = { secureContext: { _valid: false } }` 2. generate correct context 3. cache... * `['*.example.com'] = correctContext` * `['example.com'] = correctContext` It notably *won't* ever cache the correct context for `x.example.com`, so every following request to `x.example.com` will fail. I wrote a quick fix here: https://git.rootprojects.org/Corecii/greenlock-express.js/commit/a05b702e6673164794f05be26ac3b4a6ce814300 (No PR because I can't find contributing guidelines and I don't know what branch to PR into. Feel free to take this code or write your own solution! I think there may be deeper potential issues here to address.) --- It took took me hours to find this bug! Some search terms to help anyone else encountering this: * I encountered this using greenlock-express with dns wildcard certificates * Not particularly related, but I was also using greenlock-express as a reverse proxy and a https redirect. I searched those terms quite a bit as I thought something was wrong with the proxy library! * The bug was exposed as the first few requests to subdomains succeeding, then all following ones failing * Firefox would show `PR_END_OF_FILE_ERROR`. Chrome would show `ERR_CONNECTION_CLOSED` * Inspecting the TCP traffic showed a TLS `ClientHello` then the TCP connection suddenly closing on both sides. * My express app received no traffic, and the internal HTTP and HTTPS servers also received no traffic since node was dropping / killing / disconnecting the request early.

Ghost changed title from ~~SNI cached incorrectly for wildcard domains~~ to SNI cached incorrectly for wildcard domains (w/ fix)

2021-04-23 10:04:07 +00:00

Ghost commented

2021-04-26 22:52:39 +00:00

@Corecii Thank you very much for this defect report. To add some additional details, I am seeing similar behavior. Using the stock greenlock code, I can get it working with the following:

Run greenlock add w/ just the wildcard subject and altname
Let greenlock successfully receive a certificate keypair for the wildcard subject
Run another greenlock add, this time adding the fully qualified subject as the altname
This allows the stock altname handling to populate the cache
I am worried that when it comes time to renew, we will get an error about having a collision between the wildcard in the subject and overlapping fully qualified names in the altnames

I believe your solution is much more stable @Corecii.

@Corecii Thank you very much for this defect report. To add some additional details, I am seeing similar behavior. Using the stock greenlock code, I can get it working with the following: 1. Run greenlock add w/ just the wildcard subject and altname 1. Let greenlock successfully receive a certificate keypair for the wildcard subject 1. Run another greenlock add, this time adding the fully qualified subject as the altname 1. This allows the stock altname handling to populate the cache 1. I am worried that when it comes time to renew, we will get an error about having a collision between the wildcard in the subject and overlapping fully qualified names in the altnames I believe your solution is much more stable @Corecii.

coolaj86 commented

2021-04-28 17:26:36 +00:00

Thanks for the report and the workaround.

coolaj86 commented

2021-04-28 17:31:49 +00:00

Hmm... I thnk what should happen is that the domain should be queried by both it's proper and wildcard names:

x.example.com => ['x.example.com', '*.example.com']

Perhaps the cache is getting in the way of that because it's having a valid, invalid cache hit on the incorrect name.

Hmm... I thnk what _should_ happen is that the domain _should_ be queried by both it's proper and wildcard names: - x.example.com => `['x.example.com', '*.example.com']` Perhaps the cache is getting in the way of that because it's having a valid, invalid cache hit on the incorrect name.

Ghost commented

2021-05-11 07:04:57 +00:00

Yep I having the exacte same behavior of @timfulmer . but like you said for the renew time we get the error, plus since i'm using a docker the certificate reload on each new deployment so it's kind of impossible to handle it.

For for the solution of @Corecii work perfectly for me and the site config is just the wildcard with subject and altname.

I tried multiple way to make it work without the fix :

subject: .mydomain.com | alternames ['.mydomain.com', 'X.mydomain.com']
this return me the Error: [400] Error creating new order :: Domain name "x.mydomain.com" is redundant with a wildcard domain in the same request. Remove one or the other from the certificate request.
subject mydomain.com | alternames ['*.mydomain.com', 'mydomain.com']
this doesn't get an error but Chrome show ERR_CONNECTION_CLOSED.
I also try to add the alternames via my manager.js on the mergeOrCreateSite() but still get the Error: [400] Error creating new order :: Domain name "x.mydomain.com" is redundant with a wildcard domain in the same request. Remove one or the other from the certificate request.

So far only the @Corecii solution seems to work for me.

@coolaj86 maybe you have a solution for it by using the manager.js ? cause since i'm using a docker container I reload the sni.js file from the source.

Yep I having the exacte same behavior of @timfulmer . but like you said for the renew time we get the error, plus since i'm using a docker the certificate reload on each new deployment so it's kind of impossible to handle it. For for the solution of @Corecii work perfectly for me and the site config is just the wildcard with subject and altname. I tried multiple way to make it work without the fix : - subject: *.mydomain.com | alternames ['*.mydomain.com', 'X.mydomain.com'] this return me the Error: [400] Error creating new order :: Domain name "x.mydomain.com" is redundant with a wildcard domain in the same request. Remove one or the other from the certificate request. - subject mydomain.com | alternames ['*.mydomain.com', 'mydomain.com'] this doesn't get an error but Chrome show ERR_CONNECTION_CLOSED. - I also try to add the alternames via my manager.js on the mergeOrCreateSite() but still get the Error: [400] Error creating new order :: Domain name "x.mydomain.com" is redundant with a wildcard domain in the same request. Remove one or the other from the certificate request. So far only the @Corecii solution seems to work for me. @coolaj86 maybe you have a solution for it by using the manager.js ? cause since i'm using a docker container I reload the sni.js file from the source.

Sign in to join this conversation.

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: root/greenlock-express.js#74