replace hand-rolled tmp files with mkstemp()

ok florian@ back in april, reminded by theo.

diff --git a/usr.sbin/acme-client/fileproc.c b/usr.sbin/acme-client/fileproc.c
index cd4298e..00ce339 100644 (file)
--- a/usr.sbin/acme-client/fileproc.c
+++ b/usr.sbin/acme-client/fileproc.c
@@ -1,4 +1,4 @@
-/*     $Id: fileproc.c,v 1.14 2017/01/24 13:32:55 jsing Exp $ */
+/*     $Id: fileproc.c,v 1.15 2018/07/29 20:15:23 benno Exp $ */
 /*
  * Copyright (c) 2016 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -15,6 +15,8 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <sys/stat.h>
+
 #include <err.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -27,37 +29,51 @@
 #include "extern.h"
 
 static int
-serialise(const char *tmp, const char *real,
-    const char *v, size_t vsz, const char *v2, size_t v2sz)
+serialise(const char *real, const char *v, size_t vsz, const char *v2, size_t v2sz)
 {
-       int      fd;
+       int       fd;
+       char     *tmp;
 
        /*
         * Write into backup location, overwriting.
-        * Then atomically (?) do the rename.
+        * Then atomically do the rename.
         */
 
-       fd = open(tmp, O_WRONLY|O_CREAT|O_TRUNC, 0444);
-       if (fd == -1) {
-               warn("%s", tmp);
-               return 0;
-       } else if ((ssize_t)vsz != write(fd, v, vsz)) {
-               warnx("%s", tmp);
-               close(fd);
-               return 0;
-       } else if (v2 != NULL && write(fd, v2, v2sz) != (ssize_t)v2sz) {
-               warnx("%s", tmp);
-               close(fd);
-               return 0;
-       } else if (close(fd) == -1) {
-               warn("%s", tmp);
+       if (asprintf(&tmp, "%s.XXXXXXXXXX", real) == -1) {
+               warn("asprintf");
                return 0;
-       } else if (rename(tmp, real) == -1) {
+       }
+       if ((fd = mkstemp(tmp)) == -1) {
+               warn("mkstemp");
+               goto out;
+       }
+       if (fchmod(fd, 0444) == -1) {
+               warn("fchmod");
+               goto out;
+       }
+       if ((ssize_t)vsz != write(fd, v, vsz)) {
+               warnx("write");
+               goto out;
+       }
+       if (v2 != NULL && write(fd, v2, v2sz) != (ssize_t)v2sz) {
+               warnx("write");
+               goto out;
+       }
+       if (close(fd) == -1)
+               goto out;
+       if (rename(tmp, real) == -1) {
                warn("%s", real);
-               return 0;
+               goto out;
        }
 
+       free(tmp);
        return 1;
+out:
+       if (fd != -1)
+               close(fd);
+       (void) unlink(tmp);
+       free(tmp);
+       return 0;
 }
 
 int
@@ -65,15 +81,11 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
     *chainfile, const char *fullchainfile)
 {
        char            *csr = NULL, *ch = NULL;
-       char            *certfile_bak = NULL, *chainfile_bak = NULL;
-       char            *fullchainfile_bak = NULL;
        size_t           chsz, csz;
        int              rc = 0;
        long             lval;
        enum fileop      op;
 
-       /* File-system and sandbox jailing. */
-
        if (chroot(certdir) == -1) {
                warn("chroot");
                goto out;
@@ -85,9 +97,9 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
 
        /*
         * rpath and cpath for rename, wpath and cpath for
-        * writing to the temporary.
+        * writing to the temporary. fattr for fchmod.
         */
-       if (pledge("stdio cpath wpath rpath", NULL) == -1) {
+       if (pledge("stdio cpath wpath rpath fattr", NULL) == -1) {
                warn("pledge");
                goto out;
        }
@@ -148,32 +160,12 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
         * Start by downloading the chain PEM as a buffer.
         * This is not NUL-terminated, but we're just going to guess
         * that it's well-formed and not actually touch the data.
-        * Once downloaded, dump it into CHAIN_BAK.
         */
-
-       if (certfile)
-               if (asprintf(&certfile_bak, "%s~", certfile) == -1) {
-                       warn("asprintf");
-                       goto out;
-               }
-
-       if (chainfile)
-               if (asprintf(&chainfile_bak, "%s~", chainfile) == -1) {
-                       warn("asprintf");
-                       goto out;
-               }
-
-       if (fullchainfile)
-               if (asprintf(&fullchainfile_bak, "%s~", fullchainfile) == -1) {
-                       warn("asprintf");
-                       goto out;
-               }
-
        if ((ch = readbuf(certsock, COMM_CHAIN, &chsz)) == NULL)
                goto out;
 
        if (chainfile) {
-               if (!serialise(chainfile_bak, chainfile, ch, chsz, NULL, 0))
+               if (!serialise(chainfile, ch, chsz, NULL, 0))
                        goto out;
 
                dodbg("%s/%s: created", certdir, chainfile);
@@ -190,7 +182,7 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
                goto out;
 
        if (certfile) {
-               if (!serialise(certfile_bak, certfile, csr, csz, NULL, 0))
+               if (!serialise(certfile, csr, csz, NULL, 0))
                        goto out;
 
                dodbg("%s/%s: created", certdir, certfile);
@@ -203,7 +195,7 @@ fileproc(int certsock, const char *certdir, const char *certfile, const char
         * on-file certificates were changed.
         */
        if (fullchainfile) {
-               if (!serialise(fullchainfile_bak, fullchainfile, csr, csz, ch,
+               if (!serialise(fullchainfile, csr, csz, ch,
                    chsz))
                        goto out;
 
@@ -215,8 +207,5 @@ out:
        close(certsock);
        free(csr);
        free(ch);
-       free(certfile_bak);
-       free(chainfile_bak);
-       free(fullchainfile_bak);
        return rc;
 }