On Monday 29 November 2010, Dr Stephen Henson wrote:
> On 24/11/2010 07:07, Kaspar Brand wrote:
> > On 20.11.2010 20:24, Stefan Fritsch wrote:
> >> On Fri, 19 Nov 2010, Joe Orton wrote:
> >>> We could support this better by having a new set of exports:
> >>> SSL_{CLIENT,SERVER}_{I,S}_UTF8DN_*(_n)?
> >>>
> >>> (or something similarly named)
> >>>
> >>> which works the same as _DN_ but exports the attributes as a
> >>> UTF-8 byte seequence regardless of the underlying ASN.1 type;
> >>> this would be a relatively simple hack.
> >>
> >> Or have a (per vhost) directive that enables conversion for all
> >> SSL_*_S_DN_* and SSL_*_S_DN to UTF8. IMHO, this could even be
> >> enabled by default in 2.4.
> >
> > I prefer the latter approach, yes (there's already an awful lot
> > of SSL_* something variables).
> >
> > Given the fact that mod_ssl's current behavior with non-ASCII
> > characters (i.e., outside the 0-127 range) is mostly undefined
> > and/or sometimes even erroneous (a BMPString in the subject or
> > issuer DN will end up as an empty SSL_*_*_DN_* variable, due to
> > the initial null byte), I would suggest the following solution:
> >
> > - for all SSL_{CLIENT,SERVER}_{I,S}_DN_* variables, use UTF-8 by
> > default
> >
> > (i.e., adapt ssl_engine_vars.c:ssl_var_lookup_ssl_cert_dn() to
> > convert TeletexString, UniversalString and BMPString types to
> > UTF8String)
> >
> > - for SSL_{CLIENT,SERVER}_{I,S}_DN, don't use X509_NAME_oneline()
> >
> > any more and switch to X509_NAME_print_ex() instead. What flags
> > should be used is probably debatable - I would recommend to go
> > with XN_FLAG_RFC2253 (note that using XN_FLAG_ONELINE with
> > X509_NAME_print_ex doesn't produce the same string as
> > X509_NAME_oneline, so this will break backward compatibility
> > in any case)
> >
> > - add another parameter to the SSLOptions directive which allows
> >
> > re-enabling the "old" string rendering for
> > SSL_{CLIENT,SERVER}_{I,S}_DN (so the new default would be to
> > rely on X509_NAME_print_ex - even for 2.2 -, but people could
> > restore the current behavior through this option)
>
> It's a very good idea to avoid X509_NAME_oneline() wherever
> possible as it is highly broken, can produce ambiguous output (of
> the Bobby Tables variety) and at other times be just plain wrong
> (BMPStrings is one of many examples).
>
> We have to retain it in OpenSSL for backwards compatibility though.
> I'd throw it out tomorrow if I could get away with it.
>
> You can get a UTF8String from most string types using
> ASN1_STRING_to_UTF8(). This should be adequate for most purposes:
> it doesn't handle the more bizarre TeletexString shift conversions
> but those are rarely encountered in practice.
I have come up with the attached patch which more or less implements
Kaspar's suggestions. I am using
"XN_FLAG_RFC2253 & ~ASN1_STRFLGS_ESC_MSB" as flags for
X509_NAME_print_ex() for SSL_{CLIENT,SERVER}_{I,S}_DN. This changes
/C=US/ST=California/L=San Francisco/O=ASF/OU=httpd-
test/CN=ca/emailaddress=test-...@httpd.apache.org
into
emailaddress=test-...@httpd.apache.org,CN=ca,OU=httpd-test,O=ASF,L=San
Francisco,ST=California,C=US
Is this what we want? We could use XN_FLAG_DN_REV to keep the old
order. I haven't tried UTF8 characters, yet.
Instead of an SSLOption (which would require a request_rec to lookup),
I have implemented a per-vhost option for restoring compatibility.
diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c
index 3d090cb..fc36f6c 100644
--- a/modules/ssl/mod_ssl.c
+++ b/modules/ssl/mod_ssl.c
@@ -128,6 +128,8 @@ static const command_rec ssl_config_cmds[] = {
"Use the server's cipher ordering preference")
SSL_CMD_SRV(InsecureRenegotiation, FLAG,
"Enable support for insecure renegotiation")
+ SSL_CMD_SRV(LegacyDNVarFormat, FLAG,
+ "Use legacy format for SSL_{CLIENT,SERVER}_{I,S}_DN variables")
SSL_CMD_ALL(UserName, TAKE1,
"Set user name to SSL variable value")
SSL_CMD_SRV(StrictSNIVHostCheck, FLAG,
diff --git a/modules/ssl/ssl_engine_config.c b/modules/ssl/ssl_engine_config.c
index 4bccfe7..de4b621 100644
--- a/modules/ssl/ssl_engine_config.c
+++ b/modules/ssl/ssl_engine_config.c
@@ -186,6 +186,7 @@ static SSLSrvConfigRec *ssl_config_server_new(apr_pool_t *p)
sc->session_cache_timeout = UNSET;
sc->cipher_server_pref = UNSET;
sc->insecure_reneg = UNSET;
+ sc->legacy_dn_format = UNSET;
sc->proxy_ssl_check_peer_expire = SSL_ENABLED_UNSET;
sc->proxy_ssl_check_peer_cn = SSL_ENABLED_UNSET;
#ifndef OPENSSL_NO_TLSEXT
@@ -298,6 +299,7 @@ void *ssl_config_server_merge(apr_pool_t *p, void *basev, void *addv)
cfgMergeInt(session_cache_timeout);
cfgMergeBool(cipher_server_pref);
cfgMergeBool(insecure_reneg);
+ cfgMergeBool(legacy_dn_format);
cfgMerge(proxy_ssl_check_peer_expire, SSL_ENABLED_UNSET);
cfgMerge(proxy_ssl_check_peer_cn, SSL_ENABLED_UNSET);
#ifndef OPENSSL_NO_TLSEXT
@@ -669,6 +671,13 @@ const char *ssl_cmd_SSLInsecureRenegotiation(cmd_parms *cmd, void *dcfg, int fla
#endif
}
+const char *ssl_cmd_SSLLegacyDNVarFormat(cmd_parms *cmd, void *dcfg, int flag)
+{
+ SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
+ sc->legacy_dn_format = flag?TRUE:FALSE;
+ return NULL;
+}
+
static const char *ssl_cmd_check_dir(cmd_parms *parms,
const char **dir)
diff --git a/modules/ssl/ssl_engine_vars.c b/modules/ssl/ssl_engine_vars.c
index d5cf63b..30bdd81 100644
--- a/modules/ssl/ssl_engine_vars.c
+++ b/modules/ssl/ssl_engine_vars.c
@@ -40,7 +40,7 @@
*/
static char *ssl_var_lookup_ssl(apr_pool_t *p, conn_rec *c, char *var);
-static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, X509 *xs, char *var);
+static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, X509 *xs, char *var, int legacy_format);
static char *ssl_var_lookup_ssl_cert_dn(apr_pool_t *p, X509_NAME *xsname, char *var);
static char *ssl_var_lookup_ssl_cert_valid(apr_pool_t *p, ASN1_UTCTIME *tm);
static char *ssl_var_lookup_ssl_cert_remain(apr_pool_t *p, ASN1_UTCTIME *tm);
@@ -358,13 +358,19 @@ static char *ssl_var_lookup_ssl(apr_pool_t *p, conn_rec *c, char *var)
}
else if (ssl != NULL && strlen(var) > 7 && strcEQn(var, "CLIENT_", 7)) {
if ((xs = SSL_get_peer_certificate(ssl)) != NULL) {
- result = ssl_var_lookup_ssl_cert(p, xs, var+7);
+ SSLSrvConfigRec *sc = mySrvConfigFromConn(c);
+ result = ssl_var_lookup_ssl_cert(p, xs, var+7, sc->legacy_dn_format);
X509_free(xs);
}
}
else if (ssl != NULL && strlen(var) > 7 && strcEQn(var, "SERVER_", 7)) {
- if ((xs = SSL_get_certificate(ssl)) != NULL)
- result = ssl_var_lookup_ssl_cert(p, xs, var+7);
+ if ((xs = SSL_get_certificate(ssl)) != NULL) {
+ SSLSrvConfigRec *sc = mySrvConfigFromConn(c);
+ result = ssl_var_lookup_ssl_cert(p, xs, var+7, sc->legacy_dn_format);
+ /* SSL_get_certificate is different from SSL_get_peer_certificate.
+ * No need to X509_free(xs).
+ */
+ }
}
else if (ssl != NULL && strcEQ(var, "COMPRESS_METHOD")) {
result = ssl_var_lookup_ssl_compress_meth(ssl);
@@ -386,13 +392,39 @@ static char *ssl_var_lookup_ssl(apr_pool_t *p, conn_rec *c, char *var)
return result;
}
-static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, X509 *xs, char *var)
+static char *ssl_var_lookup_ssl_cert_dn_oneline(apr_pool_t *p,
+ X509_NAME *xsname,
+ int legacy_dn_format)
+{
+ char *result;
+ if (legacy_dn_format == TRUE) {
+ char *cp = X509_NAME_oneline(xsname, NULL, 0);
+ result = apr_pstrdup(p, cp);
+ modssl_free(cp);
+ }
+ else {
+ BIO* bio;
+ int n;
+ unsigned long flags = XN_FLAG_RFC2253 & ~ASN1_STRFLGS_ESC_MSB;
+ if ((bio = BIO_new(BIO_s_mem())) == NULL)
+ return NULL;
+ X509_NAME_print_ex(bio, xsname, 0, flags);
+ n = BIO_pending(bio);
+ result = apr_pcalloc(p, n+1);
+ n = BIO_read(bio, result, n);
+ result[n] = NUL;
+ BIO_free(bio);
+ }
+ return result;
+}
+
+static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, X509 *xs, char *var,
+ int legacy_format)
{
char *result;
BOOL resdup;
X509_NAME *xsname;
int nid;
- char *cp;
result = NULL;
resdup = TRUE;
@@ -416,9 +448,7 @@ static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, X509 *xs, char *var)
}
else if (strcEQ(var, "S_DN")) {
xsname = X509_get_subject_name(xs);
- cp = X509_NAME_oneline(xsname, NULL, 0);
- result = apr_pstrdup(p, cp);
- modssl_free(cp);
+ result = ssl_var_lookup_ssl_cert_dn_oneline(p, xsname, legacy_format);
resdup = FALSE;
}
else if (strlen(var) > 5 && strcEQn(var, "S_DN_", 5)) {
@@ -428,9 +458,7 @@ static char *ssl_var_lookup_ssl_cert(apr_pool_t *p, X509 *xs, char *var)
}
else if (strcEQ(var, "I_DN")) {
xsname = X509_get_issuer_name(xs);
- cp = X509_NAME_oneline(xsname, NULL, 0);
- result = apr_pstrdup(p, cp);
- modssl_free(cp);
+ result = ssl_var_lookup_ssl_cert_dn_oneline(p, xsname, legacy_format);
resdup = FALSE;
}
else if (strlen(var) > 5 && strcEQn(var, "I_DN_", 5)) {
@@ -516,13 +544,16 @@ static char *ssl_var_lookup_ssl_cert_dn(apr_pool_t *p, X509_NAME *xsname, char *
n =OBJ_obj2nid((ASN1_OBJECT *)X509_NAME_ENTRY_get_object(xsne));
if (n == ssl_var_lookup_ssl_cert_dn_rec[i].nid && idx-- == 0) {
- unsigned char *data = X509_NAME_ENTRY_get_data_ptr(xsne);
- /* cast needed from unsigned char to char */
- result = apr_pstrmemdup(p, (char *)data,
- X509_NAME_ENTRY_get_data_len(xsne));
+ unsigned char *string = NULL;
+ int ret = ASN1_STRING_to_UTF8(&string, X509_NAME_ENTRY_get_data(xsne));
+ if (ret >= 0) {
+ /* cast needed from unsigned char to char */
+ result = apr_pstrmemdup(p, (char *)string, ret);
+ modssl_free(string);
#if APR_CHARSET_EBCDIC
- ap_xlate_proto_from_ascii(result, X509_NAME_ENTRY_get_data_len(xsne));
+ ap_xlate_proto_from_ascii(result, ret);
#endif /* APR_CHARSET_EBCDIC */
+ }
break;
}
}
diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h
index ae344a1..f390808 100644
--- a/modules/ssl/ssl_private.h
+++ b/modules/ssl/ssl_private.h
@@ -528,6 +528,7 @@ struct SSLSrvConfigRec {
int session_cache_timeout;
BOOL cipher_server_pref;
BOOL insecure_reneg;
+ BOOL legacy_dn_format;
modssl_ctx_t *server;
modssl_ctx_t *proxy;
ssl_enabled_t proxy_ssl_check_peer_expire;
@@ -602,7 +603,8 @@ const char *ssl_cmd_SSLRequire(cmd_parms *, void *, const char *);
const char *ssl_cmd_SSLUserName(cmd_parms *, void *, const char *);
const char *ssl_cmd_SSLRenegBufferSize(cmd_parms *cmd, void *dcfg, const char *arg);
const char *ssl_cmd_SSLStrictSNIVHostCheck(cmd_parms *cmd, void *dcfg, int flag);
-const char *ssl_cmd_SSLInsecureRenegotiation(cmd_parms *cmd, void *dcfg, int flag);
+const char *ssl_cmd_SSLInsecureRenegotiation(cmd_parms *cmd, void *dcfg, int flag);
+const char *ssl_cmd_SSLLegacyDNVarFormat(cmd_parms *cmd, void *dcfg, int flag);
const char *ssl_cmd_SSLProxyEngine(cmd_parms *cmd, void *dcfg, int flag);
const char *ssl_cmd_SSLProxyProtocol(cmd_parms *, void *, const char *);
