Re: URL's in error pages

2018-04-12 Thread Luca Toscano
2018-04-12 23:52 GMT+02:00 Yann Ylavic :

> On Thu, Apr 12, 2018 at 11:18 PM, Eric Covener  wrote:
> > On Thu, Apr 12, 2018 at 8:33 AM, Daniel Ruggeri 
> wrote:
> >> Since the encoded form is not very useful for humans, I'd sooner remove
> the URL from the page. As you said, we have access_log. As hesitant as I am
> to suggest Yet Another Directive, I also agree that this change should be
> configurable and defaulted to 'Off' for 2.4... no preference on trunk.
> >
> > I had intended opt-out for 2.4 :/
> >
> > Others?
>
> +1 for opt-out, I'm fine with any encoding (as well as remove) by default
> too.
>

+1 for opt-out in 2.4 and remove for trunk!

Luca


Re: URL's in error pages

2018-04-12 Thread Yann Ylavic
On Thu, Apr 12, 2018 at 11:18 PM, Eric Covener  wrote:
> On Thu, Apr 12, 2018 at 8:33 AM, Daniel Ruggeri  wrote:
>> Since the encoded form is not very useful for humans, I'd sooner remove the 
>> URL from the page. As you said, we have access_log. As hesitant as I am to 
>> suggest Yet Another Directive, I also agree that this change should be 
>> configurable and defaulted to 'Off' for 2.4... no preference on trunk.
>
> I had intended opt-out for 2.4 :/
>
> Others?

+1 for opt-out, I'm fine with any encoding (as well as remove) by default too.


Weird formatting when quoting on bz

2018-04-12 Thread Yann Ylavic
In my browser at least, quoting (in reply to) messages and added text
do not mix well on our bugzilla (while emails on bugs@ looks good).

A blank line is automagically added after the quote, but none before
the next one, so it doesn't help putting replies in context in both bz
and emails..

Any idea where we could "fix" this (if it's not a local issue only)?


Re: URL's in error pages

2018-04-12 Thread Eric Covener
On Thu, Apr 12, 2018 at 8:33 AM, Daniel Ruggeri  wrote:
> Since the encoded form is not very useful for humans, I'd sooner remove the 
> URL from the page. As you said, we have access_log. As hesitant as I am to 
> suggest Yet Another Directive, I also agree that this change should be 
> configurable and defaulted to 'Off' for 2.4... no preference on trunk.

I had intended opt-out for 2.4 :/

Others?


Re: URL's in error pages

2018-04-12 Thread William A Rowe Jr
On Thu, Apr 12, 2018 at 1:14 PM, Ruediger Pluem  wrote:
>
> From an ops point of view:
>
> You do not always have an address bar visible with the affected URL. Think of 
> iframes or pop ups without address bars
> and people are bad in providing the exact point of time when the issue 
> happened and hence the access logs are a tedious
> business here on a busy server as a source for determining the issue. So I am 
> for re-encode the decoded URI so spaces
> can't be used.
> I don't like making admins life hard because of not so smart tools or people 
> reporting to security :-)

>From an ops point of view...

see access log. I don't see this information as helpful for admin/ops.

Perhaps useful for content creators with no such access. But I'd expect
the vast majority of these are 1) ignored and 2) intentional/malicious
probes.


Re: 2.4.3x regression w/SSL vhost configs

2018-04-12 Thread Ruediger Pluem


