Hi, thanks for working on this and finding another acme implementor!
On Mon, Apr 20, 2020 at 06:51:17PM +0200, Bartosz Kuzma wrote: > Hello, > > I've tried to get a certificate from Buypass Go SSL provider using > acme-client(1) but it ends with the following error: > > acme-client: https://api.buypass.com/acme-v02/new-acct: bad HTTP: 400 > acme-client: transfer buffer: > [{"type":"urn:ietf:params:acme:error:malformed","d > etail":"Email is a required > contact","code":400,"message":"MALFORMED_BAD_REQUEST > ","details":"HTTP 400 Bad Request"}] (164 bytes) > > Buypass Go SSL requires that optional field contact in Account Objects > will contain email address. > > I've added new option "contact" to acme-client.conf to fullfil this > requirement. It is also required to change MARKER in certproc.c to fix line > endings handling in pem files. they are leaving out the last newline? > > -- > Kind regards, Bartosz Kuzma. > Index: usr.sbin/acme-client/acme-client.conf.5 > =================================================================== > RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v > retrieving revision 1.22 > diff -u -p -r1.22 acme-client.conf.5 > --- usr.sbin/acme-client/acme-client.conf.5 10 Feb 2020 13:18:21 -0000 > 1.22 > +++ usr.sbin/acme-client/acme-client.conf.5 20 Apr 2020 16:24:46 -0000 > @@ -98,6 +98,10 @@ It defaults to > Specify the > .Ar url > under which the ACME API is reachable. > +.It Ic contact Ar contact > +Optional > +.Ar contact > +URLs that the authority can use to contact the client for issues related to > this account. I think we should call it emails. That's what people actually have. According to the RFC it's a list of contacts, but I'm fine with having only one. > .El > .Sh DOMAINS > The certificates to be obtained through ACME. > Index: usr.sbin/acme-client/certproc.c > =================================================================== > RCS file: /cvs/src/usr.sbin/acme-client/certproc.c,v > retrieving revision 1.12 > diff -u -p -r1.12 certproc.c > --- usr.sbin/acme-client/certproc.c 7 Jun 2019 08:07:52 -0000 1.12 > +++ usr.sbin/acme-client/certproc.c 20 Apr 2020 16:24:46 -0000 > @@ -28,7 +28,7 @@ > > #include "extern.h" > > -#define MARKER "-----END CERTIFICATE-----\n" > +#define MARKER "-----END CERTIFICATE-----" > > int > certproc(int netsock, int filesock) > @@ -94,6 +94,12 @@ certproc(int netsock, int filesock) > } > > chaincp += strlen(MARKER); > + > + if ((chaincp = strchr(chaincp, '-')) == NULL) { > + warn("strchr"); > + goto out; > + } > + I don't quite understand what this is doing. > if ((chain = strdup(chaincp)) == NULL) { > warn("strdup"); > goto out; > Index: usr.sbin/acme-client/extern.h > =================================================================== > RCS file: /cvs/src/usr.sbin/acme-client/extern.h,v > retrieving revision 1.17 > diff -u -p -r1.17 extern.h > --- usr.sbin/acme-client/extern.h 7 Feb 2020 14:34:15 -0000 1.17 > +++ usr.sbin/acme-client/extern.h 20 Apr 2020 16:24:46 -0000 > @@ -261,13 +261,13 @@ int json_parse_capaths(struct jsmnn *, > > char *json_fmt_newcert(const char *); > char *json_fmt_chkacc(void); > -char *json_fmt_newacc(void); > +char *json_fmt_newacc(const char *); > char *json_fmt_neworder(const char *const *, size_t); > char *json_fmt_protected_rsa(const char *, > const char *, const char *, const char *); > char *json_fmt_protected_ec(const char *, const char *, const char *, > const char *); > -char *json_fmt_protected_kid(const char*, const char *, const char *, > +char *json_fmt_protected_kid(const char *, const char *, const char > *, > const char *); > char *json_fmt_revokecert(const char *); > char *json_fmt_thumb_rsa(const char *, const char *); > Index: usr.sbin/acme-client/json.c > =================================================================== > RCS file: /cvs/src/usr.sbin/acme-client/json.c,v > retrieving revision 1.16 > diff -u -p -r1.16 json.c > --- usr.sbin/acme-client/json.c 22 Jan 2020 22:25:22 -0000 1.16 > +++ usr.sbin/acme-client/json.c 20 Apr 2020 16:24:46 -0000 > @@ -616,19 +616,30 @@ json_fmt_chkacc(void) > * Format the "newAccount" resource request. > */ > char * > -json_fmt_newacc(void) > +json_fmt_newacc(const char *contact) > { > int c; > - char *p; > + char *p, *cnt; > + > + c = asprintf(&cnt, "\"contact\": [ \"%s\" ], ", contact); > + if (c == -1) { > + warn("asprintf"); > + goto err; > + } > bzzzrt, you can't do this. contact might be NULL, you already need to check it here, not just further down. In this particular case I think it's fine do something like this, makes the error handling easier, you don't need a goto etc. if (contact == NULL) { p = strdup("{" "\"termsOfServiceAgreed\": true" "}"); } else { c = asprintf(&p, "{" "\"contact\": [ \"mailto:%s\"; ], " "\"termsOfServiceAgreed\": true" "}", contact); if (c == -1) p = NULL; } yes, we have two places now for the template string, but they are close enough together and I don't see them change that often (read: never). Also not the mailto, I think we should just put an email address in the config file and fill in the mailto ourselves. The rest reads good, haven't tried it yet though. Thanks, Florian > c = asprintf(&p, "{" > + "%s" > "\"termsOfServiceAgreed\": true" > - "}"); > + "}", contact == NULL ? "" : cnt); > + free(cnt); > if (c == -1) { > warn("asprintf"); > p = NULL; > } > return p; > + > +err: > + return NULL; > } > > /* > Index: usr.sbin/acme-client/netproc.c > =================================================================== > RCS file: /cvs/src/usr.sbin/acme-client/netproc.c,v > retrieving revision 1.25 > diff -u -p -r1.25 netproc.c > --- usr.sbin/acme-client/netproc.c 11 Aug 2019 19:44:25 -0000 1.25 > +++ usr.sbin/acme-client/netproc.c 20 Apr 2020 16:24:46 -0000 > @@ -368,13 +368,13 @@ sreq(struct conn *c, const char *addr, i > * Returns non-zero on success. > */ > static int > -donewacc(struct conn *c, const struct capaths *p) > +donewacc(struct conn *c, const struct capaths *p, const char *contact) > { > int rc = 0; > char *req; > long lc; > > - if ((req = json_fmt_newacc()) == NULL) > + if ((req = json_fmt_newacc(contact)) == NULL) > warnx("json_fmt_newacc"); > else if ((lc = sreq(c, p->newaccount, 0, req, &c->kid)) < 0) > warnx("%s: bad comm", p->newaccount); > @@ -397,7 +397,7 @@ donewacc(struct conn *c, const struct ca > * Returns non-zero on success. > */ > static int > -dochkacc(struct conn *c, const struct capaths *p) > +dochkacc(struct conn *c, const struct capaths *p, const char *contact) > { > int rc = 0; > char *req; > @@ -412,7 +412,7 @@ dochkacc(struct conn *c, const struct ca > else if (c->buf.buf == NULL || c->buf.sz == 0) > warnx("%s: empty response", p->newaccount); > else if (lc == 400) > - rc = donewacc(c, p); > + rc = donewacc(c, p, contact); > else > rc = 1; > > @@ -742,7 +742,7 @@ netproc(int kfd, int afd, int Cfd, int c > c.newnonce = paths.newnonce; > > /* Check if our account already exists or create it. */ > - if (!dochkacc(&c, &paths)) > + if (!dochkacc(&c, &paths, authority->contact)) > goto out; > > /* > Index: usr.sbin/acme-client/parse.h > =================================================================== > RCS file: /cvs/src/usr.sbin/acme-client/parse.h,v > retrieving revision 1.13 > diff -u -p -r1.13 parse.h > --- usr.sbin/acme-client/parse.h 17 Jun 2019 12:42:52 -0000 1.13 > +++ usr.sbin/acme-client/parse.h 20 Apr 2020 16:24:46 -0000 > @@ -38,6 +38,7 @@ struct authority_c { > char *api; > char *account; > enum keytype keytype; > + char *contact; > }; > > struct domain_c { > Index: usr.sbin/acme-client/parse.y > =================================================================== > RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v > retrieving revision 1.39 > diff -u -p -r1.39 parse.y > --- usr.sbin/acme-client/parse.y 27 Dec 2019 16:56:40 -0000 1.39 > +++ usr.sbin/acme-client/parse.y 20 Apr 2020 16:24:47 -0000 > @@ -100,7 +100,7 @@ typedef struct { > > %} > > -%token AUTHORITY URL API ACCOUNT > +%token AUTHORITY URL API ACCOUNT CONTACT > %token DOMAIN ALTERNATIVE NAMES CERT FULL CHAIN KEY SIGN WITH > CHALLENGEDIR > %token YES NO > %token INCLUDE > @@ -230,6 +230,16 @@ authorityoptsl : API URL STRING { > auth->account = s; > auth->keytype = $4; > } > + | CONTACT STRING { > + char *s; > + if (auth->contact != NULL) { > + yyerror("duplicate contact"); > + YYERROR; > + } > + if ((s = strdup($2)) == NULL) > + err(EXIT_FAILURE, "strdup"); > + auth->contact = s; > + } > ; > > domain : DOMAIN STRING { > @@ -437,6 +447,7 @@ lookup(char *s) > {"certificate", CERT}, > {"chain", CHAIN}, > {"challengedir", CHALLENGEDIR}, > + {"contact", CONTACT}, > {"domain", DOMAIN}, > {"ecdsa", ECDSA}, > {"full", FULL}, > Index: etc/examples/acme-client.conf > =================================================================== > RCS file: /cvs/src/etc/examples/acme-client.conf,v > retrieving revision 1.2 > diff -u -p -r1.2 acme-client.conf > --- etc/examples/acme-client.conf 7 Jun 2019 08:08:30 -0000 1.2 > +++ etc/examples/acme-client.conf 20 Apr 2020 16:24:56 -0000 > @@ -11,6 +11,18 @@ authority letsencrypt-staging { > account key "/etc/acme/letsencrypt-staging-privkey.pem" > } > > +authority buypass { > + api url "https://api.buypass.com/acme/directory"; > + account key "/etc/acme/buypass-privkey.pem" > + contact "mailto:j...@example.net"; > +} > + > +authority buypass-test { > + api url "https://api.test4.buypass.no/acme/directory"; > + account key "/etc/acme/buypass-test-privkey.pem" > + contact "mailto:j...@example.net"; > +} > + > domain example.com { > alternative names { secure.example.com } > domain key "/etc/ssl/private/example.com.key" -- I'm not entirely sure you are real.
