You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2014/04/16 14:15:48 UTC

Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

On Sat, Apr 5, 2014 at 2:57 PM,  <kb...@apache.org> wrote:
> Author: kbrand
> Date: Sat Apr  5 12:57:43 2014
> New Revision: 1585090
>
> URL: http://svn.apache.org/r1585090
> Log:
> Bring SNI behavior into better conformance with RFC 6066:
>
> - no longer send a warning-level unrecognized_name(112) alert
>   when no matching vhost is found (PR 56241)

>From a client perspective, it is a loss of information, couldn't the
admin have an option ...

> +                /*
> +                 * RFC 6066 section 3 says "It is NOT RECOMMENDED to send
> +                 * a warning-level unrecognized_name(112) alert, because
> +                 * the client's behavior in response to warning-level alerts
> +                 * is unpredictable."
> +                 *
> +                 * To maintain backwards compatibility in mod_ssl, we
> +                 * no longer send any alert (neither warning- nor fatal-level),
> +                 * i.e. we take the second action suggested in RFC 6066:
> +                 * "If the server understood the ClientHello extension but
> +                 * does not recognize the server name, the server SHOULD take
> +                 * one of two actions: either abort the handshake by sending
> +                 * a fatal-level unrecognized_name(112) alert or continue
> +                 * the handshake."
> +                 */

for the first action suggested in the RFC?

This base_server directive would help prevent vhost misuse at the
source, whatever the vhosts' configs are, and however we relax the
Host vs SNI check.
Thus one can trust httpd's (base) config to know whether or not the
handshake has reached the requested vhost.
This is already/always the case currently, the difference is when it
takes place (before vhosting), and the SSL level alert (vs "400 Bad
Request" now) that would help the client to know what's happening.

That option would cause empty SNI to be refused too, and maybe
multiple SSL vhosts on the same IP:port to be error-ed on startup
(when SNI is available in the underlying lib).
Whether it should be on/off by default depends on the change
introduced by this commit (SSL alert warning => 400) and how clients
handle warning level SSL alerts.

I may be misleaded though...

Regards,
Yann.

Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Apr 16, 2014 at 4:00 PM, Yann Ylavic <yl...@gmail.com> wrote:
> will pass through (unless this vhost has a ServerName and
> UseCanonicalName is on, which would result in a 400).

UseCanonicalName isn't relevant here, r->hostname (and not
ap_get_server_name()) is used for the check, so SNI == Host header is
the only condition to pass through the SNI check on the default vhost,
which depends solely on the client.

>
> Regards,
> Yann.

