Re: svn commit: r930125 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/mod_proxy_balancer.c

[Daniel Ruggeri]

On 8/19/2010 7:57 PM, Jeff Trawick wrote:
On Tue, Jul 20, 2010 at 10:59 AM, Daniel Ruggeri drugg...@primary.net 
mailto:drugg...@primary.net wrote:


On 7/16/2010 10:37 AM, Jeff Trawick wrote:

On Fri, Jul 16, 2010 at 11:27 AM, William A. Rowe Jr.
wr...@rowe-clan.net mailto:wr...@rowe-clan.net  wrote:

On 7/16/2010 9:35 AM, William A. Rowe Jr. wrote:

On 7/16/2010 6:47 AM, Jeff Trawick wrote:

We may as well leave it at erroronstatus  I
agree with Eric on
keeping your original choice and worrying about
the doc.

Can we drop the word error, since it has four
potential meanings?

backendfailureonstatus perhaps, or something along
those lines?

Hold up; see
http://tomcat.apache.org/connectors-doc/reference/workers.html
fail_on_status - why would we invent new phrases?

good catch



Sounds like this would be the best name. As promised, here is the
final patch including doc, the name failonstatus, and updates
made in trunk for this patch. Also included is an update to trunk
for the new name and doc.


Thanks; I added one tweak to the patch -- an s at the end of code 
in the following line:


+return erroronstatus must be one or more HTTP response code;

+}


For ease of reading, the doc patch reads thusly:
   A single or comma-separated list of HTTP status codes. If
set this will
   force the worker into error state when the backend returns
any status code
   in the list. Worker recovery behaves the same as other
worker errors.

Finally, here is my proposed change to STATUS to indicate
completion of Jeff's suggestions as well as update the location of
the 2.2.x patch:
--- STATUS  2010-07-20 09:53:36.298789200 -0500
+++ STATUS.mod  2010-07-20 09:54:15.466286400 -0500
@@ -145,13 +145,9 @@
statuses are found
PR: 48939
Trunk patch: http://svn.apache.org/viewvc?rev=930125view=rev
http://svn.apache.org/viewvc?rev=930125view=rev
-2.2.x patch:
https://issues.apache.org/bugzilla/attachment.cgi?id=25153
+2.2.x patch:
https://issues.apache.org/bugzilla/attachment.cgi?id=25788


Any chance you can update your 2.2.x patch in Bugzilla to include the 
tweaked wording (s/code/codes/)?   Sorry :)


Submitted by: Daniel Ruggeri DRuggeri primary.net
http://primary.net

+1: niq, jim
-trawick suggests:
-  1. somebody write doc (Daniel volunteers)
-  2. somebody create new patch which includes r962972, any
-  subsequent changes, and doc (Daniel volunteers)

  * mod_disk_cache: Decline the opportunity to cache if the
response is
a 206 Partial Content. This stops a reverse proxied partial
response


Thank you, all.
--
Daniel Ruggeri




Jeff;
   I'm not sure what happened - the 2.2 patch I submitted had a mixture 
between failonstatus and erroronstatus, but the trunk patch did not. 
I only noticed because of the message you posted about patching the 
patch and the update didn't seem right. In any event, I have updated 
Bugzilla and the new 2.2 patch can be found here:

https://issues.apache.org/bugzilla/attachment.cgi?id=25923

Sorry about the confusion.

Status update should look like this (new attachment ID included)
--- STATUS  2010-07-20 09:53:36.298789200 -0500
+++ STATUS.mod  2010-07-20 09:54:15.466286400 -0500
@@ -145,13 +145,9 @@
  statuses are found
  PR: 48939
  Trunk patch: http://svn.apache.org/viewvc?rev=930125view=rev
-2.2.x patch: https://issues.apache.org/bugzilla/attachment.cgi?id=25153
+2.2.x patch: https://issues.apache.org/bugzilla/attachment.cgi?id=25923
  Submitted by: Daniel RuggeriDRuggeri primary.net
  +1: niq, jim
-trawick suggests:
-  1. somebody write doc (Daniel volunteers)
-  2. somebody create new patch which includes r962972, any
-  subsequent changes, and doc (Daniel volunteers)

* mod_disk_cache: Decline the opportunity to cache if the response is
  a 206 Partial Content. This stops a reverse proxied partial response

Trying to drum up interest in this patch

[Daniel Ruggeri]

 All;
   With the talk about a 2.2.17 coming soon, I would very much like to 
get the remaining requisite votes and implementation of the patch (48939 
- in STATUS currently) I had submitted for inclusion. I know a lot of 
folks are rather busy these days, but I was hoping I could draw 
attention to this again in hopes of making the 2.2.17 release.


P.S.
   I would love to include details of this patch in my ApacheConNA 2010 
session as it helps address some of the shortfalls the intelligence 
shortfalls.

--
--
Daniel Ruggeri

Re: Trying to drum up interest in this patch

[Daniel Ruggeri]

 On 10/5/2010 8:56 PM, William A. Rowe Jr. wrote:

On 10/5/2010 5:41 PM, Daniel Ruggeri wrote:

  All;
With the talk about a 2.2.17 coming soon, I would very much like to get the 
remaining
requisite votes and implementation of the patch (48939 - in STATUS currently) I 
had
submitted for inclusion. I know a lot of folks are rather busy these days, but 
I was
hoping I could draw attention to this again in hopes of making the 2.2.17 
release.

P.S.
I would love to include details of this patch in my ApacheConNA 2010 
session as it
helps address some of the shortfalls the intelligence shortfalls.

Just as a suggestion, most of us don't memorize numbers (... for example, I 
can't
remember my own kids cell phone numbers, my phone does so for me.)

So when someone want eyeballs on an issue, please remind us the subject, and if
it is not too lengthy, attach the patch.  Consider that sometimes our chance to
react to your email is in the air, devoid of network access, and we are just
trying to plow through our email queue offline.

All that said, trawick, niq and wrowe have all reviewed this specific backport,
and it is in the queue to be applied to 2.2.


William;
   Understood - I was too busy repeating myself in the last sentence I 
didn't think to provide more details. I also must have misread STATUS 
when I checked on this the other day. Thank you for the response.


   On a different note, I recall you brought the topic up about worker 
acquiescence in a planned maintenance situation. I am not sure if folks 
had a chance to review what I brought up, but I have submitted a patch 
to do this. However, I would really prefer input on the patch as I am 
not 100% sure it is ready for proposal in STATUS. Also because, 
technically, one could set the redirect route for the worker and force 
its traffic elsewhere (works fine in a two node situation, but distorts 
load distribution if there are more).


Bug URL
https://issues.apache.org/bugzilla/show_bug.cgi?id=48841

Patch notes:
  I used a constant called PROXY_WORKER_NOLBFACTOR in mod_proxy.h and 
