crypto: always reload tls-auth/crypt key contexts

In preparation to having tls-auth/crypt keys per connection block, it is important to ensure that such material is always reloaded upon SIGUSR1, no matter if persist-key was specified or not.

This is required because when moving from one remote to the other the key may change and thus the key context needs to be refreshed.

To ensure that the persist-key logic will still work as expected, the tls-auth/crypt key is pre-loaded so that the keyfile is not required at runtime.

Trac: #720

Cc Steffan Karger <[email protected]...>

Signed-off-by Antonio Quartulli <[email protected]...>

---
v2:
- introduce this patch
v3:
- add key per-loading logic to this patch to avoid temporary features breakages
v4:
- rebase on top of master
- use buffer_read_from_file() implementation from "tls-crypt-v2: generate client keys". Minor change: in case of error, free buffer and remove it from gc (with new free_buf_gc()), if provided.
- don't go FATAL inside buffer_read_from_file() in case of error, but let external logic decide

 src/openvpn/buffer.c  | 61 ++++++++++++++++++++++++++++++
 src/openvpn/buffer.h  | 12 ++++++
 src/openvpn/crypto.c  | 21 +++--------
 src/openvpn/init.c    | 87 ++++++++++++++++++++++++++++---------------
 src/openvpn/options.c | 26 +++++++++++++
 5 files changed, 161 insertions(+), 46 deletions(-)

src/openvpn/buffer.c +1 -0

index becfeb93..7d4860db 100644
@@ -189,6 +189,34 @@ free_buf(struct buffer *buf)
     CLEAR(*buf);
 }
 
+static void

src/openvpn/buffer.c +50 -0

+            {
+                struct gc_entry *to_delete = *e;
+
+                /* remove element from linked list and free it */
+                *e = (*e)->next;
+                free(to_delete);
+
+                break;
+            }
+
+            e = &(*e)->next;
+        }
+    }
+
+    free_buf(buf);
+}
+
 /*
  * Return a buffer for write that is a subset of another buffer
  */

@@ -1332,3 +1360,36 @@ buffer_list_file(const char *fn, int max_line_len)
     }
     return bl;
 }
+
+struct buffer
+buffer_read_from_file(const char *filename, struct gc_arena *gc)
+{
+    struct buffer ret = { 0 };
+
+    platform_stat_t file_stat = {0};
+    if (platform_stat(filename, &file_stat) < 0)
+    {
+        return ret;
+    }
+
+    FILE *fp = platform_fopen(filename, "r");
+    if (!fp)
+    {
+        return ret;
+    }
+
+    const size_t size = file_stat.st_size;
+    ret = alloc_buf_gc(size + 1, gc); /* space for trailing \0 */
+    ssize_t read_size = fread(BPTR(&ret), 1, size, fp);
+    if (read_size < 0)
+    {
+        free_buf_gc(&ret, gc);
+        goto cleanup;
+    }
+    ASSERT(buf_inc_len(&ret, read_size));
+    buf_null_terminate(&ret);
+
+cleanup:
+    fclose(fp);
+    return ret;
+}

src/openvpn/buffer.h +0 -0

index d848490a..a11c8bd9 100644

src/openvpn/buffer.h +6 -0

+ *
+ * @return the buffer storing the file content or an invalid buffer in case of
+ * error
+ */
+struct buffer buffer_read_from_file(const char *filename, struct gc_arena *gc);
+
 #endif /* BUFFER_H */

src/openvpn/crypto.c +5 -16

index b59c1f73..5381ef2a 100644
@@ -1224,7 +1224,7 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
 {
     struct gc_arena gc = gc_new();
     struct buffer in;
-    int fd, size;
+    int size;
     uint8_t hex_byte[3] = {0, 0, 0};
     const char *error_filename = file;
 

@@ -1268,22 +1268,11 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
     }
     else /* 'file' is a filename which refers to a file containing the ascii key */
     {
-        in = alloc_buf_gc(2048, &gc);
-        fd = platform_open(file, O_RDONLY, 0);
-        if (fd == -1)
-        {
-            msg(M_ERR, "Cannot open key file '%s'", file);
-        }
-        size = read(fd, in.data, in.capacity);
-        if (size < 0)
-        {
+        in = buffer_read_from_file(file, &gc);
+        if (!buf_valid(&in))
             msg(M_FATAL, "Read error on key file ('%s')", file);
-        }
-        if (size == in.capacity)
-        {
-            msg(M_FATAL, "Key file ('%s') can be a maximum of %d bytes", file, (int)in.capacity);
-        }
-        close(fd);
+
+        size = in.len;
     }
 
     cp = (unsigned char *)in.data;

src/openvpn/init.c +57 -30

index b748357d..1b46b364 100644
@@ -2409,7 +2409,6 @@ key_schedule_free(struct key_schedule *ks, bool free_ssl_ctx)
     if (tls_ctx_initialised(&ks->ssl_ctx) && free_ssl_ctx)
     {
         tls_ctx_free(&ks->ssl_ctx);
-        free_key_ctx_bi(&ks->tls_wrap_key);
     }
     CLEAR(*ks);
 }

@@ -2499,6 +2498,48 @@ do_init_crypto_static(struct context *c, const unsigned int flags)
     check_replay_consistency(&c->c1.ks.key_type, options->replay);
 }
 
+/*
+ * Initialize the tls-auth/crypt key context
+ */
+static void
+do_init_tls_wrap_key(struct context *c)
+{
+    const struct options *options = &c->options;
+
+    /* TLS handshake authentication (--tls-auth) */
+    if (options->tls_auth_file)
+    {
+        /* Initialize key_type for tls-auth with auth only */
+        CLEAR(c->c1.ks.tls_auth_key_type);
+        if (!streq(options->authname, "none"))
+        {
+            c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname);
+                c->c1.ks.tls_auth_key_type.hmac_length =
+                    md_kt_size(c->c1.ks.tls_auth_key_type.digest);
+        }
+        else
+        {
+            msg(M_FATAL, "ERROR: tls-auth enabled, but no valid --auth "
+                "algorithm specified ('%s')", options->authname);
+        }
+
+        crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type,
+                                &c->c1.ks.tls_wrap_key,
+                                options->tls_auth_file,
+                                options->tls_auth_file_inline,
+                                options->key_direction,
+                                "Control Channel Authentication", "tls-auth");
+    }
+
+    /* TLS handshake encryption+authentication (--tls-crypt) */
+    if (options->tls_crypt_file)
+    {
+        tls_crypt_init_key(&c->c1.ks.tls_wrap_key,
+                           options->tls_crypt_file,
+                           options->tls_crypt_inline, options->tls_server);
+    }
+}
+
 /*
  * Initialize the persistent component of OpenVPN's TLS mode,
  * which is preserved across SIGUSR1 resets.

@@ -2548,35 +2589,8 @@ do_init_crypto_tls_c1(struct context *c)
         /* Initialize PRNG with config-specified digest */
         prng_init(options->prng_hash, options->prng_nonce_secret_len);
 
