On Wed, Apr 15, 2015 at 06:36:13PM +0200, Stefan Sperling wrote:
> However, the actual issue here is that mod_ssl is squatting the SSL_
> namespace.
> Historically this may have made sense (it seems mod_ssl and OpenSSL have
> shared history/authors). Bill Rowe suggested to try moving mod_ssl's
> functions into the ap_ namespace to avoid such clashes in the future.
Given the feedback so far, it seems we all generally agree that
the current namespacing is a problem that needs to be addressed.
I like Jeff's suggestion to split this task up into smaller changes
by looking at each symbol on a case-by-case basis.
And the alternative naming schemes suggested make sense to me.
So to take a first step forward, here's a diff that puts ssl_util_ssl.h
macros into the MODSSL_ namespace.
Index: modules/ssl/ssl_engine_init.c
===================================================================
--- modules/ssl/ssl_engine_init.c (revision 1674422)
+++ modules/ssl/ssl_engine_init.c (working copy)
@@ -148,12 +148,12 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_po
apr_status_t rv;
apr_array_header_t *pphrases;
- if (SSLeay() < SSL_LIBRARY_VERSION) {
+ if (SSLeay() < MODSSL_LIBRARY_VERSION) {
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, base_server, APLOGNO(01882)
"Init: this version of mod_ssl was compiled against "
"a newer library (%s, version currently loaded is %s)"
" - may result in undefined or erroneous behavior",
- SSL_LIBRARY_TEXT, SSLeay_version(SSLEAY_VERSION));
+ MODSSL_LIBRARY_TEXT, SSLeay_version(SSLEAY_VERSION));
}
/* We initialize mc->pid per-process in the child init,
@@ -242,7 +242,7 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_po
#endif
ap_log_error(APLOG_MARK, APLOG_INFO, 0, s, APLOGNO(01883)
- "Init: Initialized %s library", SSL_LIBRARY_NAME);
+ "Init: Initialized %s library", MODSSL_LIBRARY_NAME);
/*
* Seed the Pseudo Random Number Generator (PRNG)
Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c (revision 1674422)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -2186,7 +2186,7 @@ long ssl_io_data_cb(BIO *bio, int cmd,
}
ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
"%s: %s %ld/%d bytes %s BIO#%pp [mem: %pp] %s",
- SSL_LIBRARY_NAME,
+ MODSSL_LIBRARY_NAME,
(cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" : "read"),
rc, argi, (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "to" :
"from"),
bio, argp, dump);
@@ -2196,7 +2196,7 @@ long ssl_io_data_cb(BIO *bio, int cmd,
else {
ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
"%s: I/O error, %d bytes expected to %s on BIO#%pp [mem:
%pp]",
- SSL_LIBRARY_NAME, argi,
+ MODSSL_LIBRARY_NAME, argi,
(cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" : "read"),
bio, argp);
}
Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c (revision 1674422)
+++ modules/ssl/ssl_engine_kernel.c (working copy)
@@ -1654,7 +1654,7 @@ static void ssl_session_log(server_rec *s,
const char *result,
long timeout)
{
- char buf[SSL_SESSION_ID_STRING_LEN];
+ char buf[MODSSL_SESSION_ID_STRING_LEN];
char timeout_str[56] = {'\0'};
if (!APLOGdebug(s)) {
@@ -1811,32 +1811,32 @@ static void log_tracing_state(const SSL *ssl, conn
*/
if (where & SSL_CB_HANDSHAKE_START) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c,
- "%s: Handshake: start", SSL_LIBRARY_NAME);
+ "%s: Handshake: start", MODSSL_LIBRARY_NAME);
}
else if (where & SSL_CB_HANDSHAKE_DONE) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c,
- "%s: Handshake: done", SSL_LIBRARY_NAME);
+ "%s: Handshake: done", MODSSL_LIBRARY_NAME);
}
else if (where & SSL_CB_LOOP) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c,
"%s: Loop: %s",
- SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
+ MODSSL_LIBRARY_NAME, SSL_state_string_long(ssl));
}
else if (where & SSL_CB_READ) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c,
"%s: Read: %s",
- SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
+ MODSSL_LIBRARY_NAME, SSL_state_string_long(ssl));
}
else if (where & SSL_CB_WRITE) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c,
"%s: Write: %s",
- SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
+ MODSSL_LIBRARY_NAME, SSL_state_string_long(ssl));
}
else if (where & SSL_CB_ALERT) {
char *str = (where & SSL_CB_READ) ? "read" : "write";
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c,
"%s: Alert: %s:%s:%s",
- SSL_LIBRARY_NAME, str,
+ MODSSL_LIBRARY_NAME, str,
SSL_alert_type_string_long(rc),
SSL_alert_desc_string_long(rc));
}
@@ -1844,12 +1844,12 @@ static void log_tracing_state(const SSL *ssl, conn
if (rc == 0) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c,
"%s: Exit: failed in %s",
- SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
+ MODSSL_LIBRARY_NAME, SSL_state_string_long(ssl));
}
else if (rc < 0) {
ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c,
"%s: Exit: error in %s",
- SSL_LIBRARY_NAME, SSL_state_string_long(ssl));
+ MODSSL_LIBRARY_NAME, SSL_state_string_long(ssl));
}
}
Index: modules/ssl/ssl_engine_vars.c
===================================================================
--- modules/ssl/ssl_engine_vars.c (revision 1674422)
+++ modules/ssl/ssl_engine_vars.c (working copy)
@@ -131,7 +131,7 @@ static apr_status_t ssl_get_tls_cb(apr_pool_t *p,
}
static const char var_interface[] = "mod_ssl/" AP_SERVER_BASEREVISION;
-static char var_library_interface[] = SSL_LIBRARY_TEXT;
+static char var_library_interface[] = MODSSL_LIBRARY_TEXT;
static char *var_library = NULL;
static apr_array_header_t *expr_peer_ext_list_fn(ap_expr_eval_ctx_t *ctx,
@@ -185,7 +185,7 @@ void ssl_var_register(apr_pool_t *p)
APR_REGISTER_OPTIONAL_FN(ssl_ext_list);
/* Perform once-per-process library version determination: */
- var_library = apr_pstrdup(p, SSL_LIBRARY_DYNTEXT);
+ var_library = apr_pstrdup(p, MODSSL_LIBRARY_DYNTEXT);
if ((cp = strchr(var_library, ' ')) != NULL) {
*cp = '/';
@@ -406,7 +406,7 @@ static char *ssl_var_lookup_ssl(apr_pool_t *p, con
result = (char *)SSL_get_version(ssl);
}
else if (ssl != NULL && strcEQ(var, "SESSION_ID")) {
- char buf[SSL_SESSION_ID_STRING_LEN];
+ char buf[MODSSL_SESSION_ID_STRING_LEN];
SSL_SESSION *pSession = SSL_get_session(ssl);
if (pSession) {
unsigned char *id;
Index: modules/ssl/ssl_scache.c
===================================================================
--- modules/ssl/ssl_scache.c (revision 1674422)
+++ modules/ssl/ssl_scache.c (working copy)
@@ -115,7 +115,7 @@ BOOL ssl_scache_store(server_rec *s, UCHAR *id, in
apr_pool_t *p)
{
SSLModConfigRec *mc = myModConfig(s);
- unsigned char encoded[SSL_SESSION_MAX_DER], *ptr;
+ unsigned char encoded[MODSSL_SESSION_MAX_DER], *ptr;
unsigned int len;
apr_status_t rv;
@@ -148,8 +148,8 @@ SSL_SESSION *ssl_scache_retrieve(server_rec *s, UC
apr_pool_t *p)
{
SSLModConfigRec *mc = myModConfig(s);
- unsigned char dest[SSL_SESSION_MAX_DER];
- unsigned int destlen = SSL_SESSION_MAX_DER;
+ unsigned char dest[MODSSL_SESSION_MAX_DER];
+ unsigned int destlen = MODSSL_SESSION_MAX_DER;
const unsigned char *ptr;
apr_status_t rv;
Index: modules/ssl/ssl_util_ssl.h
===================================================================
--- modules/ssl/ssl_util_ssl.h (revision 1674422)
+++ modules/ssl/ssl_util_ssl.h (working copy)
@@ -38,10 +38,10 @@
* SSL library version number
*/
-#define SSL_LIBRARY_VERSION OPENSSL_VERSION_NUMBER
-#define SSL_LIBRARY_NAME "OpenSSL"
-#define SSL_LIBRARY_TEXT OPENSSL_VERSION_TEXT
-#define SSL_LIBRARY_DYNTEXT SSLeay_version(SSLEAY_VERSION)
+#define MODSSL_LIBRARY_VERSION OPENSSL_VERSION_NUMBER
+#define MODSSL_LIBRARY_NAME "OpenSSL"
+#define MODSSL_LIBRARY_TEXT OPENSSL_VERSION_TEXT
+#define MODSSL_LIBRARY_DYNTEXT SSLeay_version(SSLEAY_VERSION)
/**
* Maximum length of a DER encoded session.
@@ -48,10 +48,10 @@
* FIXME: There is no define in OpenSSL, but OpenSSL uses 1024*10,
* so this value should be ok. Although we have no warm feeling.
*/
-#define SSL_SESSION_MAX_DER 1024*10
+#define MODSSL_SESSION_MAX_DER 1024*10
-/** max length for SSL_SESSION_id2sz */
-#define SSL_SESSION_ID_STRING_LEN \
+/** max length for MODSSL_SESSION_id2sz */
+#define MODSSL_SESSION_ID_STRING_LEN \
((SSL_MAX_SSL_SESSION_ID_LENGTH + 1) * 2)
/**
