RE: Query regarding extracting ssl hello sni.
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.
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 luky...@hotmail.com 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.
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.
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 luky...@hotmail.com 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.
On Fri, Apr 11, 2014 at 04:57:17PM +0530, Pravin Tatti wrote: 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. It's not a matter of opinion but specification. If the packet format is specified as being exclusively for 3.0..3.3, then we should only match this. If it's specified as part of TLS for which only versions 3.0 to 3.3 are currently defined, then we must apply the default rule specified for the whole protocol according to how to handle newer versions. All protocols generally indicate how newer versions must be handled, and it's important to stick on the rule they dictate in order not to break interoperability with future clients. Willy
RE: Query regarding extracting ssl hello sni.
Hi, 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. Because we are talking about a industry standard with a huge user base and it is very likely that next version will be backwards compatible, like it was the case with TLSv1.2-TLSv1.1-TLSv1.0-SSLv3. Its unlikely that the IETF WG will push a change that will break this format. If the next TLS version doesn't break it, SNI will continue to work. If the next TLS version breaks SNI, we will need to fix it. If we restrict the SNI fetching to TLSv1.[0-2], we need to fix SNI in every case, without any obvious advantage. I think the major is relaxing the record layer check to SSLv3 and we should fix it. Its already fixed, Willy committed it yesterday. Tonight's snapshot already contains the fix. Regards, Lukas
RE: Query regarding extracting ssl hello sni.
Hi, It's not a matter of opinion but specification. If the packet format is specified as being exclusively for 3.0..3.3, then we should only match this. If it's specified as part of TLS for which only versions 3.0 to 3.3 are currently defined, then we must apply the default rule specified for the whole protocol according to how to handle newer versions. All protocols generally indicate how newer versions must be handled, and it's important to stick on the rule they dictate in order not to break interoperability with future clients. Mmmmh, good point. TLS extensions are documented pretty strictly against certain TLS version: RFC3546 -- TLSv1.0 RFC4366 -- TLSv1.0 and TLSv1.1 RFC6066 -- TLSv1.2 I'm not sure that those RFCs really address forward compatibility. I took a very quick look at RFC6066, but didn't found any real indication of what will happen in future version, other than: Currently, the only server names supported are DNS hostnames; however, this does not imply any dependency of TLS on DNS, and other name types may be added in the future (by an RFC that updates this document). The data structure associated with the host_name NameType is a variable-length vector that begins with a 16-bit length. For backward compatibility, all future data structures associated with new NameTypes MUST begin with a 16-bit length field. TLS MAY treat provided server names as opaque data and pass the names and types to the application. So, according to what you said and how the RFCs are written Pravin would be right suggesting to limit the SNI fetch to TLSv1.2. Anyway, even if we do this, its a good thing to separate the bugfix (relax the record version check) and a change (only fetch SNI up to TLSv1.2) instead of having both in a single commit. So, should we do this? Should both record version and client_hello be checked to be = TLSv1.2? Regards, Lukas
Re: Query regarding extracting ssl hello sni.
Hi Lukas, On Fri, Apr 11, 2014 at 03:17:32PM +0200, Lukas Tribus wrote: Hi, It's not a matter of opinion but specification. If the packet format is specified as being exclusively for 3.0..3.3, then we should only match this. If it's specified as part of TLS for which only versions 3.0 to 3.3 are currently defined, then we must apply the default rule specified for the whole protocol according to how to handle newer versions. All protocols generally indicate how newer versions must be handled, and it's important to stick on the rule they dictate in order not to break interoperability with future clients. Mmmmh, good point. TLS extensions are documented pretty strictly against certain TLS version: RFC3546 -- TLSv1.0 RFC4366 -- TLSv1.0 and TLSv1.1 RFC6066 -- TLSv1.2 I'm not sure that those RFCs really address forward compatibility. Well, I have a different interpretation of this. First, 2246 clearly says that TLSv1.0 has major 3 and minor 1 because it's a minor improvement over SSLv3.0. So that clearly states that a change of the minor here only means a minor improvement. Second, 3546 discusses extensions and says : The extensions may be used by TLS clients and servers. The extensions are backwards compatible - communication is possible between TLS 1.0 clients that support the extensions and TLS 1.0 servers that do not support the extensions, and vice versa. This implies forward compatibility since both sides may use extensions in a backwards compatible way with the other one. Otherwise, lack of forward compatibility would break this type of backwards compatibility. 5246 says this : Appendix E. Backward Compatibility E.1. Compatibility with TLS 1.0/1.1 and SSL 3.0 Since there are various versions of TLS (1.0, 1.1, 1.2, and any future versions) and SSL (2.0 and 3.0), means are needed to negotiate the specific protocol version to use. The TLS protocol provides a built-in mechanism for version negotiation so as not to bother other protocol components with the complexities of version selection. TLS versions 1.0, 1.1, and 1.2, and SSL 3.0 are very similar, and use compatible ClientHello messages; thus, supporting all of them is relatively easy. Similarly, servers can easily handle clients trying to use future versions of TLS as long as the ClientHello format remains compatible, and the client supports the highest protocol version available in the server. A TLS 1.2 client who wishes to negotiate with such older servers will send a normal TLS 1.2 ClientHello, containing { 3, 3 } (TLS 1.2) in ClientHello.client_version. If the server does not support this version, it will respond with a ServerHello containing an older version number. If the client agrees to use this version, the negotiation will proceed as appropriate for the negotiated protocol. Again, to me it clearly states that the focus is on supporting different versions on both sides, which implies that a client talking 1.2 can be understood by a server talking 1.1 etc... I see nothing in these specs mandating a limit to a certain protocol level for certain features. All I'm seeing is protocol version is { 3, 3 } for this version of the spec in 1.2, or { 3, 2 } for 1.1, etc... It just reminds what to *emit* to indicate what protocol you're speaking, not anything related to a sudden drop of forwards compatibility. So in my opinion, since the protocol is designed to be backwards and forwards compatible, and uses minor version for newer extensions, there is no reason for limiting ourselves to TLSv1.2 for extensions that exist since 1.0 and will certainly continue to be supported by later version, at least for backwards compatibility. Regards, Willy
RE: Query regarding extracting ssl hello sni.
Hi Willy, So in my opinion, since the protocol is designed to be backwards and forwards compatible, and uses minor version for newer extensions, there is no reason for limiting ourselves to TLSv1.2 for extensions that exist since 1.0 and will certainly continue to be supported by later version, at least for backwards compatibility. Ok. I do in fact agree with this and it matches with what I was saying earlier in this thread, I just didn't take the time to read the RFC carefully enough to draw the same conclusion. Thanks, Lukas
Re: Query regarding extracting ssl hello sni.
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
Re: Query regarding extracting ssl hello sni.
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 w...@1wt.eu wrote: [image: Boxbe] https://www.boxbe.com/overview Willy Tarreau (w...@1wt.eu) is not on your Guest Listhttps://www.boxbe.com/approved-list?tc_serial=16879868868tc_rand=1178275786utm_source=stfutm_medium=emailutm_campaign=ANNO_MWTPutm_content=001token=%2Fwy11T7R1QVKnkycPjMukAJgXwKDuCjUkkpoPo3fnUbD27SXBkGi8peGgEmh%2FReckey=pl7vWF6XC3JfowFZA%2F0CL7u6ZiLZzX%2BJ6UJhky1jiZM%3D| Approve senderhttps://www.boxbe.com/anno?tc_serial=16879868868tc_rand=1178275786utm_source=stfutm_medium=emailutm_campaign=ANNO_MWTPutm_content=001token=%2Fwy11T7R1QVKnkycPjMukAJgXwKDuCjUkkpoPo3fnUbD27SXBkGi8peGgEmh%2FReckey=pl7vWF6XC3JfowFZA%2F0CL7u6ZiLZzX%2BJ6UJhky1jiZM%3D| Approve domainhttps://www.boxbe.com/anno?tc_serial=16879868868tc_rand=1178275786utm_source=stfutm_medium=emailutm_campaign=ANNO_MWTPutm_content=001domtoken=%2Fwy11T7R1QVKnkycPjMukAJgXwKDuCjUkkpoPo3fnUbD27SXBkGi8peGgEmh%2FReckey=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
Re: Query regarding extracting ssl hello sni.
On Thu, Apr 10, 2014 at 06:30:26PM +0530, Pravin Tatti wrote: 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. OK thanks for clarifying. 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. Fine, could you send a patch to do that then ? Willy
RE: Query regarding extracting ssl hello sni.
Date: Thu, 10 Apr 2014 15:22:43 +0200 From: w...@1wt.eu To: tattiprav...@gmail.com CC: haproxy@formilux.org Subject: Re: Query regarding extracting ssl hello sni. On Thu, Apr 10, 2014 at 06:30:26PM +0530, Pravin Tatti wrote: 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. OK thanks for clarifying. 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? Regards, Lukas sslv3-record-layer-sni.diff Description: Binary data
RE: Query regarding extracting ssl hello sni.
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.
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 luky...@hotmail.com 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
Query regarding extracting ssl hello sni.
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.