On Fri, Jun 14, 2019 at 01:58:58PM +0200, Florian Obser wrote:
> On Fri, Jun 14, 2019 at 09:50:35AM +0200, Renaud Allard wrote:
> >
> >
> > On 6/12/19 2:30 PM, Renaud Allard wrote:
> > >
> > >
> > > On 6/11/19 2:36 PM, Sebastian Benoit wrote:
> > > > Hi,
> > > >
> > > > some feedback below.
> > > >
> > > > Renaud: maybe wait for feedback from florian or gilles until
> > > > acting on my comments, sometimes sending diffs to fast creates more
> > > > work ;)
> > > >
> > > > /Benno
> > > >
> > >
> > > As suggested by benno@
> > > removal of the global variable
> > > removal of KEYTYPE which was not used and was a leftover of a former patch
> > > define ECDSA_KEY to be more readable
> > >
> >
> > Any comment or OK on my latest patch?
> >
>
> I'd prefer to use enums like the rest of the code.
>
yes, ok gilles@
> diff --git extern.h extern.h
> index 17c6aa54f18..f6293a371ad 100644
> --- extern.h
> +++ extern.h
> @@ -207,7 +207,8 @@ int revokeproc(int, const char *, const
> char *,
> int, int, const char *const *, size_t);
> int fileproc(int, const char *, const char *, const char *,
> const char *);
> -int keyproc(int, const char *, const char **, size_t);
> +int keyproc(int, const char *, const char **, size_t,
> + enum keytype);
> int netproc(int, int, int, int, int, int, int,
> struct authority_c *, const char *const *,
> size_t);
> @@ -275,11 +276,6 @@ char *json_fmt_signed(const char *, const
> char *, const char *);
> */
> int verbose;
>
> -/*
> - * Should we switch to ecdsa?
> - */
> -int ecdsa;
> -
> /*
> * What component is the process within (COMP__MAX for none)?
> */
> diff --git keyproc.c keyproc.c
> index 9c392a0f3f6..f9ce081457a 100644
> --- keyproc.c
> +++ keyproc.c
> @@ -74,8 +74,8 @@ add_ext(STACK_OF(X509_EXTENSION) *sk, int nid, const char
> *value)
> * jail and, on success, ship it to "netsock" as an X509 request.
> */
> int
> -keyproc(int netsock, const char *keyfile,
> - const char **alts, size_t altsz)
> +keyproc(int netsock, const char *keyfile, const char **alts, size_t altsz,
> + enum keytype keytype)
> {
> char *der64 = NULL, *der = NULL, *dercp;
> char *sans = NULL, *san = NULL;
> @@ -117,14 +117,17 @@ keyproc(int netsock, const char *keyfile,
> }
>
> if (newkey) {
> - if (ecdsa) {
> + switch (keytype) {
> + case KT_ECDSA:
> if ((pkey = ec_key_create(f, keyfile)) == NULL)
> goto out;
> dodbg("%s: generated ECDSA domain key", keyfile);
> - } else {
> + break;
> + case KT_RSA:
> if ((pkey = rsa_key_create(f, keyfile)) == NULL)
> goto out;
> dodbg("%s: generated RSA domain key", keyfile);
> + break;
> }
> } else {
> if ((pkey = key_load(f, keyfile)) == NULL)
> diff --git main.c main.c
> index ea8f7c5d348..d70a7048f47 100644
> --- main.c
> +++ main.c
> @@ -49,7 +49,6 @@ main(int argc, char *argv[])
> int popts = 0;
> pid_t pids[COMP__MAX];
> extern int verbose;
> - extern int ecdsa;
> extern enum comp proccomp;
> size_t i, altsz, ne;
>
> @@ -148,10 +147,6 @@ main(int argc, char *argv[])
> errx(EXIT_FAILURE, "authority %s not found", auth);
> }
>
> - if (domain->keytype == 1) {
> - ecdsa = 1;
> - }
> -
> acctkey = authority->account;
>
> if ((chngdir = domain->challengedir) == NULL)
> @@ -258,7 +253,8 @@ main(int argc, char *argv[])
> close(file_fds[0]);
> close(file_fds[1]);
> c = keyproc(key_fds[0], domain->key,
> - (const char **)alts, altsz);
> + (const char **)alts, altsz,
> + domain->keytype);
> exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
> }
>
> diff --git parse.h parse.h
> index 78405590568..7f2d3ca546c 100644
> --- parse.h
> +++ parse.h
> @@ -27,6 +27,11 @@
> * limit all paths to PATH_MAX
> */
>
> +enum keytype {
> + KT_RSA = 0,
> + KT_ECDSA
> +};
> +
> struct authority_c {
> TAILQ_ENTRY(authority_c) entry;
> char *name;
> @@ -36,9 +41,9 @@ struct authority_c {
>
> struct domain_c {
> TAILQ_ENTRY(domain_c) entry;
> - TAILQ_HEAD(, altname_c) altname_list;
> - int altname_count;
> - int keytype;
> + TAILQ_HEAD(, altname_c) altname_list;
> + int altname_count;
> + enum keytype keytype;
> char *domain;
> char *key;
> char *cert;
> diff --git parse.y parse.y
> index 994492706bb..0b68a35fb73 100644
> --- parse.y
> +++ parse.y
> @@ -100,7 +100,7 @@ typedef struct {
> %}
>
> %token AUTHORITY URL API ACCOUNT
> -%token DOMAIN ALTERNATIVE NAMES CERT FULL CHAIN KEY SIGN WITH
> CHALLENGEDIR KEYTYPE
> +%token DOMAIN ALTERNATIVE NAMES CERT FULL CHAIN KEY SIGN WITH
> CHALLENGEDIR
> %token YES NO
> %token INCLUDE
> %token ERROR
> @@ -260,13 +260,15 @@ domain : DOMAIN STRING {
> }
> ;
>
> -keytype : RSA {
> - domain->keytype = 0;
> +keytype : RSA {
> + domain->keytype = KT_RSA;
> }
> | ECDSA {
> - domain->keytype = 1;
> + domain->keytype = KT_ECDSA;
> + }
> + | { /* nothing */
> + domain->keytype = KT_RSA;
> }
> - | /* nothing */
> ;
>
> domainopts_l : domainopts_l domainoptsl nl
>
>
> --
> I'm not entirely sure you are real.
>
--
Gilles Chehade @poolpOrg
https://www.poolp.org patreon: https://www.patreon.com/gilles
