Quantcast

[patch] Avoid system(3) in ikectl

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[patch] Avoid system(3) in ikectl

Matthew Martin
ikectl errors in a number of situations where shell special characters
are used. For example:

% doas ikectl ca test create password \'
[...]
subject=/C=DE/ST=Lower Saxony/L=Hanover/O=OpenBSD/OU=iked/CN=VPN CA/emailAddress=[hidden email]
Getting Private key
sh: no closing quote

This is because it uses system(3) in various places to run openssl, tar,
and zip. Take the hint from the system(3) man page, and write a small
function that does the fork and exec bypassing sh.

Keep in mind while reviewing ca->batch was either "" or "-batch " and is
now either "" or "-batch".

- Matthew Martin



diff --git ikeca.c ikeca.c
index cee6623a30f..69ca076407b 100644
--- ikeca.c
+++ ikeca.c
@@ -24,6 +24,7 @@
 #include <errno.h>
 #include <string.h>
 #include <stdlib.h>
+#include <sys/wait.h>
 #include <pwd.h>
 #include <fcntl.h>
 #include <fts.h>
@@ -68,7 +69,7 @@ struct ca {
  char serial[PATH_MAX];
  char sslcnf[PATH_MAX];
  char extcnf[PATH_MAX];
- char batch[PATH_MAX];
+ char batch[sizeof("-batch")];
  char *caname;
 };
 
@@ -117,6 +118,7 @@ void ca_setenv(const char *, const char *);
 void ca_clrenv(void);
 void ca_setcnf(struct ca *, const char *);
 void ca_create_index(struct ca *);
+int static run(int, ...);
 
 /* util.c */
 int expand_string(char *, size_t, const char *, const char *);
@@ -131,7 +133,6 @@ int
 ca_key_create(struct ca *ca, char *keyname)
 {
  struct stat st;
- char cmd[PATH_MAX * 2];
  char path[PATH_MAX];
 
  snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
@@ -141,10 +142,7 @@ ca_key_create(struct ca *ca, char *keyname)
  return (0);
  }
 
- snprintf(cmd, sizeof(cmd),
-    "%s genrsa -out %s 2048",
-    PATH_OPENSSL, path);
- system(cmd);
+ run(5, PATH_OPENSSL, "genrsa", "-out", path, "2048");
  chmod(path, 0600);
 
  return (0);