changed
the atoi call during configuration to strtol since atoi. I did this 
because the
atoi call returns 0 both during error situations and when the proper 
value to
return is 0. Also, the existing checks had to be refactored a little 
since (at
least on the SUN c compiler) an uninitialized integer is the same as 
`0'. Aside
from that, only the bybusiness algorithm had to be modified to avoid a 
divide

by zero error.

--
--
Daniel Ruggeri

Re: mod_proxy: optimising ProxyPass per directory

[Daniel Ruggeri]

On 10/20/2010 6:43 PM, Graham Leggett wrote:

Hi all,

On sites with large numbers of ProxyPass directives, these directives
are matched in turn on every request, and this can take a lot of time,
especially when we've already done a location walk.

A simple optimisation is to allow this existing syntax:

Location /foo
ProxyPass http://somewhere/foo
/Location

to simply be a single mod_proxy alias in a per-directory context.

If this syntax is used, the need to walk the proxy alias list is
eliminated, and a significant amount of time is saved.

This also has the side effect that ProxyPass inside LocationMatch starts
working properly (it was broken before):

LocationMatch ^/foo(.*)
ProxyPass http://somewhere/$1
/LocationMatch

In theory, the ProxyPass /foo http://somewhere/; and ProxyPassMatch
^/foo(.*) http://somewhere/$1; syntaxes can be deprecated, as
Location/LocationMatch is way simpler to handle.

Regards,
Graham
--



I like this idea quite a bit. I am not able to look at the codebase 
right now, but could this work the same for ProxyPassReverse?

--
Daniel Ruggeri

Re: mod_ssl's proxy support: make it per directory

[Daniel Ruggeri]

On 11/19/2010 9:13 AM, Graham Leggett wrote:

On 19 Nov 2010, at 3:15 PM, Plüm, Rüdiger, VF-Group wrote:


For a while, mod_ssl has been able to secure connections from
mod_proxy, backwards towards some backend server.

For some reason however, the directives that control this behavior
SSLProxy* are all scoped virtual host only, making it
possible to SSL
protect just one single ProxyPass going backwards, and not more than
one, something that severely limits the usefulness of the feature.


What limits do you see with the actual per virtual host configuration?


Most specifically, any attempt to set a client certificate to a
particular proxypass ends up being valid server wide.

Each backend server which a reverse proxy proxies to has the potential
to have different requirements for SSL, from client certs, to ciphers
used, etc.

We have worked around this to date by either delegating this task to
load balancers, or writing little php apps to proxy the connections, but
this is really ugly, when mod_proxy+mod_ssl can potentially do this itself.

Regards,
Graham
--



Indeed - this is a long standing limitation available in quite a few 
reverse proxies out there... and even several third party proxy modules 
for httpd.


--
Daniel Ruggeri

Re: Proposed: PKI Authentication for secure web access

[Daniel Ruggeri]

On 11/20/2010 2:39 PM, Rob Lemaster wrote:

Thanks for the link Issac. If this is already in Apache, why isn't
everyone using it?


On Sat, Nov 20, 2010 at 12:32 PM, Issac Goldstandmar...@beamartyr.net  wrote:


Nope, you have full x509 based authentication out-of-the-box.  See
http://httpd.apache.org/docs/2.2/ssl/ssl_howto.html#allclients

  Issac



For those who have a real security need to authenticate their clients in 
this way, and are willing to accept the hassles of this method, it is 
definitely used. However, the idea that a bank or paypal would issue 
certificates for each of its end users can get cumbersome very fast. 
See, the private key would be managed by the user. Users (and even some 
server administrators) are terribly poor at managing their private keys 
in a safe and secure fashion. Some potential complications are a user 
switching browsers, a user switching computers, a user's key becoming 
compromised, loss of the key, etc... On top of that, the signing 
institution would need to be able to keep track of certificates it 
should no longer accept via CRL's and have infrastructure ready to 
verify the cert is still valid.


Essentially, the logistics of getting END USERS to generate a key of 
appropriate size (and getting them to keep it safe), send a CSR, sign 
and return a certificate to them as well as the unavoidable technical 
support involved makes this an unattractive option to large institutions 
because the average Internet denizen isn't expected to know how to do 
this stuff The Right Way.


P.S.
IMHO, this conversation applies to PKI, X509 client authentication and 
even password authentication... all of these mechanisms boil down to the 
fact that there is some entity that knows who the user is and that your 
server will have to take a leap of faith at some point to trust that the 
user sitting at the keyboard is who they say they are.


--
Daniel Ruggeri

Removing passwords from the conf file

[Daniel Ruggeri]

In mod_ssl there is a very handy option of making an exec callout for 
SSLPassPhraseDialog rather than to put a password for your private key 
in the conf file. The obvious benefit here is that one can then design a 
solution to meet any arbitrary number of security challenges before 
allowing that password to be delivered.


One of my TODO patches is to add this same functionality in other 
places. The first that comes to mind (and something that has pestered me 
in the past) is AuthLDAPBindPassword (mod_authnz_ldap). Would anyone 
like to suggest other potential places this should be done before I put 
together a bug report and send in a patch?


P.S.
I am opposed to mod_ssl's check that the argument to SSLPassPhraseDialog 
exec:blah is a file. This prevents calling an arbitrary executable with 
parameters. Thoughts?


--
--
Daniel Ruggeri

Re: Removing passwords from the conf file

[Daniel Ruggeri]

On 11/21/2010 2:38 AM, Stefan Fritsch wrote:

On Sat, 20 Nov 2010, Daniel Ruggeri wrote:


In mod_ssl there is a very handy option of making an exec callout for
SSLPassPhraseDialog rather than to put a password for your private key
in the conf file. The obvious benefit here is that one can then design
a solution to meet any arbitrary number of security challenges before
allowing that password to be delivered.

One of my TODO patches is to add this same functionality in other
places. The first that comes to mind (and something that has pestered
me in the past) is AuthLDAPBindPassword (mod_authnz_ldap). Would
anyone like to suggest other potential places this should be done
before I put together a bug report and send in a patch?


Company policies that require passphrases not to be stored in plaintext
are not that uncommon. Therefore I agree that having a generic
functionality to be used by modules is a good thing.

But IMHO the documentation should be much clearer that this is only
security by obscurity and improves security only in some very limited
areas.
Understood and agreed - I will need to update documentation anyway to 
add this functionality and will be willing to drive this point home a 
little better.




An attacker who is root on the machine that is running HTTPD can still
get the ssl keys. Either by creating a core dump and extracting the keys
from that (there are tools that do this), or, if the passphrase dialog
yields the passphrases without human interaction, by starting HTTPD
under strace/truss.
Another very valid point - if an attacker has root, you are screwed in 
any event.




The only valid use case I see for this feature is to prevent unencrypted
ssl keys from going into the normal backup (if the file with the
passphrases is excluded and is instead backed up on paper). Are there
more valid use cases?
I stewed on this question for a while because I was *sure* I had a good 
answer somewhere in my gray matter... but no, I can't really come up 
with a solid response to this. If your server admins are putting the 
password somewhere safe and using proper file system permissions to 
prevent that information from being shared, it's all the same.


I suppose the ideas behind the policies you mention surround the fact 
that sometimes people will screw up file system permissions and users on 
the host for other purposes (content managers, appserver admins, etc) 
could see the password. Or, worse yet, your application is coerced into 
reading arbitrary files from the file system. With this in hand, it 
allows the security teams as much obscurity as they could fathom.


--
Daniel Ruggeri

Making mod_proxy_http more aware of SSL

[Daniel Ruggeri]

All;
   I opened up bug 50332 to attach/document these patches. The patch 
causes mod_ssl to create a note on the conn_req which is checked by 
mod_proxy_http when it attempts to pass the request. The intent is for 
mod_proxy_http to realize that an SSL handshake error has occurred and 
mark the worker out of service.


   This is a huge step forward in that mod_proxy will not be oblivious 
to the failed SSL connection and can take a worker out of service, 
however... it's not all roses. I don't know what it would take (or if 
it's even possible since mod_ssl and mod_proxy run in very separate 
filters), but it would be really great if mod_proxy in general were 
aware of handshake failures before it ever attempts to submit a request 
to the backend. I would envision this enlightenment to come at /* Step 
Two: Make the Connection */ in modules/proxy/mod_proxy_http.c.


Thoughts?


If the great minds of this mail list deem these patches acceptable, here 
is the proposed patch to 2.2 STATUS:

Index: httpd-2.2.x/STATUS
===
--- httpd-2.2.x/STATUS  (revision 1037345)
+++ httpd-2.2.x/STATUS  (working copy)
@@ -184,6 +184,14 @@
  enabling/disabling the basic capability is not split out into 
mod_unixd 2.2.x.

  +1: trawick

+   * mod_proxy_http: Become aware of ssl handshake failures when attempting
+ to pass request. Makes it so workers are put in error state when a
+ handshake failure is encountered.
+ PR50332
+ Trunk patch: 
https://issues.apache.org/bugzilla/attachment.cgi?id=26339
+ 2.2.x patch: 
https://issues.apache.org/bugzilla/attachment.cgi?id=26338

+ druggeri: Need doc update?
+
 PATCHES/ISSUES THAT ARE STALLED

   * core: Support wildcards in both the directory and file components of


A tag in CHANGES would be appreciated:
  *) Proxy: Detect SSL handshake failures during proxy pass attempts 
and place backend in error state. PR 50332.  [Daniel Ruggeri DRuggeri 
primary.net]


--
--
Daniel Ruggeri
Index: httpd-trunk/modules/proxy/mod_proxy_http.c
===
--- httpd-trunk/modules/proxy/mod_proxy_http.c  (revision 1037345)
+++ httpd-trunk/modules/proxy/mod_proxy_http.c  (working copy)
@@ -1468,6 +1468,10 @@
 return ap_proxyerror(r, HTTP_SERVICE_UNAVAILABLE, Timeout 
on 100-Continue);
 }
 }
+else if(strcmp(apr_table_get(backend-connection-notes, 
SSL_connect_rv), err) == 0) {
+backend-worker-s-status |= PROXY_WORKER_IN_ERROR;
+backend-worker-s-error_time = apr_time_now();
+}
 /*
  * If we are a reverse proxy request shutdown the connection
  * WITHOUT ANY response to trigger a retry by the client
Index: httpd-trunk/modules/ssl/ssl_engine_io.c
===
--- httpd-trunk/modules/ssl/ssl_engine_io.c (revision 1037345)
+++ httpd-trunk/modules/ssl/ssl_engine_io.c (working copy)
@@ -1091,6 +1091,7 @@
 ssl_log_ssl_error(SSLLOG_MARK, APLOG_INFO, server);
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return MODSSL_ERROR_BAD_GATEWAY;
 }
 
@@ -1108,6 +1109,7 @@
 }
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 X509_free(cert);
@@ -1127,10 +1129,12 @@
   hostname, hostname_note);
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 }
 
+apr_table_set(c-notes, SSL_connect_rv, ok);
 return APR_SUCCESS;
 }
 
Index: httpd-2.2.x/modules/proxy/mod_proxy_http.c
===
--- httpd-2.2.x/modules/proxy/mod_proxy_http.c  (revision 1037345)
+++ httpd-2.2.x/modules/proxy/mod_proxy_http.c  (working copy)
@@ -272,6 +272,10 @@
  proxy: pass request body failed to %pI (%s),
  conn-addr, conn-hostname);
 if (origin-aborted) { 
+if(strcmp(apr_table_get(origin-notes, SSL_connect_rv), err) 
== 0){
+conn-worker-s-status |= PROXY_WORKER_IN_ERROR;
+conn-worker-s-error_time = apr_time_now();
+}
 return APR_STATUS_IS_TIMEUP(status) ? HTTP_GATEWAY_TIME_OUT : 
HTTP_BAD_GATEWAY;
 }
 else { 
Index: httpd-2.2.x/modules/ssl/ssl_engine_io.c

Re: Making mod_proxy_http more aware of SSL

[Daniel Ruggeri]

On 11/25/2010 4:14 AM, Plüm, Rüdiger, VF-Group wrote:

the following seems better:


+else if(strcmp(apr_table_get(backend-connection-notes, SSL_connect_rv), 
err) == 0) {
+return ap_proxyerror(r, HTTP_INTERNAL_SERVER_ERROR,
+ Error during SSL Handshake with remote 
server);
+


Regards

Rüdiger


I agree that the message should be logged as such since logging higher 
than INFO would hide the actual SSL error from mod_ssl. My focus though 
is on marking the backend server out of service as you can't communicate 
unless the SSL transport has been established (essentially a failure to 
connect). Nothing else in the request/response cycle in mod_proxy_http 
does this due to handshake errors and this spot seems to be the very 
first place we can actually check for that condition.


I have updated the patches to log your suggested message after marking 
the workers to be in error state.


--
Daniel Ruggeri
Index: httpd-2.2.x/STATUS
===
--- httpd-2.2.x/STATUS  (revision 1037345)
+++ httpd-2.2.x/STATUS  (working copy)
@@ -184,6 +184,14 @@
  enabling/disabling the basic capability is not split out into mod_unixd 
2.2.x.
  +1: trawick
 
+   * mod_proxy_http: Become aware of ssl handshake failures when attempting
+ to pass request. Makes it so workers are put in error state when a
+ handshake failure is encountered.
+ PR50332
+ Trunk patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26344
+ 2.2.x patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26343
+ druggeri: Need doc update?
+
 PATCHES/ISSUES THAT ARE STALLED
 
   * core: Support wildcards in both the directory and file components of
Index: httpd-2.2.x/STATUS
===
--- httpd-2.2.x/STATUS  (revision 1037345)
+++ httpd-2.2.x/STATUS  (working copy)
@@ -184,6 +184,14 @@
  enabling/disabling the basic capability is not split out into mod_unixd 
2.2.x.
  +1: trawick
 
+   * mod_proxy_http: Become aware of ssl handshake failures when attempting
+ to pass request. Makes it so workers are put in error state when a
+ handshake failure is encountered.
+ PR50332
+ Trunk patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26339
+ 2.2.x patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26338
+ druggeri: Need doc update?
+
 PATCHES/ISSUES THAT ARE STALLED
 
   * core: Support wildcards in both the directory and file components of
Index: httpd-2.2.x/modules/proxy/mod_proxy_http.c
===
--- httpd-2.2.x/modules/proxy/mod_proxy_http.c  (revision 1037345)
+++ httpd-2.2.x/modules/proxy/mod_proxy_http.c  (working copy)
@@ -272,6 +272,12 @@
  proxy: pass request body failed to %pI (%s),
  conn-addr, conn-hostname);
 if (origin-aborted) { 
+if(strcmp(apr_table_get(origin-notes, SSL_connect_rv), err) 
== 0){
+conn-worker-s-status |= PROXY_WORKER_IN_ERROR;
+conn-worker-s-error_time = apr_time_now();
+return ap_proxyerror(r, HTTP_INTERNAL_SERVER_ERROR,
+ Error during SSL Handshake with remote 
server);
+}
 return APR_STATUS_IS_TIMEUP(status) ? HTTP_GATEWAY_TIME_OUT : 
HTTP_BAD_GATEWAY;
 }
 else { 
Index: httpd-2.2.x/modules/ssl/ssl_engine_io.c
===
--- httpd-2.2.x/modules/ssl/ssl_engine_io.c (revision 1037345)
+++ httpd-2.2.x/modules/ssl/ssl_engine_io.c (working copy)
@@ -1065,6 +1065,7 @@
 ssl_log_ssl_error(APLOG_MARK, APLOG_INFO, server);
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 
@@ -1082,6 +1083,7 @@
 }
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 X509_free(cert);
@@ -1101,10 +1103,12 @@
   hostname, hostname_note);
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 }
 
+apr_table_set(c-notes, SSL_connect_rv, ok);
 return APR_SUCCESS;
 }
 
Index: httpd-trunk/modules/proxy/mod_proxy_http.c
===
--- httpd-trunk/modules/proxy

Re: Making mod_proxy_http more aware of SSL

[Daniel Ruggeri]

The loggers get in error state automatically when you call
ap_proxyerror with HTTP_INTERNAL_SERVER_ERROR. No need to do it manually.

Regards

Rüdiger



The request goes into error state - that has never been the problem (502 
is correct here). The real issue is that the proxy worker that served 
the request does not. In a load balancing situation this means that the 
backend with broken SSL will stay in service and every other request 
will send a 502 back to the user (for a 2 node round-robin type of load 
balancing). If I'm not explaining it well enough, have a look at PR50332 
for a quick overview to duplicate the issue.


With this patch, the backend worker is put into error state and is 
subject to normal recovery attempts. In other words, one request fails 
and a 502 is returned but subsequent requests are correctly sent to the 
other backend until the error interval has expired and the first backend 
is retried. IMO, SSL handshake failures should be detected during 
connection so we could attempt another backend but I am not sure that's 
possible.


--
Daniel Ruggeri

Re: Making mod_proxy_http more aware of SSL

[Daniel Ruggeri]

Hence you should return a 500 as this signals the mod_proxy code that the
backend is broken and should be put in error state.
A 502 does not put the backend in error state (as you found out).

Regards

Rüdiger



I didn't realize this was the case - marking it in error inside 
ap_proxy_http_process_response would definitely be redundant! Thank you 
very much for catching it (and explaining this to me). I have updated 
the patches and bug report and attached the updates for reference.


--
Daniel Ruggeri
Index: httpd-2.2.x/STATUS
===
--- httpd-2.2.x/STATUS  (revision 1037345)
+++ httpd-2.2.x/STATUS  (working copy)
@@ -184,6 +184,14 @@
  enabling/disabling the basic capability is not split out into mod_unixd 
2.2.x.
  +1: trawick
 
+   * mod_proxy_http: Become aware of ssl handshake failures when attempting
+ to pass request. Makes it so workers are put in error state when a
+ handshake failure is encountered.
+ PR50332
+ Trunk patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26346
+ 2.2.x patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26345
+ druggeri: Need doc update?
+
 PATCHES/ISSUES THAT ARE STALLED
 
   * core: Support wildcards in both the directory and file components of
Index: httpd-2.2.x/STATUS
===
--- httpd-2.2.x/STATUS  (revision 1037345)
+++ httpd-2.2.x/STATUS  (working copy)
@@ -184,6 +184,14 @@
  enabling/disabling the basic capability is not split out into mod_unixd 
2.2.x.
  +1: trawick
 
+   * mod_proxy_http: Become aware of ssl handshake failures when attempting
+ to pass request. Makes it so workers are put in error state when a
+ handshake failure is encountered.
+ PR50332
+ Trunk patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26339
+ 2.2.x patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26338
+ druggeri: Need doc update?
+
 PATCHES/ISSUES THAT ARE STALLED
 
   * core: Support wildcards in both the directory and file components of
Index: httpd-2.2.x/modules/proxy/mod_proxy_http.c
===
--- httpd-2.2.x/modules/proxy/mod_proxy_http.c  (revision 1037345)
+++ httpd-2.2.x/modules/proxy/mod_proxy_http.c  (working copy)
@@ -272,6 +272,12 @@
  proxy: pass request body failed to %pI (%s),
  conn-addr, conn-hostname);
 if (origin-aborted) { 
+if(strcmp(apr_table_get(origin-notes, SSL_connect_rv), err) 
== 0){
+return ap_proxyerror(r, HTTP_INTERNAL_SERVER_ERROR,
+ Error during SSL Handshake with remote 
server);
+}
 return APR_STATUS_IS_TIMEUP(status) ? HTTP_GATEWAY_TIME_OUT : 
HTTP_BAD_GATEWAY;
 }
 else { 
Index: httpd-2.2.x/modules/ssl/ssl_engine_io.c
===
--- httpd-2.2.x/modules/ssl/ssl_engine_io.c (revision 1037345)
+++ httpd-2.2.x/modules/ssl/ssl_engine_io.c (working copy)
@@ -1065,6 +1065,7 @@
 ssl_log_ssl_error(APLOG_MARK, APLOG_INFO, server);
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 
@@ -1082,6 +1083,7 @@
 }
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 X509_free(cert);
@@ -1101,10 +1103,12 @@
   hostname, hostname_note);
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 }
 
+apr_table_set(c-notes, SSL_connect_rv, ok);
 return APR_SUCCESS;
 }
 
Index: httpd-trunk/modules/proxy/mod_proxy_http.c
===
--- httpd-trunk/modules/proxy/mod_proxy_http.c  (revision 1037345)
+++ httpd-trunk/modules/proxy/mod_proxy_http.c  (working copy)
@@ -1468,6 +1468,12 @@
 return ap_proxyerror(r, HTTP_SERVICE_UNAVAILABLE, Timeout 
on 100-Continue);
 }
 }
+else if(strcmp(apr_table_get(backend-connection-notes, 
SSL_connect_rv), err) == 0) {
+return ap_proxyerror(r, HTTP_INTERNAL_SERVER_ERROR,
+ Error during SSL Handshake with remote 
server);
+}
 /*
  * If we are a reverse proxy request shutdown

STATUS proposal for 2.2.x

[Daniel Ruggeri]

Good day, all;
   I would appreciate it if a committer could spare a moment to patch 
the 2.2 STATUS file to include this as a proposal (a +1 would be really 
great, too). For reference, the patch is also attached. The trunk patch 
was applied in r1039304.


--
--
Daniel Ruggeri
Index: httpd-2.2.x/modules/proxy/mod_proxy_http.c
===
--- httpd-2.2.x/modules/proxy/mod_proxy_http.c  (revision 1037345)
+++ httpd-2.2.x/modules/proxy/mod_proxy_http.c  (working copy)
@@ -272,6 +272,12 @@
  proxy: pass request body failed to %pI (%s),
  conn-addr, conn-hostname);
 if (origin-aborted) { 
+if(strcmp(apr_table_get(origin-notes, SSL_connect_rv), err) 
== 0){
+return ap_proxyerror(r, HTTP_INTERNAL_SERVER_ERROR,
+ Error during SSL Handshake with remote 
server);
+}
 return APR_STATUS_IS_TIMEUP(status) ? HTTP_GATEWAY_TIME_OUT : 
HTTP_BAD_GATEWAY;
 }
 else { 
Index: httpd-2.2.x/modules/ssl/ssl_engine_io.c
===
--- httpd-2.2.x/modules/ssl/ssl_engine_io.c (revision 1037345)
+++ httpd-2.2.x/modules/ssl/ssl_engine_io.c (working copy)
@@ -1065,6 +1065,7 @@
 ssl_log_ssl_error(APLOG_MARK, APLOG_INFO, server);
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 
@@ -1082,6 +1083,7 @@
 }
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 X509_free(cert);
@@ -1101,10 +1103,12 @@
   hostname, hostname_note);
 /* ensure that the SSL structures etc are freed, etc: */
 ssl_filter_io_shutdown(filter_ctx, c, 1);
+apr_table_set(c-notes, SSL_connect_rv, err);
 return HTTP_BAD_GATEWAY;
 }
 }
 
+apr_table_set(c-notes, SSL_connect_rv, ok);
 return APR_SUCCESS;
 }
 
Index: httpd-2.2.x/STATUS
===
--- httpd-2.2.x/STATUS  (revision 1037345)
+++ httpd-2.2.x/STATUS  (working copy)
@@ -184,6 +184,14 @@
  enabling/disabling the basic capability is not split out into mod_unixd 
2.2.x.
  +1: trawick
 
+   * mod_proxy_http: Become aware of ssl handshake failures when attempting
+ to pass request. Makes it so workers are put in error state when a
+ handshake failure is encountered.
+ PR50332
+ Trunk patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26339
+ 2.2.x patch: https://issues.apache.org/bugzilla/attachment.cgi?id=26374
+
 PATCHES/ISSUES THAT ARE STALLED
 
   * core: Support wildcards in both the directory and file components of

Re: Flip default of Header directive to always from onsuccess

[Daniel Ruggeri]

On Friday 29 October 2010, Eric Covener wrote:

http://httpd.apache.org/docs/2.3/mod/mod_headers.html#header

doc says:

By default, this directive only affects successful responses
(responses in the 2xx range). The optional condition can be either
onsuccess (default) or always (all status codes, including
successful responses). A value of always may be needed to
influence headers set by some internal modules even for successful
responses, and is always needed to affect non-2xx responses such
as redirects or client errors.

Any thoughts on flipping the default in 2.3 to always?  I think
this is a holdover from older releases where some of these
directives had Error* equivalents.


I fully support this!

--
Daniel Ruggeri

Some love for balancer manager?

[Daniel Ruggeri]

During Rich's ApacheCon presentation he mentioned that some much needed 
love for the balancer manager was on its way... is anyone working on 
this currently? I'm not seeing anything in the released alphas and would 
be happy to be a guinea pig to do some testing/give thoughts.


--
--
Daniel Ruggeri

Re: millisecond timeouts in mod_proxy mod_proxy_http

[Daniel Ruggeri]

On 1/14/2011 7:06 PM, Jim Jagielski wrote:

It looks like it needs some fine-tuning and some formatting changes,
but assuming we have CLA approval, I'd say lets fold it in
and clean up post submission;)

...

  - extends RewriteRules to implement a custom error document or return
  status per rule if timeout occurs


FWIW, Jim, I'm a big fan of this part.
--
Daniel Ruggeri

Re: balancer worker status

[Daniel Ruggeri]

On 2/1/2011 1:10 PM, Jim Jagielski wrote:


On Feb 1, 2011, at 1:32 PM, William A. Rowe Jr. wrote:


On 2/1/2011 11:03 AM, Jim Jagielski wrote:

Anyone have any good ideas on the best way, GUI-wise, on how
to set/reset the various worker statuses on the balancer-manager
page? Right now we just Enable|Disable, but obviously we need
more fine-grained control that that. Radio buttons? Checkboxes?

(same with actually displaying the status as well... maybe
use the actual ProxySet status flags, eg: E, H, etc...)?


For multi-choice, I'd go droplist with the current status defaulted.



To more I think about it, we have these as bitfields so you can
have combos of them, so for each status bit, I'm thinking of
a checkbox:

 StbyDisIgn
 [ ] [X][ ]

After all, I am projecting that the main method to
actually set these won't be by the balancer-manager
web interface in the long run, but rather as direct
HTTP GET requests (need to make it more REST though)...




+1 on the checkbox idea for the exact reason you mentioned.

--
Daniel Ruggeri

Re: mod_reqtimeout logging

[Daniel Ruggeri]

On 2/10/2011 2:21 AM, Nick Gearls wrote:

Probably not, but as we specify the time-outs to allow all normal
requests (we hope), I'd like to be warned when an attack occurs, but
also if one of my genuine customers is blocked (to possibly fine-tunes
the time-outs).


We should figure out what the general case would be for users. Since 
per-module logging levels is a reality, it's a trivial matter to let the 
server admin decide if they want to log these messages. My concern with 
putting it at WARN level (and a server admin doesn't want these 
messages), they may accidentally suppress other warnings. I may be 
speaking out of turn, though, since I don't know what messages this 
module emits and at what levels.




Another option would be to set an environment variable, so I could check
it and handle my notification manually.


Maybe I misunderstand the idea, but why wouldn't creating a 
'LogTimeoutErrors' (or something to that effect) directive be The Right 
Thing to do in this case?


--
Daniel Ruggeri

Re: stalled backport proposals...

[Daniel Ruggeri]

On 2/11/2011 8:26 AM, Eric Covener wrote:


 Thanks, went ahead and pushed them down since it's easy enough to put
 any of them back.


Which reminds me... anyone care to add a +1 or begin a discussion about 
the mod_proxy change I had submitted? rpluem and covener have given the 
+1... but I see a link to the patch in its current incarnation has gone 
missing from STATUS. Just wanted to bring it up so it doesn't get 
relegated to the 'forgotten' list :)


The notes about its past are here:
Merge r1039304, r1053584 from trunk:

* Put a note in the connection notes that the SSL handshake to the backend
  failed such that mod_proxy can put the worker in error state.

PR: 50332
Submitted by: Daniel Ruggeri DRuggeri primary.net
Reviewed by: rpluem


* Fix r1039304 and make the patch similar to the one proposed for
  2.2.x: If the SSL handshake to the backend fails we cannot even
  sent an HTTP request. So the check needs to happen already when
  we sent data not when we receive data.




I'd like to have this patch committed for the next stable 2.2 release if 
no one has objections (and attribution for rpluem and myself if 
appropriate in CHANGELOG). Just thought I would raise awareness in case 
anyone could spare a moment to discuss.


P.S.
I eventually plan to submit a similar change made in mod_ssl to the 
mod_nss so that same functionality can be retained. Does anyone know of 
others out there?


--
Daniel Ruggeri

2.2 crash on startup with SSLProxyMachineCertificateFile at server level?

[Daniel Ruggeri]

Hello;
   I discovered this very late yesterday and have not had the 
opportunity yet to generate a distributable test case and bug report, 
but wanted to get this in front of folks to see if this may be known.


   Some high-level settings for the httpd configuration are bulleted 
below, but otherwise this happens on an httpd 2.2.15 build for Solaris 
Sparc. No errors are emitted and the return code of the httpd binary 
itself fails the apachectl startup test (if [ $HTTPD ]  ). Because 
of the environment I was testing in, I had only truss on-hand to do 
debugging and did see SIGSEGV thrown after another signal (I wish I 
could remember it).

  *Four virtual hosts - three HTTP w/name-based virtual host
  *SSL server - distinct server cert/key in one HTTPS vhost
  *SSL Client in unencrypted PEM at server-level
  *Huge number of rewrites, redirects, file system aliases, etc
  *Lots of CGI squirreled away in htdocs directory

Only two of those points seem worth noting for this particular issue. 
I'll try to get a test case with a throw-away cert/key combo for folks 
to try and generate a formal bug today, but has anyone heard of/seen 
this behavior before?


--
--
Daniel Ruggeri

Re: 2.2 crash on startup with SSLProxyMachineCertificateFile at server level?

[Daniel Ruggeri]

On 3/11/2011 8:55 AM, Joe Orton wrote:

Hi Daniel -

On Fri, Mar 11, 2011 at 05:47:15AM -0600, Daniel Ruggeri wrote:

Some high-level settings for the httpd configuration are bulleted
below, but otherwise this happens on an httpd 2.2.15 build for


Probably https://issues.apache.org/bugzilla/show_bug.cgi?id=39915 which
was fixed in 2.2.16 - file a bug if it's reproducible with 2.2.17 at
least.  Regards, Joe



I'm putting together some finishing touches on a 2.2.17 build and will 
be testing that right away. Thank you for bringing this up! The bugfix 
definitely sounds applicable to my configuration.


--
--
Daniel Ruggeri

Re: new ProxyPass/ProxyPassReverse feature for 2.4??

[Daniel Ruggeri]

On 3/28/2011 2:08 PM, Jim Jagielski wrote:

Assuming

ProxyPass /foo http://www.example.com/foo

should ProxyPass automatically create the corresponding
ProxyPassReverse statement?

It seems bogus to me that we still require such things...

Of course, we can't get rid of it completely, to handle such
cases as:

 ProxyPass / ajp://localhost:8009/jsp/
 ProxyPassReverse / http://www.example.com/jsp/

but shouldn't we automagically handle the common case??


Big +1 here.

--
--
Daniel Ruggeri

Re: SSL related DoS

[Daniel Ruggeri]

On 4/16/2011 11:52 AM, Chris Hill wrote:

Dear Apache httpd dev list,
...
The reason why I insist in this is that the world has come to depend on
HTTP/SOAP over SSL (and Apache/OpenSSL are probably the most popular
implementation) for business critical apps, yet, it is not clear how
these businesses can play around with configuration parameters to fine
tune these SSL settings to their specific needs, e.g. *ensure client
side renegs can be disabled* or at least,*provide a way of limiting how
many of these a client initiated re-negotiations (or initial handshakes)
a server will allow per second for a specific connection/IP*. It is
great that recent Apache builds disable client initiated renegotiation
by default, but how can I ensure this will never be turned back on in
future releases given the lack of configuration parameters?



Chris;
   I believe this topic (enable/disable renegotiation) was brought up 
on this list just a matter of days ago. I don't recall seeing a 
consensus, but I would agree that a configuration parameter to 
(dis)allow client-initiated renegotiation would be a Very Good Thing. I 
don't think this would be very difficult to implement - and would be a 
good start to correct the issues you call out.


--
--
Daniel Ruggeri

Re: mod_proxy headers

[Daniel Ruggeri]

On 5/2/2011 11:19 AM, Plüm, Rüdiger, VF-Group wrote:

How about the output of mod_info. Doesn't it provide what you need?

Regards

Rüdiger


I have long wondered about exactly this information. My only rub is that 
it's not always feasible (and definitely not very intuitive) to use 
mod_info to gather this information by users.


I think that, actually, the output of mod_info if tidied up would serve 
as a great starting point for this data in its own document... perhaps 
with some brief explanations of what each hook is and when it may come 
into play for users?


I smell a small doc project...

--
--
Daniel Ruggeri

Re: id=51247 Enhance mod_proxy and _balancer with worker status flag to only accept sticky session routes

[Daniel Ruggeri]

On 5/24/2011 9:18 AM, Jim Jagielski wrote:

I like the concept... will review.

PS: Most patches should be against trunk. We fold into trunk,
 test and only then propose for backport for 2.2.x


On May 23, 2011, at 3:10 PM, Keith Mashinter wrote:


I've added a patch to the proxy/balancer to allow for route-only workers are 
only enabled for sticky session routes, allowing for an even more graceful 
fade-out of a server than making its lbfactor=1 compared to lbfactor=100 for 
others.

Please reply/vote if you also think it's useful.

https://issues.apache.org/bugzilla/show_bug.cgi?id=51247
This enhancement, actually SVN Patched against 2.2.19, provides a worker status
flag to set a proxy worker as only accepting requests with sticky session
routes, e.g. only accept requests with a .route such as Cookie
JSESSIONID=xxx.tc2.
 ...





I think there are two patches available to do the same thing - sorry for 
not following up on this sooner. I brought this up in conversation with 
Bill on this list back in October and haven't dug into it since.


I attached the patch to a bug opened by Cameron Stokes
https://issues.apache.org/bugzilla/show_bug.cgi?id=48841

I agree that this functionality would be nice to have but am agnostic as 
to which method accomplishes this :) They both seem to take different 
routes to get to the same goal.


Jim, if you wouldn't mind reviewing both and suggesting one to be 
cleaned up for a patch generated against trunk. I'm happy to volunteer 
the effort to adjust my patch or at least take care of that bug that's 
out there still.


--
--
Daniel Ruggeri
Only in httpd-2.2.15-patched: httpd2.2.15.EnableZeroLbfactor.patch
Only in httpd-2.2.15-patched/modules/proxy: httpd2.2.15.EnableZeroLbfactor.patch
diff -ru httpd-2.2.15/modules/proxy/mod_proxy.c 
httpd-2.2.15-patched/modules/proxy/mod_proxy.c
--- httpd-2.2.15/modules/proxy/mod_proxy.c  2009-01-31 14:58:07.0 
-0600
+++ httpd-2.2.15-patched/modules/proxy/mod_proxy.c  2010-04-05 
15:42:53.0 -0500
@@ -77,9 +77,13 @@
 /* Normalized load factor. Used with BalancerMamber,
  * it is a number between 1 and 100.
  */
-worker-lbfactor = atoi(val);
-if (worker-lbfactor  1 || worker-lbfactor  100)
-return LoadFactor must be number between 1..100;
+//worker-lbfactor = atoi(val);
+worker-lbfactor = strtol(val, NULL, 10);
+if (errno == EINVAL || worker-lbfactor  0 || worker-lbfactor  100)
+return LoadFactor must be number between 0..100 (0 disables this 
worker);
+
+if(worker-lbfactor == 0)
+worker-lbfactor=PROXY_WORKER_NOLBFACTOR;
 }
 else if (!strcasecmp(key, retry)) {
 /* If set it will give the retry timeout for the worker
diff -ru httpd-2.2.15/modules/proxy/mod_proxy.h 
httpd-2.2.15-patched/modules/proxy/mod_proxy.h
--- httpd-2.2.15/modules/proxy/mod_proxy.h  2010-02-15 13:54:41.0 
-0600
+++ httpd-2.2.15-patched/modules/proxy/mod_proxy.h  2010-04-05 
15:43:26.0 -0500
@@ -791,5 +791,8 @@
 
 extern int PROXY_DECLARE_DATA proxy_lb_workers;
 
+/* For setting lbfactor to zero */
+#define PROXY_WORKER_NOLBFACTOR101
+
 #endif /*MOD_PROXY_H*/
 /** @} */
diff -ru httpd-2.2.15/modules/proxy/mod_proxy_balancer.c 
httpd-2.2.15-patched/modules/proxy/mod_proxy_balancer.c
--- httpd-2.2.15/modules/proxy/mod_proxy_balancer.c 2009-04-25 
04:43:38.0 -0500
+++ httpd-2.2.15-patched/modules/proxy/mod_proxy_balancer.c 2010-04-05 
15:46:42.0 -0500
@@ -106,8 +106,11 @@
 ap_proxy_initialize_worker(workers, s);
 if (!worker_is_initialized) {
 /* Set to the original configuration */
-workers-s-lbstatus = workers-s-lbfactor =
-(workers-lbfactor ? workers-lbfactor : 1);
+workers-lbfactor = (workers-lbfactor ? workers-lbfactor : 1);
+if(workers-lbfactor == PROXY_WORKER_NOLBFACTOR)
+workers-lbfactor=0;
+
+workers-s-lbstatus = workers-s-lbfactor = workers-lbfactor;
 workers-s-lbset = workers-lbset;
 }
 ++workers;
@@ -743,7 +746,7 @@
 const char *val;
 if ((val = apr_table_get(params, lf))) {
 int ival = atoi(val);
-if (ival = 1  ival = 100) {
+if (ival = 0  ival = 100) {
 wsel-s-lbfactor = ival;
 if (bsel)
 recalc_factors(bsel);
@@ -1112,11 +1115,13 @@
  * not in error state or not disabled.
  */
 if (PROXY_WORKER_IS_USABLE(worker)) {
-mytraffic = (worker-s-transferred/worker-s-lbfactor) +
-(worker-s-read/worker-s-lbfactor);
-if (!mycandidate || mytraffic  curmin) {
-mycandidate = worker;
-curmin = mytraffic;
+if (worker-s-lbfactor != 0

Re: id=51247 Enhance mod_proxy and _balancer with worker status flag to only accept sticky session routes

[Daniel Ruggeri]

On 5/25/2011 5:41 AM, Mladen Turk wrote:

On 05/25/2011 02:27 AM, Daniel Ruggeri wrote:

I attached the patch to a bug opened by Cameron Stokes
https://issues.apache.org/bugzilla/show_bug.cgi?id=48841



Just a quick note on the first thing I saw:

+ //worker-lbfactor = atoi(val);
+ worker-lbfactor = strtol(val, NULL, 10);
+ if (errno == EINVAL || worker-lbfactor  0 || worker-lbfactor  100)

You should add
errno = 0;
before strtol call if you inspect the errno afterwards.
BTW, what's wrong with the atoi call? We ain't gonna have 64-bit lbfactors,
and the acceptable range is 0 ... 100
Also, don't use C++ comments.


Regards


Sorry about the comment, that was supposed to be removed.

I mentioned the following in the notes when I brought up the topic of 
the patch on the list some months back. I believe my intent was to allow 
setting lbfactor to zero at start time if desired and to prevent a 
'parse error' from causing an invalid value to be treated as zero instead.


Patch notes from Oct on list (sorry this never made it into the bug 
report - can add if desired):
   I used a constant called PROXY_WORKER_NOLBFACTOR in mod_proxy.h and 
changed the atoi call during configuration to strtol since the atoi call 
returns 0 both during error situations and when the proper value to 
return is 0. Also, the existing checks had to be refactored a little 
since (at least on the SUN c compiler) an  uninitialized integer is the 
same as `0'. Aside from that, only the bybusiness algorithm had to be 
modified to avoid a divide by zero error.


