Bartosz Kuzma(bartosz.ku...@release11.com) 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.