-        /* TLS handshake authentication (--tls-auth) */
-        if (options->tls_auth_file)
-        {
-            /* Initialize key_type for tls-auth with auth only */
-            CLEAR(c->c1.ks.tls_auth_key_type);
-            if (!streq(options->authname, "none"))
-            {
-                c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname);
-                c->c1.ks.tls_auth_key_type.hmac_length =
-                    md_kt_size(c->c1.ks.tls_auth_key_type.digest);
-            }
-            else
-            {
-                msg(M_FATAL, "ERROR: tls-auth enabled, but no valid --auth "
-                    "algorithm specified ('%s')", options->authname);
-            }
-
-            crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type,
-                                    &c->c1.ks.tls_wrap_key, options->tls_auth_file,
-                                    options->tls_auth_file_inline, options->key_direction,
-                                    "Control Channel Authentication", "tls-auth");
-        }
-
-        /* TLS handshake encryption+authentication (--tls-crypt) */
-        if (options->tls_crypt_file)
-        {
-            tls_crypt_init_key(&c->c1.ks.tls_wrap_key, options->tls_crypt_file,
-                               options->tls_crypt_inline, options->tls_server);
-        }
+        /* initialize tls-auth/crypt key */
+        do_init_tls_wrap_key(c);
 
         c->c1.ciphername = options->ciphername;
         c->c1.authname = options->authname;

@@ -2598,6 +2612,12 @@ do_init_crypto_tls_c1(struct context *c)
         c->options.ciphername = c->c1.ciphername;
         c->options.authname = c->c1.authname;
         c->options.keysize = c->c1.keysize;
+
+        /*
+         * tls-auth/crypt key can be configured per connection block, therefore
+         * we must reload it as it may have changed
+         */
+        do_init_tls_wrap_key(c);
     }
 }
 

@@ -3401,6 +3421,13 @@ do_close_tls(struct context *c)
 static void
 do_close_free_key_schedule(struct context *c, bool free_ssl_ctx)
 {
+    /*
+     * always free the tls_auth/crypt key. If persist_key is true, the key will
+     * be reloaded from memory (pre-cached)
+     */
+    free_key_ctx_bi(&c->c1.ks.tls_wrap_key);
+    CLEAR(c->c1.ks.tls_wrap_key);
+
     if (!(c->sig->signal_received == SIGUSR1 && c->options.persist_key))
     {
         key_schedule_free(&c->c1.ks, free_ssl_ctx);

src/openvpn/options.c +19 -0

index 426057ab..b2ca2738 100644
@@ -3019,6 +3019,32 @@ options_postprocess_mutate(struct options *o)
         options_postprocess_mutate_ce(o, o->connection_list->array[i]);
     }
 
+    /* pre-cache tls-auth/crypt key file if persist-key was specified */
+    if (o->persist_key)
+    {
+        if (o->tls_auth_file && !o->tls_auth_file_inline)
+        {
+            struct buffer in = buffer_read_from_file(o->tls_auth_file, &o->gc);
+            if (!buf_valid(&in))
+                msg(M_FATAL, "Cannot pre-load tls-auth keyfile (%s)",
+                    o->tls_auth_file);
+
+            o->tls_auth_file = INLINE_FILE_TAG;
+            o->tls_auth_file_inline = (char *)in.data;
+        }
+
+        if (o->tls_crypt_file && !o->tls_crypt_inline)
+        {
+            struct buffer in = buffer_read_from_file(o->tls_crypt_file, &o->gc);
+            if (!buf_valid(&in))
+                msg(M_FATAL, "Cannot pre-load tls-crypt keyfile (%s)",

Different VPN servers may use different tls-auth/crypt keys. For this reason it is convenient to make tls-auth/crypt per-connection-block options so that the user is allowed to specify one key per remote.

If no tls-auth/crypt option is specified in a given connection block, the global settings, if any, are used.

Trac: #720

Cc Steffan Karger <[email protected]...>

Signed-off-by Antonio Quartulli <[email protected]...>

---
v2:
- convert tls-auth keyfile to inline key if persist-key was specified v3:
- squash 2/3 and 3/3 in one patch to prevent temporary features breakages
- restyle code introduced in options_postprocess_mutate_ce() v4:
- rebase on top of latest 1/2

 doc/openvpn.8         |   3 +
 src/openvpn/init.c    |  18 ++---
 src/openvpn/options.c | 173 +++++++++++++++++++++++++++++-------------
 src/openvpn/options.h |   9 +++
 4 files changed, 142 insertions(+), 61 deletions(-)

doc/openvpn.8 +3 -0

index 46ea58b6..f01b48b6 100644
@@ -361,6 +361,7 @@ block:
 .B fragment,
 .B http\-proxy,
 .B http\-proxy\-option,
+.B key\-direction,
 .B link\-mtu,
 .B local,
 .B lport,

@@ -372,6 +373,8 @@ block:
 .B remote,
 .B rport,
 .B socks\-proxy,
+.B tls\-auth,
+.B tls\-crypt,
 .B tun\-mtu and
 .B tun\-mtu\-extra.
 

src/openvpn/init.c +9 -9

index 1b46b364..14732e34 100644
@@ -2507,7 +2507,7 @@ do_init_tls_wrap_key(struct context *c)
     const struct options *options = &c->options;
 
     /* TLS handshake authentication (--tls-auth) */
-    if (options->tls_auth_file)
+    if (options->ce.tls_auth_file)
     {
         /* Initialize key_type for tls-auth with auth only */
         CLEAR(c->c1.ks.tls_auth_key_type);

@@ -2525,18 +2525,18 @@ do_init_tls_wrap_key(struct context *c)
 
         crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type,
                                 &c->c1.ks.tls_wrap_key,
-                                options->tls_auth_file,
-                                options->tls_auth_file_inline,
-                                options->key_direction,
+                                options->ce.tls_auth_file,
+                                options->ce.tls_auth_file_inline,
+                                options->ce.key_direction,
                                 "Control Channel Authentication", "tls-auth");
     }
 
     /* TLS handshake encryption+authentication (--tls-crypt) */
-    if (options->tls_crypt_file)
+    if (options->ce.tls_crypt_file)
     {
         tls_crypt_init_key(&c->c1.ks.tls_wrap_key,
-                           options->tls_crypt_file,
-                           options->tls_crypt_inline, options->tls_server);
+                           options->ce.tls_crypt_file,
+                           options->ce.tls_crypt_inline, options->tls_server);
     }
 }
 

@@ -2806,7 +2806,7 @@ do_init_crypto_tls(struct context *c, const unsigned int flags)
 #endif
 
     /* TLS handshake authentication (--tls-auth) */
-    if (options->tls_auth_file)
+    if (options->ce.tls_auth_file)
     {
         to.tls_wrap.mode = TLS_WRAP_AUTH;
         to.tls_wrap.opt.key_ctx_bi = c->c1.ks.tls_wrap_key;

@@ -2817,7 +2817,7 @@ do_init_crypto_tls(struct context *c, const unsigned int flags)
     }
 
     /* TLS handshake encryption (--tls-crypt) */
-    if (options->tls_crypt_file)
+    if (options->ce.tls_crypt_file)
     {
         to.tls_wrap.mode = TLS_WRAP_CRYPT;
         to.tls_wrap.opt.key_ctx_bi = c->c1.ks.tls_wrap_key;

src/openvpn/options.c +121 -52

index b2ca2738..6f1a5382 100644
@@ -1506,6 +1506,11 @@ show_connection_entry(const struct connection_entry *o)
 #ifdef ENABLE_OCC
     SHOW_INT(explicit_exit_notification);
 #endif
+
+    SHOW_STR(tls_auth_file);
+    SHOW_PARM(key_direction, keydirection2ascii(o->key_direction, false, true),
+              "%s");
+    SHOW_STR(tls_crypt_file);
 }
 
 

@@ -1786,9 +1791,6 @@ show_settings(const struct options *o)
     SHOW_BOOL(push_peer_info);
     SHOW_BOOL(tls_exit);
 
-    SHOW_STR(tls_auth_file);
-    SHOW_STR(tls_crypt_file);
-
 #ifdef ENABLE_PKCS11
     {
         int i;

@@ -2723,7 +2725,7 @@ options_postprocess_verify_ce(const struct options *options, const struct connec
                 notnull(options->priv_key_file, "private key file (--key) or PKCS#12 file (--pkcs12)");
             }
         }
-        if (options->tls_auth_file && options->tls_crypt_file)
+        if (ce->tls_auth_file && ce->tls_crypt_file)
         {
             msg(M_USAGE, "--tls-auth and --tls-crypt are mutually exclusive");
         }

@@ -2869,6 +2871,49 @@ options_postprocess_mutate_ce(struct options *o, struct connection_entry *ce)
         }
     }
 