@@ -201,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
 int
 ca_request(struct ca *ca, char *keyname, int type)
 {
- char cmd[PATH_MAX * 2];
  char hostname[HOST_NAME_MAX+1];
  char name[128];
+ char key[PATH_MAX];
  char path[PATH_MAX];
 
  ca_setenv("$ENV::CERT_CN", keyname);
@@ -227,13 +225,11 @@ ca_request(struct ca *ca, char *keyname, int type)
 
  ca_setcnf(ca, keyname);
 
+ snprintf(key, sizeof(key), "%s/private/%s.key",  ca->sslpath, keyname);
  snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
- snprintf(cmd, sizeof(cmd), "%s req %s-new"
-    " -key %s/private/%s.key -out %s -config %s",
-    PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
-    path, ca->sslcnf);
 
- system(cmd);
+ run(10, PATH_OPENSSL, "req", ca->batch, "-new", "-key", key,
+    "-out", path, "-config", ca->sslcnf);
  chmod(path, 0600);
 
  return (0);
@@ -242,7 +238,11 @@ ca_request(struct ca *ca, char *keyname, int type)
 int
 ca_sign(struct ca *ca, char *keyname, int type)
 {
- char cmd[PATH_MAX * 2];
+ char keyfile[PATH_MAX];
+ char cert[PATH_MAX];
+ char out[PATH_MAX];
+ char in[PATH_MAX];
+ char passfile[PATH_MAX+5];
  const char *extensions = NULL;
 
  if (type == HOST_IPADDR) {
@@ -259,19 +259,16 @@ ca_sign(struct ca *ca, char *keyname, int type)
  ca_setenv("$ENV::CASERIAL", ca->serial);
  ca_setcnf(ca, keyname);
 
- snprintf(cmd, sizeof(cmd),
-    "%s ca -config %s -keyfile %s/private/ca.key"
-    " -cert %s/ca.crt"
-    " -extfile %s -extensions %s -out %s/%s.crt"
-    " -in %s/private/%s.csr"
-    " -passin file:%s -outdir %s -batch",
-    PATH_OPENSSL, ca->sslcnf, ca->sslpath,
-    ca->sslpath,
-    ca->extcnf, extensions, ca->sslpath, keyname,
-    ca->sslpath, keyname,
-    ca->passfile, ca->sslpath);
+ snprintf(keyfile, sizeof(keyfile), "%s/private/ca.key",  ca->sslpath);
+ snprintf(cert, sizeof(cert), "%s/ca.crt", ca->sslpath);
+ snprintf(out, sizeof(out), "%s/%s.crt", ca->sslpath, keyname);
+ snprintf(in, sizeof(in), "%s/private/%s.csr", ca->sslpath, keyname);
+ snprintf(passfile, sizeof(passfile), "file:%s", ca->passfile);
 
- system(cmd);
+ run(21, PATH_OPENSSL, "ca", "-config", ca->sslcnf, "-keyfile", keyfile,
+    "-cert", cert, "-extfile", ca->extcnf, "-extensions", extensions,
+    "-out", out, "-in", in, "-passin", passfile,
+    "-outdir", ca->sslpath, "-batch");
 
  return (0);
 }
@@ -314,9 +311,9 @@ int
 ca_key_install(struct ca *ca, char *keyname, char *dir)
 {
  struct stat st;
- char cmd[PATH_MAX * 2];
  char src[PATH_MAX];
  char dst[PATH_MAX];
+ char out[PATH_MAX];
  char *p = NULL;
 
  snprintf(src, sizeof(src), "%s/private/%s.key", ca->sslpath, keyname);
@@ -336,9 +333,9 @@ ca_key_install(struct ca *ca, char *keyname, char *dir)
  snprintf(dst, sizeof(dst), "%s/private/local.key", dir);
  fcopy(src, dst, 0600);
 
- snprintf(cmd, sizeof(cmd), "%s rsa -out %s/local.pub"
-    " -in %s/private/local.key -pubout", PATH_OPENSSL, dir, dir);
- system(cmd);
+ snprintf(out, sizeof(out), "%s/local.pub", dir);
+
+ run(7, PATH_OPENSSL, "rsa", "-out", out, "-in", dst, "-pubout");
 
  free(p);
 
@@ -406,37 +403,33 @@ ca_newpass(char *passfile, char *password)
 int
 ca_create(struct ca *ca)
 {
- char cmd[PATH_MAX * 2];
- char path[PATH_MAX];
+ char key[PATH_MAX];
+ char passfile[PATH_MAX+5];
+ char csr[PATH_MAX];
+ char crt[PATH_MAX];
 
  ca_clrenv();
 
- snprintf(path, sizeof(path), "%s/private/ca.key", ca->sslpath);
- snprintf(cmd, sizeof(cmd), "%s genrsa -aes256 -out"
-    " %s -passout file:%s 2048", PATH_OPENSSL,
-    path, ca->passfile);
- system(cmd);
- chmod(path, 0600);
+ snprintf(key, sizeof(key), "%s/private/ca.key", ca->sslpath);
+ snprintf(passfile, sizeof(passfile), "file:%s", ca->passfile);
+
+ run(8, PATH_OPENSSL, "genrsa", "-aes256", "-out", key,
+    "-passout", passfile, "2048");
+ chmod(key, 0600);
 
  ca_setenv("$ENV::CERT_CN", "VPN CA");
  ca_setenv("$ENV::REQ_EXT", "x509v3_CA");
  ca_setcnf(ca, "ca");
 
- snprintf(path, sizeof(path), "%s/private/ca.csr", ca->sslpath);
- snprintf(cmd, sizeof(cmd), "%s req %s-new"
-    " -key %s/private/ca.key"
-    " -config %s -out %s -passin file:%s", PATH_OPENSSL,
-    ca->batch, ca->sslpath, ca->sslcnf, path, ca->passfile);
- system(cmd);
- chmod(path, 0600);
+ snprintf(csr, sizeof(csr), "%s/private/ca.csr", ca->sslpath);
+ run(12, PATH_OPENSSL, "req", ca->batch, "-new", "-key", key,
+    "-config", ca->sslcnf, "-out", csr, "-passin", passfile);
+ chmod(csr, 0600);
 
- snprintf(cmd, sizeof(cmd), "%s x509 -req -days 365"
-    " -in %s/private/ca.csr -signkey %s/private/ca.key"
-    " -sha256"
-    " -extfile %s -extensions x509v3_CA -out %s/ca.crt -passin file:%s",
-    PATH_OPENSSL, ca->sslpath, ca->sslpath, ca->extcnf, ca->sslpath,
-    ca->passfile);
- system(cmd);
+ snprintf(crt, sizeof(crt), "%s/ca.crt", ca->sslpath);
+ run(18, PATH_OPENSSL, "x509", "-req", "-days", "365", "-in", csr,
+    "-signkey", key, "-sha256", "-extfile", ca->extcnf,
+    "-extensions", "x509v3_CA", "-out", crt, "-passin", passfile);
 
  /* Create the CRL revocation list */
  ca_revoke(ca, NULL);
@@ -486,7 +479,6 @@ ca_show_certs(struct ca *ca, char *name)
 {
  DIR *dir;
  struct dirent *de;
- char cmd[PATH_MAX * 2];
  char path[PATH_MAX];
  char *p;
  struct stat st;
@@ -496,9 +488,7 @@ ca_show_certs(struct ca *ca, char *name)
     ca->sslpath, name);
  if (stat(path, &st) != 0)
  err(1, "could not open file %s.crt", name);
- snprintf(cmd, sizeof(cmd), "%s x509 -text"
-    " -in %s", PATH_OPENSSL, path);
- system(cmd);
+ run(5, PATH_OPENSSL, "x509", "-text", "-in", path);
  printf("\n");
  return (0);
  }
@@ -513,10 +503,8 @@ ca_show_certs(struct ca *ca, char *name)
  continue;
  snprintf(path, sizeof(path), "%s/%s", ca->sslpath,
     de->d_name);
- snprintf(cmd, sizeof(cmd), "%s x509 -subject"
-    " -fingerprint -dates -noout -in %s",
-    PATH_OPENSSL, path);
- system(cmd);
+ run(8, PATH_OPENSSL, "x509", "-subject", "-fingerprint",
+    "-dates", "-noout", "-in", path);
  printf("\n");
  }
  }
@@ -650,10 +638,16 @@ ca_export(struct ca *ca, char *keyname, char *myname, char *password)
  struct stat st;
  char *pass;
  char prev[_PASSWORD_LEN + 1];
- char cmd[PATH_MAX * 2];
+ char passenv[_PASSWORD_LEN + 8];
  char oname[PATH_MAX];
  char src[PATH_MAX];
  char dst[PATH_MAX];
+ char cacrt[PATH_MAX];
+ char key[PATH_MAX];
+ char crt[PATH_MAX];
+ char pfx[PATH_MAX];
+ char capfx[PATH_MAX];
+ char passfile[PATH_MAX];
  char *p;
  char tpl[] = "/tmp/ikectl.XXXXXXXXXX";
  unsigned int i;
@@ -671,7 +665,7 @@ ca_export(struct ca *ca, char *keyname, char *myname, char *password)
  *p = '_';
 
  if (password != NULL)
- pass = password;
+ snprintf(passenv, sizeof(passenv), "EXPASS=%s", password);
  else {
  pass = getpass("Export passphrase:");
  if (pass == NULL || *pass == '\0')
@@ -681,24 +675,30 @@ ca_export(struct ca *ca, char *keyname, char *myname, char *password)
  pass = getpass("Retype export passphrase:");
  if (pass == NULL || strcmp(prev, pass) != 0)
  errx(1, "passphrase does not match!");
+
+ snprintf(passenv, sizeof(passenv), "EXPASS=%s", pass);
  }
 
+ snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
+ snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
+ snprintf(crt, sizeof(crt), "%s/%s.crt", ca->sslpath, keyname);
+ snprintf(pfx, sizeof(pfx), "%s/private/%s.pfx", ca->sslpath, oname);
+ snprintf(capfx, sizeof(capfx), "%s/ca.pfx", ca->sslpath);
+ snprintf(passfile, sizeof(passfile), "file:%s", ca->passfile);
+
+ putenv(passenv);
  if (keyname != NULL) {
- snprintf(cmd, sizeof(cmd), "env EXPASS=%s %s pkcs12 -export"
-    " -name %s -CAfile %s/ca.crt -inkey %s/private/%s.key"
-    " -in %s/%s.crt -out %s/private/%s.pfx -passout env:EXPASS"
-    " -passin file:%s", pass, PATH_OPENSSL, keyname,
-    ca->sslpath, ca->sslpath, keyname, ca->sslpath, keyname,
-    ca->sslpath, oname, ca->passfile);
- system(cmd);
- }
-
- snprintf(cmd, sizeof(cmd), "env EXPASS=%s %s pkcs12 -export"
-    " -caname '%s' -name '%s' -cacerts -nokeys"
-    " -in %s/ca.crt -out %s/ca.pfx -passout env:EXPASS -passin file:%s",
-    pass, PATH_OPENSSL, ca->caname, ca->caname, ca->sslpath,
-    ca->sslpath, ca->passfile);
- system(cmd);
+ run(17, PATH_OPENSSL, "pkcs12", "-export", "-name", keyname,
+    "-CAfile", cacrt, "-inkey", key, "-in", crt, "-out", pfx,
+    "-passout", "env:EXPASS", "-passin", passfile);
+ }
+
+ run(17, PATH_OPENSSL, "pkcs12", "-export", "-caname", ca->caname,
+    "-name", ca->caname, "-cacerts", "-nokeys", "-in", cacrt,
+    "-out", capfx, "-passout", "env:EXPASS", "-passin", passfile);
+
+ unsetenv("EXPASS");
+ explicit_bzero(passenv, sizeof(passenv));
 
  if ((p = mkdtemp(tpl)) == NULL)
  err(1, "could not create temp dir");