RE: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Sent: Mittwoch, 16. April 2014 16:01
> To: httpd
> Subject: Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES
> modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c
> 
> On Wed, Apr 16, 2014 at 3:11 PM, Plüm, Rüdiger, Vodafone Group
> <ru...@vodafone.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> >> Sent: Mittwoch, 16. April 2014 15:00
> >> To: httpd
> >> Subject: Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES
> >> modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c
> >>
> >> On Wed, Apr 16, 2014 at 2:41 PM, Plüm, Rüdiger, Vodafone Group
> >> <ru...@vodafone.com> wrote:
> >> >
> >> >> -----Original Message-----
> >> >> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> >> >> This base_server directive would help prevent vhost misuse at the
> >> >> source, whatever the vhosts' configs are, and however we relax the
> >> >> Host vs SNI check.
> >> >
> >> > I don't think so. The SNI provided hostname and the HTTP host header
> >> still need to match.
> >>
> >> Which can't be if no vhost is defined for that SNI, the option would
> >> not break that (it's more a hardening feature).
> >
> > You are confusing me. In this case we would fall to the default vhost.
> > But I guess I am currently not understanding what you try to resolve /
> what goes wrong
> > without a patch. Care to give an example setup to clear up my confusion?
> 
> Before this commit, the client knew it was not reaching any vhost by
> receiving an SSL alert (warning), and could stop.
> Now, has you said, it will reach the default vhost, and provided the
> SNI is the same as the Host header (or some other mean blocks), it
> will pass through (unless this vhost has a ServerName and
> UseCanonicalName is on, which would result in a 400). The certificate
> used on the default vhost can alert it though, but this is out of SNI
> handling.
> This is not a behaviour change on the server side (the SSL alert
> warning was not abortive), but for the client.
> 
> The new option would prevent the default vhost to be used (when SNI is
> available), and let the client know (still).

I don't see why we should prevent using the default host. Maybe it has a wildcard cert
and everything is good.

Regards

Rüdiger

Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Kaspar Brand <ht...@velox.ch>.
On 18.04.2014 11:11, Yann Ylavic wrote:
> Agreed, still it may be useful (for some potential client) to get the
> real error when it handshakes with an SNI which is not acceptable on
> this server, and for the server the sooner the better IMHO.

mod_ssl will send a "Certificate" TLS message either way (whether we
return SSL_TLSEXT_ERR_ALERT_WARNING or SSL_TLSEXT_ERR_NOACK). From a
client perspective, if you really want to avoid processing/validating
the cert (and abort early based on the server's hint that he found no
matching certificate), then you can check the ServerHello for the
presence of an (empty) SNI extension. With r1585090 / r1588424, we're
omitting the warning-level alert, but there's no change for the ServerHello.

> Well, if one want to be really strict about SNI, and don't have a
> wildcard cert (say the poor man, still with multiple name-based
> vhost), is the default SSL vhost relevant?

Yes, as it simply the "one and only" SSL host for a given TCP port, in
this case, and completely denying access to non-SNI clients that early
in the handshake already isn't a very useful thing to go for, I think.

> to deny at handshake time, with the relevant message for the client to
> know why (instead of 400, 503 or even bad cert which just leaks
> another service).

Browsers are pretty bad with handling failures at the TLS level (fatal
alerts in particular). Handling them with a decent ErrorDocument etc. on
the HTTP layer is much better, IMO.

Kaspar

Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Apr 18, 2014 at 10:34 AM, Kaspar Brand <ht...@velox.ch> wrote:
>
> On 16.04.2014 16:00, Yann Ylavic wrote:
>> Before this commit, the client knew it was not reaching any vhost by
>> receiving an SSL alert (warning), and could stop.
>
> In practice, most SNI-capable clients have ignored these warning-level
> alerts (which is completely in line with RFC 5246, which states "If an
> alert with a level of warning is sent and received, generally the
> connection can continue normally").
>
> Whether or not the server acknowledges to have a suitable cert for the
> name requested by the client in the SNI extension isn't that relevant,
> IMO. The most important (and absolutely essential) thing the client has
> to do is to check the expected name against those included in the
> certificate (RFC 6125, acceptable reference identifiers vs. presented
> identifiers), irrespective of any potential TLS warning-level alert he
> might have received on the connection.

Agreed, still it may be useful (for some potential client) to get the
real error when it handshakes with an SNI which is not acceptable on
this server, and for the server the sooner the better IMHO.

>
>> The new option would prevent the default vhost to be used (when SNI is
>> available), and let the client know (still).
>
> As pointed out by Rüdiger in his latest message, there is no point in
> disallowing access to the default vhost based on whether an SNI
> extension was sent by the client or not: the default vhost is
> effectively the "global" one for SSL connections - "SSLEngine on" should
> only be configured within vhosts, see also [1]. Ever since mod_ssl was
> added to 2.0.x [2], a "_default_:443" vhost was used to handle all SSL
> requests.

Well, if one want to be really strict about SNI, and don't have a
wildcard cert (say the poor man, still with multiple name-based
vhost), is the default SSL vhost relevant?

I'm not arguing about setting this by default, nor we can't do it
another way, but that it's a legitimate situation where one could want
to deny at handshake time, with the relevant message for the client to
know why (instead of 400, 503 or even bad cert which just leaks
another service).

>
> Thanks for your vote with r1588257, Yann, in any case - now merged to
> 2.4.x with r1588424.

You're welcome, this could be done in a second step :p

Regards,
Yann.

Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Kaspar Brand <ht...@velox.ch>.
Sorry for being late with my reply.

On 16.04.2014 16:00, Yann Ylavic wrote:
> Before this commit, the client knew it was not reaching any vhost by
> receiving an SSL alert (warning), and could stop.

In practice, most SNI-capable clients have ignored these warning-level
alerts (which is completely in line with RFC 5246, which states "If an
alert with a level of warning is sent and received, generally the
connection can continue normally").

Whether or not the server acknowledges to have a suitable cert for the
name requested by the client in the SNI extension isn't that relevant,
IMO. The most important (and absolutely essential) thing the client has
to do is to check the expected name against those included in the
certificate (RFC 6125, acceptable reference identifiers vs. presented
identifiers), irrespective of any potential TLS warning-level alert he
might have received on the connection.

> The new option would prevent the default vhost to be used (when SNI is
> available), and let the client know (still).

As pointed out by Rüdiger in his latest message, there is no point in
disallowing access to the default vhost based on whether an SNI
extension was sent by the client or not: the default vhost is
effectively the "global" one for SSL connections - "SSLEngine on" should
only be configured within vhosts, see also [1]. Ever since mod_ssl was
added to 2.0.x [2], a "_default_:443" vhost was used to handle all SSL
requests.

Thanks for your vote with r1588257, Yann, in any case - now merged to
2.4.x with r1588424.

Kaspar

[1] https://mail-archives.apache.org/mod_mbox/httpd-dev/201011.mbox/%3C7BF538E6-DAFB-4638-B845-09BEDF673327@rcbowen.com%3E

[2] https://svn.apache.org/viewvc?view=revision&revision=91707

Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Apr 16, 2014 at 3:11 PM, Plüm, Rüdiger, Vodafone Group
<ru...@vodafone.com> wrote:
>
>
>> -----Original Message-----
>> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
>> Sent: Mittwoch, 16. April 2014 15:00
>> To: httpd
>> Subject: Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES
>> modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c
>>
>> On Wed, Apr 16, 2014 at 2:41 PM, Plüm, Rüdiger, Vodafone Group
>> <ru...@vodafone.com> wrote:
>> >
>> >> -----Original Message-----
>> >> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
>> >> This base_server directive would help prevent vhost misuse at the
>> >> source, whatever the vhosts' configs are, and however we relax the
>> >> Host vs SNI check.
>> >
>> > I don't think so. The SNI provided hostname and the HTTP host header
>> still need to match.
>>
>> Which can't be if no vhost is defined for that SNI, the option would
>> not break that (it's more a hardening feature).
>
> You are confusing me. In this case we would fall to the default vhost.
> But I guess I am currently not understanding what you try to resolve / what goes wrong
> without a patch. Care to give an example setup to clear up my confusion?

Before this commit, the client knew it was not reaching any vhost by
receiving an SSL alert (warning), and could stop.
Now, has you said, it will reach the default vhost, and provided the
SNI is the same as the Host header (or some other mean blocks), it
will pass through (unless this vhost has a ServerName and
UseCanonicalName is on, which would result in a 400). The certificate
used on the default vhost can alert it though, but this is out of SNI
handling.
This is not a behaviour change on the server side (the SSL alert
warning was not abortive), but for the client.

The new option would prevent the default vhost to be used (when SNI is
available), and let the client know (still).

Regards,
Yann.

RE: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Sent: Mittwoch, 16. April 2014 15:00
> To: httpd
> Subject: Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES
> modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c
> 
> On Wed, Apr 16, 2014 at 2:41 PM, Plüm, Rüdiger, Vodafone Group
> <ru...@vodafone.com> wrote:
> >
> >> -----Original Message-----
> >> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> >> This base_server directive would help prevent vhost misuse at the
> >> source, whatever the vhosts' configs are, and however we relax the
> >> Host vs SNI check.
> >
> > I don't think so. The SNI provided hostname and the HTTP host header
> still need to match.
> 
> Which can't be if no vhost is defined for that SNI, the option would
> not break that (it's more a hardening feature).

You are confusing me. In this case we would fall to the default vhost.
But I guess I am currently not understanding what you try to resolve / what goes wrong
without a patch. Care to give an example setup to clear up my confusion?

Regards

Rüdiger

Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Apr 16, 2014 at 2:41 PM, Plüm, Rüdiger, Vodafone Group
<ru...@vodafone.com> wrote:
>
>> -----Original Message-----
>> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
>> This base_server directive would help prevent vhost misuse at the
>> source, whatever the vhosts' configs are, and however we relax the
>> Host vs SNI check.
>
> I don't think so. The SNI provided hostname and the HTTP host header still need to match.

Which can't be if no vhost is defined for that SNI, the option would
not break that (it's more a hardening feature).
I'm not arguing we should relax the check (now), but when/if
everything can be done/renegociated at hook_Access time, hook_ReadReq
will have to let it go, still the check at SSL (alert) level is
relevant IMHO.

>
> Regards
>
> Rüdiger
>

RE: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Yann Ylavic [mailto:ylavic.dev@gmail.com]
> Sent: Mittwoch, 16. April 2014 14:16
> To: httpd
> Subject: Re: svn commit: r1585090 - in /httpd/httpd/trunk: CHANGES
> modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c
> 
> On Sat, Apr 5, 2014 at 2:57 PM,  <kb...@apache.org> wrote:
> > Author: kbrand
> > Date: Sat Apr  5 12:57:43 2014
> > New Revision: 1585090
> >
> > URL: http://svn.apache.org/r1585090
> > Log:
> > Bring SNI behavior into better conformance with RFC 6066:
> >
> > - no longer send a warning-level unrecognized_name(112) alert
> >   when no matching vhost is found (PR 56241)
> 
> From a client perspective, it is a loss of information, couldn't the
> admin have an option ...
> 
> > +                /*
> > +                 * RFC 6066 section 3 says "It is NOT RECOMMENDED to
> send
> > +                 * a warning-level unrecognized_name(112) alert,
> because
> > +                 * the client's behavior in response to warning-level
> alerts
> > +                 * is unpredictable."
> > +                 *
> > +                 * To maintain backwards compatibility in mod_ssl, we
> > +                 * no longer send any alert (neither warning- nor
> fatal-level),
> > +                 * i.e. we take the second action suggested in RFC
> 6066:
> > +                 * "If the server understood the ClientHello extension
> but
> > +                 * does not recognize the server name, the server
> SHOULD take
> > +                 * one of two actions: either abort the handshake by
> sending
> > +                 * a fatal-level unrecognized_name(112) alert or
> continue
> > +                 * the handshake."
> > +                 */
> 
> for the first action suggested in the RFC?
> 
> This base_server directive would help prevent vhost misuse at the
> source, whatever the vhosts' configs are, and however we relax the
> Host vs SNI check.

I don't think so. The SNI provided hostname and the HTTP host header still need to match.

Regards

Rüdiger