+    /*
+     * Set per-connection block tls-auth/crypt fields if undefined.
+     *
+     * At the end only one of the two will be really set because the parser
+     * logic prevents configurations where both are set.
+     */
+    if (!ce->tls_auth_file && !ce->tls_crypt_file)
+    {
+        ce->tls_auth_file = o->tls_auth_file;
+        ce->tls_auth_file_inline = o->tls_auth_file_inline;
+        ce->key_direction = o->key_direction;
+
+        ce->tls_crypt_file = o->tls_crypt_file;
+        ce->tls_crypt_inline = o->tls_crypt_inline;
+    }
+
+    /* pre-cache tls-auth/crypt key file if persist-key was specified and keys
+     * were not already embedded in the config file
+     */
+    if (o->persist_key)
+    {
+        if (ce->tls_auth_file && !ce->tls_auth_file_inline)
+        {
+            struct buffer in = buffer_read_from_file(o->tls_auth_file, &o->gc);
+            if (!buf_valid(&in))
+                msg(M_FATAL, "Cannot pre-load tls-auth keyfile (%s)",
+                    o->tls_auth_file);
+
+            ce->tls_auth_file = INLINE_FILE_TAG;
+            ce->tls_auth_file_inline = (char *)in.data;
+        }
+
+        if (ce->tls_crypt_file && !ce->tls_crypt_inline)
+        {
+            struct buffer in = buffer_read_from_file(o->tls_crypt_file, &o->gc);
+            if (!buf_valid(&in))
+                msg(M_FATAL, "Cannot pre-load tls-crypt keyfile (%s)",
+                    o->tls_auth_file);
+
+            ce->tls_crypt_file = INLINE_FILE_TAG;
+            ce->tls_crypt_inline = (char *)in.data;
+        }
+    }
 }
 
 #ifdef _WIN32

@@ -3019,32 +3064,6 @@ options_postprocess_mutate(struct options *o)
         options_postprocess_mutate_ce(o, o->connection_list->array[i]);
     }
 
-    /* pre-cache tls-auth/crypt key file if persist-key was specified */
-    if (o->persist_key)
-    {
-        if (o->tls_auth_file && !o->tls_auth_file_inline)
-        {
-            struct buffer in = buffer_read_from_file(o->tls_auth_file, &o->gc);
-            if (!buf_valid(&in))
-                msg(M_FATAL, "Cannot pre-load tls-auth keyfile (%s)",
-                    o->tls_auth_file);
-
-            o->tls_auth_file = INLINE_FILE_TAG;
-            o->tls_auth_file_inline = (char *)in.data;
-        }
-
-        if (o->tls_crypt_file && !o->tls_crypt_inline)
-        {
-            struct buffer in = buffer_read_from_file(o->tls_crypt_file, &o->gc);
-            if (!buf_valid(&in))
-                msg(M_FATAL, "Cannot pre-load tls-crypt keyfile (%s)",
-                    o->tls_auth_file);
-
-            o->tls_crypt_file = INLINE_FILE_TAG;
-            o->tls_crypt_inline = (char *)in.data;
-        }
-    }
-
     if (o->tls_server)
     {
         /* Check that DH file is specified, or explicitly disabled */

@@ -3311,12 +3330,22 @@ options_postprocess_filechecks(struct options *options)
                                          options->crl_file, R_OK, "--crl-verify");
     }
 
-    errs |= check_file_access(CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE,
-                              options->tls_auth_file, R_OK, "--tls-auth");
-    errs |= check_file_access(CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE,
-                              options->tls_crypt_file, R_OK, "--tls-crypt");
+    ASSERT(options->connection_list);
+    for (int i = 0; i < options->connection_list->len; ++i)
+    {
+        struct connection_entry *ce = options->connection_list->array[i];
+
+        errs |= check_file_access(CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE,
+                                  ce->tls_auth_file, R_OK, "--tls-auth");
+
+        errs |= check_file_access(CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE,
+                                  ce->tls_crypt_file, R_OK, "--tls-crypt");
+
+    }
+
     errs |= check_file_access(CHKACC_FILE|CHKACC_INLINE|CHKACC_PRIVATE,
                               options->shared_secret_file, R_OK, "--secret");
