Bug 37791 - SEGV if the client is connection plain to a SSL enabled port
Summary: SEGV if the client is connection plain to a SSL enabled port
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_ssl (show other bugs)
Version: 2.5-HEAD
Hardware: Other other
: P2 major (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-12-05 17:04 UTC by keilh
Modified: 2014-02-17 13:51 UTC (History)
0 users



Attachments
Patch against 2.0.x (681 bytes, patch)
2006-01-11 23:28 UTC, Ruediger Pluem
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description keilh 2005-12-05 17:04:52 UTC
Consider the following configuration:


ErrorDocument 400 /server_error.html

Listen  some.server.com:443 
<VirtualHost some.server.com:443>

ServerName   some.server.com:443

SSLEngine               On
SSLProtocol             +TLSv1 +SSLv3
SSLCipherSuite          !EXPORT56:RC4-MD5:DES-CBC3-SHA:EXP-RC4-MD5
SSLOptions              +OptRenegotiate +StdEnvVars
SSLCertificateFile      server-cert.pem

<Location / >
SSLCipherSuite DES-CBC3-SHA
</Location>

</VirtualHost>  

If a client is connection plain to the SSL port, the server cores with the
following stack: (debug build on solaris)

dummy_worker(opaque = 0x156088)
worker_thread(thd = 0x156088, dummy = 0x22b020)
process_socket(p = 0x23a4a8, sock = 0x23a4e0, my_child_num = 0, my_thread_num =
9, bucket_alloc = 0x23c4b0)
ap_process_connection(c = 0x23a5d0, csd = 0x23a4e0)
ap_run_process_connection(0x23a5d0, 0x23a4e0, 0x23a4e0, 0x9, 0x23a5c8, 0x23c4b0)
ap_process_http_connection(c = 0x23a5d0)
ap_read_request(conn = 0x23a5d0)
ap_die(type = 400, r = 0x244500)
ap_internal_redirect(new_uri = 0x150410 "/server_error.html", r = 0x244500)
ap_process_request_internal(r = 0x245388)
ap_run_access_checker(0x245388, 0xfee9630c, 0x245c92, 0xffffffff, 0xfffffff8,
0x245510)
ssl_hook_Access(r = 0x245388)
SSL_get_current_cipher(s = (nil))

The problems is that by ap_internal_redirect the ssl_hook_Access(..)
will now be called, and that method does not handle the case ssl == NULL.

Fix:
do not run the access-checker-hooks for the error handling

Config workaround:
Configure also 'SSSLRequire' if you configure 'SSLSLCipherSuite'
for any location.

We tested the described behaviour with apache/2.0.53 and apache/2.1.9
Comment 1 Joe Orton 2005-12-06 17:57:32 UTC
Thanks for the report, this has been fixed on the trunk:

http://svn.apache.org/viewcvs.cgi?rev=354394&view=rev
Comment 2 Ruediger Pluem 2006-01-11 23:28:08 UTC
Created attachment 17393 [details]
Patch against 2.0.x
Comment 3 Ruediger Pluem 2006-01-11 23:39:35 UTC
Proposed for backport to 2.2.x as r355720
(http://svn.apache.org/viewcvs.cgi?rev=355720&view=rev) and proposed for
backport to 2.0.x as r368152
(http://svn.apache.org/viewcvs.cgi?rev=368152&view=rev).
There is also a CVEID for this bug: CAN-2005-3357
Comment 4 Marc Stern 2006-01-27 15:14:58 UTC
Doesn't the same problem also appear at the very beginning of in ssl_hook_Fixup() ?

if (!(sc->enabled && sslconn && (ssl = sslconn->ssl)))
   should become
if ( !sc->enabled || !sslconn || (ssl != sslconn->ssl) )

Right ?
Comment 5 Ruediger Pluem 2006-01-28 01:17:36 UTC
(In reply to comment #4)
> 
> if (!(sc->enabled && sslconn && (ssl = sslconn->ssl)))
>    should become
> if ( !sc->enabled || !sslconn || (ssl != sslconn->ssl) )
> 
> Right ?

No.

1. Your version is nearly the same as above because

!(sc->enabled && sslconn && (ssl = sslconn->ssl))  is equal to

!sc->enabled || !sslconn || !(ssl == sslconn->ssl)

keep in mind that 

ssl = sslconn->ssl

and

ssl == sslconn->ssl

are different things. So the original condition becomes true if sslconn->ssl is
equal to NULL which is only checked if sslconn is different from NULL.
But I need to check on the trunk where the current condition is somewhat
different and maybe also wrong. So thanks for the pointer.
Comment 6 Ruediger Pluem 2006-01-29 12:12:19 UTC
Meanwhile I checked the slightly different condition on trunk and 2.2.x and they
are also correct.
Comment 7 Marc Stern 2006-01-30 09:25:36 UTC
I read too fast, sorry :-(