Re: ssl renegotiate

2016-02-09 Thread Rainer Jung

Am 09.02.2016 um 19:20 schrieb Stefan Eissing:

Ah, closer look revealed that the first test was a cipher renegotiation using 
HTTP/1.1. That no longer works, but the slave connection checks do. So, false 
alarm on that front. Will disable the renegotiation tests that fail for now 
until the 1.1.0 openssl work is done...

Sorry for the confusion.


Am 09.02.2016 um 19:11 schrieb Stefan Eissing :

With the new renegotiate code, I get failures in trunk for my tests that expect 
renegotiation to fail on slave connections. Rainer, not sure how this works 
now. Can you have a look?


No problem, thanks for doing tests as well. Yes, the cipher change reneg 
is still expected to fail (only when using OpenSSL 1.1.0).


Regards,

Rainer


Re: balancer-manager docs

2016-02-09 Thread Luca Toscano
Hi Jim,

2016-02-09 21:25 GMT+01:00 Jim Jagielski :

> The more I think of it, a HowTo guide, like the ones we have
> for public_html, Authn, etc regarding reverse proxy
> makes the most sense...
>
> > On Feb 9, 2016, at 12:55 PM, Tim Bannister 
> wrote:
> >
> > On 9 Feb 2016, at 16:02, Rainer Jung  wrote:
> > The module pages can document the module; I think that's appropriate for
> reference documentation.
> >
> > What's missing is more of a “how do I set up X” guide. I think the
> topics could be:
> >
> > • forward proxy (and access control) with or without cacheing
> > • reverse proxy with or without cacheing
> > • balancing and high availability for reverse proxies
> >
> > I think this is me volunteering to at least draft some text, if people
> agree this approach makes sense.
>

+1 for the howto guides suggested by Tim and separate pages for each module
for whoever is interested to dive deep into options.

Ideally the howto summary page (https://httpd.apache.org/docs/trunk/howto/)
could be linked to https://httpd.apache.org/download.cgi as "suggested
list" of things to bookmark for whoever is interested in httpd.

Also I volunteer for helping writing/reviewing the new docs if needed :)

Luca


APLOGNO() in mod_rewrite

2016-02-09 Thread Yehuda Katz
I noticed today that errors about invalid flags on rewrite rules do not
have APLOGNO() on them.
cmd_rewriterule calls cmd_rewriterule_setflag and if a string is returned,
prefixes "RewriteRule: " and returns that as an error.

Should these have APLOGNO()? They are errors, but they don't
use ap_log_rerror.
If those have APLOGNO() added, should each possible flag error have a
different one or are all flag errors the same and the code should be added
before the "RewriteRule:" prefix?

- Y


Re: balancer-manager docs

2016-02-09 Thread Jim Jagielski
The more I think of it, a HowTo guide, like the ones we have
for public_html, Authn, etc regarding reverse proxy
makes the most sense...

> On Feb 9, 2016, at 12:55 PM, Tim Bannister  wrote:
> 
> On 9 Feb 2016, at 16:02, Rainer Jung  wrote:
>> Am 09.02.2016 um 13:25 schrieb Jim Jagielski:
>>> We currently have really really little info about the balancer-manager in 
>>> our docs, just a short little blurb on how to enable it and a brief 
>>> description of what it does [1]. I'd like to extend that, but does it make 
>>> sense to add it to the mod_proxy_balancer module page, or have a separate 
>>> page dedicated to it which we can link to?
>>> 
>>> 1. https://httpd.apache.org/docs/trunk/mod/mod_proxy_balancer.html
>> 
>> Adding even more questions:
>> 
>> I always think it is confusing for newbies, that all configuration 
>> directives for any mod_proxy_* are documented on the mod_proxy page. 
>> Although this reflects the code, config is done by mod_proxy, it is not what 
>> a user would expect. If e.g. He is working with a balancer, he would expect 
>> more info about how to configure a balancer in the mod_proxy_balancer page.
> 
> Cc: to docs@
> 
> The module pages can document the module; I think that's appropriate for 
> reference documentation.
> 
> What's missing is more of a “how do I set up X” guide. I think the topics 
> could be:
> 
> • forward proxy (and access control) with or without cacheing
> • reverse proxy with or without cacheing
> • balancing and high availability for reverse proxies
> 
> I think this is me volunteering to at least draft some text, if people agree 
> this approach makes sense.
> 
> -- 
> Tim Bannister – is...@c8h10n4o2.org.uk
> 



Re: svn commit: r1726233 - in /httpd/httpd/trunk: docs/manual/mod/core.xml include/http_core.h server/core.c