+
     errs |= check_file_access(CHKACC_DIRPATH|CHKACC_FILEXSTWR,
                               options->packet_id_file, R_OK|W_OK, "--replay-persist");
 

@@ -3673,7 +3702,7 @@ options_string(const struct options *o,
     {
         if (TLS_CLIENT || TLS_SERVER)
         {
-            if (o->tls_auth_file)
+            if (o->ce.tls_auth_file)
             {
                 buf_printf(&out, ",tls-auth");
             }

@@ -7446,10 +7475,19 @@ add_option(struct options *options,
     {
         int key_direction;
 
+        VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION);
+
         key_direction = ascii2keydirection(msglevel, p[1]);
         if (key_direction >= 0)
         {
-            options->key_direction = key_direction;
+            if (permission_mask & OPT_P_GENERAL)
+            {
+                options->key_direction = key_direction;
+            }
+            else if (permission_mask & OPT_P_CONNECTION)
+            {
+                options->ce.key_direction = key_direction;
+            }
         }
         else
         {

@@ -8018,35 +8056,66 @@ add_option(struct options *options,
     }
     else if (streq(p[0], "tls-auth") && p[1] && !p[3])
     {
-        VERIFY_PERMISSION(OPT_P_GENERAL);
-        if (streq(p[1], INLINE_FILE_TAG) && p[2])
+        int key_direction = -1;
+
+        VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION);
+
+        if (permission_mask & OPT_P_GENERAL)
         {
-            options->tls_auth_file_inline = p[2];
+            if (streq(p[1], INLINE_FILE_TAG) && p[2])
+            {
+                options->tls_auth_file_inline = p[2];
+            }
+            else if (p[2])
+            {
+                key_direction = ascii2keydirection(msglevel, p[2]);
+                if (key_direction < 0)
+                {
+                    goto err;
+                }
+                options->key_direction = key_direction;
+            }
+            options->tls_auth_file = p[1];
         }
-        else if (p[2])
+        else if (permission_mask & OPT_P_CONNECTION)
         {
-            int key_direction;
-
-            key_direction = ascii2keydirection(msglevel, p[2]);
-            if (key_direction >= 0)
+            options->ce.key_direction = KEY_DIRECTION_BIDIRECTIONAL;
+            if (streq(p[1], INLINE_FILE_TAG) && p[2])
             {
-                options->key_direction = key_direction;
+                options->ce.tls_auth_file_inline = p[2];
             }
-            else
+            else if (p[2])
             {
-                goto err;
+                key_direction = ascii2keydirection(msglevel, p[2]);
+                if (key_direction < 0)
+                {
+                    goto err;
+                }
+                options->ce.key_direction = key_direction;
             }
+            options->ce.tls_auth_file = p[1];
         }
-        options->tls_auth_file = p[1];
     }
     else if (streq(p[0], "tls-crypt") && p[1] && !p[3])
     {
-        VERIFY_PERMISSION(OPT_P_GENERAL);
-        if (streq(p[1], INLINE_FILE_TAG) && p[2])
+        VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION);
+        if (permission_mask & OPT_P_GENERAL)
         {
-            options->tls_crypt_inline = p[2];
+            if (streq(p[1], INLINE_FILE_TAG) && p[2])
+            {
+                options->tls_crypt_inline = p[2];
+            }
+            options->tls_crypt_file = p[1];
+        }
+        else if (permission_mask & OPT_P_CONNECTION)
+        {
+            if (streq(p[1], INLINE_FILE_TAG) && p[2])
+            {
+                options->ce.tls_crypt_inline = p[2];
+            }
+            options->ce.tls_crypt_file = p[1];
+
         }
-        options->tls_crypt_file = p[1];
     }
     else if (streq(p[0], "key-method") && p[1] && !p[2])
     {

src/openvpn/options.h +2 -0

index 3a6c33f8..acbd1087 100644
@@ -130,6 +130,15 @@ struct connection_entry
 #define CE_MAN_QUERY_REMOTE_MASK   (0x07)
 #define CE_MAN_QUERY_REMOTE_SHIFT  (2)
     unsigned int flags;
+
+    /* Shared secret used for TLS control channel authentication */

Hi,

Quoted 5 lines

src/openvpn/buffer.c +5 -0

+static void
+free_buf_gc(struct buffer *buf, struct gc_arena *gc)
+{
+    if (gc)
+    {

src/openvpn/buffer.c free_buf(struct buffer *buf) · line 202

Quoted 6 lines

src/openvpn/buffer.c +6 -0

+        struct gc_entry **e = &gc->list;
+
+        while (*e)
+        {
+            /* check if this object is the one we want to delete */
+            if ((uint8_t *)((*e) + sizeof(*e)) == buf->data)

I think this should be sizeof(**e).

Quoted 6 lines

+/**
+ * buffer_read_from_file - copy the content of a file into a buffer
+ *

+ * @param file      path to the file to read
+ * @param gc        the garbage collector to use when allocating the buffer. It
+ *                  passed to alloc_buf_gc() and therefore can be NULL.

It *is* passed ?

Otherwise, this looks good.

In preparation to having tls-auth/crypt keys per connection block, it is important to ensure that such material is always reloaded upon SIGUSR1, no matter if persist-key was specified or not.

This is required because when moving from one remote to the other the key may change and thus the key context needs to be refreshed.

To ensure that the persist-key logic will still work as expected, the tls-auth/crypt key is pre-loaded so that the keyfile is not required at runtime.

Trac: #720

Cc Steffan Karger <[email protected]...>

Signed-off-by Antonio Quartulli <[email protected]...>

---
v2:
- introduce this patch
v3:
- add key per-loading logic to this patch to avoid temporary features breakages
v4:
- rebase on top of master
- use buffer_read_from_file() implementation from "tls-crypt-v2: generate client keys". Minor change: in case of error, free buffer and remove it from gc (with free_buf_gc()), if provided.
- don't go FATAL inside buffer_read_from_file() in case of error, but let external logic decide
v5:
- fix various bits of pointer logic in free_buf_gc()
- implement unit test for free_buf_gc()
- use CLEAR() instead of free_buf() at the end of free_buf_gc() (problem found thanks to the unit-test)

 src/openvpn/buffer.c                   | 61 ++++++++++++++++++
 src/openvpn/buffer.h                   | 12 ++++
 src/openvpn/crypto.c                   | 21 ++-----
 src/openvpn/init.c                     | 87 +++++++++++++++++---------
 src/openvpn/options.c                  | 26 ++++++++
 tests/unit_tests/openvpn/Makefile.am   |  1 -
 tests/unit_tests/openvpn/test_buffer.c | 45 +++++++++++++
 7 files changed, 206 insertions(+), 47 deletions(-)

src/openvpn/buffer.c +61 -0

index becfeb93..0972139f 100644
@@ -189,6 +189,34 @@ free_buf(struct buffer *buf)
     CLEAR(*buf);
 }
 
+static void
+free_buf_gc(struct buffer *buf, struct gc_arena *gc)
+{
+    if (gc)
+    {
+        struct gc_entry **e = &gc->list;
+
+        while (*e)
+        {
+            /* check if this object is the one we want to delete */
+            if ((uint8_t *)(*e + 1) == buf->data)
+            {
+                struct gc_entry *to_delete = *e;
+
+                /* remove element from linked list and free it */
+                *e = (*e)->next;
+                free(to_delete);
+
+                break;
+            }
+
+            e = &(*e)->next;
+        }
+    }
+
+    CLEAR(*buf);
+}
+
 /*
  * Return a buffer for write that is a subset of another buffer
  */

@@ -1332,3 +1360,36 @@ buffer_list_file(const char *fn, int max_line_len)
     }
     return bl;
 }
+
+struct buffer
+buffer_read_from_file(const char *filename, struct gc_arena *gc)
+{
+    struct buffer ret = { 0 };
+
+    platform_stat_t file_stat = {0};
+    if (platform_stat(filename, &file_stat) < 0)
+    {
+        return ret;
+    }
+
+    FILE *fp = platform_fopen(filename, "r");
+    if (!fp)
+    {
+        return ret;
+    }
+
+    const size_t size = file_stat.st_size;
+    ret = alloc_buf_gc(size + 1, gc); /* space for trailing \0 */
+    ssize_t read_size = fread(BPTR(&ret), 1, size, fp);
+    if (read_size < 0)
+    {
+        free_buf_gc(&ret, gc);
+        goto cleanup;
+    }
+    ASSERT(buf_inc_len(&ret, read_size));
+    buf_null_terminate(&ret);
+
+cleanup:
+    fclose(fp);
+    return ret;
+}

src/openvpn/buffer.h +0 -0

index d848490a..a11c8bd9 100644

src/openvpn/buffer.h +6 -0

+ *
+ * @return the buffer storing the file content or an invalid buffer in case of
+ * error
+ */
+struct buffer buffer_read_from_file(const char *filename, struct gc_arena *gc);
+
 #endif /* BUFFER_H */

src/openvpn/crypto.c +5 -16

index b59c1f73..5381ef2a 100644
@@ -1224,7 +1224,7 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
 {
     struct gc_arena gc = gc_new();
     struct buffer in;
-    int fd, size;
+    int size;
     uint8_t hex_byte[3] = {0, 0, 0};
     const char *error_filename = file;
 

@@ -1268,22 +1268,11 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
     }
     else /* 'file' is a filename which refers to a file containing the ascii key */
     {
-        in = alloc_buf_gc(2048, &gc);
-        fd = platform_open(file, O_RDONLY, 0);
-        if (fd == -1)
-        {
-            msg(M_ERR, "Cannot open key file '%s'", file);
-        }
-        size = read(fd, in.data, in.capacity);
-        if (size < 0)
-        {
+        in = buffer_read_from_file(file, &gc);
+        if (!buf_valid(&in))
             msg(M_FATAL, "Read error on key file ('%s')", file);
-        }
-        if (size == in.capacity)
-        {
-            msg(M_FATAL, "Key file ('%s') can be a maximum of %d bytes", file, (int)in.capacity);
-        }
-        close(fd);
+
+        size = in.len;
     }
 
     cp = (unsigned char *)in.data;

src/openvpn/init.c +57 -30

index b748357d..1b46b364 100644
@@ -2409,7 +2409,6 @@ key_schedule_free(struct key_schedule *ks, bool free_ssl_ctx)
     if (tls_ctx_initialised(&ks->ssl_ctx) && free_ssl_ctx)
     {
         tls_ctx_free(&ks->ssl_ctx);
-        free_key_ctx_bi(&ks->tls_wrap_key);
     }
     CLEAR(*ks);
 }

@@ -2499,6 +2498,48 @@ do_init_crypto_static(struct context *c, const unsigned int flags)
     check_replay_consistency(&c->c1.ks.key_type, options->replay);
 }
 
