acme-client(1): remove A and D flags

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

acme-client(1): remove A and D flags

Florian Obser-2


Remove A and D flag, they are superfluous.
One could always use them on the command line and acme-client would do
the right thing.

IIRC this is a leftover from when we moved to the config file and we
never mopped this up.

It's kinda getting in the way and kinda an itch I have to scratch.

OK?

diff --git acctproc.c acctproc.c
index 23d3a026c04..de32d968dcd 100644
--- acctproc.c
+++ acctproc.c
@@ -311,13 +311,13 @@ out:
 }
 
 int
-acctproc(int netsock, const char *acctkey, int newacct)
+acctproc(int netsock, const char *acctkey)
 {
  FILE *f = NULL;
  EVP_PKEY *pkey = NULL;
  long lval;
  enum acctop op;
- int rc = 0, cc;
+ int rc = 0, cc, newacct = 0;
  mode_t prev;
 
  /*
@@ -327,7 +327,10 @@ acctproc(int netsock, const char *acctkey, int newacct)
  */
 
  prev = umask((S_IWUSR | S_IXUSR) | S_IRWXG | S_IRWXO);
- f = fopen(acctkey, newacct ? "wx" : "r");
+ if ((f = fopen(acctkey, "r")) == NULL && errno == ENOENT) {
+ f = fopen(acctkey, "wx");
+ newacct = 1;
+ }
  umask(prev);
 
  if (f == NULL) {
diff --git acme-client.1 acme-client.1
index fd4cde3133e..ba8c16bc60a 100644
--- acme-client.1
+++ acme-client.1
@@ -22,7 +22,7 @@
 .Nd ACME client
 .Sh SYNOPSIS
 .Nm acme-client
-.Op Fl ADFnrv
+.Op Fl Fnrv
 .Op Fl f Ar configfile
 .Ar domain
 .Sh DESCRIPTION
@@ -40,16 +40,6 @@ The certificates are typically used to provide HTTPS for web servers,
 but can be used in any situation where domain name validation is required
 (such as mail servers).
 .Pp
-Before a certificate can be requested, an account key needs to be
-created using the
-.Fl A
-argument.
-The first time a certificate is requested, a domain key needs to be created with
-.Fl D .
-So a typical invocation the first time it's run would be:
-.Pp
-.Dl # acme-client -ADv example.com
-.Pp
 If the certificate already exists and is less than 30 days from expiry,
 .Nm
 attempts to renew the certificate.
@@ -76,10 +66,6 @@ location "/.well-known/acme-challenge/*" {
 .Pp
 The options are as follows:
 .Bl -tag -width Ds
-.It Fl A
-Create a new RSA account key if one does not already exist.
-.It Fl D
-Create a new RSA domain key if one does not already exist.
 .It Fl F
 Force certificate renewal, even if it's too soon.
 .It Fl f Ar configfile
@@ -127,7 +113,7 @@ and
 .Pa httpd.conf
 and run:
 .Pp
-.Dl # acme-client -ADv example.com && rcctl reload httpd
+.Dl # acme-client -v example.com && rcctl reload httpd
 .Pp
 A
 .Xr cron 8
diff --git extern.h extern.h
index c7a11195530..a61f55e897a 100644
--- extern.h
+++ extern.h
@@ -199,7 +199,7 @@ __BEGIN_DECLS
  * Start with our components.
  * These are all isolated and talk to each other using sockets.
  */
-int acctproc(int, const char *, int);
+int acctproc(int, const char *);
 int certproc(int, int);
 int chngproc(int, const char *);
 int dnsproc(int);
@@ -207,8 +207,7 @@ 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);
+int keyproc(int, const char *, const char **, size_t);
 int netproc(int, int, int, int, int, int, int,
  struct authority_c *, const char *const *,
  size_t);
diff --git keyproc.c keyproc.c
index 1d18bdeb408..3e7075434da 100644
--- keyproc.c
+++ keyproc.c
@@ -75,7 +75,7 @@ add_ext(STACK_OF(X509_EXTENSION) *sk, int nid, const char *value)
  */
 int
 keyproc(int netsock, const char *keyfile,
-    const char **alts, size_t altsz, int newkey)
+    const char **alts, size_t altsz)
 {
  char *der64 = NULL, *der = NULL, *dercp;
  char *sans = NULL, *san = NULL;
@@ -85,7 +85,7 @@ keyproc(int netsock, const char *keyfile,
  EVP_PKEY *pkey = NULL;
  X509_REQ *x = NULL;
  X509_NAME *name = NULL;
- int len, rc = 0, cc, nid;
+ int len, rc = 0, cc, nid, newkey = 0;
  mode_t prev;
  STACK_OF(X509_EXTENSION) *exts = NULL;
 
@@ -96,7 +96,10 @@ keyproc(int netsock, const char *keyfile,
  */
 
  prev = umask((S_IWUSR | S_IXUSR) | S_IRWXG | S_IRWXO);
- f = fopen(keyfile, newkey ? "wx" : "r");
+ if ((f = fopen(keyfile, "r")) == NULL && errno == ENOENT) {
+ f = fopen(keyfile, "wx");
+ newkey = 1;
+ }
  umask(prev);
 
  if (f == NULL) {
diff --git main.c main.c
index 9c301f586e5..bf9d51c182e 100644
--- main.c
+++ main.c
@@ -57,14 +57,8 @@ main(int argc, char *argv[])
  struct domain_c *domain = NULL;
  struct altname_c *ac;
 
- while ((c = getopt(argc, argv, "ADFnrvf:")) != -1)
+ while ((c = getopt(argc, argv, "Fnrvf:")) != -1)
  switch (c) {
- case 'A':
- popts |= ACME_OPT_NEWACCT;
- break;
- case 'D':
- popts |= ACME_OPT_NEWDKEY;
- break;
  case 'F':
  force = 1;
  break;
@@ -173,28 +167,11 @@ main(int argc, char *argv[])
  ne++;
  }
 
- if (!(popts & ACME_OPT_NEWDKEY) && access(domain->key, R_OK) == -1) {
- warnx("%s: domain key file must exist", domain->key);
- ne++;
- } else if ((popts & ACME_OPT_NEWDKEY) && access(domain->key, R_OK)
-    != -1) {
- dodbg("%s: domain key exists (not creating)", domain->key);
- popts &= ~ACME_OPT_NEWDKEY;
- }
-
  if (access(chngdir, R_OK) == -1) {
  warnx("%s: challenge directory must exist", chngdir);
  ne++;
  }
 
- if (!(popts & ACME_OPT_NEWACCT) && access(acctkey, R_OK) == -1) {
- warnx("%s: account key file must exist", acctkey);
- ne++;
- } else if ((popts & ACME_OPT_NEWACCT) && access(acctkey, R_OK) != -1) {
- dodbg("%s: account key exists (not creating)", acctkey);
- popts &= ~ACME_OPT_NEWACCT;
- }
-
  if (ne > 0)
  return EXIT_FAILURE;
 
@@ -276,7 +253,7 @@ main(int argc, char *argv[])
  close(file_fds[0]);
  close(file_fds[1]);
  c = keyproc(key_fds[0], domain->key,
-    (const char **)alts, altsz, (popts & ACME_OPT_NEWDKEY));
+    (const char **)alts, altsz);
  exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
  }
 
@@ -295,7 +272,7 @@ main(int argc, char *argv[])
  close(chng_fds[0]);
  close(file_fds[0]);
  close(file_fds[1]);
- c = acctproc(acct_fds[0], acctkey, (popts & ACME_OPT_NEWACCT));
+ c = acctproc(acct_fds[0], acctkey);
  exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
  }
 
@@ -408,6 +385,6 @@ main(int argc, char *argv[])
  return rc != COMP__MAX ? EXIT_FAILURE : (c == 2 ? EXIT_SUCCESS : 2);
 usage:
  fprintf(stderr,
-    "usage: acme-client [-ADFnrv] [-f configfile] domain\n");
+    "usage: acme-client [-Fnrv] [-f configfile] domain\n");
  return EXIT_FAILURE;
 }
