Fix message for too long tls-crypt-v2 metadata

The current code only checks if the base64-encoded metadata is at most
980 characters. However, that can encode up to 735 bytes of data, while
only up to 733 bytes are allowed. When passing 734 or 735 bytes, openvpn
prints a misleading error message saying that the base64 cannot be
decoded.

This patch checks the decoded length to show an accurate error message.

v2: Remove now-unused macro and fix an off-by-one error.

Signed-off-by: Max Fillinger <maximilian.fillinger@foxcrypto.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221214153414.12671-1-maximilian.fillinger@foxcrypto.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25694.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 860bf4bf9248077259690a518925ecc14da4b320)

diff --git a/src/openvpn/base64.h b/src/openvpn/base64.h
index f49860fc65a967026f2fad7bc1bda1673ff1ca8d..7b4224a51b4b635703250a77c42426f7c89fd018 100644 (file)
--- a/src/openvpn/base64.h
+++ b/src/openvpn/base64.h
@@ -38,6 +38,10 @@
 #define OPENVPN_BASE64_LENGTH(binary_length) \
     ((((8 * binary_length) / 6) + 3) & ~3)
 
+/** Compute the maximal number of bytes encoded in a base64 string. */
+#define OPENVPN_BASE64_DECODED_LENGTH(base64_length) \
+    ((base64_length / 4) * 3)
+
 int openvpn_base64_encode(const void *data, int size, char **str);
 
 int openvpn_base64_decode(const char *str, void *data, int size);

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 2fc791119360890e823ec6f0dbfc4c292e398bf5..1e461fcf6b53032339b6aeda403c27750c39e7b7 100644 (file)
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -627,15 +627,11 @@ tls_crypt_v2_write_client_key_file(const char *filename,
     }
     ASSERT(buf_write(&dst, client_key.keys, sizeof(client_key.keys)));
 
-    struct buffer metadata = alloc_buf_gc(TLS_CRYPT_V2_MAX_METADATA_LEN, &gc);
+    struct buffer metadata;
     if (b64_metadata)
     {
-        if (TLS_CRYPT_V2_MAX_B64_METADATA_LEN < strlen(b64_metadata))
-        {
-            msg(M_FATAL,
-                "ERROR: metadata too long (%d bytes, max %u bytes)",
-                (int)strlen(b64_metadata), TLS_CRYPT_V2_MAX_B64_METADATA_LEN);
-        }
+        size_t b64_length = strlen(b64_metadata);
+        metadata = alloc_buf_gc(OPENVPN_BASE64_DECODED_LENGTH(b64_length) + 1, &gc);
         ASSERT(buf_write(&metadata, &TLS_CRYPT_METADATA_TYPE_USER, 1));
         int decoded_len = openvpn_base64_decode(b64_metadata, BEND(&metadata),
                                                 BCAP(&metadata));
@@ -644,10 +640,18 @@ tls_crypt_v2_write_client_key_file(const char *filename,
             msg(M_FATAL, "ERROR: failed to base64 decode provided metadata");
             goto cleanup;
         }
+        if (decoded_len > TLS_CRYPT_V2_MAX_METADATA_LEN - 1)
+        {
+            msg(M_FATAL,
+                "ERROR: metadata too long (%d bytes, max %u bytes)",
+                decoded_len, TLS_CRYPT_V2_MAX_METADATA_LEN - 1);
+            goto cleanup;
+        }
         ASSERT(buf_inc_len(&metadata, decoded_len));
     }
     else
     {
+        metadata = alloc_buf_gc(1 + sizeof(int64_t), &gc);
         int64_t timestamp = htonll((uint64_t)now);
         ASSERT(buf_write(&metadata, &TLS_CRYPT_METADATA_TYPE_TIMESTAMP, 1));
         ASSERT(buf_write(&metadata, &timestamp, sizeof(timestamp)));

diff --git a/src/openvpn/tls_crypt.h b/src/openvpn/tls_crypt.h
index 928ff5475836623ecb834a6a889a7bc231e0188d..d5c737523702a6b93c1c85e0f03ac47dd0faf3d7 100644 (file)
--- a/src/openvpn/tls_crypt.h
+++ b/src/openvpn/tls_crypt.h
@@ -101,8 +101,6 @@
 #define TLS_CRYPT_V2_MAX_METADATA_LEN (unsigned)(TLS_CRYPT_V2_MAX_WKC_LEN \
                                                  - (TLS_CRYPT_V2_CLIENT_KEY_LEN + TLS_CRYPT_V2_TAG_SIZE \
                                                     + sizeof(uint16_t)))
-#define TLS_CRYPT_V2_MAX_B64_METADATA_LEN \
-    OPENVPN_BASE64_LENGTH(TLS_CRYPT_V2_MAX_METADATA_LEN - 1)
 
 /**
  * Initialize a key_ctx_bi structure for use with --tls-crypt.