@@ -753,21 +753,18 @@ ca_export(struct ca *ca, char *keyname, char *myname, char *password)
  snprintf(dst, sizeof(dst), "%s/certs/%s.crt", p, keyname);
  fcopy(src, dst, 0644);
 
- snprintf(cmd, sizeof(cmd), "%s rsa -out %s/local.pub"
-    " -in %s/private/%s.key -pubout", PATH_OPENSSL, p,
-    ca->sslpath, keyname);
- system(cmd);
+ snprintf(src, sizeof(src), "%s/private/%s.key", ca->sslpath,
+    keyname);
+ snprintf(dst, sizeof(dst), "%s/local.pub", p);
+ run(7, PATH_OPENSSL, "rsa", "-out", dst, "-in", src, "-pubout");
  }
 
  if (stat(PATH_TAR, &st) == 0) {
+ snprintf(src, sizeof(src), "%s.tgz", oname);
  if (keyname == NULL)
- snprintf(cmd, sizeof(cmd), "%s -zcf %s.tgz -C %s .",
-    PATH_TAR, oname, ca->sslpath);
+ run(6, PATH_TAR, "-zcf", src, "-C", ca->sslpath, ".");
  else
- snprintf(cmd, sizeof(cmd), "%s -zcf %s.tgz -C %s .",
-    PATH_TAR, oname, p);
- system(cmd);
- snprintf(src, sizeof(src), "%s.tgz", oname);
+ run(6, PATH_TAR, "-zcf", src, "-C", p, ".");
  if (realpath(src, dst) != NULL)
  printf("exported files in %s\n", dst);
  }
