On 8/12/25 11:12 AM, Ruediger Pluem wrote:
>
>
> On 2/4/22 5:05 PM, Stefan Eissing wrote:
>>
>>
>>> Am 04.02.2022 um 16:18 schrieb Yann Ylavic <ylavic....@gmail.com>:
>>>
>>> On Fri, Feb 4, 2022 at 1:22 PM <ic...@apache.org> wrote:
>>>>
>>>> Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
>>>> URL:
>>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=1897760&r1=1897759&r2=1897760&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
>>>> +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Fri Feb 4 12:22:26 2022
>>>> @@ -690,80 +689,31 @@ static int ssl_hook_process_connection(c
>>>> {
>>> []
>>>> temp = apr_brigade_create(c->pool, c->bucket_alloc);
>>>> + rv = ap_get_brigade(c->input_filters, temp,
>>>> + AP_MODE_INIT, APR_BLOCK_READ, 0);
>>>> + apr_brigade_destroy(temp);
>>>>
>>> []
>>>> + if (APR_SUCCESS != APR_SUCCESS) {
>>>
>>> This one's unlikely to trigger :)
>>>
>>>> + if (c->cs) {
>>>> + c->cs->state = CONN_STATE_LINGER;
>>>> }
>>>> + ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(10373)
>>>> + "SSL handshake was not completed, "
>>>> + "closing connection");
>>>> + return OK;
>>>> }
>>> []
>>>> +
>>>> + return DECLINED;
>>>> }
>>
>> That was is my mistakes. I tried to rescue Graham's check on a failed
>> handshake. Maybe you can come up with an improvement on that...
>
> Reviving this old topic again. Someone reported the above condition in PR
> 69773
> (https://bz.apache.org/bugzilla/show_bug.cgi?id=69773). When I tried the
> obvious and proposed fix by the reporter,
> replacing one APR_SUCCESS with rv, 4 tests failed:
>
> t/security/CVE-2005-3357.t (Wstat: 0 Tests: 3 Failed: 2)
> Failed tests: 1, 3
> t/ssl/http.t (Wstat: 0 Tests: 2 Failed: 2)
> Failed tests: 1-2
>
> They fail because they try to speak plain HTTP to a TLS server and expect a
> 400 response with the error message
> "speaking plain HTTP to an SSL-enabled server port" , which is generated
> later in the connection / request life cycle.
> Letting the above error out causes the connection to be closed before the
> response can be send. I assume we want to keep this
> behavior. Hence I think we should either
>
> * Document why this strange code is here and that it is work in progress.
> Probably we could also change one APR_SUCCESS to rv
> already and put the whole if block in #if 0 / #endif
> * Remove the if block
> * Use the following patch which causes the tests to pass again and restores
> the return of the error message while at the same time
> closing the connection early if we do not have the "Plain HTTP" scenario.
>
> Index: modules/ssl/mod_ssl.c
> ===================================================================
> --- modules/ssl/mod_ssl.c (revision 1927749)
> +++ modules/ssl/mod_ssl.c (working copy)
> @@ -709,14 +709,23 @@
> AP_MODE_INIT, APR_BLOCK_READ, 0);
> apr_brigade_destroy(temp);
>
> - if (APR_SUCCESS != APR_SUCCESS) {
> - if (c->cs) {
> - c->cs->state = CONN_STATE_LINGER;
> + if (rv != APR_SUCCESS) {
> + /*
> + * We handle NON_SSL_SEND_REQLINE and NON_SSL_SEND_HDR_SEP
> + * later again in the input filter and in ssl_hook_ReadReq
> + * to return an appropriate error message to the client.
> + * Hence do not close the connection here.
> + */
> + if ((sslconn->non_ssl_request != NON_SSL_SEND_REQLINE) &&
> + (sslconn->non_ssl_request != NON_SSL_SEND_HDR_SEP)) {
> + if (c->cs) {
> + c->cs->state = CONN_STATE_LINGER;
> + }
> + ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(10373)
> + "SSL handshake was not completed, "
> + "closing connection");
> + return OK;
> }
> - ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(10373)
> - "SSL handshake was not completed, "
> - "closing connection");
> - return OK;
> }
> }
>
>
>
Commited as r1927880.
Regards
Rüdiger