2016-02-09 Thread Yann Ylavic
On Fri, Jan 22, 2016 at 4:30 PM,   wrote:
> Author: covener
> Date: Fri Jan 22 15:30:19 2016
> New Revision: 1726233
>
> URL: http://svn.apache.org/viewvc?rev=1726233=rev
> Log:
> from feedback, assume all parameters to SetHandler are expressions.
>
> Modified:
> httpd/httpd/trunk/server/core.c
>
[]
> Modified: httpd/httpd/trunk/server/core.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1726233=1726232=1726233=diff
> ==
> --- httpd/httpd/trunk/server/core.c (original)
> +++ httpd/httpd/trunk/server/core.c Fri Jan 22 15:30:19 2016
[]
> @@ -4668,7 +4655,13 @@ static int core_override_type(request_re
>  return HTTP_INTERNAL_SERVER_ERROR;
>  }
>  if (strcmp(val, "none")) {
> -r->handler = apr_pstrdup(r->pool, val);
> +if (val != ap_strstr_c(val, "proxy:unix")) {
> +/* Retained for compatibility --  but not for UDS */
> +char *tmp = apr_pstrdup(r->pool, val);
> +ap_str_tolower(tmp);
> +r->handler = tmp;
> +}
> +r->handler = val;

Missing an "else"?

>  }
>  }
>  else if (conf->handler && strcmp(conf->handler, "none")) {
>
>


Re: mod_proxy_http2

2016-02-09 Thread Stefan Eissing

> Am 09.02.2016 um 00:38 schrieb Gregg Smith :
> 
>> On 2/8/2016 9:07 AM, Stefan Eissing wrote:
>> PS. I did not update Windows Makefiles. I feel bad.
> 
> Don't, I need to play catch-up anyway :)
> 
> Should this be in modules/proxy like all the rest of the mod_proxy_* modules?
> I personally do not care, I was just thinking (which in itself can be 
> dangerous).

There is a tight coupling to mod_http2 and both rely on nghttp2 lib. Since the 
exact interfaces between both mods are evolving that place fits best. 

Thanks for taking care of the Makefiles!

Stefan

Support for OpenSSL 1.1.0

2016-02-09 Thread Rainer Jung

I started adding support for OpenSSL 1.1.0 in trunk.

As some might know, the OpenSSL API changes and especially many 
structures have been made opaque.


I resolved all the stuff that could be done on a local/formal level, but 
some items remain, where I'm not sure how to proceed. I have marked all 
of them with "XXX: OpenSSL 1.1.0:" in the sources.


I tested a straight backport to 2.4 against OpenSSL 1.1.0pre2 plus patch 
https://github.com/openssl/openssl/commit/311f27852a18fb9c10f0c1283b639f12eea06de2 
and there were only 7 ssl test failures.


Open problems:

1) HTTP on HTTPS

OpenSSL 1.1.0 currently doesn't support the "HTTP spoken on HTTPS port" 
error. The code to detect HTTP was removed due to a major rewrite of the 
state engine. The OpenSSL project is willing to review patches for 
reintroducing the feature there but currently doesn't plan to work on it 
themselves.


2) Renegotiation

It needs to be implemented differently. The OpenSSL project suggest to 
try reading application data until the renegotiation has finished. I 
committed some rather ugly code that does loop waiting for reneg, but it 
has a couple of problems:


a) it will not work for EC or DH ciphers. Some opaque structure element 
in the ssl struct is already set and confuses the state machine. I hope 
to get some helpful feedback from the OpenSSL project for this.


b) the loop impl currently assumes we wait for client certs during the 
reneg. It will hang for the full loop duration when only the cipher 
changed but no certs will be send. We need a better loop end check.


3) ssl_engine_ocsp.c

In modssl_verify_ocsp() the following code accesses the struct member 
"valid", for which currently no accessor function exists in 1.1.0:


268 else if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
269 /* don't do OCSP checking for valid self-issued certs */
270 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
271   "Skipping OCSP check for valid self-issued cert");
272 X509_STORE_CTX_set_error(ctx, X509_V_OK);
273 return 1;
274 }


4) ssl_util_stapling.c

In ssl_stapling_init_cert() there's a compiler warning:
  "passing argument 1 of 'sk_value' from incompatible pointer type
   expected 'const struct _STACK *' but argument is of type
   'struct stack_st_OPENSSL_STRING *'":

179cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0));


5) ssl_engine_kernel.c

In ssl_callback_Info() the explicit state constants 
SSL3_ST_SR_CLNT_HELLO_A and SSL23_ST_SR_CLNT_HELLO_A are used which no 
longer exist. I can't find obvious replacements in the list of new state 
constants:


2100 int state = SSL_get_state((SSL *)ssl);
2101
2102 if (state == SSL3_ST_SR_CLNT_HELLO_A
2103 || state == SSL23_ST_SR_CLNT_HELLO_A) {
2104 scr->reneg_state = RENEG_ABORT;
2105 ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(02042)
2106   "rejecting client initiated renegotiation");
2107 }


Regards,

Rainer



Re: Support for OpenSSL 1.1.0