+/*
+ * Initialize the tls-auth/crypt key context
+ */
+static void
+do_init_tls_wrap_key(struct context *c)
+{
+    const struct options *options = &c->options;
+
+    /* TLS handshake authentication (--tls-auth) */
+    if (options->tls_auth_file)
+    {
+        /* Initialize key_type for tls-auth with auth only */
+        CLEAR(c->c1.ks.tls_auth_key_type);
+        if (!streq(options->authname, "none"))
+        {
+            c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname);
+                c->c1.ks.tls_auth_key_type.hmac_length =
+                    md_kt_size(c->c1.ks.tls_auth_key_type.digest);
+        }
+        else
+        {
+            msg(M_FATAL, "ERROR: tls-auth enabled, but no valid --auth "
+                "algorithm specified ('%s')", options->authname);
+        }
+
+        crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type,
+                                &c->c1.ks.tls_wrap_key,
+                                options->tls_auth_file,
+                                options->tls_auth_file_inline,
+                                options->key_direction,
+                                "Control Channel Authentication", "tls-auth");
+    }
+
+    /* TLS handshake encryption+authentication (--tls-crypt) */
+    if (options->tls_crypt_file)
+    {
+        tls_crypt_init_key(&c->c1.ks.tls_wrap_key,
+                           options->tls_crypt_file,
+                           options->tls_crypt_inline, options->tls_server);
+    }
+}
+
 /*
  * Initialize the persistent component of OpenVPN's TLS mode,
  * which is preserved across SIGUSR1 resets.

@@ -2548,35 +2589,8 @@ do_init_crypto_tls_c1(struct context *c)
         /* Initialize PRNG with config-specified digest */
         prng_init(options->prng_hash, options->prng_nonce_secret_len);
 
-        /* TLS handshake authentication (--tls-auth) */
-        if (options->tls_auth_file)
-        {
-            /* Initialize key_type for tls-auth with auth only */
-            CLEAR(c->c1.ks.tls_auth_key_type);
-            if (!streq(options->authname, "none"))
-            {
-                c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname);
-                c->c1.ks.tls_auth_key_type.hmac_length =
-                    md_kt_size(c->c1.ks.tls_auth_key_type.digest);
-            }
-            else
-            {
-                msg(M_FATAL, "ERROR: tls-auth enabled, but no valid --auth "
-                    "algorithm specified ('%s')", options->authname);
-            }
-
-            crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type,
-                                    &c->c1.ks.tls_wrap_key, options->tls_auth_file,
-                                    options->tls_auth_file_inline, options->key_direction,
-                                    "Control Channel Authentication", "tls-auth");
-        }
-
-        /* TLS handshake encryption+authentication (--tls-crypt) */
-        if (options->tls_crypt_file)
-        {
-            tls_crypt_init_key(&c->c1.ks.tls_wrap_key, options->tls_crypt_file,
-                               options->tls_crypt_inline, options->tls_server);
-        }
+        /* initialize tls-auth/crypt key */
+        do_init_tls_wrap_key(c);
 
         c->c1.ciphername = options->ciphername;
         c->c1.authname = options->authname;

