From fcdcba51f5e07a62b1f8d7c0a648cf1c7d29c639 Mon Sep 17 00:00:00 2001 From: devgianlu Date: Sun, 24 Nov 2024 21:39:56 +0100 Subject: [PATCH] LibTLS+LibWeb: Decouple EC parameters from `TLS::SupportedGroup` This is in preparation of the next commits to split the changes. --- Libraries/LibTLS/Certificate.cpp | 14 ++----------- Libraries/LibTLS/Certificate.h | 2 +- Libraries/LibTLS/HandshakeServer.cpp | 10 ++++++++-- Libraries/LibTLS/TLSv12.cpp | 21 ++++++++++++++++++-- Libraries/LibTLS/TLSv12.h | 2 ++ Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp | 8 ++++---- 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/Libraries/LibTLS/Certificate.cpp b/Libraries/LibTLS/Certificate.cpp index a3c82a93438..d80d8bbfc56 100644 --- a/Libraries/LibTLS/Certificate.cpp +++ b/Libraries/LibTLS/Certificate.cpp @@ -76,16 +76,6 @@ namespace TLS { } \ } while (0) -static ErrorOr oid_to_curve(Vector curve) -{ - if (curve == curve_ansip384r1) - return SupportedGroup::SECP384R1; - else if (curve == curve_prime256) - return SupportedGroup::SECP256R1; - - return Error::from_string_literal("Unknown curve oid"); -} - static ErrorOr parse_certificate_version(Crypto::ASN1::Decoder& decoder, Vector current_scope) { // Version ::= INTEGER {v1(0), v2(1), v3(2)} @@ -111,7 +101,7 @@ static ErrorOr parse_serial_number(Crypto::ASN1::Dec return serial; } -static ErrorOr parse_ec_parameters(Crypto::ASN1::Decoder& decoder, Vector current_scope) +static ErrorOr> parse_ec_parameters(Crypto::ASN1::Decoder& decoder, Vector current_scope) { // ECParameters ::= CHOICE { // namedCurve OBJECT IDENTIFIER @@ -136,7 +126,7 @@ static ErrorOr parse_ec_parameters(Crypto::ASN1::Decoder& decode ERROR_WITH_SCOPE(TRY(String::formatted("Unknown named curve {}", named_curve))); } - return oid_to_curve(named_curve); + return named_curve; } static ErrorOr parse_algorithm_identifier(Crypto::ASN1::Decoder& decoder, Vector current_scope) diff --git a/Libraries/LibTLS/Certificate.h b/Libraries/LibTLS/Certificate.h index fc96e1d6844..88945c6ebfb 100644 --- a/Libraries/LibTLS/Certificate.h +++ b/Libraries/LibTLS/Certificate.h @@ -188,7 +188,7 @@ struct AlgorithmIdentifier { } Vector identifier; - SupportedGroup ec_parameters {}; + Optional> ec_parameters; }; struct BasicConstraints { diff --git a/Libraries/LibTLS/HandshakeServer.cpp b/Libraries/LibTLS/HandshakeServer.cpp index 4f94b95f460..8593cd3e375 100644 --- a/Libraries/LibTLS/HandshakeServer.cpp +++ b/Libraries/LibTLS/HandshakeServer.cpp @@ -488,7 +488,13 @@ ssize_t TLSv12::verify_ecdsa_server_key_exchange(ReadonlyBytes server_key_info_b ErrorOr res = AK::Error::from_errno(ENOTSUP); auto& public_key = m_context.certificates.first().public_key; - switch (public_key.algorithm.ec_parameters) { + auto ec_curve = oid_to_curve(public_key.algorithm.ec_parameters.value_or({})); + if (ec_curve.is_error()) { + dbgln("verify_ecdsa_server_key_exchange failed: Unknown curve for ECDSA signature verification"); + return (i8)Error::NotUnderstood; + } + + switch (ec_curve.release_value()) { case SupportedGroup::SECP256R1: { Crypto::Hash::Manager manager(hash_kind); manager.update(message); @@ -508,7 +514,7 @@ ssize_t TLSv12::verify_ecdsa_server_key_exchange(ReadonlyBytes server_key_info_b break; } default: { - dbgln("verify_ecdsa_server_key_exchange failed: Server certificate public key algorithm is not supported: {}", to_underlying(public_key.algorithm.ec_parameters)); + dbgln("verify_ecdsa_server_key_exchange failed: Server certificate public key algorithm is not supported: {}", to_underlying(ec_curve.release_value())); break; } } diff --git a/Libraries/LibTLS/TLSv12.cpp b/Libraries/LibTLS/TLSv12.cpp index 97b3799d0fc..2863dd2ad77 100644 --- a/Libraries/LibTLS/TLSv12.cpp +++ b/Libraries/LibTLS/TLSv12.cpp @@ -364,7 +364,13 @@ bool Context::verify_certificate_pair(Certificate const& subject, Certificate co } // ECDSA hash verification: hash, then check signature against the specific curve - switch (issuer.public_key.algorithm.ec_parameters) { + auto ec_curve = oid_to_curve(issuer.public_key.algorithm.ec_parameters.value_or({})); + if (ec_curve.is_error()) { + dbgln("verify_certificate_pair: Unknown curve for ECDSA signature verification"); + return false; + } + + switch (ec_curve.release_value()) { case SupportedGroup::SECP256R1: { Crypto::Hash::Manager hasher(kind); hasher.update(subject.tbs_asn1.bytes()); @@ -401,7 +407,7 @@ bool Context::verify_certificate_pair(Certificate const& subject, Certificate co return result; } default: - dbgln("verify_certificate_pair: Don't know how to verify signature for curve {}", to_underlying(issuer.public_key.algorithm.ec_parameters)); + dbgln("verify_certificate_pair: Don't know how to verify signature for curve {}", to_underlying(ec_curve.release_value())); return false; } } @@ -588,4 +594,15 @@ ErrorOr> DefaultRootCACertificates::parse_pem_root_certifica return certificates; } + +ErrorOr oid_to_curve(Vector curve) +{ + if (curve == curve_ansip384r1) + return SupportedGroup::SECP384R1; + if (curve == curve_prime256) + return SupportedGroup::SECP256R1; + + return AK::Error::from_string_literal("Unknown curve oid"); +} + } diff --git a/Libraries/LibTLS/TLSv12.h b/Libraries/LibTLS/TLSv12.h index efc09c45637..3330743808c 100644 --- a/Libraries/LibTLS/TLSv12.h +++ b/Libraries/LibTLS/TLSv12.h @@ -139,6 +139,8 @@ constexpr CipherAlgorithm get_cipher_algorithm(CipherSuite suite) } } +ErrorOr oid_to_curve(Vector curve); + struct Options { static Vector default_usable_cipher_suites() { diff --git a/Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp b/Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp index 2b8766754fd..8d0963d1edf 100644 --- a/Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp +++ b/Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp @@ -2724,7 +2724,7 @@ WebIDL::ExceptionOr> ED25519::import_key( return WebIDL::DataError::create(m_realm, "Invalid algorithm identifier"_string); // 5. If the parameters field of the algorithm AlgorithmIdentifier field of spki is present, then throw a DataError. - if (static_cast(spki.algorithm.ec_parameters) != 0) + if (spki.algorithm.ec_parameters.has_value()) return WebIDL::DataError::create(m_realm, "Invalid algorithm parameters"_string); // 6. Let publicKey be the Ed25519 public key identified by the subjectPublicKey field of spki. @@ -2767,7 +2767,7 @@ WebIDL::ExceptionOr> ED25519::import_key( // 5. If the parameters field of the privateKeyAlgorithm PrivateKeyAlgorithmIdentifier field of privateKeyInfo is present, // then throw a DataError. - if (static_cast(private_key_info.algorithm.ec_parameters) != 0) + if (private_key_info.algorithm.ec_parameters.has_value()) return WebIDL::DataError::create(m_realm, "Invalid algorithm parameters"_string); // 6. Let curvePrivateKey be the result of performing the parse an ASN.1 structure algorithm, @@ -3417,7 +3417,7 @@ WebIDL::ExceptionOr> X25519::import_key([[maybe_unused]] Web: return WebIDL::DataError::create(m_realm, "Invalid algorithm"_string); // 5. If the parameters field of the algorithm AlgorithmIdentifier field of spki is present, then throw a DataError. - if (static_cast(spki.algorithm.ec_parameters) != 0) + if (spki.algorithm.ec_parameters.has_value()) return WebIDL::DataError::create(m_realm, "Invalid algorithm parameters"_string); // 6. Let publicKey be the X25519 public key identified by the subjectPublicKey field of spki. @@ -3458,7 +3458,7 @@ WebIDL::ExceptionOr> X25519::import_key([[maybe_unused]] Web: return WebIDL::DataError::create(m_realm, "Invalid algorithm"_string); // 5. If the parameters field of the privateKeyAlgorithm PrivateKeyAlgorithmIdentifier field of privateKeyInfo is present, then throw a DataError. - if (static_cast(private_key_info.algorithm.ec_parameters) != 0) + if (private_key_info.algorithm.ec_parameters.has_value()) return WebIDL::DataError::create(m_realm, "Invalid algorithm parameters"_string); // 6. Let curvePrivateKey be the result of performing the parse an ASN.1 structure algorithm,