Am 26.11.22 um 17:26 schrieb Max Fillinger:
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.
Gert looked at this again and we found some issues now:
This patch leaves an unused define TLS_CRYPT_V2_MAX_B64_METADATA_LEN
that is no longer used.
TLS_CRYPT_V2_MAX_METADATA_LEN is actually 734 sice it includes the type
byte for the type of metadata, which gives us the 733 bytes of metadata
decoded from base64.
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)
+ {
+ msg(M_FATAL,
+ "ERROR: metadata too long (%d bytes, max %u bytes)",
+ decoded_len, TLS_CRYPT_V2_MAX_METADATA_LEN - 1);
+ goto cleanup;
+ }
The error message correctly uses 733 as length but the check should also
check for TLS_CRYPT_V2_MAX_METADATA_LEN -1 or use >=
Maybe we can add a unit test for this
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
