You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2010/02/03 14:38:10 UTC
svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c
ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c ssl_private.h
Author: jorton
Date: Wed Feb 3 13:38:09 2010
New Revision: 906039
URL: http://svn.apache.org/viewvc?rev=906039&view=rev
Log:
New releases of OpenSSL will only allow secure renegotiation by
default. Add an "SSLInsecureRenegotiation" directive to enable
renegotiation against unpatched clients, to ease transition:
* modules/ssl/ssl_private.h (struct SSLSrvConfigRec): Add
insecure_reneg field.
* modules/ssl/ssl_engine_config.c (ssl_config_server_new,
ssl_config_server_merge): Handle the insecure_reneg flag.
(ssl_cmd_SSLInsecureRenegotiation): New function.
* modules/ssl/ssl_engine_init.c (ssl_init_ctx_protocol): Set the
SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION option if insecure_reneg is
enabled.
* modules/ssl/ssl_engine_kernel.c (ssl_hook_Access): Log level of
support for secure reneg.
* modules/ssl/mod_ssl.c: Add the directive definition.
Modified:
httpd/httpd/trunk/modules/ssl/mod_ssl.c
httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
httpd/httpd/trunk/modules/ssl/ssl_private.h
Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=906039&r1=906038&r2=906039&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
+++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Wed Feb 3 13:38:09 2010
@@ -123,6 +123,8 @@
"('[+-][SSLv2|SSLv3|TLSv1] ...' - see manual)")
SSL_CMD_SRV(HonorCipherOrder, FLAG,
"Use the server's cipher ordering preference")
+ SSL_CMD_SRV(InsecureRenegotiation, FLAG,
+ "Enable support for insecure renegotiation")
SSL_CMD_ALL(UserName, TAKE1,
"Set user name to SSL variable value")
SSL_CMD_SRV(LogLevelDebugDump, TAKE1,
Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=906039&r1=906038&r2=906039&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Wed Feb 3 13:38:09 2010
@@ -185,6 +185,7 @@
sc->vhost_id_len = 0; /* set during module init */
sc->session_cache_timeout = UNSET;
sc->cipher_server_pref = UNSET;
+ sc->insecure_reneg = UNSET;
sc->ssl_log_level = SSL_LOG_UNSET;
sc->proxy_ssl_check_peer_expire = SSL_ENABLED_UNSET;
sc->proxy_ssl_check_peer_cn = SSL_ENABLED_UNSET;
@@ -294,6 +295,7 @@
cfgMergeBool(proxy_enabled);
cfgMergeInt(session_cache_timeout);
cfgMergeBool(cipher_server_pref);
+ cfgMergeBool(insecure_reneg);
cfgMerge(ssl_log_level, SSL_LOG_UNSET);
cfgMerge(proxy_ssl_check_peer_expire, SSL_ENABLED_UNSET);
cfgMerge(proxy_ssl_check_peer_cn, SSL_ENABLED_UNSET);
@@ -628,6 +630,18 @@
#endif
}
+const char *ssl_cmd_SSLInsecureRenegotiation(cmd_parms *cmd, void *dcfg, int flag)
+{
+#ifdef SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
+ SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
+ sc->insecure_reneg = flag?TRUE:FALSE;
+ return NULL;
+#else
+ return "SSLInsecureRenegotiation is not supported by the SSL library";
+#endif
+}
+
+
static const char *ssl_cmd_check_dir(cmd_parms *parms,
const char **dir)
{
Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_init.c?rev=906039&r1=906038&r2=906039&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_init.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_init.c Wed Feb 3 13:38:09 2010
@@ -414,6 +414,7 @@
MODSSL_SSL_METHOD_CONST SSL_METHOD *method = NULL;
char *cp;
int protocol = mctx->protocol;
+ SSLSrvConfigRec *sc = mySrvConfig(s);
/*
* Create the new per-server SSL context
@@ -473,11 +474,14 @@
}
#ifdef SSL_OP_CIPHER_SERVER_PREFERENCE
- {
- SSLSrvConfigRec *sc = mySrvConfig(s);
- if (sc->cipher_server_pref == TRUE) {
- SSL_CTX_set_options(ctx, SSL_OP_CIPHER_SERVER_PREFERENCE);
- }
+ if (sc->cipher_server_pref == TRUE) {
+ SSL_CTX_set_options(ctx, SSL_OP_CIPHER_SERVER_PREFERENCE);
+ }
+#endif
+
+#ifdef SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION
+ if (sc->insecure_reneg == TRUE) {
+ SSL_CTX_set_options(ctx, SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION);
}
#endif
Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=906039&r1=906038&r2=906039&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Wed Feb 3 13:38:09 2010
@@ -764,10 +764,17 @@
r->connection->keepalive = AP_CONN_CLOSE;
}
- /* do a full renegotiation */
+ /* Perform a full renegotiation. */
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "Performing full renegotiation: "
- "complete handshake protocol");
+ "Performing full renegotiation: complete handshake "
+ "protocol (%s support secure renegotiation)",
+#if defined(SSL_get_secure_renegotiation_support)
+ SSL_get_secure_renegotiation_support(ssl) ?
+ "client does" : "client does not"
+#else
+ "server does not"
+#endif
+ );
SSL_set_session_id_context(ssl,
(unsigned char *)&id,
Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=906039&r1=906038&r2=906039&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_private.h Wed Feb 3 13:38:09 2010
@@ -507,6 +507,7 @@
int vhost_id_len;
int session_cache_timeout;
BOOL cipher_server_pref;
+ BOOL insecure_reneg;
modssl_ctx_t *server;
modssl_ctx_t *proxy;
ssl_log_level_e ssl_log_level;
@@ -580,6 +581,7 @@
const char *ssl_cmd_SSLLogLevelDebugDump(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_SSLProxyEngine(cmd_parms *cmd, void *dcfg, int flag);
const char *ssl_cmd_SSLProxyProtocol(cmd_parms *, void *, const char *);
Re: svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c
ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c ssl_private.h
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 2/3/2010 4:23 PM, Joe Orton wrote:
>
> Does this make sense?
Much more - thanks for the explanation!
Re: svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl:
mod_ssl.c ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c
ssl_private.h
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Feb 03, 2010 at 03:33:23PM -0600, William Rowe wrote:
> On 2/3/2010 3:18 PM, Joe Orton wrote:
> > On Wed, Feb 03, 2010 at 12:44:45PM -0500, Eric Covener wrote:
> >> On Wed, Feb 3, 2010 at 12:09 PM, Joe Orton <jo...@redhat.com> wrote:
> >>> I considered logging a warning for each client which renegotiates
> >>> insecurely (whether due to lack of support on client or server), but,
> >>> that's likely to be very noisy.
...
> How would this work if the renegotiation occurred after the request
> was actually sent? [And of course, once the MITM is injected, you have
> no trust of the chain of authority].
Sorry, that's really sloppy wording on my part above. The information
we want to log here is the single bit of data:
"does the client support the secure reneg protocol: yes/no"
New clients will advertise support for the secure reneg protocol in the
initial handshake of any SSL connection. Old clients, and any attacker
trying to perform the MITM, will not. All renegs on the connection will
either be secure for new clients, or, if now permitted by config,
insecure for old clients/attackers.
So with my patch, if you log %{ssl-secure-reneg}n it will be invariant
for all the requests on any given SSL connection, and orthogonal to
whether any reneg actually takes place on that connection. It's just
reporting the client's capability, rather than indicating any particular
reneg is secure/insecure.
Does this make sense?
Regards, Joe
Re: svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c
ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c ssl_private.h
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 2/3/2010 3:18 PM, Joe Orton wrote:
> On Wed, Feb 03, 2010 at 12:44:45PM -0500, Eric Covener wrote:
>> On Wed, Feb 3, 2010 at 12:09 PM, Joe Orton <jo...@redhat.com> wrote:
>>
>>> I considered logging a warning for each client which renegotiates
>>> insecurely (whether due to lack of support on client or server), but,
>>> that's likely to be very noisy.
>>
>> Any way to note the insecure renegotiation and save it long enough to
>> be associated with a r->notes or subprocess_env?
>>
>> That would let you log it with IP and user-agent in access log (and
>> help you convince yourself it might be safe to turn on strict
>> renegotiation based on log analysis)
>
> Nice idea, yes, that seems sensible. This seems to work:
How would this work if the renegotiation occurred after the request
was actually sent? [And of course, once the MITM is injected, you have
no trust of the chain of authority].
I can see a value in allow-from {trusted IP pool}. But not sure this
really helps, since it doesn't limit the renegotiation, it limits the
request acceptance.
Re: svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl:
mod_ssl.c ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c
ssl_private.h
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Feb 03, 2010 at 12:44:45PM -0500, Eric Covener wrote:
> On Wed, Feb 3, 2010 at 12:09 PM, Joe Orton <jo...@redhat.com> wrote:
>
> > I considered logging a warning for each client which renegotiates
> > insecurely (whether due to lack of support on client or server), but,
> > that's likely to be very noisy.
>
> Any way to note the insecure renegotiation and save it long enough to
> be associated with a r->notes or subprocess_env?
>
> That would let you log it with IP and user-agent in access log (and
> help you convince yourself it might be safe to turn on strict
> renegotiation based on log analysis)
Nice idea, yes, that seems sensible. This seems to work:
Index: ssl_engine_kernel.c
===================================================================
--- ssl_engine_kernel.c (revision 906057)
+++ ssl_engine_kernel.c (working copy)
@@ -242,6 +242,15 @@ int ssl_hook_ReadReq(request_rec *r)
ssl_util_vhostid(r->pool, r->server));
}
+ apr_table_setn(r->notes, "ssl-secure-reneg",
+#ifdef SSL_get_secure_renegotiation_support
+ SSL_get_secure_renegotiation_support(ssl)
+ ? "1" : "0"
+#else
+ "0"
+#endif
+ );
+
/* SetEnvIf ssl-*-shutdown flags can only be per-server,
* so they won't change across keepalive requests
*/
Re: svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl:
mod_ssl.c ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c
ssl_private.h
Posted by Eric Covener <co...@gmail.com>.
On Wed, Feb 3, 2010 at 12:09 PM, Joe Orton <jo...@redhat.com> wrote:
> I considered logging a warning for each client which renegotiates
> insecurely (whether due to lack of support on client or server), but,
> that's likely to be very noisy.
Any way to note the insecure renegotiation and save it long enough to
be associated with a r->notes or subprocess_env?
That would let you log it with IP and user-agent in access log (and
help you convince yourself it might be safe to turn on strict
renegotiation based on log analysis)
--
Eric Covener
covener@gmail.com
Re: svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl:
mod_ssl.c ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c
ssl_private.h
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Feb 03, 2010 at 11:51:16AM -0500, Dan Poirier wrote:
> How about logging a dire warning during startup if insecure
> renegotiation has been enabled?
Hmmm, I'm not sure. If the user has configured this it seems slightly
patronising to then berate them for doing so. Also, why log in the case
that the directive is supported and enabled, but not the case where the
directive is unsupported because OpenSSL is too old? In either case
reneg is (or may be) insecure.
I considered logging a warning for each client which renegotiates
insecurely (whether due to lack of support on client or server), but,
that's likely to be very noisy.
Regards, Joe
Re: svn commit: r906039 - in /httpd/httpd/trunk/modules/ssl:
mod_ssl.c ssl_engine_config.c ssl_engine_init.c ssl_engine_kernel.c
ssl_private.h
Posted by Dan Poirier <po...@pobox.com>.
How about logging a dire warning during startup if insecure
renegotiation has been enabled?
Dan