On 04/12/2018 09:28 AM, Joe Orton wrote:
> On Wed, Apr 11, 2018 at 10:24:23PM +0200, Yann Ylavic wrote:
>> On Wed, Apr 11, 2018 at 7:54 PM, Joe Orton  wrote:
>>> Yes, exactly - and for affected configs the defining feature is the
>>> absence of SSL* in the second vhost.  The non-SSL config still takes
>>> effect as before.
>>
>> Does it still work with SNI sent by the client (i.e. when negotiation
>> should be based on the second NVH's SSL config)?
> 
> Not sure how to test nbvh selection based off SNI rather than Host:?  
> I'm testing with:
> 
>ErrorDocument 404 "default-ssl\n"
> 
> in the default SSL vhost for :8043 followed by:
> 
> 
>   ServerName whatever.localdomain:8043
>   ErrorDocument 404 "non-default\n"
> 
> 
> I've also changed the logging to log %{HTTPS}e and %{SSL_TLS_SNI}e.  And 
> so:
> 
> $ curl -k https://localhost.localdomain:8043/agag
> default-ssl
> $ curl -k https://whatever.localdomain:8043/agag
> non-default
> 
> ... this works.  Also I can still trigger the 421 if SNI name & Host are 
> different.
> 
> But logged is:
> 
> ::1 - - [12/Apr/2018:08:11:12 +0100] "GET /agag HTTP/1.1" 404 12 HTTPS=on 
> SNI=localhost.localdomain
> 127.0.0.1 - - [12/Apr/2018:08:11:15 +0100] "GET /agag HTTP/1.1" 404 12 
> HTTPS=- SNI=-
> 
> Now mod_ssl only sees the "off" SSLSrvConfigRec in the second vhost so 
> the logging is wrong.

What does the same test result in with 2.4.29?

Regards

Rüdiger



Re: URL's in error pages

2018-04-12 Thread Ruediger Pluem


On 04/12/2018 02:08 PM, Yann Ylavic wrote:
> On Thu, Apr 12, 2018 at 1:46 PM, Eric Covener  wrote:
>>
>> Here are a few options to silencing these scans/reports:
>>
> [X] remove the URL's
> 
> The URL is already in the address bar if any screenshot/report matters, IMHO.
> 

>From an ops point of view:

You do not always have an address bar visible with the affected URL. Think of 
iframes or pop ups without address bars
and people are bad in providing the exact point of time when the issue happened 
and hence the access logs are a tedious
business here on a busy server as a source for determining the issue. So I am 
for re-encode the decoded URI so spaces
can't be used.
I don't like making admins life hard because of not so smart tools or people 
reporting to security :-)

Regards

Rüdiger


Re: unsubscribe

2018-04-12 Thread Luca Toscano
As any other Apache project, you can find the instructions about how to
unsubscribe in http://httpd.apache.org/lists.html#http-dev

Luca

2018-04-12 17:35 GMT+02:00 Ray Jender :

> Please remove me from this mailing list!
>


unsubscribe

2018-04-12 Thread Ray Jender
Please remove me from this mailing list!



Re: URL's in error pages

2018-04-12 Thread Nick Kew

> On 12 Apr 2018, at 12:46, Eric Covener  wrote:
> 
> Scanners at $dayjob (and reports on security@) frequently report that
> built-in error documents suffer from non-xss HTML injection from the
> request URL.

Deja vu there.  I’m sure we’ve fixed some such, and done a grep on
the errordocs repo.  I guess the continuing flow comes from the
multiplicity of ways we might generate an error page.

> Here are a few options to silencing these scans/reports:

One more: insert an output filter when generating an error page.
Escape URLs and scripts to plain text and highlight them.
OK, it’s an overhead, but error pages are small.

A sysop could of course have the option to disable it.

— 
Nick Kew

Win Code Analyses Trunk some modules

2018-04-12 Thread Steffen


I reported before warnings from 2.4.33, see   
http://apache-http-server.18135.x6.nabble.com/Build-warnings-2-4-33-Win32-td5042506.html


For your info:

We have run on Trunk ( revision 1828799) some modules the GUI code 
analyses: mod_cache_socache mod_ssl mod_proxy mod_md mod_remoteip 
mod_http2 mod_filter and mod_lua


There is popping up quite some advises above the reported  regular 
warnings, see attachment.



Steffen

 Trunk revision 
1828799 

mod_cache_socache

c:\vc15\win32\httpd-trunk\modules\cache\mod_cache_socache.c(699): warning 
C6387: 'nkey' could be '0':  this does not adhere to the specification for the 
function 'strlen'. 


mod_proxy