@@ -2598,6 +2612,12 @@ do_init_crypto_tls_c1(struct context *c)
         c->options.ciphername = c->c1.ciphername;
         c->options.authname = c->c1.authname;
         c->options.keysize = c->c1.keysize;
+
+        /*
+         * tls-auth/crypt key can be configured per connection block, therefore
+         * we must reload it as it may have changed
+         */
+        do_init_tls_wrap_key(c);
     }
 }
 

@@ -3401,6 +3421,13 @@ do_close_tls(struct context *c)
 static void
 do_close_free_key_schedule(struct context *c, bool free_ssl_ctx)
 {
+    /*
+     * always free the tls_auth/crypt key. If persist_key is true, the key will
+     * be reloaded from memory (pre-cached)
+     */
+    free_key_ctx_bi(&c->c1.ks.tls_wrap_key);
+    CLEAR(c->c1.ks.tls_wrap_key);
+
     if (!(c->sig->signal_received == SIGUSR1 && c->options.persist_key))
     {
         key_schedule_free(&c->c1.ks, free_ssl_ctx);

src/openvpn/options.c +26 -0

index 426057ab..b2ca2738 100644
@@ -3019,6 +3019,32 @@ options_postprocess_mutate(struct options *o)
         options_postprocess_mutate_ce(o, o->connection_list->array[i]);
     }
 
+    /* pre-cache tls-auth/crypt key file if persist-key was specified */
+    if (o->persist_key)
+    {
+        if (o->tls_auth_file && !o->tls_auth_file_inline)
+        {
+            struct buffer in = buffer_read_from_file(o->tls_auth_file, &o->gc);
+            if (!buf_valid(&in))
+                msg(M_FATAL, "Cannot pre-load tls-auth keyfile (%s)",
+                    o->tls_auth_file);
+
+            o->tls_auth_file = INLINE_FILE_TAG;
+            o->tls_auth_file_inline = (char *)in.data;
+        }
+
+        if (o->tls_crypt_file && !o->tls_crypt_inline)
+        {
+            struct buffer in = buffer_read_from_file(o->tls_crypt_file, &o->gc);
+            if (!buf_valid(&in))
+                msg(M_FATAL, "Cannot pre-load tls-crypt keyfile (%s)",
+                    o->tls_auth_file);
+
+            o->tls_crypt_file = INLINE_FILE_TAG;
+            o->tls_crypt_inline = (char *)in.data;
+        }
+    }
+
     if (o->tls_server)
     {
         /* Check that DH file is specified, or explicitly disabled */

tests/unit_tests/openvpn/Makefile.am +0 -1

index 23d758b7..4395afc1 100644
@@ -26,7 +26,6 @@ argv_testdriver_SOURCES = test_argv.c mock_msg.c \
 buffer_testdriver_CFLAGS  = @TEST_CFLAGS@ -I$(openvpn_srcdir) -I$(compat_srcdir)
 buffer_testdriver_LDFLAGS = @TEST_LDFLAGS@ -L$(openvpn_srcdir) -Wl,--wrap=parse_line
 buffer_testdriver_SOURCES = test_buffer.c mock_msg.c \
-	$(openvpn_srcdir)/buffer.c \
 	$(openvpn_srcdir)/platform.c
 
 packet_id_testdriver_CFLAGS  = @TEST_CFLAGS@ \

tests/unit_tests/openvpn/test_buffer.c +43 -0

index d083b78f..fd64ee59 100644
@@ -33,6 +33,7 @@
 #include <cmocka.h>
 
 #include "buffer.h"
+#include "buffer.c"
 
 static void
 test_buffer_strprefix(void **state)

@@ -197,6 +198,48 @@ test_buffer_list_aggregate_separator_emptybuffers(void **state)
     assert_int_equal(BLEN(buf), 0);
 }
 
+static void
+test_buffer_free_gc_one(void **state)
+{
+    struct gc_arena gc = gc_new();
+    struct buffer buf = alloc_buf_gc(1024, &gc);
+
+    assert_ptr_equal(gc.list + 1, buf.data);
+    free_buf_gc(&buf, &gc);
+    assert_null(gc.list);
+
+    gc_free(&gc);
+}
+
+static void
+test_buffer_free_gc_two(void **state)
+{
+    struct gc_arena gc = gc_new();
+    struct buffer buf1 = alloc_buf_gc(1024, &gc);
+    struct buffer buf2 = alloc_buf_gc(1024, &gc);
+    struct buffer buf3 = alloc_buf_gc(1024, &gc);
+
+    struct gc_entry *e;
+
+    e = gc.list;
+
+    assert_ptr_equal(e + 1, buf3.data);
+    assert_ptr_equal(e->next + 1, buf2.data);
+    assert_ptr_equal(e->next->next + 1, buf1.data);
+
+    free_buf_gc(&buf2, &gc);
+
+    assert_non_null(gc.list);
+
+    while (e)
+    {
+        assert_ptr_not_equal(e + 1, buf2.data);
+        e = e->next;
+    }
+
+    gc_free(&gc);
+}
+
 int
 main(void)
 {

In preparation to having tls-auth/crypt keys per connection block, it is important to ensure that such material is always reloaded upon SIGUSR1, no matter if persist-key was specified or not.

This is required because when moving from one remote to the other the key may change and thus the key context needs to be refreshed.

To ensure that the persist-key logic will still work as expected, the tls-auth/crypt key is pre-loaded so that the keyfile is not required at runtime.

Trac: #720

Cc Steffan Karger <[email protected]...>

Signed-off-by Antonio Quartulli <[email protected]...>

---
v2:
- introduce this patch
v3:
- add key per-loading logic to this patch to avoid temporary features breakages
v4:
- rebase on top of master
- use buffer_read_from_file() implementation from "tls-crypt-v2: generate client keys". Minor change: in case of error, free buffer and remove it from gc (with free_buf_gc()), if provided.
- don't go FATAL inside buffer_read_from_file() in case of error, but let external logic decide
v5:
- fix various bits of pointer logic in free_buf_gc()
- implement unit test for free_buf_gc()
- use CLEAR() instead of free_buf() at the end of free_buf_gc() v6:
- rebase on top of latest master
- fix typ0 in function doc
- use cmocka_unit_test() instead of cmocka_unit_test_setup_teardown()

 src/openvpn/buffer.c                   | 61 ++++++++++++++++++
 src/openvpn/buffer.h                   | 12 ++++
 src/openvpn/crypto.c                   | 21 ++-----
 src/openvpn/init.c                     | 87 +++++++++++++++++---------
 src/openvpn/options.c                  | 26 ++++++++
 tests/unit_tests/openvpn/Makefile.am   |  1 -
 tests/unit_tests/openvpn/test_buffer.c | 45 +++++++++++++
 7 files changed, 206 insertions(+), 47 deletions(-)

src/openvpn/buffer.c +61 -0

index becfeb93..0972139f 100644
@@ -189,6 +189,34 @@ free_buf(struct buffer *buf)
     CLEAR(*buf);
 }
 
+static void
+free_buf_gc(struct buffer *buf, struct gc_arena *gc)
+{
+    if (gc)
+    {
+        struct gc_entry **e = &gc->list;
+
+        while (*e)
+        {
+            /* check if this object is the one we want to delete */
+            if ((uint8_t *)(*e + 1) == buf->data)
+            {
+                struct gc_entry *to_delete = *e;
+
+                /* remove element from linked list and free it */
+                *e = (*e)->next;
+                free(to_delete);
+
+                break;
+            }
+
+            e = &(*e)->next;
+        }
+    }
+
+    CLEAR(*buf);
+}
+
 /*
  * Return a buffer for write that is a subset of another buffer
  */

@@ -1332,3 +1360,36 @@ buffer_list_file(const char *fn, int max_line_len)
     }
     return bl;
 }
