Hi Lukas,
On Thu, Nov 05, 2015 at 12:00:50AM +0100, Lukas Tribus wrote:
> The initial record layer version in a SSL handshake may be set to TLSv1.0
> or similar for compatibility reasons, this is allowed as per RFC5246
> Appendix E.1 [1]. Some implementations are Openssl [2] and NSS [3].
>
> A related issue has been fixed some time ago in commit 57d229747
> ("BUG/MINOR: acl: req_ssl_sni fails with SSLv3 record version").
>
> Fix this by using the real client hello version instead of the record
> layer version.
>
> This was reported by Julien Vehent and analyzed by Cyril Bonté.
> The initial patch is from Julien Vehent as well.
>
> This should be backported to stable series, the req_ssl_ver keyword was
> first introduced in 1.3.16.
Thanks to all for this very detailed analysis, but this patch introduces a
bug :
> @@ -402,7 +402,7 @@ smp_fetch_req_ssl_ver(const struct arg *args, struct
> sample *smp, const char *kw
> if (bleft < 5)
> goto too_short;
>
> - version = (data[1] << 16) + data[2]; /* version: major, minor */
> + version = (data[9] << 16) + data[10]; /* client hello version:
> major, minor */
> msg_len = (data[3] << 8) + data[4]; /* record length */
See above ? we check for 5 bytes minimum because we didn't parse further
than data[4], and now we're reading data[10], so the test on bleft above
must be changed to bleft < 11.
Can someone please check if the other patch referenced above has the same
bug ?
Thanks,
Willy