c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(187): warning C6246: Local 
declaration of 's' hides declaration of the same name in outer scope. For 
additional information, see previous declaration at line '96' of 
'c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c'.
c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(1062): warning C6246: Local 
declaration of 'access_status' hides declaration of the same name in outer 
scope. For additional information, see previous declaration at line '1020' of 
'c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c'.
c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(1069): warning C6246: Local 
declaration of 'access_status' hides declaration of the same name in outer 
scope. For additional information, see previous declaration at line '1020' of 
'c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c'.
c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(1640): warning C6340: 
Mismatch on sign: 'int' passed as _Param_(3) when some unsigned type is 
required in call to 'sscanf'.
c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(1841): warning C6246: Local 
declaration of 'err' hides declaration of the same name in outer scope. For 
additional information, see previous declaration at line '1734' of 
'c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c'.
c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(1849): warning C6246: Local 
declaration of 'err' hides declaration of the same name in outer scope. For 
additional information, see previous declaration at line '1734' of 
'c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c'.
c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(1860): warning C6246: Local 
declaration of 'err' hides declaration of the same name in outer scope. For 
additional information, see previous declaration at line '1734' of 
'c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c'.
c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(1893): warning C6246: Local 
declaration of 'err' hides declaration of the same name in outer scope. For 
additional information, see previous declaration at line '1734' of 
'c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c'.
c:\vc15\win32\httpd-trunk\modules\proxy\mod_proxy.c(1744): warning C28183: 
'word' could be '0', and is a copy of the value found in 'f':  this does not 
adhere to the specification for the function 'strcmp'. 
c:\vc15\win32\httpd-trunk\modules\proxy\proxy_util.c(2572): warning C6262: 
Function uses '16428' bytes of stack:  exceeds /analyze:stacksize '16384'.  
Consider moving some data to heap.
c:\vc15\win32\httpd-trunk\modules\proxy\proxy_util.c(2646): warning C6054: 
String 'code_str' might not be zero-terminated.
c:\vc15\win32\httpd-trunk\modules\proxy\proxy_util.c(2870): warning C6011: 
Dereferencing NULL pointer 'backend_addr'. 
c:\vc15\win32\httpd-trunk\modules\proxy\proxy_util.c(3644): warning C6246: 
Local declaration of 'buf' hides declaration of the same name in outer scope. 
For additional information, see previous declaration at line '3479' of 
'c:\vc15\win32\httpd-trunk\modules\proxy\proxy_util.c'.


mod_remoteIP

c:\vc15\win32\httpd-trunk\modules\metadata\mod_remoteip.c(707): warning C6011: 
Dereferencing NULL pointer 'req'. 

mod_ssl

c:\vc15\win32\httpd-trunk\modules\ssl\ssl_engine_init.c(400): warning C6011: 
Dereferencing NULL pointer 'sc->server'. 
c:\vc15\win32\httpd-trunk\modules\ssl\ssl_engine_init.c(1233): warning C6244: 
Local declaration of 'dhparams' hides previous declaration at line '108' of 
'c:\vc15\win32\httpd-trunk\modules\ssl\ssl_engine_init.c'.
c:\vc15\win32\httpd-trunk\modules\ssl\ssl_engine_init.c(1842): warning C6246: 
Local declaration of 'i' hides declaration of the same name in outer scope. For 
additional information, see previous declaration at line '1730' of 
'c:\vc15\win32\httpd-trunk\modules\ssl\ssl_engine_init.c'.
c:\vc15\win32\httpd-trunk\modules\ssl\ssl_engine_pphrase.c(193): warning C6246: 
Local declaration of 'asn1' hides declaration of the same name in outer scope. 
For additional information, see previous declaration at line '139' of 
'c:\vc15\win32\httpd-trunk\modules\ssl\ssl_engine_pphrase.c'.
c:\vc15\win32\httpd-trunk\modules\ssl\ssl_util.c(179): warning C6001: Using 
uninitialized memory 'finfo'.

mod_http2

c:\vc15\

