Re: URL's in error pages
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
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
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
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
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
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
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
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
Please remove me from this mailing list!
Re: URL's in error pages
> 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
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?
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
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
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
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
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
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
> 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
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
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
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
> 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
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
> 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
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