Skip to contentSkip to conversation

Remove duplicate X509 env variables

PATCH In review Steffan Karger 2 participants 2 messages Mar 9, 2017 – Mar 15, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Remove duplicate X509 env variables

Commit 13b585e8 added support for multiple X509 env variables with the same name, but as a side effect caused these variables to pile up for each renegotiation. The old code would simply overwrite the old variables (as long as an equally-long chain was used for the new session).

To stop the variables from piling up, this commit removes any old X509 env variables if we start negotiating a new TLS session.

Trac: #854

---
 src/openvpn/ssl.c        |  3 +++
 src/openvpn/ssl_verify.c | 17 +++++++++++++++++
 src/openvpn/ssl_verify.h |  3 +++
 3 files changed, 23 insertions(+)
src/openvpn/ssl.c +3 -0
index 401b8fd..1189f56 100644
@@ -2821,6 +2821,9 @@ tls_process(struct tls_multi *multi,
                                    session->opt->crl_file, session->opt->crl_file_inline);
             }
 
+            /* New connection, remove any old X509 env variables */
+            tls_x509_clear_env(session->opt->es);
+
             dmsg(D_TLS_DEBUG_MED, "STATE S_START");
         }
 
src/openvpn/ssl_verify.c +17 -0
index 9f12ab8..a6e9be3 100644
@@ -1486,4 +1486,21 @@ verify_final_auth_checks(struct tls_multi *multi, struct tls_session *session)
         gc_free(&gc);
     }
 }
+
+void
+tls_x509_clear_env(struct env_set *es)
+{
+    struct env_item *item = es->list;
+    while (item)
+    {
+        struct env_item *next = item->next;
+        if (item->string
+            && 0 == strncmp("X509_", item->string, strlen("X509_")))
+        {
+            env_set_del(es, item->string);
+        }
+        item = next;
+    }
+}
+
 #endif /* ENABLE_CRYPTO */
src/openvpn/ssl_verify.h +3 -0
index ffab218..d91799e 100644
@@ -238,6 +238,9 @@ tls_client_reason(struct tls_multi *multi)
 #endif
 }
 
+/** Remove any X509_ env variables from env_set es */
+void tls_x509_clear_env(struct env_set *es);
+
 #endif /* ENABLE_CRYPTO */
 
 #endif /* SSL_VERIFY_H_ */
Signature · 1 line
2.7.4
#1 lore raw

ACK. "Stared at the code and mulled over whether it's safe" (plus ran my test set). Tested by the reporter in trac #854, so that should be good enough.

Your patch has been applied to the master and release/2.4 branch.

commit fd0361813cd3d5a55f3408a018e2ed776d79fef6 (master) commit 19d6cd4e63557c983cb0ab5094688c974588c2ff (release/2.4)

Author: Steffan Karger
Date:   Thu Mar 9 09:13:32 2017 +0100
     Remove duplicate X509 env variables

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14237.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering