I fixed warn() and changed certproc.c to be less confusing.
On 21/04/2020 21:17, Sebastian Benoit wrote:
Bartosz Kuzma([email protected]) on 2020.04.21 20:59:54 +0200:
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.
The warn() should be a warnx() then, because strchr() does not set errno.
When it returns NULL it just means that it did not find the string. So
warnx("invalid certificate chain");
would be a good error message here i think.
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 25 Apr 2020 09:52:11 -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 25 Apr 2020 09:52:11 -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 25 Apr 2020 09:52:11 -0000
@@ -28,7 +28,8 @@
#include "extern.h"
-#define MARKER "-----END CERTIFICATE-----\n"
+#define BEGIN_MARKER "-----BEGIN CERTIFICATE-----"
+#define END_MARKER "-----END CERTIFICATE-----"
int
certproc(int netsock, int filesock)
@@ -81,19 +82,25 @@ certproc(int netsock, int filesock)
if ((csr = readbuf(netsock, COMM_CSR, &csrsz)) == NULL)
goto out;
- if (csrsz < strlen(MARKER)) {
+ if (csrsz < strlen(END_MARKER)) {
warnx("invalid cert");
goto out;
}
- chaincp = strstr(csr, MARKER);
+ chaincp = strstr(csr, END_MARKER);
if (chaincp == NULL) {
warnx("invalid cert");
goto out;
}
- chaincp += strlen(MARKER);
+ chaincp += strlen(END_MARKER);
+
+ if ((chaincp = strstr(chaincp, BEGIN_MARKER)) == NULL) {
+ warnx("invalid certificate chain");
+ 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 25 Apr 2020 09:52:11 -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 25 Apr 2020 09:52:11 -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 25 Apr 2020 09:52:12 -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 25 Apr 2020 09:52:12 -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 25 Apr 2020 09:52:12 -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},
