Skip to content

Commit 860bf4b

Browse files
mfilcron2
authored andcommitted
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>
1 parent 235161c commit 860bf4b

File tree

3 files changed

+15
-9
lines changed

3 files changed

+15
-9
lines changed

src/openvpn/base64.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@
3838
#define OPENVPN_BASE64_LENGTH(binary_length) \
3939
((((8 * binary_length) / 6) + 3) & ~3)
4040

41+
/** Compute the maximal number of bytes encoded in a base64 string. */
42+
#define OPENVPN_BASE64_DECODED_LENGTH(base64_length) \
43+
((base64_length / 4) * 3)
44+
4145
int openvpn_base64_encode(const void *data, int size, char **str);
4246

4347
int openvpn_base64_decode(const char *str, void *data, int size);

src/openvpn/tls_crypt.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -627,15 +627,11 @@ tls_crypt_v2_write_client_key_file(const char *filename,
627627
}
628628
ASSERT(buf_write(&dst, client_key.keys, sizeof(client_key.keys)));
629629

630-
struct buffer metadata = alloc_buf_gc(TLS_CRYPT_V2_MAX_METADATA_LEN, &gc);
630+
struct buffer metadata;
631631
if (b64_metadata)
632632
{
633-
if (TLS_CRYPT_V2_MAX_B64_METADATA_LEN < strlen(b64_metadata))
634-
{
635-
msg(M_FATAL,
636-
"ERROR: metadata too long (%d bytes, max %u bytes)",
637-
(int)strlen(b64_metadata), TLS_CRYPT_V2_MAX_B64_METADATA_LEN);
638-
}
633+
size_t b64_length = strlen(b64_metadata);
634+
metadata = alloc_buf_gc(OPENVPN_BASE64_DECODED_LENGTH(b64_length) + 1, &gc);
639635
ASSERT(buf_write(&metadata, &TLS_CRYPT_METADATA_TYPE_USER, 1));
640636
int decoded_len = openvpn_base64_decode(b64_metadata, BEND(&metadata),
641637
BCAP(&metadata));
@@ -644,10 +640,18 @@ tls_crypt_v2_write_client_key_file(const char *filename,
644640
msg(M_FATAL, "ERROR: failed to base64 decode provided metadata");
645641
goto cleanup;
646642
}
643+
if (decoded_len > TLS_CRYPT_V2_MAX_METADATA_LEN - 1)
644+
{
645+
msg(M_FATAL,
646+
"ERROR: metadata too long (%d bytes, max %u bytes)",
647+
decoded_len, TLS_CRYPT_V2_MAX_METADATA_LEN - 1);
648+
goto cleanup;
649+
}
647650
ASSERT(buf_inc_len(&metadata, decoded_len));
648651
}
649652
else
650653
{
654+
metadata = alloc_buf_gc(1 + sizeof(int64_t), &gc);
651655
int64_t timestamp = htonll((uint64_t)now);
652656
ASSERT(buf_write(&metadata, &TLS_CRYPT_METADATA_TYPE_TIMESTAMP, 1));
653657
ASSERT(buf_write(&metadata, &timestamp, sizeof(timestamp)));

src/openvpn/tls_crypt.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,6 @@
101101
#define TLS_CRYPT_V2_MAX_METADATA_LEN (unsigned)(TLS_CRYPT_V2_MAX_WKC_LEN \
102102
- (TLS_CRYPT_V2_CLIENT_KEY_LEN + TLS_CRYPT_V2_TAG_SIZE \
103103
+ sizeof(uint16_t)))
104-
#define TLS_CRYPT_V2_MAX_B64_METADATA_LEN \
105-
OPENVPN_BASE64_LENGTH(TLS_CRYPT_V2_MAX_METADATA_LEN - 1)
106104

107105
/**
108106
* Initialize a key_ctx_bi structure for use with --tls-crypt.

0 commit comments

Comments
 (0)