On Thu, Mar 30, 2023 at 07:28:20PM +0200, Ruediger Pluem wrote: > > > 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)) > good catch, thanks. Giovanni
> > > >> 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 >
