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.
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.