--
--
Daniel Ruggeri

Re: MPM-Event, renaming MaxClients, etc.

[Daniel Ruggeri]

On 6/20/2011 4:48 PM, William A. Rowe Jr. wrote:
 On 6/20/2011 4:36 PM, Stefan Fritsch wrote:
 On Monday 20 June 2011, Roy T. Fielding wrote:
 On Jun 20, 2011, at 12:01 PM, Stefan Fritsch wrote:
 
 That kind of last-minute change is going to kill people trying to
 upgrade from 2.2 to 2.4 with a shared config.  We should make
 MaxRequestWorkers an alias for MaxClients in a released 2.2.x
 I think you missed that MaxClients still works in 2.4 (albeit with a 
 warning on startup). So I don't think it's such a big deal.
 Perhaps we should lower the severity from warning to info?  Not every
 admin needs constant reminders while they are running 2.2 and 2.4 from
 a single config.

+1... warning seems a bit dire for the circumstances.

-- 
--
Daniel Ruggeri

Re: MPM-Event, renaming MaxClients, etc.

[Daniel Ruggeri]

On 6/20/2011 6:22 PM, Roy T. Fielding wrote:
 My point wasn't the warning, actually, but rather the fact that a config
 that uses MaxRequestWorkers (instead of MaxClients) will abort an instance
 of httpd 2.2.x.  Hence, a person upgrading to 2.4.x will get tripped up
 if they try to do so incrementally with shared config files across
 many web servers, unless they happen to notice that this change is
 merely cosmetic and they keep MaxClients instead.

 For a trivial improvement like this, we should make it easier on admins
 by backporting the alias to 2.2.x (even if we do not use it on 2.2.x).

 Roy
+1  Sorry, I didn't catch that - good point.

-- 
--
Daniel Ruggeri

Re: RUNPATH for module dependencies on Unix/Linux

[Daniel Ruggeri]

On 7/5/2011 2:46 AM, Joe Orton wrote:
 IMO it is much better to leave this stuff in the control of people who 
 build the software, who can already set env vars or LDFLAGS as they 
 require.  (It would also be much better if everybody used libtool, since 
 as you say, the .la files make this problem goes away :)
Yup - also consider that a lot of folks build the software on different
machines (and potentially different environments/layouts) than the ones
the software runs on.

-- 
--
Daniel Ruggeri

Re: reallyall vs. all vs. most

[Daniel Ruggeri]

On 7/5/2011 4:21 PM, Stefan Fritsch wrote:
 Here is what gets built for most (M) and all (A) and what I would like 
 to change. The choices are certainly subjective, but I think there 
 should be many changes where we all agree. So please comment.
+1 to all of your suggestions, though I wonder about mod_lua since it is
also one of the mentioned experimental modules. I would also see a case
for mod_log_debug to be in MOST. It's one of those modules that one
wouldn't care too much about until it's needed.

-- 
--
Daniel Ruggeri

Re: [vote] mod_ldap

[Daniel Ruggeri]

On 7/7/2011 5:16 PM, Graham Leggett wrote:
 On 07 Jul 2011, at 10:51 PM, Jim Jagielski wrote:

 On Jul 7, 2011, at 2:44 PM, Nick Kew wrote:


 On 7 Jul 2011, at 17:55, William A. Rowe Jr. wrote:

 [ ]  Retain ap_ldap API's in httpd 2.3 mod_ldap, as currently in trunk
(binding mod_ldap to ldap libs)

 +1.   But get it right: not a botch job just because there's
   pressure to release.  Should really have been alpha rather
   than beta when this blew up, but too late to change that.


 +1

 +1.

+1   Since it's being pulled from APR, this is the most sane place to
put it in order to maintain the functionality for httpd.

-- 
--
Daniel Ruggeri

Question and request for comments on patch

[Daniel Ruggeri]

All;
   I am attaching a patch that will allow for a comma separated list of
directives permissable for override. I am doing this because the
existing AllowOverride list of override-able directives sometimes has
too many things, sometimes allows more than is documented and it also
forces third party modules into one of the four predefined lists. With
this patch, an AllowOverrideList directive is added for the server admin
to specify individual directives for override. No other functionality
changes. FWIW, In my particular use case, I would like to grant a
content admin the ability to implement a server-side redirect in
.htacess, but would NOT like to grant them the ability to stand up a
proxy via RewriteRule / http://someOtherLocation/ [P] (which is what
would happen if FileInfo was set in AllowOverride). Note how Redirect
and friends are not documented as directives in core.html#allowoverride
for FileInfo... another patch, another day :) .


   With that in mind, I'd like to request comments on the idea and the
implementation supplied. I can see that there could be efficiency gained
without having to tolower each directive before comparison in
invoke_cmd, but I could not find a way outside of server/config.c
(server/core.c constructs the list I am using) to look up a directive's
normal case like ReDiREcT becomes Redirect before the call to
invoke_cmd as a result of ml = apr_hash_get(ap_config_hash, dir,
APR_HASH_KEY_STRING);.


   The second thing I'd like to bring up is that I see a lot of use in
backporting this, but I have made a change that I think would be
considered an API change (added param to ap_parse_htaccess in
include/http_config.h). Since I know this is not allowed, would it be
acceptable to rename the new ap_parse_htaccess function and implement a
wrapper as ap_parse_htaccess? I would foresee that such a wrapper would
issue a deprecation warning when called, but will call ap_parse_htaccess
with a NULL in place of the (new) override_list.

cheers!
-- 
--
Daniel Ruggeri
diff -ru httpd-2.3.12-beta/include/http_config.h 
httpd-2.3.12-beta.AllowOverrideList/include/http_config.h
--- httpd-2.3.12-beta/include/http_config.h 2011-05-09 10:36:32.0 
-0500
+++ httpd-2.3.12-beta.AllowOverrideList/include/http_config.h   2011-07-19 
18:23:10.0 -0500
@@ -28,6 +28,7 @@
 
 #include util_cfgtree.h
 #include ap_config.h
+#include apr_tables.h
 
 #ifdef __cplusplus
 extern C {
@@ -282,6 +283,8 @@
 int override;
 /** Which allow-override-opts bits are set */
 int override_opts;
+/** Table of directives allowed per AllowOverrideList */
+apr_table_t *override_list;
 /** Which methods are lt;Limitgt;ed */
 apr_int64_t limited;
 /** methods which are limited */
@@ -1064,6 +1067,7 @@
  * @param r The request currently being served
  * @param override Which overrides are active
  * @param override_opts Which allow-override-opts bits are set
+ * @param override_list Table of directives allowed for override
  * @param path The path to the htaccess file
  * @param access_name The list of possible names for .htaccess files
  * int The status of the current request
@@ -1072,6 +1076,7 @@
request_rec *r,
int override,
int override_opts,
+   apr_table_t *override_list,
const char *path,
const char *access_name);
 
diff -ru httpd-2.3.12-beta/include/http_core.h 
httpd-2.3.12-beta.AllowOverrideList/include/http_core.h
--- httpd-2.3.12-beta/include/http_core.h   2011-05-09 13:43:50.0 
-0500
+++ httpd-2.3.12-beta.AllowOverrideList/include/http_core.h 2011-07-19 
18:23:10.0 -0500
@@ -31,6 +31,7 @@
 #include apr_optional.h
 #include util_filter.h
 #include ap_expr.h
+#include apr_tables.h
 
 #include http_config.h
 
@@ -542,6 +543,9 @@
 struct ap_logconf *log;
 
 unsigned int decode_encoded_slashes : 1; /* whether to decode encoded 
slashes in URLs */
+
+/** Table of directives allowed per AllowOverrideList */
+apr_table_t *override_list;
 } core_dir_config;
 
 /* macro to implement off by default behaviour */
diff -ru httpd-2.3.12-beta/include/httpd.h 
httpd-2.3.12-beta.AllowOverrideList/include/httpd.h
--- httpd-2.3.12-beta/include/httpd.h   2011-04-25 17:49:59.0 -0500
+++ httpd-2.3.12-beta.AllowOverrideList/include/httpd.h 2011-07-19 
18:23:10.0 -0500
@@ -706,6 +706,8 @@
 int override;
 /** the override options allowed for the .htaccess file */
 int override_opts;
+/** Table of allowed directives for override */
+apr_table_t *override_list;
 /** the configuration directives */
 struct ap_conf_vector_t *htaccess;
 /** the next one, or NULL if no more; N.B. never change this */
diff -ru httpd-2.3.12-beta/server/config.c 
httpd-2.3.12-beta.AllowOverrideList

Re: Question and request for comments on patch

[Daniel Ruggeri]

On 7/21/2011 3:32 AM, Igor Galić wrote:
 I think you're missing an MMN bump, regarding backporting - or API in 
 general, the wrapper is the right way to go.
 Also: Why not patch against trunk?

 i

 Daniel Ruggeri drugg...@primary.net wrote:

 All;
   I am attaching a patch that will allow for a comma separated list of
 directives permissable for override. I am doing this because the
 existing AllowOverride list of override-able directives sometimes has
 too many things, sometimes allows more than is documented and it also
 forces third party modules into one of the four predefined lists. With
 this patch, an AllowOverrideList directive is added for the server admin
 to specify individual directives for override. No other functionality
 changes. FWIW, In my particular use case, I would like to grant a
 content admin the ability to implement a server-side redirect in
 .htacess, but would NOT like to grant them the ability to stand up a
 proxy via RewriteRule / http://someOtherLocation/ [P] (which is what
 would happen if FileInfo was set in AllowOverride). Note how Redirect
 and friends are not documented as directives in core.html#allowoverride
 for FileInfo... another patch, another day :) .


   With that in mind, I'd like to request comments on the idea and the
 implementation supplied. I can see that there could be efficiency gained
 without having to tolower each directive before comparison in
 invoke_cmd, but I could not find a way outside of server/config.c
 (server/core.c constructs the list I am using) to look up a directive's
 normal case like ReDiREcT becomes Redirect before the call to
 invoke_cmd as a result of ml = apr_hash_get(ap_config_hash, dir,
 APR_HASH_KEY_STRING);.


   The second thing I'd like to bring up is that I see a lot of use in
 backporting this, but I have made a change that I think would be
 considered an API change (added param to ap_parse_htaccess in
 include/http_config.h). Since I know this is not allowed, would it be
 acceptable to rename the new ap_parse_htaccess function and implement a
 wrapper as ap_parse_htaccess? I would foresee that such a wrapper would
 issue a deprecation warning when called, but will call ap_parse_htaccess
 with a NULL in place of the (new) override_list.

 cheers!
 -- 
 --
 Daniel Ruggeri

Attached is the final cut of the patch including doco and MMN bump as
you brought up. I plan to commit this on Monday, time permitting (and of
course in the absence of objections). I'll cobble something together
afterwards for a 2.2 backport.

Regarding the question, Igor, there was not much thought put into which
source tree I started hacking on - I wanted to get something together
before this weekend hit and already had this version configured/built.

-- 
--
Daniel Ruggeri
Index: httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml
===
--- httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml  (revision 
1149429)
+++ httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml  (working copy)
@@ -327,7 +327,8 @@
 directive type=section module=coreFiles/directive sections.
 /note
 
-pWhen this directive is set to codeNone/code, then
+pWhen this directive is set to codeNone/code and directive
+module=coreAllowOverrideList/directive is not set, then
 a href=#accessfilename.htaccess/a files are completely ignored.
 In this case, the server will not even attempt to read
 code.htaccess/code files in the filesystem./p
@@ -442,7 +443,60 @@
 /note
 /usage
 
+directivesynopsis
+nameAllowOverrideList/name
+descriptionIndividual directives that are allowed in
+code.htaccess/code files/description
+syntaxAllowOverrideList directive1,directve2,.../syntax
+contextlistcontextdirectory/context/contextlist
+
+usage
+pWhen the server finds an code.htaccess/code file (as
+specified by directive module=coreAccessFileName/directive)
+it needs to know which directives declared in that file can override
+earlier configuration directives./p
+
+notetitleOnly available in lt;Directorygt; sections/title
+directiveAllowOverrideList/directive is valid only in
+directive type=section module=coreDirectory/directive
+sections specified without regular expressions, not in directive
+type=section module=coreLocation/directive, directive
+module=core type=sectionDirectoryMatch/directive or
+directive type=section module=coreFiles/directive sections.
+/note
+
+pWhen this directive is not set and directive module=core
+AllowOverride/directive is set to codeNone/code, then
+a href=#accessfilename.htaccess/a files are completely ignored.
+In this case, the server will not even attempt to read
+code.htaccess/code files in the filesystem./p
+
+pExample:/p
+
+example
+  AllowOverride None
+  AllowOverrideList Redirect,RedirectMatch
+/example
+
+pIn the example above only the codeRedirect/code

Re: Question and request for comments on patch

[Daniel Ruggeri]

On 7/24/2011 2:12 AM, Stefan Fritsch wrote:
 On Friday 22 July 2011, Daniel Ruggeri wrote:
 Attached is the final cut of the patch including doco and MMN bump
 as you brought up. I plan to commit this on Monday, time
 permitting (and of course in the absence of objections). I'll
 cobble something together afterwards for a 2.2 backport.
 +1 to the principle. This also allows to use LogLevel in .htaccess, 
 which I wanted to implement but never got around to.

 Some thoughts / nitpicks:
 style: Include a space between an if and the opening brace.
Done - looks like I only did this in a few spots some how.
 We need to support C90:
 core.c: In function 'set_override_list':
 core.c:1626:5: error: ISO C90 forbids mixed declarations and code [-
 Werror=declaration-after-statement]
Done also

 When parsing the list, look in ap_config_hash if a directive exists. 
 If not, either log a warning or error out (not sure which is better).
I may be missing something, but isn't ap_config_hash private to config.c
and unavailable to core.c where the configuration directive is
implemented? If I'm off, I'd definitely like to add this as well as
remove the need for the 'tolower' calls by looking up the expected
CamelCase of a directive as it shows up in config.c.
 I think the word1,word2,... syntax is unwieldly, especially if the 
 list gets long. Maybe use AP_INIT_TAKE_ARGV instead (see e.g. 
 AllowMethods).
This is far from a nitpick and makes a lot more sense. I have
implemented this as well.
 It should be possible to reset AllowOverrideList to an empty list in a 
 subdir, even if it is set in the parent dir. This is important in the 
 case that there is a permissive AllowOverrideList in main server scope 
 and an empty one for some virtual hosts.  In the case of an empty 
 list, d-override_list should be set to NULL instead of an empty table 
 for better performance.  Maybe one could allow disabled or reset 
 as alias for an empty list (like in UserDir/DirectoryIndex).
I realized I did not add this about five minutes after sending the patch
along to the list. I have added this also. Your followup email about
making the list NULL complicating things is also true - I had a lot of
trouble merging properly when NULL was considered nothing in the list
as opposed to not configured.
 It may be possible that some modules react badly if a directive is 
 used in .htaccess that has previously not been allowed there. For 
 example if the module needs to do some global initialization when a 
 directive is used for the first time. I am not aware of such a case, 
 but it is something we may want to keep in mind if this is backported. 
 And it is definitely something that should go into the new_api_2.4 
 document.
Yes, very good point - I did not realize this would become a side
effect. I thought there were upstream context checks before a command is
invoked to prevent this.
I have taken the second suggestion and added a note in new_api_2.4.xml also.

Thank you for the review. Much appreciated. Attached is the patch
including the suggested changes except the directive look up.
-- 
--
Daniel Ruggeri
Index: httpd-trunk.AllowOverrideList/docs/manual/developer/new_api_2_4.xml
===
--- httpd-trunk.AllowOverrideList/docs/manual/developer/new_api_2_4.xml 
(revision 1150484)
+++ httpd-trunk.AllowOverrideList/docs/manual/developer/new_api_2_4.xml 
(working copy)
@@ -92,6 +92,13 @@
 pcommon structures for heartbeat modules (should this be public API?)/p
   /section
 