@@ -796,8 +793,7 @@ ca_export(struct ca *ca, char *keyname, char *myname, char *password)
  err(1, "could not change %s", dst);
 
  snprintf(dst, sizeof(dst), "%s/%s.zip", src, oname);
- snprintf(cmd, sizeof(cmd), "%s -qr %s .", PATH_ZIP, dst);
- system(cmd);
+ run(4, PATH_ZIP, "-qr", dst, ".");
  printf("exported files in %s\n", dst);
 
  if (chdir(src) == -1)
@@ -877,8 +873,9 @@ int
 ca_revoke(struct ca *ca, char *keyname)
 {
  struct stat st;
- char cmd[PATH_MAX * 2];
  char path[PATH_MAX];
+ char cakey[PATH_MAX];
+ char cacrt[PATH_MAX];
  char *pass;
  size_t len;
 
@@ -902,27 +899,20 @@ ca_revoke(struct ca *ca, char *keyname)
  ca_setenv("$ENV::CASERIAL", ca->serial);
  ca_setcnf(ca, "ca-revoke");
 
+ snprintf(cakey, sizeof(cakey), "%s/private/ca.key", ca->sslpath);
+ snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
+
  if (keyname) {
- snprintf(cmd, sizeof(cmd),
-    "%s ca %s-config %s -keyfile %s/private/ca.key"
-    " -key %s"
-    " -cert %s/ca.crt"
-    " -revoke %s/%s.crt",
-    PATH_OPENSSL, ca->batch, ca->sslcnf,
-    ca->sslpath, pass, ca->sslpath, ca->sslpath, keyname);
- system(cmd);
- }
-
- snprintf(cmd, sizeof(cmd),
-    "%s ca %s-config %s -keyfile %s/private/ca.key"
-    " -key %s"
-    " -gencrl"
-    " -cert %s/ca.crt"
-    " -crldays 365"
-    " -out %s/ca.crl",
-    PATH_OPENSSL, ca->batch, ca->sslcnf, ca->sslpath,
-    pass, ca->sslpath, ca->sslpath);
- system(cmd);
+ snprintf(path, sizeof(path), "%s/%s.crt", ca->sslpath, keyname);
+ run(13, PATH_OPENSSL, "ca", ca->batch, "-config", ca->sslcnf,
+    "-keyfile", cakey, "-key", pass, "-cert", cacrt,
+    "-revoke", path);
+ }
+
+ snprintf(path, sizeof(path), "%s/ca.crl", ca->sslpath);
+ run(16, PATH_OPENSSL, "ca", ca->batch, "-config", ca->sslcnf,
+    "-keyfile", cakey, "-key", pass, "-gencrl", "-cert", cacrt,
+    "-crldays", "365", "-out", path);
 
  explicit_bzero(pass, len);
  free(pass);
