Improve certificate sanity checks

Signed-off-by: Zoltan Fridrich <zfridric@redhat.com>

diff --git a/NEWS b/NEWS
index 8727a89b28e31031679803aea2c216c27624b99b..9b7986c68808e60b7eeddc389986734b306bc19b 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,16 @@ See the end for copying conditions.
    has been changed from tls-unique to tls-exporter as tls-unique is
    not supported in TLS 1.3.
 
+** libgnutls: Certificate sanity checks has been enhanced to make
+   gnutls more RFC 5280 compliant (!1583).
+   Following changes were included:
+   - critical extensions are parsed when loading x509
+     certificate to prohibit any random octet strings.
+     Requires strict-x509 configure option to be enabled
+   - garbage bits in Key Usage extension are prohibited
+   - empty DirectoryStrings in Distinguished name structures
+     of Issuer and Subject name are prohibited
+
 ** libgnutls: Removed 3DES from FIPS approved algorithms (#1353).
    According to the section 2 of SP800-131A Rev.2, 3DES algorithm
    will be disallowed for encryption after December 31, 2023:

diff --git a/lib/x509/common.c b/lib/x509/common.c
index fad9da51fb12ec1cf4baa32b01662ba252257f29..ca0b71cb69efd15f86ff59d70271569514b505e9 100644 (file)
--- a/lib/x509/common.c
+++ b/lib/x509/common.c
@@ -150,6 +150,12 @@ const struct oid_to_string *_gnutls_oid_get_entry(const struct oid_to_string *ot
        return NULL;
 }
 
+const char *_gnutls_oid_get_asn_desc(const char *oid)
+{
+       const struct oid_to_string *entry = _gnutls_oid_get_entry(_oid2str, oid);
+       return entry ? entry->asn_desc : NULL;
+}
+
 const char *_gnutls_ldap_string_to_oid(const char *str, unsigned str_len)
 {
        unsigned int i = 0;

diff --git a/lib/x509/common.h b/lib/x509/common.h
index 19d29ddca59efbc6e5d7e199691c2c67cc88c2e8..457d4c4efb917a0348d8386704a9af333bdfcc19 100644 (file)
--- a/lib/x509/common.h
+++ b/lib/x509/common.h
@@ -130,6 +130,8 @@ struct oid_to_string {
 
 const struct oid_to_string *_gnutls_oid_get_entry(const struct oid_to_string *ots, const char *oid);
 
+const char *_gnutls_oid_get_asn_desc(const char *oid);
+
 int _gnutls_x509_set_time(asn1_node c2, const char *where, time_t tim,
                          int force_general);
 int

diff --git a/lib/x509/dn.c b/lib/x509/dn.c
index 0a703a84641013d4e12220faf8124320bd200734..8d89f56259635962db51b7b2fbb12a7bfb054cbb 100644 (file)
--- a/lib/x509/dn.c
+++ b/lib/x509/dn.c
@@ -156,6 +156,20 @@ static int append_elements(asn1_node asn1_struct, const char *asn1_rdn_name, gnu
                STR_APPEND(ldap_desc);
                STR_APPEND("=");
 
+               /* DirectoryString by definition in RFC 5280 cannot be empty.
+                * If asn_node.value_len = 0 the parser correctly rejects such DirectoryString.
+                * However, if asn_node.value contains ASN.1 TLV triplet with length = 0,
+                * such DirectoryString is not rejected by the parser as the node itself is not empty.
+                * Explicitly reject DirectoryString in such case.
+                */
+               const char *asn_desc = _gnutls_oid_get_asn_desc(oid);
+               if (asn_desc && !strcmp(asn_desc, "PKIX1.DirectoryString") && tvd.data[1] == 0) {
+                       gnutls_assert();
+                       result = GNUTLS_E_ASN1_VALUE_NOT_VALID;
+                       _gnutls_debug_log("Empty DirectoryString\n");
+                       goto cleanup;
+               }
+
                result =
                    _gnutls_x509_dn_to_string(oid, tvd.data,
                                              tvd.size, &td);

diff --git a/lib/x509/x509.c b/lib/x509/x509.c
index 490e8bdba07a5e1879698eeee6e9df4e1a1d08b3..50dcc8e6508688a52f761b6c8c66e36920028425 100644 (file)
--- a/lib/x509/x509.c
+++ b/lib/x509/x509.c
@@ -463,6 +463,94 @@ static bool has_valid_serial(gnutls_x509_crt_t cert)
        return true;
 }
 
+/* Check if extension can be successfully parsed */
+static bool is_valid_extension(const char *oid, gnutls_datum_t *der)
+{
+       int err = 0, i;
+       unsigned u;
+       size_t sz;
+       time_t t1, t2;
+       char *s1 = NULL, *s2 = NULL;
+       gnutls_datum_t datum = {NULL, 0};
+
+       if (!strcmp(oid, GNUTLS_X509EXT_OID_BASIC_CONSTRAINTS)) {
+               err = gnutls_x509_ext_import_basic_constraints(der, &u, &i);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_SUBJECT_KEY_ID)) {
+               err = gnutls_x509_ext_import_subject_key_id(der, &datum);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_CRT_POLICY)) {
+               gnutls_x509_policies_t policies;
+               if (gnutls_x509_policies_init(&policies) < 0)
+                       return false;
+               err = gnutls_x509_ext_import_policies(der, policies, 0);
+               gnutls_x509_policies_deinit(policies);
+       } else if (!strcmp(oid, GNUTLS_X509_OID_POLICY_ANY)) {
+               err = gnutls_x509_ext_import_inhibit_anypolicy(der, &u);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_AUTHORITY_KEY_ID)) {
+               gnutls_x509_aki_t aki;
+               if (gnutls_x509_aki_init(&aki) < 0)
+                       return false;
+               err = gnutls_x509_ext_import_authority_key_id(der, aki, 0);
+               gnutls_x509_aki_deinit(aki);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_KEY_USAGE)) {
+               err = gnutls_x509_ext_import_key_usage(der, &u);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_PRIVATE_KEY_USAGE_PERIOD)) {
+               err = gnutls_x509_ext_import_private_key_usage_period(der, &t1, &t2);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_EXTENDED_KEY_USAGE)) {
+               gnutls_x509_key_purposes_t purposes;
+               if (gnutls_x509_key_purpose_init(&purposes) < 0)
+                       return false;
+               err = gnutls_x509_ext_import_key_purposes(der, purposes, 0);
+               gnutls_x509_key_purpose_deinit(purposes);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_SAN) ||
+                  !strcmp(oid, GNUTLS_X509EXT_OID_IAN)) {
+               gnutls_subject_alt_names_t names;
+               if (gnutls_subject_alt_names_init(&names) < 0)
+                       return false;
+               err = gnutls_x509_ext_import_subject_alt_names(der, names, 0);
+               gnutls_subject_alt_names_deinit(names);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_CRL_DIST_POINTS)) {
+               gnutls_x509_crl_dist_points_t dp;
+               if (gnutls_x509_crl_dist_points_init(&dp) < 0)
+                       return false;
+               err = gnutls_x509_ext_import_crl_dist_points(der, dp, 0);
+               gnutls_x509_crl_dist_points_deinit(dp);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_PROXY_CRT_INFO)) {
+               err = gnutls_x509_ext_import_proxy(der, &i, &s1, &s2, &sz);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_AUTHORITY_INFO_ACCESS)) {
+               gnutls_x509_aia_t aia;
+               if (gnutls_x509_aia_init(&aia) < 0)
+                       return false;
+               err = gnutls_x509_ext_import_aia(der, aia, 0);
+               gnutls_x509_aia_deinit(aia);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_CT_SCT_V1)) {
+               gnutls_x509_ct_scts_t scts;
+               if (gnutls_x509_ext_ct_scts_init(&scts) < 0)
+                       return false;
+               err = gnutls_x509_ext_ct_import_scts(der, scts, 0);
+               gnutls_x509_ext_ct_scts_deinit(scts);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_NAME_CONSTRAINTS)) {
+               gnutls_x509_name_constraints_t nc;
+               if (gnutls_x509_name_constraints_init(&nc) < 0)
+                       return false;
+               err = gnutls_x509_ext_import_name_constraints(der, nc, 0);
+               gnutls_x509_name_constraints_deinit(nc);
+       } else if (!strcmp(oid, GNUTLS_X509EXT_OID_TLSFEATURES)) {
+               gnutls_x509_tlsfeatures_t features;
+               if (gnutls_x509_tlsfeatures_init(&features) < 0)
+                       return false;
+               err = gnutls_x509_ext_import_tlsfeatures(der, features, 0);
+               gnutls_x509_tlsfeatures_deinit(features);
+       } else {
+               return true;
+       }
+
+       gnutls_free(s1);
+       gnutls_free(s2);
+       _gnutls_free_datum(&datum);
+
+       return err == 0;
+}
+
 #endif /* STRICT_X509 */
 
 int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