+  section id=ap_parse_htaccess
+titleap_parse_htaccess (changed)/title
+pThe function signature for codeap_parse_htaccess/code has been
+changed. A codeapr_table_t/code of individual directives allowed
+for override must now be passed (override remains)./p
+  /section
+
   section id=http_config
 titlehttp_config (changed)/title
 ul
Index: httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml
===
--- httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml  (revision 
1150484)
+++ httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml  (working copy)
@@ -327,10 +327,11 @@
 directive type=section module=coreFiles/directive sections.
 /note
 
-pWhen this directive is set to codeNone/code, then
-a href=#accessfilename.htaccess/a files are completely ignored.
-In this case, the server will not even attempt to read
-code.htaccess/code files in the filesystem./p
+pWhen this directive is set to codeNone/code and directive
+module=coreAllowOverrideList/directive is set to
+codeNone/code a href=#accessfilename.htaccess/a files are
+completely ignored. In this case, the server will not even attempt
+to read code.htaccess/code files in the filesystem./p
 
 pWhen this directive is set to codeAll/code, then any
 directive which has the .htaccess a
@@ -442,7 +443,63 @@
 /note
 /usage

Re: Question and request for comments on patch

[Daniel Ruggeri]

On 7/26/2011 5:02 AM, Stefan Fritsch wrote:
 This is valid use-case which justifies making it available somehow. 
 But actually, I think something like

 module *mod = ap_top_module;
 ap_find_command_in_modules(cmd, mod);

 should already work without the need for an additional API. This way 
 is not very efficient but as this is only done on server startup, 
 that's not important.

On 7/26/2011 5:35 AM, Plüm, Rüdiger, VF-Group wrote:
 IMHO the keys in apr_tables are case insensitive. So this tolower shouldn't 
 be needed.

Both points taken and implemented. Regarding invalid directives, I set
it as a warning informing that the directive is being discarded. I never
actually tested apr_tables to see if they were case sensitive but had
assumed they were. The offending lines of code have been removed. Thank
you, guys.


I attempted to commit this, but ran into the following error. This is my
first try to commit, so I very well may be doing something wrong. I'm
attempting from my machine here at home rather than minotaur. Any
suggestions?

svn checkout http://svn.apache.org/repos/asf/httpd/httpd/trunk
httpd-trunk  cd http-trunk
svn changelist httpd-trunk.AllowOverrideList
docs/manual/developer/new_api_2_4.xml docs/manual/mod/core.xml
server/config.c server/core.c server/request.c include/http_config.h
include/ap_mmn.h include/httpd.h include/http_core.h
...
svn commit --username druggeri --password REDACTED --message Add
AllowOverrideList directive and documentation --changelist
httpd-trunk.AllowOverrideList
svn: Commit failed (details follow):
svn: Server sent unexpected return value (403 Forbidden) in response to
MKACTIVITY request for
'/repos/asf/!svn/act/2f9005f7-2cd2-4c3f-9311-77bdcf4322e2'

User credentials to svn.apache.org web interface and people.apache.org
work fine so I suspect there's more to the equation.


-- 
--
Daniel Ruggeri

Re: Question and request for comments on patch

[Daniel Ruggeri]

On 7/26/2011 6:29 PM, Daniel Ruggeri wrote:
 Both points taken and implemented. Regarding invalid directives, I set
 it as a warning informing that the directive is being discarded. I never
 actually tested apr_tables to see if they were case sensitive but had
 assumed they were. The offending lines of code have been removed. Thank
 you, guys.
Finally committed as revision 1151654.

... not sure if this was one of the things holding up the beta, but I'm
glad it's in before then :-)

-- 
--
Daniel Ruggeri

Re: id=51247 Enhance mod_proxy and _balancer with worker status flag to only accept sticky session routes

[Daniel Ruggeri]

On 5/25/2011 7:49 AM, Keith Mashinter wrote:
 I've reviewed the other
 patch https://issues.apache.org/bugzilla/show_bug.cgi?id=48841 and I
 had a similar idea, wondering if the route-only intent would happen if
 I tried to set lbfactor=0 but it only allowed values 1-100 and I
 worried about the complexity of changing the lbmethod formulae so
 using a separate status code seemed cleaner.  It's a bit of a magic
 value, but an intuitive one I think.  On the user surface lbfactor=0
 requires less change than my ROUTE_ONLY to the configuration and
 balancer-manager but it needs some documentation to clarify the intent.

 I also attached a patch to
 https://issues.apache.org/bugzilla/show_bug.cgi?id=51247 for the
 trunk, but since I'm having trouble with the overall compile it's in
 theory.  Please forgive compile issues, but I wanted to at least
 share the thought and will update when I can verify a compile and test
 run.

Jim/Bill/others who have mentioned this;
   Just wanted to drop a friendly reminder that I'm waiting on direction
between these two options. I can quickly roll a trunk or 2.2 patch for
either of these if there is consensus for either mechanism.

Both will allow for taking a server offline after bleeding traffic away
by means of sending only existing sessions to said server. The
difference is in approach:
48841Allowing zero as lbfactor tweaks the math a bit for the lbmethods
51247Adds a Route-Only radio box to balancer manager and a constant
in the code to recognize the change

-- 
--
Daniel Ruggeri

Re: CVE-2003-1418 - still affects apache 2 current

[Daniel Ruggeri]

On 9/1/2011 10:23 AM, Marcus Meissner wrote:
 On Thu, Sep 01, 2011 at 05:17:16PM +0200, Reindl Harald wrote:
 ..
 mtime - well, is directly in the header - Last-Modified
 size - well, directly in the header - Content-Length
 inode - well, where is there any security implication?
 I could not directly think of one.

 The reason is just that there is a CVE entry that checkers check for
 and every user of those checkers asks back from their vendors.

 A statement from Apache project that its not seen as security issue is
 probably sufficient.

 Ciao, Marcus

This is a sane response to the problem. I've been asking why this is a
vulnerability for years and have yet to receive an answer... Maybe I
haven't asked the right people.

-- 
--
Daniel Ruggeri

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/modules/ssl/

[Daniel Ruggeri]

On 9/3/2011 1:06 AM, Kaspar Brand wrote:
 Nit: could you replace intermediary by intermediate in all log
 messages and comments? The former isn't really an X.509/PKIX term. (In
 the above message, I suggest saying intermediate CA certificates.)
No problem.

 I think it's preferrable to let OpenSSL build the chain (instead of
 doing it ourselves). There's no readily available function for this,
 unfortunately, but could you try something along the lines in OpenSSL's
 s3_both.c:ssl3_output_cert_chain()? See

   http://cvs.openssl.org/chngview?cn=18326

 I.e., use X509_verify_cert(), ignore its result, but grab the chain from
 the X509_STORE_CTX afterwards. (And when you're done, it's probably
 wise to call ERR_clear_error, see http://cvs.openssl.org/chngview?cn=19472).
I searched for a function to do exactly this and came up empty. Thank
you very much for bringing this to my attention! I'll definitely update
the patch with this because the method I'm using is certainly a
sticks-and-stones approach.

-- 
--
Daniel Ruggeri

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

[Daniel Ruggeri]

On 9/3/2011 11:26 AM, Dr Stephen Henson wrote:
 Some errors (signature error, expired certificates) should arguably logged or
 even treated as fatal errors. This would be because the cause is a badly
 configured server and it is better to get the user to fix their configuration
 than send a certificate chain that is invalid.

 In other cases you may hit problems because sometimes a certificate chain
 which doesn't quite fit the PKIX definition is used. An example would be a 
 proxy
 certificate chain (for some value of proxy, not necessarily standard)
 where some certificates in the chain are not CA certificates in the normal
 definition (basic constraints CA=TRUE). That kind of chain cannot directly 
 be
 built up using X509_verify_cert().

Thank you for the note. I was hoping you and Kaspar would comment on the
updated method I'm using. Rather than storing the chain as
STACK_OF(X509_INFO) I have switched to STACK_OF(X509) and am using the
following function to build the chain.
Comments are definitely appreciated as I don't have a very good frame of
reference for using X509_verify_cert().

int SSL_X509_create_chain(const X509 *x509,
  STACK_OF(X509_INFO) *ca_certs,
  STACK_OF(X509) *chain)
{
int i;
X509_STORE_CTX *ctx;
X509 *cert = (X509 *)x509;
X509_INFO *ca_cert;
STACK_OF(X509) *verified_stack;
STACK_OF(X509) *tmp_stack=sk_X509_new_null();

/* construct a temporary X509 chain from the X509_INFO chain */
for(i = 0; i  sk_X509_INFO_num(ca_certs); i++) {
ca_cert=sk_X509_INFO_value(ca_certs, i);
sk_X509_push(tmp_stack, ca_cert-x509);
}

ctx = X509_STORE_CTX_new();
if (ctx == NULL){
sk_X509_pop_free(tmp_stack, X509_free);
return -1;
}
if (!X509_STORE_CTX_init(ctx, NULL, cert, NULL)) {
sk_X509_pop_free(tmp_stack, X509_free);
return -1;
}
X509_STORE_CTX_trusted_stack(ctx, tmp_stack);

X509_verify_cert(ctx);
/* Ignore verification errors */
ERR_clear_error();

verified_stack=X509_STORE_CTX_get1_chain(ctx);

for(i = sk_X509_num(tmp_stack) - 1; i = 0; i--) {
sk_X509_push(chain, sk_X509_value(tmp_stack, i));
}

X509_STORE_CTX_free(ctx);
return sk_X509_num(chain);

}

-- 
--
Daniel Ruggeri

Re: CVE-2003-1418 - still affects apache 2 current

[Daniel Ruggeri]

On 9/5/2011 8:21 AM, Joe Orton wrote:
 = http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2003-1418

 Is there consensus to treat the issues described there as not being 
 security-sensitive?  If so we can probably put tihs on the vulnerability 
 list is as a not-a-bug as an official statement.

 Regards, Joe

If we are taking score, count me as a +1.
-- 
--
Daniel Ruggeri

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

[Daniel Ruggeri]

On 9/5/2011 11:32 AM, Kaspar Brand wrote:
 Attached is an *untested* patch which hopefully gives you an idea of the
 approach I'm suggesting (you might still want to separate the chain
 building into a function of its own, I simply left t inline in
 ssl_init_proxy_certs for easier editing). Not sure if it works, but
 would appreciate if you could give it a try.

Yes, I like the suggestion. I added some constraints to what I was doing
by trying to design a function that would take X509_INFO so the function
could be reused to build a chain for the server-side of mod_ssl (because
today, the chain certs get presented in whatever order they are in the
file resulting in unhappy java clients). With a little bit of
refactoring on the server side, this could be taken care of just as well.

I've made a few adjustments and built/tested the snippet below. Works
well, though in my test cases I can't tell if the chain is being sent or
not (suggestions on how to verify?).


On 9/5/2011 11:52 AM, Dr Stephen Henson wrote:
 Potential gotcha is that you end up loading up client CAs in the trusted
 certificate store which isn't always what you want. For example if that 
 context
 gets reused they'll be trusted server CA certificates later.

I would say that a case where a server admin doesn't wish to trust
issuers of their own certs is remote, but possible. I think an
appropriately worded blurb in the documentation would be important.
Also, since this functionality hasn't existed yet, I'm inclined to think
that even fewer folks would be impacted.
A potential solution to this is to create a directive controlling
whether a new NULL context is used when loading the store or the
existing SSL context. In the documentation for both directives, we could
inform the server admin the impact of either decision.

FWIW, RFC 2246 (SSL 3.1/TLS 1.0), RFC 4346 (SSL 3.2/TLS 1.1) and RFC
5246 (SSL 3.3/TLS 1.2) place no requirements on sending a chain aside
from making it clear that a chain can be sent. I would say for the
largest range of compatibility, a chain should be sent, but it's not a
requirement if it makes the server admin uncomfortable with the openssl
trust side effect.


I'll clean up the work and update trunk as well as the 2.2 backport
patch sometime later this week.


static void ssl_init_proxy_certs(server_rec *s,
 apr_pool_t *p,
 apr_pool_t *ptemp,
 modssl_ctx_t *mctx)
{
int n, ncerts = 0;
STACK_OF(X509_INFO) *sk;
STACK_OF(X509) *chain;
X509_STORE_CTX *sctx;
X509_STORE *store = SSL_CTX_get_cert_store(mctx-ssl_ctx);
modssl_pk_proxy_t *pkp = mctx-pkp;

/* ... */

if (!pkp-ca_cert_file || !store) {
return;
}

/* Load all of the CA certs and construct a chain */
pkp-ca_certs = (STACK_OF(X509) **) apr_pcalloc(p, ncerts * sizeof(sk));
sctx = X509_STORE_CTX_new();

if (!sctx) {
ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s,
 SSL proxy client cert initialization failed);
ssl_log_ssl_error(APLOG_MARK, APLOG_ERR, s);
ssl_die();
}

X509_STORE_load_locations(store, pkp-ca_cert_file, NULL);

for (n = 0; n  ncerts; n++) {
int i;
X509_INFO *inf = sk_X509_INFO_value(pkp-certs, n);
X509_STORE_CTX_init(sctx, store, inf-x509, NULL);
X509_verify_cert(sctx);
ERR_clear_error();

chain = X509_STORE_CTX_get1_chain(sctx);
sk_X509_shift(chain);
i=sk_X509_num(chain);
pkp-ca_certs[n] = chain;
X509_STORE_CTX_cleanup(sctx);

ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
 client certificate %i has loaded %i 
 intermediate CA%s, n, i, i == 1 ?  : s);
}

X509_STORE_CTX_free(sctx);
}

-- 
--
Daniel Ruggeri

Re: svn commit: r1160863 - in /httpd/httpd/trunk: docs/manual/mod/ modules/ssl/

[Daniel Ruggeri]

On 9/17/2011 6:02 AM, Dr Stephen Henson wrote:
 Yes you need store the returned value and free it with X509_free().

 Note also that because you ignore return values of X509_verify_cert() you 
 might
 have a situation where the chain is not complete and so deleting the last
 element will remove a non-root CA.

Both suggestions make sense - here is what was just committed to
trunk... I also added logging of verification failures at WARNING level.
Since I was in the file again anyhow, I added logging at DEBUG of what
gets loaded and the order so there is no ambiguity.

...
for (n = 0; n  ncerts; n++) {
int i, res;
char cert_cn[256];

X509_INFO *inf = sk_X509_INFO_value(pkp-certs, n);
X509_NAME *name = X509_get_subject_name(inf-x509);
X509_NAME_oneline(name, cert_cn, sizeof(cert_cn));
X509_STORE_CTX_init(sctx, store, inf-x509, NULL);

res=X509_verify_cert(sctx);
chain = X509_STORE_CTX_get1_chain(sctx);

if (res == 1) {
/* Removing the client cert if verification is OK
 * could save a loop when choosing which cert to send
 * when more than one is available */
/* XXX: This is not needed if we collapse the two
 * checks in ssl_engine_kernel in the future */
X509_free(sk_X509_shift(chain));
}
else {
int n = X509_STORE_CTX_get_error(sctx);
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
 SSL proxy client cert chain verification
failed for %s: %s,
 cert_cn, X509_verify_cert_error_string(n));
}
ERR_clear_error();
i = sk_X509_num(chain);
pkp-ca_certs[n] = chain;
if (i == 0 || (res != 1  i == 1) ) {
/* zero or only the client cert won't be very useful
 * due to verification failure */
sk_X509_pop_free(chain, X509_free);
i = 0;
pkp-ca_certs[n] = NULL;
}

X509_STORE_CTX_cleanup(sctx);

ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
 loaded %i intermediate CA%s for cert %i (%s),
 i, i == 1 ?  : s, n, cert_cn);
if (i  0) {
int j;
for (j=0; ji; j++) {
char ca_cn[256];
X509_NAME *ca_name =
X509_get_subject_name(sk_X509_value(chain, j));
X509_NAME_oneline(ca_name, ca_cn, sizeof(ca_cn));
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, %i: %s, j,
ca_cn);
}
}
}

X509_STORE_CTX_free(sctx);
...

-- 
Daniel Ruggeri

Re: svn commit: r1172010 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c

[Daniel Ruggeri]

On 9/19/2011 12:55 AM, Ruediger Pluem wrote:
 On 09/17/2011 06:25 PM, drugg...@apache.org wrote:
  Author: druggeri
  Date: Sat Sep 17 16:25:17 2011
  New Revision: 1172010
  
  URL: http://svn.apache.org/viewvc?rev=1172010view=rev
  Log:
  Log better information and prevent leak of an X509 structure for 
  SSLProxyMachineCertificateChainFile
  
  Modified:
  httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
  
  Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
  URL: 
  http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_init.c?rev=1172010r1=1172009r2=1172010view=diff
  ==
  --- httpd/httpd/trunk/modules/ssl/ssl_engine_init.c (original)
  +++ httpd/httpd/trunk/modules/ssl/ssl_engine_init.c Sat Sep 17 16:25:17 
  2011
  @@ -1181,21 +1181,57 @@ static void ssl_init_proxy_certs(server_
   X509_STORE_load_locations(store, pkp-ca_cert_file, NULL);
   
   for (n = 0; n  ncerts; n++) {
  -int i;
  +int i, res;
  +char cert_cn[256];
  +
   X509_INFO *inf = sk_X509_INFO_value(pkp-certs, n);
  +X509_NAME *name = X509_get_subject_name(inf-x509);
  +X509_NAME_oneline(name, cert_cn, sizeof(cert_cn));
   X509_STORE_CTX_init(sctx, store, inf-x509, NULL);
  -X509_verify_cert(sctx);
  -ERR_clear_error();
   
  +res=X509_verify_cert(sctx);
 Style violation.

   chain = X509_STORE_CTX_get1_chain(sctx);
  -sk_X509_shift(chain);
  +
  +if (res == 1) {
  +/* Removing the client cert if verification is OK
  + * could save a loop when choosing which cert to send
  + * when more than one is available */
  +/* XXX: This is not needed if we collapse the two
  + * checks in ssl_engine_kernel in the future */
  +X509_free(sk_X509_shift(chain));
  +}
  +else {
  +int n=X509_STORE_CTX_get_error(sctx);
 Overwriting a symbol from the loop is IMHO bad and makes code hard to read. 
 Please use
 another name instead of n. Besides we have a style violation here again.


  +ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
  + SSL proxy client cert chain verification failed 
  for %s: %s,
  + cert_cn, X509_verify_cert_error_string(n));
  +}
  +ERR_clear_error();
   i=sk_X509_num(chain);
   pkp-ca_certs[n] = chain;
  +
  +if (i == 0 || (res != 1  i == 1) ) {
  +/* zero or only the client cert won't be very useful
  + * due to verification failure */
  +sk_X509_pop_free(chain, X509_free);
  +i = 0;
  +pkp-ca_certs[n] = NULL;
  +}
  + 
   X509_STORE_CTX_cleanup(sctx);
   
   ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
  - client certificate %i has loaded %i 
  - intermediate CA%s, n, i, i == 1 ?  : s);
  + loaded %i intermediate CA%s for cert %i (%s),
  + i, i == 1 ?  : s, n, cert_cn);
  +if (i  0) {
  +int j;
  +for (j=0; ji; j++) {
  +char ca_cn[256];
  +X509_NAME *ca_name = 
  X509_get_subject_name(sk_X509_value(chain, j));
  +X509_NAME_oneline(ca_name, ca_cn, sizeof(ca_cn));
  +ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, %i: %s, j, 
  ca_cn);
  +}
  +}
   }
   
   X509_STORE_CTX_free(sctx);
  
  
  
 Regards

 Rüdiger


Thank you. Fixed in r1172562.

-- 
Daniel Ruggeri

Re: Pushing for httpd 2.4.0 GA

[Daniel Ruggeri]

On 9/19/2011 8:42 PM, Keith Mashinter wrote:
 Just a reminder about this, providing a way to phase out a server
 by only accepting existing sessions/routed requests.
  
 |51247|New|Enh|2011-05-23|Enhance mod_proxy and _balancer with worker
 status
  


Jim did add this feature as indicated in that bug report. The patch
provides for a 'drain' setting which should do the trick.

-- 
Daniel Ruggeri

Re: svn commit: r1172010 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c

[Daniel Ruggeri]

On 9/19/2011 3:28 PM, Kaspar Brand wrote:
 IMO, you can always drop the first element of the chain, since you only
 want to remember CA certs in pkp-ca_certs.


OK, cool - I was unsure if the chain would ALWAYS contain the cert in
cases of validation OK or error. I'll make this quick update.

 +else {
 +int n=X509_STORE_CTX_get_error(sctx);
 +ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
 + SSL proxy client cert chain verification
failed for %s: %s,
 + cert_cn, X509_verify_cert_error_string(n));
 +}
 Here, cert_cn holds the X509_NAME_oneline() string of the subject DN.
 Either the variable name is a misnomer or a typo (did you mean cert_dn
 instead of cert_cn?)

