On Thu, Jun 08, 2017 at 10:39:08PM +1000, Jonathan Gray wrote:
> This would be simpler if the 'run' style function just took a NULL
> terminated array.  Closer to how other things work and could then
> be passed directly to an exec call.

Like so?

Not sure if the indentation is correct or if it's ok to rely on
calloc'ing a struct setting pointers to NULL. I ran out of entropy for
picking names, so I've added some braces (but really initialization
syntax is nicer).

- Matthew Martin




diff --git ikeca.c ikeca.c
index 3dacac9e83e..c04d0b7229c 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;
        char            *caname;
 };
 
@@ -116,6 +117,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(char *const []);
 
 /* util.c */
 int             expand_string(char *, size_t, const char *, const char *);
@@ -130,7 +132,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);
@@ -140,10 +141,8 @@ ca_key_create(struct ca *ca, char *keyname)
                return (0);
        }
 
-       snprintf(cmd, sizeof(cmd),
-           "%s genrsa -out %s 2048",
-           PATH_OPENSSL, path);
-       system(cmd);
+       char *cmd[6] = { PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL };
+       run(cmd);
        chmod(path, 0600);
 
        return (0);
@@ -200,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);
@@ -226,13 +225,12 @@ 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);
+       char *cmd[11] = { PATH_OPENSSL, "req", "-new", "-key", key, "-out",
+           path, "-config", ca->sslcnf, ca->batch, NULL };
+       run(cmd);
        chmod(path, 0600);
 
        return (0);
@@ -241,8 +239,12 @@ ca_request(struct ca *ca, char *keyname, int type)
 int
 ca_sign(struct ca *ca, char *keyname, int type)
 {
-       char            cmd[PATH_MAX * 2];
-       const char      *extensions = NULL;
+       char            keyfile[PATH_MAX];
+       char            cert[PATH_MAX];
+       char            out[PATH_MAX];
+       char            in[PATH_MAX];
+       char            passfile[PATH_MAX+5];
+       char            *extensions = NULL;
 
        if (type == HOST_IPADDR) {
                extensions = "x509v3_IPAddr";
@@ -258,19 +260,17 @@ 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);
+       char *cmd[22] = { 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", NULL };
+       run(cmd);
 
        return (0);
 }
@@ -313,9 +313,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);
@@ -335,9 +335,11 @@ 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);
+
+       char *cmd[8] = { PATH_OPENSSL, "rsa", "-out", out, "-in", dst,
+           "-pubout", NULL };
+       run(cmd);
 
        free(p);
 
@@ -405,37 +407,44 @@ 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);
+
+       {
+               char *cmd[9] = { PATH_OPENSSL, "genrsa", "-aes256", "-out", key,
+                   "-passout", passfile, "2048", NULL };
+               run(cmd);
+       }
+       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(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(csr, sizeof(csr), "%s/private/ca.csr", ca->sslpath);
+       {
+               char *cmd[13] = { PATH_OPENSSL, "req", "-new", "-key", key,
+                   "-config", ca->sslcnf, "-out", csr, "-passin", passfile,
+                   ca->batch, NULL };
+               run(cmd);
+       }
+       chmod(csr, 0600);
+
+       snprintf(crt, sizeof(crt), "%s/ca.crt", ca->sslpath);
+       {
+               char *cmd[19] = { PATH_OPENSSL, "x509", "-req", "-days", "365",
+                   "-in", csr, "-signkey", key, "-sha256", "-extfile",
+                   ca->extcnf, "-extensions", "x509v3_CA", "-out", crt,
+                   "-passin", passfile, NULL };
+               run(cmd);
+       }
 
        /* Create the CRL revocation list */
        ca_revoke(ca, NULL);
@@ -485,7 +494,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;
@@ -495,9 +503,11 @@ 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);
+               {
+                       char *cmd[6] = { PATH_OPENSSL, "x509", "-text", "-in",
+                           path, NULL };
+                       run(cmd);
+               }
                printf("\n");
                return (0);
        }
@@ -512,10 +522,10 @@ 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);
+                       char *cmd[9] = { PATH_OPENSSL, "x509", "-subject",
+                           "-fingerprint", "-dates", "-noout", "-in", path,
+                           NULL };
+                       run(cmd);
                        printf("\n");
                }
        }
