Joe Orton wrote:
> On Mon, Sep 12, 2005 at 04:02:02PM +0100, David Reid wrote:
>
>>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.
>
>
> "OpenSSL... isn't up to much" isn't really very helpful (or sensible).
> If the problem is that X509_ext_print will only handle particular types
> of extension and that you can fall back on ASN1_print for extensions
> which are simple e.g. string types then say that and cut out the waffle.
OK. Guess spending too long figuring out what did didn't work led me to
a dark place :-)
>>It removes the nastiness of the len pointer and also converts the
>>extlist fucntion to simply call into ssl_ext_lookup.
>
>
> That's pretty nasty, going through all the setup overhead and iterating
> through the extension list again for each call. But you miss my point:
> the overlap in functionality is a bad thing not an missed opportunity
> for refactoring.
OK, then what about the below.
>
>>I've changed the log level down to INFO.
>
>
> DEBUG is the maximum acceptable IMO.
OK. OK. OK. I give in. DEBUG.
david
Index: modules/ssl/ssl_private.h
===================================================================
--- modules/ssl/ssl_private.h (revision 279892)
+++ modules/ssl/ssl_private.h (working copy)
@@ -646,10 +646,8 @@
/** 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);
+apr_array_header_t *ssl_ext_list(apr_pool_t *p, conn_rec *c, int peer,
const char *extension);
-extern apr_array_header_t *ssl_extlist_by_oid(request_rec *r, const
char *oidstr);
-
void ssl_var_log_config_register(apr_pool_t *p);
#define APR_SHM_MAXSIZE (64 * 1024 * 1024)
Index: modules/ssl/ssl_expr_eval.c
===================================================================
--- modules/ssl/ssl_expr_eval.c (revision 279892)
+++ modules/ssl/ssl_expr_eval.c (working copy)
@@ -198,63 +198,6 @@
}
}
-#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 @@
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;
}
Index: modules/ssl/ssl_engine_vars.c
===================================================================
--- modules/ssl/ssl_engine_vars.c (revision 279892)
+++ modules/ssl/ssl_engine_vars.c (working copy)
@@ -62,7 +62,7 @@
{
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 @@
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,25 +692,42 @@
if (xs == NULL) {
return NULL;
}
-
+
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 *));
+ if (array) {
+ for (j = 0; j < count; j++) {
+ X509_EXTENSION *ext = X509_get_ext(xs, j);
- 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 (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;
-
- BIO_get_mem_ptr(bio, &buf);
- result = apr_pstrmemdup(p, buf->data, buf->length);
+ /* 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);
+ *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);
}
-
- BIO_vfree(bio);
- break;
}
+ if (array->nelts == 0)
+ array = NULL;
}
if (peer) {
@@ -712,7 +736,7 @@
}
ERR_clear_error();
- return result;
+ return array;
}
static char *ssl_var_lookup_ssl_compress_meth(SSL *ssl)
Index: modules/ssl/mod_ssl.c
===================================================================
--- modules/ssl/mod_ssl.c (revision 279892)
+++ modules/ssl/mod_ssl.c (working copy)
@@ -504,8 +504,6 @@
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 = {
Index: modules/ssl/mod_ssl.h
===================================================================
--- modules/ssl/mod_ssl.h (revision 279892)
+++ modules/ssl/mod_ssl.h (working copy)
@@ -36,15 +36,20 @@
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_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__ */
/** @} */