Yes, indeed. s/_cn/_dn/g


 I have just added ssl_log_xerror() and SSL_X509_NAME_to_string() in
 r1172797, can you adapt the code in ssl_callback_proxy_cert() to make
 use of these where applicable/possible? Hopefully this makes logging
 cert details in mod_ssl more straightforward.

 Kaspar

Sure - no problem. Have a look at the update below. If this jives, I'll
commit to trunk when I have a minute to do so.

Also - any opposition to including SSL_X509_NAME_to_string as part of
the backport proposal? I would like to keep the patches consistent. If
not, would you prefer me to roll it into the
SSLProxyMachineCertificateChainFile patch or propose it separately?



Update...

X509_STORE_load_locations(store, pkp-ca_cert_file, NULL);

for (n = 0; n  ncerts; n++) {
int i, res;

X509_INFO *inf = sk_X509_INFO_value(pkp-certs, n);
X509_NAME *name = X509_get_subject_name(inf-x509);
char *cert_dn = SSL_X509_NAME_to_string(ptemp, name, 0);
X509_STORE_CTX_init(sctx, store, inf-x509, NULL);

if (X509_verify_cert(sctx) != 1) {
int err = X509_STORE_CTX_get_error(sctx);
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
 SSL proxy client cert chain verification
failed for %s: %s,
 cert_dn, X509_verify_cert_error_string(err));
}

chain = X509_STORE_CTX_get1_chain(sctx);

/* Removing the client cert if verification is OK
 * could save a loop when choosing which cert to send
 * when more than one is available */
/* XXX: This is not needed if we collapse the two
 * checks in ssl_engine_kernel in the future */
X509_free(sk_X509_shift(chain));

ERR_clear_error();
i = sk_X509_num(chain);
pkp-ca_certs[n] = chain;

if (i = 1) {
/* zero or only the client cert won't be very useful
 * due to verification failure */
sk_X509_pop_free(chain, X509_free);
i = 0;
pkp-ca_certs[n] = NULL;
}

X509_STORE_CTX_cleanup(sctx);

ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
 loaded %i intermediate CA%s for cert %i (%s),
 i, i == 1 ?  : s, n, cert_dn);
if (i  0) {
int j;
for (j=0; ji; j++) {
X509_NAME *ca_name =
X509_get_subject_name(sk_X509_value(chain, j));
char *ca_dn = SSL_X509_NAME_to_string(ptemp, ca_name, 0);
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, %i: %s, j,
ca_dn);
}
}
}

X509_STORE_CTX_free(sctx);



-- 
Daniel Ruggeri

Re: svn commit: r1172010 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c

[Daniel Ruggeri]

On 9/22/2011 5:39 AM, Kaspar Brand wrote:
 Having it in one patch seems fine to me, but in the end, it's the
 PMC members who will vote on backport proposals (IIUC), so it's
 their opinion which really matters.

IINM, I believe we as committers all have a vote... that said, I hope
you would drop a +1 in the 2.2 STATUS file after the dust settles on
this change :-)


 For trunk, I would prefer ssl_log_xerror being used, like so:

  ssl_log_xerror(SSLLOG_MARK, APLOG_WARNING, 0, ptemp, s, inf-x509,
 SSL proxy client cert chain verification failed: %s:,

X509_verify_cert_error_string(X509_STORE_CTX_get_error(sctx)));

 Only having the subject DN in the log message sometimes makes it
 difficult to unambiguously identify the cert (think e.g. of the
 situation when replacing a previously used cert with a freshly
 issued one, where both have exactly the same subject DN).

