[Openvpn-devel] [PATCH 07/10] Implemented x509-track for PolarSSL.
Signed-off-by: James Yonan
---
src/openvpn/ssl_verify_polarssl.c | 166 ++
src/openvpn/syshead.h | 2 +-
2 files changed, 167 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/ssl_verify_polarssl.c
b/src/openvpn/ssl_verify_polarssl.c
index 9d0d086..ab693d2 100644
--- a/src/openvpn/ssl_verify_polarssl.c
+++ b/src/openvpn/ssl_verify_polarssl.c
@@ -198,6 +198,172 @@ x509_get_subject(x509_crt *cert, struct gc_arena *gc)
return subject;
}
+#ifdef ENABLE_X509_TRACK
+
+/* these match NID's in OpenSSL crypto/objects/objects.h */
+#define NID_undef 0
+#define NID_sha164
+#define NID_commonName 13
+#define NID_countryName 14
+#define NID_localityName15
+#define NID_stateOrProvinceName 16
+#define NID_organizationName 17
+#define NID_organizationalUnitName 18
+#define NID_pkcs9_emailAddress 48
+
+struct nid_entry {
+ const char *name;
+ int nid;
+};
+
+static const struct nid_entry nid_list[] = {
+ { "SHA1", NID_sha1 },
+ { "CN", NID_commonName },
+ { "C",NID_countryName },
+ { "L",NID_localityName },
+ { "ST", NID_stateOrProvinceName },
+ { "O",NID_organizationName },
+ { "OU", NID_organizationalUnitName },
+ { "emailAddress", NID_pkcs9_emailAddress },
+ { NULL, 0 }
+};
+
+static int
+name_to_nid(const char *name)
+{
+ const struct nid_entry *e = nid_list;
+ while (e->name)
+{
+ if (!strcmp(name, e->name))
+ return e->nid;
+ ++e;
+}
+ return NID_undef;
+}
+
+static void
+do_setenv_x509 (struct env_set *es, const char *name, char *value, int depth)
+{
+ char *name_expand;
+ size_t name_expand_size;
+
+ string_mod (value, CC_ANY, CC_CRLF, '?');
+ msg (D_X509_ATTR, "X509 ATTRIBUTE name='%s' value='%s' depth=%d", name,
value, depth);
+ name_expand_size = 64 + strlen (name);
+ name_expand = (char *) malloc (name_expand_size);
+ check_malloc_return (name_expand);
+ openvpn_snprintf (name_expand, name_expand_size, "X509_%d_%s", depth, name);
+ setenv_str (es, name_expand, value);
+ free (name_expand);
+}
+
+static void
+do_setenv_nid_value(struct env_set *es, const struct x509_track *xt, const
x509_name *xn,
+ int depth, struct gc_arena *gc)
+{
+ size_t i;
+ char *val;
+
+ for (i = 0; i < xn->val.len; ++i)
+if (xn->val.p[i] == '\0') /* error if embedded null in value */
+ return;
+ val = gc_malloc(xn->val.len+1, false, gc);
+ memcpy(val, xn->val.p, xn->val.len);
+ val[xn->val.len] = '\0';
+ do_setenv_x509(es, xt->name, val, depth);
+}
+
+static void
+do_setenv_nid(struct env_set *es, const struct x509_track *xt, const x509_crt
*cert,
+ int depth, struct gc_arena *gc)
+{
+ const x509_name *xn;
+ for (xn = &cert->subject; xn != NULL; xn = xn->next)
+{
+ switch (xt->nid)
+ {
+ case NID_commonName:
+ if (OID_CMP(OID_AT_CN, &xn->oid))
+ do_setenv_nid_value(es, xt, xn, depth, gc);
+ break;
+ case NID_countryName:
+ if (OID_CMP(OID_AT_COUNTRY, &xn->oid))
+ do_setenv_nid_value(es, xt, xn, depth, gc);
+ break;
+ case NID_localityName:
+ if (OID_CMP(OID_AT_LOCALITY, &xn->oid))
+ do_setenv_nid_value(es, xt, xn, depth, gc);
+ break;
+ case NID_stateOrProvinceName:
+ if (OID_CMP(OID_AT_STATE, &xn->oid))
+ do_setenv_nid_value(es, xt, xn, depth, gc);
+ break;
+ case NID_organizationName:
+ if (OID_CMP(OID_AT_ORGANIZATION, &xn->oid))
+ do_setenv_nid_value(es, xt, xn, depth, gc);
+ break;
+ case NID_organizationalUnitName:
+ if (OID_CMP(OID_AT_ORG_UNIT, &xn->oid))
+ do_setenv_nid_value(es, xt, xn, depth, gc);
+ break;
+ case NID_pkcs9_emailAddress:
+ if (OID_CMP(OID_PKCS9_EMAIL, &xn->oid))
+ do_setenv_nid_value(es, xt, xn, depth, gc);
+ break;
+ }
+}
+}
+
+void
+x509_track_add (const struct x509_track **ll_head, const char *name, int
msglevel, struct gc_arena *gc)
+{
+ struct x509_track *xt;
+ ALLOC_OBJ_CLEAR_GC (xt, struct x509_track, gc);
+ if (*name == '+')
+{
+ xt->flags |= XT_FULL_CHAIN;
+ ++name;
+}
+ xt->name = name;
+ xt->nid = name_to_nid(name);
+ if (xt->nid != NID_undef)
+{
+ xt->next = *ll_head;
+ *ll_head = xt;
+}
+ else
+msg(msglevel, "x509_track: no such attribute '%s'", name);
+}
+
+void
+x509_setenv_track (const struct x509_track *xt, struct env_set *es, const int
depth, x509_crt *cert)
+{
+ struct gc_arena gc = gc_new();
+ while (xt)
+{
+ if (depth == 0 || (xt->flags & XT_FULL_CHAIN))
+ {
+ switch (xt->nid)
+ {
+ case NID_sha1:
+ {
+ unsigned char *sha1_hash = x509_get_sha1_hash(cert, &gc);
+
Re: [Openvpn-devel] [PATCH 07/10] Implemented x509-track for PolarSSL.
Hi, This addition is welcome and the code does the job it promises to do, but after reviewing the code I would like to propose a different implementation. The reasons for this are gives as inline replies below. The alternative patch proposal is attached. On Thu, Mar 3, 2016 at 9:19 AM, James Yonan wrote: > Signed-off-by: James Yonan > --- > src/openvpn/ssl_verify_polarssl.c | 166 > ++ > src/openvpn/syshead.h | 2 +- > 2 files changed, 167 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/ssl_verify_polarssl.c > b/src/openvpn/ssl_verify_polarssl.c > index 9d0d086..ab693d2 100644 > --- a/src/openvpn/ssl_verify_polarssl.c > +++ b/src/openvpn/ssl_verify_polarssl.c > @@ -198,6 +198,172 @@ x509_get_subject(x509_crt *cert, struct gc_arena *gc) >return subject; > } > > +#ifdef ENABLE_X509_TRACK > + > +/* these match NID's in OpenSSL crypto/objects/objects.h */ > +#define NID_undef 0 > +#define NID_sha164 > +#define NID_commonName 13 > +#define NID_countryName 14 > +#define NID_localityName15 > +#define NID_stateOrProvinceName 16 > +#define NID_organizationName 17 > +#define NID_organizationalUnitName 18 > +#define NID_pkcs9_emailAddress 48 Hmm, I don't really like to maintain lists in the source code. If possible, I would prefer an implementation that fits the polarssl approach, rather than wrapping polarssl with openssl-like code. > diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h > index 7e77b6c..25aa69b 100644 > --- a/src/openvpn/syshead.h > +++ b/src/openvpn/syshead.h > @@ -634,7 +634,7 @@ socket_defined (const socket_descriptor_t sd) > /* > * Enable x509-track feature? > */ > -#if defined(ENABLE_CRYPTO) && defined (ENABLE_CRYPTO_OPENSSL) > +#if defined(ENABLE_CRYPTO) && (defined(ENABLE_CRYPTO_OPENSSL) || > defined(ENABLE_CRYPTO_POLARSSL)) Since both crypto backends now support --x509-track, let's just get rid of this define all together :) Finally, while reviewing I noticed that the --x509-track code in options.c lives inside #ifdef MANAGEMENT, but there seems to be no valid reason for this. Let's move it outside of this #ifdef. The attached patch takes all these remarks into account. The upsides of my alternative are less code, and no lists to maintain. The downside is less error reporting. I'm curious to hear what you think of the alternative implementation. -Steffan From 2363205265ebc77c1dab740b56239ca6e78f5d11 Mon Sep 17 00:00:00 2001 From: Steffan Karger Date: Sat, 5 Mar 2016 17:08:22 +0100 Subject: [PATCH] Implemented x509-track for PolarSSL. This patch is a variant of the patch to implement x509-track for PolarSSL that was sent to openvpn-devel@ by James Yonan (<1456993146-63968-7-git-send-email-ja...@openvpn.net>). It still uses some of the original code from James, but proposes a different implementation. This patch does the following things differently: * Do not introduce NID_* defines that need to be maintained. Instead, just use the short name of the attribute for identification. This has the advantage that we automatically support everything that PolarSSL supports, it is less code and we do not have maintain the list. But the disadvantage is that this approach will not error out when an unknown attribute name is supplied. PolarSSL (at least 1.3, I didn't check 2.x) does not provide the functions required to do that. Instead of erroring out, this implementation will just silently ignore the unknown --x509-track attribute name. * Remove the ENABLE_X509_TRACK define completely - it depended just on ENABLE_CRYPTO anyway. * Move the --x509-track option parsing out of ENABLE_MANAGEMENT, since it does not depend on management functionality. Signed-off-by: Steffan Karger --- doc/openvpn.8 | 1 - src/openvpn/init.c| 2 - src/openvpn/options.c | 14 +++--- src/openvpn/options.h | 2 - src/openvpn/ssl_common.h | 2 - src/openvpn/ssl_verify.c | 16 ++- src/openvpn/ssl_verify.h | 6 --- src/openvpn/ssl_verify_backend.h | 4 -- src/openvpn/ssl_verify_openssl.c | 3 -- src/openvpn/ssl_verify_polarssl.c | 90 +++ src/openvpn/syshead.h | 7 --- 11 files changed, 99 insertions(+), 48 deletions(-) diff --git a/doc/openvpn.8 b/doc/openvpn.8 index decffc7..76b04f6 100644 --- a/doc/openvpn.8 +++ b/doc/openvpn.8 @@ -5112,7 +5112,6 @@ to save values from full cert chain. Values will be encoded as X509__=. Multiple .B \-\-x509\-track options can be defined to track multiple attributes. -Not available with PolarSSL. .\"* .TP .B \-\-ns\-cert\-type client|server diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 84fac07..42baf97 100644 ---
Re: [Openvpn-devel] [PATCH 07/10] Implemented x509-track for PolarSSL.
Am 09.03.16 um 00:10 schrieb Steffan Karger: > Hi, > > This addition is welcome and the code does the job it promises to do, > but after reviewing the code I would like to propose a different > implementation. The reasons for this are gives as inline replies > below. The alternative patch proposal is attached. Code looks good to me, ACK. Arne
