Re: acme-client(1) make -F flag use more obvious

[Stuart Henderson]

On 2020/12/15 17:49, Florian Obser wrote:
> 
> 
> On 15 December 2020 14:56:41 CET, Stuart Henderson  
> wrote:
> >On 2020/12/15 10:18, Solene Rapenne wrote:
> >> This is a small change to acme-client(1) because I find
> >> the explanation of -F flag not being obvious that you
> >> need it when you add/remove an alternative name in your
> >> domain config.
> >
> >This only works directly for adding. For removal you need to rm
> >the existing certificate.
> 
> -F only handles forced renewals correctly.
> That it can be (ab)used to add subject alt names to a cert is an 
> implementation detail.
> 
> It would be nice if someone™ would fix this properly by acme-client detecting 
> that cert and config do not agree anymore.

like this perhaps?

if we don't want to do this automatically for some reason, then we should
at least extend beck's recent change so that -F handles names that are
_removed_ from config, not just added as he did.



Index: revokeproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/revokeproc.c,v
retrieving revision 1.16
diff -u -p -r1.16 revokeproc.c
--- revokeproc.c18 Nov 2020 20:54:43 -  1.16
+++ revokeproc.c23 Dec 2020 22:20:43 -
@@ -202,7 +202,9 @@ revokeproc(int fd, const char *certfile,
 
if (san == NULL) {
warnx("%s: does not have a SAN entry", certfile);
-   goto out;
+   if (revocate)
+   goto out;
+   force = 2;
}
 
/* An array of buckets: the number of entries found. */
@@ -230,20 +232,29 @@ revokeproc(int fd, const char *certfile,
if (strcmp(tok, alts[j]) == 0)
break;
if (j == altsz) {
-   warnx("%s: unknown SAN entry: %s", certfile, tok);
-   goto out;
+   if (revocate) {
+   warnx("%s: unknown SAN entry: %s", certfile, 
tok);
+   goto out;
+   }
+   force = 2;
}
if (found[j]++) {
-   warnx("%s: duplicate SAN entry: %s", certfile, tok);
-   goto out;
+   if (revocate) {
+   warnx("%s: duplicate SAN entry: %s", certfile, 
tok);
+   goto out;
+   }
+   force = 2;
}
}
 
-   for (j = 0; !force && j < altsz; j++) {
+   for (j = 0; j < altsz; j++) {
if (found[j])
continue;
-   warnx("%s: domain not listed: %s", certfile, alts[j]);
-   goto out;
+   if (revocate) {
+   warnx("%s: domain not listed: %s", certfile, alts[j]);
+   goto out;
+   }
+   force = 2;
}
 
/*
@@ -294,7 +305,8 @@ revokeproc(int fd, const char *certfile,
certfile, (long long)(t - time(NULL)) / 24 / 60 / 60);
 
if (rop == REVOKE_OK && force) {
-   warnx("%s: forcing renewal", certfile);
+   warnx("%s: %sforcing renewal", certfile,
+   force == 2 ? "domain list changed, " : "");
rop = REVOKE_EXP;
}
 
Index: acme-client.1
===
RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
retrieving revision 1.38
diff -u -p -r1.38 acme-client.1
--- acme-client.1   19 Dec 2020 18:05:44 -  1.38
+++ acme-client.1   23 Dec 2020 22:20:43 -
@@ -67,10 +67,8 @@ location "/.well-known/acme-challenge/*"
 The options are as follows:
 .Bl -tag -width Ds
 .It Fl F
-Force certificate renewal, even if it's too soon.
-This is required if new domain alternative names
-were added to
-.Xr acme-client.conf 5 .
+Force certificate renewal, even if it has more than 30 days
+validity.
 .It Fl f Ar configfile
 Specify an alternative configuration file.
 .It Fl n

Re: acme-client(1) make -F flag use more obvious

[Renaud Allard]

On 12/16/20 11:13 AM, Janne Johansson wrote:
Den ons 16 dec. 2020 kl 10:42 skrev Renaud Allard >:


 > While there, I propose to change the proposed crontab to once a day
 > instead of every hour. The certificates can be renewed 1 full month
 > before expiracy, I think trying to renew every hour is too much.

I think that, while waiting for someone to fix acme-client correctly as
suggested by Florian, this patch is worth committing.
The crontab change in particular is quite useful, there is really no
reason to check every hour (even every day is probably too much
already).


But it is a local check for the local date vs the date in the 
certificate, and perhaps your box is not on at 03:00 on Saturdays as you 
thought 3 months ago.




If your clock is 3 months off, it could also be off the other way round. 
That means you would try to renew every hour and get blacklisted for 
hitting rate limits. I don't think the example crontab should take into 
account a wrong config in the first place.




smime.p7s
Description: S/MIME Cryptographic Signature

Re: acme-client(1) make -F flag use more obvious

[Renaud Allard]

On 12/16/20 9:44 AM, Solene Rapenne wrote:

On Tue, 15 Dec 2020 10:18:41 +0100
Solene Rapenne :


This is a small change to acme-client(1) because I find
the explanation of -F flag not being obvious that you
need it when you add/remove an alternative name in your
domain config.

Maybe wording could be better, if a native English
speaker could give it a look.

ok?



I added 's to domain and specified -F only works for new domains.

While there, I propose to change the proposed crontab to once a day
instead of every hour. The certificates can be renewed 1 full month
before expiracy, I think trying to renew every hour is too much.



I think that, while waiting for someone to fix acme-client correctly as 
suggested by Florian, this patch is worth committing.
The crontab change in particular is quite useful, there is really no 
reason to check every hour (even every day is probably too much already).




smime.p7s
Description: S/MIME Cryptographic Signature

Re: acme-client(1) make -F flag use more obvious

[Solene Rapenne]

On Tue, 15 Dec 2020 10:18:41 +0100
Solene Rapenne :

> This is a small change to acme-client(1) because I find
> the explanation of -F flag not being obvious that you
> need it when you add/remove an alternative name in your
> domain config.
> 
> Maybe wording could be better, if a native English
> speaker could give it a look.
> 
> ok?
> 

I added 's to domain and specified -F only works for new domains.

While there, I propose to change the proposed crontab to once a day
instead of every hour. The certificates can be renewed 1 full month
before expiracy, I think trying to renew every hour is too much.

Index: acme-client.1
===
RCS file: /home/reposync/src/usr.sbin/acme-client/acme-client.1,v
retrieving revision 1.36
diff -u -p -r1.36 acme-client.1
--- acme-client.1   4 Nov 2020 10:34:18 -   1.36
+++ acme-client.1   16 Dec 2020 08:42:36 -
@@ -68,6 +68,9 @@ The options are as follows:
 .Bl -tag -width Ds
 .It Fl F
 Force certificate renewal, even if it's too soon.
+This is required if new domain's alternatives names
+were added to
+.Xr acme-client.conf 5 .
 .It Fl f Ar configfile
 Specify an alternative configuration file.
 .It Fl n
@@ -123,7 +126,7 @@ On renewal,
 .Xr httpd 8
 is reloaded:
 .Bd -literal -offset indent
-~  *   *   *   *   acme-client example.com && \e
+~  ~   *   *   *   acme-client example.com && \e
rcctl reload httpd
 .Ed
 .Sh SEE ALSO

Re: acme-client(1) make -F flag use more obvious

[Stuart Henderson]

On 2020/12/16 11:47, Renaud Allard wrote:
> On 12/16/20 11:13 AM, Janne Johansson wrote:
> > 
> > But it is a local check for the local date vs the date in the
> > certificate, and perhaps your box is not on at 03:00 on Saturdays as you
> > thought 3 months ago.
> > 
> 
> If your clock is 3 months off, it could also be off the other way round.
> That means you would try to renew every hour and get blacklisted for hitting
> rate limits. I don't think the example crontab should take into account a
> wrong config in the first place.
> 

JJ isn't talking about the clock being set incorrectly, he's talking
about the machine being turned off. Even part time servers (say, a test
server running on a laptop) may still need a signed certificate.

If the machine clock is correct then there's no issue, it is a very
quick local file check only.

If the clock is incorrect then, for letsencrypt, the relevant limit is
the Duplicate Certificate limit, which is 5 per week, so a daily check
will still trip this. I'd argue that it is better to know sooner rather
than later if there is a problem as it will give you more time to fix it
before the certificate expires.

Re: acme-client(1) make -F flag use more obvious

[Janne Johansson]

Den ons 16 dec. 2020 kl 10:42 skrev Renaud Allard :

> > While there, I propose to change the proposed crontab to once a day
> > instead of every hour. The certificates can be renewed 1 full month
> > before expiracy, I think trying to renew every hour is too much.
>
> I think that, while waiting for someone to fix acme-client correctly as
> suggested by Florian, this patch is worth committing.
> The crontab change in particular is quite useful, there is really no
> reason to check every hour (even every day is probably too much already).
>

But it is a local check for the local date vs the date in the certificate,
and perhaps your box is not on at 03:00 on Saturdays as you thought 3
months ago.

-- 
May the most significant bit of your life be positive.

Re: acme-client(1) make -F flag use more obvious

[Florian Obser]

On 15 December 2020 14:56:41 CET, Stuart Henderson  wrote:
>On 2020/12/15 10:18, Solene Rapenne wrote:
>> This is a small change to acme-client(1) because I find
>> the explanation of -F flag not being obvious that you
>> need it when you add/remove an alternative name in your
>> domain config.
>
>This only works directly for adding. For removal you need to rm
>the existing certificate.

-F only handles forced renewals correctly.
That it can be (ab)used to add subject alt names to a cert is an implementation 
detail.

It would be nice if someone™ would fix this properly by acme-client detecting 
that cert and config do not agree anymore.

-- 
Sent from a mobile device. Please excuse poor formating.

Re: acme-client(1) make -F flag use more obvious

[Stuart Henderson]

On 2020/12/15 10:18, Solene Rapenne wrote:
> This is a small change to acme-client(1) because I find
> the explanation of -F flag not being obvious that you
> need it when you add/remove an alternative name in your
> domain config.

This only works directly for adding. For removal you need to rm
the existing certificate.

Re: acme-client(1) make -F flag use more obvious

[Jason McIntyre]

On Tue, Dec 15, 2020 at 10:18:41AM +0100, Solene Rapenne wrote:
> This is a small change to acme-client(1) because I find
> the explanation of -F flag not being obvious that you
> need it when you add/remove an alternative name in your
> domain config.
> 
> Maybe wording could be better, if a native English
> speaker could give it a look.
> 
> ok?
> 

hi.

i think your text reads fine. perhaps "domain's alternative names" (with
an apostrophe) would be better, but otherwise ok/

jmc

> Index: acme-client.1
> ===
> RCS file: /home/reposync/src/usr.sbin/acme-client/acme-client.1,v
> retrieving revision 1.36
> diff -u -p -r1.36 acme-client.1
> --- acme-client.1 4 Nov 2020 10:34:18 -   1.36
> +++ acme-client.1 15 Dec 2020 09:14:07 -
> @@ -68,6 +68,9 @@ The options are as follows:
>  .Bl -tag -width Ds
>  .It Fl F
>  Force certificate renewal, even if it's too soon.
> +This is required if the domain alternatives names changed
> +in
> +.Xr acme-client.conf 5 .
>  .It Fl f Ar configfile
>  Specify an alternative configuration file.
>  .It Fl n
> 

acme-client(1) make -F flag use more obvious

[Solene Rapenne]

This is a small change to acme-client(1) because I find
the explanation of -F flag not being obvious that you
need it when you add/remove an alternative name in your
domain config.

Maybe wording could be better, if a native English
speaker could give it a look.

ok?

Index: acme-client.1
===
RCS file: /home/reposync/src/usr.sbin/acme-client/acme-client.1,v
retrieving revision 1.36
diff -u -p -r1.36 acme-client.1
--- acme-client.1   4 Nov 2020 10:34:18 -   1.36
+++ acme-client.1   15 Dec 2020 09:14:07 -
@@ -68,6 +68,9 @@ The options are as follows:
 .Bl -tag -width Ds
 .It Fl F
 Force certificate renewal, even if it's too soon.
+This is required if the domain alternatives names changed
+in
+.Xr acme-client.conf 5 .
 .It Fl f Ar configfile
 Specify an alternative configuration file.
 .It Fl n