Re: Query regarding extracting ssl hello sni.

2014-04-11 Thread Pravin Tatti
Ok fine you can be forward compatible but i still don't agree its my
personal opinion if I don't know what the packet format for next version
why should I support it. But this was not the major issue for what i
started the discussion. I think the major is relaxing the record layer
check to SSLv3 and we should fix it.




On Fri, Apr 11, 2014 at 4:32 PM, Lukas Tribus  wrote:

> Hi,
>
>
> > I think the next version may or may not contain the same client hello
> > format if it allows i don't have any issues if it doesn't allows then
> > the code may crash or it may return bad value for SNI. I just suggested
> > it for safety reasons its just my input.
>
> If HAproxy would crash, we would need to fix the actual reason of the
> crash, not ignore SNI when TLS version is higher than 1.2, because an
> attacker can always send packets with TLSv1.2 and the offending payload,
> even if its not valid packet as per RFC.
>
>
> As for bad values: SNI is a client provided value and thus must never
> be trusted. We can use it for routing the request to different backends,
> but we always need to validate it before doing something with it.
>
>
>
>
> Regards,
>
> Lukas
>
>


Re: Query regarding extracting ssl hello sni.

2014-04-11 Thread Pravin Tatti
I too agree with your first comment relax only the check for record layer
version as SNI is extensions for TLS protocols.
I think the next version may or may not contain the same client hello
format if it allows i don't have any issues if it doesn't allows then the
code may crash or it may return bad value for SNI. I just suggested it for
safety reasons its just my input.


On Fri, Apr 11, 2014 at 12:09 PM, Lukas Tribus  wrote:

> Hi,
>
>
> > I would suggest that it will not harm even if you relax the check for
> > client hello too as the old client can using SSL 3.0 is still supported
> > and its according to RFC
>
> I disagree. SNI is documented as a TLS extension and unsupported in SSLv3.
> RFC3546 and RFC6066 are the relevant RFCs, not RFC5246.
>
> Try SSLv3 with SNI:
>  openssl s_client -connect localhost:443 -servername snitest -ssl3
>
> The client_hello doesn't contain SNI, because TLS extensions are not
> supported in SSLv3.
>
>
>
> > and also note that the max supported TLS version is 3.3. I would suggest
> > the below mentioned changes.
>
> I disagree here also. Why should we limit the max supported TLS version and
> introduce forward compatibility issues? We will just be a like those stupid
> obsolete SSL middleboxes the browsers need to workaround everytime they
> enable a new SSL feature. I would rather avoid that.
>
>
>
> Regards,
>
> Lukas
>
>


Re: Query regarding extracting ssl hello sni.

2014-04-10 Thread Pravin Tatti
I would suggest that it will not harm even if you relax the check for
client hello too as the old client can using SSL 3.0 is still supported and
its according to RFC and also note that the max supported TLS version is
3.3. I would suggest the below mentioned changes.

288c288
< /* Check for TLSv1 or later (SSL version >= 3.1) */
---
> /* Check for SSLv3 or later (SSL version >= 3.0) */
291c291
< if (data[1] < 0x03 || data[2] < 0x01)
---
> if (data[1] < 0x03 || data[2] > 0x03)
322c322
< if (data[0] < 0x03 || data[1] < 0x01) /* TLSv1 minimum */
---
> if (data[0] < 0x03 || data[1] > 0x03) /* SSLv3 minimum TLSv1.2
maximum */


On Fri, Apr 11, 2014 at 12:43 AM, Lukas Tribus  wrote:

> Hi,
>
>
> > Basically we just need to relax the record layer check to SSLv3 - and
> > leave the clienthello check as is, right?
> >
> > Does the attached diff do the job for you correctly, Pravin?
>
> I have reproduced the issue with gnutls and can confirm that the patch
> fixes the problem.
>
> The function now requires only SSLv3 or later in the record layer, but
> still requires at least TLSv1.0 in the client hello.
>
> I don't think any SNI capable client announces SSLv2 in the record layer
> or worse.
>
>
> I will submit the patch formally.
>
>
>
> Regards,
>
> Lukas
>
>


Re: Query regarding extracting ssl hello sni.

2014-04-10 Thread Pravin Tatti
I think you still didn't understood the problem. There are two versions in
SSL one is record layer hello version and the client hello version. Any
application that support TLS versions 1.0, 1.1, 1.3 or SSLv3 (client hello
version) may contain SSL 3.0 as the record layer version number and the
once the negotiation is done the record layer version is updated.
The problem is not with SSLv3 alone the problem is with all the TLS
versions 1.0, 1.1, 1.3 or SSLv3 who has the record layer version as SSLv3
for client hello packet.

The problem is the application using gnutls instead of openssl has record
layer hello version set to SSL 3.0 for client hello handshake and the
client hello version to TLSv2 (max TLS version supported by client).

What i suggest is fetching of SNI is still valid even if the record layer
version is 3.0 and the actual client hello version is any of the TLS
versions including SSLv3.




On Thu, Apr 10, 2014 at 2:34 PM, Willy Tarreau  wrote:

