On 3/30/23 7:19 PM, Ruediger Pluem wrote:
>
>
> On 3/30/23 7:09 PM, gbec...@apache.org wrote:
>> Author: gbechis
>> Date: Thu Mar 30 17:09:09 2023
>> New Revision: 1908805
>>
>> URL: http://svn.apache.org/viewvc?rev=1908805&view=rev
>> Log:
>> check for more possible SSL failures
>> bz #66225
>>
>> Modified:
>> httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
>>
>> 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=1908805&r1=1908804&r2=1908805&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Thu Mar 30 17:09:09
>> 2023
>> @@ -997,10 +997,7 @@ static int ssl_hook_Access_classic(reque
>> * handshake to proceed. */
>> modssl_set_reneg_state(sslconn, RENEG_ALLOW);
>>
>> - SSL_renegotiate(ssl);
>> - SSL_do_handshake(ssl);
>> -
>> - if (!SSL_is_init_finished(ssl)) {
>> + if(!SSL_renegotiate(ssl) || !SSL_do_handshake(ssl) ||
>> !SSL_is_init_finished(ssl)) {
>
> Wouldn't
>
> if (!(SSL_renegotiate(ssl) && SSL_do_handshake(ssl) &&
> SSL_is_init_finished(ssl))) {
>
> be better as it would stop the calls as soon as one fails (due to boolean
> shortcuts)?
> Or is it mandatory that SSL_do_handshake and / or SSL_is_init_finished get
> executed if one of steps before fails?
Scratch the above. This was already true for the expression in the commit.
But for SSL_do_handshake only the return value 1 indicates success, all values
<= 0 indicate failure.
https://www.openssl.org/docs/man1.1.1/man3/SSL_do_handshake.html
Hence the proposal would be
if (!SSL_renegotiate(ssl) || (SSL_do_handshake(ssl) != 1) ||
!SSL_is_init_finished(ssl))
>
>> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(02225)
>> "Re-negotiation request failed");
>> ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, r->server);
>>
>>
>>
>
Regards
Rüdiger
