You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2016/04/26 11:00:01 UTC
Re: svn commit: r1740928 - in /httpd/httpd/trunk: ./ include/
modules/http2/ modules/proxy/ modules/ssl/ server/
On 04/26/2016 02:04 AM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Tue Apr 26 00:04:57 2016
> New Revision: 1740928
>
> URL: http://svn.apache.org/viewvc?rev=1740928&view=rev
> Log:
> mod_proxy, mod_ssl: Handle SSLProxy* directives in <Proxy> sections,
> allowing different TLS configurations per backend.
>
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/include/http_config.h
> httpd/httpd/trunk/modules/http2/h2_h2.c
> httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
> httpd/httpd/trunk/modules/proxy/mod_proxy.c
> httpd/httpd/trunk/modules/proxy/mod_proxy.h
> httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
> httpd/httpd/trunk/modules/proxy/proxy_util.c
> httpd/httpd/trunk/modules/ssl/mod_ssl.c
> httpd/httpd/trunk/modules/ssl/mod_ssl.h
> httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
> httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
> httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
> httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
> httpd/httpd/trunk/modules/ssl/ssl_private.h
> httpd/httpd/trunk/server/config.c
> httpd/httpd/trunk/server/core.c
>
> Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=1740928&r1=1740927&r2=1740928&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
> +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Tue Apr 26 00:04:57 2016
> @@ -561,34 +605,21 @@ static apr_port_t ssl_hook_default_port(
>
> static int ssl_hook_pre_connection(conn_rec *c, void *csd)
> {
> -
> SSLSrvConfigRec *sc;
> SSLConnRec *sslconn = myConnConfig(c);
>
> - if (sslconn) {
> - sc = mySrvConfig(sslconn->server);
> - }
> - else {
> - sc = mySrvConfig(c->base_server);
> - }
> /*
> * Immediately stop processing if SSL is disabled for this connection
> */
> - if (c->master || !(sc && (sc->enabled == SSL_ENABLED_TRUE ||
> - (sslconn && sslconn->is_proxy))))
> - {
> + if (ssl_engine_status(c, sslconn) != OK) {
> return DECLINED;
> }
>
> - /*
> - * Create SSL context
> - */
> - if (!sslconn) {
> - sslconn = ssl_init_connection_ctx(c);
> + if (sslconn) {
> + sc = mySrvConfig(sslconn->server);
> }
> -
> - if (sslconn->disabled) {
> - return DECLINED;
> + else {
> + sc = mySrvConfig(c->base_server);
> }
We have a change in behaviour here. We no longer guarantee that we have an sslconn created and connected to c if SSL is
enabled. Is this intended?
>
> /*
Regards
Rüdiger
Re: svn commit: r1740928 - in /httpd/httpd/trunk: ./ include/
modules/http2/ modules/proxy/ modules/ssl/ server/
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Aug 19, 2016 at 5:49 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> Additional issues since this patch in our maintainer mode...
>
> httpd-2.x/modules/ssl/ssl_engine_config.c: In function
> 'ssl_cmd_SSLVerifyClient':
> httpd-2.x/modules/ssl/ssl_engine_config.c:1083:27: warning: 'mode' may be
> used uninitialized in this function [-Wmaybe-uninitialized]
> dc->nVerifyClient = mode;
> ^
> httpd-2.x/modules/ssl/ssl_engine_config.c: In function
> 'ssl_cmd_SSLProxyVerify':
> httpd-2.x/modules/ssl/ssl_engine_config.c:1463:33: warning: 'mode' may be
> used uninitialized in this function [-Wmaybe-uninitialized]
> dc->proxy->auth.verify_mode = mode;
> ^
> You might assert that ssl_cmd_verify_parse always populates the third arg
> except in case of error, you didn't satisfy the compiler of this fact ;-)
How couldn't it figure out that apr_pstrcat() never returns NULL?
Clever compilers should really read all the docs :)
Anyway, "fixed" in r1756976, thanks!
Re: svn commit: r1740928 - in /httpd/httpd/trunk: ./ include/
modules/http2/ modules/proxy/ modules/ssl/ server/
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
Additional issues since this patch in our maintainer mode...
httpd-2.x/modules/ssl/ssl_engine_config.c: In function
'ssl_cmd_SSLVerifyClient':
httpd-2.x/modules/ssl/ssl_engine_config.c:1083:27: warning: 'mode' may be
used uninitialized in this function [-Wmaybe-uninitialized]
dc->nVerifyClient = mode;
^
httpd-2.x/modules/ssl/ssl_engine_config.c: In function
'ssl_cmd_SSLProxyVerify':
httpd-2.x/modules/ssl/ssl_engine_config.c:1463:33: warning: 'mode' may be
used uninitialized in this function [-Wmaybe-uninitialized]
dc->proxy->auth.verify_mode = mode;
^
You might assert that ssl_cmd_verify_parse always populates the third arg
except in case of error, you didn't satisfy the compiler of this fact ;-)
On Tue, Apr 26, 2016 at 4:46 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Apr 26, 2016 at 11:00 AM, Ruediger Pluem <rp...@apache.org>
> wrote:
> >
> > On 04/26/2016 02:04 AM, ylavic@apache.org wrote:
> >> static int ssl_hook_pre_connection(conn_rec *c, void *csd)
> >> {
> >> -
> >> SSLSrvConfigRec *sc;
> >> SSLConnRec *sslconn = myConnConfig(c);
> >>
> >> - if (sslconn) {
> >> - sc = mySrvConfig(sslconn->server);
> >> - }
> >> - else {
> >> - sc = mySrvConfig(c->base_server);
> >> - }
> >> /*
> >> * Immediately stop processing if SSL is disabled for this
> connection
> >> */
> >> - if (c->master || !(sc && (sc->enabled == SSL_ENABLED_TRUE ||
> >> - (sslconn && sslconn->is_proxy))))
> >> - {
> >> + if (ssl_engine_status(c, sslconn) != OK) {
> >> return DECLINED;
> >> }
> >>
> >> - /*
> >> - * Create SSL context
> >> - */
> >> - if (!sslconn) {
> >> - sslconn = ssl_init_connection_ctx(c);
> >> + if (sslconn) {
> >> + sc = mySrvConfig(sslconn->server);
> >> }
> >> -
> >> - if (sslconn->disabled) {
> >> - return DECLINED;
> >> + else {
> >> + sc = mySrvConfig(c->base_server);
> >> }
> >
> > We have a change in behaviour here. We no longer guarantee that we have
> an sslconn created and connected to c if SSL is
> > enabled. Is this intended?
>
> Actually ssl_init_connection_ctx(c) is done by
> ssl_init_ssl_connection() called just below (on return).
>
> Regards,
> Yann.
>
Re: svn commit: r1740928 - in /httpd/httpd/trunk: ./ include/
modules/http2/ modules/proxy/ modules/ssl/ server/
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Apr 26, 2016 at 11:00 AM, Ruediger Pluem <rp...@apache.org> wrote:
>
> On 04/26/2016 02:04 AM, ylavic@apache.org wrote:
>> static int ssl_hook_pre_connection(conn_rec *c, void *csd)
>> {
>> -
>> SSLSrvConfigRec *sc;
>> SSLConnRec *sslconn = myConnConfig(c);
>>
>> - if (sslconn) {
>> - sc = mySrvConfig(sslconn->server);
>> - }
>> - else {
>> - sc = mySrvConfig(c->base_server);
>> - }
>> /*
>> * Immediately stop processing if SSL is disabled for this connection
>> */
>> - if (c->master || !(sc && (sc->enabled == SSL_ENABLED_TRUE ||
>> - (sslconn && sslconn->is_proxy))))
>> - {
>> + if (ssl_engine_status(c, sslconn) != OK) {
>> return DECLINED;
>> }
>>
>> - /*
>> - * Create SSL context
>> - */
>> - if (!sslconn) {
>> - sslconn = ssl_init_connection_ctx(c);
>> + if (sslconn) {
>> + sc = mySrvConfig(sslconn->server);
>> }
>> -
>> - if (sslconn->disabled) {
>> - return DECLINED;
>> + else {
>> + sc = mySrvConfig(c->base_server);
>> }
>
> We have a change in behaviour here. We no longer guarantee that we have an sslconn created and connected to c if SSL is
> enabled. Is this intended?
Actually ssl_init_connection_ctx(c) is done by
ssl_init_ssl_connection() called just below (on return).
Regards,
Yann.