diff --git a/Userland/Libraries/LibTLS/HandshakeClient.cpp b/Userland/Libraries/LibTLS/HandshakeClient.cpp index 52ec095677..cc4e0271b0 100644 --- a/Userland/Libraries/LibTLS/HandshakeClient.cpp +++ b/Userland/Libraries/LibTLS/HandshakeClient.cpp @@ -152,38 +152,6 @@ bool TLSv12::compute_master_secret_from_pre_master_secret(size_t length) return true; } -static bool wildcard_matches(StringView host, StringView subject) -{ - if (host.matches(subject)) - return true; - - if (subject.starts_with("*.")) - return wildcard_matches(host, subject.substring_view(2)); - - return false; -} - -Optional TLSv12::verify_chain_and_get_matching_certificate(StringView host) const -{ - if (m_context.certificates.is_empty() || !m_context.verify_chain()) - return {}; - - if (host.is_empty()) - return 0; - - for (size_t i = 0; i < m_context.certificates.size(); ++i) { - auto& cert = m_context.certificates[i]; - if (wildcard_matches(host, cert.subject.subject)) - return i; - for (auto& san : cert.SAN) { - if (wildcard_matches(host, san)) - return i; - } - } - - return {}; -} - void TLSv12::build_rsa_pre_master_secret(PacketBuilder& builder) { u8 random_bytes[48]; @@ -211,14 +179,7 @@ void TLSv12::build_rsa_pre_master_secret(PacketBuilder& builder) } m_context.premaster_key = premaster_key_result.release_value(); - auto const& certificate_option = verify_chain_and_get_matching_certificate(m_context.extensions.SNI); // if the SNI is empty, we'll make a special case and match *a* leaf certificate. - if (!certificate_option.has_value()) { - dbgln("certificate verification failed :("); - alert(AlertLevel::Critical, AlertDescription::BadCertificate); - return; - } - - auto& certificate = m_context.certificates[certificate_option.value()]; + auto& certificate = m_context.certificates.first(); if constexpr (TLS_DEBUG) { dbgln("PreMaster secret"); print_buffer(m_context.premaster_key); @@ -248,13 +209,6 @@ void TLSv12::build_rsa_pre_master_secret(PacketBuilder& builder) void TLSv12::build_dhe_rsa_pre_master_secret(PacketBuilder& builder) { - const auto& certificate_option = verify_chain_and_get_matching_certificate(m_context.extensions.SNI); // if the SNI is empty, we'll make a special case and match *a* leaf certificate. - if (!certificate_option.has_value()) { - dbgln("certificate verification failed :("); - alert(AlertLevel::Critical, AlertDescription::BadCertificate); - return; - } - auto& dh = m_context.server_diffie_hellman_params; auto dh_p = Crypto::UnsignedBigInteger::import_data(dh.p.data(), dh.p.size()); auto dh_g = Crypto::UnsignedBigInteger::import_data(dh.g.data(), dh.g.size()); @@ -302,13 +256,6 @@ void TLSv12::build_dhe_rsa_pre_master_secret(PacketBuilder& builder) void TLSv12::build_ecdhe_rsa_pre_master_secret(PacketBuilder& builder) { - const auto& certificate_option = verify_chain_and_get_matching_certificate(m_context.extensions.SNI); // if the SNI is empty, we'll make a special case and match *a* leaf certificate. - if (!certificate_option.has_value()) { - dbgln("certificate verification failed :("); - alert(AlertLevel::Critical, AlertDescription::BadCertificate); - return; - } - // Create a random private key auto private_key_result = m_context.server_key_exchange_curve->generate_private_key(); if (private_key_result.is_error()) { @@ -414,6 +361,13 @@ ByteBuffer TLSv12::build_certificate() ByteBuffer TLSv12::build_client_key_exchange() { + bool chain_verified = m_context.verify_chain(m_context.extensions.SNI); + if (!chain_verified) { + dbgln("certificate verification failed :("); + alert(AlertLevel::Critical, AlertDescription::BadCertificate); + return {}; + } + PacketBuilder builder { MessageType::Handshake, m_context.options.version }; builder.append((u8)HandshakeType::ClientKeyExchange); diff --git a/Userland/Libraries/LibTLS/TLSv12.cpp b/Userland/Libraries/LibTLS/TLSv12.cpp index 572c778e52..ab95df9e1c 100644 --- a/Userland/Libraries/LibTLS/TLSv12.cpp +++ b/Userland/Libraries/LibTLS/TLSv12.cpp @@ -191,7 +191,31 @@ void TLSv12::set_root_certificates(Vector certificates) dbgln_if(TLS_DEBUG, "{}: Set {} root certificates", this, m_context.root_certificates.size()); } -bool Context::verify_chain() const +static bool wildcard_matches(StringView host, StringView subject) +{ + if (host.matches(subject)) + return true; + + if (subject.starts_with("*.")) + return wildcard_matches(host, subject.substring_view(2)); + + return false; +} + +static bool certificate_subject_matches_host(Certificate& cert, StringView host) +{ + if (wildcard_matches(host, cert.subject.subject)) + return true; + + for (auto& san : cert.SAN) { + if (wildcard_matches(host, san)) + return true; + } + + return false; +} + +bool Context::verify_chain(StringView host) const { if (!options.validate_certificates) return true; @@ -209,6 +233,19 @@ bool Context::verify_chain() const return false; } + if (!host.is_empty()) { + auto first_certificate = local_chain->first(); + auto subject_matches = certificate_subject_matches_host(first_certificate, host); + if (!subject_matches) { + dbgln("verify_chain: First certificate does not match the hostname"); + return false; + } + } else { + // FIXME: The host is taken from m_context.extensions.SNI, when is this empty? + dbgln("FIXME: verify_chain called without host"); + return false; + } + for (size_t cert_index = 0; cert_index < local_chain->size(); ++cert_index) { auto cert = local_chain->at(cert_index); diff --git a/Userland/Libraries/LibTLS/TLSv12.h b/Userland/Libraries/LibTLS/TLSv12.h index 20a8478388..8f6bef402b 100644 --- a/Userland/Libraries/LibTLS/TLSv12.h +++ b/Userland/Libraries/LibTLS/TLSv12.h @@ -261,7 +261,7 @@ struct Options { }; struct Context { - bool verify_chain() const; + bool verify_chain(StringView host) const; bool verify_certificate_pair(Certificate& subject, Certificate& issuer) const; Options options; @@ -561,8 +561,6 @@ private: bool compute_master_secret_from_pre_master_secret(size_t length); - Optional verify_chain_and_get_matching_certificate(StringView host) const; - void try_disambiguate_error() const; bool m_eof { false };