Re: SNI normalization?

2018-04-12 Thread Stefan Eissing
Regarding this, I wrote the attached patch that adds a new method

AP_DECLARE(apr_status_t) ap_normalize_hostname(conn_rec *c, const char 
**phostname);

to http_vhost.h with some internal rewiring so that request_rec fix_hostname()
and this method have a common base.



sni_fixup_hostname.patch
Description: Binary data


If this looks ok, mod_ssl should invoke that on SNI names before further 
processing.

Cheers,

Stefan

> Am 11.04.2018 um 12:22 schrieb Yann Ylavic :
> 
> ISTR that the RFC about SNI forbids port numbers (I find it
> unfortunate as a matter of fact, given that host names may contain
> ports...).
> Just to say that normalization may come with ports handling/relaxing
> in several places, which I support!
> 
> On Wed, Apr 11, 2018 at 11:52 AM, Plüm, Rüdiger, Vodafone Group
>  wrote:
>> I guess this makes sense to avoid these kind of issues.
>> 
>> Regards
>> 
>> Rüdiger
>> 
>>> -Ursprüngliche Nachricht-
>>> Von: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>>> Gesendet: Mittwoch, 11. April 2018 11:49
>>> An: dev@httpd.apache.org
>>> Betreff: SNI normalization?
>>> 
>>> Feedback desired:
>>> 
>>> Checking my server logs, I regularly see clients using SNI with port
>>> identifier,
>>> as in: test.example.org:443
>>> 
>>> I am not sure what client that is, but we do not identify the vhost that
>>> is
>>> (probably) intended. Then the request comes in, and there we have magic
>>> that
>>> finds the correct r->server. Then we mod_ssl sees that sslconn->server
>>> != r->server
>>> and does some compatibility checks. If the base server and vhost have
>>> incompatible
>>> settings (e.g. other certs/ciphers etc.), the request fails.
>>> 
>>> This seems to be wrong. Do we need the same normalization that we have
>>> in Host: header
>>> parsing in SNI?
>>> 
>>> -Stefan



Re: URL's in error pages

2018-04-12 Thread Daniel Ruggeri
Since the encoded form is not very useful for humans, I'd sooner remove the URL 
from the page. As you said, we have access_log. As hesitant as I am to suggest 
Yet Another Directive, I also agree that this change should be configurable and 
defaulted to 'Off' for 2.4... no preference on trunk.
-- 
Daniel Ruggeri

On April 12, 2018 6:46:35 AM CDT, Eric Covener  wrote:
>Scanners at $dayjob (and reports on security@) frequently report that
>built-in error documents suffer from non-xss HTML injection from the
>request URL.
>
>Here are a few options to silencing these scans/reports:
>
>[ ] remove the URL's
>[ ] truncate them
>[ ] put them in HTML comments
>[ ] use CSS to make some -like tag
>[ ] use CSS to make the URL non-selectable/copyable
>[ ] base64 encode the URL's and surround with some text that says its
>only useful for the webserver administrator.
>[ ] use r->the_request or r->unparsed_uri or re-encode the decoded URI
>so spaces can't be used.
>
>I was initially leaning towards the CSS options, but after tinkering
>with a spoiler tag you still have something tempting to copy/paste.
>
>Now I am thinking base64  + html comments is the best. This does make
>screenshots of error documents kind of useless, but we still have
>access logs.
>
>We could also make all of this configurable so whatever obfuscates the
>URL could provide different methods.
>
>Any other ideas / preferences?
>
>-- 
>Eric Covener
>cove...@gmail.com


Re: URL's in error pages

2018-04-12 Thread Jim Jagielski
In order of pref I'd say:

  o base64 encode the URL's and surround with some text that says its only 
useful for the webserver administrator.
  o remove the URLs

