[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: https vhosts

First off, I need to call this out to point you to folks who have walked

That said, our "radically pedantic" switch with HttpProtocolOptions Strict
inflicts such changes on users. Some will be unhappy, but if they serve
the constraints of the host, it isn't our job to accommodate every case of
misrouted traffic or misimplemented clients.

Looking at the gawdawful list of SSL directives, it is overtime to finally
implement [TLS|SSL]ProtocolOptions directive (not really SSLOptions,
as those doesn't alter the protocol at all.) There are a half or baker's
dozen of directives that were simply toggles, and should have been
rolled into a single protocol behavior description.

On Fri, May 25, 2018 at 3:34 AM, Stefan Eissing <stefan.eissing@xxxxxxxxxxxxx> wrote:

> Am 24.05.2018 um 14:22 schrieb Yann Ylavic <ylavic.dev@xxxxxxxxx>:
> On Thu, May 24, 2018 at 2:09 PM, Eric Covener <covener@xxxxxxxxx> wrote:
>> Thinking about base server and how scanners report it the "vulnerability"...
>> AllowUnmatchedHost[name]?
>> RejectUnknownHost[name]?
> The one or the other is probably a better name than UseDefaultVHost,
> it allows to specify it by vhost (really meaningful on base servers
> though) OR globally to avoid using fake base servers in the whole
> configuration (when relevant).
> Looks good to me.

Like it. For simplicity, I think it should only be global.
-1; this has physical vhost scope... What you do on an internally bound monitoring / internal consumers / routed external / or outward facing traffic can all behave quite differently with legitimate purpose. Clients can't send an SNI with a by-IP "name". This is fine to prevent external abuse but breaks many examples/tutorials.
Using it in the first vhost has the same effect. Using it
in a subsequent vhost will have no effect. Does not really make sense, or?

We get it, when NameVirtualHost was removed, people got confused. But the first matching phys vhost continues to control across mod_ssl, http, http2, etc etc. Admins need to learn this difference to understand how their config behaves. If our docs don't get this right, shame on us (but I think they do).

It would be cool to actually flag those directives which are exclusive to physical vhosts, and not applicable after named vhost routing. We could amplify the signal from debug -> info -> warning to caution admins that their config never did what they expected it would.

So, proposal:

RejectUnknownHosts [ on | off ]   (Default: off)

as core directive.

I see three possible behaviors; treat as a physical vhost match (today's behavior), fall out to global match, or reject.

There is another possibility, in the much longer term. Demand users to use ServerAlias * to clearly indicate their preferred default vhost, treat the absence as a fall-back to global, which can be easily configured to reject everything or serve a diagnostic to admins.

So, we are proposing (my reading) that any protocol module can inspect and react to this directive. So this would be implemented at the core, and in mod_ssl (perhaps again in h2?) and any other connection or request handler?

Core http would respond 400 Bad Request (connection: close), with almost no response headers. [If we were to define a new error, because a fitting one doesn't exist, I'd title the RFC Dave's Not Here, a Cheech and Chong joke corresponding to Twitter's 420 response.]

Before this, mod_ssl used to respond SSL_TLSEXT_ERR_NOACK, so this is simply a transition to SSL_TLSEXT_ERR_ALERT_FATAL in the unrecognized SNI hostname case.

I'd prefer that the SSL fatal no-handshake toggle lives in an SSLProtocolOptions and we migrate all those binary/tristate decisions to one place. Absent this flag, your example still fails at the http layer with an http 400 response.

For mod_ssl, it would be good to move its "ssl_find_vhost()" partially
into the core. We could add a method

AP_DECLARE(server_rec *) ap_vhost_find_server(conn_rec *c, const char *hostname);

that returns the match, the base or NULL if rejected. That would remove
parts of the vhost matching implementation in mod_ssl and we would not
need to expose the core config.


Note that semantics matter; our name matching is not TLS's matching, but I don't believe the function above is moving SSL functionality into core, it is more one of returning core functionality to core from mod_ssl. But it must behave identically to our lookup code IMO, so a mismatch still returns to the phys server_rec in this version of httpd.