@@ -649,10 +659,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;
@@ -670,7 +686,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')
@@ -680,24 +696,36 @@ 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);
+               char *cmd[18] = { PATH_OPENSSL, "pkcs12", "-export", "-name",
+                   keyname, "-CAfile", cacrt, "-inkey", key, "-in", crt,
+                   "-out", pfx, "-passout", "env:EXPASS", "-passin", passfile,
+                   NULL };
+               run(cmd);
+       }
+
+       {
+               char *cmd[18] = { PATH_OPENSSL, "pkcs12", "-export", "-caname",
+                   ca->caname, "-name", ca->caname, "-cacerts", "-nokeys",
+                   "-in", cacrt, "-out", capfx, "-passout", "env:EXPASS",
+                   "-passin", passfile, NULL };
+               run(cmd);
+       }
+
+       unsetenv("EXPASS");
+       explicit_bzero(passenv, sizeof(passenv));
 
        if ((p = mkdtemp(tpl)) == NULL)
                err(1, "could not create temp dir");
@@ -752,21 +780,25 @@ 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);
+               char *cmd[8] = { PATH_OPENSSL, "rsa", "-out", dst, "-in", src,
+                   "-pubout", NULL };
+               run(cmd);
        }
 
        if (stat(PATH_TAR, &st) == 0) {
-               if (keyname == NULL)
-                       snprintf(cmd, sizeof(cmd), "%s -zcf %s.tgz -C %s .",
-                           PATH_TAR, oname, 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);
+               if (keyname == NULL) {
+                       char *cmd[7] = { PATH_TAR, "-zcf", src, "-C",
+                           ca->sslpath, ".", NULL };
+                       run(cmd);
+               } else {
+                       char *cmd[7] = { PATH_TAR, "-zcf", src, "-C", p, ".",
+                           NULL };
+                       run(cmd);
+               }
                if (realpath(src, dst) != NULL)
                        printf("exported files in %s\n", dst);
        }
@@ -795,8 +827,8 @@ 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);
+               char *cmd[5] = { PATH_ZIP, "-qr", dst, ".", NULL };
+               run(cmd);
                printf("exported files in %s\n", dst);
 
                if (chdir(src) == -1)
@@ -849,8 +881,10 @@ int
 ca_revoke(struct ca *ca, char *keyname)
 {
        struct stat      st;
-       char             cmd[PATH_MAX * 2];
        char             path[PATH_MAX];
+       char             passfile[PATH_MAX+5];
+       char             cakey[PATH_MAX];
+       char             cacrt[PATH_MAX];
 
        if (keyname) {
                snprintf(path, sizeof(path), "%s/%s.crt",
@@ -870,27 +904,22 @@ ca_revoke(struct ca *ca, char *keyname)
 
        ca_setcnf(ca, "ca-revoke");
 
+       snprintf(passfile, sizeof(passfile), "file:%s", ca->passfile);
+       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"
-                   " -passin file:%s"
-                   " -cert %s/ca.crt"
-                   " -revoke %s/%s.crt",
-                   PATH_OPENSSL, ca->batch, ca->sslcnf,
-                   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"
-           " -passin file:%s"
-           " -gencrl"
-           " -cert %s/ca.crt"
-           " -crldays 365"
-           " -out %s/ca.crl",
-           PATH_OPENSSL, ca->batch, ca->sslcnf, ca->sslpath,
-           ca->passfile, ca->sslpath, ca->sslpath);
-       system(cmd);
+               char *cmd[14] = { PATH_OPENSSL, "ca", "-config", ca->sslcnf,
+                   "-keyfile", cakey, "-passin", passfile, "-cert", cacrt,
+                   "-revoke", path, ca->batch, NULL };
+               run(cmd);
+       }
+
+       snprintf(path, sizeof(path), "%s/ca.crl", ca->sslpath);
+       char *cmd[17] = { PATH_OPENSSL, "ca", "-config", ca->sslcnf, "-keyfile",
+           cakey, "-passin", passfile, "-gencrl", "-cert", cacrt, "-crldays",
+           "365", "-out", path, ca->batch, NULL };
+       run(cmd);
 
        return (0);
 }
@@ -961,7 +990,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));
+               ca->batch = "-batch";
 
        if (create == 0 && stat(ca->sslpath, &st) == -1) {
                free(ca->caname);
@@ -981,3 +1010,23 @@ ca_setup(char *caname, int create, int quiet, char *pass)
 
        return (ca);
 }
+
+int static
+run(char *const argv[])
+{
+       pid_t pid, cpid;
+       int status;
+
+       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 via email to