> On Apr 12, 2018, at 7:46 AM, Eric Covener  wrote:
> 
> Scanners at $dayjob (and reports on security@) frequently report that
> built-in error documents suffer from non-xss HTML injection from the
> request URL.
> 
> Here are a few options to silencing these scans/reports:
> 
> [ ] remove the URL's
> [ ] truncate them
> [ ] put them in HTML comments
> [ ] use CSS to make some -like tag
> [ ] use CSS to make the URL non-selectable/copyable
> [ ] base64 encode the URL's and surround with some text that says its
> only useful for the webserver administrator.
> [ ] use r->the_request or r->unparsed_uri or re-encode the decoded URI
> so spaces can't be used.
> 
> I was initially leaning towards the CSS options, but after tinkering
> with a spoiler tag you still have something tempting to copy/paste.
> 
> Now I am thinking base64  + html comments is the best. This does make
> screenshots of error documents kind of useless, but we still have
> access logs.
> 
> We could also make all of this configurable so whatever obfuscates the
> URL could provide different methods.
> 
> Any other ideas / preferences?
> 
> -- 
> Eric Covener
> cove...@gmail.com



Re: svn commit: r1828926 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_proxy.xml include/ap_mmn.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h modules/proxy/mod_proxy_http.c

2018-04-12 Thread Jim Jagielski
If only for trunk then I would say Yes, lets optimize these struct fields.

> On Apr 11, 2018, at 3:14 PM, Eric Covener  wrote:
> 
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy.h (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.h Wed Apr 11 19:11:52 2018
>> @@ -459,6 +459,8 @@ typedef struct {
>> char  secret[PROXY_WORKER_MAX_SECRET_SIZE]; /* authentication secret 
>> (e.g. AJP13) */
>> char  upgrade[PROXY_WORKER_MAX_SCHEME_SIZE];/* upgrade protocol used 
>> by mod_proxy_wstunnel */
>> char  hostname_ex[PROXY_RFC1035_HOSTNAME_SIZE];  /* RFC1035 
>> compliant version of the remote backend address */
>> +apr_size_t   response_field_size; /* Size of proxy response buffer in 
>> bytes. */
>> +unsigned int response_field_size_set:1;
>> } proxy_worker_shared;
> 
> 
> If this is for trunk only, should I move the bit field up and call it
> major?  I don't plan to backport it.
> Whether I move it or not, should I reserve the next range of bytes after it?



Re: URL's in error pages

2018-04-12 Thread Yann Ylavic
On Thu, Apr 12, 2018 at 1:46 PM, Eric Covener  wrote:
>
> Here are a few options to silencing these scans/reports:
>
[X] remove the URL's

The URL is already in the address bar if any screenshot/report matters, IMHO.


URL's in error pages

2018-04-12 Thread Eric Covener
Scanners at $dayjob (and reports on security@) frequently report that
built-in error documents suffer from non-xss HTML injection from the
request URL.

Here are a few options to silencing these scans/reports:

[ ] remove the URL's
[ ] truncate them
[ ] put them in HTML comments
[ ] use CSS to make some -like tag
[ ] use CSS to make the URL non-selectable/copyable
[ ] base64 encode the URL's and surround with some text that says its
only useful for the webserver administrator.
[ ] use r->the_request or r->unparsed_uri or re-encode the decoded URI
so spaces can't be used.

I was initially leaning towards the CSS options, but after tinkering
with a spoiler tag you still have something tempting to copy/paste.

Now I am thinking base64  + html comments is the best. This does make
screenshots of error documents kind of useless, but we still have
access logs.

We could also make all of this configurable so whatever obfuscates the
URL could provide different methods.

Any other ideas / preferences?

-- 
Eric Covener
cove...@gmail.com


Re: 2.4.3x regression w/SSL vhost configs

2018-04-12 Thread Stefan Eissing


> Am 12.04.2018 um 12:49 schrieb Yann Ylavic :
> 
> On Thu, Apr 12, 2018 at 11:34 AM, Stefan Eissing
>  wrote:
>> 
>> 
>>> Am 12.04.2018 um 11:23 schrieb Yann Ylavic :
>>> 
>>> Hi Stefan,
>>> 
>>> On Thu, Apr 12, 2018 at 11:09 AM, Stefan Eissing
>>>  wrote:
 
> Am 11.04.2018 um 22:24 schrieb Yann Ylavic :
> 
> On Wed, Apr 11, 2018 at 7:54 PM, Joe Orton  wrote:
>> 
>> Is mod_md expected to work for vhosts without "SSLEngine on/optional"
>> configured explicitly?  Didn't get a clear answer to this before.
> 
> Dunno, but wouldn't be worried to much is that were a new requirement
> for it to work explicitely.
 
 I think mod_md will survive if mod_ssl switches off the new flag. mod_md
 itself however uses it and needs the functionality.
>>> 
>>> I think it was less about AP_MODULE_FLAG_ALWAYS_MERGE than whether
>>> mod_md should work/handle (or not) for vhosts switched from SSLEngine
>>> "undef" to "on" implicitely (i.e. the ssl_init_Module() code patched
>>> by Joe), than .
>>> 
>>> My opinion is that if it did work in 2.4.33 (but wouldn't anymore
>>> after the patch), it's not really an issue because mod_md is
>>> experimental still and, more importantly, you like things to be said
>>> explicitly :)
>> 
>> :)
>> 
>> If memory serves me well, the mod_ssl issue really came up when I
>> epxerimented with a global "SSLEngine *:443" where individual
>> vhosts no longer needed any SSL* basically...
> 
> This is merging, so not really this implicit case I think.