@@ -996,7 +986,7 @@ ca_setup(char *caname, int create, int quiet, char *pass)
  strlcat(ca->passfile, "/ikeca.passwd", sizeof(ca->passfile));
 
  if (quiet)
- strlcpy(ca->batch, "-batch ", sizeof(ca->batch));
+ strlcpy(ca->batch, "-batch", sizeof(ca->batch));
 
  if (create == 0 && stat(ca->sslpath, &st) == -1) {
  free(ca->caname);
@@ -1016,3 +1006,37 @@ ca_setup(char *caname, int create, int quiet, char *pass)
 
  return (ca);
 }
+
+int static
+run(int n, ...)
+{
+ pid_t pid, cpid;
+ va_list ap;
+ int i = 0, status;
+ char **argv;
+
+ argv = reallocarray(NULL, n + 1, sizeof(char *));
+
+ va_start(ap, n);
+ while (n--) {
+ argv[i] = va_arg(ap, char *);
+ // Ignore empty arguments for ca->batch
+ if (argv[i][0] != '\0')
+ i++;
+ }
+ va_end(ap);
+ argv[i] = NULL;
+
+ switch (cpid = fork()) {
+ case -1:
+ return -1;
+ case 0:
+ return execv(argv[0], argv);
+ }
+
+ do {
+ pid = waitpid(cpid, &status, 0);
+ } while (pid == -1 && errno == EINTR);
+
+ return (pid == -1 ? -1 : WEXITSTATUS(status));
+}

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [patch] Avoid system(3) in ikectl

