You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by kb...@apache.org on 2015/01/07 13:24:48 UTC

svn commit: r1650047 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_private.h modules/ssl/ssl_util_ssl.c modules/ssl/ssl_util_ssl.h

Author: kbrand
Date: Wed Jan  7 12:24:48 2015
New Revision: 1650047

URL: http://svn.apache.org/r1650047
Log:
Add support for extracting subjectAltName entries of type
rfc822Name and dNSName into SSL_{CLIENT,SERVER}_SAN_{Email,DNS}_n
variables.

* docs/manual/mod/mod_ssl.xml: add SSL_*_SAN_*_n entries to the
  environment variables table

* modules/ssl/ssl_engine_kernel.c: in ssl_hook_Fixup, add extraction
  of subjectAltName entries for the "StdEnvVars" case

* modules/ssl/ssl_engine_vars.c: add support for retrieving the
  SSL_{CLIENT,SERVER}_SAN_{Email,DNS}_n variables, either with
  individual on-demand lookup (ssl_var_lookup_ssl_cert_san),
  or with full-list extraction to the environment ("StdEnvVars")

* modules/ssl/ssl_private.h: add modssl_var_extract_san_entries prototype

* modules/ssl/ssl_util_ssl.c: implement SSL_X509_getSAN and
  SSL_ASN1_STRING_to_utf8 helper functions, with factoring out common
  code from SSL_X509_getIDs and SSL_X509_NAME_ENTRY_to_string where
  suitable. Limit SSL_X509_getSAN to the two most common subjectAltName
  entry types appearing in user or server certificates (i.e., rfc822Name
  and dNSName), for the time being.

* modules/ssl/ssl_util_ssl.h: add SSL_ASN1_STRING_to_utf8
  and SSL_X509_getSAN prototypes

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml
    httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
    httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
    httpd/httpd/trunk/modules/ssl/ssl_private.h
    httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c
    httpd/httpd/trunk/modules/ssl/ssl_util_ssl.h

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1650047&r1=1650046&r2=1650047&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Jan  7 12:24:48 2015
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_ssl: Add support for extracting subjectAltName entries of type
+     rfc822Name and dNSName into SSL_{CLIENT,SERVER}_SAN_{Email,DNS}_n
+     environment variables. Also addresses PR 57207. [Kaspar Brand]
+
   *) mod_proxy: Don't put non balancer-member workers in error state by
      default for connection or 500/503 errors, and honor status=+I for
      any error.  PR 48388.  [Yann Ylavic]

Modified: httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml?rev=1650047&r1=1650046&r2=1650047&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml Wed Jan  7 12:24:48 2015
@@ -75,6 +75,8 @@ compatibility variables.</p>
 <tr><td><code>SSL_CLIENT_M_SERIAL</code></td>           <td>string</td>    <td>The serial of the client certificate</td></tr>
 <tr><td><code>SSL_CLIENT_S_DN</code></td>               <td>string</td>    <td>Subject DN in client's certificate</td></tr>
 <tr><td><code>SSL_CLIENT_S_DN_</code><em>x509</em></td> <td>string</td>    <td>Component of client's Subject DN</td></tr>
+<tr><td><code>SSL_CLIENT_SAN_Email_</code><em>n</em></td> <td>string</td>  <td>Client certificate's subjectAltName extension entries of type rfc822Name</td></tr>
+<tr><td><code>SSL_CLIENT_SAN_DNS_</code><em>n</em></td> <td>string</td>    <td>Client certificate's subjectAltName extension entries of type dNSName</td></tr>
 <tr><td><code>SSL_CLIENT_I_DN</code></td>               <td>string</td>    <td>Issuer DN of client's certificate</td></tr>
 <tr><td><code>SSL_CLIENT_I_DN_</code><em>x509</em></td> <td>string</td>    <td>Component of client's Issuer DN</td></tr>
 <tr><td><code>SSL_CLIENT_V_START</code></td>            <td>string</td>    <td>Validity of client's certificate (start time)</td></tr>
