checkAsync found existing certificates - bug when adding an additional alt name to an existing cert #17

Closed
by Ghost opened 6 years ago · 3 comments
Ghost commented 6 years ago

Whenever a new certificate is requested that appends an additional alternative name to an already generated certificate, greenlock incorrectly responds with the local copy of the certificate whenever the first domain in the new array is the same as the previous.

To reproduce;

  1. Request a certificate with a single domain - test123.com
  2. Request a new certificate with the following;
    [
    test123.com,
    test456.com
    ]

Greenlock will respond with "checkAsync found existing certificates" incorrectly. If you switch the order in the array so test456.com is first, it will generate a new certificate. It needs to check all domains in the array to verify if an existing certificate exists.

Whenever a new certificate is requested that appends an additional alternative name to an already generated certificate, greenlock incorrectly responds with the local copy of the certificate whenever the first domain in the new array is the same as the previous. To reproduce; 1. Request a certificate with a single domain - test123.com 2. Request a new certificate with the following; [ test123.com, test456.com ] Greenlock will respond with "checkAsync found existing certificates" incorrectly. If you switch the order in the array so test456.com is first, it will generate a new certificate. It needs to check all domains in the array to verify if an existing certificate exists.

The existing code is simple and assumes that a certificate set (the list of domains) is fixed and is how many cert auths work too. The design is the first matching domain is checked and a cert match is successful.

Effectively a certificate is a signed document against a time, validity and list of domains so if you change the list of domains you have invalidated the old certificate and the new certificate is a different issue. It is possible to query the cert itself but this is a very expensive operation so best avoided.

You might be able to do a force refresh if you read through the code...

In general does your use case require you to reissue the certificate with additional domain? If call limits to LE allow you to do it, it would be better to issue and renew individual certificates for the various domains and refresh them independently of each other.

This will give you domain mobility should your hosting needs become more complex.

If you want to extend the list of domains, you need to delete the old certs and issue a fresh challenge/response with the current code.

Alternatively, have a look at the plugins for more sophisticated implementations which record and check individual domains, the list of total domains, and so on. If you find that you need to amend the greenlock core code to provide your plugin with more info so that you can fulfill your requirement then create a pull request but I recommend trying to work with the code you have got - i.e. certificate per domain list rather than extending the list of domains - as additional complexity can create a maintenance overhead and a lag in that maintenance going into production.

In particular the complexity can increase the testing overhead - particularly since it needs to be done against publicly resolvable IP addresses and domains.

The existing code is simple and assumes that a certificate set (the list of domains) is fixed and is how many cert auths work too. The design is the first matching domain is checked and a cert match is successful. Effectively a certificate is a signed document against a time, validity and list of domains so if you change the list of domains you have invalidated the old certificate and the new certificate is a different issue. It is possible to query the cert itself but this is a very expensive operation so best avoided. You might be able to do a force refresh if you read through the code... In general does your use case require you to reissue the certificate with additional domain? If call limits to LE allow you to do it, it would be better to issue and renew individual certificates for the various domains and refresh them independently of each other. This will give you domain mobility should your hosting needs become more complex. If you want to extend the list of domains, you need to delete the old certs and issue a fresh challenge/response with the current code. Alternatively, have a look at the plugins for more sophisticated implementations which record and check individual domains, the list of total domains, and so on. If you find that you need to amend the greenlock core code to provide your plugin with more info so that you can fulfill your requirement then create a pull request but I recommend trying to work with the code you have got - i.e. certificate per domain list rather than extending the list of domains - as additional complexity can create a maintenance overhead and a lag in that maintenance going into production. In particular the complexity can increase the testing overhead - particularly since it needs to be done against publicly resolvable IP addresses and domains.
Owner

If you watch the second tutorial video at https://git.coolaj86.com/coolaj86/greenlock-express.js you can watch me swap out the approveDomains array for the approveDomains function, which is what I recommend you do.

I'm considering ways to simplify the function usage of approveDomains so that it can work as a short and sweet example, but doesn't break backwards compatibility or limit it's capabilities.

I'm also working up the motivation to provide updated default plugins.

If you watch the second tutorial video at https://git.coolaj86.com/coolaj86/greenlock-express.js you can watch me swap out the `approveDomains` array for the `approveDomains` function, which is what I recommend you do. I'm considering ways to simplify the function usage of `approveDomains` so that it can work as a short and sweet example, but doesn't break backwards compatibility or limit it's capabilities. I'm also working up the motivation to provide updated default plugins.
Owner

I just did a whole bunch of cleanup so that the default plugins now work with wildcards and multiple domains per certificate without a hitch.

See

I'm going to close out this issue due to inactivity, but feel free to re-open if you still have trouble.

I just did a whole bunch of cleanup so that the default plugins now work with wildcards and multiple domains per certificate without a hitch. See * https://git.coolaj86.com/coolaj86/greenlock-express.js/src/branch/master/examples/wildcard.js * https://git.coolaj86.com/coolaj86/le-store-fs.js * https://git.coolaj86.com/coolaj86/le-challenge-dns.js I'm going to close out this issue due to inactivity, but feel free to re-open if you still have trouble.
coolaj86 closed this issue 5 years ago
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.