Absolutely. I see it is quite a bit more verbose, but definitely
provides enough information.


 For reasons of readability, I would reorder the OpenSSL calls
 after X509_verify_cert() somewhat:
 ...

 Note that X509_STORE_CTX_get1_chain() may also return NULL
 (not under normal circumstances, but still). For reasons of
 robustness, we should fail gracefully in this case.

 And for the logging statements, I would again prefer ssl_log_xerror
 being used (especially since it's APLOG_DEBUG level).


Sure... no problem - updates per feedback provided below. I can't think
what action we *could* take if the chain comes back NULL, so I wrapped
the remaining logic within a check that it is not.

Since this is where the trunk and 2.2 patches differ, feel free to have
a look at the 2.2. patch here:
http://people.apache.org/~druggeri/patches/httpd-2.2-SSLProxyMachineCertificateChainFile.patch

The only difference worth noting is the usage of the new logging routine
you provided in trunk, but not 2.2.



trunk suggestion - if this jives, I'll commit later when I have a bit

if (!pkp-ca_cert_file || !store) {
return;
}

/* Load all of the CA certs and construct a chain */
pkp-ca_certs = (STACK_OF(X509) **) apr_pcalloc(p, ncerts * sizeof(sk));
sctx = X509_STORE_CTX_new();

if (!sctx) {
ap_log_error(APLOG_MARK, APLOG_EMERG, 0, s,
 SSL proxy client cert initialization failed);
ssl_die();
}

X509_STORE_load_locations(store, pkp-ca_cert_file, NULL);

for (n = 0; n  ncerts; n++) {
int i, res;

X509_INFO *inf = sk_X509_INFO_value(pkp-certs, n);
X509_STORE_CTX_init(sctx, store, inf-x509, NULL);

/* Attempt to verify the client cert */
if (X509_verify_cert(sctx) != 1) {
int err = X509_STORE_CTX_get_error(sctx);
ssl_log_xerror(SSLLOG_MARK, APLOG_WARNING, 0, ptemp, s,
inf-x509,
 SSL proxy client cert chain verification
failed: %s :,
  X509_verify_cert_error_string(err));
}

/* Clear X509_verify_cert errors */
ERR_clear_error();

/* Obtain a copy of the verified chain */
chain = X509_STORE_CTX_get1_chain(sctx);

if (chain != NULL) {
/* Dicard end entity cert from the chain */
/* XXX: This is not needed if we collapse the two
 * checks in ssl_engine_kernel in the future */
X509_free(sk_X509_shift(chain));

if ((i = sk_X509_num(chain))  0) {
/* Store the chain for later use */
pkp-ca_certs[n] = chain;
}
else {
/* Discard empty chain */
sk_X509_pop_free(chain, X509_free);
pkp-ca_certs[n] = NULL;
}

ssl_log_xerror(SSLLOG_MARK, APLOG_DEBUG, 0, ptemp, s, inf-x509,
 loaded %i intermediate CA%s for cert %i: ,
 i, i == 1 ?  : s, n);
if (i  0) {
int j;
for (j=0; ji; j++) {
ssl_log_xerror(SSLLOG_MARK, APLOG_DEBUG, 0, ptemp, s,
   sk_X509_value(chain, j), %i:, j);
}
}
}

/* get ready for next X509_STORE_CTX_init */
X509_STORE_CTX_cleanup(sctx);
}

X509_STORE_CTX_free(sctx);

-- 
Daniel Ruggeri

Re: svn commit: r1172010 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c

[Daniel Ruggeri]

On 9/23/2011 10:07 AM, Kaspar Brand wrote:
 On 22.09.2011 22:25, Daniel Ruggeri wrote:
 trunk suggestion - if this jives, I'll commit later when I have a bit
 Looks good, just some nits:

 for (n = 0; n  ncerts; n++) {
 int i, res;
 res is no longer used, AFAICT

Correct - removed


 if (chain != NULL) {
 /* Dicard end entity cert from the chain */
 /* XXX: This is not needed if we collapse the two
  * checks in ssl_engine_kernel in the future */
 X509_free(sk_X509_shift(chain));
 s/Di/Dis/. As for the XXX, do you mean the idea of having a common
 routine for checking server certs and proxy client certs? That would
 probably go to ssl_engine_init.c as well, as sort of a companion to
 ssl_check_public_cert().

In the proxy client cert callback function in ssl_engine_kernel, each
cert is first checked if it is directly signed by each of the CA's in
the list. If that fails, then we start trying to match by chain. The
comment I added just points out that if we leave the end cert in the
STACK_OF(X509) we will perform the same check twice - once for the
direct issuer check and once again for the first item in the chain
without shifting it off.

Alternatively, we could adjust the callback and init functions to always
build a chain (even if SSLProxyMachineCertificateChainFile is not set)
and check by chain by doing the X509_NAME_cmp for each item in the
STACK_OF(X509) in pkp-ca_certs rather than checking the issuer of each
item in pkp-certs. If the new directive is not set, everything would
*essentially* function the same way. To me, they are two ways to do the
same thing, though with the current approach, the verification messages
in startup will not show up unless using the new directive.

... I'm not sure if I explained my thought process well, though, so let
me know if I should elaborate further.


 else {
 /* Discard empty chain */
 sk_X509_pop_free(chain, X509_free);
 pkp-ca_certs[n] = NULL;
 Strictly speaking, the last assignment isn't necessary, since your
 calloc'ing ca_certs before.

Setting to NULL will be caught by the update Rüdiger put in for 1162103
and will skip all of the new logic in the callback function. IMO, I feel
this way is just a bit cleaner and easier to follow. I can be swayed if
you feel strongly about it, though.


 Style - missing spaces. Kaspar 

I'm so bad about this. Corrected also. Thank you very much for
reviewing. I'll wait for feedback before committing and updating 2.2 STATUS.

-- 
Daniel Ruggeri

Re: svn commit: r1172010 - /httpd/httpd/trunk/modules/ssl/ssl_engine_init.c

[Daniel Ruggeri]

On 9/26/2011 2:12 AM, Kaspar Brand wrote:

 Go ahead, I'll add my (nonbinding) vote afterwards :-)

 Just one (hopefully last) thing I overlooked before: the
 ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); line before the
 ssl_die() call apparently got lost somewhere on its way to the tree -
 could you re-add that? It makes sure we also capture the OpenSSL error
 in the log, before aborting.

 Kaspar

All set - the suggestions you made have been added and the results
committed to trunk. STATUS and the 2.2 patch have been updated as well.
Thanks again - cheers!

-- 
Daniel Ruggeri

Re: svn commit: r1176749 - /httpd/httpd/branches/2.2.x/STATUS

[Daniel Ruggeri]

On 9/28/2011 1:34 AM, kbr...@apache.org wrote:
 Author: kbrand
 Date: Wed Sep 28 06:34:33 2011
 New Revision: 1176749

 URL: http://svn.apache.org/viewvc?rev=1176749view=rev
 Log:
 vote/comment

 Modified:
 httpd/httpd/branches/2.2.x/STATUS

 Modified: httpd/httpd/branches/2.2.x/STATUS
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1176749r1=1176748r2=1176749view=diff
 ==
 --- httpd/httpd/branches/2.2.x/STATUS (original)
 +++ httpd/httpd/branches/2.2.x/STATUS Wed Sep 28 06:34:33 2011
 @@ -132,6 +132,10 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
  2.2.x patch: 
 http://people.apache.org/~druggeri/patches/httpd-2.2-SSLProxyMachineCertificateChainFile.patch
  +0 covener: needs r1162103  needs druggeri's vote?
  +1: druggeri
 +kbrand: +1 (non-binding) with the following fixes applied: indentation of
 +the if (ca_cert_chains) line, ca_certs assignment (5 lines 
 below)
 +changed as in r1162103, and in ssl_toolkit_compat.h, omit
 +sk_X509_new_null and remove (st) from the sk_X509_shift define
  
* mod_cache: * Do not cache 206 responses in any case since we currently 
 do not provide any
  backends that are capable to cache partial responses. PR 49113.



Sorry for not getting to this sooner. You will find the patch at
http://people.apache.org/~druggeri/patches/httpd-2.2-SSLProxyMachineCertificateChainFile.patch
has been updated today to reflect this. Please have a look when you can.

-- 
Daniel Ruggeri

Re: Make loglevel of File does not exist configurable

[Daniel Ruggeri]

On 10/4/2011 2:12 PM, Graham Leggett wrote:
 On 04 Oct 2011, at 8:00 PM, Stefan Fritsch wrote:

 I think this one has been controversial in the past, therefore I
 thought I'd ask for comments before making this change:

 The File does not exist log messages are logged at level error,
 while not providing additional information over the 404 access log
 line (at least in many setups). There have been requests to lower the
 loglevel for these messages (PR 35768), which have been denied in the
 past because of compatibility and it's an error. The per-module
 loglevels help not much, because most of the File does not exist
 messages come from core, and one usually does not want to set
 LogLevel core:crit.

 I therefore intend to add a LogLevelFileNotFound config directive
 that allows to make it configurable and a int
 ap_loglevel_file_not_found(request_rec *) API that allows modules to
 query the setting. I think per-virtual server would be enough for the
 setting, but with this API that could be changed to per-dir later on.

 Hmmm... it seems unclean to me and counterintuitive.

 How about moving the default_handler() into the http module?

 Regards,
 Graham
 -- 


Big +1 on achieving this objective. One question, though, about moving
the handler into http... Does that also imply adjusting the logging
level by using http:crit? Wouldn't we swallow several other important
messages by changing logging levels there?

-- 
Daniel Ruggeri

Re: Change loglevel of File does not exist messages

[Daniel Ruggeri]

On 10/5/2011 4:18 PM, Stefan Fritsch wrote:
 True. But a generic apparatus for even more fine-grained log
 configuration won't happen in time for 2.4.

I have toyed with the idea of this... do you have suggestions on how
this might be implemented? One of the ideas I heard (or thought of -
don't recall) was to implement an error log filter such that messages
matching a certain regex get dropped. To me that seems like it would be
very expensive for busier sites, but would serve the purpose for some of
the user base.

 So, if there is consensous to change the messages to info, I will
 commit that after waiting a bit more for possible objections. BTW,
 debug would be fine for me, too. 

+1 for info level

-- 
Daniel Ruggeri

Re: mod_proxy_html

[Daniel Ruggeri]

On 10/13/2011 3:54 AM, Rainer Jung wrote:
 I'm neutral on whether to bundle or not. We would need to handle the
 dependecies like for some other module when bundling, i.e. disabling the
 build if the deps are not found.

So long as this happens, I am +1 for bundling it. Seems it would save on
some of the other tasks of integrating it into the code base.

BTW, Nick, I think this is a really good idea - thank you for bringing
it up.

-- 
Daniel Ruggeri

Re: mod_proxy_html

[Daniel Ruggeri]

On 10/28/2011 4:31 AM, Nick Kew wrote:
 As previously discussed on the dev list, I've recently relicensed
 mod_proxy_html and mod_xml2enc and donated them to Apache.

 Details in my blog piece at
 http://bahumbug.wordpress.com/2011/10/28/modules-move-home/

 Of possible interest to developers, packagers, and end-users who build
 from source rather than packages!


Awesome :)

-- 
Daniel Ruggeri

Trolling for votes

[Daniel Ruggeri]

Hi, folks;
   I wanted draw attention to the 2.2 STATUS file where the
SSLProxyMachineCertificateChainFile directive awaits any additional
votes. I know there was a lot of discussion between Kaspar and myself
getting things in place, but I hope that didn't turn folks off to the patch.

   For quick reference, the patch makes it so a target server can trust
a root CA (for client auth) and allows httpd to choose the right
certificate if the client cert is not directly issued by a trusted CA
(2+ chain length). Depending on interpretation of RFC5246, adding this
patch would bring httpd into compliance. More back and forth at
https://issues.apache.org/bugzilla/show_bug.cgi?id=50812.

P.S.
   Have fun at ACNA2011 - wish I could be there!

-- 
Daniel Ruggeri

Re: Small things to do

[Daniel Ruggeri]

On 11/8/2011 3:10 PM, Stefan Fritsch wrote:
   * mod_ssl's proxy support only allows one proxy client certificate per
 frontend virtual host. Lift this restriction.
 jim sez: Why a blocker?, pgollucci +1 jim
 wrowe asks: what's the API change required? 

I'm not sure I understand this one... does anyone have the history to
elaborate?

-- 
Daniel Ruggeri

Re: Small things to do

[Daniel Ruggeri]

On 11/8/2011 5:37 PM, Graham Leggett wrote:
 Currently in our environment we have reverse proxies connecting to
 client-cert-authenticated backends, and one of the things we can't do
 is this:

 VirtualHost ...
   Location /foo
  ProxyPass https://some.where.back.there/foo
  ...
   /Location
   Location /bar
  ProxyPass https://some.where.different/bar
  ...
   /Location
 /VirtualHost

 where https://some.where.back.there; and
 https://some.where.different; are authenticated by separate sets of
 client certs and separate CA certs.

 We do some nasty php to get around this, it isn't ideal. It is nice to
 have though, and shouldn't block 2.4.

 Regards,
 Graham
 -- 


   I'd have to set up a test case to mimic what you have, but as I
understand it, the SSLProxyMachineCertificateFile at the vhost level
should have both client certs in use within the file. When connecting to
both backends, mod_proxy's client cert callback should pick the correct
client cert from those in the file already (if you look in the code it
actually uses X509_NAME_cmp on each item in the store)... This was one
of the things I verified with my patch for
SSLProxyMachineCertificateChainFile.

   One thing I know for certain that does not fall in line with this is
if some.where.back.there and some.where.different are signed out of the
same CA, but you wish to send different client certs based on path (such
a use case exists, silly as it may seem in my eyes). Is this more in
line with what you are doing? If not, can you email me directly or share
a bit more of a complete example configuration? I have a few test CA's I
stood up for the patch mentioned above that I wouldn't mind taking a
crack at this one. FWIW, In all of my test cases I used ProxyPass to
balancers.

-- 
Daniel Ruggeri

Re: [VOTE] Formal deprecation of 2.0.x branch

[Daniel Ruggeri]

On 11/11/2011 10:49 AM, William A. Rowe Jr. wrote:
 And as we did with 1.3, we would start a 12 month clock to removing
 2.0.x pretty much in its entirety from the live httpd.apache.org site
 and /dist/ mirrors (although still available from archive.a.o/dist/). 

+1. It's time to say goodbye.

-- 
Daniel Ruggeri

Re: [DRAFT] Wanted: Patch Manager

[Daniel Ruggeri]

On 11/16/2011 2:28 AM, Igor Galić wrote:
 I really like the basic outline, but what I feel is missing, is the
 notion of watching $issuetracker. Often we have patches getting lost
 in there. So our patch manager should also look out for $PATCH issues
 and make sure they are followed up, or assigned, or whatever.

Yes, agreed - we always do ask for an issue to be opened up, but
sometimes those can also fall through the cracks, too. The first part of
the email seems pretty straight forward since we don't seem to really
get many emails on dev@ containing patches that don't end up with an
associated issue. Having a bugzilla cop to identify duplicate issues,
pester the dev list about lingering issues, provide gentle reminders of
issues in progress if they stall, close out junk and how to tickets
and even bring up issues opened in the tracker that never had a
corresponding dev discussion would be pretty great.

I'm more than happy to help in this role, but don't always consistently
have the time available to keep as sharp an eye on the tracker as I
would like.

-- 
Daniel Ruggeri

Zombies from rotatelogs

[Daniel Ruggeri]

I was taking a look at a server that had a handful of zombies and came
to see they are caused by rotatelogs. It seems pretty straight forward
why - I am calling gzip post-rotate to compress the log file and child
cleanup only happens during the post_rotate function, but before
apr_proc_create for this rotation. So, effectively, you will have a
zombie process from time_child_dies to next_rotation.

I would like to get rid of those zombie processes sooner but am wary to
call apr_proc_wait_all_procs very often... I don't know what the expense
of doing that would be.

So... I'm seeking advice for the best option here:
*Call apr_proc_wait_all_procs in the main loop (expensive?)
*Spawn a thread on the side to reap the child a la blocking mode
*Check an interval in the main loop to call apr_proc_wait_all_procs once
per X time
*or... your idea

Thanks

P.S.
ApacheCon was a blast - can't wait until next one!

--
Daniel Ruggeri

Re: Zombies from rotatelogs

[Daniel Ruggeri]

On 4/14/2014 11:41 AM, Joe Orton wrote:
 It's free... dunno why I didn't think of this before.

 http://svn.apache.org/viewvc?view=revisionrevision=1587255

 Regards, Joe

Awesome - proposed for backport in 2.4. Thanks!

-- 
Daniel Ruggeri

Re: svn commit: r1587650 - /httpd/httpd/branches/2.4.x/STATUS

[Daniel Ruggeri]

On 4/15/2014 2:21 PM, Jim Jagielski wrote:
 I can't recall... isn't the issue still being worked an
 additional aspect of mod_rewrite and UDS; that is, a new
 behavior to be added (or handled) rather than a broken
 behavior.
That was my understanding, too

-- 
Daniel Ruggeri

Re: failonstatus only works on backend provided status codes

[Daniel Ruggeri]

On 5/12/2014 7:31 AM, Ruediger Pluem wrote:
 While trying to use the failonstatus option of a balancer for the first time 
 I noticed that it only works on status
 codes provided by the backend not on status codes only set by the proxy (in 
 my case a 502 because the backend timed
 out). Is this intentional?

Hi, Ruediger;
   Yes, that was the original goal. The use case I was tackling was a
case where a backend application server started accepting HTTP requests
before the Servlets had all completed init(). In that case, the backend
returns a 503.

-- 
Daniel Ruggeri

Re: ApacheCon Austin, httpd track

[Daniel Ruggeri]

On 11/30/2014 11:08 AM, Jeff Trawick wrote:
 * deploying Python web apps under uWSGI behind mod_proxy_fcgi/scgi
 (some material
 here: http://emptyhammock.com/projects/info/pyweb/index.html)
 * a debugging tricks talk I've given a few times (relatively minor
 updates from the last North America AC)
 * drastically updated (rewritten) version of an old
 capacity-tuning-and-performance talk I gave at a Sun conference in
 2009
 (https://blogs.oracle.com/trawick/resource/DeepDive/WebStackDeepDiveApache.pdf)

Similarly, I'm always up for giving my proxy talk if it's welcome (after
the first day since I can't make it until Tues). If we think proxy is a
big topic, we ought to arrange for a general overview like my proxy talk
followed by more specific deep dives such as what Jeff mentions here and
a session on new sexiness like WebSockets. Tuning for throughput is also
an interesting topic and in line with the conversations lately (Re:
commercial support).

A side note on SSL/security: I had the idea a few years back that there
is probably enough content to do a here is 5 minutes about how to
configure SSL in httpd and then 50 minutes of other important security
topics (What Ciphers should I enable? Should I use SSLv3 any more? How
to treat my keys and what the hell is an HSM anyway? Passphrase
encrypted keys or not? Should I trust my distro's build?). Thoughts are
welcome on that topic... not sure if I'm overly paranoid or if these are
things that people actually want to hear?

-- 
Daniel Ruggeri

Re: [for beginners] How to have a patch validated?

[Daniel Ruggeri]

Bruno;
   You did everything right. I have committed this to trunk in r1648201
and proposed for 2.4 backport in STATUS. Thanks for the patch.

-- 
Daniel Ruggeri

On 12/28/2014 6:28 AM, Bruno Raoult wrote:
 Hi,

 I am really sorry for this stupid question.

 I did send a bug report, with a patch, bug number is 57227
 (https://issues.apache.org/bugzilla/show_bug.cgi?id=57227).

 However, I believe this bug could stay forever if the patch (or
 better) is not applied.
 Did I do something wrong when I did submit the bug and patch together?

 Thanks,

 Bruno.


 -- 
 2 + 2 = 5, for very large values of 2.

Re: [APACHECON] Proposed httpd (and related) track

[Daniel Ruggeri]

Hi, Rich;
   I dig it. I'm all for presenting and helping to make ApacheCon great
but I won't be able to make it on day 1 since I'll probably be somewhere
in the air over the Gulf of Mexico mid-day. If you are struggling for
content, I can be convinced to take on the topic that I brought up
earlier on the list
(http://marc.info/?l=apache-httpd-devm=141753066016122w=2) or to jump
in to help Bill with what he is working on (FWIW, I can't see
http://events.linuxfoundation.org/cfp/proposals/4346, so I don't know
what Bill has up his sleeve other than what was brought up in the
context of this discussion).

   Also, don't hesitate to reach out if I can help out with any of the
regular or extracurricular activities during/after/around the conference.

-- 
Daniel Ruggeri

On 2/10/2015 1:36 PM, Rich Bowen wrote:
 Here's my proposed httpd (and related) track. If anyone has any
 objections, changes, suggestions, whatever, please speak up. Thanks.


 Day 1:

 * Panel: The Apache Group greybeards: If I'd only known then (Don’t
 yet have confirmation of who’s actually going to be there. Brian has
 declined to speak on such a panel.) Note if that these folks don't
 show up, we'll need to find something to fill this slot with. Brian
 has confirmed that he'll attend, but so far I don't have an absolute
 confirmation from anybody else from that era.

 * What's New In Apache HTTPD 2.4 - jimjag -
 http://events.linuxfoundation.org/cfp/proposals/4014

 * mod_rewrite and friends - rbowen -
 http://events.linuxfoundation.org/cfp/proposals/1528/4013

 * The State of TLS on Apache HTTP Server - wrowe -
 http://events.linuxfoundation.org/cfp/proposals/4346

 * Reverse Proxy with Apache HTTPD 2.4: The hidden gem - jimjag -
 http://events.linuxfoundation.org/cfp/proposals/4015

 *  The mod_proxy cookbook -  Daniel Ruggeri -
 http://events.linuxfoundation.org/cfp/proposals/3816


 Day 2:


 * Apache HTTP Configuration API for Developers - wrowe -
 http://events.linuxfoundation.org/cfp/proposals/4347

 * Begone mod_php! - Jim Riggs -
 http://events.linuxfoundation.org/cfp/proposals/4208

 * A Peek at PHP 7 - John Coggeshall -
 http://events.linuxfoundation.org/cfp/proposals/4226

 * Using Apache Traffic Server to cache Live TV - Mark Torluemke -
 http://events.linuxfoundation.org/cfp/proposals/4180

 * Traffic Server on the Edge - Alan Carroll -
 http://events.linuxfoundation.org/cfp/proposals/4133

 * Replacing Squid with Apache Traffic Server for Yahoo - Shu Kit Chan
 - http://events.linuxfoundation.org/cfp/proposals/4118

 Day 3 - TOMCAT


 * Intro to Load-Balancing Tomcat with httpd and mod_jk - Christopher
 Schultz - http://events.linuxfoundation.org/cfp/proposals/4206

 * Tomcat clustering: Part 1 - Reverse Proxies - Mark Thomas -
 http://events.linuxfoundation.org/cfp/proposals/4053

 * Tomcat clustering: Part 2 - Load-balancing - Mark Thomas -
 http://events.linuxfoundation.org/cfp/proposals/4054

 * Tomcat clustering: Part 3 - Session Replication - Mark Thomas -
 http://events.linuxfoundation.org/cfp/proposals/4055

 * Monitoring Apache Tomcat - Christopher Schultz -
 http://events.linuxfoundation.org/cfp/proposals/3847

 * Choosing tomcat connectors: internals and performances -
 Jean-Frederic Clere -
 http://events.linuxfoundation.org/cfp/proposals/3839

Re: Balancer manager

[Daniel Ruggeri]

+1

There are also some neat-o features I added in my notes during ACNA to
stick into the balancer manager, too... I plan to get around to them in
vague, noncommittal reference to free time as it permits days.

-- 
Daniel Ruggeri

On 4/24/2015 7:52 AM, Jim Jagielski wrote:
 Right now, the balancer manager allows for a member to be
 disabled/stopped, but it cannot *remove* that member...
 Seems to me that that would be good, especially since
 we could always re-use that slot.

 Comments?

Re: *Match, RewriteRule POLA violation?

[Daniel Ruggeri]

+1
By unbreaking configurations we are indeed changing behavior. This could
be an unexpected change for an admin during a minor upgrade but I weigh
that against the fact that directives enclosed by these matches may be
intended to add security/authorization/authentication which a badly
written link could circumvent if an admin isn't using the appropriate regex.

-- 
Daniel Ruggeri

On 4/30/2015 8:16 AM, Yann Ylavic wrote:
 On Thu, Apr 30, 2015 at 2:57 PM, Jim Riggs apache-li...@riggs.me wrote:
 Thanks, Yann. I remember looking at this code before. The question remains, 
 though: Is it currently wrong?
 Does it need to be fixed, or was this distinction made intentionally?
 Is there a specific use case that requires the regex-matching directives to 
 not get slash-normalized URIs?
 I would like it to be fixed, non leading /+ is equivalent to /,
 this would break very few (if any) cases IMHO, and may even unbreak
 more ones .

Re: Proposal/RFC: informed load balancing

[Daniel Ruggeri]

 httpd is used as the load balancer. For this, I have 
 created a module tentatively named mod_lbmethod_bybackendinfo that will:

 1. Periodically (based on elapsed time, number of requests, or both since 
 last update) insert the X-Backend-Info request header into a proxied request.

 2. Parse and remove the X-Backend-Info response header.

 3. Calculate the member's informed load factor based on a formula specified 
 by the user/admin in the configuration. I hope to just use the existing 
 lbfactor field to store this calculated value. Then we can use existing logic 
 to balance based on lbset and lbfactor for subsequent requests.

 4. Store the current time and request count in the member's data structure so 
 the lbmethod knows when it needs to be updated again.


 What I need from all of you:

 - Input/commentary on the proposed idea, approach, and implementation. 
 Renaming things, additional fields that might be useful, etc., are all up for 
 discussion.

 - Help with handling the configuration formula mentioned in #3 above. Can we 
 just add some math operators to the expression parser to handle this? What 
 all operations/functions might we need (+-*/? max? min? ternary if-then-else? 
 ...)? A simple-ish example (something like this maybe?):

 Proxy balancer://...
   BalancerMember ...
   ...
   ProxySet \
 lbmethod=bybackendinfo \
 backendupdateseconds=30 \
 backendupdaterequests=100 \
 backendformula=%{BACKEND:uptime} -lt 120 ? 1 : %{BACKEND:workers-free} / 
 %{BACKEND:workers-max} * 100
 /Proxy

Since the spec above states that one or more X-Backend-Info headers is
acceptable, there should be a way for the admin to declare upon which
header the calculation should be performed. A provider flag or option
(FIRST|LAST|provider-name) ought to do the trick.

Side note: Is there a generic place for maths based on strings in httpd
at all? Creating our own mini-language makes me nervous since we have to
consider that backends can and will do stupid things that could result
in httpd attempting an operation that could cause a crash if we aren't
very careful about validating inputs and making sure we aren't doing
silly things like div/0


 - [Near-long-term] Help adding X-Backend-Info backend support and 
 documentation to various projects. Tomcat, php-fpm, others(?) should be 
 fairly easy to implement and submit patches. This work does us no good if 
 none of our backends support it.

I will be happy to provide a ServletFilter that can be used in a J2EE
app or a Tomcat Valve (good for the many Tomcat variations and JBoss)
once we settle on details.


 - [Long-term] Help adding X-Backend-Info frontend support and documentation 
 to various projects to help this become an accepted ad-hoc standard...or 
 something like that. Nginx, haproxy, and many others would be targets.


 Warn out from writing all of this and hopeful that someone other than me 
 actually cares, I wish you all well today/tonight!

 - Jim


All in all, man, this is solid. I like what you've done here.

--
Daniel Ruggeri

Re: Balancer manager

[Daniel Ruggeri]

(oops - saw this sitting int he outbox for the past week - sorry for
slow reply)

These were the notes I took. I was going to start biting them off after
I wrapped up splitting/editing the recordings from the ACNA talks:
*Ensuring all stats showed up on the page (I don't recall if any stuck
out that were missing)
*Add ability to reset the stats captured
*Set or adjust min/max for the connection pooling
*Send what httpd thinks the worker status is (useful for backends that
would like to know about drain, etc) to the backend in a header

-- 
Daniel Ruggeri

On 4/27/2015 9:43 AM, Jim Jagielski wrote:
 Can you list 'em here? I'd like to help w/ adding them :)

 On Apr 25, 2015, at 12:28 PM, Daniel Ruggeri drugg...@primary.net wrote:

 +1

 There are also some neat-o features I added in my notes during ACNA to
 stick into the balancer manager, too... I plan to get around to them in
 vague, noncommittal reference to free time as it permits days.

 -- 
 Daniel Ruggeri

 On 4/24/2015 7:52 AM, Jim Jagielski wrote:
 Right now, the balancer manager allows for a member to be
 disabled/stopped, but it cannot *remove* that member...
 Seems to me that that would be good, especially since
 we could always re-use that slot.

 Comments?

Re: cPanel Apache 2.4

[Daniel Ruggeri]

Nice!
-- 
Daniel Ruggeri


 Original Message 
From: Jacob Perkins jacob.perk...@cpanel.net
Sent: May 15, 2015 10:18:08 AM CDT
To: dev@httpd.apache.org
Subject: cPanel  Apache 2.4

Good afternoon,

As some of you may be aware, cPanel is a leader in the hosting industry as we 
provide software that allows end users to easily run a server and associated 
LAMP stack.  Yesterday, May 14th, we changed the default installation target to 
Apache 2.4 over Apache 2.2.

Currently, around 80% of our user base uses Apache 2.2.  I’m hoping to have 
those customers using Apache 2.4 in the next year.

I know that you guys like to keep track of 2.2 vs 2.4 usage, so hopefully we’ll 
start noticing the 2.4 usage increase gradually over the next year or so.

Thanks for your time!
—
Jacob Perkins
Product Owner
cPanel Inc.

jacob.perk...@cpanel.net mailto:jacob.perk...@cpanel.net
Office:  713-529-0800 x 4046
Cell:  713-560-8655

Re: silly ab patch for SNI and OCSP stapling

[Daniel Ruggeri]

Yep, my mistake. I thought there was a command line switch to change the
host header. You're correct - it wouldn't make much sense to override
one and not the other.

-- 
Daniel Ruggeri

On 5/16/2015 11:25 AM, Jeff Trawick wrote:
 in that case shouldn't you also be overriding Host:, so the SNI host
 name can use the same override?  I think this may lead the user into a
 more helpful scenario, if indeed they don't already know when to
 override Host:, and I don't know how useful it is to have different
 values for Host: and SNI.

Re: silly ab patch for SNI and OCSP stapling

[Daniel Ruggeri]

+1, but I would also propose a command line flag to override the SNI host name 
supplied in case one is testing directly by IP address.
-- 
Daniel Ruggeri


 Original Message 
From: Jeff Trawick traw...@gmail.com
Sent: May 12, 2015 2:31:37 PM CDT
To: Apache HTTP Server Development List dev@httpd.apache.org
Subject: silly ab patch for SNI and OCSP stapling

... where OCSP stapling means get the server to do the related work 
but don't care what you get back.

Perhaps this doesn't save any time for anybody that would want to test 
such a thing, but who knows?

Index: support/ab.c
===
--- support/ab.c(revision 1679028)
+++ support/ab.c(working copy)
@@ -1287,6 +1287,8 @@
  bio = BIO_new_socket(fd, BIO_NOCLOSE);
  SSL_set_bio(c-ssl, bio, bio);
  SSL_set_connect_state(c-ssl);
+SSL_set_tlsext_host_name(c-ssl, hostname);
+SSL_set_tlsext_status_type(c-ssl, TLSEXT_STATUSTYPE_ocsp);
  if (verbosity = 4) {
  BIO_set_callback(bio, ssl_print_cb);
  BIO_set_callback_arg(bio, (void *)bio_err);

The lack of SNI is a pretty big hole now; it probably doesn't need much 
extra in the way of #if/if to do the right thing.

Style checker?

[Daniel Ruggeri]

All;
   I still develop in what a lot of folks would consider a fairly primitive 
environment (vi) that doesn't do anything for style checking things like line 
width/spacing before and after control statements/indentation/variable 
declaration/etc. I know of the indent tool available on most unix-like systems, 
but was wondering if you folks use any other tools to help along that path?
-- 
Daniel Ruggeri

Re: [VOTE] Release Apache httpd 2.4.13 as GA

[Daniel Ruggeri]

On 6/4/2015 8:43 PM, Noel Butler wrote:
 I disagree with this removal, it will cause more problems then benefits
 IMHO, make it a recommendation to use SSLCertificateFile, but not an
 essential requirement, perhaps log it, and log it only once, not per
 every domain, not throw them to std out.

+1

FWIW, I think Kaspar had a driving technical reason for its deprecation,
but I can't seem to find the original email talking about it.

-- 
Daniel Ruggeri

Re: Additional LB providers

[Daniel Ruggeri]

Additional providers is cool... but what do you mean by fold in? Add them as 
additional modules?

(Sorry for top-post... mobile email client)
-- 
Daniel Ruggeri


 Original Message 
From: Jim Jagielski j...@jagunet.com
Sent: June 18, 2015 11:52:12 AM CDT
To: httpd dev@httpd.apache.org
Subject: Additional LB providers

I'm playing around w/ a newish LB provider that balances based
on latency. I'd like to fold it in after I clean it up a bit
but it seems to me that we could also fold in the round-robin
provider in example (after a suitable cleanup) as well as add
in a simply by_random as well.

Comments?

Re: Proxy balancer providers and aging

[Daniel Ruggeri]

On 6/24/2015 3:12 PM, Jim Jagielski wrote:
 All LBmethods have an age function which is designed to appropriately
 age the data so that things even out after awhile. Of course, right
 now, none actually *uses* that.
 

I had never realized this function exists... what triggers it? ... or
are you saying that today nothing triggers it because of the next sentence?

 But I think the reason is because there is no real good way,
 currently, in mod_proxy to do that...
 
 Well, in the LBmethod I was hacking away on, it really DOES need
 its age function, and currently it's doing so via mod_watchdog.
 But the more I think of it, ideally, mod_proxy *itself* should
 create the watchdog thread and just handle the age itself, rather
 than having a LBmethod provider having to be responsible for
 creating that (it also kind of destroys the abstraction that pure
 providers should have, imo)...
 

Yup - if there is some sort of contract between mod_proxy such that each
balancer ought to be able to age its data, mod_proxy should provide
the facilities to call that age function on some interval. However,
looking at the age function signature in the existing balancers, I'm
seeing a conspicuous lack of a time delta since the last call to age().
It seems to me that if mod_proxy is responsible for the 'tick' it should
also be responsible for providing a time delta so the lbmethod can age
the data proportionally to the time since last tick.

(or, maybe I'm missing it altogether and the module should be managing
this itself which kinda puts us back to the beginning of which module
should be responsible for this stuff)


 Anyway... anyone opposed to me adding this to mod_proxy in
 trunk with hope towards it backporting to 2.4? It does mean
 that mod_proxy will now have a dependency on mod_watchdog.

I'm conflicted. This is useful functionality but would break existing
configurations using 2.4 in an unexpected way. I remember dealing with
lots of questions about why is this slotmem thing needed all of a
sudden when moving from 2.2 to 2.4.

I think that if I were to be forced to work through the cognitive
dissonance I'd be +1 for adding this to trunk but -1 for 2.4 unless we
can find a way to avoid the dependency unless the lbmethod really needs
it (I don't see how, but please do enlighten me if this is possible).

-- 
Daniel Ruggeri

Re: PMC Reporting [Was: Re: 2.2 and 2.4 and 2.6/3.0]

[Daniel Ruggeri]

On 5/30/2015 9:03 PM, William A Rowe Jr wrote:
 So I'll let Eric share what he submitted for May on our behalf, but here
 is the submitted/accepted/recorded report of Feb '15 - it's awfully high 
 level, so I'm not sure that updating dev@ regularly with the contents 
 offers a whole lot of benefit.  Thoughts?

Yeah - The information seems great to share with the community behind
httpd, IMO, so I think it would make sense to have on the dev@ list...
and it's not like this is a particularly low volume mailing list that
such emails would be considered inbox pollution.

I guess it's just as easy to pull up the minutes each month by hand, too.

-- 
Daniel Ruggeri

Re: 2.2 and 2.4 and 2.6/3.0

[Daniel Ruggeri]

On 5/30/2015 1:47 PM, Daniel Ruggeri wrote: Thinking about this more,
what are the things preventing people from an
 _easy_ upgrade path configuration-wise? A lot of this conversation
 surrounded users and the impact of an upgrade to them. The interface for
 the users' to the server is the configuration file. Maybe if we can
 tackle that we can greatly reduce a barrier to upgrade (or maybe I'm too
 optimistic)?

 For the majority of my configs, it was the changes to the authorization
 directives - it takes brainpower to figure out what AdminXYZ was trying
 to do years ago and reflect that with the new directives. However, this
 is deterministic... a perl script could do this work for me if I'd just
 write it.

 At $dayjob, this is the stuff I focus on. Tweaks to an existing script I
 put together years ago to upgrade from 2.0 to 2.2 (or as Rich eloquently
 put, poop out new configs) only required an hour's worth of work or so
 to support upgrades from 2.2 to 2.4 minus the aforementioned authz
 directives.

 -- Daniel Ruggeri

aand on the original topic at hand, I'll share that none of us
are really *forced* to focus on any particular branch or any particular
area of the code base. As an army of volunteers, we scratch our own
itch. I, too, have noticed that 2.2 hasn't had a whole lot going on
lately. I don't know if that means we ought to get ready to drop the axe
on it, but I also don't know if that means we shouldn't be thinking
about it, either.

I think Jim's email served it's purpose of at least vocalizing a thought
that's probably in the back of all of our minds: 2.2 isn't getting a ton
of love (or at least as much love as 2.4) lately. What that means to
each of us is clearly different... for me, I noticed that the effort to
backport some of the work I've done in 2.4 doesn't pay off so I don't do
it. I haven't put much more thought into 2.2 than that. For others it
might be a push to backport more stuff.

At the same time, Bill points out a reality we're faced with. As the
most widely deployed HTTP server, we have some sort of responsibility
(whether real or imagined) to the users not to cut them off if we can
avoid it. The point about distros taking their sweet time to update are
well taken - it's one of the reasons I always build from latest source.
Maybe there's a reason other than inertia for the slow adoption. Their
own lack of volunteer cycles? Configuration differences? Performance
differences? What we have works, so why change it?
Do we know those reasons enough to try to keep ahead of them?


P.S.
I'm not a Member or PMC... do I have access to the report that spurred
the conversation?

P.P.S.
Wow... I don't read my email for a few days...

-- 
Daniel Ruggeri

Re: 2.2 and 2.4 and 2.6/3.0

[Daniel Ruggeri]

On 5/28/2015 2:54 PM, Jim Riggs wrote:
 Having to expend effort (e.g. re-design/update config and deployment)
to switch/update/upgrade to a new paradigm does not, IMO, mean that it's
not a solution for everyone. Anyone can take the time to implement and
automate the switch. Once that effort has been spent, you should have
nearly the same (maybe better) setup, with hopefully better security and
resource utilization in many cases. All of those php_admin_* directives
end up in a php-fpm conf file that can easily be automatically updated
and deployed.


Thinking about this more, what are the things preventing people from an
_easy_ upgrade path configuration-wise? A lot of this conversation
surrounded users and the impact of an upgrade to them. The interface for
the users' to the server is the configuration file. Maybe if we can
tackle that we can greatly reduce a barrier to upgrade (or maybe I'm too
optimistic)?

For the majority of my configs, it was the changes to the authorization
directives - it takes brainpower to figure out what AdminXYZ was trying
to do years ago and reflect that with the new directives. However, this
is deterministic... a perl script could do this work for me if I'd just
write it.

At $dayjob, this is the stuff I focus on. Tweaks to an existing script I
put together years ago to upgrade from 2.0 to 2.2 (or as Rich eloquently
put, poop out new configs) only required an hour's worth of work or so
to support upgrades from 2.2 to 2.4 minus the aforementioned authz
directives.

-- 
Daniel Ruggeri

Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c

[Daniel Ruggeri]

On 8/25/2015 10:11 AM, Jim Jagielski wrote:
 Again, if the slotmem exists and is persisted, the assumption
 is that THAT is what the admin wants, and when Apache restarts,
 THAT is the running config they desire. If there are changes
 to the reverse proxy setup, the assumption must be we are
 starting from scratch; There are, iirc, a number of places where
 these kinds of checks are done. Trying to somehow merge a just-changed
 file config and a running config (based on an older file config
 with dynamic changes) is nasty and tough to do correctly.

This is a common problem with serialization. The persisted config is
serialized data of what is in memory. It can only represent the
version of the conf file that created it. If you change the conf,
deserialization should fail because there could be other material
changes that might get in the way.

I'm +1 on the idea that using bgrowth space to add stuff via
conf+graceful restart should be avoided.

-- 
Daniel Ruggeri

Re: Work in progress: mod_proxy Health Check module

[Daniel Ruggeri]

++1 on this vein of thought. Implementing the health checking similar to
the modularized lb methods makes the most sense to me. I can already
think of several methods (file, conf, external script, lua script) of
obtaining data on how to probe and react.

Just some thoughts worth considering... since you asked :-)
*The module should not probe if a probe is already running against that
backend (hang or takes too long to respond)
*The probes to different backends should be done in parallel rather than
serially to avoid pileups due to a slow responder
*How will the existing balancer re-enable logic interfere with ERR backends?
   *Maybe another flag that avoids retry if the monitor marked it down
requiring the monitor to bring it back up before any traffic is allowed
to go to it?

I've followed along with what you described by using watchdog to trigger
the probing, but I'm curious where your thoughts are for submitting a
request to the backend. Are you planning to simulate one from your
module by directly connecting to the backend or is the plan to run the
request through the existing proxy infrastructure (the former allows
lots of control while the latter permits other directives to come into
play that might be handy like disabling based on status code). I haven't
seen the code, but your previous email said you were thinking of the
former case.

P.S.
Thanks for taking this on. It's been on my own todo list for a long time.

-- 
Daniel Ruggeri

On 1/8/2016 1:09 PM, Jim Jagielski wrote:
> Thx! Any other comments/suggestions?
>
>> On Jan 8, 2016, at 10:53 AM, Eric Covener <cove...@gmail.com> wrote:
>>
>> On Fri, Jan 8, 2016 at 10:18 AM, Jim Jagielski <j...@jagunet.com> wrote:
>>> I am thinking about the actual health-check impl a bit more,
>>> hence the lack of commits. As noted, initially I was thinking
>>> about optional functions, defined in mod_proxy_http, mod_proxy_ajp,
>>> etc. Then I started thinking that maybe doing it via providers
>>> might be better, allowing for more "custom" health checks via
>>> the end user. Then I started thinking about doing it rough-and-
>>> ready and just adding 'em in mod_proxy_hcheck...
>>>
>>> Before I spend too much time on this, any prefs one way or
>>> another regarding this?
>> I haven't looked at the architecture so far -- but I think basics in
>> mod_proxy_hcheck
>> and at least one "standalone" provider (as an example) would be a good
>> middle ground.
>>
>> Maybe the standalone one would try to deal with the body whereas the
>> built-in ones
>> would be TCP up, SSL up, status-code, etc.

Re: [POLL] Commitment to 2.2.x lifecycle? (Was: End of the road of 2.2.x maintenance?)

[Daniel Ruggeri]

 *) I intend to help maintain/test 2.2.x releases over the next [_12___] mos
 *) I intend to backport/review 2.2.x security patches over the next
[_18___] mos

-- 
Daniel Ruggeri

Re: [Patch] mod_tcp / mod_proxy_tcp / mod_ssl_tcp

[Daniel Ruggeri]

+1
Really nice work

-- 
Daniel Ruggeri

On 3/13/2016 10:45 AM, Jim Jagielski wrote:
> I've given it a quick look-thru and I. Am. Impressed.
>
> This is more Super Cool Mojo!

Re: Status for 2.4.20

[Daniel Ruggeri]

On 3/23/2016 7:27 AM, Jim Jagielski wrote:
> Release Often is hardly a Bad Thing, at least IMHO. When the
> time is right for a release, then we release. It seemed a
> good time, again IMHO.

Kinda late to the party, but shouldn't what's committed to a stable
branch _always_ be ready to release?



Just my .02 on the side conversation. As a sysadmin, I have no strong
preference for release often versus release seldom... just so long as
what is released is stable and won't break my stuff.
At $dayjob where I have to answer to regulatory and audit authorities,
there have been plenty of releases of various software platforms that I
have chosen not to pursue because they don't address anything I need to
worry about. No harm done.

So, my stance as a sysadmin (and probably a fairly common stance, I'd
guess) can be summarized as:
If there's a bug fix for a problem I'm seeing, I'd really like the
release sooner rather than later. On the other hand, if there's a
security fix, I need it released immediately. On yet another hand
(because it seems I have three), if the release has nothing I care
about... then I don't care.
But whatever happens, don't break my configs :-)



As an even further side note... after following this dev list
(community) as long as I have, I'd happily put an httpd 2.4.nightly
against just about any other released software. I can't think of a
single case where something got *committed* to 2.4 that would break my
configs let alone something that got formally released. Indeed, the few
thousand httpd servers I run haven't been harmed by a release in all of
my time as an admin (1.3, 2.0 and 2.4 alike). Our release process is,
indeed, pretty damn good. So whether we release often or release seldom,
we can at least hold our head high while we do it.

-- 
Daniel Ruggeri

Re: [users@httpd] Strange with AllowOverrideList Directive

[Daniel Ruggeri]

I'm assuming that compiler optimizations would make both patches "six to
one, half dozen to the other" as far as code path followed during the
request cycle... but I agree.

Fixed in trunk in r1737114 and proposed for backport in 2.4 in STATUS.

-- 
Daniel Ruggeri

On 3/30/2016 8:02 AM, Plüm, Rüdiger, Vodafone Group wrote:
>
> But the attached patch is not correct. IMHO it needs to be
>
>  
>
> Index: request.c
>
> ===
>
> --- request.c   (revision 1735931)
>
> +++ request.c   (working copy)
>
> @@ -1009,7 +1009,9 @@
>
>  /* No htaccess in an incomplete root path,
>
>   * nor if it's disabled
>
>   */
>
> -if (seg < startseg || (!opts.override &&
> opts.override_list == NULL)) {
>
> +if (seg < startseg || (!opts.override
>
> +   &&
> apr_is_empty_table(opts.override_list)
>
> +   )) {
>
>  break;
>
>  }
>
>  
>
>  
>
> Regards
>
>  
>
> Rüdiger
>

Re: mod_proxy_hcheck backport

[Daniel Ruggeri]

On 5/16/2016 8:19 AM, Jim Jagielski wrote:
> THANKS! This feature seemed to cause a lot of buzz @ ApacheCon so
> would be

I believe I heard and/or used the term "sexy" at least once to describe
it ;-)


On 5/16/2016 8:19 AM, Jim Jagielski wrote:
> Hmmm... The balancer-manager page does display the configured
> values for 'hcpasses' and 'hcfails' as well as the current count
> of passes/fails. Is that sufficient?

Yeah - didn't think about that. It'd be fine for my purposes. It could
be a PITA if someone is monitoring for a specific string like
"transitive" or "fail", but it's probably not worth monkeying with.

-- 
Daniel Ruggeri

Re: Allow SSLProxy* config in context?

[Daniel Ruggeri]

On 4/13/2016 2:22 PM, Rainer Jung wrote:
>
> We could pass the worker name from mod_proxy to mod_ssl via a
> connection note, similar to currently already passing the SNI name via
> the connection note proxy-request-hostname.

+1 on the connection note idea, but see below about having to inform the
callback function about too much information

On 4/13/2016 10:04 AM, Graham Leggett wrote:
> What I had in mind was a syntax where the certs were named, for example:
>
> SSLProxyCertificate foo /path/to/foo.cert
>
> Followed somewhere else by:
>
> 
>   SSLProxyEnable foo
> 

On one hand, this is an attractive idea because we have already
conditioned users/administrators to think about naming things via config
as you can do with authn providers and aliases.

On another hand, there are two gotchas I see. Giving something a name in
the configuration does not necessarily mean anything to the file or
certificate store because they aren't married in the same way that authn
providers and their aliases are. That is, if a different team/individual
manages the certificate store than the server configuration, the two
would have to keep in sync with additions or removals. This can be a
PITA if you have a short certificate lifecycle and have to renew often.
PLUS, there are cases where there may be many certificates in the file.
Combine that with the previous point and you could have a nightmare.

Perhaps in addition to or instead of aliases via config, something more
"dynamic" could be used where a DN, CN or some other attribute about the
cert that was parsed during startup becomes the representation?


   ...
  #As today
   SSLProxyMachineCertificateFile /path/to/file
   
  #Plus new stuff
  SSLProxyMachineCertificateCN myIdentity
  #or
  SSLProxyMachineCertificateDN /C=US/ST=Missouri/L=St.
Louis/O=BitNebula/CN=DanielRuggeri.bitnebula.com
   


This could be a really clean solution because the proxy can just pass
this name or DN to the ssl module in a note and the ssl module could
make the selection of certs from that data.

Stashing this (completely untested and thrown together) code around line
1778 in modules/ssl/ssl_engine_kernel.c could be all that's needed in
mod_ssl
char *preferred_name;
..
if (preferred_name) {
for (i = 0; i < sk_X509_INFO_num(certs); i++) {
X509_NAME *name;
info = sk_X509_INFO_value(certs, i);
name = X509_get_subject_name(inf->x509);

for(j = 0; j < X509_NAME_entry_count(name); j++) {
const char *this_name = //something
if (strcmp(preferred_name, this_name) == 0) {
modssl_proxy_info_log(c, info, APLOGNO(02279)
  "found acceptable cert");

modssl_set_cert_info(info, x509, pkey);
return TRUE;
}
}
}
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, APLOGNO(00)
SSLPROXY_CERT_CB_LOG_FMT
"server configuration requested certificate name %s"
"but it was not found in the certificate store",
preferred_name);
}



I haven't cracked open the proxy code to see what it would take to
collapse this down to a per proxy/worker/etc, but it doesn't seem like
terrible endeavor.

-- 
Daniel Ruggeri

Re: Autobuild Progress (was Re: Automated tests)

[Daniel Ruggeri]

On 1/31/2017 4:30 PM, Jacob Champion wrote:
> On 01/30/2017 05:39 PM, Daniel Ruggeri wrote:
>> I'm tremendously inspired by this work. What are your thoughts on the
>> idea of having a series of docker container builds that compile and run
>> the test suite on various distributions? I'll volunteer to give this a
>> whack since it's something that's been in the back of my mind for a long
>> while...
>
> I think that would be awesome. The cheaper we can make new test
> distributions, the easier we can test all sorts of different
> configurations (which, given how many knobs and buttons we expose, is
> important).
>
> I don't know how much of Infra's current Puppet/Buildbot framework is
> Docker-friendly, but if there's currently no cheap virtualization
> solution there for build slaves, then anything we added would
> potentially be useful for other ASF projects as well. Definitely
> something to start a conversation over.
>

Yes, definitely. Thinking more about this, even adding something
heavyweight like a type 2 hypervisor could potentially provide value so
long as the VM image is stripped down enough and we don't leave junk
behind on the slave. I'm not concerned about Puppet and buildbot
integration since Puppet is a great way to manage the configuration of
the slave (assuming that's what it's used for) which makes it easy to
have docker, virtualbox, vagrant or whatever installed and configured.

As far as buildbot, I'm sure it will support the execution of a script
which is all that's needed. My latest work with the
RemoteIPProxyProtocol stuff has me compiling httpd on my build machine
and standing up a docker container with haproxy inside. Hitting the
resulting build under various circumstances with wget scratches the
itch. I've got this distilled down into only four files (Dockerfile,
haproxy.cfg, setup script and test script). This is nice because...
well... I just don't want to install haproxy on my build box for this

In any event, I've started the conversation with builds@a.o to see
what's doable. Can crosspost or just return with feedback when I hear.


> (Side thought: since raw speed is typically one of the top priorities
> for a CI test platform, we'd have to carefully consider which items we
> tested by spinning up containers and which we ran directly on a
> physical machine. Though I don't know how fast Docker has gotten with
> all of the fancy virtualization improvements.)

Amen to that. Docker's quite fast since lxc and all the stuff around it
are very lightweight. The slowest parts are pulling the base image and
setting it up (installing compilers, the test framework, tools, etc).
This can be sped up greatly by building the image and publishing it back
to a (or "the") registry or keeping it local on the machine, but we'd
then have to maintain images which I'm not a fan of.


>
>> I think with the work you've done and plan to do, a step like above to
>> increase our ability to test against many distributions all at once (and
>> cheaply) and also making the test framework more approachable, we could
>> seriously increase our confidence when pulling the trigger on a release
>> or accepting a backport.
>
> +1. It'll take some doing (mostly focused on the coverage of the test
> suite itself), but we can get there.
>
>> I'm also a big fan of backports requiring tests, but am honestly
>> intimidated by the testing framework...
>
> What would make it less intimidating for you? (I agree with you, but
> I'm hoping to get your take without biasing it with my already-strong
> opinions. :D)

Opinions here... so take them with a grain of salt.
* The immediate barrier to entry is doco. From the test landing page,
you are greeted with a list of what's in the test project and links to
the components. Of the links there (our super important one is Perl
Framework), only the flood link leads to a useful getting started guide.
This may be lazy and kinda preachy, but not having good developer info
easily accessible is a Bad Thing(tm) since it's a surefire way to scare
off those potentially interested in participating in a project.
* It's also intimidating when a developer realizes they need to learn a
new skill set to create tests. Writing tests for Perl's testing
framework feels archaic, and I'm not sure is a skill many potential
contributors would possess unless they have developed Perl modules for
distribution. I understand the history of the suite so I _get_ why it's
this way... it's just that it is likely a turn-off. Disclaimer: I'm not
saying Perl has a bad testing framework. I have yet to find a testing
framework I'm a big fan of since they all have their idiosyncrasies. No
holy wars, please :-)
* Another barrier that I think is very much worth pointing out is that
several Perl modules must be installed. I have some history fighting
with Cry

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

[Daniel Ruggeri]

On 1/30/2017 4:45 AM, Ruediger Pluem wrote:
> Thinking of all the above it might be best if you read in mode 
> AP_MODE_SPECULATIVE on your own from upstream until
> you have MIN_HDR_LEN data. If the PROXY header is present, read MIN_HDR_LEN 
> in AP_MODE_READBYTES to finally consume the
> data and move on. If the PROXY header is not present, well then just forward 
> the original request and you are fine.
> This way you leave all the hassle to the upstream filters.

Yes, definitely. I was contemplating the same thing given the
permutations of modes it may be called in and the various cases to deal
with. That's the approach I've taken in the latest commit because the
hassle is definitely best left to upstream :-)

