From d31dd9e00be371a2e763a51e7c7526f6da99ba76 Mon Sep 17 00:00:00 2001
From: Nicolas Williams <nico@twosigma.com>
Date: Tue, 8 Oct 2019 16:53:56 -0500
Subject: [PATCH] hx509: make file store writes atomic

Now we'll use mkostemp() and rename() into place to make
hx509_certs_store() atomic for FILE/DER-FILE/PEM-FILE stores.

This is not ideal, as it can leave temp files in place if a process
crashes in between the mkostemp() and the rename into place.

On Linux we'll eventually make use of O_TMPFILE and linkat().  The idea
will be to first create an anonymous, zero-link file in the directory
that will contain the file at the end, write the file, then linkat() the
file into place as a .new file, then rename() the .new into place.  That
will limit the amount of junk that may be left behind to just one file.
(If the linkat() fails, then unlink() the .new and try again.  If the
rename() fails that just means the caller raced with another and the
operation is complete.)

We should really make a lib/roken interface that does this.
---
 lib/hx509/ks_file.c | 54 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/lib/hx509/ks_file.c b/lib/hx509/ks_file.c
index eb193a445..bf34b60d8 100644
--- a/lib/hx509/ks_file.c
+++ b/lib/hx509/ks_file.c
@@ -32,6 +32,9 @@
  */
 
 #include "hx_locl.h"
+#ifndef WIN32
+#include <libgen.h>
+#endif
 
 typedef enum { USE_PEM, USE_DER } outformat;
 
@@ -583,17 +586,56 @@ store_func(hx509_context context, void *ctx, hx509_cert c)
     return 0;
 }
 
+static int
+mk_temp(const char *fn, char **tfn)
+{
+    char *ds;
+    int ret = -1;
+
+#ifdef WIN32
+    char buf[PATH_MAX];
+    char *p;
+
+    *tfn = NULL;
+
+    if ((ds = _fullpath(buf, fn, sizeof(buf))) == NULL) {
+        errno = errno ? errno : ENAMETOOLONG;
+        return -1;
+    }
+
+    if (p = strrchr(ds, '\\') == NULL) {
+        ret = asprintf(tfn, ".%s-XXXXXX", ds); /* XXX can't happen */
+    } else {
+        *(p++) = '\0';
+        ret = asprintf(tfn, "%s/.%s-XXXXXX", ds, p);
+    }
+#else
+    *tfn = NULL;
+    if ((ds = strdup(fn)))
+        ret = asprintf(tfn, "%s/.%s-XXXXXX", dirname(ds), basename(ds));
+    free(ds);
+#endif
+
+    /*
+     * Using mkostemp() risks leaving garbage files lying around.  To do better
+     * without resorting to file locks (which have their own problems) we need
+     * O_TMPFILE and linkat(2), which only Linux has.
+     */
+    return  (ret == -1 || *tfn == NULL) ? -1 : mkostemp(*tfn, O_CLOEXEC);
+}
+
 static int
 file_store(hx509_context context,
 	   hx509_certs certs, void *data, int flags, hx509_lock lock)
 {
     struct ks_file *ksf = data;
     struct store_ctx sc;
+    char *tfn;
     int ret;
     int fd;
 
     sc.f = NULL;
-    fd = open(ksf->fn, O_CREAT | O_WRONLY, 0600);
+    fd = mk_temp(ksf->fn, &tfn);
     if (fd > -1)
         sc.f = fdopen(fd, "w");
     if (sc.f == NULL) {
@@ -607,7 +649,15 @@ file_store(hx509_context context,
     sc.format = ksf->format;
 
     ret = hx509_certs_iter_f(context, ksf->certs, store_func, &sc);
-    fclose(sc.f);
+    if (ret == 0)
+        ret = fclose(sc.f);
+    else
+        (void) fclose(sc.f);
+    if (ret)
+        (void) unlink(tfn);
+    else
+        (void) rename(tfn, ksf->fn);
+    free(tfn);
     return ret;
 }