2016-02-09 Thread Dr Stephen Henson
On 09/02/2016 14:36, Rainer Jung wrote:
> Hi Steve,
> 
> thanks a lot for your review and comments. More inline.
> 
> Am 09.02.2016 um 13:34 schrieb Dr Stephen Henson:
>> On 09/02/2016 10:20, Rainer Jung wrote:
>>>
>>> 3) ssl_engine_ocsp.c
>>>
>>> In modssl_verify_ocsp() the following code accesses the struct member 
>>> "valid",
>>> for which currently no accessor function exists in 1.1.0:
>>>
>>> 268 else if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
>>> 269 /* don't do OCSP checking for valid self-issued certs */
>>> 270 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
>>> 271   "Skipping OCSP check for valid self-issued cert");
>>> 272 X509_STORE_CTX_set_error(ctx, X509_V_OK);
>>> 273 return 1;
>>> 274 }
>>>
>>>
>>
>> It's not clear what that code is supposed to do. The check isn't for
>> "self-issued" because that would just require comparing the subject and 
>> issuer
>> names it is actually checking for a self signed certificate.
> 
>> Is it supposed to be skipping OCSP checking for a trusted root?
> 
> The svn log message says "Don't do OCSP checks for valid self-issued certs". 
> The
> change was discussed here
> 
> http://marc.info/?t=12921462952=1=2
> 
> with some older discussion here
> 
> http://marc.info/?t=11963686385=1=2
> 
> As far as I get it, it is meant as an optimization to skip OCSP in cases where
> it isn't needed or useful. But I'm far from being an expert here. Kaspar, who
> introduced it originally formulated "prevents mod_ssl from doing unnecessary
> OCSP checks (valid self-issued certs, i.e. trust anchors configured through
> SSLCACertificate{File,Path})".
> 
> I'll CC Kaspar directly.
> 

OK it looks like at that point the certificate has been verified anyway so you
don't gain anything by checking cert->valid.

Steve.
-- 
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shen...@opensslfoundation.com


Re: Support for OpenSSL 1.1.0

2016-02-09 Thread Rainer Jung

Hi Steve,

thanks a lot for your review and comments. More inline.

Am 09.02.2016 um 13:34 schrieb Dr Stephen Henson:

On 09/02/2016 10:20, Rainer Jung wrote:


3) ssl_engine_ocsp.c

In modssl_verify_ocsp() the following code accesses the struct member "valid",
for which currently no accessor function exists in 1.1.0:

268 else if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
269 /* don't do OCSP checking for valid self-issued certs */
270 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
271   "Skipping OCSP check for valid self-issued cert");
272 X509_STORE_CTX_set_error(ctx, X509_V_OK);
273 return 1;
274 }




It's not clear what that code is supposed to do. The check isn't for
"self-issued" because that would just require comparing the subject and issuer
names it is actually checking for a self signed certificate.



Is it supposed to be skipping OCSP checking for a trusted root?


The svn log message says "Don't do OCSP checks for valid self-issued 
certs". The change was discussed here


http://marc.info/?t=12921462952=1=2

with some older discussion here

http://marc.info/?t=11963686385=1=2

As far as I get it, it is meant as an optimization to skip OCSP in cases 
where it isn't needed or useful. But I'm far from being an expert here. 
Kaspar, who introduced it originally formulated "prevents mod_ssl from 
doing unnecessary OCSP checks (valid self-issued certs, i.e. trust 
anchors configured through SSLCACertificate{File,Path})".


I'll CC Kaspar directly.


4) ssl_util_stapling.c

In ssl_stapling_init_cert() there's a compiler warning:
   "passing argument 1 of 'sk_value' from incompatible pointer type
expected 'const struct _STACK *' but argument is of type
'struct stack_st_OPENSSL_STRING *'":

179cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0));



In ssl_private.h the checks like this:

#ifndef sk_OPENSSL_STRING_value
#define sk_OPENSSL_STRING_value sk_value
#endif

no longer work because stacks are now inline functions. If you put that block
round an appropriate #ifdef it should be fine.

I had a quick look at the changes and did notice that some of the direct
structure access (extensions, X509_NAME) is unnecessary in existing versions of
OpenSSL. So in some cases you don't need to only use them for 1.1: they'll work
in all versions of OpenSSL but it's only in 1.1 they are enforced.


I'll get this tested/included. I kept the direct structure accesses for 
pre-1.1.0 just for the moment to stay on the safe side. Once this works 
reasonably well, I'll clean up the code to reduce the version dependent 
ifdefs.


Getting closer :)

Regards,

Rainer


Re: mod_proxy_http2

2016-02-09 Thread Yann Ylavic
On Mon, Feb 8, 2016 at 6:07 PM, Stefan Eissing
 wrote:
>
> One thing: the ssl_hostname that is used for SNI by the generic proxy utils 
> seems to get lost when the socket needs to reset and is then not available on 
> the next connect. That should affect mod_proxy_http as far as I can tell. 
> Maybe someone with more experience in that module wants to take a look.

This is intended (for http/1 at least), why is it an issue?
When mod_proxy closes the backend connection, it indeed clears any
associated SNI.
But it sets the SNI for any new connection, based on the Host
requested on that connection.
Keep in mind that with "ProxyPreserveHost on" the requested Host may
be the one given by the client, which may hence differ for each
request (mod_proxy won't reuse a connection with a different SNI than
the Host it requests).


Re: mod_proxy_http2

2016-02-09 Thread Yann Ylavic
On Mon, Feb 8, 2016 at 6:07 PM, Stefan Eissing
 wrote:
> FYI: I just checked in a very experimental mod_proxy_http2 that registers on 
> h2:// and h2c:// proxy URLs. I did this naming to have the module totally 
> separate from mod_proxy_http, not wanting to make a mess.