Stuart Henderson
On 2017/05/19 00:32, Matthew Martin wrote:

> ikectl errors in a number of situations where shell special characters
> are used. For example:
>
> % doas ikectl ca test create password \'
> [...]
> subject=/C=DE/ST=Lower Saxony/L=Hanover/O=OpenBSD/OU=iked/CN=VPN CA/emailAddress=[hidden email]
> Getting Private key
> sh: no closing quote
>
> This is because it uses system(3) in various places to run openssl, tar,
> and zip. Take the hint from the system(3) man page, and write a small
> function that does the fork and exec bypassing sh.

This seems like a good idea anyway, but this diff from Andrei-Marius Radu
to stop passing the password on the command line is still pending :

https://marc.info/?l=openbsd-bugs&m=149064755410645&w=2

Index: ikeca.c
===================================================================
RCS file: /cvs/src/usr.sbin/ikectl/ikeca.c,v
retrieving revision 1.42
diff -u -p -r1.42 ikeca.c
--- ikeca.c 29 Mar 2017 08:19:13 -0000 1.42
+++ ikeca.c 19 May 2017 08:55:36 -0000
@@ -108,7 +108,6 @@ const char *ca_env[][2] = {
 int ca_sign(struct ca *, char *, int);
 int ca_request(struct ca *, char *, int);
 void ca_newpass(char *, char *);
-char *ca_readpass(char *, size_t *);
 int fcopy(char *, char *, mode_t);
 void fcopy_env(const char *, const char *, mode_t);
 int rm_dir(char *);
@@ -809,33 +808,6 @@ ca_export(struct ca *ca, char *keyname,
  return (0);
 }
 
-char *
-ca_readpass(char *path, size_t *len)
-{
- FILE *f;
- char *p, *r;
-
- if ((f = fopen(path, "r")) == NULL) {
- warn("fopen %s", path);
- return (NULL);
- }
-
- if ((p = fgetln(f, len)) != NULL) {
- if ((r = malloc(*len + 1)) == NULL)
- err(1, "malloc");
- memcpy(r, p, *len);
- if (r[*len - 1] == '\n')
- r[*len - 1] = '\0';
- else
- r[*len] = '\0';
- } else
- r = NULL;
-
- fclose(f);
-
- return (r);
-}
-
 /* create index if it doesn't already exist */
 void
 ca_create_index(struct ca *ca)
@@ -879,8 +851,6 @@ ca_revoke(struct ca *ca, char *keyname)
  struct stat st;
  char cmd[PATH_MAX * 2];
  char path[PATH_MAX];
- char *pass;
- size_t len;
 
  if (keyname) {
  snprintf(path, sizeof(path), "%s/%s.crt",
@@ -891,11 +861,6 @@ ca_revoke(struct ca *ca, char *keyname)
  }
  }
 
- snprintf(path, sizeof(path), "%s/ikeca.passwd", ca->sslpath);
- pass = ca_readpass(path, &len);
- if (pass == NULL)
- errx(1, "could not open passphrase file");
-
  ca_create_index(ca);
 
  ca_setenv("$ENV::CADB", ca->index);
@@ -905,27 +870,24 @@ ca_revoke(struct ca *ca, char *keyname)
  if (keyname) {
  snprintf(cmd, sizeof(cmd),
     "%s ca %s-config %s -keyfile %s/private/ca.key"
-    " -key %s"
+    " -passin file:%s"
     " -cert %s/ca.crt"
     " -revoke %s/%s.crt",
     PATH_OPENSSL, ca->batch, ca->sslcnf,
-    ca->sslpath, pass, ca->sslpath, ca->sslpath, keyname);
+    ca->sslpath, ca->passfile, ca->sslpath, ca->sslpath, keyname);
  system(cmd);
  }
 
  snprintf(cmd, sizeof(cmd),
     "%s ca %s-config %s -keyfile %s/private/ca.key"
-    " -key %s"
+    " -passin file:%s"
     " -gencrl"
     " -cert %s/ca.crt"
     " -crldays 365"
     " -out %s/ca.crl",
     PATH_OPENSSL, ca->batch, ca->sslcnf, ca->sslpath,
-    pass, ca->sslpath, ca->sslpath);
+    ca->passfile, ca->sslpath, ca->sslpath);
  system(cmd);
