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

Reply via email to