Re: svn commit: r1729927 - in /httpd/httpd/trunk/modules/ssl: ssl_engine_kernel.c ssl_private.h
On 02/12/2016 01:44 AM, rj...@apache.org wrote: > Author: rjung > Date: Fri Feb 12 00:44:22 2016 > New Revision: 1729927 > > URL: http://svn.apache.org/viewvc?rev=1729927=rev > Log: > Support for OpenSSL 1.1.0: > - further improvements for renegotiation > No more test suite failures for reneg, > but still using not so nice polling. > > Modified: > httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c > httpd/httpd/trunk/modules/ssl/ssl_private.h > > 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=1729927=1729926=1729927=diff > == > --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original) > +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Fri Feb 12 00:44:22 2016 > @@ -1040,16 +1040,20 @@ int ssl_hook_Access(request_rec *r) > /* XXX: Polling is bad, alternatives? */ > for (i = 0; i < SSL_HANDSHAKE_MAX_POLLS; i++) { > has_buffered_data(r); > -if (sslconn->ssl == NULL || SSL_is_init_finished(ssl)) { > +if (sslconn->ssl == NULL || > +sslconn->reneg_state == RENEG_DONE || > +sslconn->reneg_state == RENEG_ALLOW) { > break; > } > apr_sleep(SSL_HANDSHAKE_POLL_MS); > } > ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r, APLOGNO() >"Renegotiation loop %d iterations, " > + "reneg_state=%d, " >"in_init=%d, init_finished=%d, " >"state=%s, sslconn->ssl=%s, peer_certs=%s", > - i, SSL_in_init(ssl), SSL_is_init_finished(ssl), > + i, sslconn->reneg_state, > + SSL_in_init(ssl), SSL_is_init_finished(ssl), >SSL_state_string_long(ssl), >sslconn->ssl != NULL ? "yes" : "no", >SSL_get_peer_certificate(ssl) != NULL ? "yes" : > "no"); > @@ -2142,6 +2146,18 @@ void ssl_callback_Info(const SSL *ssl, i > } > #endif > } > +#if OPENSSL_VERSION_NUMBER >= 0x1000L Is this correct or should it be 0x1010L instead? Regards Rüdiger > +else if ((where & SSL_CB_HANDSHAKE_START) && scr->reneg_state == > RENEG_ALLOW) { > +scr->reneg_state = RENEG_STARTED; > +} > +else if ((where & SSL_CB_HANDSHAKE_DONE) && scr->reneg_state == > RENEG_STARTED) { > +scr->reneg_state = RENEG_DONE; > +} > +else if ((where & SSL_CB_ALERT) && > + (scr->reneg_state == RENEG_ALLOW || scr->reneg_state == > RENEG_STARTED)) { > +scr->reneg_state = RENEG_ALERT; > +} > +#endif > /* If the first handshake is complete, change state to reject any > * subsequent client-initiated renegotiation. */ > else if ((where & SSL_CB_HANDSHAKE_DONE) && scr->reneg_state == > RENEG_INIT) { > > Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=1729927=1729926=1729927=diff > == > --- httpd/httpd/trunk/modules/ssl/ssl_private.h (original) > +++ httpd/httpd/trunk/modules/ssl/ssl_private.h Fri Feb 12 00:44:22 2016 > @@ -444,12 +444,17 @@ typedef struct { > * partial fix for CVE-2009-3555. */ > enum { > RENEG_INIT = 0, /* Before initial handshake */ > -RENEG_REJECT, /* After initial handshake; any client-initiated > - * renegotiation should be rejected */ > -RENEG_ALLOW, /* A server-initiated renegotiation is taking > - * place (as dictated by configuration) */ > -RENEG_ABORT /* Renegotiation initiated by client, abort the > - * connection */ > +RENEG_REJECT, /* After initial handshake; any client-initiated > + * renegotiation should be rejected */ > +RENEG_ALLOW,/* A server-initiated renegotiation is taking > + * place (as dictated by configuration) */ > +#if OPENSSL_VERSION_NUMBER >= 0x1000L Same question as above > +RENEG_STARTED, /* A renegotiation has started after RENEG_ALLOW */ > +RENEG_DONE, /* A renegotiation has finished after RENEG_STARTED > */ > +RENEG_ALERT,/* A renegotiation has finished with an SSL Alert */ > +#endif > +RENEG_ABORT /* Renegotiation initiated by client, abort the > + * connection */ > } reneg_state; > > server_rec *server; > > > Regards Rüdiger
Re: svn commit: r1729927 - in /httpd/httpd/trunk/modules/ssl: ssl_engine_kernel.c ssl_private.h
Yes, thanks for the review! Wrong checked version in "#if" (copy typo) fixed with r1729998. Regards, Rainer Am 12.02.2016 um 11:25 schrieb Ruediger Pluem: On 02/12/2016 01:44 AM, rj...@apache.org wrote: Author: rjung Date: Fri Feb 12 00:44:22 2016 New Revision: 1729927 URL: http://svn.apache.org/viewvc?rev=1729927=rev Log: Support for OpenSSL 1.1.0: - further improvements for renegotiation No more test suite failures for reneg, but still using not so nice polling. Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c httpd/httpd/trunk/modules/ssl/ssl_private.h 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=1729927=1729926=1729927=diff == --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original) +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Fri Feb 12 00:44:22 2016 @@ -1040,16 +1040,20 @@ int ssl_hook_Access(request_rec *r) /* XXX: Polling is bad, alternatives? */ for (i = 0; i < SSL_HANDSHAKE_MAX_POLLS; i++) { has_buffered_data(r); -if (sslconn->ssl == NULL || SSL_is_init_finished(ssl)) { +if (sslconn->ssl == NULL || +sslconn->reneg_state == RENEG_DONE || +sslconn->reneg_state == RENEG_ALLOW) { break; } apr_sleep(SSL_HANDSHAKE_POLL_MS); } ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r, APLOGNO() "Renegotiation loop %d iterations, " + "reneg_state=%d, " "in_init=%d, init_finished=%d, " "state=%s, sslconn->ssl=%s, peer_certs=%s", - i, SSL_in_init(ssl), SSL_is_init_finished(ssl), + i, sslconn->reneg_state, + SSL_in_init(ssl), SSL_is_init_finished(ssl), SSL_state_string_long(ssl), sslconn->ssl != NULL ? "yes" : "no", SSL_get_peer_certificate(ssl) != NULL ? "yes" : "no"); @@ -2142,6 +2146,18 @@ void ssl_callback_Info(const SSL *ssl, i } #endif } +#if OPENSSL_VERSION_NUMBER >= 0x1000L Is this correct or should it be 0x1010L instead? Regards Rüdiger +else if ((where & SSL_CB_HANDSHAKE_START) && scr->reneg_state == RENEG_ALLOW) { +scr->reneg_state = RENEG_STARTED; +} +else if ((where & SSL_CB_HANDSHAKE_DONE) && scr->reneg_state == RENEG_STARTED) { +scr->reneg_state = RENEG_DONE; +} +else if ((where & SSL_CB_ALERT) && + (scr->reneg_state == RENEG_ALLOW || scr->reneg_state == RENEG_STARTED)) { +scr->reneg_state = RENEG_ALERT; +} +#endif /* If the first handshake is complete, change state to reject any * subsequent client-initiated renegotiation. */ else if ((where & SSL_CB_HANDSHAKE_DONE) && scr->reneg_state == RENEG_INIT) { Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=1729927=1729926=1729927=diff == --- httpd/httpd/trunk/modules/ssl/ssl_private.h (original) +++ httpd/httpd/trunk/modules/ssl/ssl_private.h Fri Feb 12 00:44:22 2016 @@ -444,12 +444,17 @@ typedef struct { * partial fix for CVE-2009-3555. */ enum { RENEG_INIT = 0, /* Before initial handshake */ -RENEG_REJECT, /* After initial handshake; any client-initiated - * renegotiation should be rejected */ -RENEG_ALLOW, /* A server-initiated renegotiation is taking - * place (as dictated by configuration) */ -RENEG_ABORT /* Renegotiation initiated by client, abort the - * connection */ +RENEG_REJECT, /* After initial handshake; any client-initiated + * renegotiation should be rejected */ +RENEG_ALLOW,/* A server-initiated renegotiation is taking + * place (as dictated by configuration) */ +#if OPENSSL_VERSION_NUMBER >= 0x1000L Same question as above +RENEG_STARTED, /* A renegotiation has started after RENEG_ALLOW */ +RENEG_DONE, /* A renegotiation has finished after RENEG_STARTED */ +RENEG_ALERT,/* A renegotiation has finished with an SSL Alert */ +#endif +RENEG_ABORT /* Renegotiation initiated by client, abort the + * connection */ } reneg_state; server_rec *server; Regards Rüdiger