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

Open
by Ghost opened 3 years ago · 4 comments
Ghost commented 3 years ago

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: 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) 3 years ago

@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
  2. Let greenlock successfully receive a certificate keypair for the wildcard subject
  3. Run another greenlock add, this time adding the fully qualified subject as the altname
  4. This allows the stock altname handling to populate the cache
  5. 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.
Owner

Thanks for the report and the workaround.

Thanks for the report and the workaround.
Owner

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.

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

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.