Matthew Martin wrote:
> ping
>
> On Thu, Apr 25, 2019 at 11:21:00PM -0500, Matthew Martin wrote:
> > On Thu, Apr 25, 2019 at 08:59:56PM -0600, Theo de Raadt wrote:
> > > > + argv = alloca((n + 1) * sizeof(*argv));
> > >
> > > Our source tree is exceedingly sparing in the use of alloca().
> > > This will not do.
> >
> > Was staying as close as possible to exec.c, but avoiding alloca is
> > preferable; replaced with reallocarray. err message is "reallocarray" to
> > match style with the calloc call in the same file.
> >
> >
> - 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);
> + snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
> + snprintf(capfx, sizeof(capfx), "%s/ca.pfx", 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(passenv, sizeof(passenv), "EXPASS=%s", pass);
> + putenv(passenv);
> +
> + if (keyname != NULL)
> + ca_system(PATH_OPENSSL, "pkcs12", "-export", "-name", keyname,
> + "-CAfile", cacrt, "-inkey", key, "-in", crt, "-out", pfx,
> + "-passout", "env:EXPASS", "-passin", ca->passfile, NULL);
> +
> + ca_system(PATH_OPENSSL, "pkcs12", "-export", "-caname", ca->caname,
> + "-name", ca->caname, "-cacerts", "-nokeys", "-in", cacrt,
> + "-out", capfx, "-passout", "env:EXPASS", "-passin", ca->passfile,
> + NULL);
> +
> + unsetenv("EXPASS");
> + explicit_bzero(passenv, sizeof(passenv));
This looks weird, but it's still probably an improvement over the original
code, which is trying to avoid leaking password via argv but only mostly
succeeds.
There's probably a better way to do environment handling, but I think this
diff is better than using system() now. ok with me.
