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.

[1] https://tools.ietf.org/html/rfc5246#appendix-E.1
[2] 
https://github.com/openssl/openssl/commit/4a1cf50187659e60c5867ecbbc36e37b2605d2c3
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=774547
---

Here's the v2 patch adressing the problems of the patch posted yesterday.
I quickly tested it and it does work fine.

Regarding the code comments, I think you mixed the 2 versions up:
record layer version number (TLSPlaintext.version) is the 'envelope's
version' (we are currently returning this version in req_ssl_ver)

client hello client version (ClientHello.client_version - this
path makes sure we return this version in req_ssl_ver)

---
 src/payload.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/payload.c b/src/payload.c
index ce9280c..710ed4c 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -399,21 +399,24 @@ smp_fetch_req_ssl_ver(const struct arg *args, struct 
sample *smp, const char *kw
        data = (const unsigned char *)req->buf->p;
        if ((*data >= 0x14 && *data <= 0x17) || (*data == 0xFF)) {
                /* SSLv3 header format */
-               if (bleft < 5)
+               if (bleft < 11)
                        goto too_short;
 
-               version = (data[1] << 16) + data[2]; /* version: major, minor */
+               version = (data[1] << 16) + data[2]; /* record layer version: 
major, minor */
                msg_len = (data[3] <<  8) + data[4]; /* record length */
 
                /* format introduced with SSLv3 */
                if (version < 0x00030000)
                        goto not_ssl;
 
-               /* message length between 1 and 2^14 + 2048 */
-               if (msg_len < 1 || msg_len > ((1<<14) + 2048))
+               /* message length between 6 and 2^14 + 2048 */
+               if (msg_len < 6 || msg_len > ((1<<14) + 2048))
                        goto not_ssl;
 
                bleft -= 5; data += 5;
+
+               /* return the client hello client version, not the record layer 
version */
+               version = (data[4] << 16) + data[5]; /* client hello version: 
major, minor */
        } else {
                /* SSLv2 header format, only supported for hello (msg type 1) */
                int rlen, plen, cilen, silen, chlen;
-- 
1.9.1


Reply via email to