Following the comments from Joe, here is a revised patch that should
work better :-) I've tried to add a sensible comment about why we have
both functions listed.
It removes the nastiness of the len pointer and also converts the
extlist fucntion to simply call into ssl_ext_lookup.
I've changed the log level down to INFO.
david
Index: modules/ssl/ssl_private.h
===================================================================
--- modules/ssl/ssl_private.h (revision 279892)
+++ modules/ssl/ssl_private.h (working copy)
@@ -646,7 +646,7 @@
/** 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);
+const char *ssl_ext_lookup(apr_pool_t *p, conn_rec *c, int peer, const
char *extension, int index);
extern apr_array_header_t *ssl_extlist_by_oid(request_rec *r, const
char *oidstr);
Index: modules/ssl/ssl_expr_eval.c
===================================================================
--- modules/ssl/ssl_expr_eval.c (revision 279892)
+++ modules/ssl/ssl_expr_eval.c (working copy)
@@ -200,55 +200,24 @@
#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)
+apr_array_header_t *ssl_extlist_by_oid(request_rec *r, const char *extname)
{
- int count = 0, j;
- X509 *xs = NULL;
- ASN1_OBJECT *oid;
+ int count = 0, j, n = 0;
apr_array_header_t *val_array;
- SSLConnRec *sslconn = myConnConfig(r->connection);
+ const char *val = NULL;
/* trivia */
- if (oidstr == NULL || sslconn == NULL || sslconn->ssl == NULL)
+ if (extname == 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);
- }
+ while ((val = ssl_ext_lookup(r->pool, r->connection, 1,
+ extname, n++))) {
+ char **newentry =apr_array_push(val_array);
+ *newentry = (char *)val;
}
- X509_free(xs);
- ERR_clear_error();
-
if (val_array->nelts == 0)
return NULL;
else
Index: modules/ssl/ssl_engine_vars.c
===================================================================
--- modules/ssl/ssl_engine_vars.c (revision 279892)
+++ modules/ssl/ssl_engine_vars.c (working copy)
@@ -661,7 +661,7 @@
}
const char *ssl_ext_lookup(apr_pool_t *p, conn_rec *c, int peer,
- const char *oidnum)
+ const char *extension, int index)
{
SSLConnRec *sslconn = myConnConfig(c);
SSL *ssl;
@@ -669,14 +669,21 @@
ASN1_OBJECT *oid;
int count = 0, j;
char *result = NULL;
-
+
if (!sslconn || !sslconn->ssl) {
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_INFO, 0, c,
+ "Failed to create an object for extension '%s'",
+ extension);
ERR_clear_error();
return NULL;
}
@@ -692,13 +699,31 @@
X509_EXTENSION *ext = X509_get_ext(xs, j);
if (OBJ_cmp(ext->object, oid) == 0) {
- BIO *bio = BIO_new(BIO_s_mem());
+ BIO *bio = NULL;
- if (X509V3_EXT_print(bio, ext, 0, 0) == 1) {
+ if (index != -1 && --index > 0)
+ continue;
+
+ bio = BIO_new(BIO_s_mem());
+
+ /* As Joe keeps pointing out, the object of this function
+ * is to return a string. Sadly the function that openssl
+ * provides (X509V3_EXT_print) isn't up to much and often fails
+ * to provide a string. Thankfully what we want is contained
+ * in ext->value, which is an ASN1_OCTET_STRING. The function
+ * ASN1_STRING_print is able to handle these, so we fallback to
+ * this fucntion.
+ */
+ if (X509V3_EXT_print(bio, ext, 0, 0) == 1 ||
+ ASN1_STRING_print(bio, ext->value) == 1) {
BUF_MEM *buf;
BIO_get_mem_ptr(bio, &buf);
result = apr_pstrmemdup(p, buf->data, buf->length);
+ } else {
+ ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, c,
+ "Found an extension '%s', but failed to "
+ "create a string from it", extension);
}
BIO_vfree(bio);
Index: modules/ssl/mod_ssl.h
===================================================================
--- modules/ssl/mod_ssl.h (revision 279892)
+++ modules/ssl/mod_ssl.h (working copy)
@@ -37,14 +37,20 @@
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. */
+ * certificate X.509 extension.
+ * 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.
+ * The index parameter allows for multiple values to be retrieved by
+ * repeated calls with the index incremented. Using an index of 0 will
+ * provide the first matching result.
+ * The string value of the extension is returned, or NULL on error.
+ */
APR_DECLARE_OPTIONAL_FN(const char *, ssl_ext_lookup,
(apr_pool_t *p, conn_rec *c, int peer,
- const char *oidnum));
+ const char *extension, int index));
/** An optional function which returns non-zero if the given connection
* is using SSL/TLS. */
