You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Kaspar Brand <ht...@velox.ch> on 2014/04/21 10:01:15 UTC

Re: svn commit: r1587607 - in /httpd/httpd/trunk: ./ include/ modules/ssl/

On 15.04.2014 17:25, trawick@apache.org wrote:
> Author: trawick
> Date: Tue Apr 15 15:25:03 2014
> New Revision: 1587607
> 
> URL: http://svn.apache.org/r1587607
> Log:
> mod_ssl: Add hooks to allow other modules to perform processing at
> several stages of initialization and connection handling.  See
> mod_ssl_openssl.h.
> 
> This is enough to allow implementation of Certificate Transparency
> outside of mod_ssl.
> 

[...]

> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=1587607&r1=1587606&r2=1587607&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Tue Apr 15 15:25:03 2014
> @@ -29,8 +29,13 @@
>                                              -- Unknown    */
>  #include "ssl_private.h"
>  #include "mod_ssl.h"
> +#include "mod_ssl_openssl.h"
>  #include "apr_date.h"
>  
> +APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(ssl, SSL, int, proxy_post_handshake,
> +                                    (conn_rec *c,SSL *ssl),
> +                                    (c,ssl),OK,DECLINED);
> +
>  /*  _________________________________________________________________
>  **
>  **  I/O Hooks
> @@ -1119,6 +1124,8 @@ static apr_status_t ssl_io_filter_handsh
>          const char *hostname_note = apr_table_get(c->notes,
>                                                    "proxy-request-hostname");
>          BOOL proxy_ssl_check_peer_ok = TRUE;
> +        int post_handshake_rc;
> +
>          sc = mySrvConfig(server);
>  
>  #ifdef HAVE_TLSEXT
> @@ -1208,11 +1215,17 @@ static apr_status_t ssl_io_filter_handsh
>              }
>          }
>  
> +        if (proxy_ssl_check_peer_ok == TRUE) {
> +            /* another chance to fail */
> +            post_handshake_rc = ssl_run_proxy_post_handshake(c, filter_ctx->pssl);
> +        }
> +
>          if (cert) {
>              X509_free(cert);
>          }
>  
> -        if (proxy_ssl_check_peer_ok != TRUE) {
> +        if (proxy_ssl_check_peer_ok != TRUE
> +            || (post_handshake_rc != OK && post_handshake_rc != DECLINED)) {
>              /* ensure that the SSL structures etc are freed, etc: */
>              ssl_filter_io_shutdown(filter_ctx, c, 1);
>              apr_table_setn(c->notes, "SSL_connect_rv", "err");

GCC warns about "'post_handshake_rc' may be used uninitialized in this
function"... which would be true for the "proxy_ssl_check_peer_ok !=
TRUE" case (line 1228), I think.

Kaspar

Re: svn commit: r1587607 - in /httpd/httpd/trunk: ./ include/ modules/ssl/

Posted by Jeff Trawick <tr...@gmail.com>.
On Mon, Apr 21, 2014 at 4:01 AM, Kaspar Brand <ht...@velox.ch>wrote:

> On 15.04.2014 17:25, trawick@apache.org wrote:
> > Author: trawick
> > Date: Tue Apr 15 15:25:03 2014
> > New Revision: 1587607
> >
> > URL: http://svn.apache.org/r1587607
> > Log:
> > mod_ssl: Add hooks to allow other modules to perform processing at
> > several stages of initialization and connection handling.  See
> > mod_ssl_openssl.h.
> >
> > This is enough to allow implementation of Certificate Transparency
> > outside of mod_ssl.
> >
>
> [...]
>
> > Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_io.c?rev=1587607&r1=1587606&r2=1587607&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
> > +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Tue Apr 15 15:25:03
> 2014
> > @@ -29,8 +29,13 @@
> >                                              -- Unknown    */
> >  #include "ssl_private.h"
> >  #include "mod_ssl.h"
> > +#include "mod_ssl_openssl.h"
> >  #include "apr_date.h"
> >
> > +APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(ssl, SSL, int, proxy_post_handshake,
> > +                                    (conn_rec *c,SSL *ssl),
> > +                                    (c,ssl),OK,DECLINED);
> > +
> >  /*  _________________________________________________________________
> >  **
> >  **  I/O Hooks
> > @@ -1119,6 +1124,8 @@ static apr_status_t ssl_io_filter_handsh
> >          const char *hostname_note = apr_table_get(c->notes,
> >
>  "proxy-request-hostname");
> >          BOOL proxy_ssl_check_peer_ok = TRUE;
> > +        int post_handshake_rc;
> > +
> >          sc = mySrvConfig(server);
> >
> >  #ifdef HAVE_TLSEXT
> > @@ -1208,11 +1215,17 @@ static apr_status_t ssl_io_filter_handsh
> >              }
> >          }
> >
> > +        if (proxy_ssl_check_peer_ok == TRUE) {
> > +            /* another chance to fail */
> > +            post_handshake_rc = ssl_run_proxy_post_handshake(c,
> filter_ctx->pssl);
> > +        }
> > +
> >          if (cert) {
> >              X509_free(cert);
> >          }
> >
> > -        if (proxy_ssl_check_peer_ok != TRUE) {
> > +        if (proxy_ssl_check_peer_ok != TRUE
> > +            || (post_handshake_rc != OK && post_handshake_rc !=
> DECLINED)) {
> >              /* ensure that the SSL structures etc are freed, etc: */
> >              ssl_filter_io_shutdown(filter_ctx, c, 1);
> >              apr_table_setn(c->notes, "SSL_connect_rv", "err");
>
> GCC warns about "'post_handshake_rc' may be used uninitialized in this
> function"... which would be true for the "proxy_ssl_check_peer_ok !=
> TRUE" case (line 1228), I think.
>
> Kaspar
>

Fixed in r1588868.

FWIW, post_handshake_rc wouldn't be referenced in the case where it was
uninitialized (proxy_ssl_check_peer_ok != TRUE), but it is nice to keep
more levels of gcc quiet.  (My gcc 4.8.1 from Ubuntu doesn't say anything.)

Thanks!

-- 
Born in Roswell... married an alien...
http://emptyhammock.com/
http://edjective.org/