On Tue, May 05, 2020 at 03:23:18PM +0200, Ruediger Pluem wrote: > On 5/5/20 2:40 PM, jor...@apache.org wrote: > > Author: jorton > > Date: Tue May 5 12:40:38 2020 > > New Revision: 1877397 > > > > URL: http://svn.apache.org/viewvc?rev=1877397&view=rev > > Log: > > mod_ssl: Switch to using SSL_OP_NO_RENEGOTATION (where available) to > > block client-initiated renegotiation with TLSv1.2 and earlier. ... > > 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=1877397&r1=1877396&r2=1877397&view=diff > > ============================================================================== > > --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original) > > +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Tue May 5 12:40:38 > > 2020 > > > @@ -1182,8 +1182,6 @@ static int ssl_hook_Access_modern(reques > > return HTTP_FORBIDDEN; > > } > > > > - old_state = sslconn->reneg_state; > > - sslconn->reneg_state = RENEG_ALLOW; > > modssl_set_app_data2(ssl, r); > > > > SSL_do_handshake(ssl); > > @@ -1193,7 +1191,6 @@ static int ssl_hook_Access_modern(reques > > */ > > SSL_peek(ssl, peekbuf, 0); > > > > - sslconn->reneg_state = old_state; > > modssl_set_app_data2(ssl, NULL); > > > > /* > > I don't understand why this can be removed unconditionally.
It is removed unconditionally in ssl_hook_Access_modern, which is used only for TLSv1.3. The aim of this code was to protect against ClientHello being sent unexpectedly and that is always illegal in TLSv1.3. https://www.rfc-editor.org/rfc/rfc8446.html#page-28 - Because TLS 1.3 forbids renegotiation, if a server has negotiated TLS 1.3 and receives a ClientHello at any other time, it MUST terminate the connection with an "unexpected_message" alert. So the ->reneg_state setting already had no effect for TLSv1.3, and the code in ssl_callback_Info *before* my commit already checked and was a noop for the TLSv1.3 case, in line 2289 of the function: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?revision=1877349&view=markup&pathrev=1877397#l2273 Does that make sense? Regards, Joe
