kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Support the OpenSSL “pass zero for strlen” when setting X.509 hostnames.

Browse Source 
BoringSSL does not generally support this quirk but, in this case, we
didn't make it a fatal error and it's instead a silent omission of
hostname checking. This doesn't affect Chrome but, in case something is
using BoringSSL and using this trick, this change makes it safe.

BUG=chromium:824799

Change-Id: If417817b997b9faa9963c09dfc95d06a5d445e0b
Reviewed-on: https://boringssl-review.googlesource.com/26724
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
Adam Langley 2018-03-22 10:02:54 -07:00 committed by CQ bot account: commit-bot@chromium.org
parent d67e311ce4
commit e759a9cd84
2 changed files with 29 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
crypto/x509/x509_test.cc
View File

@ -487,7 +487,9 @@ static int Verify(X509 *leaf, const std::vector<X509 *> &roots,
const std::vector<X509 *> &intermediates,
const std::vector<X509_CRL *> &crls,
unsigned long flags,
bool use_additional_untrusted) {
bool use_additional_untrusted,
const char *hostname,
size_t hostname_len) {
bssl::UniquePtr<STACK_OF(X509)> roots_stack(CertsToStack(roots));
bssl::UniquePtr<STACK_OF(X509)> intermediates_stack(
CertsToStack(intermediates));
@ -526,6 +528,7 @@ static int Verify(X509 *leaf, const std::vector<X509 *> &roots,
}
X509_VERIFY_PARAM_set_time(param, 1474934400 /* Sep 27th, 2016 */);
X509_VERIFY_PARAM_set_depth(param, 16);
X509_VERIFY_PARAM_set1_host(param, hostname, hostname_len);
if (flags) {
X509_VERIFY_PARAM_set_flags(param, flags);
}
@ -543,8 +546,10 @@ static int Verify(X509 *leaf, const std::vector<X509 *> &roots,
const std::vector<X509 *> &intermediates,
const std::vector<X509_CRL *> &crls,
unsigned long flags = 0) {
const int r1 = Verify(leaf, roots, intermediates, crls, flags, false);
const int r2 = Verify(leaf, roots, intermediates, crls, flags, true);
const int r1 =
Verify(leaf, roots, intermediates, crls, flags, false, nullptr, 0);
const int r2 =
Verify(leaf, roots, intermediates, crls, flags, true, nullptr, 0);
if (r1 != r2) {
fprintf(stderr,
@ -612,6 +617,19 @@ TEST(X509Test, TestVerify) {
Verify(forgery.get(),
{intermediate_self_signed.get(), root_cross_signed.get()},
{leaf_no_key_usage.get(), intermediate.get()}, empty_crls));
static const char kHostname[] = "example.com";
static const char kWrongHostname[] = "example2.com";
ASSERT_EQ(X509_V_OK,
Verify(leaf.get(), {root.get()}, {intermediate.get()}, empty_crls,
0, false, kHostname, strlen(kHostname)));
// The wrong hostname should trigger a hostname error.
ASSERT_EQ(X509_V_ERR_HOSTNAME_MISMATCH,
Verify(leaf.get(), {root.get()}, {intermediate.get()}, empty_crls,
0, false, kWrongHostname, strlen(kWrongHostname)));
// Passing zero, for this API, is supported for compatibility with OpenSSL.
ASSERT_EQ(X509_V_OK, Verify(leaf.get(), {root.get()}, {intermediate.get()},
empty_crls, 0, false, kHostname, 0));
}
TEST(X509Test, TestCRL) {

8
crypto/x509/x509_vpm.c
View File

@ -89,6 +89,14 @@ static int int_x509_param_set_hosts(X509_VERIFY_PARAM_ID *id, int mode,
{
char *copy;
// This is an OpenSSL quirk that BoringSSL typically doesn't support.
// However, we didn't make this a fatal error at the time, which was a
// mistake. Because of that, given the risk that someone could assume the
// OpenSSL semantics from BoringSSL, it's supported in this case.
if (name != NULL && namelen == 0) {
namelen = strlen(name);
}
/*
* Refuse names with embedded NUL bytes.
* XXX: Do we need to push an error onto the error stack?