- remove ssl_ext_lookup and replace it with ssl_ext_list

- change ssl_expr_eval_oid to use ssl_ext_list

This change provides for a singfle function that provides an array of all
values from a certificate that match a given extension and removes the
duplictaed code that was present.

Reviewed by: Joe Orton

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@289444 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c
index 5829947bd45943df4b13630d8dc228fe9bda0e73..01a851c66dd6a56f165d6a393067f9a2b67bfceb 100644 (file)
--- a/modules/ssl/mod_ssl.c
+++ b/modules/ssl/mod_ssl.c
@@ -504,8 +504,6 @@ static void ssl_register_hooks(apr_pool_t *p)
 
     APR_REGISTER_OPTIONAL_FN(ssl_proxy_enable);
     APR_REGISTER_OPTIONAL_FN(ssl_engine_disable);
-
-    APR_REGISTER_OPTIONAL_FN(ssl_extlist_by_oid);
 }
 
 module AP_MODULE_DECLARE_DATA ssl_module = {

diff --git a/modules/ssl/mod_ssl.h b/modules/ssl/mod_ssl.h
index fa3253c53c679cc00ae8595c89a4180c0224a7c2..5dff0c5d5a35f2816b7948ddfb1aac9b22edf1f5 100644 (file)
--- a/modules/ssl/mod_ssl.h
+++ b/modules/ssl/mod_ssl.h
@@ -36,15 +36,20 @@ APR_DECLARE_OPTIONAL_FN(char *, ssl_var_lookup,
                          conn_rec *, request_rec *,
                          char *));
 
-/** The ssl_ext_lookup() optional function retrieves the value of a SSL
- * certificate X.509 extension.  The client certificate is used if
- * peer is non-zero; the server certificate is used otherwise.  The
- * oidnum parameter specifies the numeric OID (e.g. "1.2.3.4") of the
- * desired extension.  The string value of the extension is returned,
- * or NULL on error. */
-APR_DECLARE_OPTIONAL_FN(const char *, ssl_ext_lookup,
+/** The ssl_ext_list() optional function attempts to build an array
+ * of all the values contained in the named X.509 extension. The 
+ * returned array will be created in the supplied pool.
+ * The client certificate is used if peer is non-zero; the server 
+ * certificate is used otherwise.
+ * Extension specifies the extensions to use as a string. This can be
+ * one of the "known" long or short names, or a numeric OID,
+ * e.g. "1.2.3.4", 'nsComment' and 'DN' are all valid.
+ * A pointer to an apr_array_header_t structure is returned if at
+ * least one matching extension is found, NULL otherwise.
+ */
+APR_DECLARE_OPTIONAL_FN(apr_array_header_t *, ssl_ext_list,
                         (apr_pool_t *p, conn_rec *c, int peer,
-                         const char *oidnum));
+                         const char *extension));
 
 /** An optional function which returns non-zero if the given connection
  * is using SSL/TLS. */
@@ -58,7 +63,5 @@ APR_DECLARE_OPTIONAL_FN(int, ssl_proxy_enable, (conn_rec *));
 
 APR_DECLARE_OPTIONAL_FN(int, ssl_engine_disable, (conn_rec *));
 
-APR_DECLARE_OPTIONAL_FN(apr_array_header_t *, ssl_extlist_by_oid, (request_rec *r, const char *oidstr));
-
 #endif /* __MOD_SSL_H__ */
 /** @} */

diff --git a/modules/ssl/ssl_engine_vars.c b/modules/ssl/ssl_engine_vars.c
index ca505f87bbb82fc72ce68b46b844f9292d449398..35dbc55c64bda7eca45e8ebb18e46e15e43bbbbf 100644 (file)
--- a/modules/ssl/ssl_engine_vars.c
+++ b/modules/ssl/ssl_engine_vars.c
@@ -62,7 +62,7 @@ void ssl_var_register(void)
 {
     APR_REGISTER_OPTIONAL_FN(ssl_is_https);
     APR_REGISTER_OPTIONAL_FN(ssl_var_lookup);
-    APR_REGISTER_OPTIONAL_FN(ssl_ext_lookup);
+    APR_REGISTER_OPTIONAL_FN(ssl_ext_list);
     return;
 }
 
