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

diff --git a/openssl.c b/openssl.c
index 4dac8ea..a57cb32 100644 (file)
--- 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 491a970..001f25e 100644 (file)
--- 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 b253e19..e07397f 100644 (file)
--- 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 a8dd371..35970eb 100644 (file)
--- 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);
    }
 }