Issue with renewal #22

New Issue

Ghost · 2018-10-20T13:05:44Z

Ghost commented

2018-10-20 13:05:44 +00:00

Hi,
I'm trying to renew a certificate for a wildcard domain using

var domainName = "example.com";
Greenlock.register({
    domains: [
        domainName,
        '*.'+domainName
    ],
    challengeType: 'dns-01',
    waitForRenewal: true
});

This creates the certificate on the folder live/example.com and is all good.

When we try to renew, this line
[https://git.coolaj86.com/coolaj86/greenlock.js/src/branch/master/lib/core.js#L355-L366] overwrites the domains array and so the renewal ends on this folder live/*.example.com because alt names are sorted alphabetically.

Maybe the IF should be something like

if(args.domains.length < certs.altnames.length) {

so you renew the full certificate if it doesn't contain the full list.

Hi, I'm trying to renew a certificate for a wildcard domain using var domainName = "example.com"; Greenlock.register({ domains: [ domainName, '*.'+domainName ], challengeType: 'dns-01', waitForRenewal: true }); This creates the certificate on the folder `live/example.com` and is all good. When we try to renew, this line [https://git.coolaj86.com/coolaj86/greenlock.js/src/branch/master/lib/core.js#L355-L366] overwrites the `domains` array and so the renewal ends on this folder `live/*.example.com` because alt names are sorted alphabetically. Maybe the IF should be something like if(args.domains.length < certs.altnames.length) { so you renew the full certificate if it doesn't contain the full list.

Ghost started working 2018-10-20 13:13:11 +00:00

Ghost canceled time tracking 2018-10-20 13:13:15 +00:00

coolaj86 commented

2018-11-05 07:31:13 +00:00

I think the reason I originally set that up was so that if people are using approveDomains as an array and they aren't doing anything else to actually make the renewal work as they probably want it to, that it would "do the right thing".

However, I see where that's a problem.

I'm thinking the best thing I can do is probably to remove it and update the docs and examples.

I think the worst that will happen is that some people will experience more renewals than they did previously, but I don't suspect anything would break.

What are your thoughts?

I think the reason I originally set that up was so that if people are using `approveDomains` as an array and they aren't doing anything else to actually make the renewal work as they probably want it to, that it would "do the right thing". However, I see where that's a problem. I'm thinking the best thing I can do is probably to remove it and update the docs and examples. I think the worst that will happen is that some people will experience more renewals than they did previously, but I don't suspect anything would break. What are your thoughts?

Ghost commented

2018-11-05 08:08:11 +00:00

In my situation when I call the library I'm sure that for every cert there is exactly 1 domain so it's not a problem as a short term solution.

However, looking further at it, if you are trying to fix the input data to renew a full cert when the domains don't corresponds to the ones on the cert, maybe the best thing to do is to get the original settings from the renewal.conf

In this way you don't mix up folders and you are sure of what the original domains are

In my situation when I call the library I'm sure that for every cert there is exactly 1 domain so it's not a problem as a short term solution. However, looking further at it, if you are trying to fix the input data to renew a full cert when the domains don't corresponds to the ones on the cert, maybe the best thing to do is to get the original settings from the renewal.conf In this way you don't mix up folders and you are sure of what the original domains are

coolaj86 commented

2018-11-05 08:27:01 +00:00

Well, part of the problem is that the plugin that handled renewal.conf just needs to die. It was based on the original python certbot client and I just haven't had enough motivation to do a rewrite.

I need to write something that doesn't rely on cert.subject, but rather should be able to use any of the cert.altnames to correctly relate the domains to the user account, as well as wildcards.

When I do I'll create a JSON store (for small devices) and a SQL store (for ephemeral cloud systems).

Well, part of the problem is that the plugin that handled `renewal.conf` just needs to die. It was based on the original python certbot client and I just haven't had enough motivation to do a rewrite. I need to write something that doesn't rely on `cert.subject`, but rather should be able to use any of the `cert.altnames` to correctly relate the domains to the user account, as well as wildcards. When I do I'll create a JSON store (for small devices) and a SQL store (for ephemeral cloud systems).

Ghost commented

2018-11-05 08:56:26 +00:00

So in the meantime I think this code should be removed because it makes false assumptions and "corrupts" the folder structure

Ghost commented

2019-02-02 20:59:41 +00:00

I'm having the same issue. Is there a way to look up what altnames are on a previously isusued cert, compare it manually, and if it's not an exact match, clear out the old cache folder to force a reissue?

coolaj86 commented

2019-02-05 05:16:58 +00:00

@noyearzero Yes, it's on the object in the approveDomains callback function.

@noyearzero Yes, it's on the object in the `approveDomains` callback function.

coolaj86 closed this issue

2019-02-05 05:16:58 +00:00

Ghost commented

2019-02-05 17:44:36 +00:00

I can't get that callback to trigger. I'm using greenlock.js v2.6.7

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: coolaj86/greenlock.js-ARCHIVED#22