@@ -506,7 +594,7 @@ int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
                }
        } else {
                /* Version is 3; ensure no duplicate extensions are present. */
-               unsigned i;
+               unsigned i, critical;
                char oid[MAX_OID_SIZE];
                size_t oid_size;
                char *o;
@@ -517,7 +605,7 @@ int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
 
                for (i=0;;i++) {
                        oid_size = sizeof(oid);
-                       ret = gnutls_x509_crt_get_extension_info(cert, i, oid, &oid_size, NULL);
+                       ret = gnutls_x509_crt_get_extension_info(cert, i, oid, &oid_size, &critical);
                        if (ret < 0) {
                                if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE)
                                        break;
@@ -542,6 +630,19 @@ int _gnutls_check_cert_sanity(gnutls_x509_crt_t cert)
                                ret = gnutls_assert_val(GNUTLS_E_X509_DUPLICATE_EXTENSION);
                                goto cleanup;
                        }
+
+#ifdef STRICT_X509
+                       gnutls_datum_t der = { NULL, 0 };
+                       ret = gnutls_x509_crt_get_extension_data2(cert, i, &der);
+                       if (ret < 0)
+                               continue;
+                       if (critical && !is_valid_extension(oid, &der)) {
+                               _gnutls_free_datum(&der);
+                               _gnutls_debug_log("error: could not parse extension (%s)\n");
+                               return gnutls_assert_val(GNUTLS_E_X509_CERTIFICATE_ERROR);
+                       }
+                       _gnutls_free_datum(&der);
+#endif
                }
 
                hash_free(htable);

diff --git a/lib/x509/x509_ext.c b/lib/x509/x509_ext.c
index 8bcf183a2f846387fd9100ee61c328d7436c83c9..0b4c6a0134d4e663c89df7b23f0b37c14af02efd 100644 (file)
--- a/lib/x509/x509_ext.c
+++ b/lib/x509/x509_ext.c
@@ -1120,7 +1120,7 @@ int gnutls_x509_ext_import_key_usage(const gnutls_datum_t * ext,
        if (result != ASN1_SUCCESS) {
                gnutls_assert();
                asn1_delete_structure(&c2);
-               return 0;
+               return _gnutls_asn2err(result);
        }
 
        *key_usage = str[0] | (str[1] << 8);