BUG/MEDIUM: ssl: Verify error codes can exceed 63

The CRT and CA verify error codes were stored in 6 bits each in the
xprt_st field of the ssl_sock_ctx meaning that only error code up to 63
could be stored. Likewise, the ca-ignore-err and crt-ignore-err options
relied on two unsigned long longs that were used as bitfields for all
the ignored error codes. On the latest OpenSSL1.1.1 and with OpenSSLv3
and newer, verify errors have exceeded this value so these two storages
must be increased. The error codes will now be stored on 7 bits each and
the ignore-err bitfields are replaced by a big enough array and
dedicated bit get and set functions.

It can be backported on all stable branches.

[wla: let it be tested a little while before backport]
Signed-off-by: William Lallemand <wlallemand@haproxy.org>

diff --git a/include/haproxy/listener-t.h b/include/haproxy/listener-t.h
index 17f3251b6a658fbe4d915cd29e82c1caae56e989..46921a4b98f33bf48cb2f768a935c5b37f9720d8 100644 (file)
--- a/include/haproxy/listener-t.h
+++ b/include/haproxy/listener-t.h
@@ -158,12 +158,21 @@ struct ssl_bind_conf {
 #endif
 };
 
+/*
+ * In OpenSSL 3.0.0, the biggest verify error code's value is 94 and on the
+ * latest 1.1.1 it already reaches 79 so we need to size the ca/crt-ignore-err
+ * arrays accordingly. If the max error code increases, the arrays might need to
+ * be resized.
+ */
+#define SSL_MAX_VFY_ERROR_CODE 94
+#define IGNERR_BF_SIZE ((SSL_MAX_VFY_ERROR_CODE >> 6) + 1)
+
 /* "bind" line settings */
 struct bind_conf {
 #ifdef USE_OPENSSL
        struct ssl_bind_conf ssl_conf; /* ssl conf for ctx setting */
-       unsigned long long ca_ignerr;  /* ignored verify errors in handshake if depth > 0 */
-       unsigned long long crt_ignerr; /* ignored verify errors in handshake if depth == 0 */
+       unsigned long long ca_ignerr_bitfield[IGNERR_BF_SIZE];   /* ignored verify errors in handshake if depth > 0 */
+       unsigned long long crt_ignerr_bitfield[IGNERR_BF_SIZE];  /* ignored verify errors in handshake if depth == 0 */
        void *initial_ctx;             /* SSL context for initial negotiation */
        void *default_ctx;             /* SSL context of first/default certificate */
        struct ckch_inst *default_inst;

diff --git a/include/haproxy/ssl_sock-t.h b/include/haproxy/ssl_sock-t.h
index b404defdb2b3840a5cfc4d6e3b3926fe594a4b3b..278c8f7e88549a8b5ea17456bfe55190e77d9b1d 100644 (file)
--- a/include/haproxy/ssl_sock-t.h
+++ b/include/haproxy/ssl_sock-t.h
@@ -50,16 +50,20 @@
 #define SSL_SOCK_SEND_UNLIMITED     0x00000004
 #define SSL_SOCK_RECV_HEARTBEAT     0x00000008
 
-/* bits 0xFFFF0000 are reserved to store verify errors */
+/* bits 0xFFFFFF00 are reserved to store verify errors.
+ * The CA en CRT error codes will be stored on 7 bits each
+ * (since the max verify error code does not exceed 127)
+ * and the CA error depth will be stored on 4 bits.
+ */
 
 /* Verify errors macros */
-#define SSL_SOCK_CA_ERROR_TO_ST(e) (((e > 63) ? 63 : e) << (16))
-#define SSL_SOCK_CAEDEPTH_TO_ST(d) (((d > 15) ? 15 : d) << (6+16))
-#define SSL_SOCK_CRTERROR_TO_ST(e) (((e > 63) ? 63 : e) << (4+6+16))
+#define SSL_SOCK_CA_ERROR_TO_ST(e) (((e > 127) ? 127 : e) << (8))
+#define SSL_SOCK_CAEDEPTH_TO_ST(d) (((d > 15) ? 15 : d) << (7+8))
+#define SSL_SOCK_CRTERROR_TO_ST(e) (((e > 127) ? 127 : e) << (4+7+8))
 
-#define SSL_SOCK_ST_TO_CA_ERROR(s) ((s >> (16)) & 63)
-#define SSL_SOCK_ST_TO_CAEDEPTH(s) ((s >> (6+16)) & 15)
-#define SSL_SOCK_ST_TO_CRTERROR(s) ((s >> (4+6+16)) & 63)
+#define SSL_SOCK_ST_TO_CA_ERROR(s) ((s >> (8)) & 127)
+#define SSL_SOCK_ST_TO_CAEDEPTH(s) ((s >> (7+8)) & 15)
+#define SSL_SOCK_ST_TO_CRTERROR(s) ((s >> (4+7+8)) & 127)
 
 /* ssl_methods flags for ssl options */
 #define MC_SSL_O_ALL            0x0000

diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h
index d7cc33075ac9a8daa0e51fa7f6994f1508ef6c4a..d24b17f5b987006f59d3e406bbbfd2d396c68ff5 100644 (file)
--- a/include/haproxy/ssl_sock.h
+++ b/include/haproxy/ssl_sock.h
@@ -158,6 +158,29 @@ int ssl_sock_register_msg_callback(ssl_sock_msg_callback_func func);
 
 SSL *ssl_sock_get_ssl_object(struct connection *conn);
 
+static inline int cert_ignerr_bitfield_get(const unsigned long long *bitfield, int bit_index)
+{
+       int byte_index = bit_index >> 6;
+       int val = 0;
+
+       if (byte_index < IGNERR_BF_SIZE)
+               val = bitfield[byte_index] & (1 << (bit_index & 0x3F));
+
+       return val != 0;
+}
+
+static inline void cert_ignerr_bitfield_set(unsigned long long *bitfield, int bit_index)
+{
+       int byte_index = bit_index >> 6;
+
+       if (byte_index < IGNERR_BF_SIZE)
+               bitfield[byte_index] |= (1 << (bit_index & 0x3F));
+}
+
+static inline void cert_ignerr_bitfield_set_all(unsigned long long *bitfield)
+{
+       memset(bitfield, -1, IGNERR_BF_SIZE*sizeof(*bitfield));
+}
 
 #endif /* USE_OPENSSL */
 #endif /* _HAPROXY_SSL_SOCK_H */

diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c
index 6abcd38ebbb60d6251b2e1b4ca14c0d670233f1f..35780adff4f34602a3ec31b1c811fc2d9b2f9266 100644 (file)
--- a/src/cfgparse-ssl.c
+++ b/src/cfgparse-ssl.c
@@ -825,7 +825,7 @@ static int bind_parse_ignore_err(char **args, int cur_arg, struct proxy *px, str
 {
        int code;
        char *p = args[cur_arg + 1];
-       unsigned long long *ignerr = &conf->crt_ignerr;
+       unsigned long long *ignerr = conf->crt_ignerr_bitfield;
 
        if (!*p) {
                memprintf(err, "'%s' : missing error IDs list", args[cur_arg]);
@@ -833,21 +833,21 @@ static int bind_parse_ignore_err(char **args, int cur_arg, struct proxy *px, str
        }
 
        if (strcmp(args[cur_arg], "ca-ignore-err") == 0)
-               ignerr = &conf->ca_ignerr;
+               ignerr = conf->ca_ignerr_bitfield;
 
        if (strcmp(p, "all") == 0) {
-               *ignerr = ~0ULL;
+               cert_ignerr_bitfield_set_all(ignerr);
                return 0;
        }
 
        while (p) {
                code = atoi(p);
-               if ((code <= 0) || (code > 63)) {
-                       memprintf(err, "'%s' : ID '%d' out of range (1..63) in error IDs list '%s'",
-                                 args[cur_arg], code, args[cur_arg + 1]);
+               if ((code <= 0) || (code > SSL_MAX_VFY_ERROR_CODE)) {
+                       memprintf(err, "'%s' : ID '%d' out of range (1..%d) in error IDs list '%s'",
+                                 args[cur_arg], code, SSL_MAX_VFY_ERROR_CODE, args[cur_arg + 1]);
                        return ERR_ALERT | ERR_FATAL;
                }
-               *ignerr |= 1ULL << code;
+               cert_ignerr_bitfield_set(ignerr, code);
                p = strchr(p, ',');
                if (p)
                        p++;

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 55602055353d763252f8079b5c9892e51cb2c3c2..811b3d8ea1b66333da223e933ae8541170e77d23 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1777,7 +1777,8 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
                        ctx->xprt_st |= SSL_SOCK_CAEDEPTH_TO_ST(depth);
                }
 
-               if (err < 64 && bind_conf->ca_ignerr & (1ULL << err))
+               if (err <= SSL_MAX_VFY_ERROR_CODE &&
+                   cert_ignerr_bitfield_get(__objt_listener(conn->target)->bind_conf->ca_ignerr_bitfield, err))
                        goto err_ignored;
 
                /* TODO: for QUIC connection, this error code is lost */
@@ -1790,7 +1791,8 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
                ctx->xprt_st |= SSL_SOCK_CRTERROR_TO_ST(err);
 
        /* check if certificate error needs to be ignored */
-       if (err < 64 && bind_conf->crt_ignerr & (1ULL << err))
+       if (err <= SSL_MAX_VFY_ERROR_CODE &&
+           cert_ignerr_bitfield_get(__objt_listener(conn->target)->bind_conf->crt_ignerr_bitfield, err))
                goto err_ignored;
 
        /* TODO: for QUIC connection, this error code is lost */