Re: acme-client config grammar improvements
On Fri, Jul 24, 2020 at 10:47 AM Florian Obser wrote:
> On Thu, Jul 16, 2020 at 07:40:35AM +0200, Daniel Eisele wrote:
> > Also it would be nice to have a feature to update all domains of the
> > config file. I currently do that in a shell script by parsing the output
> > of acme-client -nv with sed and then calling acme-client multiple times.
> >
> > Maybe an easy solution would be an option that prints the list of all
> > domains, so I can avoid the sed parsing, as this is prone to breaking.
>
> I'm not opposed to that. You will probably need to output some form of
> csv.
>
> Consider this:
>
> domain handle1-example.com {
> domain name example.com
> alternative names { www.example.com secure.example.com }
> domain key "/etc/ssl..." rsa
> }
> domain handle2-example.com {
> domain name example.com
> alternative names { mail.example.com }
> domain key "/etc/ssl..." ecdsa
> }
>
> Should it be output like this?
>
> handle1-example.com; example.com; www.example.com, secure.example.com
> handle2-example.com; example.com; mail.example.com
>
> Or this?
>
> handle1-example.com; example.com; www.example.com
> handle1-example.com; example.com; secure.example.com
> handle2-example.com; example.com; mail.example.com
>
>
> >
> > Another solution is obviously to just add an "update all" command line
> > option (or maybe even in the config?), but that is probably more
> > complicated to implement.
>
> I'm more worried that you will very soon end up with some form of exec
> plugin mechanism. Typically you need to do something when a cert is
> renewed (restart daemon).
>
> My acme-client.conf is generate by a config management system which
> also creates individual cronjobs for each renew job and knows how to
> handle a cert renew.
>
> >
> > What do you think about that?
> >
>
A management system may auto reload services when the configuration files
changes , an update all would be convenient.
Moreover
Acme-client update && rcctl reload nginx
Once a week is easy , as acme-client will not return 0 if nothing is
changed .
> --
> I'm not entirely sure you are real.
>
> --
--
-
Knowing is not enough; we must apply. Willing is not enough; we must do
Re: acme-client config grammar improvements
On Thu, Jul 16, 2020 at 07:40:35AM +0200, Daniel Eisele wrote:
> Also it would be nice to have a feature to update all domains of the
> config file. I currently do that in a shell script by parsing the output
> of acme-client -nv with sed and then calling acme-client multiple times.
>
> Maybe an easy solution would be an option that prints the list of all
> domains, so I can avoid the sed parsing, as this is prone to breaking.
I'm not opposed to that. You will probably need to output some form of
csv.
Consider this:
domain handle1-example.com {
domain name example.com
alternative names { www.example.com secure.example.com }
domain key "/etc/ssl..." rsa
}
domain handle2-example.com {
domain name example.com
alternative names { mail.example.com }
domain key "/etc/ssl..." ecdsa
}
Should it be output like this?
handle1-example.com; example.com; www.example.com, secure.example.com
handle2-example.com; example.com; mail.example.com
Or this?
handle1-example.com; example.com; www.example.com
handle1-example.com; example.com; secure.example.com
handle2-example.com; example.com; mail.example.com
>
> Another solution is obviously to just add an "update all" command line
> option (or maybe even in the config?), but that is probably more
> complicated to implement.
I'm more worried that you will very soon end up with some form of exec
plugin mechanism. Typically you need to do something when a cert is
renewed (restart daemon).
My acme-client.conf is generate by a config management system which
also creates individual cronjobs for each renew job and knows how to
handle a cert renew.
>
> What do you think about that?
>
--
I'm not entirely sure you are real.
Re: acme-client config grammar improvements
On Tue, Jul 21, 2020 at 10:24:25PM +0200, Sebastian Benoit wrote:
> Daniel Eisele(daniel_eis...@gmx.de) on 2020.07.16 07:40:35 +0200:
> > Am 15.07.2020 um 23:51 schrieb Sebastian Benoit:
> > >> src/usr.sbin/acme-client/parse.y:
> > >> * The grammar allows the user to omit the newline after the first line
> > >> in a domain or authority block.
> > >
> > > Yes. I'm still pnodering this. What are the chances that someone does
> > > that?
> > >
> > > Probably no one does, but it worthwhile to break someones config for such
> > > a
> > > change?
> >
> > I can't imagine that anyone uses this, I only noticed it by reading the
> > grammar. But still not an important fix, I just noticed it and wanted to
> > point it out...
> >
> > >> src/usr.sbin/acme-client/dbg.c doesn't build because in the included
> > >> header file extern.h the type pid_t is missing (unistd.h).
> > >
> > > extern.h should #include for that, no?
> >
> > Yes, sys/types.h is better, sorry about that.
>
> So here is a patch i would like to commit.
>
> ok?
>
> diff --git usr.sbin/acme-client/dnsproc.c usr.sbin/acme-client/dnsproc.c
> index 664ef8d9b8b..72a0ea6b30e 100644
> --- usr.sbin/acme-client/dnsproc.c
> +++ usr.sbin/acme-client/dnsproc.c
> @@ -16,6 +16,7 @@
> */
>
> #include
> +#include
> #include
>
> #include
> diff --git usr.sbin/acme-client/extern.h usr.sbin/acme-client/extern.h
> index 529d3350205..76c86b5cce0 100644
> --- usr.sbin/acme-client/extern.h
> +++ usr.sbin/acme-client/extern.h
> @@ -17,6 +17,8 @@
> #ifndef EXTERN_H
> #define EXTERN_H
>
> +#include
> +
> #include "parse.h"
>
these two chunks are OK florian
> #define MAX_SERVERS_DNS 8
> diff --git usr.sbin/acme-client/parse.y usr.sbin/acme-client/parse.y
> index 120f253a63f..0b96794f8da 100644
> --- usr.sbin/acme-client/parse.y
> +++ usr.sbin/acme-client/parse.y
> @@ -170,11 +170,11 @@ varset : STRING '=' string {
> }
> ;
>
> -optnl: '\n' optnl
> +optnl: nl
> |
> ;
>
> -nl : '\n' optnl/* one newline or more */
> +nl : optnl '\n'/* one newline or more */
> ;
>
I'm not a fan of this.
We currently have two different optnl implementations in 24 parsers
(hostappd and radiusd are weird):
optnl : '\n' optnl
|
;
and
optnl : /* empty */
| '\n' optnl
;
I don't think we need a third one. Also it shouldn't depend on nl
because nl is not implemented in all parsers.
Can we go with this and leave nl alone because nl is not recursive
itself so there is no problem there.
optnl : /* empty */
| optnl '\n'
;
I think we can copy that to all other parse.y.
The rest is OK florian
> comma: ','
> @@ -190,7 +190,7 @@ authority : AUTHORITY STRING {
> yyerror("authority already defined");
> YYERROR;
> }
> - } '{' optnl authorityopts_l '}' {
> + } '{' optnl authorityopts_l optnl '}' {
> if (auth->api == NULL) {
> yyerror("authority %s: no api URL specified",
> auth->name);
> @@ -205,8 +205,8 @@ authority : AUTHORITY STRING {
> }
> ;
>
> -authorityopts_l : authorityopts_l authorityoptsl nl
> - | authorityoptsl optnl
> +authorityopts_l : authorityopts_l nl authorityoptsl
> + | authorityoptsl
> ;
>
> authorityoptsl : API URL STRING {
> @@ -246,7 +246,7 @@ domain: DOMAIN STRING {
> yyerror("domain already defined");
> YYERROR;
> }
> - } '{' optnl domainopts_l '}' {
> + } '{' optnl domainopts_l optnl '}' {
> if (domain->domain == NULL) {
> if ((domain->domain = strdup(domain->handle))
> == NULL)
> @@ -273,8 +273,8 @@ keytype : RSA { $$ = KT_RSA; }
> | { $$ = KT_RSA; }
> ;
>
> -domainopts_l : domainopts_l domainoptsl nl
> - | domainoptsl optnl
> +domainopts_l : domainopts_l nl domainoptsl
> + | domainoptsl
> ;
>
> domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
> @@ -385,7 +385,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
> }
> ;
>
> -altname_l: altname comma altname_l
> +altname_l: altname_l comma altname
> | altname
> ;
>
--
I'm not entirely sure you are real.
Re: acme-client config grammar improvements
Daniel Eisele(daniel_eis...@gmx.de) on 2020.07.16 07:40:35 +0200:
> Am 15.07.2020 um 23:51 schrieb Sebastian Benoit:
> >> src/usr.sbin/acme-client/parse.y:
> >> * The grammar allows the user to omit the newline after the first line
> >> in a domain or authority block.
> >
> > Yes. I'm still pnodering this. What are the chances that someone does that?
> >
> > Probably no one does, but it worthwhile to break someones config for such a
> > change?
>
> I can't imagine that anyone uses this, I only noticed it by reading the
> grammar. But still not an important fix, I just noticed it and wanted to
> point it out...
>
> >> src/usr.sbin/acme-client/dbg.c doesn't build because in the included
> >> header file extern.h the type pid_t is missing (unistd.h).
> >
> > extern.h should #include for that, no?
>
> Yes, sys/types.h is better, sorry about that.
So here is a patch i would like to commit.
ok?
diff --git usr.sbin/acme-client/dnsproc.c usr.sbin/acme-client/dnsproc.c
index 664ef8d9b8b..72a0ea6b30e 100644
--- usr.sbin/acme-client/dnsproc.c
+++ usr.sbin/acme-client/dnsproc.c
@@ -16,6 +16,7 @@
*/
#include
+#include
#include
#include
diff --git usr.sbin/acme-client/extern.h usr.sbin/acme-client/extern.h
index 529d3350205..76c86b5cce0 100644
--- usr.sbin/acme-client/extern.h
+++ usr.sbin/acme-client/extern.h
@@ -17,6 +17,8 @@
#ifndef EXTERN_H
#define EXTERN_H
+#include
+
#include "parse.h"
#define MAX_SERVERS_DNS 8
diff --git usr.sbin/acme-client/parse.y usr.sbin/acme-client/parse.y
index 120f253a63f..0b96794f8da 100644
--- usr.sbin/acme-client/parse.y
+++ usr.sbin/acme-client/parse.y
@@ -170,11 +170,11 @@ varset: STRING '=' string {
}
;
-optnl : '\n' optnl
+optnl : nl
|
;
-nl : '\n' optnl/* one newline or more */
+nl : optnl '\n'/* one newline or more */
;
comma : ','
@@ -190,7 +190,7 @@ authority : AUTHORITY STRING {
yyerror("authority already defined");
YYERROR;
}
- } '{' optnl authorityopts_l '}' {
+ } '{' optnl authorityopts_l optnl '}' {
if (auth->api == NULL) {
yyerror("authority %s: no api URL specified",
auth->name);
@@ -205,8 +205,8 @@ authority : AUTHORITY STRING {
}
;
-authorityopts_l: authorityopts_l authorityoptsl nl
- | authorityoptsl optnl
+authorityopts_l: authorityopts_l nl authorityoptsl
+ | authorityoptsl
;
authorityoptsl : API URL STRING {
@@ -246,7 +246,7 @@ domain : DOMAIN STRING {
yyerror("domain already defined");
YYERROR;
}
- } '{' optnl domainopts_l '}' {
+ } '{' optnl domainopts_l optnl '}' {
if (domain->domain == NULL) {
if ((domain->domain = strdup(domain->handle))
== NULL)
@@ -273,8 +273,8 @@ keytype : RSA { $$ = KT_RSA; }
| { $$ = KT_RSA; }
;
-domainopts_l : domainopts_l domainoptsl nl
- | domainoptsl optnl
+domainopts_l : domainopts_l nl domainoptsl
+ | domainoptsl
;
domainoptsl: ALTERNATIVE NAMES '{' altname_l '}'
@@ -385,7 +385,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
}
;
-altname_l : altname comma altname_l
+altname_l : altname_l comma altname
| altname
;
Re: acme-client config grammar improvements
Am 16.07.2020 um 07:49 schrieb Paul de Weerd:
> What I have is this:
>
> for DOMAIN in `awk '/^domain/ {print $2}' /etc/acme-client.conf`
My script is similar to this...
> As I need to update OCSP staples per domain anyway, doing a per-domain
> song-and-dance makes sense for me. Since I set the 'handle' of the
> domain to the actual (DNS) domain name (no need for the domain name
> option in the file), this works out quite well.
Yes, handling this externally is probably best...
> | Maybe an easy solution would be an option that prints the list of all
> | domains, so I can avoid the sed parsing, as this is prone to breaking.
>
> Why is it prone to breaking? You can only break it yourself by
> introducing changes that break your script. Sounds like a case of
> "doctor, it hurts when I do X"?
Well, my script actually broke twice, once with the introduction of the
-nv option and once with the introduction of the domain name option.
As you've showed, this can be fixed by just parsing the config file
itself instead of the acme-client output, but it still might be cleaner
to have this handled by acme-client with the proper parser...
Re: acme-client config grammar improvements
On Thu, Jul 16, 2020 at 07:40:35AM +0200, Daniel Eisele wrote:
| Also it would be nice to have a feature to update all domains of the
| config file. I currently do that in a shell script by parsing the output
| of acme-client -nv with sed and then calling acme-client multiple times.
What I have is this:
for DOMAIN in `awk '/^domain/ {print $2}' /etc/acme-client.conf`
As I need to update OCSP staples per domain anyway, doing a per-domain
song-and-dance makes sense for me. Since I set the 'handle' of the
domain to the actual (DNS) domain name (no need for the domain name
option in the file), this works out quite well.
| Maybe an easy solution would be an option that prints the list of all
| domains, so I can avoid the sed parsing, as this is prone to breaking.
Why is it prone to breaking? You can only break it yourself by
introducing changes that break your script. Sounds like a case of
"doctor, it hurts when I do X"?
Cheers,
Paul 'WEiRD' de Weerd
--
>[<++>-]<+++.>+++[<-->-]<.>+++[<+
+++>-]<.>++[<>-]<+.--.[-]
http://www.weirdnet.nl/
Re: acme-client config grammar improvements
Am 15.07.2020 um 23:51 schrieb Sebastian Benoit: >> src/usr.sbin/acme-client/parse.y: >> * The grammar allows the user to omit the newline after the first line >> in a domain or authority block. > > Yes. I'm still pnodering this. What are the chances that someone does that? > > Probably no one does, but it worthwhile to break someones config for such a > change? I can't imagine that anyone uses this, I only noticed it by reading the grammar. But still not an important fix, I just noticed it and wanted to point it out... >> src/usr.sbin/acme-client/dbg.c doesn't build because in the included >> header file extern.h the type pid_t is missing (unistd.h). > > extern.h should #include for that, no? Yes, sys/types.h is better, sorry about that. Also it would be nice to have a feature to update all domains of the config file. I currently do that in a shell script by parsing the output of acme-client -nv with sed and then calling acme-client multiple times. Maybe an easy solution would be an option that prints the list of all domains, so I can avoid the sed parsing, as this is prone to breaking. Another solution is obviously to just add an "update all" command line option (or maybe even in the config?), but that is probably more complicated to implement. What do you think about that?
Re: acme-client config grammar improvements
Daniel Eisele(daniel_eis...@gmx.de) on 2020.07.15 09:32:37 +0200:
> Hi,
>
> I'm currently porting acme-client to FreeBSD and while doing that I've
> noticed a few small issues.
>
> src/usr.sbin/acme-client/parse.y:
> * The grammar allows the user to omit the newline after the first line
> in a domain or authority block.
Yes. I'm still pnodering this. What are the chances that someone does that?
Probably no one does, but it worthwhile to break someones config for such a
change?
> * The grammar uses right recursion for lists, left recursion is
> probably the better option here
> (https://docs.oracle.com/cd/E19504-01/802-5880/yacc-13/index.html)
>
> src/usr.sbin/acme-client/dbg.c doesn't build because in the included
> header file extern.h the type pid_t is missing (unistd.h).
extern.h should #include for that, no?
> src/usr.sbin/acme-client/dnsproc.c also fails to build because
> struct sockaddr_in and struct sockaddr_in6 are missing (netinet/in.h).
correct.
> The missing header files only cause a build failure on FreeBSD, but I
> think it would still be a good idea to fix them in OpenBSD...
>
> Regards,
> Daniel Eisele
>
> --- parse.y.orig 2020-07-10 12:36:30 UTC
> +++ parse.y
> @@ -170,11 +170,11 @@ varset : STRING '=' string {
> }
> ;
>
> -optnl: '\n' optnl
> +optnl: nl
> |
> ;
>
> -nl : '\n' optnl/* one newline or more */
> +nl : optnl '\n'/* one newline or more */
> ;
>
> comma: ','
> @@ -190,7 +190,7 @@ authority : AUTHORITY STRING {
> yyerror("authority already defined");
> YYERROR;
> }
> - } '{' optnl authorityopts_l '}' {
> + } '{' optnl authorityopts_l optnl '}' {
> if (auth->api == NULL) {
> yyerror("authority %s: no api URL specified",
> auth->name);
> @@ -205,8 +205,8 @@ authority : AUTHORITY STRING {
> }
> ;
>
> -authorityopts_l : authorityopts_l authorityoptsl nl
> - | authorityoptsl optnl
> +authorityopts_l : authorityopts_l nl authorityoptsl
> + | authorityoptsl
> ;
>
> authorityoptsl : API URL STRING {
> @@ -246,7 +246,7 @@ domain: DOMAIN STRING {
> yyerror("domain already defined");
> YYERROR;
> }
> - } '{' optnl domainopts_l '}' {
> + } '{' optnl domainopts_l optnl '}' {
> if (domain->domain == NULL) {
> if ((domain->domain = strdup(domain->handle))
> == NULL)
> @@ -273,8 +273,8 @@ keytype : RSA { $$ = KT_RSA; }
> | { $$ = KT_RSA; }
> ;
>
> -domainopts_l : domainopts_l domainoptsl nl
> - | domainoptsl optnl
> +domainopts_l : domainopts_l nl domainoptsl
> + | domainoptsl
> ;
>
> domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
> @@ -385,7 +385,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
> }
> ;
>
> -altname_l: altname comma altname_l
> +altname_l: altname_l comma altname
> | altname
> ;
>
acme-client config grammar improvements
Hi,
I'm currently porting acme-client to FreeBSD and while doing that I've
noticed a few small issues.
src/usr.sbin/acme-client/parse.y:
* The grammar allows the user to omit the newline after the first line
in a domain or authority block.
* The grammar uses right recursion for lists, left recursion is
probably the better option here
(https://docs.oracle.com/cd/E19504-01/802-5880/yacc-13/index.html)
src/usr.sbin/acme-client/dbg.c doesn't build because in the included
header file extern.h the type pid_t is missing (unistd.h).
src/usr.sbin/acme-client/dnsproc.c also fails to build because
struct sockaddr_in and struct sockaddr_in6 are missing (netinet/in.h).
The missing header files only cause a build failure on FreeBSD, but I
think it would still be a good idea to fix them in OpenBSD...
Regards,
Daniel Eisele
--- parse.y.orig2020-07-10 12:36:30 UTC
+++ parse.y
@@ -170,11 +170,11 @@ varset: STRING '=' string {
}
;
-optnl : '\n' optnl
+optnl : nl
|
;
-nl : '\n' optnl/* one newline or more */
+nl : optnl '\n'/* one newline or more */
;
comma : ','
@@ -190,7 +190,7 @@ authority : AUTHORITY STRING {
yyerror("authority already defined");
YYERROR;
}
- } '{' optnl authorityopts_l '}' {
+ } '{' optnl authorityopts_l optnl '}' {
if (auth->api == NULL) {
yyerror("authority %s: no api URL specified",
auth->name);
@@ -205,8 +205,8 @@ authority : AUTHORITY STRING {
}
;
-authorityopts_l: authorityopts_l authorityoptsl nl
- | authorityoptsl optnl
+authorityopts_l: authorityopts_l nl authorityoptsl
+ | authorityoptsl
;
authorityoptsl : API URL STRING {
@@ -246,7 +246,7 @@ domain : DOMAIN STRING {
yyerror("domain already defined");
YYERROR;
}
- } '{' optnl domainopts_l '}' {
+ } '{' optnl domainopts_l optnl '}' {
if (domain->domain == NULL) {
if ((domain->domain = strdup(domain->handle))
== NULL)
@@ -273,8 +273,8 @@ keytype : RSA { $$ = KT_RSA; }
| { $$ = KT_RSA; }
;
-domainopts_l : domainopts_l domainoptsl nl
- | domainoptsl optnl
+domainopts_l : domainopts_l nl domainoptsl
+ | domainoptsl
;
domainoptsl: ALTERNATIVE NAMES '{' altname_l '}'
@@ -385,7 +385,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
}
;
-altname_l : altname comma altname_l
+altname_l : altname_l comma altname
| altname
;
