Hello,
thanks for looking at this!
On 21/04/2020 17:43, Florian Obser wrote:
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?
It seems that chain can be provided with "\n" or "\r\n" line separator.
I removed it from MARKER to handle both cases.
--
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.
For me one contact is also enough.
.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.
Because I removed new line character from MARKER strchr() just move
pointer to -----BEGIN CERTIFICATE---- to skip any new lines character
from chain. It can be replaced by strstr("-----BEGIN CERTIFICATE-----")
if this improve readability.
if ((chain = strdup(chaincp)) == NULL) {
warn("strdup");
goto out;
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.
You are right! I've improved json_fmt_newacc function and it should be
good now.
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.
For me having contact here is more generic approach and allow us to
handle more cases than just email. But if you prefer I can change it to
email.
The rest reads good, haven't tried it yet though.
Thanks,
Florian
Kind regards, Bartosz Kuzma.
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 21 Apr 2020 18:40:30 -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:[email protected]";
+}
+
+authority buypass-test {
+ api url "https://api.test4.buypass.no/acme/directory";
+ account key "/etc/acme/buypass-test-privkey.pem"
+ contact "mailto:[email protected]";
+}
+
domain example.com {
alternative names { secure.example.com }
domain key "/etc/ssl/private/example.com.key"
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 21 Apr 2020 18:40:31 -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.
.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 21 Apr 2020 18:40:31 -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;
+ }
+
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 21 Apr 2020 18:40:31 -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 21 Apr 2020 18:40:31 -0000
@@ -616,19 +616,32 @@ 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 = NULL;
+
+ if (contact != NULL) {
+ c = asprintf(&cnt, "\"contact\": [ \"%s\" ], ", contact);
+ if (c == -1) {
+ warn("asprintf");
+ goto err;
+ }
+ }
c = asprintf(&p, "{"
+ "%s"
"\"termsOfServiceAgreed\": true"
- "}");
+ "}", cnt == 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 21 Apr 2020 18:40:31 -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 21 Apr 2020 18:40:31 -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 21 Apr 2020 18:40:31 -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},