@@ -88,6 +90,8 @@ compatibility variables.</p>
 <tr><td><code>SSL_SERVER_M_VERSION</code></td>          <td>string</td>    <td>The version of the server certificate</td></tr>
 <tr><td><code>SSL_SERVER_M_SERIAL</code></td>           <td>string</td>    <td>The serial of the server certificate</td></tr>
 <tr><td><code>SSL_SERVER_S_DN</code></td>               <td>string</td>    <td>Subject DN in server's certificate</td></tr>
+<tr><td><code>SSL_SERVER_SAN_Email_</code><em>n</em></td> <td>string</td>  <td>Server certificate's subjectAltName extension entries of type rfc822Name</td></tr>
+<tr><td><code>SSL_SERVER_SAN_DNS_</code><em>n</em></td> <td>string</td>    <td>Server certificate's subjectAltName extension entries of type dNSName</td></tr>
 <tr><td><code>SSL_SERVER_S_DN_</code><em>x509</em></td> <td>string</td>    <td>Component of server's Subject DN</td></tr>
 <tr><td><code>SSL_SERVER_I_DN</code></td>               <td>string</td>    <td>Issuer DN of server's certificate</td></tr>
 <tr><td><code>SSL_SERVER_I_DN_</code><em>x509</em></td> <td>string</td>    <td>Component of server's Issuer DN</td></tr>

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=1650047&r1=1650046&r2=1650047&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Wed Jan  7 12:24:48 2015
@@ -1199,6 +1199,7 @@ int ssl_hook_Fixup(request_rec *r)
     /* standard SSL environment variables */
     if (dc->nOptions & SSL_OPT_STDENVVARS) {
         modssl_var_extract_dns(env, sslconn->ssl, r->pool);
+        modssl_var_extract_san_entries(env, sslconn->ssl, r->pool);
 
         for (i = 0; ssl_hook_Fixup_vars[i]; i++) {
             var = (char *)ssl_hook_Fixup_vars[i];

Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c?rev=1650047&r1=1650046&r2=1650047&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c Wed Jan  7 12:24:48 2015
@@ -42,6 +42,7 @@
 static char *ssl_var_lookup_ssl(apr_pool_t *p, conn_rec *c, request_rec *r, char *var);
 static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, request_rec *r, X509 *xs, char *var);
 static char *ssl_var_lookup_ssl_cert_dn(apr_pool_t *p, X509_NAME *xsname, char *var);
+static char *ssl_var_lookup_ssl_cert_san(apr_pool_t *p, X509 *xs, char *var);
 static char *ssl_var_lookup_ssl_cert_valid(apr_pool_t *p, ASN1_TIME *tm);
 static char *ssl_var_lookup_ssl_cert_remain(apr_pool_t *p, ASN1_TIME *tm);
 static char *ssl_var_lookup_ssl_cert_serial(apr_pool_t *p, X509 *xs);
@@ -564,6 +565,10 @@ static char *ssl_var_lookup_ssl_cert(apr
         result = ssl_var_lookup_ssl_cert_dn(p, xsname, var+5);
         resdup = FALSE;
     }
+    else if (strlen(var) > 4 && strcEQn(var, "SAN_", 4)) {
+        result = ssl_var_lookup_ssl_cert_san(p, xs, var+4);
+        resdup = FALSE;
+    }
     else if (strcEQ(var, "A_SIG")) {
         nid = OBJ_obj2nid((ASN1_OBJECT *)(xs->cert_info->signature->algorithm));
         result = apr_pstrdup(p,
@@ -652,6 +657,34 @@ static char *ssl_var_lookup_ssl_cert_dn(
     return result;
 }
 
+static char *ssl_var_lookup_ssl_cert_san(apr_pool_t *p, X509 *xs, char *var)
+{
+    int type, numlen;
+    apr_array_header_t *entries;
+
+    if (strcEQn(var, "Email_", 6)) {
+        type = GEN_EMAIL;
+        var += 6;
+    }
+    else if (strcEQn(var, "DNS_", 4)) {
+        type = GEN_DNS;
+        var += 4;
+    }
+    else
+        return NULL;
+
+    /* sanity check: number must be between 1 and 4 digits */
+    numlen = strspn(var, "0123456789");
+    if ((numlen < 1) || (numlen > 4) || (numlen != strlen(var)))
+        return NULL;
+
+    if (SSL_X509_getSAN(p, xs, type, atoi(var), &entries))
+       /* return the first entry from this 1-element array */
+       return APR_ARRAY_IDX(entries, 0, char *);
+    else
+       return NULL;
+}
+
 static char *ssl_var_lookup_ssl_cert_valid(apr_pool_t *p, ASN1_TIME *tm)
 {
     char *result;
@@ -944,6 +977,47 @@ void modssl_var_extract_dns(apr_table_t
         X509_free(xs);
     }
 }
+
+static void extract_san_array(apr_table_t *t, const char *pfx,
+                              apr_array_header_t *entries, apr_pool_t *p)
+{
+    int i;
+
+    for (i = 0; i < entries->nelts; i++) {
+        const char *key = apr_psprintf(p, "%s_%d", pfx, i);
+        apr_table_setn(t, key, APR_ARRAY_IDX(entries, i, const char *));
+    }
+}
+
+void modssl_var_extract_san_entries(apr_table_t *t, SSL *ssl, apr_pool_t *p)
+{
+    X509 *xs;
+    apr_array_header_t *entries;
+
+    /* subjectAltName entries of the server certificate */
+    xs = SSL_get_certificate(ssl);
+    if (xs) {
+        if (SSL_X509_getSAN(p, xs, GEN_EMAIL, -1, &entries)) {
+            extract_san_array(t, "SSL_SERVER_SAN_Email", entries, p);
+        }
+        if (SSL_X509_getSAN(p, xs, GEN_DNS, -1, &entries)) {
+            extract_san_array(t, "SSL_SERVER_SAN_DNS", entries, p);
+        }
+        /* no need to free xs (refcount does not increase) */
+    }
+
+    /* subjectAltName entries of the client certificate */
+    xs = SSL_get_peer_certificate(ssl);
+    if (xs) {
+        if (SSL_X509_getSAN(p, xs, GEN_EMAIL, -1, &entries)) {
+            extract_san_array(t, "SSL_CLIENT_SAN_Email", entries, p);
+        }
+        if (SSL_X509_getSAN(p, xs, GEN_DNS, -1, &entries)) {
+            extract_san_array(t, "SSL_CLIENT_SAN_DNS", entries, p);
+        }
+        X509_free(xs);
+    }
+}
 
 /* For an extension type which OpenSSL does not recognize, attempt to
  * parse the extension type as a primitive string.  This will fail for

Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=1650047&r1=1650046&r2=1650047&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_private.h Wed Jan  7 12:24:48 2015
@@ -922,6 +922,10 @@ void         ssl_var_log_config_register
  * allocating from 'p': */
 void modssl_var_extract_dns(apr_table_t *t, SSL *ssl, apr_pool_t *p);
 
+/* Extract SSL_*_SAN_* variables (subjectAltName entries) into table 't'
+ * from SSL object 'ssl', allocating from 'p'. */
+void modssl_var_extract_san_entries(apr_table_t *t, SSL *ssl, apr_pool_t *p);
+
 #ifndef OPENSSL_NO_OCSP
 /* Perform OCSP validation of the current cert in the given context.
  * Returns non-zero on success or zero on failure.  On failure, the

Modified: httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c?rev=1650047&r1=1650046&r2=1650047&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c Wed Jan  7 12:24:48 2015
@@ -185,22 +185,32 @@ BOOL SSL_X509_getBC(X509 *cert, int *ca,
     return TRUE;
 }
 
-/* convert a NAME_ENTRY to UTF8 string */
-char *SSL_X509_NAME_ENTRY_to_string(apr_pool_t *p, X509_NAME_ENTRY *xsne)
+/* convert an ASN.1 string to a UTF-8 string (escaping control characters) */
+char *SSL_ASN1_STRING_to_utf8(apr_pool_t *p, ASN1_STRING *asn1str)
 {
     char *result = NULL;
-    BIO* bio;
+    BIO *bio;
     int len;
 
     if ((bio = BIO_new(BIO_s_mem())) == NULL)
         return NULL;
-    ASN1_STRING_print_ex(bio, X509_NAME_ENTRY_get_data(xsne),
-                         ASN1_STRFLGS_ESC_CTRL|ASN1_STRFLGS_UTF8_CONVERT);
+
+    ASN1_STRING_print_ex(bio, asn1str, ASN1_STRFLGS_ESC_CTRL|
+                                       ASN1_STRFLGS_UTF8_CONVERT);
     len = BIO_pending(bio);
-    result = apr_palloc(p, len+1);
-    len = BIO_read(bio, result, len);
-    result[len] = NUL;
+    if (len > 0) {
+        result = apr_palloc(p, len+1);
+        len = BIO_read(bio, result, len);
+        result[len] = NUL;
+    }
     BIO_free(bio);
+    return result;
+}
+
+/* convert a NAME_ENTRY to UTF8 string */
+char *SSL_X509_NAME_ENTRY_to_string(apr_pool_t *p, X509_NAME_ENTRY *xsne)
+{
+    char *result = SSL_ASN1_STRING_to_utf8(p, X509_NAME_ENTRY_get_data(xsne));
     ap_xlate_proto_from_ascii(result, len);
     return result;
 }
@@ -237,51 +247,84 @@ char *SSL_X509_NAME_to_string(apr_pool_t
     return result;
 }
 
-/* return an array of (RFC 6125 coined) DNS-IDs and CN-IDs in a certificate */
-BOOL SSL_X509_getIDs(apr_pool_t *p, X509 *x509, apr_array_header_t **ids)
+/* 
+ * Return an array of subjectAltName entries of type "type". If idx is -1,
+ * return all entries of the given type, otherwise return an array consisting
+ * of the n-th occurrence of that type only. Currently supported types:
+ * GEN_EMAIL (rfc822Name)
+ * GEN_DNS (dNSName)
+ */
+BOOL SSL_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int idx,
+                     apr_array_header_t **entries)
 {
     STACK_OF(GENERAL_NAME) *names;
-    BIO *bio;
-    X509_NAME *subj;
-    char **cpp;
-    int i, n;
 
-    if (!x509 || !(*ids = apr_array_make(p, 0, sizeof(char *)))) {
-        *ids = NULL;
+    if (!x509 || (type < GEN_OTHERNAME) || (type > GEN_RID) || (idx < -1) ||
+        !(*entries = apr_array_make(p, 0, sizeof(char *)))) {
+        *entries = NULL;
         return FALSE;
     }
 
-    /* First, the DNS-IDs (dNSName entries in the subjectAltName extension) */
-    if ((names = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL)) &&
-        (bio = BIO_new(BIO_s_mem()))) {
+    if ((names = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL))) {
+        int i, n = 0;
         GENERAL_NAME *name;
+        const char *utf8str;
 
         for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
             name = sk_GENERAL_NAME_value(names, i);
-            if (name->type == GEN_DNS) {
-                ASN1_STRING_print_ex(bio, name->d.ia5, ASN1_STRFLGS_ESC_CTRL|
-                                     ASN1_STRFLGS_UTF8_CONVERT);
-                n = BIO_pending(bio);
-                if (n > 0) {
-                    cpp = (char **)apr_array_push(*ids);
-                    *cpp = apr_palloc(p, n+1);
-                    n = BIO_read(bio, *cpp, n);
-                    (*cpp)[n] = NUL;
+            if (name->type == type) {
+                if ((idx == -1) || (n == idx)) {
+                    switch (type) {
+                    case GEN_EMAIL:
+                    case GEN_DNS:
+                        utf8str = SSL_ASN1_STRING_to_utf8(p, name->d.ia5);
+                        if (utf8str) {
+                            APR_ARRAY_PUSH(*entries, const char *) = utf8str;
+                        }
+                        break;
+                    default:
+                        /*
+                         * Not implemented right now:
+                         * GEN_OTHERNAME (otherName)
+                         * GEN_X400 (x400Address)
+                         * GEN_DIRNAME (directoryName)
+                         * GEN_EDIPARTY (ediPartyName)
+                         * GEN_URI (uniformResourceIdentifier)
+                         * GEN_IPADD (iPAddress)
+                         * GEN_RID (registeredID)
+                         */
+                        break;
+                    }
                 }
+                if ((idx != -1) && (n++ > idx))
+                   break;
             }
         }
-        BIO_free(bio);
-    }
 
-    if (names)
         sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
+    }
+
+    return apr_is_empty_array(*entries) ? FALSE : TRUE;
+}
+
+/* return an array of (RFC 6125 coined) DNS-IDs and CN-IDs in a certificate */
+BOOL SSL_X509_getIDs(apr_pool_t *p, X509 *x509, apr_array_header_t **ids)
+{
+    X509_NAME *subj;
+    int i = -1;
+
+    /* First, the DNS-IDs (dNSName entries in the subjectAltName extension) */
+    if (!x509 ||
+        (SSL_X509_getSAN(p, x509, GEN_DNS, -1, ids) == FALSE && !*ids)) {
+        *ids = NULL;
+        return FALSE;
+    }
 
     /* Second, the CN-IDs (commonName attributes in the subject DN) */
     subj = X509_get_subject_name(x509);
-    i = -1;
     while ((i = X509_NAME_get_index_by_NID(subj, NID_commonName, i)) != -1) {
-        cpp = (char **)apr_array_push(*ids);
-        *cpp = SSL_X509_NAME_ENTRY_to_string(p, X509_NAME_get_entry(subj, i));
+        APR_ARRAY_PUSH(*ids, const char *) = 
+            SSL_X509_NAME_ENTRY_to_string(p, X509_NAME_get_entry(subj, i));
     }
 
     return apr_is_empty_array(*ids) ? FALSE : TRUE;

Modified: httpd/httpd/trunk/modules/ssl/ssl_util_ssl.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_util_ssl.h?rev=1650047&r1=1650046&r2=1650047&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_util_ssl.h (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_util_ssl.h Wed Jan  7 12:24:48 2015
@@ -63,8 +63,10 @@ void        SSL_set_app_data2(SSL *, voi
 EVP_PKEY   *SSL_read_PrivateKey(const char *, EVP_PKEY **, pem_password_cb *, void *);
 int         SSL_smart_shutdown(SSL *ssl);
 BOOL        SSL_X509_getBC(X509 *, int *, int *);
+char       *SSL_ASN1_STRING_to_utf8(apr_pool_t *, ASN1_STRING *);
 char       *SSL_X509_NAME_ENTRY_to_string(apr_pool_t *p, X509_NAME_ENTRY *xsne);
 char       *SSL_X509_NAME_to_string(apr_pool_t *, X509_NAME *, int);
+BOOL        SSL_X509_getSAN(apr_pool_t *, X509 *, int, int, apr_array_header_t **);
 BOOL        SSL_X509_getIDs(apr_pool_t *, X509 *, apr_array_header_t **);
 BOOL        SSL_X509_match_name(apr_pool_t *, X509 *, const char *, BOOL, server_rec *);
 BOOL        SSL_X509_INFO_load_file(apr_pool_t *, STACK_OF(X509_INFO) *, const char *);



Re: AW: svn commit: r1650047 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_private.h modules/ssl/ssl_util_ssl.c modules/ssl/ssl_util_ssl.h

Posted by Kaspar Brand <ht...@velox.ch>.
On 07.01.2015 15:17, Plüm, Rüdiger, Vodafone Group wrote:
>>> Why checking for FALSE and !*ids? Shouldn't the empty array cause a
>> return of FALSE?
>>
>> Not necessarily. Early returns in SSL_X509_getSAN (when argument
>> checking etc. is taking place) may return a NULL pointer for the array,
> 
> But don't they always return FALSE in this case as well? If yes a check for FALSE should be sufficient,
> or if we only want to ensure that the array is available for !*ids. My point is more: Why do we need to do both checks. Wouldn't be one sufficient either?

Ah, my first answer was missing an essential part, I think. Only
checking for

 SSL_X509_getSAN(p, x509, GEN_DNS, -1, ids) == FALSE

would mean that we would stop when we get back an empty array from
SSL_X509_getSAN. For SSL_X509_getIDs, however, we want to continue and
push the CN-IDs to the array (i.e., in the case of a certificate without
a subjectAltName extension, which was relatively common until a few
years ago). That's the reason for the additional "&& !*ids" - to make
sure that we continue when getting back an empty array from SSL_X509_getSAN.

Kaspar

AW: svn commit: r1650047 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_private.h modules/ssl/ssl_util_ssl.c modules/ssl/ssl_util_ssl.h

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Ursprüngliche Nachricht-----
> Von: Kaspar Brand [mailto:httpd-dev.2014@velox.ch]
> Gesendet: Mittwoch, 7. Januar 2015 15:01
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1650047 - in /httpd/httpd/trunk: CHANGES
> docs/manual/mod/mod_ssl.xml modules/ssl/ssl_engine_kernel.c
> modules/ssl/ssl_engine_vars.c modules/ssl/ssl_private.h
> modules/ssl/ssl_util_ssl.c modules/ssl/ssl_util_ssl.h
> 
> On 07.01.2015 14:03, Ruediger Pluem wrote:
> >> +/* return an array of (RFC 6125 coined) DNS-IDs and CN-IDs in a
> certificate */
> >> +BOOL SSL_X509_getIDs(apr_pool_t *p, X509 *x509, apr_array_header_t
> **ids)
> >> +{
> >> +    X509_NAME *subj;
> >> +    int i = -1;
> >> +
> >> +    /* First, the DNS-IDs (dNSName entries in the subjectAltName
> extension) */
> >> +    if (!x509 ||
> >> +        (SSL_X509_getSAN(p, x509, GEN_DNS, -1, ids) == FALSE &&
> !*ids)) {
> >> +        *ids = NULL;
> >
> > Why checking for FALSE and !*ids? Shouldn't the empty array cause a
> return of FALSE?
> 
> Not necessarily. Early returns in SSL_X509_getSAN (when argument
> checking etc. is taking place) may return a NULL pointer for the array,

But don't they always return FALSE in this case as well? If yes a check for FALSE should be sufficient,
or if we only want to ensure that the array is available for !*ids. My point is more: Why do we need to do both checks. Wouldn't be one sufficient either?

Regards

Rüdiger

Re: svn commit: r1650047 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_private.h modules/ssl/ssl_util_ssl.c modules/ssl/ssl_util_ssl.h

Posted by Kaspar Brand <ht...@velox.ch>.
On 07.01.2015 14:03, Ruediger Pluem wrote:
>> +/* return an array of (RFC 6125 coined) DNS-IDs and CN-IDs in a certificate */
>> +BOOL SSL_X509_getIDs(apr_pool_t *p, X509 *x509, apr_array_header_t **ids)
>> +{
>> +    X509_NAME *subj;
>> +    int i = -1;
>> +
>> +    /* First, the DNS-IDs (dNSName entries in the subjectAltName extension) */
>> +    if (!x509 ||
>> +        (SSL_X509_getSAN(p, x509, GEN_DNS, -1, ids) == FALSE && !*ids)) {
>> +        *ids = NULL;
> 
> Why checking for FALSE and !*ids? Shouldn't the empty array cause a return of FALSE?

Not necessarily. Early returns in SSL_X509_getSAN (when argument
checking etc. is taking place) may return a NULL pointer for the array,
and since we want to add the CN-ID elements further down here in
SSL_X509_getIDs, we have to make sure that we really have an array to
push to.

Kaspar

Re: svn commit: r1650047 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_ssl.xml modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_private.h modules/ssl/ssl_util_ssl.c modules/ssl/ssl_util_ssl.h

Posted by Ruediger Pluem <rp...@apache.org>.

On 01/07/2015 01:24 PM, kbrand@apache.org wrote:
> Author: kbrand
> Date: Wed Jan  7 12:24:48 2015
> New Revision: 1650047
> 
> URL: http://svn.apache.org/r1650047
> Log:
> Add support for extracting subjectAltName entries of type
> rfc822Name and dNSName into SSL_{CLIENT,SERVER}_SAN_{Email,DNS}_n
> variables.
> 
> * docs/manual/mod/mod_ssl.xml: add SSL_*_SAN_*_n entries to the
>   environment variables table
> 
> * modules/ssl/ssl_engine_kernel.c: in ssl_hook_Fixup, add extraction
>   of subjectAltName entries for the "StdEnvVars" case
> 
> * modules/ssl/ssl_engine_vars.c: add support for retrieving the
>   SSL_{CLIENT,SERVER}_SAN_{Email,DNS}_n variables, either with
>   individual on-demand lookup (ssl_var_lookup_ssl_cert_san),
>   or with full-list extraction to the environment ("StdEnvVars")
> 
> * modules/ssl/ssl_private.h: add modssl_var_extract_san_entries prototype
> 
> * modules/ssl/ssl_util_ssl.c: implement SSL_X509_getSAN and
>   SSL_ASN1_STRING_to_utf8 helper functions, with factoring out common
>   code from SSL_X509_getIDs and SSL_X509_NAME_ENTRY_to_string where
>   suitable. Limit SSL_X509_getSAN to the two most common subjectAltName
>   entry types appearing in user or server certificates (i.e., rfc822Name
>   and dNSName), for the time being.
> 
> * modules/ssl/ssl_util_ssl.h: add SSL_ASN1_STRING_to_utf8
>   and SSL_X509_getSAN prototypes
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/docs/manual/mod/mod_ssl.xml
>     httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
>     httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
>     httpd/httpd/trunk/modules/ssl/ssl_private.h
>     httpd/httpd/trunk/modules/ssl/ssl_util_ssl.c
>     httpd/httpd/trunk/modules/ssl/ssl_util_ssl.h
> 

> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c?rev=1650047&r1=1650046&r2=1650047&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_vars.c Wed Jan  7 12:24:48 2015

> @@ -237,51 +247,84 @@ char *SSL_X509_NAME_to_string(apr_pool_t
>      return result;
>  }
>  
> -/* return an array of (RFC 6125 coined) DNS-IDs and CN-IDs in a certificate */
> -BOOL SSL_X509_getIDs(apr_pool_t *p, X509 *x509, apr_array_header_t **ids)
> +/* 
> + * Return an array of subjectAltName entries of type "type". If idx is -1,
> + * return all entries of the given type, otherwise return an array consisting
> + * of the n-th occurrence of that type only. Currently supported types:
> + * GEN_EMAIL (rfc822Name)
> + * GEN_DNS (dNSName)
> + */
> +BOOL SSL_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int idx,
> +                     apr_array_header_t **entries)
>  {
>      STACK_OF(GENERAL_NAME) *names;
> -    BIO *bio;
> -    X509_NAME *subj;
> -    char **cpp;
> -    int i, n;
>  
> -    if (!x509 || !(*ids = apr_array_make(p, 0, sizeof(char *)))) {
> -        *ids = NULL;
> +    if (!x509 || (type < GEN_OTHERNAME) || (type > GEN_RID) || (idx < -1) ||
> +        !(*entries = apr_array_make(p, 0, sizeof(char *)))) {
> +        *entries = NULL;
>          return FALSE;
>      }
>  
> -    /* First, the DNS-IDs (dNSName entries in the subjectAltName extension) */
> -    if ((names = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL)) &&
> -        (bio = BIO_new(BIO_s_mem()))) {
> +    if ((names = X509_get_ext_d2i(x509, NID_subject_alt_name, NULL, NULL))) {
> +        int i, n = 0;
>          GENERAL_NAME *name;
> +        const char *utf8str;
>  
>          for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
>              name = sk_GENERAL_NAME_value(names, i);
> -            if (name->type == GEN_DNS) {
> -                ASN1_STRING_print_ex(bio, name->d.ia5, ASN1_STRFLGS_ESC_CTRL|
> -                                     ASN1_STRFLGS_UTF8_CONVERT);
> -                n = BIO_pending(bio);
> -                if (n > 0) {
> -                    cpp = (char **)apr_array_push(*ids);
> -                    *cpp = apr_palloc(p, n+1);
> -                    n = BIO_read(bio, *cpp, n);
> -                    (*cpp)[n] = NUL;
> +            if (name->type == type) {
> +                if ((idx == -1) || (n == idx)) {
> +                    switch (type) {
> +                    case GEN_EMAIL:
> +                    case GEN_DNS:
> +                        utf8str = SSL_ASN1_STRING_to_utf8(p, name->d.ia5);
> +                        if (utf8str) {
> +                            APR_ARRAY_PUSH(*entries, const char *) = utf8str;
> +                        }
> +                        break;
> +                    default:
> +                        /*
> +                         * Not implemented right now:
> +                         * GEN_OTHERNAME (otherName)
> +                         * GEN_X400 (x400Address)
> +                         * GEN_DIRNAME (directoryName)
> +                         * GEN_EDIPARTY (ediPartyName)
> +                         * GEN_URI (uniformResourceIdentifier)
> +                         * GEN_IPADD (iPAddress)
> +                         * GEN_RID (registeredID)
> +                         */
> +                        break;
> +                    }
>                  }
> +                if ((idx != -1) && (n++ > idx))
> +                   break;
>              }
>          }
> -        BIO_free(bio);
> -    }
>  
> -    if (names)
>          sk_GENERAL_NAME_pop_free(names, GENERAL_NAME_free);
> +    }
> +
> +    return apr_is_empty_array(*entries) ? FALSE : TRUE;
> +}
> +
> +/* return an array of (RFC 6125 coined) DNS-IDs and CN-IDs in a certificate */
> +BOOL SSL_X509_getIDs(apr_pool_t *p, X509 *x509, apr_array_header_t **ids)
> +{
> +    X509_NAME *subj;
> +    int i = -1;
> +
> +    /* First, the DNS-IDs (dNSName entries in the subjectAltName extension) */
> +    if (!x509 ||
> +        (SSL_X509_getSAN(p, x509, GEN_DNS, -1, ids) == FALSE && !*ids)) {
> +        *ids = NULL;

Why checking for FALSE and !*ids? Shouldn't the empty array cause a return of FALSE?

> +        return FALSE;
> +    }
>  
>      /* Second, the CN-IDs (commonName attributes in the subject DN) */
>      subj = X509_get_subject_name(x509);
> -    i = -1;
>      while ((i = X509_NAME_get_index_by_NID(subj, NID_commonName, i)) != -1) {
> -        cpp = (char **)apr_array_push(*ids);
> -        *cpp = SSL_X509_NAME_ENTRY_to_string(p, X509_NAME_get_entry(subj, i));
> +        APR_ARRAY_PUSH(*ids, const char *) = 
> +            SSL_X509_NAME_ENTRY_to_string(p, X509_NAME_get_entry(subj, i));
>      }
>  
>      return apr_is_empty_array(*ids) ? FALSE : TRUE;
> 

Regards

Rüdiger