From 804af863b45d352fcc0dd9d76d701a7b3becc0a1 Mon Sep 17 00:00:00 2001 From: Michiel Visser Date: Wed, 23 Feb 2022 17:35:05 +0100 Subject: [PATCH] LibCrypto+LibTLS: Implement Key Usage and Basic Constraints extensions Root and intermediate CA certificates should have these extensions set to indicate that they are allowed to sign other certificates. The values reported in these extensions is now also checked by `verify_chain` to make sure no non-CA certificates are used to sign another certificate. The certificate parser now also aborts when a critical extension is detected which is unsupported, as is required by the specification. --- Userland/Libraries/LibTLS/Certificate.cpp | 35 ++++++++++++++++++++++- Userland/Libraries/LibTLS/Certificate.h | 3 ++ Userland/Libraries/LibTLS/TLSv12.cpp | 9 ++++++ 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/Userland/Libraries/LibTLS/Certificate.cpp b/Userland/Libraries/LibTLS/Certificate.cpp index 1be8e1e999..af85e7f153 100644 --- a/Userland/Libraries/LibTLS/Certificate.cpp +++ b/Userland/Libraries/LibTLS/Certificate.cpp @@ -28,7 +28,9 @@ constexpr static Array rsa_sha512_encryption_oid { 1, 2, 840, 113549, 1, 1, 13 }; constexpr static Array - subject_alternative_name_oid { 2, 5, 29, 17 }; + key_usage_oid { 2, 5, 29, 15 }, + subject_alternative_name_oid { 2, 5, 29, 17 }, + basic_constraints_oid { 2, 5, 29, 19 }; Optional Certificate::parse_asn1(ReadonlyBytes buffer, bool) { @@ -457,6 +459,37 @@ Optional Certificate::parse_asn1(ReadonlyBytes buffer, bool) DROP_OBJECT_OR_FAIL("Certificate::TBSCertificate::Extensions::$::Extension::extension_value::SubjectAlternativeName::$::???"); } } + } else if (extension_id == key_usage_oid) { + // RFC5280 section 4.2.1.3: The keyCertSign bit is asserted when the subject public key is used + // for verifying signatures on public key certificates. If the keyCertSign bit is asserted, + // then the cA bit in the basic constraints extension MUST also be asserted. + Crypto::ASN1::Decoder decoder { extension_value.bytes() }; + READ_OBJECT_OR_FAIL(BitString, Crypto::ASN1::BitStringView, usage, "Certificate::TBSCertificate::Extensions::$::Extension::extension_value::KeyUsage"); + + // keyCertSign (5) + certificate.is_allowed_to_sign_certificate = usage.get(5); + } else if (extension_id == basic_constraints_oid) { + // RFC5280 section 4.2.1.9: The cA boolean indicates whether the certified public key may be + // used to verify certificate signatures. If the cA boolean is not asserted, then the keyCertSign + // bit in the key usage extension MUST NOT be asserted. If the basic constraints extension is + // not present in a version 3 certificate, or the extension is present but the cA boolean is + // not asserted, then the certified public key MUST NOT be used to verify certificate signatures. + Crypto::ASN1::Decoder decoder { extension_value.bytes() }; + ENTER_SCOPE_OR_FAIL(Sequence, "Certificate::TBSCertificate::Extensions::$::Extension::extension_value::BasicConstraints"); + + if (auto tag = decoder.peek(); !tag.is_error() && tag.value().kind == Crypto::ASN1::Kind::Boolean) { + READ_OBJECT_OR_FAIL(Boolean, bool, is_certificate_authority, "Certificate::TBSCertificate::Extensions::$::Extension::extension_value::BasicConstraints::cA"); + certificate.is_certificate_authority = is_certificate_authority; + + if (auto tag = decoder.peek(); !tag.is_error() && tag.value().kind == Crypto::ASN1::Kind::Integer) { + READ_OBJECT_OR_FAIL(Integer, Crypto::UnsignedBigInteger, path_length_constraint, "Certificate::TBSCertificate::Extensions::$::Extension::extension_value::BasicConstraints::pathLenConstraint"); + certificate.path_length_constraint = path_length_constraint.to_u64(); + } + } + } else { + dbgln_if(TLS_DEBUG, "Certificate::TBSCertificate::Extensions::$::Extension::extension_id: unknown extension {} (critical: {})", extension_id, is_critical); + if (is_critical) + return {}; } EXIT_SCOPE("Certificate::TBSCertificate::Extensions::$::Extension"); diff --git a/Userland/Libraries/LibTLS/Certificate.h b/Userland/Libraries/LibTLS/Certificate.h index aef856406a..416da5745b 100644 --- a/Userland/Libraries/LibTLS/Certificate.h +++ b/Userland/Libraries/LibTLS/Certificate.h @@ -56,6 +56,9 @@ public: CertificateKeyAlgorithm signature_algorithm { CertificateKeyAlgorithm::Unsupported }; ByteBuffer signature_value {}; ByteBuffer original_asn1 {}; + bool is_allowed_to_sign_certificate { false }; + bool is_certificate_authority { false }; + Optional path_length_constraint {}; static Optional parse_asn1(ReadonlyBytes, bool client_cert = false); diff --git a/Userland/Libraries/LibTLS/TLSv12.cpp b/Userland/Libraries/LibTLS/TLSv12.cpp index 88af0f2635..bfd4749745 100644 --- a/Userland/Libraries/LibTLS/TLSv12.cpp +++ b/Userland/Libraries/LibTLS/TLSv12.cpp @@ -297,6 +297,15 @@ bool Context::verify_chain(StringView host) const return false; } + if (!(parent_certificate.is_allowed_to_sign_certificate && parent_certificate.is_certificate_authority)) { + dbgln("verify_chain: {} is not marked as certificate authority", issuer_string); + return false; + } + if (parent_certificate.path_length_constraint.has_value() && cert_index > parent_certificate.path_length_constraint.value()) { + dbgln("verify_chain: Path length for certificate exceeded"); + return false; + } + bool verification_correct = verify_certificate_pair(cert, parent_certificate); if (!verification_correct) { dbgln("verify_chain: Signature inconsistent, {} was not signed by {}", subject_string, issuer_string);