-
- explicit_bzero(pass, len);
- free(pass);
 
  return (0);
 }

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [patch] Avoid system(3) in ikectl

Jonathan Gray-11
On Fri, May 19, 2017 at 09:56:14AM +0100, Stuart Henderson wrote:

> On 2017/05/19 00:32, Matthew Martin wrote:
> > ikectl errors in a number of situations where shell special characters
> > are used. For example:
> >
> > % doas ikectl ca test create password \'
> > [...]
> > subject=/C=DE/ST=Lower Saxony/L=Hanover/O=OpenBSD/OU=iked/CN=VPN CA/emailAddress=[hidden email]
> > Getting Private key
> > sh: no closing quote
> >
> > This is because it uses system(3) in various places to run openssl, tar,
> > and zip. Take the hint from the system(3) man page, and write a small
> > function that does the fork and exec bypassing sh.
>
> This seems like a good idea anyway, but this diff from Andrei-Marius Radu
> to stop passing the password on the command line is still pending :
>
> https://marc.info/?l=openbsd-bugs&m=149064755410645&w=2

Trying to test this gets me

ikectl -q ca test2 certificate 10.0.0.6 revoke
Using configuration from /etc/ssl/test2/ca-revoke-ssl.cnf
error on line 27 of config file '/etc/ssl/test2/ca-revoke-ssl.cnf'
30728805483616:error:0EFFF068:configuration file routines:CRYPTO_internal:variable has no value:/usr/src/lib/libcrypto/conf/conf_def.c:563:line 27
Using configuration from /etc/ssl/test2/ca-revoke-ssl.cnf
error on line 27 of config file '/etc/ssl/test2/ca-revoke-ssl.cnf'
19976781949600:error:0EFFF068:configuration file routines:CRYPTO_internal:variable has no value:/usr/src/lib/libcrypto/conf/conf_def.c:563:line 27

    21  [ req ]
    22  #default_bits           = 2048
    23  #default_md             = sha256
    24  #default_keyfile        = privkey.pem
    25  distinguished_name      = req_distinguished_name
    26  #attributes             = req_attributes
    27  req_extensions          = $ENV::REQ_EXT

I'm ok with the Andrei-Marius Radu diff going in.  Though below diff
should go in as well.  Then we can start looking at the other two diffs.

Index: ikeca.c
===================================================================
RCS file: /cvs/src/usr.sbin/ikectl/ikeca.c,v
retrieving revision 1.43
diff -u -p -r1.43 ikeca.c
--- ikeca.c 21 May 2017 02:37:52 -0000 1.43
+++ ikeca.c 22 May 2017 10:17:08 -0000
@@ -900,6 +900,7 @@ ca_revoke(struct ca *ca, char *keyname)
 
  ca_setenv("$ENV::CADB", ca->index);
  ca_setenv("$ENV::CASERIAL", ca->serial);
+ ca_setenv("$ENV::REQ_EXT", "req");
  ca_setcnf(ca, "ca-revoke");
 
  if (keyname) {

Loading...