@@ -660,23 +660,30 @@ static char *ssl_var_lookup_ssl_version(apr_pool_t *p, char *var)
     return result;
 }
 
-const char *ssl_ext_lookup(apr_pool_t *p, conn_rec *c, int peer,
-                           const char *oidnum)
+apr_array_header_t *ssl_ext_list(apr_pool_t *p, conn_rec *c, int peer,
+                                 const char *extension)
 {
     SSLConnRec *sslconn = myConnConfig(c);
-    SSL *ssl;
+    SSL *ssl = NULL;
+    apr_array_header_t *array = NULL;
     X509 *xs = NULL;
-    ASN1_OBJECT *oid;
+    ASN1_OBJECT *oid = NULL;
     int count = 0, j;
-    char *result = NULL;
-    
-    if (!sslconn || !sslconn->ssl) {
+
+    if (!sslconn || !sslconn->ssl || !extension) {
         return NULL;
     }
     ssl = sslconn->ssl;
 
-    oid = OBJ_txt2obj(oidnum, 1);
+    /* We accept the "extension" string to be converted as 
+     * a long name (nsComment), short name (DN) or 
+     * numeric OID (1.2.3.4).
+     */
+    oid = OBJ_txt2obj(extension, 0);
     if (!oid) {
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, 
+                      "Failed to create an object for extension '%s'",
+                      extension);
         ERR_clear_error();
         return NULL;
     }
@@ -685,34 +692,50 @@ const char *ssl_ext_lookup(apr_pool_t *p, conn_rec *c, int peer,
     if (xs == NULL) {
         return NULL;
     }
-    
-    count = X509_get_ext_count(xs);
 
+    count = X509_get_ext_count(xs);
+    /* Create an array large enough to accomodate every extension. This is
+     * likely overkill, but safe.
+     */
+    array = apr_array_make(p, count, sizeof(char *));
     for (j = 0; j < count; j++) {
         X509_EXTENSION *ext = X509_get_ext(xs, j);
 
         if (OBJ_cmp(ext->object, oid) == 0) {
             BIO *bio = BIO_new(BIO_s_mem());
 
-            if (X509V3_EXT_print(bio, ext, 0, 0) == 1) {
+            /* We want to obtain a string representation of the extensions
+             * value and add it to the array we're building.
+             * X509V3_EXT_print() doesn't know about all the possible 
+             * data types, but the value is stored as an ASN1_OCTET_STRING
+             * allowing us a fallback in case of X509V3_EXT_print 
+             * not knowing how to handle the data.
+             */
+            if (X509V3_EXT_print(bio, ext, 0, 0) == 1 ||
+                ASN1_STRING_print(bio, ext->value) == 1) {
                 BUF_MEM *buf;
-
+                char **ptr = apr_array_push(array);
                 BIO_get_mem_ptr(bio, &buf);
-                result = apr_pstrmemdup(p, buf->data, buf->length);
+                *ptr = apr_pstrmemdup(p, buf->data, buf->length);
+            } else {
+                ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
+                              "Found an extension '%s', but failed to "
+                              "create a string from it", extension); 
             }
-
             BIO_vfree(bio);
-            break;
         }
     }
 
+    if (array->nelts == 0)
+        array = NULL;
+
     if (peer) {
         /* only SSL_get_peer_certificate raises the refcount */
         X509_free(xs);
     }
 
     ERR_clear_error();
-    return result;
+    return array;
 }
 
 static char *ssl_var_lookup_ssl_compress_meth(SSL *ssl)

diff --git a/modules/ssl/ssl_expr_eval.c b/modules/ssl/ssl_expr_eval.c
index e40df2dccbb7fd434ff69c0fd393b572bad57be3..89aaff8a9fe2e43d4ec601b20c9db3415fd06e0c 100644 (file)
--- a/modules/ssl/ssl_expr_eval.c
+++ b/modules/ssl/ssl_expr_eval.c
@@ -198,63 +198,6 @@ static char *ssl_expr_eval_word(request_rec *r, ssl_expr *node)
     }
 }
 
