From 85bc700695d99d5858dbaa1448251e48df9ce747 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Wed, 17 Mar 2021 09:13:53 +0100 Subject: [PATCH] Store the PEM certificate in a dynamically allocated buffer ... when https-inspecting. Should prevent errors like: 2021-03-16 22:36:19.148 7f47bbfff700 Error: X509 PEM cert len 16694 is larger than buffer len 16383 As a bonus it should slightly reduce the memory usage as most certificates are smaller than the previously used fixed buffer. Reported by: Wen Yue --- openssl.c | 13 ++++++++----- project.h | 3 +-- ssl.c | 24 ++++++++++++++++++++++-- ssl_common.c | 9 +++++++-- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/openssl.c b/openssl.c index 4dac8ea9..a57cb32e 100644 --- a/openssl.c +++ b/openssl.c @@ -302,7 +302,7 @@ static int ssl_store_cert(struct client_state *csp, X509 *crt) last->next = malloc_or_die(sizeof(struct certs_chain)); last->next->next = NULL; memset(last->next->info_buf, 0, sizeof(last->next->info_buf)); - memset(last->next->file_buf, 0, sizeof(last->next->file_buf)); + last->next->file_buf = NULL; /* * Saving certificate file into buffer @@ -316,15 +316,18 @@ static int ssl_store_cert(struct client_state *csp, X509 *crt) len = BIO_get_mem_data(bio, &bio_mem_data); - if (len > (sizeof(last->file_buf) - 1)) + last->file_buf = malloc((size_t)len + 1); + if (last->file_buf == NULL) { log_error(LOG_LEVEL_ERROR, - "X509 PEM cert len %ld is larger than buffer len %lu", - len, sizeof(last->file_buf) - 1); - len = sizeof(last->file_buf) - 1; + "Failed to allocate %lu bytes to store the X509 PEM certificate", + len + 1); + ret = -1; + goto exit; } strncpy(last->file_buf, bio_mem_data, (size_t)len); + last->file_buf[len] = '\0'; BIO_free(bio); bio = BIO_new(BIO_s_mem()); if (!bio) diff --git a/project.h b/project.h index 491a970c..001f25e9 100644 --- a/project.h +++ b/project.h @@ -49,7 +49,6 @@ * Macros for SSL structures */ #define CERT_INFO_BUF_SIZE 4096 -#define CERT_FILE_BUF_SIZE 16384 #define ISSUER_NAME_BUF_SIZE 2048 #define HASH_OF_HOST_BUF_SIZE 16 #endif /* FEATURE_HTTPS_INSPECTION */ @@ -363,7 +362,7 @@ struct http_request */ typedef struct certs_chain { char info_buf[CERT_INFO_BUF_SIZE]; /* text info about properties of certificate */ - char file_buf[CERT_FILE_BUF_SIZE]; /* buffer for whole certificate - format to save in file */ + char *file_buf; /* buffer for whole certificate - format to save in file */ struct certs_chain *next; /* next certificate in chain of trust */ } certs_chain_t; #endif diff --git a/ssl.c b/ssl.c index b253e193..e07397f9 100644 --- a/ssl.c +++ b/ssl.c @@ -1707,6 +1707,7 @@ static int ssl_verify_callback(void *csp_void, mbedtls_x509_crt *crt, struct certs_chain *last = &(csp->server_certs_chain); size_t olen = 0; int ret = 0; + size_t pem_buffer_length; /* * Searching for last item in certificates linked list @@ -1722,14 +1723,33 @@ static int ssl_verify_callback(void *csp_void, mbedtls_x509_crt *crt, last->next = malloc_or_die(sizeof(struct certs_chain)); last->next->next = NULL; memset(last->next->info_buf, 0, sizeof(last->next->info_buf)); - memset(last->next->file_buf, 0, sizeof(last->next->file_buf)); + last->next->file_buf = NULL; + + ret = mbedtls_pem_write_buffer(PEM_BEGIN_CRT, PEM_END_CRT, crt->raw.p, + crt->raw.len, NULL, 0, &olen); + if (MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL != ret) + { + log_error(LOG_LEVEL_ERROR, + "Failed to figure out the required X509 PEM certificate buffer size"); + return -1; + } + pem_buffer_length = olen; + + last->file_buf = malloc(pem_buffer_length); + if (last->file_buf == NULL) + { + log_error(LOG_LEVEL_ERROR, + "Failed to allocate %lu bytes to store the X509 PEM certificate", + pem_buffer_length); + return -1; + } /* * Saving certificate file into buffer */ if ((ret = mbedtls_pem_write_buffer(PEM_BEGIN_CRT, PEM_END_CRT, crt->raw.p, crt->raw.len, (unsigned char *)last->file_buf, - sizeof(last->file_buf)-1, &olen)) != 0) + pem_buffer_length, &olen)) != 0) { char err_buf[ERROR_BUF_SIZE]; diff --git a/ssl_common.c b/ssl_common.c index a8dd371e..35970eb9 100644 --- a/ssl_common.c +++ b/ssl_common.c @@ -290,8 +290,8 @@ extern void free_certificate_chain(struct client_state *csp) /* Cleaning buffers */ memset(csp->server_certs_chain.info_buf, 0, sizeof(csp->server_certs_chain.info_buf)); - memset(csp->server_certs_chain.file_buf, 0, - sizeof(csp->server_certs_chain.file_buf)); + freez(csp->server_certs_chain.file_buf); + csp->server_certs_chain.next = NULL; /* Freeing memory in whole linked list */ @@ -299,6 +299,11 @@ extern void free_certificate_chain(struct client_state *csp) { struct certs_chain *cert_for_free = cert; cert = cert->next; + + /* Cleaning buffers */ + memset(cert_for_free->info_buf, 0, sizeof(cert_for_free->info_buf)); + freez(cert_for_free->file_buf); + freez(cert_for_free); } } -- 2.39.2