Skip to content

Commit 5774eca

Browse files
divVerentBoringssl LUCI CQ
authored andcommitted
crypto/x509: Fix interaction of DNS exclude constraints with wildcard DNS names.
An exclusion of "foo.example.com" must match a DNS name of "*.example.com". Bug: 488306305 Change-Id: Id911cb841451da1568c4f938a42c75316a6a6964 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/90167 Auto-Submit: Rudolf Polzer <rpolzer@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: David Benjamin <davidben@google.com>
1 parent e16f17a commit 5774eca

File tree

2 files changed

+194
-94
lines changed

2 files changed

+194
-94
lines changed

crypto/x509/v3_ncons.cc

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include <string_view>
16+
1517
#include <stdio.h>
1618
#include <string.h>
1719

@@ -40,9 +42,11 @@ static int do_i2r_name_constraints(const X509V3_EXT_METHOD *method,
4042
static int print_nc_ipadd(BIO *bp, const ASN1_OCTET_STRING *ip);
4143

4244
static int nc_match(const GENERAL_NAME *gen, const NAME_CONSTRAINTS *nc);
43-
static int nc_match_single(const GENERAL_NAME *sub, const GENERAL_NAME *gen);
45+
static int nc_match_single(const GENERAL_NAME *sub, const GENERAL_NAME *gen,
46+
bool excluding);
4447
static int nc_dn(const X509_NAME *sub, const X509_NAME *nm);
45-
static int nc_dns(const ASN1_IA5STRING *sub, const ASN1_IA5STRING *dns);
48+
static int nc_dns(const ASN1_IA5STRING *sub, const ASN1_IA5STRING *dns,
49+
bool excluding);
4650
static int nc_email(const ASN1_IA5STRING *sub, const ASN1_IA5STRING *eml);
4751
static int nc_uri(const ASN1_IA5STRING *uri, const ASN1_IA5STRING *base);
4852

@@ -269,7 +273,7 @@ static int nc_match(const GENERAL_NAME *gen, const NAME_CONSTRAINTS *nc) {
269273
if (match == 0) {
270274
match = 1;
271275
}
272-
int r = nc_match_single(gen, sub->base);
276+
int r = nc_match_single(gen, sub->base, /*excluding=*/false);
273277
if (r == X509_V_OK) {
274278
match = 2;
275279
} else if (r != X509_V_ERR_PERMITTED_VIOLATION) {
@@ -290,7 +294,7 @@ static int nc_match(const GENERAL_NAME *gen, const NAME_CONSTRAINTS *nc) {
290294
return X509_V_ERR_SUBTREE_MINMAX;
291295
}
292296

293-
int r = nc_match_single(gen, sub->base);
297+
int r = nc_match_single(gen, sub->base, /*excluding=*/true);
294298
if (r == X509_V_OK) {
295299
return X509_V_ERR_EXCLUDED_VIOLATION;
296300
} else if (r != X509_V_ERR_PERMITTED_VIOLATION) {
@@ -301,13 +305,14 @@ static int nc_match(const GENERAL_NAME *gen, const NAME_CONSTRAINTS *nc) {
301305
return X509_V_OK;
302306
}
303307

304-
static int nc_match_single(const GENERAL_NAME *gen, const GENERAL_NAME *base) {
308+
static int nc_match_single(const GENERAL_NAME *gen, const GENERAL_NAME *base,
309+
bool excluding) {
305310
switch (base->type) {
306311
case GEN_DIRNAME:
307312
return nc_dn(gen->d.directoryName, base->d.directoryName);
308313

309314
case GEN_DNS:
310-
return nc_dns(gen->d.dNSName, base->d.dNSName);
315+
return nc_dns(gen->d.dNSName, base->d.dNSName, excluding);
311316

312317
case GEN_EMAIL:
313318
return nc_email(gen->d.rfc822Name, base->d.rfc822Name);
@@ -348,6 +353,15 @@ static int starts_with(const CBS *cbs, uint8_t c) {
348353
return CBS_len(cbs) > 0 && CBS_data(cbs)[0] == c;
349354
}
350355

356+
static int starts_with_str(const CBS *cbs, std::string_view str) {
357+
return CBS_len(cbs) >= str.size() &&
358+
!OPENSSL_memcmp(CBS_data(cbs), str.data(), str.size());
359+
}
360+
361+
static int ends_with(const CBS *cbs, uint8_t c) {
362+
return CBS_len(cbs) > 0 && CBS_data(cbs)[CBS_len(cbs) - 1] == c;
363+
}
364+
351365
static int equal_case(const CBS *a, const CBS *b) {
352366
if (CBS_len(a) != CBS_len(b)) {
353367
return 0;
@@ -372,7 +386,8 @@ static int has_suffix_case(const CBS *a, const CBS *b) {
372386
return equal_case(&copy, b);
373387
}
374388

375-
static int nc_dns(const ASN1_IA5STRING *dns, const ASN1_IA5STRING *base) {
389+
static int nc_dns(const ASN1_IA5STRING *dns, const ASN1_IA5STRING *base,
390+
bool excluding) {
376391
CBS dns_cbs, base_cbs;
377392
CBS_init(&dns_cbs, dns->data, dns->length);
378393
CBS_init(&base_cbs, base->data, base->length);
@@ -382,6 +397,34 @@ static int nc_dns(const ASN1_IA5STRING *dns, const ASN1_IA5STRING *base) {
382397
return X509_V_OK;
383398
}
384399

400+
// Normalize absolute DNS names by removing the trailing dot, if any.
401+
if (ends_with(&dns_cbs, '.')) {
402+
uint8_t unused;
403+
CBS_get_last_u8(&dns_cbs, &unused);
404+
}
405+
if (ends_with(&base_cbs, '.')) {
406+
uint8_t unused;
407+
CBS_get_last_u8(&base_cbs, &unused);
408+
}
409+
410+
// Wildcard partial-match handling ("*.bar.com" matching name constraint
411+
// "foo.bar.com"). This only handles the case where the the dnsname and the
412+
// constraint match after removing the leftmost label, otherwise it is handled
413+
// by falling through to the check of whether the dnsname is fully within or
414+
// fully outside of the constraint.
415+
if (excluding && starts_with_str(&dns_cbs, "*.")) {
416+
CBS unused;
417+
CBS base_parent_cbs = base_cbs;
418+
CBS dns_parent_cbs = dns_cbs;
419+
CBS_skip(&dns_parent_cbs, 2);
420+
if (CBS_get_until_first(&base_parent_cbs, &unused, '.') &&
421+
CBS_skip(&base_parent_cbs, 1)) {
422+
if (equal_case(&dns_parent_cbs, &base_parent_cbs)) {
423+
return X509_V_OK;
424+
}
425+
}
426+
}
427+
385428
// If |base_cbs| begins with a '.', do a simple suffix comparison. This is
386429
// not part of RFC5280, but is part of OpenSSL's original behavior.
387430
if (starts_with(&base_cbs, '.')) {

0 commit comments

Comments
 (0)