Hi,
I originally created this on github[0], but as it's not part of the
portable bits, I've asked to post this on this OpenBSD tech mailing
list, I hope this is the correct way of addressing this.
The attached patch aims to fix an API difference between OpenSSL and
LibreSSL for X509_check_host/X509_check_email and also makes them
behave like described in the respective man page.
According to the X509_check_host(3)
(src/lib/libcrypto/man/X509_check_host.3), "The namelen argument must
be the number of characters in the name string or zero, in which case
the length is calculated with strlen(name)" for both functions
mentioned above. Even though I think this not a good API (I would
always make the called pass the exact length and call it a day), this
causes compatibility problems with code written for OpenSSL, a good
example is MariaDB's SSL certificate hostname check found here:
https://github.com/MariaDB/server/blob/10.3/sql-common/client.c#L1825
This change uses the code from OpenSSL's implementation, which on top of
calculating the string length also tolerates the caller including
terminating NUL in string length. This way, LibreSSL's code should be
API compatible with OpenSSL's interface (even though I don't get why
it's necessary for an API to correct invalid input, but that's a
different story I guess).
Quick and dirty example code demonstrating the problem:
#include <stdio.h>
#include <string.h>
#include <openssl/x509v3.h>
int main()
{
X509_NAME *name = X509_NAME_new();
X509 *cert = X509_new();
const char* cn = "test.example.org";
X509_NAME_add_entry_by_NID(name, NID_commonName,
MBSTRING_ASC, (unsigned char *)cn, -1, -1, 0);
X509_set_subject_name(cert, name);
X509_NAME_free(name);
printf("Check host without strlen for %s results in %i\n",
cn, X509_check_host(cert, cn, 0, 0, 0));
printf("Check host with strlen for %s results in %i\n",
cn, X509_check_host(cert, cn, strlen(cn), 0, 0));
}
outputs
Check host without strlen for test.example.org results in 0
Check host with strlen for test.example.org results in 1
After the patch (as expected):
Check host without strlen for test.example.org results in 1
Check host with strlen for test.example.org results in 1
Best,
Michael
[0]https://github.com/libressl-portable/openbsd/pull/87
--
Michael Gmelin
diff --git a/src/lib/libcrypto/x509v3/v3_utl.c b/src/lib/libcrypto/x509v3/v3_utl.c
index 04c789922b..58020214fc 100644
--- a/src/lib/libcrypto/x509v3/v3_utl.c
+++ b/src/lib/libcrypto/x509v3/v3_utl.c
@@ -1015,8 +1015,17 @@ int X509_check_host(X509 *x, const char *chk, size_t chklen,
{
if (chk == NULL)
return -2;
- if (memchr(chk, '\0', chklen))
+ /*
+ * Embedded NULs are disallowed, except as the last character of a
+ * string of length 2 or more (tolerate caller including terminating
+ * NUL in string length).
+ */
+ if (chklen == 0)
+ chklen = strlen(chk);
+ else if (memchr(chk, '\0', chklen > 1 ? chklen - 1 : chklen))
return -2;
+ if (chklen > 1 && chk[chklen - 1] == '\0')
+ --chklen;
return do_x509_check(x, chk, chklen, flags, GEN_DNS, peername);
}
@@ -1025,8 +1034,17 @@ int X509_check_email(X509 *x, const char *chk, size_t chklen,
{
if (chk == NULL)
return -2;
- if (memchr(chk, '\0', chklen))
+ /*
+ * Embedded NULs are disallowed, except as the last character of a
+ * string of length 2 or more (tolerate caller including terminating
+ * NUL in string length).
+ */
+ if (chklen == 0)
+ chklen = strlen(chk);
+ else if (memchr(chk, '\0', chklen > 1 ? chklen - 1 : chklen))
return -2;
+ if (chklen > 1 && chk[chklen - 1] == '\0')
+ --chklen;
return do_x509_check(x, chk, chklen, flags, GEN_EMAIL, NULL);
}