You lost me there...


Re: 2.4.3x regression w/SSL vhost configs

2018-04-12 Thread Yann Ylavic
On Thu, Apr 12, 2018 at 11:34 AM, Stefan Eissing
 wrote:
>
>
>> Am 12.04.2018 um 11:23 schrieb Yann Ylavic :
>>
>> Hi Stefan,
>>
>> On Thu, Apr 12, 2018 at 11:09 AM, Stefan Eissing
>>  wrote:
>>>
 Am 11.04.2018 um 22:24 schrieb Yann Ylavic :

 On Wed, Apr 11, 2018 at 7:54 PM, Joe Orton  wrote:
>
> Is mod_md expected to work for vhosts without "SSLEngine on/optional"
> configured explicitly?  Didn't get a clear answer to this before.

 Dunno, but wouldn't be worried to much is that were a new requirement
 for it to work explicitely.
>>>
>>> I think mod_md will survive if mod_ssl switches off the new flag. mod_md
>>> itself however uses it and needs the functionality.
>>
>> I think it was less about AP_MODULE_FLAG_ALWAYS_MERGE than whether
>> mod_md should work/handle (or not) for vhosts switched from SSLEngine
>> "undef" to "on" implicitely (i.e. the ssl_init_Module() code patched
>> by Joe), than .
>>
>> My opinion is that if it did work in 2.4.33 (but wouldn't anymore
>> after the patch), it's not really an issue because mod_md is
>> experimental still and, more importantly, you like things to be said
>> explicitly :)
>
> :)
>
> If memory serves me well, the mod_ssl issue really came up when I
> epxerimented with a global "SSLEngine *:443" where individual
> vhosts no longer needed any SSL* basically...

This is merging, so not really this implicit case I think.


Re: t/ssl/proxy.t

2018-04-12 Thread Stefan Eissing
Forget it. It was the usual openssl linked vs. openssl in $PATH mixup...

> Am 12.04.2018 um 12:17 schrieb Stefan Eissing :
> 
> Does that work for anyone against a trunk server right now?
> 
> On my MacOS, I get:
> 
>> curl -k http://localhost:8555/
> 
> 
> 504 Proxy Error
> 
> Proxy Error
> The gateway did not receive a timely response
> from the upstream server or application.
> 
> 



t/ssl/proxy.t

2018-04-12 Thread Stefan Eissing
Does that work for anyone against a trunk server right now?

On my MacOS, I get:

> curl -k http://localhost:8555/


504 Proxy Error

Proxy Error
The gateway did not receive a timely response
from the upstream server or application.




Re: 2.4.3x regression w/SSL vhost configs

