rpms/gnutls
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Tolerate duplicate certs in the chain

Browse Source
This commit is contained in:
Daiki Ueno 2021-02-10 16:42:27 +01:00
parent 07736fd804
commit 25ae742c52
2 changed files with 408 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

403
gnutls-3.7.0-duplicate-certs.patch Normal file
View File

@ -0,0 +1,403 @@
From 09b40be6e0e0a59ba4bd764067eb353241043a70 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Mon, 28 Dec 2020 12:14:13 +0100
Subject: [PATCH] gnutls_x509_trust_list_verify_crt2: ignore duplicate
certificates
The commit ebb19db9165fed30d73c83bab1b1b8740c132dfd caused a
regression, where duplicate certificates in a certificate chain are no
longer ignored but treated as a non-contiguous segment and that
results in calling the issuer callback, or a verification failure.
This adds a mechanism to record certificates already seen in the
chain, and skip them while still allow the caller to inject missing
certificates.
Signed-off-by: Daiki Ueno <ueno@gnu.org>
Co-authored-by: Andreas Metzler <ametzler@debian.org>
---
lib/x509/common.c | 8 ++
lib/x509/verify-high.c | 157 +++++++++++++++++++++++++++++++------
tests/missingissuer.c | 2 +
tests/test-chains-issuer.h | 101 +++++++++++++++++++++++-
4 files changed, 245 insertions(+), 23 deletions(-)
diff --git a/lib/x509/common.c b/lib/x509/common.c
index 3301aaad0..10c8db53c 100644
--- a/lib/x509/common.c
+++ b/lib/x509/common.c
@@ -1758,6 +1758,14 @@ unsigned int _gnutls_sort_clist(gnutls_x509_crt_t *clist,
* increasing DEFAULT_MAX_VERIFY_DEPTH.
*/
for (i = 0; i < clist_size; i++) {
+ /* Self-signed certificate found in the chain; skip it
+ * as it should only appear in the trusted set.
+ */
+ if (gnutls_x509_crt_check_issuer(clist[i], clist[i])) {
+ _gnutls_cert_log("self-signed cert found", clist[i]);
+ continue;
+ }
+
for (j = 1; j < clist_size; j++) {
if (i == j)
continue;
diff --git a/lib/x509/verify-high.c b/lib/x509/verify-high.c
index 588e7ee0d..9a16e6b42 100644
--- a/lib/x509/verify-high.c
+++ b/lib/x509/verify-high.c
@@ -67,6 +67,80 @@ struct gnutls_x509_trust_list_iter {
#define DEFAULT_SIZE 127
+struct cert_set_node_st {
+ gnutls_x509_crt_t *certs;
+ unsigned int size;
+};
+
+struct cert_set_st {
+ struct cert_set_node_st *node;
+ unsigned int size;
+};
+
+static int
+cert_set_init(struct cert_set_st *set, unsigned int size)
+{
+ memset(set, 0, sizeof(*set));
+
+ set->size = size;
+ set->node = gnutls_calloc(size, sizeof(*set->node));
+ if (!set->node) {
+ return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
+ }
+
+ return 0;
+}
+
+static void
+cert_set_deinit(struct cert_set_st *set)
+{
+ size_t i;
+
+ for (i = 0; i < set->size; i++) {
+ gnutls_free(set->node[i].certs);
+ }
+
+ gnutls_free(set->node);
+}
+
+static bool
+cert_set_contains(struct cert_set_st *set, const gnutls_x509_crt_t cert)
+{
+ size_t hash, i;
+
+ hash = hash_pjw_bare(cert->raw_dn.data, cert->raw_dn.size);
+ hash %= set->size;
+
+ for (i = 0; i < set->node[hash].size; i++) {
+ if (unlikely(gnutls_x509_crt_equals(set->node[hash].certs[i], cert))) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static int
+cert_set_add(struct cert_set_st *set, const gnutls_x509_crt_t cert)
+{
+ size_t hash;
+
+ hash = hash_pjw_bare(cert->raw_dn.data, cert->raw_dn.size);
+ hash %= set->size;
+
+ set->node[hash].certs =
+ gnutls_realloc_fast(set->node[hash].certs,
+ (set->node[hash].size + 1) *
+ sizeof(*set->node[hash].certs));
+ if (!set->node[hash].certs) {
+ return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);
+ }
+ set->node[hash].certs[set->node[hash].size] = cert;
+ set->node[hash].size++;
+
+ return 0;
+}
+
/**
* gnutls_x509_trust_list_init:
* @list: A pointer to the type to be initialized
@@ -1328,6 +1402,7 @@ gnutls_x509_trust_list_verify_crt2(gnutls_x509_trust_list_t list,
unsigned have_set_name = 0;
unsigned saved_output;
gnutls_datum_t ip = {NULL, 0};
+ struct cert_set_st cert_set = { NULL, 0 };
if (cert_list == NULL || cert_list_size < 1)
return gnutls_assert_val(GNUTLS_E_INVALID_REQUEST);
@@ -1376,36 +1451,68 @@ gnutls_x509_trust_list_verify_crt2(gnutls_x509_trust_list_t list,
memcpy(sorted, cert_list, cert_list_size * sizeof(gnutls_x509_crt_t));
cert_list = sorted;
+ ret = cert_set_init(&cert_set, DEFAULT_MAX_VERIFY_DEPTH);
+ if (ret < 0) {
+ return ret;
+ }
+
for (i = 0; i < cert_list_size &&
- cert_list_size <= DEFAULT_MAX_VERIFY_DEPTH; i++) {
- if (!(flags & GNUTLS_VERIFY_DO_NOT_ALLOW_UNSORTED_CHAIN)) {
- unsigned int sorted_size;
+ cert_list_size <= DEFAULT_MAX_VERIFY_DEPTH; ) {
+ unsigned int sorted_size = 1;
+ unsigned int j;
+ gnutls_x509_crt_t issuer;
+ if (!(flags & GNUTLS_VERIFY_DO_NOT_ALLOW_UNSORTED_CHAIN)) {
sorted_size = _gnutls_sort_clist(&cert_list[i],
cert_list_size - i);
- i += sorted_size - 1;
}
- if (i == cert_list_size - 1) {
- gnutls_x509_crt_t issuer;
-
- /* If it is the last certificate and its issuer is
- * known, don't need to run issuer callback. */
- if (_gnutls_trust_list_get_issuer(list,
- cert_list[i],
- &issuer,
- 0) == 0) {
+ /* Remove duplicates. Start with index 1, as the first element
+ * may be re-checked after issuer retrieval. */
+ for (j = 1; j < sorted_size; j++) {
+ if (cert_set_contains(&cert_set, cert_list[i + j])) {
+ if (i + j < cert_list_size - 1) {
+ memmove(&cert_list[i + j],
+ &cert_list[i + j + 1],
+ sizeof(cert_list[i]));
+ }
+ cert_list_size--;
break;
}
- } else if (gnutls_x509_crt_check_issuer(cert_list[i],
- cert_list[i + 1])) {
- /* There is no gap between this and the next
- * certificate. */
+ }
+ /* Found a duplicate, try again with the same index. */
+ if (j < sorted_size) {
+ continue;
+ }
+
+ /* Record the certificates seen. */
+ for (j = 0; j < sorted_size; j++, i++) {
+ ret = cert_set_add(&cert_set, cert_list[i]);
+ if (ret < 0) {
+ goto cleanup;
+ }
+ }
+
+ /* If the issuer of the certificate is known, no need
+ * for further processing. */
+ if (_gnutls_trust_list_get_issuer(list,
+ cert_list[i - 1],
+ &issuer,
+ 0) == 0) {
+ cert_list_size = i;
+ break;
+ }
+
+ /* If there is no gap between this and the next certificate,
+ * proceed with the next certificate. */
+ if (i < cert_list_size &&
+ gnutls_x509_crt_check_issuer(cert_list[i - 1],
+ cert_list[i])) {
continue;
}
ret = retrieve_issuers(list,
- cert_list[i],
+ cert_list[i - 1],
&retrieved[retrieved_size],
DEFAULT_MAX_VERIFY_DEPTH -
MAX(retrieved_size,
@@ -1413,15 +1520,20 @@ gnutls_x509_trust_list_verify_crt2(gnutls_x509_trust_list_t list,
if (ret < 0) {
break;
} else if (ret > 0) {
- memmove(&cert_list[i + 1 + ret],
- &cert_list[i + 1],
- (cert_list_size - i - 1) *
+ assert((unsigned int)ret <=
+ DEFAULT_MAX_VERIFY_DEPTH - cert_list_size);
+ memmove(&cert_list[i + ret],
+ &cert_list[i],
+ (cert_list_size - i) *
sizeof(gnutls_x509_crt_t));
- memcpy(&cert_list[i + 1],
+ memcpy(&cert_list[i],
&retrieved[retrieved_size],
ret * sizeof(gnutls_x509_crt_t));
retrieved_size += ret;
cert_list_size += ret;
+
+ /* Start again from the end of the previous segment. */
+ i--;
}
}
@@ -1581,6 +1693,7 @@ gnutls_x509_trust_list_verify_crt2(gnutls_x509_trust_list_t list,
for (i = 0; i < retrieved_size; i++) {
gnutls_x509_crt_deinit(retrieved[i]);
}
+ cert_set_deinit(&cert_set);
return ret;
}
diff --git a/tests/missingissuer.c b/tests/missingissuer.c
index f21e2b6b0..226d09592 100644
--- a/tests/missingissuer.c
+++ b/tests/missingissuer.c
@@ -145,6 +145,8 @@ void doit(void)
printf("[%d]: Chain '%s'...\n", (int)i, chains[i].name);
for (j = 0; chains[i].chain[j]; j++) {
+ assert(j < MAX_CHAIN);
+
if (debug > 2)
printf("\tAdding certificate %d...", (int)j);
diff --git a/tests/test-chains-issuer.h b/tests/test-chains-issuer.h
index 543e2d71f..bf1e65c95 100644
--- a/tests/test-chains-issuer.h
+++ b/tests/test-chains-issuer.h
@@ -24,7 +24,7 @@
#ifndef GNUTLS_TESTS_TEST_CHAINS_ISSUER_H
#define GNUTLS_TESTS_TEST_CHAINS_ISSUER_H
-#define MAX_CHAIN 6
+#define MAX_CHAIN 15
#define SERVER_CERT "-----BEGIN CERTIFICATE-----\n" \
"MIIDATCCAbmgAwIBAgIUQdvdegP8JFszFHLfV4+lrEdafzAwPQYJKoZIhvcNAQEK\n" \
@@ -338,11 +338,102 @@ static const char *missing_middle_unrelated_extra_insert[] = {
NULL,
};
+static const char *missing_middle_single_duplicate[] = {
+ SERVER_CERT,
+ SERVER_CERT,
+ CA_CERT_5,
+ CA_CERT_5,
+ CA_CERT_4,
+ CA_CERT_4,
+ CA_CERT_2,
+ CA_CERT_2,
+ CA_CERT_1,
+ CA_CERT_1,
+ NULL,
+};
+
+static const char *missing_middle_multiple_duplicate[] = {
+ SERVER_CERT,
+ SERVER_CERT,
+ CA_CERT_5,
+ CA_CERT_5,
+ CA_CERT_4,
+ CA_CERT_4,
+ CA_CERT_1,
+ CA_CERT_1,
+ NULL,
+};
+
+static const char *missing_last_single_duplicate[] = {
+ SERVER_CERT,
+ SERVER_CERT,
+ CA_CERT_5,
+ CA_CERT_5,
+ CA_CERT_4,
+ CA_CERT_4,
+ CA_CERT_3,
+ CA_CERT_3,
+ CA_CERT_2,
+ CA_CERT_2,
+ NULL,
+};
+
+static const char *missing_last_multiple_duplicate[] = {
+ SERVER_CERT,
+ SERVER_CERT,
+ CA_CERT_5,
+ CA_CERT_5,
+ CA_CERT_4,
+ CA_CERT_4,
+ CA_CERT_3,
+ CA_CERT_3,
+ NULL,
+};
+
+static const char *missing_skip_single_duplicate[] = {
+ SERVER_CERT,
+ SERVER_CERT,
+ CA_CERT_5,
+ CA_CERT_5,
+ CA_CERT_3,
+ CA_CERT_3,
+ CA_CERT_1,
+ CA_CERT_1,
+ NULL,
+};
+
+static const char *missing_skip_multiple_duplicate[] = {
+ SERVER_CERT,
+ SERVER_CERT,
+ CA_CERT_5,
+ CA_CERT_5,
+ CA_CERT_3,
+ CA_CERT_3,
+ NULL,
+};
+
static const char *missing_ca[] = {
CA_CERT_0,
NULL,
};
+static const char *middle_single_duplicate_ca[] = {
+ SERVER_CERT,
+ CA_CERT_5,
+ CA_CERT_0,
+ CA_CERT_4,
+ CA_CERT_0,
+ CA_CERT_2,
+ CA_CERT_0,
+ CA_CERT_1,
+ NULL,
+};
+
+static const char *missing_middle_single_duplicate_ca_unrelated_insert[] = {
+ CA_CERT_0,
+ NULL,
+};
+
static struct chains {
const char *name;
const char **chain;
@@ -377,6 +468,14 @@ static struct chains {
{ "skip multiple unsorted", missing_skip_multiple_unsorted, missing_skip_multiple_insert, missing_ca, 0, 0 },
{ "unrelated", missing_middle_single, missing_middle_unrelated_insert, missing_ca, 0, GNUTLS_CERT_INVALID | GNUTLS_CERT_SIGNER_NOT_FOUND },
{ "unrelated extra", missing_middle_single, missing_middle_unrelated_extra_insert, missing_ca, 0, 0 },
+ { "middle single duplicate", missing_middle_single_duplicate, missing_middle_single_insert, missing_ca, 0, 0 },
+ { "middle multiple duplicate", missing_middle_multiple_duplicate, missing_middle_multiple_insert, missing_ca, 0, 0 },
+ { "last single duplicate", missing_last_single_duplicate, missing_last_single_insert, missing_ca, 0, 0 },
+ { "last multiple duplicate", missing_last_multiple_duplicate, missing_last_multiple_insert, missing_ca, 0, 0 },
+ { "skip single duplicate", missing_skip_single_duplicate, missing_skip_single_insert, missing_ca, 0, 0 },
+ { "skip multiple duplicate", missing_skip_multiple_duplicate, missing_skip_multiple_insert, missing_ca, 0, 0 },
+ { "middle single duplicate ca", middle_single_duplicate_ca, missing_middle_single_insert, missing_ca, 0, 0 },
+ { "middle single duplicate ca - insert unrelated", middle_single_duplicate_ca, missing_middle_single_duplicate_ca_unrelated_insert, missing_ca, 0, GNUTLS_CERT_INVALID | GNUTLS_CERT_SIGNER_NOT_FOUND },
{ NULL, NULL, NULL, NULL },
};
--
2.29.2

6
gnutls.spec
View File

@ -1,10 +1,11 @@
# This spec file has been automatically updated
Version: 3.7.0
Release: 1%{?dist}
Release: 2%{?dist}
Patch1: gnutls-3.6.7-no-now-guile.patch
Patch2: gnutls-3.2.7-rpath.patch
Patch3: gnutls-3.7.0-test-fixes.patch
Patch4: gnutls-3.7.0-gost.patch
Patch5: gnutls-3.7.0-duplicate-certs.patch
%bcond_without dane
%if 0%{?rhel}
%bcond_with guile
@ -284,6 +285,9 @@ make check %{?_smp_mflags} GNUTLS_SYSTEM_PRIORITY_FILE=/dev/null
%endif
%changelog
* Wed Feb 10 2021 Daiki Ueno <dueno@redhat.com> - 3.7.0-2
- Tolerate duplicate certs in the chain
* Mon Feb 8 2021 Daiki Ueno <dueno@redhat.com> - 3.7.0-1
- Update to upstream 3.7.0 release
- Temporarily disable LTO