+
+struct buffer
+buffer_read_from_file(const char *filename, struct gc_arena *gc)
+{
+    struct buffer ret = { 0 };
+
+    platform_stat_t file_stat = {0};
+    if (platform_stat(filename, &file_stat) < 0)
+    {
+        return ret;
+    }
+
+    FILE *fp = platform_fopen(filename, "r");
+    if (!fp)
+    {
+        return ret;
+    }
+
+    const size_t size = file_stat.st_size;
+    ret = alloc_buf_gc(size + 1, gc); /* space for trailing \0 */
+    ssize_t read_size = fread(BPTR(&ret), 1, size, fp);
+    if (read_size < 0)
+    {
+        free_buf_gc(&ret, gc);
+        goto cleanup;
+    }
+    ASSERT(buf_inc_len(&ret, read_size));
+    buf_null_terminate(&ret);
+
+cleanup:
+    fclose(fp);
+    return ret;
+}

src/openvpn/buffer.h +12 -0

index d848490a..6b6025ed 100644
@@ -1112,4 +1112,16 @@ void buffer_list_aggregate_separator(struct buffer_list *bl,
 
 struct buffer_list *buffer_list_file(const char *fn, int max_line_len);
 
+/**
+ * buffer_read_from_file - copy the content of a file into a buffer
+ *
+ * @param file      path to the file to read
+ * @param gc        the garbage collector to use when allocating the buffer. It
+ *                  is passed to alloc_buf_gc() and therefore can be NULL.
+ *
+ * @return the buffer storing the file content or an invalid buffer in case of
+ * error
+ */
+struct buffer buffer_read_from_file(const char *filename, struct gc_arena *gc);
+
 #endif /* BUFFER_H */

src/openvpn/crypto.c +5 -16

index b59c1f73..5381ef2a 100644
@@ -1224,7 +1224,7 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
 {
     struct gc_arena gc = gc_new();
     struct buffer in;
-    int fd, size;
+    int size;
     uint8_t hex_byte[3] = {0, 0, 0};
     const char *error_filename = file;
 

@@ -1268,22 +1268,11 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
     }
     else /* 'file' is a filename which refers to a file containing the ascii key */
     {
-        in = alloc_buf_gc(2048, &gc);
-        fd = platform_open(file, O_RDONLY, 0);
-        if (fd == -1)
-        {
-            msg(M_ERR, "Cannot open key file '%s'", file);
-        }
-        size = read(fd, in.data, in.capacity);
-        if (size < 0)
-        {
+        in = buffer_read_from_file(file, &gc);
+        if (!buf_valid(&in))
             msg(M_FATAL, "Read error on key file ('%s')", file);
-        }
-        if (size == in.capacity)
-        {
-            msg(M_FATAL, "Key file ('%s') can be a maximum of %d bytes", file, (int)in.capacity);
-        }
-        close(fd);
+
+        size = in.len;
     }
 
     cp = (unsigned char *)in.data;

src/openvpn/init.c +57 -30

index f6c8f08e..90c987b2 100644
@@ -2409,7 +2409,6 @@ key_schedule_free(struct key_schedule *ks, bool free_ssl_ctx)
     if (tls_ctx_initialised(&ks->ssl_ctx) && free_ssl_ctx)
     {
         tls_ctx_free(&ks->ssl_ctx);
-        free_key_ctx_bi(&ks->tls_wrap_key);
     }
     CLEAR(*ks);
 }

@@ -2499,6 +2498,48 @@ do_init_crypto_static(struct context *c, const unsigned int flags)
     check_replay_consistency(&c->c1.ks.key_type, options->replay);
 }
 
+/*
+ * Initialize the tls-auth/crypt key context
+ */
+static void
+do_init_tls_wrap_key(struct context *c)
+{
+    const struct options *options = &c->options;
+
+    /* TLS handshake authentication (--tls-auth) */
+    if (options->tls_auth_file)
+    {
+        /* Initialize key_type for tls-auth with auth only */
+        CLEAR(c->c1.ks.tls_auth_key_type);
+        if (!streq(options->authname, "none"))
+        {
+            c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname);
+                c->c1.ks.tls_auth_key_type.hmac_length =
+                    md_kt_size(c->c1.ks.tls_auth_key_type.digest);
+        }
+        else
+        {
+            msg(M_FATAL, "ERROR: tls-auth enabled, but no valid --auth "
+                "algorithm specified ('%s')", options->authname);
+        }
+
+        crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type,
+                                &c->c1.ks.tls_wrap_key,
+                                options->tls_auth_file,
+                                options->tls_auth_file_inline,
+                                options->key_direction,
+                                "Control Channel Authentication", "tls-auth");
+    }
+
+    /* TLS handshake encryption+authentication (--tls-crypt) */
+    if (options->tls_crypt_file)
+    {
+        tls_crypt_init_key(&c->c1.ks.tls_wrap_key,
+                           options->tls_crypt_file,
+                           options->tls_crypt_inline, options->tls_server);
+    }
+}
+
 /*
  * Initialize the persistent component of OpenVPN's TLS mode,
  * which is preserved across SIGUSR1 resets.

@@ -2548,35 +2589,8 @@ do_init_crypto_tls_c1(struct context *c)
         /* Initialize PRNG with config-specified digest */
         prng_init(options->prng_hash, options->prng_nonce_secret_len);
 