2018-04-12 Thread Stefan Eissing


> Am 12.04.2018 um 11:23 schrieb Yann Ylavic :
> 
> Hi Stefan,
> 
> On Thu, Apr 12, 2018 at 11:09 AM, Stefan Eissing
>  wrote:
>> 
>>> Am 11.04.2018 um 22:24 schrieb Yann Ylavic :
>>> 
>>> On Wed, Apr 11, 2018 at 7:54 PM, Joe Orton  wrote:
 
 Is mod_md expected to work for vhosts without "SSLEngine on/optional"
 configured explicitly?  Didn't get a clear answer to this before.
>>> 
>>> Dunno, but wouldn't be worried to much is that were a new requirement
>>> for it to work explicitely.
>> 
>> I think mod_md will survive if mod_ssl switches off the new flag. mod_md
>> itself however uses it and needs the functionality.
> 
> I think it was less about AP_MODULE_FLAG_ALWAYS_MERGE than whether
> mod_md should work/handle (or not) for vhosts switched from SSLEngine
> "undef" to "on" implicitely (i.e. the ssl_init_Module() code patched
> by Joe), than .
> 
> My opinion is that if it did work in 2.4.33 (but wouldn't anymore
> after the patch), it's not really an issue because mod_md is
> experimental still and, more importantly, you like things to be said
> explicitly :)

:)

If memory serves me well, the mod_ssl issue really came up when I
epxerimented with a global "SSLEngine *:443" where individual
vhosts no longer needed any SSL* basically...

-Stefan



Re: 2.4.3x regression w/SSL vhost configs

2018-04-12 Thread Yann Ylavic
Hi Stefan,

On Thu, Apr 12, 2018 at 11:09 AM, Stefan Eissing
 wrote:
>
>> Am 11.04.2018 um 22:24 schrieb Yann Ylavic :
>>
>> On Wed, Apr 11, 2018 at 7:54 PM, Joe Orton  wrote:
>>>
>>> Is mod_md expected to work for vhosts without "SSLEngine on/optional"
>>> configured explicitly?  Didn't get a clear answer to this before.
>>
>> Dunno, but wouldn't be worried to much is that were a new requirement
>> for it to work explicitely.
>
> I think mod_md will survive if mod_ssl switches off the new flag. mod_md
> itself however uses it and needs the functionality.

I think it was less about AP_MODULE_FLAG_ALWAYS_MERGE than whether
mod_md should work/handle (or not) for vhosts switched from SSLEngine
"undef" to "on" implicitely (i.e. the ssl_init_Module() code patched
by Joe), than .

My opinion is that if it did work in 2.4.33 (but wouldn't anymore
after the patch), it's not really an issue because mod_md is
experimental still and, more importantly, you like things to be said
explicitly :)


Re: 2.4.3x regression w/SSL vhost configs

2018-04-12 Thread Stefan Eissing


> Am 11.04.2018 um 22:24 schrieb Yann Ylavic :
> 
> On Wed, Apr 11, 2018 at 7:54 PM, Joe Orton  wrote:
>> On Wed, Apr 11, 2018 at 01:37:22PM -0400, Eric Covener wrote:
>>> On Wed, Apr 11, 2018 at 1:07 PM, Yann Ylavic  wrote:
 On Wed, Apr 11, 2018 at 7:03 PM, Joe Orton  wrote:
> Like this?  Is this likely to break some other currently-working config?
> 
> Index: modules/ssl/ssl_engine_init.c
> ===
> --- modules/ssl/ssl_engine_init.c   (revision 1828914)
> +++ modules/ssl/ssl_engine_init.c   (working copy)
> @@ -261,7 +261,8 @@
>  * the protocol is https. */
> if (ap_get_server_protocol(s)
> && strcmp("https", ap_get_server_protocol(s)) == 0
> -&& sc->enabled == SSL_ENABLED_UNSET) {
> +&& sc->enabled == SSL_ENABLED_UNSET
> +&& (!apr_is_empty_array(sc->server->pks->cert_files))) {
> sc->enabled = SSL_ENABLED_TRUE;
> }
 
 So now your configuration would work because the second vhost wouldn't
 have SSL enabled?
 But doesn't the user want SSL on this vhost in the first place?