>  [image: Boxbe] <https://www.boxbe.com/overview> Willy Tarreau (w...@1wt.eu)
> is not on your Guest 
> List<https://www.boxbe.com/approved-list?tc_serial=16879868868&tc_rand=1178275786&utm_source=stf&utm_medium=email&utm_campaign=ANNO_MWTP&utm_content=001&token=%2Fwy11T7R1QVKnkycPjMukAJgXwKDuCjUkkpoPo3fnUbD27SXBkGi8peGgEmh%2FRec&key=pl7vWF6XC3JfowFZA%2F0CL7u6ZiLZzX%2BJ6UJhky1jiZM%3D>|
>  Approve
> sender<https://www.boxbe.com/anno?tc_serial=16879868868&tc_rand=1178275786&utm_source=stf&utm_medium=email&utm_campaign=ANNO_MWTP&utm_content=001&token=%2Fwy11T7R1QVKnkycPjMukAJgXwKDuCjUkkpoPo3fnUbD27SXBkGi8peGgEmh%2FRec&key=pl7vWF6XC3JfowFZA%2F0CL7u6ZiLZzX%2BJ6UJhky1jiZM%3D>|
>  Approve
> domain<https://www.boxbe.com/anno?tc_serial=16879868868&tc_rand=1178275786&utm_source=stf&utm_medium=email&utm_campaign=ANNO_MWTP&utm_content=001&dom&token=%2Fwy11T7R1QVKnkycPjMukAJgXwKDuCjUkkpoPo3fnUbD27SXBkGi8peGgEmh%2FRec&key=pl7vWF6XC3JfowFZA%2F0CL7u6ZiLZzX%2BJ6UJhky1jiZM%3D>
>
> Hi,
>
> On Thu, Apr 10, 2014 at 10:33:38AM +0530, Pravin Tatti wrote:
> > Hi,
> >
> > The function smp_fetch_ssl_hello_sni() only supports record layer version
> > and client hello version greater than or equal to 3.1. But as in the
> > RFC5246 in appendix E says that "TLS versions 1.0, 1.1, and 1.2, and SSL
> > 3.0 are very similar" and also if we check the last 2 paras as mentioned
> > below
> >
> > " Earlier versions of the TLS specification were not fully clear on what
> > the record layer version number (TLSPlaintext.version) should contain
> when
> > sending ClientHello (i.e., before it is known which version of the
> protocol
> > will be employed). Thus, TLS servers compliant with this specification
> MUST
> > accept any value {03,XX} as the record layer version number for
> > ClientHello.
> >
> > TLS clients that wish to negotiate with older servers MAY send any value
> > {03,XX} as the record layer version number. Typical values would be
> > {03,00}, the lowest version number supported by the client, and the value
> > of ClientHello.client_version. No single value will guarantee
> > interoperability with all old servers, but this is a complex topic beyond
> > the scope of this document. "
> >
> > This indicates that the tls record version might be SSL 3.0 and the
> client
> > hello might be TLS version 1.0, 1.1 or 1.2.
> >
> > Do we need to consider this case while parsing in
> smp_fetch_ssl_hello_sni()
> > or is there any other specific reason for it.
> >
> > I noticed such kind of issue when application is using gnutls instead of
> > openssl.
> > ie., the record layer version is SSL 3.0 and client hello is 3.3.
>
> Well, there is no particular reason for being limited to a certain set of
> versions, however I think we did it this way because SNI started to be used
> long after any client totally stopped supporting SSLv3, so it did not
> appear
> that there was any intersection between SSLv3 and SNI. But if there is,
> sure,
> maybe propose a patch!
>
> Thanks,
> Willy
>
>
>


Query regarding extracting ssl hello sni.

2014-04-09 Thread Pravin Tatti
Hi,

The function smp_fetch_ssl_hello_sni() only supports record layer version
and client hello version greater than or equal to 3.1. But as in the
RFC5246 in appendix E says that "TLS versions 1.0, 1.1, and 1.2, and SSL
3.0 are very similar" and also if we check the last 2 paras as mentioned
below

" Earlier versions of the TLS specification were not fully clear on what
the record layer version number (TLSPlaintext.version) should contain when
sending ClientHello (i.e., before it is known which version of the protocol
will be employed). Thus, TLS servers compliant with this specification MUST
accept any value {03,XX} as the record layer version number for
ClientHello.

TLS clients that wish to negotiate with older servers MAY send any value
{03,XX} as the record layer version number. Typical values would be
{03,00}, the lowest version number supported by the client, and the value
of ClientHello.client_version. No single value will guarantee
interoperability with all old servers, but this is a complex topic beyond
the scope of this document. "

This indicates that the tls record version might be SSL 3.0 and the client
hello might be TLS version 1.0, 1.1 or 1.2.

Do we need to consider this case while parsing in smp_fetch_ssl_hello_sni()
or is there any other specific reason for it.

I noticed such kind of issue when application is using gnutls instead of
openssl.
ie., the record layer version is SSL 3.0 and client hello is 3.3.