diff mbox series

[Openvpn-devel,M] Change in openvpn[master]: Keep exported certificate files for following calls

Message ID fec0381cb912c2749608d1e69054cdcf9bacf3b8-HTML@gerrit.openvpn.net
State New
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: Keep exported certificate files for following calls | expand

Commit Message

ordex (Code Review) Jan. 2, 2024, 1:46 p.m. UTC 
Attention is currently required from: flichtenheld.

Hello flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/487?usp=email

to review the following change.


Change subject: Keep exported certificate files for following calls
......................................................................

Keep exported certificate files for following calls

Since the lifetime of environment variables is quite different, we
need to tie the lifetime of these files to their environment variables
which in turn requires a special function to be called on the removal
of these env variables.

Change-Id: Ic494d43c835220ae71f10e3afbe53db918887370
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
M src/openvpn/ssl_verify.c
1 file changed, 31 insertions(+), 37 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/87/487/1
diff mbox series

Patch

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 35d3377..ff1a932 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -39,6 +39,7 @@ 
 #include "run_command.h"
 #include "ssl_verify.h"
 #include "ssl_verify_backend.h"
+#include "platform.h"
 
 #ifdef ENABLE_CRYPTO_OPENSSL
 #include "ssl_verify_openssl.h"
@@ -459,53 +460,51 @@ 
 }
 
 /**
+ * Unlinks a file specified by in the env item that has the form
+ * key=filename.
+ */
+static void
+unlink_file_env(struct env_item *env)
+{
+    /* values in env are always x=y */
+    const char *filename = strchr(env->string, '=');
+    ASSERT(filename);
+
+    /* Move just past the = */
+    filename += 1;
+
+    platform_unlink((const char *) filename);
+}
+
+/**
  * Exports the certificate in \c peer_cert into the environment and adds
  * the filname
  */
 static bool
-verify_cert_cert_export_env(struct env_set *es, openvpn_x509_cert_t *peer_cert,
-                            int cert_depth, const char *pem_export_fname)
+verify_cert_cert_export_env(const struct tls_options *opt,
+                            openvpn_x509_cert_t *peer_cert, int cert_depth)
 {
-    char envname[64];
-    /* Make copy of the filename to manage that copy by the gc_arena */
-    char *pem_export_filename = strdup(pem_export_fname);
-
-    if (!pem_export_filename)
-    {
-        return false;
-    }
+    struct gc_arena gc = gc_new();
+    const char *pem_export_filename = platform_create_temp_file(opt->export_peer_cert_dir,
+                                                                "pef", &gc);
+    char envstr[128];
 
     /* export the path to the certificate in pem file format */
-    openvpn_snprintf(envname, sizeof(envname), "peer_cert_%d", cert_depth);
-    setenv_str(es, envname, pem_export_filename);
+    openvpn_snprintf(envstr, sizeof(envstr), "peer_cert_%d=%s", cert_depth,
+                     pem_export_filename);
+    setenv_str(opt->es, envstr, pem_export_filename);
+    env_set_add_specialfree(opt->es, envstr, &unlink_file_env);
 
     /* compatibility with older scripts/plugins that expect peer_cert without
      * suffix */
     if (cert_depth == 0)
     {
-        setenv_str(es, "peer_cert", pem_export_filename);
+        setenv_str(opt->es, "peer_cert", pem_export_filename);
     }
 
     return backend_x509_write_pem(peer_cert, pem_export_filename) == SUCCESS;
 }
 
-static void
-verify_cert_cert_delete_env(struct env_set *es, int cert_depth,
-                            const char *pem_export_fname)
-{
-    char envname[64];
-    openvpn_snprintf(envname, sizeof(envname), "peer_cert_%d", cert_depth);
-    env_set_del(es, envname);
-
-    /* compatibility with older scripts/plugins that expect peer_cert without
-     * suffix */
-    if (cert_depth == 0)
-    {
-        env_set_del(es, "peer_cert");
-    }
-    unlink(pem_export_fname);
-}
-
 /*
  * call --tls-verify plug-in(s)
  */
@@ -625,7 +624,6 @@ 
      * them defined */
     result_t ret = FAILURE;
     struct gc_arena gc = gc_new();
-    const char *pem_export_fname = NULL;
 
     const struct tls_options *opt = session->opt;
     ASSERT(opt);
@@ -758,12 +756,9 @@ 
 
     if (opt->export_peer_cert_dir)
     {
-        pem_export_fname = platform_create_temp_file(opt->export_peer_cert_dir,
-                                                     "pef", &gc);
 
-        if (!pem_export_fname
-            || !verify_cert_cert_export_env(opt->es, cert, cert_depth,
-                                            pem_export_fname))
+
+        if (!verify_cert_cert_export_env(opt, cert, cert_depth))
         {
             msg(D_TLS_ERRORS, "TLS Error: Failed to export certificate for "
                 "--tls-export-cert in %s", opt->export_peer_cert_dir);
@@ -821,7 +816,6 @@ 
     ret = SUCCESS;
 
 cleanup:
-    verify_cert_cert_delete_env(opt->es, cert_depth, pem_export_fname);
     if (ret != SUCCESS)
     {
         tls_clear_error(); /* always? */