Does mod_proxy_http2 (like mod_http2) still generates an HTTP/1
request run through the "usual" hooks?
Can a module/hook/filter (eg. rewrite, headers, security, ...) still
see/modify/refuse the inner HTTP/1?


Re: ABI report

2016-02-09 Thread Jeff Trawick
On Mon, Feb 8, 2016 at 12:57 PM, William A Rowe Jr 
wrote:

> This is excellent, thanks for the effort!
>
> You should note that there was no binary compatibility between 2.2.x final
> and 2.4.x.  And there will be no binary compatibility between 2.next (3.0?)
> and 2.4.x.  The interesting branches to compare for 2.2.next and 2.4.next
> to anticipate any binary breakage before we release would be in subversion
> under the paths;
>
> http://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x
> http://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x
>
> while 2.next (3.0) with binary-breaking changes lives at the path;
> http://svn.apache.org/repos/asf/httpd/httpd/trunk
>
> This looks like a great tool, much appreciated :)
>
> Bill
>
>
> On Sun, Feb 7, 2016 at 2:32 AM, Ponomarenko Andrey <
> andrewponomare...@yandex.ru> wrote:
>
>> Hello,
>>
>> I've started to maintain binary compatibility report for the recent
>> versions of the httpd: http://abi-laboratory.pro/tracker/timeline/httpd/
>>
>> The report is updated every other day. The report for the latest version
>> from git is also there. Hope this helps Linux maintainers to be aware of
>> ABI changes and added/removed symbols.
>>
>> Thank you.
>>
>
>
Perhaps there could be a way to configure a message related to the intended
breakage between 2.2.last and 2.4.first, since that is interesting from an
API migration standpoint.  (Link to
https://httpd.apache.org/docs/2.4/developer/new_api_2_4.html)


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


balancer-manager docs

2016-02-09 Thread Jim Jagielski
We currently have really really little info about the balancer-
manager in our docs, just a short little blurb on how to enable
it and a brief description of what it does [1]. I'd like to extend
that, but does it make sense to add it to the mod_proxy_balancer
module page, or have a separate page dedicated to it which we can
link to?


1. https://httpd.apache.org/docs/trunk/mod/mod_proxy_balancer.html


Re: ABI report

2016-02-09 Thread Ponomarenko Andrey
Hello Bill, I'll try to add 2.2.x and 2.4.x branches to the compatibility table soon (the 2.2.x on top of all 2.2.* versions and 2.4.x on top of all 2.4.*). But it requires some time to improve the code base. Thank you. 08.02.2016, 21:20, "William A Rowe Jr":This is excellent, thanks for the effort! You should note that there was no binary compatibility between 2.2.x finaland 2.4.x.  And there will be no binary compatibility between 2.next (3.0?)and 2.4.x.  The interesting branches to compare for 2.2.next and 2.4.nextto anticipate any binary breakage before we release would be in subversion under the paths; http://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.xhttp://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x while 2.next (3.0) with binary-breaking changes lives at the path;http://svn.apache.org/repos/asf/httpd/httpd/trunk This looks like a great tool, much appreciated :) Bill On Sun, Feb 7, 2016 at 2:32 AM, Ponomarenko Andrey wrote:Hello,  I've started to maintain binary compatibility report for the recent versions of the httpd: http://abi-laboratory.pro/tracker/timeline/httpd/  The report is updated every other day. The report for the latest version from git is also there. Hope this helps Linux maintainers to be aware of ABI changes and added/removed symbols.  Thank you.

Re: Support for OpenSSL 1.1.0

2016-02-09 Thread Dr Stephen Henson
On 09/02/2016 10:20, Rainer Jung wrote:
> 
> 3) ssl_engine_ocsp.c
> 
> In modssl_verify_ocsp() the following code accesses the struct member "valid",
> for which currently no accessor function exists in 1.1.0:
> 
> 268 else if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
> 269 /* don't do OCSP checking for valid self-issued certs */
> 270 ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
> 271   "Skipping OCSP check for valid self-issued cert");
> 272 X509_STORE_CTX_set_error(ctx, X509_V_OK);
> 273 return 1;
> 274 }
> 
> 

It's not clear what that code is supposed to do. The check isn't for
"self-issued" because that would just require comparing the subject and issuer
names it is actually checking for a self signed certificate.

Is it supposed to be skipping OCSP checking for a trusted root?

> 4) ssl_util_stapling.c
> 
> In ssl_stapling_init_cert() there's a compiler warning:
>   "passing argument 1 of 'sk_value' from incompatible pointer type
>expected 'const struct _STACK *' but argument is of type
>'struct stack_st_OPENSSL_STRING *'":
> 
> 179cinf->uri = apr_pstrdup(p, sk_OPENSSL_STRING_value(aia, 0));
>

In ssl_private.h the checks like this:

#ifndef sk_OPENSSL_STRING_value
#define sk_OPENSSL_STRING_value sk_value
#endif

no longer work because stacks are now inline functions. If you put that block
round an appropriate #ifdef it should be fine.

I had a quick look at the changes and did notice that some of the direct
structure access (extensions, X509_NAME) is unnecessary in existing versions of
OpenSSL. So in some cases you don't need to only use them for 1.1: they'll work
in all versions of OpenSSL but it's only in 1.1 they are enforced.