At a high level, it no longer stores the data to pass along. When
optional processing is enabled, the filter starts in speculative read
mode. Once MIN_HDR_LEN is read and we know if a header is there or not
we can discard ctx->bb, reinitialize ctx and move to READBYTES mode.

-- 
Daniel Ruggeri

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

[Daniel Ruggeri]

 add = _conf->proxy_protocol_disabled;
>> +rem_list[0] = _conf->proxy_protocol_enabled;
>> +rem_list[1] = _conf->proxy_protocol_optional;
>> +}
>> +else {
>> +return apr_pstrcat(cmd->pool, "Unrecognized option for 
>> ProxyProtocolEnable `%s'", arg, NULL);
> s/ProxyProtocolEnable/RemoteIPProxyProtocolEnable/
> or cmd->cmd->name to be safe.
I like that even better - updated to the safest case

>
>> +}
>> +
>> +for (addr = cmd->server->addrs; addr; addr = addr->next) {
>> +/* remove address from other lists */
>> +for (i = 0; i < list_len ; i++) {
>> +remoteip_addr_info **rem = rem_list[i];
>> +if (*rem) {
>> +if (remoteip_sockaddr_equal((*rem)->addr, addr->host_addr)) 
>> {
>> +remoteip_warn_enable_conflict(*rem, cmd->server, arg);
>> +*rem = (*rem)->next;
>> +    }
>> +else {
>> +for (list = *rem; list->next; list = list->next) {
>> +if (remoteip_sockaddr_equal(list->next->addr, 
>> addr->host_addr)) {
>> +remoteip_warn_enable_conflict(list->next, 
>> cmd->server, arg);
>> +list->next = list->next->next;
>> +break;
>> +}
>> +}
>> +}
>> +}
>> +}
>> +
>> +/* add address to desired list */
>> +if (!remoteip_addr_in_list(*add, addr->host_addr)) {
>> +remoteip_addr_info *info = apr_palloc(global_conf->pool, 
>> sizeof(*info));
> Could cmd->pool be used here, instead?

This came from the original authors of the code, but I think it's
correct. This is the only place remoteip_config_t->pool is allocated
into. A collection of all enabled, disabled and optional
remoteip_addr_info structs is kept and examined pre-connection to
determine if the filter should be inserted for the connection. Since the
server is not known pre-connection, this must be stored in the global
server. The lifetime of cmd->pool would prevent using it here.


>
>> . . . 
>>   static const command_rec remoteip_cmds[] =
>>   {
>>   AP_INIT_TAKE1("RemoteIPHeader", header_name_set, NULL, RSRC_CONF,
>> @@ -450,11 +1211,21 @@ static const command_rec remoteip_cmds[]
>> RSRC_CONF | EXEC_ON_READ,
>> "The filename to read the list of internal proxies, "
>> "see the RemoteIPInternalProxy directive"),
>> +AP_INIT_TAKE1("RemoteIPProxyProtocolEnable", 
>> remoteip_enable_proxy_protocol, NULL,
>> +  RSRC_CONF, "Enable proxy-protocol handling (`on', 
>> `off')"),
> `optional' is missing

Fixed - thanks!


>
>>   { NULL }
>>   };
>>   
>>   static void register_hooks(apr_pool_t *p)
>>   {
>> +/* mod_ssl is CONNECTION + 5, so we want something higher (earlier);
>> + * mod_reqtimeout is CONNECTION + 8, so we want something lower (later) 
>> */
>> +ap_register_input_filter(remoteip_filter_name, remoteip_input_filter, 
>> NULL,
>> + AP_FTYPE_CONNECTION + 7);
>> +
>> +ap_hook_pre_config(remoteip_hook_pre_config, NULL, NULL, 
>> APR_HOOK_MIDDLE);
>> +ap_hook_post_config(remoteip_hook_post_config, NULL, NULL, 
>> APR_HOOK_MIDDLE);
>> +ap_hook_pre_connection(remoteip_hook_pre_connection, NULL, NULL, 
>> APR_HOOK_MIDDLE);
>>   ap_hook_post_read_request(remoteip_modify_request, NULL, NULL, 
>> APR_HOOK_FIRST);
>>   }
>>   

-- 
Daniel Ruggeri

Re: Autobuild Progress (was Re: Automated tests)

[Daniel Ruggeri]

I'm tremendously inspired by this work. What are your thoughts on the
idea of having a series of docker container builds that compile and run
the test suite on various distributions? I'll volunteer to give this a
whack since it's something that's been in the back of my mind for a long
while...

I think with the work you've done and plan to do, a step like above to
increase our ability to test against many distributions all at once (and
cheaply) and also making the test framework more approachable, we could
seriously increase our confidence when pulling the trigger on a release
or accepting a backport.

P.S.

I'm also a big fan of backports requiring tests, but am honestly
intimidated by the testing framework...

-- 
Daniel Ruggeri

On 1/30/2017 2:02 PM, Jacob Champion wrote:
> On 01/02/2017 07:53 AM, Daniel Shahaf wrote:
>> Setting this up isn't a lot more complicated than filing an INFRA ticket
>> with a build script, a list of build dependencies, and a list of
>> branches to build, and deciding how build failures would be notified.
>
> To follow up on this, we now have an operational (if not yet fully
> functional) buildbot for trunk:
>
> https://ci.apache.org/builders/httpd-trunk
>
> There's a lot of work yet to do, but for now we have an Ubuntu machine
> that can be manually triggered to run an incremental build on trunk.
>
> Here's my list of TODOs:
> - run per-commit incremental builds
> - run nightly clean builds
> - run a nightly test suite
> - set up 2.4.x in addition to trunk
> - set up Windows in addition to Ubuntu
>
> == Details ==
>
> The bot is building against Ubuntu-packaged dependencies, which
> requires a new apr-config option for buildconf (run `./buildconf
> --help` on the latest trunk for info). This leaves out a few modules
> that need some bleeding-edge dependencies:
>
> - mod_brotli (needs the unreleased libbrotli)
> - mod_crypto (needs APR 1.6)
> - mod[_proxy]_http2 (needs libnghttp2)
> - mod_lua (needs our configure script to recognize Lua 5.3)
>
> So to run a full test suite, eventually we'll need to build those
> dependencies too. I figure this is a good start for now.
>
> The following modules aren't built because of platform-specific stuff:
>
> - mod_socache_dc (distcache)
> - mod_journald, mod_systemd
> - mod_privileges (sys/priv.h)
> - mpm_os2
> - mpm_winnt
>
> If you'd like to poke around, our buildbot configuration file is in
> the infra repository and is, I believe, open to all our committers:
>
>
> https://svn.apache.org/repos/infra/infrastructure/buildbot/aegis/buildmaster/master1/projects/httpd.conf
>
>
> --Jacob

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

[Daniel Ruggeri]

On 1/25/2017 9:02 PM, Daniel Ruggeri wrote:
> On 1/25/2017 6:53 PM, Daniel Ruggeri wrote:
>> I'd say that not returning until ctx->bb has enough information to
>> determine if the header is present or not would be sufficient. Isn't
>> this already done in the potentially repeated calls to ap_get_brigade on
>> line no 1056 inside the loop verifying that ctx->done is not set? If
>> we're not intentionally holding onto the data until we know it's safe,
>> then I think there's a structural problem in this module that would
>> require us to start doing so.
> Sorry - I neglected to notice the obvious check that the brigade is not
> empty JUST before this line. Nevermind this silly comment...
>
> Indeed, if the brigade runs dry before the minimum number of bytes
> needed has been read, the data should not be deleted.
> I'm thinking apr_bucket_setaside(b, f->c->pool) in place of
> apr_bucket_delete(b) would be good here... I will also experiment with
> having the filter ask for less data than it needs to verify these
> multiple calls through the filter work as expected.
>

Changes to set aside the bucket data are in r1780725. I'll await
updating the 2.4 backport patch with this and the compiler warning fix
when we're good on how to proceed regarding the other email I just sent.

To verify behavior, I forced the filter to receive only a few bytes at a
time until it "built up" enough data to evaluate the header type and
pass all data set aside in bb_out when RemoteIPProxyProtocol is set to
Optional. Thanks for the pointers.

-- 
Daniel Ruggeri

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

[Daniel Ruggeri]

Ruediger Pluem wrote:
> +/* try to read a header's worth of data */
> +while (!ctx->done) {
> +if (APR_BRIGADE_EMPTY(ctx->bb)) {
> +ret = ap_get_brigade(f->next, ctx->bb, ctx->mode, block,
> + ctx->need - ctx->rcvd);
> +if (ret != APR_SUCCESS) {
> +return ret;
> +}
> +}
> +if (APR_BRIGADE_EMPTY(ctx->bb)) {
What about the case of an non blocking read where the upstream filter
returns an empty brigade
and APR_SUCCESS. This is equal to returning EAGAIN.

> +return APR_EOF;
> +}

Coming back to this one after correcting the setaside stuff... Is this
what you have in mind or should we actually return APR_EAGAIN?

return block == APR_NONBLOCK_READ ? APR_SUCCESS : APR_EOF;

-- 
Daniel Ruggeri

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

[Daniel Ruggeri]

To clarify, the issues with handling of the buckets and the fact that
buckets are not being set aside came from the original code. The
question about what to do regarding f->next versus passing the data
along some other way is the only one that came from the code I added to
make the header optional.

Being honest, I don't fully grok all the details of the filter stack and
what kinds of buckets to expect emanating from core as the request for
data propagates up through the input filters, but what Rudiger has
pointed out seems like a structural problem with the module that could
bite users under certain circumstances. What I'm particularly unclear
about is what those circumstances would be.

I'll try to reply to the other thread to provide more clarity.

-- 
Daniel Ruggeri

On 1/24/2017 8:36 AM, Jim Jagielski wrote:
> ++1. I know that Daniel is out of pocket for a little bit so I'l
> give it a coupla more days before I "restore" to the original filter
> code...
>> On Jan 24, 2017, at 3:46 AM, Ruediger Pluem <rpl...@apache.org> wrote:
>>
>>
>>
>> On 01/17/2017 02:48 PM, Jim Jagielski wrote:
>>>> On Jan 16, 2017, at 6:57 PM, Daniel Ruggeri <drugg...@primary.net> wrote:
>>>>
>>>> For the most part, yes except the portions that make the header presence
>>>> optional (the HDR_MISSING case). Those were added as it came into the
>>>> code base to handle a use case I was working on. I've added some
>>>> comments inline since I won't have time to poke around myself for a
>>>> while yet.
>>>>
>>>>
>>>> For convenience, here's a link to the original code
>>>>
>>>> https://github.com/roadrunner2/mod-proxy-protocol/blob/master/mod_proxy_protocol.c
>>>>
>>> Would it make sense to have the "stable" version available
>>> for backport, and keep in the WIP in trunk?
>>>
>> This would be an option, but apart from this I would like to see the WIP in 
>> trunk
>> somehow fixed. Otherwise it is a perfect candidate for falling through the 
>> cracks
>> and giving yet another surprise once we branch "whatever we name it" from 
>> trunk.
>> IMHO easier to fix that now or even revert that part for the time being 
>> while people
>> remember what is going on then later digging through it while hitting the 
>> issues.
>>
>> Regards
>>
>> Rüdiger

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

[Daniel Ruggeri]

First, my apologies, but it looks like line wrapping is going to exceed
the usual number of columns so formatting may get wonky in this reply.

On 1/17/2017 3:48 AM, Plüm, Rüdiger, Vodafone Group wrote:
>
>> -Ursprüngliche Nachricht-
>> Von: Daniel Ruggeri [mailto:drugg...@primary.net]
>> Gesendet: Dienstag, 17. Januar 2017 00:57
>> An: dev@httpd.apache.org
>> Betreff: Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-
>> message-tags/next-number docs/manual/mod/mod_remoteip.xml
>> modules/metadata/mod_remoteip.c
>>
>> For the most part, yes except the portions that make the header presence
>> optional (the HDR_MISSING case). Those were added as it came into the
>> code base to handle a use case I was working on. I've added some
>> comments inline since I won't have time to poke around myself for a
>> while yet.
>>
>>
>> For convenience, here's a link to the original code
>>
>> https://github.com/roadrunner2/mod-proxy-
>> protocol/blob/master/mod_proxy_protocol.c
>>
>>
>> On 1/16/2017 10:14 AM, Jim Jagielski wrote:
>>> Let me take a look... afaict, this is a copy of what
>>> was donated, which has been working for numerous people.
>>> But that doesn't mean it can't have bugs ;)
>>>
>>>> On Jan 16, 2017, at 7:20 AM, Ruediger Pluem <rpl...@apache.org>
>> wrote:
>>>> Anyone?
>> Apologies for missing the original reply
>>
>>>> Regards
>>>>
>>>> Rüdiger
>>>>
>>>> On 01/10/2017 12:39 PM, Ruediger Pluem wrote:
>>>>> On 12/30/2016 03:20 PM, drugg...@apache.org wrote:
>>>>>> Author: druggeri
>>>>>> Date: Fri Dec 30 14:20:48 2016
>>>>>> New Revision: 1776575
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1776575=rev
>>>>>> Log:
>>>>>> Merge new PROXY protocol code into mod_remoteip
>>>>>>
>>>>>> Modified:
>>>>>>httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml
>>>>>>httpd/httpd/trunk/modules/metadata/mod_remoteip.c
>>>>>>
>>>>>>
>> 
>> ==
>>>>>> --- httpd/httpd/trunk/modules/metadata/mod_remoteip.c (original)
>>>>>> +++ httpd/httpd/trunk/modules/metadata/mod_remoteip.c Fri Dec 30
>> 14:20:48 2016
>>>>>> + */
>>>>>> +static int remoteip_hook_pre_connection(conn_rec *c, void *csd)
>>>>>> +{
>>>>>> +remoteip_config_t *conf;
>>>>>> +remoteip_conn_config_t *conn_conf;
>>>>>> +int optional;
>>>>>> +
>>>>>> +conf = ap_get_module_config(ap_server_conf->module_config,
>>>>>> +_module);
>>>>>> +
>>>>>> +/* Used twice - do the check only once */
>>>>>> +optional = remoteip_addr_in_list(conf-
>>> proxy_protocol_optional, c->local_addr);
>>>>>> +
>>>>>> +/* check if we're enabled for this connection */
>>>>>> +if ((!remoteip_addr_in_list(conf->proxy_protocol_enabled, c-
>>> local_addr)
>>>>>> +  && !optional )
>>>>>> +|| remoteip_addr_in_list(conf->proxy_protocol_disabled, c-
>>> local_addr)) {
>>>>>> +return DECLINED;
>>>>>> +}
>>>>>> +
>>>>>> +/* mod_proxy creates outgoing connections - we don't want
>> those */
>>>>>> +if (!remoteip_is_server_port(c->local_addr->port)) {
>>>>>> +return DECLINED;
>>>>>> +}
>>>>> Why is the c->local_addr->port set to a port we are listening on in
>> case of proxy connections?
>>
>> This is from the original code, but wouldn't this be the local port on
>> the outbound connection (some high number)? The remoteip_is_server_port
>> should therefore fail this check.
> My bad I missed the ! in the condition. So this should work.
>
>>>>>> +
>>>>>> +switch (psts) {
>>>>>> +case HDR_MISSING:
>>>>>> +if (conn_conf->proxy_protocol_optional) {
>>>>>> + 

Re: AW: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

[Daniel Ruggeri]

On 1/25/2017 6:53 PM, Daniel Ruggeri wrote:
> I'd say that not returning until ctx->bb has enough information to
> determine if the header is present or not would be sufficient. Isn't
> this already done in the potentially repeated calls to ap_get_brigade on
> line no 1056 inside the loop verifying that ctx->done is not set? If
> we're not intentionally holding onto the data until we know it's safe,
> then I think there's a structural problem in this module that would
> require us to start doing so.
Sorry - I neglected to notice the obvious check that the brigade is not
empty JUST before this line. Nevermind this silly comment...

Indeed, if the brigade runs dry before the minimum number of bytes
needed has been read, the data should not be deleted.
I'm thinking apr_bucket_setaside(b, f->c->pool) in place of
apr_bucket_delete(b) would be good here... I will also experiment with
having the filter ask for less data than it needs to verify these
multiple calls through the filter work as expected.

-- 
Daniel Ruggeri

Re: mod_remoteip and mod_http2 combined

[Daniel Ruggeri]

On 2017-02-15 09:07 (-0600), William A Rowe Jr  wrote: 
> On Wed, Feb 15, 2017 at 9:02 AM, Sander Hoentjen  wrote:
> >
> > mod_remote ip has:
> > /* mod_proxy creates outgoing connections - we don't want those */
> > if (!remoteip_is_server_port(c->local_addr->port)) {
> > return DECLINED;
> > }
> > I am guessing something similar is needed for h2 connections?
> 
> I suspect that the mod_remoteip logic is wrong, that it should be guarding
> against any subordinate connections and examining only explicitly configured
> ports / origin IPs. the PROXY protocol is not part of the HTTP protocol and
> incompatible with it, so the trust list logic isn't directly compatible (this 
> is
> clearly explained in the PROXY pseudo-RFC.)
> 

Hi, Bill. That is what the module is doing. The original authors wrote it to 
have a list of virtual hosts it is explicitly enabled for and explicitly 
disabled for. I added a third list for optional vhosts. In the pre_connection 
hook, it checks to see if the connection's local_addr (which should normally be 
the server's IP) is explicitly configured to enable PROXY handling. It then 
checks to see if the local port is a server port.

Looking at the logs shared, 192.168.122.249:84 is the server IP:Port combo and 
is also the local IP:Port from mod_h2. If h2 sets the master of this 
connection, then we could skip the whole ordeal with this patch:

Index: modules/metadata/mod_remoteip.c
===
--- modules/metadata/mod_remoteip.c (revision 1781701)
+++ modules/metadata/mod_remoteip.c (working copy)
@@ -862,6 +862,10 @@
 remoteip_conn_config_t *conn_conf;
 int optional;

+if (c->master != NULL) {
+return DECLINED;
+}
+
 conf = ap_get_module_config(ap_server_conf->module_config,
 _module);

.. but I don't know if that potentially means we are looking at the wrong 
connection.

Sander, would it be possible to try this out?

Re: svn commit: r1781701 - in /httpd/httpd/trunk: docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

[Daniel Ruggeri]

   else if (ctx->version == 2) {
>> +ctx->need = MIN_V2_HDR_LEN;
>> +}
>>  }
>>  }
>>  }
>
> Other comments:
>
> If you are reading in SPECULATIVE mode and renter the filter (e.g. 
> MIN_HDR_LEN bytes were not available and you were
> reading non blocking) or if you just do a second ap_get_brigade in the outer 
> loop, the returned brigade will contain
> all the data you had already seen plus potential new data. So you don't need 
> to tuck it away in ctx->header.

