For the mailing list archive: fixed in commits usr.sbin/acme-client/main.c rev 1.42 and 1.43.
Thanks fo reporting this bug. /Benno Wolf([email protected]) on 2019.03.08 11:44:58 +0100: > Hi, > me again with another thing about acme-client. This time I run into an > issue with how it determines certdir. It uses > > if ((certdir = dirname(domain->cert)) != NULL) { > if ((certdir = strdup(certdir)) == NULL) > > which is wrong according to POSIX spec. Citing from > http://pubs.opengroup.org/onlinepubs/9699919799/ : > > The dirname() function may modify the string pointed to by path, and may > return a pointer to internal storage. > > On linux (at least on Alpine with musl-c), it actually does modify the > argument. That means that it changes `domain->cert', which introduces > issues later on. > > I assume this currently is not an issue on openbsd since it probably > takes the second route (`... a pointer to internal storage.'). > > This patch fixes the issue for me: > > diff --git a/main.c b/main.c > index fb895f1..d682658 100644 > --- a/main.c > +++ b/main.c > @@ -37,7 +37,7 @@ int > main(int argc, char *argv[]) > { > const char **alts = NULL; > - char *certdir = NULL, *certfile = NULL; > + char *certdir = NULL, *certdir_src = NULL, *certfile = > NULL; > char *chainfile = NULL, *fullchainfile = NULL; > char *acctkey = NULL; > char *chngdir = NULL, *auth = NULL; > @@ -104,20 +104,13 @@ main(int argc, char *argv[]) > argc--; > argv++; > > - if (domain->cert != NULL) { > - if ((certdir = dirname(domain->cert)) != NULL) { > - if ((certdir = strdup(certdir)) == NULL) > - err(EXIT_FAILURE, "strdup"); > - } else > - err(EXIT_FAILURE, "dirname"); > - } else { > - /* the parser enforces that at least cert or fullchain is set > */ > - if ((certdir = dirname(domain->fullchain)) != NULL) { > - if ((certdir = strdup(certdir)) == NULL) > - err(EXIT_FAILURE, "strdup"); > - } else > - err(EXIT_FAILURE, "dirname"); > - > + /* the parser enforces that at least cert or fullchain is set */ > + certdir_src = domain->cert ? domain->cert : domain->fullchain; > + if ((certdir_src = strdup(certdir_src)) == NULL) { > + err(EXIT_FAILURE, "strdup"); > + } > + if ((certdir = dirname(certdir_src)) == NULL) { > + err(EXIT_FAILURE, "dirname"); > } > > if (domain->cert != NULL) { > @@ -415,6 +408,7 @@ main(int argc, char *argv[]) > checkexit(pids[COMP_DNS], COMP_DNS) + > checkexit(pids[COMP_REVOKE], COMP_REVOKE); > > + free(certdir_src); > free(alts); > return rc != COMP__MAX ? EXIT_FAILURE : (c == 2 ? EXIT_SUCCESS : 2); > usage: > > > > Output of `acme-client -ADFv db.wolfsden.cz' before > > ... > acme-client: /etc/acme/acme: created > acme-client: /etc/acme/db.wolfsden.cz.pem: created > > and after: > > ... > acme-client: /etc/acme/db.wolfsden.cz.crt: created > acme-client: /etc/acme/db.wolfsden.cz.pem: created > > I'm not sure if you consider this a problem, but thought I would let you > know. > > Have a nice day, > Gray Wolf > > -- > There are only two hard things in Computer Science: > cache invalidation, naming things and off-by-one errors.
