Re: svn commit: r1729927 - in /httpd/httpd/trunk/modules/ssl: ssl_engine_kernel.c ssl_private.h

2016-02-12 Thread 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


Re: svn commit: r1729927 - in /httpd/httpd/trunk/modules/ssl: ssl_engine_kernel.c ssl_private.h

2016-02-12 Thread Rainer Jung

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