I think it's still necessary to copy each bucket's data to ctx->header
unless there's a guarantee that the bytes in the first bucket will be
followed in memory by the bytes in the next bucket and so on. Otherwise,
the function that examines the header as a char array may read beyond
the length of a single bucket into unsafe memory. By stashing whatever
we get into ctx->header, we make a nice and neat array for the function
to examine.


> You always
> need to examine the whole returned brigade for the header and the brigade 
> should become longer with a second
> ap_get_brigade.

Seems like resetting rcvd and need in ctx after the outer loop's
ap_get_brigade call would satisfy both cases mentioned above since the
filter would then just fill ctx->header from 0 index and continue asking
for a full header's worth of data.


> If not and you are in non blocking mode no new data was available and you 
> should leave.

I've implemented a counter for data read from the buckets in this
brigade which is compared with the previous amount of data read when in
SPECULATIVE and NONBLOCK. See attached suggested patch. I'm returning
APR_SUCCESS, but it seems like APR_EAGAIN or APR_WOULDBLOCK could be
viable options, but that may be overkill.


> Also take care to handle meta buckets correctly. You could probably hit an 
> EOS bucket which tells you that no more
> data will be delivered.

Indeed - seems EOS would be the only bucket to look out for. I've added
that in the attached patch.


>
> Regards
>
> Rüdiger

Thank you for another review and comments. Let me know if the attached
diff lines up with what you had in mind for the suggestions.

-- 
Daniel Ruggeri

Index: modules/metadata/mod_remoteip.c
===
--- modules/metadata/mod_remoteip.c (revision 1781701)
+++ modules/metadata/mod_remoteip.c (working copy)
@@ -1031,6 +1031,8 @@
 remoteip_parse_status_t psts = HDR_NEED_MORE;
 const char *ptr;
 apr_size_t len;
+apr_size_t this_read = 0; /* Track bytes read in each brigade */
+apr_size_t prev_read = 0;

 if (f->c->aborted) {
 return APR_ECONNABORTED;
@@ -1077,9 +1079,23 @@
 return block == APR_NONBLOCK_READ ? APR_SUCCESS : APR_EOF;
 }

+if (ctx->peeking) {
+ctx->rcvd = 0;
+ctx->need = MIN_HDR_LEN;
+}
+
 while (!ctx->done && !APR_BRIGADE_EMPTY(ctx->bb)) {
 b = APR_BRIGADE_FIRST(ctx->bb);

+if (ctx->peeking && APR_BUCKET_IS_EOS(b)) {
+/* Shortcut - we know no header was found yet and an
+   EOS indicates we never will */
+apr_brigade_destroy(ctx->bb);
+ctx->bb = NULL;
+ctx->done = 1;
+return APR_SUCCESS;
+}
+
 ret = apr_bucket_read(b, , , block);
 if (APR_STATUS_IS_EAGAIN(ret) && block == APR_NONBLOCK_READ) {
 return APR_SUCCESS;
@@ -1091,6 +1107,10 @@
 memcpy(ctx->header + ctx->rcvd, ptr, len);
 ctx->rcvd += len;

+if (ctx->peeking && block == APR_NONBLOCK_READ) {
+this_read += len;
+}
+
 apr_bucket_delete(b);
 psts = HDR_NEED_MORE;

@@ -1103,12 +1123,11 @@
we purge the bb and can decide to step aside or switch 
to
non-speculative read to consume the data */
 if (ctx->peeking) {
-apr_brigade_destroy(ctx->bb);
-
 if (ctx->version < 0) {
 ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, f->c, 
APLOGNO(03512)
   "RemoteIPProxyProtocol: PROXY header 
is missing from "
   "request. Stepping aside.");
+apr_brigade_destroy(ctx->bb);
 ctx->bb = NULL;
 ctx->done = 1;
 return ap_get_brigade(f->next, bb_out, mode, 
block, readbytes);
@@ -1118,10 +1137,10 @@
   

Re: svn commit: r1783256 - /httpd/httpd/branches/2.4.x/STATUS

[Daniel Ruggeri]

On 2/16/2017 11:48 AM, wr...@apache.org wrote:
> Author: wrowe
> Date: Thu Feb 16 17:48:28 2017
> New Revision: 1783256
>
> URL: http://svn.apache.org/viewvc?rev=1783256=rev
> Log:
> Slow two still-wobbly horses
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1783256=1783255=1783256=diff
> ==
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Thu Feb 16 17:48:28 2017
> @@ -167,6 +167,9 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   2.4 convenience patch (includes CHANGES):
>
> http://people.apache.org/~druggeri/patches/RemoteIPProxyProtocol.2.4.x.patch
>   +1: druggeri, jim
> + -1: wrowe (as noted on list, not limiting to inbound primary pre_conn
> +scope correctly; lots of redundant code happpening 
> per-request
> +for a per-connection behavior. Investigating further.)
>  

Hi, Bill;
   I've replied about the pre_connnection situation - hoping someone can
give the proposed patch a test as I don't have a handy H2 testbed.

On the other comment, can you help me understand what redundant code is
happening per-request? When manipulating the request, there are only
four things happening differently:
1. A check that we have data stored away from the connection filter
2. A check that the connection data has a client IP
3. The assignment of the data to the request_rec's structure and logging
at TRACE1
4. If no data was found, a check to see if it was optional and a logging
statement/return according to that result

This should all be quite straight forward per request... In fact, it's a
much shorter logical path and less work than having to parse the
X-Forwarded-For header.


>*) mod_brotli: Backport of mod_brotli filter
>   trunk patch: http://svn.apache.org/r1761714
> @@ -176,6 +179,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>http://svn.apache.org/r1779077
>   2.4.x patch: http://home.apache.org/~jim/patches/brotli-2.4.patch
>   +1: jim, jorton,
> + -1: wrowe (Premature, waiting on github.com/google/brotli stable 
> release)
>   jailletc36: doc should also be back-ported (r1779091 + r1779699)
>  
>*) mod_ssl: work around leaks on (graceful) restart.
>
>

-- 
Daniel Ruggeri