Hi,
On Sun, Nov 29, 2015 at 6:29 AM, Selva Nair <selva.n...@gmail.com> wrote:
> A useful change and clean code. A couple of places could benefit from const
> qualifiers, though
You're absolutely right. Thanks. Attached a v2 patch that adds the
suggested const qualifiers.
Now that I was looking at my own code again, I also realized a
strcpy() makes more sense in setenv_str_incr() than the memcpy() from
the previous patch. (Both work just fine, but this should be easier
to read.)
-Steffan
From 44a5af585953d5384d3bbd64e55c1de6343919d8 Mon Sep 17 00:00:00 2001
From: Steffan Karger <stef...@karger.me>
Date: Sun, 29 Nov 2015 10:39:24 +0100
Subject: [PATCH] Support duplicate x509 field values in environment
As reported in trac #387, an x509 DN can contain duplicate fields.
Previously, we would overwrite any previous field value with a new one if
we would process a second same-name field. Now, instead, append _$N,
starting at N=1 to the name for each consequent field to export all fields
to the enviroment.
v2 - make better use of const qualifiers in env_set_get(), and use strcpy()
instead of memcpy() in setenv_str_incr()
Signed-off-by: Steffan Karger <stef...@karger.me>
---
Changes.rst | 7 +++++++
src/openvpn/misc.c | 32 ++++++++++++++++++++++++++++++++
src/openvpn/misc.h | 7 +++++++
src/openvpn/ssl_verify_openssl.c | 2 +-
src/openvpn/ssl_verify_polarssl.c | 2 +-
5 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/Changes.rst b/Changes.rst
index c674da5..e712f63 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -36,6 +36,13 @@ LZ4 Compression
User-visible Changes
--------------------
+- For certificate DNs with duplicate fields, e.g. "OU=one,OU=two", both fields
+ are now exported to the environment, where each second and later occurrence
+ of a field get _$N appended to it's field name, starting at N=1. For the
+ example above, that would result in e.g. X509_0_OU=one, X509_0_OU_1=two.
+ Note that this breaks setups that rely on the fact that OpenVPN would
+ previously (incorrectly) only export the last occurence of a field.
+
- proto udp and proto tcp specify to use IPv4 and IPv6. The new
options proto udp4 and tcp4 specify to use IPv4 only.
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 0f04d09..6bf46ce 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -607,6 +607,16 @@ env_set_add (struct env_set *es, const char *str)
env_set_add_nolock (es, str);
}
+const char*
+env_set_get (const struct env_set *es, const char *name)
+{
+ const struct env_item *item = es->list;
+ while (item && !env_string_equal(item->string, name)) {
+ item = item->next;
+ }
+ return item ? item->string : NULL;
+}
+
void
env_set_print (int msglevel, const struct env_set *es)
{
@@ -741,6 +751,28 @@ setenv_str_safe (struct env_set *es, const char *name, const char *value)
msg (M_WARN, "setenv_str_safe: name overflow");
}
+void setenv_str_incr(struct env_set *es, const char *name, const char *value)
+{
+ unsigned int counter = 1;
+ const size_t tmpname_len = strlen(name) + 5; /* 3 digits counter max */
+ char *tmpname = gc_malloc(tmpname_len, true, NULL);
+ strcpy(tmpname, name);
+ while (NULL != env_set_get(es, tmpname) && counter < 1000)
+ {
+ ASSERT (openvpn_snprintf (tmpname, tmpname_len, "%s_%u", name, counter));
+ counter++;
+ }
+ if (counter < 1000)
+ {
+ setenv_str (es, tmpname, value);
+ }
+ else
+ {
+ msg (D_TLS_DEBUG_MED, "Too many same-name env variables, ignoring: %s", name);
+ }
+ free (tmpname);
+}
+
void
setenv_del (struct env_set *es, const char *name)
{
diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h
index ec2e417..7686399 100644
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -133,6 +133,12 @@ void setenv_str (struct env_set *es, const char *name, const char *value);
void setenv_str_safe (struct env_set *es, const char *name, const char *value);
void setenv_del (struct env_set *es, const char *name);
+/**
+ * Store the supplied name value pair in the env_set. If the variable with the
+ * supplied name already exists, append _N to the name, starting at N=1.
+ */
+void setenv_str_incr(struct env_set *es, const char *name, const char *value);
+
void setenv_int_i (struct env_set *es, const char *name, const int value, const int i);
void setenv_str_i (struct env_set *es, const char *name, const char *value, const int i);
@@ -142,6 +148,7 @@ struct env_set *env_set_create (struct gc_arena *gc);
void env_set_destroy (struct env_set *es);
bool env_set_del (struct env_set *es, const char *str);
void env_set_add (struct env_set *es, const char *str);
+const char* env_set_get (const struct env_set *es, const char *name);
void env_set_print (int msglevel, const struct env_set *es);
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index bf53522..d014f9d 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -448,7 +448,7 @@ x509_setenv (struct env_set *es, int cert_depth, openvpn_x509_cert_t *peer_cert)
objbuf);
string_mod (name_expand, CC_PRINT, CC_CRLF, '_');
string_mod ((char*)buf, CC_PRINT, CC_CRLF, '_');
- setenv_str (es, name_expand, (char*)buf);
+ setenv_str_incr (es, name_expand, (char*)buf);
free (name_expand);
OPENSSL_free (buf);
}
diff --git a/src/openvpn/ssl_verify_polarssl.c b/src/openvpn/ssl_verify_polarssl.c
index 62818ad..a2e6a8e 100644
--- a/src/openvpn/ssl_verify_polarssl.c
+++ b/src/openvpn/ssl_verify_polarssl.c
@@ -245,7 +245,7 @@ x509_setenv (struct env_set *es, int cert_depth, openvpn_x509_cert_t *cert)
/* Check both strings, set environment variable */
string_mod (name_expand, CC_PRINT, CC_CRLF, '_');
string_mod ((char*)s, CC_PRINT, CC_CRLF, '_');
- setenv_str (es, name_expand, (char*)s);
+ setenv_str_incr (es, name_expand, (char*)s);
name = name->next;
}
--
2.5.0