-#define NUM_OID_ELTS 8 /* start with 8 oid slots, resize when needed */
-
-apr_array_header_t *ssl_extlist_by_oid(request_rec *r, const char *oidstr)
-{
-    int count = 0, j;
-    X509 *xs = NULL;
-    ASN1_OBJECT *oid;
-    apr_array_header_t *val_array;
-    SSLConnRec *sslconn = myConnConfig(r->connection);
-
-    /* trivia */
-    if (oidstr == NULL || sslconn == NULL || sslconn->ssl == NULL)
-        return NULL;
-
-    /* Determine the oid we are looking for */
-    if ((oid = OBJ_txt2obj(oidstr, 1)) == NULL) {
-        ERR_clear_error();
-        return NULL;
-    }
-
-    /* are there any extensions in the cert? */
-    if ((xs = SSL_get_peer_certificate(sslconn->ssl)) == NULL ||
-        (count = X509_get_ext_count(xs)) == 0) {
-        return NULL;
-    }
-
-    val_array = apr_array_make(r->pool, NUM_OID_ELTS, sizeof(char *));
-
-    /* Loop over all extensions, extract the desired oids */
-    for (j = 0; j < count; j++) {
-        X509_EXTENSION *ext = X509_get_ext(xs, j);
-
-        if (OBJ_cmp(ext->object, oid) == 0) {
-            BIO *bio = BIO_new(BIO_s_mem());
-
-            if (X509V3_EXT_print(bio, ext, 0, 0) == 1) {
-                BUF_MEM *buf;
-                char **new = apr_array_push(val_array);
-
-                BIO_get_mem_ptr(bio, &buf);
-
-                *new = apr_pstrdup(r->pool, buf->data);
-            }
-
-            BIO_vfree(bio);
-        }
-    }
-
-    X509_free(xs);
-    ERR_clear_error();
-
-    if (val_array->nelts == 0)
-        return NULL;
-    else
-        return val_array;
-}
-
 static BOOL ssl_expr_eval_oid(request_rec *r, const char *word, const char *oidstr)
 {
     int j;
@@ -262,7 +205,7 @@ static BOOL ssl_expr_eval_oid(request_rec *r, const char *word, const char *oids
     apr_array_header_t *oid_array;
     char **oid_value;
 
-    if (NULL == (oid_array = ssl_extlist_by_oid(r, oidstr))) {
+    if (NULL == (oid_array = ssl_ext_list(r->pool, r->connection, 1, oidstr))) {
         return FALSE;
     }
 

diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h
index c48832dd75781604bf3d515fb88bd98093c2cf30..88510bc2acb9b0a3145996213bf339b3e150fc45 100644 (file)
--- a/modules/ssl/ssl_private.h
+++ b/modules/ssl/ssl_private.h
@@ -646,9 +646,7 @@ void         ssl_log_ssl_error(const char *, int, int, server_rec *);
 /**  Variables  */
 void         ssl_var_register(void);
 char        *ssl_var_lookup(apr_pool_t *, server_rec *, conn_rec *, request_rec *, char *);
-const char  *ssl_ext_lookup(apr_pool_t *p, conn_rec *c, int peer, const char *oid);
-
-extern apr_array_header_t *ssl_extlist_by_oid(request_rec *r, const char *oidstr);
+apr_array_header_t *ssl_ext_list(apr_pool_t *p, conn_rec *c, int peer, const char *extension);
 
 void         ssl_var_log_config_register(apr_pool_t *p);