-        /* TLS handshake authentication (--tls-auth) */
-        if (options->tls_auth_file)
-        {
-            /* Initialize key_type for tls-auth with auth only */
-            CLEAR(c->c1.ks.tls_auth_key_type);
-            if (!streq(options->authname, "none"))
-            {
-                c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname);
-                c->c1.ks.tls_auth_key_type.hmac_length =
-                    md_kt_size(c->c1.ks.tls_auth_key_type.digest);
-            }
-            else
-            {
-                msg(M_FATAL, "ERROR: tls-auth enabled, but no valid --auth "
-                    "algorithm specified ('%s')", options->authname);
-            }
-
-            crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type,
-                                    &c->c1.ks.tls_wrap_key, options->tls_auth_file,
-                                    options->tls_auth_file_inline, options->key_direction,
-                                    "Control Channel Authentication", "tls-auth");
-        }
-
-        /* TLS handshake encryption+authentication (--tls-crypt) */
-        if (options->tls_crypt_file)
-        {
-            tls_crypt_init_key(&c->c1.ks.tls_wrap_key, options->tls_crypt_file,
-                               options->tls_crypt_inline, options->tls_server);
-        }
+        /* initialize tls-auth/crypt key */
+        do_init_tls_wrap_key(c);
 
         c->c1.ciphername = options->ciphername;
         c->c1.authname = options->authname;

@@ -2598,6 +2612,12 @@ do_init_crypto_tls_c1(struct context *c)
         c->options.ciphername = c->c1.ciphername;
         c->options.authname = c->c1.authname;
         c->options.keysize = c->c1.keysize;
+
+        /*
+         * tls-auth/crypt key can be configured per connection block, therefore
+         * we must reload it as it may have changed
+         */
+        do_init_tls_wrap_key(c);
     }
 }
 

@@ -3401,6 +3421,13 @@ do_close_tls(struct context *c)
 static void
 do_close_free_key_schedule(struct context *c, bool free_ssl_ctx)
 {
+    /*
+     * always free the tls_auth/crypt key. If persist_key is true, the key will
+     * be reloaded from memory (pre-cached)
+     */
+    free_key_ctx_bi(&c->c1.ks.tls_wrap_key);
+    CLEAR(c->c1.ks.tls_wrap_key);
+
     if (!(c->sig->signal_received == SIGUSR1 && c->options.persist_key))
     {
         key_schedule_free(&c->c1.ks, free_ssl_ctx);

src/openvpn/options.c +26 -0

index 426057ab..b2ca2738 100644
@@ -3019,6 +3019,32 @@ options_postprocess_mutate(struct options *o)
         options_postprocess_mutate_ce(o, o->connection_list->array[i]);
     }
 
+    /* pre-cache tls-auth/crypt key file if persist-key was specified */
+    if (o->persist_key)
+    {
+        if (o->tls_auth_file && !o->tls_auth_file_inline)
+        {
+            struct buffer in = buffer_read_from_file(o->tls_auth_file, &o->gc);
+            if (!buf_valid(&in))
+                msg(M_FATAL, "Cannot pre-load tls-auth keyfile (%s)",
+                    o->tls_auth_file);
+
+            o->tls_auth_file = INLINE_FILE_TAG;
+            o->tls_auth_file_inline = (char *)in.data;
+        }
+
+        if (o->tls_crypt_file && !o->tls_crypt_inline)
+        {
+            struct buffer in = buffer_read_from_file(o->tls_crypt_file, &o->gc);
+            if (!buf_valid(&in))
+                msg(M_FATAL, "Cannot pre-load tls-crypt keyfile (%s)",
+                    o->tls_auth_file);
+
+            o->tls_crypt_file = INLINE_FILE_TAG;
+            o->tls_crypt_inline = (char *)in.data;
+        }
+    }
+
     if (o->tls_server)
     {
         /* Check that DH file is specified, or explicitly disabled */

tests/unit_tests/openvpn/Makefile.am +0 -1

index db4d46e1..fe39f809 100644
@@ -28,7 +28,6 @@ buffer_testdriver_CFLAGS  = @TEST_CFLAGS@ -I$(openvpn_srcdir) -I$(compat_srcdir)
 buffer_testdriver_LDFLAGS = @TEST_LDFLAGS@ -L$(openvpn_srcdir) -Wl,--wrap=parse_line
 buffer_testdriver_SOURCES = test_buffer.c mock_msg.c \
 	mock_get_random.c \
-	$(openvpn_srcdir)/buffer.c \
 	$(openvpn_srcdir)/platform.c
 
 packet_id_testdriver_CFLAGS  = @TEST_CFLAGS@ \

tests/unit_tests/openvpn/test_buffer.c +43 -0

index d083b78f..7c9a9e2f 100644
@@ -33,6 +33,7 @@
 #include <cmocka.h>
 
 #include "buffer.h"
+#include "buffer.c"
 
 static void
 test_buffer_strprefix(void **state)

@@ -197,6 +198,48 @@ test_buffer_list_aggregate_separator_emptybuffers(void **state)
     assert_int_equal(BLEN(buf), 0);
 }
 
+static void
+test_buffer_free_gc_one(void **state)
+{
+    struct gc_arena gc = gc_new();
+    struct buffer buf = alloc_buf_gc(1024, &gc);
+
+    assert_ptr_equal(gc.list + 1, buf.data);
+    free_buf_gc(&buf, &gc);
+    assert_null(gc.list);
+
+    gc_free(&gc);
+}
+
+static void
+test_buffer_free_gc_two(void **state)
+{
+    struct gc_arena gc = gc_new();
+    struct buffer buf1 = alloc_buf_gc(1024, &gc);
+    struct buffer buf2 = alloc_buf_gc(1024, &gc);
+    struct buffer buf3 = alloc_buf_gc(1024, &gc);
+
+    struct gc_entry *e;
+
+    e = gc.list;
+
+    assert_ptr_equal(e + 1, buf3.data);
+    assert_ptr_equal(e->next + 1, buf2.data);
+    assert_ptr_equal(e->next->next + 1, buf1.data);
+
+    free_buf_gc(&buf2, &gc);
+
+    assert_non_null(gc.list);
+
+    while (e)
+    {
+        assert_ptr_not_equal(e + 1, buf2.data);
+        e = e->next;
+    }
+
+    gc_free(&gc);
+}
+
 int
 main(void)
 {