diff --git parse.h parse.h
index e371f191910..d1c1fee2515 100644
--- parse.h
+++ parse.h
@@ -58,9 +58,7 @@ struct keyfile {
 };
 
 #define ACME_OPT_VERBOSE 0x00000001
-#define ACME_OPT_NEWACCT 0x00000002
-#define ACME_OPT_NEWDKEY 0x00000004
-#define ACME_OPT_CHECK 0x00000008
+#define ACME_OPT_CHECK 0x00000004
 
 struct acme_conf {
  int opts;
diff --git parse.y parse.y
index 220269ed279..e3c375c15b0 100644
--- parse.y
+++ parse.y
@@ -29,6 +29,7 @@
 #include <sys/stat.h>
 #include <ctype.h>
 #include <err.h>
+#include <errno.h>
 #include <limits.h>
 #include <stdarg.h>
 #include <stdio.h>
@@ -69,7 +70,7 @@ struct domain_c *conf_new_domain(struct acme_conf *, char *);
 struct keyfile *conf_new_keyfile(struct acme_conf *, char *);
 void clear_config(struct acme_conf *);
 void print_config(struct acme_conf *);
-int conf_check_file(char *, int);
+int conf_check_file(char *);
 
 TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
 struct sym {
@@ -270,8 +271,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
  }
  if ((s = strdup($3)) == NULL)
  err(EXIT_FAILURE, "strdup");
- if (!conf_check_file(s,
-    (conf->opts & ACME_OPT_NEWDKEY))) {
+ if (!conf_check_file(s)) {
  free(s);
  YYERROR;
  }
@@ -1046,7 +1046,7 @@ domain_valid(const char *cp)
 }
 
 int
-conf_check_file(char *s, int dontstat)
+conf_check_file(char *s)
 {
  struct stat st;
 
@@ -1054,14 +1054,14 @@ conf_check_file(char *s, int dontstat)
  warnx("%s: not an absolute path", s);
  return 0;
  }
- if (dontstat)
- return 1;
  if (stat(s, &st)) {
+ if (errno == ENOENT)
+ return 1;
  warn("cannot stat %s", s);
  return 0;
  }
- if (st.st_mode & (S_IRWXG | S_IRWXO)) {
- warnx("%s: group read/writable or world read/writable", s);
+ if (st.st_mode & S_IRWXO) {
+ warnx("%s: world read/writable", s);
  return 0;
  }
  return 1;


--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: acme-client(1): remove A and D flags

Sebastian Benoit-3
Florian Obser([hidden email]) on 2019.06.07 19:52:21 +0200:

>
>
> Remove A and D flag, they are superfluous.
> One could always use them on the command line and acme-client would do
> the right thing.
>
> IIRC this is a leftover from when we moved to the config file and we
> never mopped this up.
>
> It's kinda getting in the way and kinda an itch I have to scratch.
>
> OK?

yes, ok

>
> diff --git acctproc.c acctproc.c
> index 23d3a026c04..de32d968dcd 100644
> --- acctproc.c
> +++ acctproc.c
> @@ -311,13 +311,13 @@ out:
>  }
>  
>  int
> -acctproc(int netsock, const char *acctkey, int newacct)
> +acctproc(int netsock, const char *acctkey)
>  {
>   FILE *f = NULL;
>   EVP_PKEY *pkey = NULL;
>   long lval;
>   enum acctop op;
> - int rc = 0, cc;
> + int rc = 0, cc, newacct = 0;
>   mode_t prev;
>  
>   /*
> @@ -327,7 +327,10 @@ acctproc(int netsock, const char *acctkey, int newacct)
>   */
>  
>   prev = umask((S_IWUSR | S_IXUSR) | S_IRWXG | S_IRWXO);
> - f = fopen(acctkey, newacct ? "wx" : "r");
> + if ((f = fopen(acctkey, "r")) == NULL && errno == ENOENT) {
> + f = fopen(acctkey, "wx");
> + newacct = 1;
> + }
>   umask(prev);
>  
>   if (f == NULL) {
> diff --git acme-client.1 acme-client.1
> index fd4cde3133e..ba8c16bc60a 100644
> --- acme-client.1
> +++ acme-client.1
> @@ -22,7 +22,7 @@
>  .Nd ACME client
>  .Sh SYNOPSIS
>  .Nm acme-client
> -.Op Fl ADFnrv
> +.Op Fl Fnrv
>  .Op Fl f Ar configfile
>  .Ar domain
>  .Sh DESCRIPTION
> @@ -40,16 +40,6 @@ The certificates are typically used to provide HTTPS for web servers,
>  but can be used in any situation where domain name validation is required
>  (such as mail servers).
>  .Pp
> -Before a certificate can be requested, an account key needs to be
> -created using the
> -.Fl A
> -argument.
> -The first time a certificate is requested, a domain key needs to be created with
> -.Fl D .
> -So a typical invocation the first time it's run would be:
> -.Pp
> -.Dl # acme-client -ADv example.com
> -.Pp
>  If the certificate already exists and is less than 30 days from expiry,
>  .Nm
>  attempts to renew the certificate.
> @@ -76,10 +66,6 @@ location "/.well-known/acme-challenge/*" {
>  .Pp
>  The options are as follows:
>  .Bl -tag -width Ds
> -.It Fl A
> -Create a new RSA account key if one does not already exist.
> -.It Fl D
> -Create a new RSA domain key if one does not already exist.
>  .It Fl F
>  Force certificate renewal, even if it's too soon.
>  .It Fl f Ar configfile
> @@ -127,7 +113,7 @@ and
>  .Pa httpd.conf
>  and run:
>  .Pp
> -.Dl # acme-client -ADv example.com && rcctl reload httpd
> +.Dl # acme-client -v example.com && rcctl reload httpd
>  .Pp
>  A
>  .Xr cron 8
> diff --git extern.h extern.h
> index c7a11195530..a61f55e897a 100644
> --- extern.h
> +++ extern.h
> @@ -199,7 +199,7 @@ __BEGIN_DECLS
>   * Start with our components.
>   * These are all isolated and talk to each other using sockets.
>   */
> -int acctproc(int, const char *, int);
> +int acctproc(int, const char *);
>  int certproc(int, int);
>  int chngproc(int, const char *);
>  int dnsproc(int);
> @@ -207,8 +207,7 @@ 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);
> +int keyproc(int, const char *, const char **, size_t);
>  int netproc(int, int, int, int, int, int, int,
>   struct authority_c *, const char *const *,
>   size_t);
> diff --git keyproc.c keyproc.c
> index 1d18bdeb408..3e7075434da 100644
> --- keyproc.c
> +++ keyproc.c
> @@ -75,7 +75,7 @@ add_ext(STACK_OF(X509_EXTENSION) *sk, int nid, const char *value)
>   */
>  int
>  keyproc(int netsock, const char *keyfile,
> -    const char **alts, size_t altsz, int newkey)
> +    const char **alts, size_t altsz)
>  {
>   char *der64 = NULL, *der = NULL, *dercp;
>   char *sans = NULL, *san = NULL;
> @@ -85,7 +85,7 @@ keyproc(int netsock, const char *keyfile,
>   EVP_PKEY *pkey = NULL;
>   X509_REQ *x = NULL;
>   X509_NAME *name = NULL;
> - int len, rc = 0, cc, nid;
> + int len, rc = 0, cc, nid, newkey = 0;
>   mode_t prev;
>   STACK_OF(X509_EXTENSION) *exts = NULL;
>  
> @@ -96,7 +96,10 @@ keyproc(int netsock, const char *keyfile,
>   */
>  
>   prev = umask((S_IWUSR | S_IXUSR) | S_IRWXG | S_IRWXO);
> - f = fopen(keyfile, newkey ? "wx" : "r");
> + if ((f = fopen(keyfile, "r")) == NULL && errno == ENOENT) {
> + f = fopen(keyfile, "wx");
> + newkey = 1;
> + }
>   umask(prev);
>  
>   if (f == NULL) {
> diff --git main.c main.c
> index 9c301f586e5..bf9d51c182e 100644
> --- main.c
> +++ main.c
> @@ -57,14 +57,8 @@ main(int argc, char *argv[])
>   struct domain_c *domain = NULL;
>   struct altname_c *ac;
>  
> - while ((c = getopt(argc, argv, "ADFnrvf:")) != -1)
> + while ((c = getopt(argc, argv, "Fnrvf:")) != -1)
>   switch (c) {
> - case 'A':
> - popts |= ACME_OPT_NEWACCT;
> - break;
> - case 'D':
> - popts |= ACME_OPT_NEWDKEY;
> - break;
>   case 'F':
>   force = 1;
>   break;
> @@ -173,28 +167,11 @@ main(int argc, char *argv[])
>   ne++;
>   }
>  
> - if (!(popts & ACME_OPT_NEWDKEY) && access(domain->key, R_OK) == -1) {
> - warnx("%s: domain key file must exist", domain->key);
> - ne++;
> - } else if ((popts & ACME_OPT_NEWDKEY) && access(domain->key, R_OK)
> -    != -1) {
> - dodbg("%s: domain key exists (not creating)", domain->key);
> - popts &= ~ACME_OPT_NEWDKEY;
> - }
> -
>   if (access(chngdir, R_OK) == -1) {
>   warnx("%s: challenge directory must exist", chngdir);
>   ne++;
>   }
>  
> - if (!(popts & ACME_OPT_NEWACCT) && access(acctkey, R_OK) == -1) {
> - warnx("%s: account key file must exist", acctkey);
> - ne++;
> - } else if ((popts & ACME_OPT_NEWACCT) && access(acctkey, R_OK) != -1) {
> - dodbg("%s: account key exists (not creating)", acctkey);
> - popts &= ~ACME_OPT_NEWACCT;
> - }
> -
>   if (ne > 0)
>   return EXIT_FAILURE;
>  
> @@ -276,7 +253,7 @@ main(int argc, char *argv[])
>   close(file_fds[0]);
>   close(file_fds[1]);
>   c = keyproc(key_fds[0], domain->key,
> -    (const char **)alts, altsz, (popts & ACME_OPT_NEWDKEY));
> +    (const char **)alts, altsz);
>   exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
>   }
>  
> @@ -295,7 +272,7 @@ main(int argc, char *argv[])
>   close(chng_fds[0]);
>   close(file_fds[0]);
>   close(file_fds[1]);
> - c = acctproc(acct_fds[0], acctkey, (popts & ACME_OPT_NEWACCT));
> + c = acctproc(acct_fds[0], acctkey);
>   exit(c ? EXIT_SUCCESS : EXIT_FAILURE);
>   }
>  
> @@ -408,6 +385,6 @@ main(int argc, char *argv[])
>   return rc != COMP__MAX ? EXIT_FAILURE : (c == 2 ? EXIT_SUCCESS : 2);
>  usage:
>   fprintf(stderr,
> -    "usage: acme-client [-ADFnrv] [-f configfile] domain\n");
> +    "usage: acme-client [-Fnrv] [-f configfile] domain\n");
>   return EXIT_FAILURE;
>  }
> diff --git parse.h parse.h
> index e371f191910..d1c1fee2515 100644
> --- parse.h
> +++ parse.h
> @@ -58,9 +58,7 @@ struct keyfile {
>  };
>  
>  #define ACME_OPT_VERBOSE 0x00000001
> -#define ACME_OPT_NEWACCT 0x00000002
> -#define ACME_OPT_NEWDKEY 0x00000004
> -#define ACME_OPT_CHECK 0x00000008
> +#define ACME_OPT_CHECK 0x00000004
>  
>  struct acme_conf {
>   int opts;
> diff --git parse.y parse.y
> index 220269ed279..e3c375c15b0 100644
> --- parse.y
> +++ parse.y
> @@ -29,6 +29,7 @@
>  #include <sys/stat.h>
>  #include <ctype.h>
>  #include <err.h>
> +#include <errno.h>
>  #include <limits.h>
>  #include <stdarg.h>
>  #include <stdio.h>
> @@ -69,7 +70,7 @@ struct domain_c *conf_new_domain(struct acme_conf *, char *);
>  struct keyfile *conf_new_keyfile(struct acme_conf *, char *);
>  void clear_config(struct acme_conf *);
>  void print_config(struct acme_conf *);
> -int conf_check_file(char *, int);
> +int conf_check_file(char *);
>  
>  TAILQ_HEAD(symhead, sym) symhead = TAILQ_HEAD_INITIALIZER(symhead);
>  struct sym {
> @@ -270,8 +271,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
>   }
>   if ((s = strdup($3)) == NULL)
>   err(EXIT_FAILURE, "strdup");
> - if (!conf_check_file(s,
> -    (conf->opts & ACME_OPT_NEWDKEY))) {
> + if (!conf_check_file(s)) {
>   free(s);
>   YYERROR;
>   }
> @@ -1046,7 +1046,7 @@ domain_valid(const char *cp)
>  }
>  
>  int
> -conf_check_file(char *s, int dontstat)
> +conf_check_file(char *s)
>  {
>   struct stat st;
>  
> @@ -1054,14 +1054,14 @@ conf_check_file(char *s, int dontstat)
>   warnx("%s: not an absolute path", s);
>   return 0;
>   }
> - if (dontstat)
> - return 1;
>   if (stat(s, &st)) {
> + if (errno == ENOENT)
> + return 1;
>   warn("cannot stat %s", s);
>   return 0;
>   }
> - if (st.st_mode & (S_IRWXG | S_IRWXO)) {
> - warnx("%s: group read/writable or world read/writable", s);
> + if (st.st_mode & S_IRWXO) {
> + warnx("%s: world read/writable", s);
>   return 0;
>   }
>   return 1;
>
>
> --
> I'm not entirely sure you are real.
>