From 64fa46abccf9f9599b575ba57ea4786c53fae9df Mon Sep 17 00:00:00 2001 From: Remi Tricot-Le Breton Date: Thu, 10 Nov 2022 10:48:58 +0100 Subject: [PATCH] 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 (cherry picked from commit 9b25982716f0416c28f8fc894c58eb40885cf9e5) Signed-off-by: William Lallemand --- include/haproxy/listener-t.h | 13 +++++++++++-- include/haproxy/ssl_sock-t.h | 18 +++++++++++------- include/haproxy/ssl_sock.h | 23 +++++++++++++++++++++++ src/cfgparse-ssl.c | 14 +++++++------- src/ssl_sock.c | 6 ++++-- 5 files changed, 56 insertions(+), 18 deletions(-) diff --git a/include/haproxy/listener-t.h b/include/haproxy/listener-t.h index 17f3251b6..46921a4b9 100644 --- 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 b404defdb..278c8f7e8 100644 --- 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 d7cc33075..d24b17f5b 100644 --- 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 6abcd38eb..35780adff 100644 --- 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 7845afb51..3cf038324 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -1779,7 +1779,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 */ @@ -1792,7 +1793,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 */