Steve.
-- 
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shen...@opensslfoundation.com


Re: ABI report

2016-02-09 Thread Ponomarenko Andrey
09.02.2016, 15:02, "Jeff Trawick" :On Mon, Feb 8, 2016 at 12:57 PM, William A Rowe Jr  wrote:This is excellent, thanks for the effort! You should note that there was no binary compatibility between 2.2.x finaland 2.4.x.  And there will be no binary compatibility between 2.next (3.0?)and 2.4.x.  The interesting branches to compare for 2.2.next and 2.4.nextto anticipate any binary breakage before we release would be in subversion under the paths; http://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.xhttp://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x while 2.next (3.0) with binary-breaking changes lives at the path;http://svn.apache.org/repos/asf/httpd/httpd/trunk This looks like a great tool, much appreciated :) Bill On Sun, Feb 7, 2016 at 2:32 AM, Ponomarenko Andrey  wrote:Hello,  I've started to maintain binary compatibility report for the recent versions of the httpd: http://abi-laboratory.pro/tracker/timeline/httpd/  The report is updated every other day. The report for the latest version from git is also there. Hope this helps Linux maintainers to be aware of ABI changes and added/removed symbols.  Thank you. Perhaps there could be a way to configure a message related to the intended breakage between 2.2.last and 2.4.first, since that is interesting from an API migration standpoint.  (Link to https://httpd.apache.org/docs/2.4/developer/new_api_2_4.html)  Interesting idea! I'll implement it too. Thank you.

Re: balancer-manager docs

2016-02-09 Thread Rainer Jung

Am 09.02.2016 um 13:25 schrieb Jim Jagielski:

We currently have really really little info about the balancer-
manager in our docs, just a short little blurb on how to enable
it and a brief description of what it does [1]. I'd like to extend
that, but does it make sense to add it to the mod_proxy_balancer
module page, or have a separate page dedicated to it which we can
link to?


1. https://httpd.apache.org/docs/trunk/mod/mod_proxy_balancer.html


Adding even more questions:

I always think it is confusing for newbies, that all configuration 
directives for any mod_proxy_* are documented on the mod_proxy page. 
Although this reflects the code, config is done by mod_proxy, it is not 
what a user would expect. If e.g. He is working with a balancer, he 
would expect more info about how to configure a balancer in the 
mod_proxy_balancer page.


So I wonder whether proxy is an example, where the config reference 
should move to the docs page, for the module that is actually being 
configured, not the module that handles the configuration. I guess 
that's not possible without some redundancy and it's not an easy call to 
decide, what should go to the central mod_proxy page, and what to 
individual mod_proxy_xyz pages.


There are other cases as well (ajp, fcgi, ...).

Regards,

Rainer



Negative mod_proxy ping_timeout..

2016-02-09 Thread Yann Ylavic
.. is meant to do a simple TCP readability check on the connection,
using ap_proxy_is_socket_connected().
This is trunk only AFAICT.

However all our proxy modules handle this just after calling
ap_proxy_connect_backend() which already calls
ap_proxy_is_socket_connected() for the same purpose.

Couldn't we simply remove this ping_timeout < 0 code (eg. attached patch)?

