You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@httpd.apache.org by GitBox <gi...@apache.org> on 2021/07/06 12:10:54 UTC
[GitHub] [httpd] icing opened a new pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
icing opened a new pull request #203:
URL: https://github.com/apache/httpd/pull/203
merge of r1890605 from apache svn:
*) core/mod_proxy/mod_ssl:
Adding `outgoing` flag to conn_rec, indicating a connection is
initiated by the server to somewhere, in contrast to incoming
connections from clients.
Adding 'ap_ssl_bind_outgoing()` function that marks a connection
as outgoing and is used by mod_proxy instead of the previous
optional function `ssl_engine_set`. This enables other SSL
module to secure proxy connections.
The optional functions `ssl_engine_set`, `ssl_engine_disable` and
`ssl_proxy_enable` are now provided by the core to have backward
compatibility with non-httpd modules that might use them. mod_ssl
itself no longer registers these functions, but keeps them in its
header for backward compatibility.
The core provided optional function wrap any registered function
like it was done for `ssl_is_ssl`.
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1890605 13f79535-47bb-0310-9956-ffa450edef68
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] rpluem commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
rpluem commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r665335854
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (_hooks.link_ssl_bind_outgoing && _hooks.link_ssl_bind_outgoing->nelts > 0)
Review comment:
Shouldn't we use `ap_hook_get_ssl_bind_outgoing()`` instead of `_hooks.link_ssl_bind_outgoing`?
##########
File path: include/http_ssl.h
##########
@@ -49,6 +51,40 @@ AP_DECLARE_HOOK(int,ssl_conn_is_ssl,(conn_rec *c))
*/
AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c);
+/**
+ * This hook declares a connection to be outgoing and the configuration that applies to it.
+ * This hook can be called several times in the lifetime of an outgoing connection, e.g.
+ * when it is re-used in different request contexts. It will at least be called after the
+ * connection was created and before the pre-connection hooks is invoked.
+ * All outgoing-connection hooks are run until one returns something other than ok or decline.
+ * if enable_ssl != 0, a hook that sets up SSL for the connection needs to return DONE.
+ *
+ * @param c The connection on which requests/data are to be sent.
+ * @param dir_conf The directory configuration in which this connection is being used.
+ * @param enable_ssl If != 0, the SSL protocol should be enabled for this connection.
+ * @return OK or DECLINED, DONE when ssl was enabled
+ */
Review comment:
First of all sorry for missing this on #190.
This is a run first hook. Hence it stops when the hook participant returns anything else but DECLINED.
1. What should be the difference between DONE and OK then? OK says I did not enable SSL, but I wanted to stop the hook?
2. The hook implementation in mod_ssl returns OK and not done when it enabled SSL. And `ap_ssl_bind_outgoing` checks for `OK` and not `DONE`
I think the fix is to adjust the documentation (`OK` signals enabled, `DECLINED` signals nothing done) here and leave the code as is.
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
Review comment:
Maybe I miss the point, but this looks like the same condition as on line 136. Can't we do it at the end of the if block starting in line 136?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on pull request #203:
URL: https://github.com/apache/httpd/pull/203#issuecomment-916690968
merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] rpluem commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
rpluem commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r665335854
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (_hooks.link_ssl_bind_outgoing && _hooks.link_ssl_bind_outgoing->nelts > 0)
Review comment:
Shouldn't we use `ap_hook_get_ssl_bind_outgoing()` instead of `_hooks.link_ssl_bind_outgoing`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r704164310
##########
File path: include/http_ssl.h
##########
@@ -49,6 +51,40 @@ AP_DECLARE_HOOK(int,ssl_conn_is_ssl,(conn_rec *c))
*/
AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c);
+/**
+ * This hook declares a connection to be outgoing and the configuration that applies to it.
+ * This hook can be called several times in the lifetime of an outgoing connection, e.g.
+ * when it is re-used in different request contexts. It will at least be called after the
+ * connection was created and before the pre-connection hooks is invoked.
+ * All outgoing-connection hooks are run until one returns something other than ok or decline.
+ * if enable_ssl != 0, a hook that sets up SSL for the connection needs to return DONE.
+ *
+ * @param c The connection on which requests/data are to be sent.
+ * @param dir_conf The directory configuration in which this connection is being used.
+ * @param enable_ssl If != 0, the SSL protocol should be enabled for this connection.
+ * @return OK or DECLINED, DONE when ssl was enabled
+ */
Review comment:
Resolved by update, I think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r705080413
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (_hooks.link_ssl_bind_outgoing && _hooks.link_ssl_bind_outgoing->nelts > 0)
Review comment:
Thanks for the explanation. I changed the code in the recent update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r703301092
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
Review comment:
This checks if the old optional functions that might be present from another ssl module have enabled ssl or not. So, this could happen at the end of the if from line 136, but I preferred a catch at the end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing closed pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing closed pull request #203:
URL: https://github.com/apache/httpd/pull/203
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r705412937
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (ap_hook_get_ssl_bind_outgoing() && ap_hook_get_ssl_bind_outgoing()->nelts > 0)
+ || module_ssl_engine_set || module_ssl_proxy_enable;
Review comment:
Looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] rpluem commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
rpluem commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r665326791
##########
File path: include/http_ssl.h
##########
@@ -49,6 +51,40 @@ AP_DECLARE_HOOK(int,ssl_conn_is_ssl,(conn_rec *c))
*/
AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c);
+/**
+ * This hook declares a connection to be outgoing and the configuration that applies to it.
+ * This hook can be called several times in the lifetime of an outgoing connection, e.g.
+ * when it is re-used in different request contexts. It will at least be called after the
+ * connection was created and before the pre-connection hooks is invoked.
+ * All outgoing-connection hooks are run until one returns something other than ok or decline.
+ * if enable_ssl != 0, a hook that sets up SSL for the connection needs to return DONE.
+ *
+ * @param c The connection on which requests/data are to be sent.
+ * @param dir_conf The directory configuration in which this connection is being used.
+ * @param enable_ssl If != 0, the SSL protocol should be enabled for this connection.
+ * @return OK or DECLINED, DONE when ssl was enabled
+ */
Review comment:
First of all sorry for missing this on #190.
This is a run first hook. Hence it stops when the hook participant returns anything else but DECLINED.
1. What should be the difference between `DONE` and `OK` then? `OK` says I did not enable SSL, but I wanted to stop the hook?
2. The hook implementation in mod_ssl (`ssl_hook_ssl_bind_outgoing`) returns `OK` and not done when it enabled SSL. And `ap_ssl_bind_outgoing` checks for `OK` and not `DONE`
I think the fix is to adjust the documentation (`OK` signals enabled, `DECLINED` signals nothing done) here and leave the code as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r703304311
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (_hooks.link_ssl_bind_outgoing && _hooks.link_ssl_bind_outgoing->nelts > 0)
Review comment:
There are checks in mod_proxy and friends on a NULL connection to see if ssl can be possibly enabled at all or if not ssl module is present. Is there a better way to check if any callbacks have registered at the hook?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] rpluem commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
rpluem commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r665326791
##########
File path: include/http_ssl.h
##########
@@ -49,6 +51,40 @@ AP_DECLARE_HOOK(int,ssl_conn_is_ssl,(conn_rec *c))
*/
AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c);
+/**
+ * This hook declares a connection to be outgoing and the configuration that applies to it.
+ * This hook can be called several times in the lifetime of an outgoing connection, e.g.
+ * when it is re-used in different request contexts. It will at least be called after the
+ * connection was created and before the pre-connection hooks is invoked.
+ * All outgoing-connection hooks are run until one returns something other than ok or decline.
+ * if enable_ssl != 0, a hook that sets up SSL for the connection needs to return DONE.
+ *
+ * @param c The connection on which requests/data are to be sent.
+ * @param dir_conf The directory configuration in which this connection is being used.
+ * @param enable_ssl If != 0, the SSL protocol should be enabled for this connection.
+ * @return OK or DECLINED, DONE when ssl was enabled
+ */
Review comment:
First of all sorry for missing this on #190.
This is a run first hook. Hence it stops when the hook participant returns anything else but DECLINED.
1. What should be the difference between `DONE` and `OK` then? `OK` says I did not enable SSL, but I wanted to stop the hook?
2. The hook implementation in mod_ssl (`ssl_hook_ssl_bind_outgoing`) returns `OK` and not `DONE` when it enabled SSL. And `ap_ssl_bind_outgoing` checks for `OK` and not `DONE`
I think the fix is to adjust the documentation (`OK` signals enabled, `DECLINED` signals nothing done) here and leave the code as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r703298575
##########
File path: include/http_ssl.h
##########
@@ -49,6 +51,40 @@ AP_DECLARE_HOOK(int,ssl_conn_is_ssl,(conn_rec *c))
*/
AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c);
+/**
+ * This hook declares a connection to be outgoing and the configuration that applies to it.
+ * This hook can be called several times in the lifetime of an outgoing connection, e.g.
+ * when it is re-used in different request contexts. It will at least be called after the
+ * connection was created and before the pre-connection hooks is invoked.
+ * All outgoing-connection hooks are run until one returns something other than ok or decline.
+ * if enable_ssl != 0, a hook that sets up SSL for the connection needs to return DONE.
+ *
+ * @param c The connection on which requests/data are to be sent.
+ * @param dir_conf The directory configuration in which this connection is being used.
+ * @param enable_ssl If != 0, the SSL protocol should be enabled for this connection.
+ * @return OK or DECLINED, DONE when ssl was enabled
+ */
Review comment:
You are right. I will update the documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r705416015
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (ap_hook_get_ssl_bind_outgoing() && ap_hook_get_ssl_bind_outgoing()->nelts > 0)
+ || module_ssl_engine_set || module_ssl_proxy_enable;
Review comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] rpluem commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
rpluem commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r704410297
##########
File path: include/http_ssl.h
##########
@@ -49,6 +51,40 @@ AP_DECLARE_HOOK(int,ssl_conn_is_ssl,(conn_rec *c))
*/
AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c);
+/**
+ * This hook declares a connection to be outgoing and the configuration that applies to it.
+ * This hook can be called several times in the lifetime of an outgoing connection, e.g.
+ * when it is re-used in different request contexts. It will at least be called after the
+ * connection was created and before the pre-connection hooks is invoked.
+ * All outgoing-connection hooks are run until one returns something other than ok or decline.
+ * if enable_ssl != 0, a hook that sets up SSL for the connection needs to return DONE.
+ *
+ * @param c The connection on which requests/data are to be sent.
+ * @param dir_conf The directory configuration in which this connection is being used.
+ * @param enable_ssl If != 0, the SSL protocol should be enabled for this connection.
+ * @return OK or DECLINED, DONE when ssl was enabled
+ */
Review comment:
Thanks. It is resolved for the backport, but the old documentation is still in trunk
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (_hooks.link_ssl_bind_outgoing && _hooks.link_ssl_bind_outgoing->nelts > 0)
Review comment:
`ap_hook_get_ssl_bind_outgoing()` returns `_hooks.link_ssl_bind_outgoing`, but it avoids using the internal hook data structures directly. Instead we use a function that is part of the API. I admit that for `_hooks.link_ssl_bind_outgoing->nelts` `ap_hook_get_ssl_bind_outgoing()->nelts` would still use internal hook data structures, but we have no API for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] icing commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
icing commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r705413775
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (ap_hook_get_ssl_bind_outgoing() && ap_hook_get_ssl_bind_outgoing()->nelts > 0)
+ || module_ssl_engine_set || module_ssl_proxy_enable;
Review comment:
adding it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] ylavic commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
ylavic commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r705411735
##########
File path: server/ssl.c
##########
@@ -85,6 +96,77 @@ AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c)
return r;
}
+static int ssl_engine_set(conn_rec *c,
+ ap_conf_vector_t *per_dir_config,
+ int proxy, int enable)
+{
+ if (proxy) {
+ return ap_ssl_bind_outgoing(c, per_dir_config, enable) == OK;
+ }
+ else if (module_ssl_engine_set) {
+ return module_ssl_engine_set(c, per_dir_config, 0, enable);
+ }
+ else if (enable && module_ssl_proxy_enable) {
+ return module_ssl_proxy_enable(c);
+ }
+ else if (!enable && module_ssl_engine_disable) {
+ return module_ssl_engine_disable(c);
+ }
+ return 0;
+}
+
+static int ssl_proxy_enable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 1);
+}
+
+static int ssl_engine_disable(conn_rec *c)
+{
+ return ap_ssl_bind_outgoing(c, NULL, 0);
+}
+
+AP_DECLARE(int) ap_ssl_bind_outgoing(conn_rec *c, struct ap_conf_vector_t *dir_conf,
+ int enable_ssl)
+{
+ int rv, enabled = 0;
+
+ c->outgoing = 1;
+ rv = ap_run_ssl_bind_outgoing(c, dir_conf, enable_ssl);
+ enabled = (rv == OK);
+ if (enable_ssl && !enabled) {
+ /* the hooks did not take over. Is there an old skool optional that will? */
+ if (module_ssl_engine_set) {
+ enabled = module_ssl_engine_set(c, dir_conf, 1, 1);
+ }
+ else if (module_ssl_proxy_enable) {
+ enabled = module_ssl_proxy_enable(c);
+ }
+ }
+ else {
+ /* !enable_ssl || enabled
+ * any existing optional funcs need to not enable here */
+ if (module_ssl_engine_set) {
+ module_ssl_engine_set(c, dir_conf, 1, 0);
+ }
+ else if (module_ssl_engine_disable) {
+ module_ssl_engine_disable(c);
+ }
+ }
+ if (enable_ssl && !enabled) {
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0,
+ c, APLOGNO(01961) " failed to enable ssl support "
+ "[Hint: if using mod_ssl, see SSLProxyEngine]");
+ return DECLINED;
+ }
+ return OK;
+}
+
+AP_DECLARE(int) ap_ssl_has_outgoing_handlers(void)
+{
+ return (ap_hook_get_ssl_bind_outgoing() && ap_hook_get_ssl_bind_outgoing()->nelts > 0)
+ || module_ssl_engine_set || module_ssl_proxy_enable;
Review comment:
```
apr_array_header_t *hooks = ap_hook_get_ssl_bind_outgoing();
return (hooks && hooks->nelts > 0)
|| module_ssl_engine_set || module_ssl_proxy_enable;
```
to avoid 2 function calls that the compiler won't optimize?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org
[GitHub] [httpd] rpluem commented on a change in pull request #203: new ap_ssl_bind_outgoing for multi ssl support in proxy connections
Posted by GitBox <gi...@apache.org>.
rpluem commented on a change in pull request #203:
URL: https://github.com/apache/httpd/pull/203#discussion_r665326791
##########
File path: include/http_ssl.h
##########
@@ -49,6 +51,40 @@ AP_DECLARE_HOOK(int,ssl_conn_is_ssl,(conn_rec *c))
*/
AP_DECLARE(int) ap_ssl_conn_is_ssl(conn_rec *c);
+/**
+ * This hook declares a connection to be outgoing and the configuration that applies to it.
+ * This hook can be called several times in the lifetime of an outgoing connection, e.g.
+ * when it is re-used in different request contexts. It will at least be called after the
+ * connection was created and before the pre-connection hooks is invoked.
+ * All outgoing-connection hooks are run until one returns something other than ok or decline.
+ * if enable_ssl != 0, a hook that sets up SSL for the connection needs to return DONE.
+ *
+ * @param c The connection on which requests/data are to be sent.
+ * @param dir_conf The directory configuration in which this connection is being used.
+ * @param enable_ssl If != 0, the SSL protocol should be enabled for this connection.
+ * @return OK or DECLINED, DONE when ssl was enabled
+ */
Review comment:
First of all sorry for missing this on #190.
This is a run first hook. Hence it stops when the hook participant returns anything else but DECLINED.
1. What should be the difference between `DONE` and `OK` then? `OK` says I did not enable SSL, but I wanted to stop the hook?
2. The hook implementation in mod_ssl (`ssl_hook_ssl_bind_outgoing`) returns `OK` and not `DONE` when it enabled SSL. And `ap_ssl_bind_outgoing` checks for `OK` and not `DONE`
I think the fix is to adjust the documentation (`OK` signals enabled, `DECLINED` signals nothing done) here and leave the code as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org