>>> 
>>> If they worked before, it seems like they were relying on a handshake
>>> with the default VH for the NVH -- which they still get?
>> 
>> Yes, exactly - and for affected configs the defining feature is the
>> absence of SSL* in the second vhost.  The non-SSL config still takes
>> effect as before.
> 
> Does it still work with SNI sent by the client (i.e. when negotiation
> should be based on the second NVH's SSL config)?
> 
>> 
>> This seems to work for the trivial test cases I have based off user
>> reports, but I'm worried this is going to based some other case for
>> which the implicit-on is still needed.
> 
> Maybe the test could be based off the "base server" (read future
> c->base_server, or first of the NVH, not the base_server pointer in
> ssl_init_Module() which is really the main server) if we could
> determine that at ssl_init_Module() time? Something like
> (!apr_is_empty_array(sc->server->pks->cert_files) || "base
> server"->sc->enabled), but I don't see another example where "base
> server" is determined/needed at load time...
> 
>> 
>> Is mod_md expected to work for vhosts without "SSLEngine on/optional"
>> configured explicitly?  Didn't get a clear answer to this before.
> 
> Dunno, but wouldn't be worried to much is that were a new requirement
> for it to work explicitely.

I think mod_md will survive if mod_ssl switches off the new flag. mod_md
itself however uses it and needs the functionality.

Cheers, Stefan






Re: 2.4.3x regression w/SSL vhost configs

2018-04-12 Thread Joe Orton
On Wed, Apr 11, 2018 at 10:24:23PM +0200, Yann Ylavic wrote:
> On Wed, Apr 11, 2018 at 7:54 PM, Joe Orton  wrote:
> > Yes, exactly - and for affected configs the defining feature is the
> > absence of SSL* in the second vhost.  The non-SSL config still takes
> > effect as before.
> 
> Does it still work with SNI sent by the client (i.e. when negotiation
> should be based on the second NVH's SSL config)?

Not sure how to test nbvh selection based off SNI rather than Host:?  
I'm testing with:

   ErrorDocument 404 "default-ssl\n"

in the default SSL vhost for :8043 followed by:


  ServerName whatever.localdomain:8043
  ErrorDocument 404 "non-default\n"


I've also changed the logging to log %{HTTPS}e and %{SSL_TLS_SNI}e.  And 
so:

$ curl -k https://localhost.localdomain:8043/agag
default-ssl
$ curl -k https://whatever.localdomain:8043/agag
non-default

... this works.  Also I can still trigger the 421 if SNI name & Host are 
different.

But logged is:

::1 - - [12/Apr/2018:08:11:12 +0100] "GET /agag HTTP/1.1" 404 12 HTTPS=on 
SNI=localhost.localdomain
127.0.0.1 - - [12/Apr/2018:08:11:15 +0100] "GET /agag HTTP/1.1" 404 12 HTTPS=- 
SNI=-

Now mod_ssl only sees the "off" SSLSrvConfigRec in the second vhost so 
the logging is wrong.

I'm now wondering if this partial fix is going to be worse than not 
starting in these configs, since the config could rely on the SSL 
variables being "correct".

> > This seems to work for the trivial test cases I have based off user
> > reports, but I'm worried this is going to based some other case for
> > which the implicit-on is still needed.
> 
> Maybe the test could be based off the "base server" (read future
> c->base_server, or first of the NVH, not the base_server pointer in
> ssl_init_Module() which is really the main server) if we could
> determine that at ssl_init_Module() time? Something like
> (!apr_is_empty_array(sc->server->pks->cert_files) || "base
> server"->sc->enabled), but I don't see another example where "base
> server" is determined/needed at load time...

I had thoughts along similar lines but I also can't see how the "base" 
server_rec is ever accessible here.

Regards, Joe