Regards,
Yann.
Index: modules/proxy/mod_proxy.c
===
--- modules/proxy/mod_proxy.c	(revision 1729388)
+++ modules/proxy/mod_proxy.c	(working copy)
@@ -280,7 +280,7 @@ static const char *set_worker_param(apr_pool_t *p,
  */
 if (ap_timeout_parameter_parse(val, , "s") != APR_SUCCESS)
 return "Ping/Pong timeout has wrong format";
-if (timeout < 1000 && timeout >= 0)
+if (timeout < 1000)
 return "Ping/Pong timeout must be at least one millisecond";
 worker->s->ping_timeout = timeout;
 worker->s->ping_timeout_set = 1;
Index: modules/proxy/mod_proxy.h
===
--- modules/proxy/mod_proxy.h	(revision 1729388)
+++ modules/proxy/mod_proxy.h	(working copy)
@@ -365,7 +365,6 @@ do { \
 
 #define PROXY_DO_100_CONTINUE(w, r) \
 ((w)->s->ping_timeout_set \
- && ((w)->s->ping_timeout >= 0) \
  && (PROXYREQ_REVERSE == (r)->proxyreq) \
  && !(apr_table_get((r)->subprocess_env, "force-proxy-request-1.0")) \
  && ap_request_has_body((r)))
Index: modules/proxy/mod_proxy_ajp.c
===
--- modules/proxy/mod_proxy_ajp.c	(revision 1729388)
+++ modules/proxy/mod_proxy_ajp.c	(working copy)
@@ -342,8 +342,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
  * but doesn't affect the whole worker.
  */
 if (APR_STATUS_IS_TIMEUP(status) &&
-conn->worker->s->ping_timeout_set &&
-conn->worker->s->ping_timeout >= 0) {
+conn->worker->s->ping_timeout_set) {
 return HTTP_GATEWAY_TIME_OUT;
 }
 
@@ -680,8 +679,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
  * but doesn't affect the whole worker.
  */
 if (APR_STATUS_IS_TIMEUP(status) &&
-conn->worker->s->ping_timeout_set &&
-conn->worker->s->ping_timeout >= 0) {
+conn->worker->s->ping_timeout_set) {
 apr_table_setn(r->notes, "proxy_timedout", "1");
 rv = HTTP_GATEWAY_TIME_OUT;
 }
@@ -791,36 +789,23 @@ static int proxy_ajp_handler(request_rec *r, proxy
 
 /* Handle CPING/CPONG */
 if (worker->s->ping_timeout_set) {
-if (worker->s->ping_timeout < 0) {
-if (!ap_proxy_is_socket_connected(backend->sock)) {
-backend->close = 1;
-ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(02534)
-  "socket check failed to %pI (%s)",
-  worker->cp->addr, worker->s->hostname);
-status = HTTP_SERVICE_UNAVAILABLE;
-retry++;
-continue;
-}
+status = ajp_handle_cping_cpong(backend->sock, r,
+worker->s->ping_timeout);
+/*
+ * In case the CPING / CPONG failed for the first time we might be
+ * just out of luck and got a faulty backend connection, but the
+ * backend might be healthy nevertheless. So ensure that the backend
+ * TCP connection gets closed and try it once again.
+ */
+if (status != APR_SUCCESS) {
+backend->close = 1;
+ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897)
+  "cping/cpong failed to %pI (%s)",
+  worker->cp->addr, worker->s->hostname);
+status = HTTP_SERVICE_UNAVAILABLE;
+retry++;
+continue;
 }
-else {
-status = ajp_handle_cping_cpong(backend->sock, r,
-worker->s->ping_timeout);
-/*
- * In case the CPING / CPONG failed for the first time we might be
- * just out of luck and got a faulty backend connection, but the
- * backend might be healthy nevertheless. So ensure that the backend
- * TCP connection gets closed and try it once again.
- */
-if (status != APR_SUCCESS) {
-backend->close = 1;
-ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897)
-  "cping/cpong failed to %pI (%s)",
-  worker->cp->addr, 

Re: Negative mod_proxy ping_timeout..

2016-02-09 Thread Jim Jagielski
Why the change to modules/proxy/mod_proxy_hcheck.c?

> On Feb 9, 2016, at 10:58 AM, Yann Ylavic  wrote:
> 
> 



Re: mod_proxy_http2

2016-02-09 Thread Stefan Eissing
I had the effect that when a socket was determined to be dead, the SNI was 
cleared and a new connection was made without any SNI. So, I save the first 
ssl_hostname I see and set that on every new connection.

> Am 09.02.2016 um 15:42 schrieb Yann Ylavic :
> 
> On Mon, Feb 8, 2016 at 6:07 PM, Stefan Eissing
>  wrote:
>> 
>> One thing: the ssl_hostname that is used for SNI by the generic proxy utils 
>> seems to get lost when the socket needs to reset and is then not available 
>> on the next connect. That should affect mod_proxy_http as far as I can tell. 
>> Maybe someone with more experience in that module wants to take a look.
> 
> This is intended (for http/1 at least), why is it an issue?
> When mod_proxy closes the backend connection, it indeed clears any
> associated SNI.
> But it sets the SNI for any new connection, based on the Host
> requested on that connection.
> Keep in mind that with "ProxyPreserveHost on" the requested Host may
> be the one given by the client, which may hence differ for each
> request (mod_proxy won't reuse a connection with a different SNI than
> the Host it requests).



Re: Negative mod_proxy ping_timeout..

2016-02-09 Thread Yann Ylavic
On Tue, Feb 9, 2016 at 5:42 PM, Jim Jagielski  wrote:
> Why the change to modules/proxy/mod_proxy_hcheck.c?

The call to ap_proxy_connect_backend() just above already calls
ap_proxy_is_socket_connected() for any reused connection, so why check
it twice?

If the connection is not reused but a new one created instead, the
check is also pointless IMHO.


Re: mod_proxy_http2

2016-02-09 Thread Stefan Eissing
For the request itself, yes. mod_proxy_http2 follows the same pattern as 
mod_proxy_http. It does not setup a faked request_rec that mod_proxy_http does 
since I did not need it and did not see the point. Might have missed something.

> Am 09.02.2016 um 15:51 schrieb Yann Ylavic :
> 
> On Mon, Feb 8, 2016 at 6:07 PM, Stefan Eissing
>  wrote:
>> FYI: I just checked in a very experimental mod_proxy_http2 that registers on 
>> h2:// and h2c:// proxy URLs. I did this naming to have the module totally 
>> separate from mod_proxy_http, not wanting to make a mess.
> 
> Does mod_proxy_http2 (like mod_http2) still generates an HTTP/1
> request run through the "usual" hooks?
> Can a module/hook/filter (eg. rewrite, headers, security, ...) still
> see/modify/refuse the inner HTTP/1?



Re: Proposal for a new mod_event documentation page

2016-02-09 Thread Luca Toscano
After some feedbacks I updated the trunk documentation:

https://httpd.apache.org/docs/trunk/mod/event.html

Please let me know if you like the new page and the new information
contained. In case, I'll backport the change to 2.4.x, otherwise I'll
revert.

Thanks!

Luca


2016-02-08 15:32 GMT+01:00 Luca Toscano :

> +dev for visibility
> -- Messaggio inoltrato --
> Da: "Luca Toscano" 
> Data: 06 feb 2016 15:10
> Oggetto: Proposal for a new mod_event documentation page
> A: 
> Cc:
>
> Hi Apache Devs!
>
> I started an email thread some days ago on the dev mailing list to gather
> information about mod_event, getting tons of interesting things to read. I
> attached to this email a draft of how the new page should look like in my
> opinion, please let me know your thoughts!
>
> Notes about the changes:
>
> - clear statement that event shares all the configuration directives with
> worker, together with the fact that the number of threads is regulated by
> processes multiplied by threads per child.
>
> - explanation of the new async connections fields in mod_status.
>
> - tried to add all the information about keep alives, writing and closing
> connections that I've gathered so far. I added a disclaimer for the writing
> ones and a big section called "limitations", but I have probably failed to
> describe all the closing ones (not only lingering closes I suppose, but I
> wasn't sure).
>
> - clear logic connection between how it works and the
> "AsyncRequestWorkerFactor" directive section (I am referring to the
> sentence "The event MPM handles some connections in an asynchronous way").
>
> Apologies for technical inaccuracies and/or typos!
>
> Luca
>


Re: ssl renegotiate

2016-02-09 Thread Stefan Eissing
Testing in trunk, 2.4.x seems to be fine. It's the httpd/test/mod_h2/trunk test 
cases (do not expect you to get that running). I will take a closer look 
tomorrow as there is more fishy than just the renegotiation. I see more 
failures than that. I will try an earlier mod_ssl tomorrow and try to narrow it 
down.

Cheers,

Stefan

> Am 09.02.2016 um 21:47 schrieb Rainer Jung :
> 
> Am 09.02.2016 um 20:03 schrieb Stefan Eissing:
>> 
>>> Am 09.02.2016 um 19:58 schrieb Rainer Jung :
>>> 
>>> Am 09.02.2016 um 19:20 schrieb Stefan Eissing:
 Ah, closer look revealed that the first test was a cipher renegotiation 
 using HTTP/1.1. That no longer works, but the slave connection checks do. 
 So, false alarm on that front. Will disable the renegotiation tests that 
 fail for now until the 1.1.0 openssl work is done...
 
 Sorry for the confusion.
 
> Am 09.02.2016 um 19:11 schrieb Stefan Eissing 
> :
> 
> With the new renegotiate code, I get failures in trunk for my tests that 
> expect renegotiation to fail on slave connections. Rainer, not sure how 
> this works now. Can you have a look?
>>> 
>>> No problem, thanks for doing tests as well. Yes, the cipher change reneg is 
>>> still expected to fail (only when using OpenSSL 1.1.0).
>> 
>> Was using OpenSSL 1.0.2 in my tests...
> 
> That's strange then, because I didn't (intentionally) change the behavior for 
> pre 1.1.0. Instead I tried to keep the code the same in that case and 
> postpone any cleanups (let pre-1.1.0 use the same code as 1.1.0) to the time 
> 1.1.0 runs fine.
> 
> So could it be something else? Is it a test case which is part of the test 
> suite? If yes, which one? Which version (trunk or 2.4) did you test. I would 
> try to reproduce here.
> 
> Regards,
> 
> Rainer
> 



Re: "httpd -X" segfaults with 2.4.17

2016-02-09 Thread Brian J. France
This hasn't made it into the 2.4.x branch yet, what is the status of getting 
this in the .19 release?

http://svn.apache.org/viewvc?view=revision=1711479

Thanks,

Brian


> On Oct 16, 2015, at 8:06 AM, Yann Ylavic  wrote:
> 
> Hi Jan,
> 
> On Fri, Oct 16, 2015 at 1:58 PM, Jan Kaluža  wrote:
>> Hi,
>> 
>> httpd 2.4.17 segfaults when used with prefork MPM (and probably also with
>> other MPMs) and -X option since r1705492.
>> 
>> The crash happens in the following call in prefork.c (and probably also
>> worker.c and so on):
>> 
>>ap_mpm_pod_check(my_bucket->pod)
>> 
>> pod is NULL and later dereferenced.
>> 
>> The pod is NULL because of the following:
>> 
>>if (!one_process && /* no POD in one_process mode */
>>(rv = ap_mpm_pod_open(pconf, _buckets[i].pod))) {
> 
> This is a bad copy-and-paste from the worker/event logic, which I'm to
> blame for...
> I think we should simply remove the !one_process check here.
> 
> Regards,
> Yann.
> 



Re: ssl renegotiate

2016-02-09 Thread Rainer Jung

Am 09.02.2016 um 20:03 schrieb Stefan Eissing:



Am 09.02.2016 um 19:58 schrieb Rainer Jung :

Am 09.02.2016 um 19:20 schrieb Stefan Eissing:

Ah, closer look revealed that the first test was a cipher renegotiation using 
HTTP/1.1. That no longer works, but the slave connection checks do. So, false 
alarm on that front. Will disable the renegotiation tests that fail for now 
until the 1.1.0 openssl work is done...

Sorry for the confusion.


Am 09.02.2016 um 19:11 schrieb Stefan Eissing :

With the new renegotiate code, I get failures in trunk for my tests that expect 
renegotiation to fail on slave connections. Rainer, not sure how this works 
now. Can you have a look?


No problem, thanks for doing tests as well. Yes, the cipher change reneg is 
still expected to fail (only when using OpenSSL 1.1.0).


Was using OpenSSL 1.0.2 in my tests...


That's strange then, because I didn't (intentionally) change the 
behavior for pre 1.1.0. Instead I tried to keep the code the same in 
that case and postpone any cleanups (let pre-1.1.0 use the same code as 
1.1.0) to the time 1.1.0 runs fine.


So could it be something else? Is it a test case which is part of the 
test suite? If yes, which one? Which version (trunk or 2.4) did you 
test. I would try to reproduce here.


Regards,

Rainer



Re: ssl renegotiate

2016-02-09 Thread Stefan Eissing

> Am 09.02.2016 um 19:58 schrieb Rainer Jung :
> 
> Am 09.02.2016 um 19:20 schrieb Stefan Eissing:
>> Ah, closer look revealed that the first test was a cipher renegotiation 
>> using HTTP/1.1. That no longer works, but the slave connection checks do. 
>> So, false alarm on that front. Will disable the renegotiation tests that 
>> fail for now until the 1.1.0 openssl work is done...
>> 
>> Sorry for the confusion.
>> 
>>> Am 09.02.2016 um 19:11 schrieb Stefan Eissing 
>>> :
>>> 
>>> With the new renegotiate code, I get failures in trunk for my tests that 
>>> expect renegotiation to fail on slave connections. Rainer, not sure how 
>>> this works now. Can you have a look?
> 
> No problem, thanks for doing tests as well. Yes, the cipher change reneg is 
> still expected to fail (only when using OpenSSL 1.1.0).

Was using OpenSSL 1.0.2 in my tests...

ssl renegotiate

2016-02-09 Thread Stefan Eissing
With the new renegotiate code, I get failures in trunk for my tests that expect 
renegotiation to fail on slave connections. Rainer, not sure how this works 
now. Can you have a look?

-Stefan

Re: ssl renegotiate

2016-02-09 Thread Stefan Eissing
Ah, closer look revealed that the first test was a cipher renegotiation using 
HTTP/1.1. That no longer works, but the slave connection checks do. So, false 
alarm on that front. Will disable the renegotiation tests that fail for now 
until the 1.1.0 openssl work is done...

Sorry for the confusion.

> Am 09.02.2016 um 19:11 schrieb Stefan Eissing :
> 
> With the new renegotiate code, I get failures in trunk for my tests that 
> expect renegotiation to fail on slave connections. Rainer, not sure how this 
> works now. Can you have a look?
> 
> -Stefan



Re: Negative mod_proxy ping_timeout..

2016-02-09 Thread Jim Jagielski

> On Feb 9, 2016, at 11:49 AM, Yann Ylavic  wrote:
> 
> On Tue, Feb 9, 2016 at 5:42 PM, Jim Jagielski  wrote:
>> Why the change to modules/proxy/mod_proxy_hcheck.c?
> 
> The call to ap_proxy_connect_backend() just above already calls
> ap_proxy_is_socket_connected() for any reused connection, so why check
> it twice?
> 

I recall a discussion somewhat long ago where we also
do a specific check after paths where we "just checked"
and the reasoning was to avoid race conditions, esp in
situations where we created but then it gets immediately killed.

I am sure there are other places where we do the same,
likely w/ comments. :)

> If the connection is not reused but a new one created instead, the
> check is also pointless IMHO.



Re: balancer-manager docs

2016-02-09 Thread Tim Bannister
On 9 Feb 2016, at 16:02, Rainer Jung  wrote:
> Am 09.02.2016 um 13:25 schrieb Jim Jagielski:
>> We currently have really really little info about the balancer-manager in 
>> our docs, just a short little blurb on how to enable it and a brief 
>> description of what it does [1]. I'd like to extend that, but does it make 
>> sense to add it to the mod_proxy_balancer module page, or have a separate 
>> page dedicated to it which we can link to?
>> 
>> 1. https://httpd.apache.org/docs/trunk/mod/mod_proxy_balancer.html
> 
> Adding even more questions:
> 
> I always think it is confusing for newbies, that all configuration directives 
> for any mod_proxy_* are documented on the mod_proxy page. Although this 
> reflects the code, config is done by mod_proxy, it is not what a user would 
> expect. If e.g. He is working with a balancer, he would expect more info 
> about how to configure a balancer in the mod_proxy_balancer page.

Cc: to docs@

The module pages can document the module; I think that's appropriate for 
reference documentation.

What's missing is more of a “how do I set up X” guide. I think the topics could 
be:

• forward proxy (and access control) with or without cacheing
• reverse proxy with or without cacheing
• balancing and high availability for reverse proxies

I think this is me volunteering to at least draft some text, if people agree 
this approach makes sense.

-- 
Tim Bannister – is...@c8h10n4o2.org.uk