From 105599c1bab84f67e013a266fd846799c7075b69 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 25 Feb 2020 08:59:23 +0100 Subject: [PATCH] BUG/MEDIUM: ssl: fix several bad pointer aliases in a few sample fetch functions Sample fetch functions ssl_x_sha1(), ssl_fc_npn(), ssl_fc_alpn(), ssl_fc_session_id(), as well as the CLI's "show cert details" handler used to dereference the output buffer's field by casting it to "unsigned *". But while doing this could work before 1.9, it broke starting with commit 843b7cbe9d ("MEDIUM: chunks: make the chunk struct's fields match the buffer struct") which merged chunks and buffers, causing the field to become a size_t. The impact is only on 64-bit platform and depends on the endianness: on little endian, there should never be any non-zero bits in the field as it is supposed to have been zeroed before the call, so it shouldbe harmless; on big endian, the high part of the value only is written instead of the lower one, often making the result appear 4 billion times larger, and making such values dropped everywhere due to being larger than a buffer. It seems that it would be wise to try to re-enable strict-aliasing to catch such errors. This must be backported till 1.9. --- src/ssl_sock.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 3943d186b9..c3c05c157b 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -7649,6 +7649,7 @@ smp_fetch_ssl_x_sha1(const struct arg *args, struct sample *smp, const char *kw, X509 *crt = NULL; const EVP_MD *digest; int ret = 0; + unsigned int len = 0; struct buffer *smp_trash; struct connection *conn; struct ssl_sock_ctx *ctx; @@ -7672,9 +7673,8 @@ smp_fetch_ssl_x_sha1(const struct arg *args, struct sample *smp, const char *kw, smp_trash = get_trash_chunk(); digest = EVP_sha1(); - X509_digest(crt, digest, (unsigned char *) smp_trash->area, - (unsigned int *)&smp_trash->data); - + X509_digest(crt, digest, (unsigned char *) smp_trash->area, &len); + smp_trash->data = len; smp->data.u.str = *smp_trash; smp->data.type = SMP_T_BIN; ret = 1; @@ -8208,6 +8208,7 @@ smp_fetch_ssl_fc_npn(const struct arg *args, struct sample *smp, const char *kw, { struct connection *conn; struct ssl_sock_ctx *ctx; + unsigned int len = 0; smp->flags = SMP_F_CONST; smp->data.type = SMP_T_STR; @@ -8220,12 +8221,13 @@ smp_fetch_ssl_fc_npn(const struct arg *args, struct sample *smp, const char *kw, smp->data.u.str.area = NULL; SSL_get0_next_proto_negotiated(ctx->ssl, - (const unsigned char **)&smp->data.u.str.area, - (unsigned *)&smp->data.u.str.data); + (const unsigned char **)&smp->data.u.str.area, + &len); if (!smp->data.u.str.area) return 0; + smp->data.u.str.data = len; return 1; } #endif @@ -8236,6 +8238,7 @@ smp_fetch_ssl_fc_alpn(const struct arg *args, struct sample *smp, const char *kw { struct connection *conn; struct ssl_sock_ctx *ctx; + unsigned int len = 0; smp->flags = SMP_F_CONST; smp->data.type = SMP_T_STR; @@ -8249,12 +8252,13 @@ smp_fetch_ssl_fc_alpn(const struct arg *args, struct sample *smp, const char *kw smp->data.u.str.area = NULL; SSL_get0_alpn_selected(ctx->ssl, - (const unsigned char **)&smp->data.u.str.area, - (unsigned *)&smp->data.u.str.data); + (const unsigned char **)&smp->data.u.str.area, + &len); if (!smp->data.u.str.area) return 0; + smp->data.u.str.data = len; return 1; } #endif @@ -8298,6 +8302,7 @@ smp_fetch_ssl_fc_session_id(const struct arg *args, struct sample *smp, const ch smp->strm ? cs_conn(objt_cs(smp->strm->si[1].end)) : NULL; SSL_SESSION *ssl_sess; struct ssl_sock_ctx *ctx; + unsigned int len = 0; smp->flags = SMP_F_CONST; smp->data.type = SMP_T_BIN; @@ -8310,11 +8315,11 @@ smp_fetch_ssl_fc_session_id(const struct arg *args, struct sample *smp, const ch if (!ssl_sess) return 0; - smp->data.u.str.area = (char *)SSL_SESSION_get_id(ssl_sess, - (unsigned int *)&smp->data.u.str.data); + smp->data.u.str.area = (char *)SSL_SESSION_get_id(ssl_sess, &len); if (!smp->data.u.str.area || !smp->data.u.str.data) return 0; + smp->data.u.str.data = len; return 1; } #endif @@ -10633,6 +10638,7 @@ static int cli_io_handler_show_cert_detail(struct appctx *appctx) struct buffer *out = alloc_trash_chunk(); struct buffer *tmp = alloc_trash_chunk(); X509_NAME *name = NULL; + unsigned int len = 0; int write = -1; BIO *bio = NULL; @@ -10706,9 +10712,10 @@ static int cli_io_handler_show_cert_detail(struct appctx *appctx) chunk_reset(tmp); chunk_appendf(out, "SHA1 FingerPrint: "); - if (X509_digest(ckchs->ckch->cert, EVP_sha1(), (unsigned char *) tmp->area, - (unsigned int *)&tmp->data) == 0) + if (X509_digest(ckchs->ckch->cert, EVP_sha1(), (unsigned char *) tmp->area, &len) == 0) goto end; + + tmp->data = len; dump_binary(out, tmp->area, tmp->data); chunk_appendf(out, "\n"); } -- 2.39.5