From 898be38517286c5df4dd827bbad278246282e768 Mon Sep 17 00:00:00 2001 From: Michiel Visser Date: Fri, 18 Feb 2022 10:58:56 +0100 Subject: [PATCH] LibTLS: Add signature verification for DHE and ECDHE key exchange This will verify that the signature of the ephemeral key used in the DHE and ECDHE key exchanges is actually generated by the server. This verification is done using the first certificate provided by the server, however the validity of this certificate is not checked here. Instead this code expects the validity to be checked earlier by `TLSv12::handle_certificate`. --- .../Libraries/LibCrypto/Hash/HashManager.h | 5 ++ .../LibCrypto/PK/Code/EMSA_PKCS1_V1_5.h | 26 +++++- Userland/Libraries/LibTLS/Handshake.cpp | 5 ++ Userland/Libraries/LibTLS/HandshakeServer.cpp | 79 +++++++++++++++++-- Userland/Libraries/LibTLS/TLSv12.h | 2 + 5 files changed, 108 insertions(+), 9 deletions(-) diff --git a/Userland/Libraries/LibCrypto/Hash/HashManager.h b/Userland/Libraries/LibCrypto/Hash/HashManager.h index 28a9bf4e9e..10628bc52c 100644 --- a/Userland/Libraries/LibCrypto/Hash/HashManager.h +++ b/Userland/Libraries/LibCrypto/Hash/HashManager.h @@ -200,6 +200,11 @@ public: } #endif + inline HashKind kind() const + { + return m_kind; + } + inline bool is(HashKind kind) const { return m_kind == kind; diff --git a/Userland/Libraries/LibCrypto/PK/Code/EMSA_PKCS1_V1_5.h b/Userland/Libraries/LibCrypto/PK/Code/EMSA_PKCS1_V1_5.h index 987e5d56c3..1de4d73b99 100644 --- a/Userland/Libraries/LibCrypto/PK/Code/EMSA_PKCS1_V1_5.h +++ b/Userland/Libraries/LibCrypto/PK/Code/EMSA_PKCS1_V1_5.h @@ -6,10 +6,7 @@ #pragma once -#include -#include -#include -#include +#include #include #include #include @@ -116,5 +113,26 @@ inline ReadonlyBytes EMSA_PKCS1_V1_5::hash_function_digest return { "\x30\x51\x30\x0d\x06\x09\x60\x86\x48\x01\x65\x03\x04\x02\x03\x05\x00\x04\x40", 19 }; } +template<> +inline ReadonlyBytes EMSA_PKCS1_V1_5::hash_function_digest_info() +{ + // RFC8017 section 9.2 notes 1 + switch (hasher().kind()) { + case Hash::HashKind::MD5: + return { "\x30\x20\x30\x0c\x06\x08\x2a\x86\x48\x86\xf7\x0d\x02\x05\x05\x00\x04\x10", 18 }; + case Hash::HashKind::SHA1: + return { "\x30\x21\x30\x09\x06\x05\x2b\x0e\x03\x02\x1a\x05\x00\x04\x14", 15 }; + case Hash::HashKind::SHA256: + return { "\x30\x31\x30\x0d\x06\x09\x60\x86\x48\x01\x65\x03\x04\x02\x01\x05\x00\x04\x20", 19 }; + case Hash::HashKind::SHA384: + return { "\x30\x41\x30\x0d\x06\x09\x60\x86\x48\x01\x65\x03\x04\x02\x02\x05\x00\x04\x30", 19 }; + case Hash::HashKind::SHA512: + return { "\x30\x51\x30\x0d\x06\x09\x60\x86\x48\x01\x65\x03\x04\x02\x03\x05\x00\x04\x40", 19 }; + case Hash::HashKind::None: + default: + VERIFY_NOT_REACHED(); + } +} + } } diff --git a/Userland/Libraries/LibTLS/Handshake.cpp b/Userland/Libraries/LibTLS/Handshake.cpp index 51fab9ddee..6a7c160546 100644 --- a/Userland/Libraries/LibTLS/Handshake.cpp +++ b/Userland/Libraries/LibTLS/Handshake.cpp @@ -489,6 +489,11 @@ ssize_t TLSv12::handle_handshake_payload(ReadonlyBytes vbuffer) write_packet(packet); break; } + case Error::NotSafe: { + auto packet = build_alert(true, (u8)AlertDescription::DecryptError); + write_packet(packet); + break; + } case Error::NeedMoreData: // Ignore this, as it's not an "error" dbgln_if(TLS_DEBUG, "More data needed"); diff --git a/Userland/Libraries/LibTLS/HandshakeServer.cpp b/Userland/Libraries/LibTLS/HandshakeServer.cpp index e5106467d3..ffad3216b7 100644 --- a/Userland/Libraries/LibTLS/HandshakeServer.cpp +++ b/Userland/Libraries/LibTLS/HandshakeServer.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -282,9 +283,9 @@ ssize_t TLSv12::handle_dhe_rsa_server_key_exchange(ReadonlyBytes buffer) dbgln("dh_Ys: {:hex-dump}", dh_Ys); } - // FIXME: Validate signature of Diffie-Hellman parameters as defined in RFC 5246 section 7.4.3. - - return 0; + auto server_key_info = buffer.slice(3, 6 + dh_p_length + dh_g_length + dh_Ys_length); + auto signature = buffer.slice(9 + dh_p_length + dh_g_length + dh_Ys_length); + return verify_rsa_server_key_exchange(server_key_info, signature); } ssize_t TLSv12::handle_ecdhe_rsa_server_key_exchange(ReadonlyBytes buffer) @@ -317,9 +318,77 @@ ssize_t TLSv12::handle_ecdhe_rsa_server_key_exchange(ReadonlyBytes buffer) dbgln("ECDHE server public key: {:hex-dump}", server_public_key); } - // FIXME: Validate signature of Elliptic Curve Diffie-Hellman public key + auto server_key_info = buffer.slice(3, 4 + server_public_key_length); + auto signature = buffer.slice(7 + server_public_key_length); + return verify_rsa_server_key_exchange(server_key_info, signature); +} + +ssize_t TLSv12::verify_rsa_server_key_exchange(ReadonlyBytes server_key_info_buffer, ReadonlyBytes signature_buffer) +{ + auto signature_hash = signature_buffer[0]; + auto signature_algorithm = signature_buffer[1]; + if (signature_algorithm != (u8)SignatureAlgorithm::RSA) { + dbgln("verify_rsa_server_key_exchange failed: Signature algorithm is not RSA, instead {}", signature_algorithm); + return (i8)Error::NotUnderstood; + } + + auto signature_length = AK::convert_between_host_and_network_endian(ByteReader::load16(signature_buffer.offset_pointer(2))); + auto signature = signature_buffer.slice(4, signature_length); + + if (m_context.certificates.is_empty()) { + dbgln("verify_rsa_server_key_exchange failed: Attempting to verify signature without certificates"); + return (i8)Error::NotSafe; + } + auto certificate_public_key = m_context.certificates.first().public_key; + Crypto::PK::RSAPrivateKey dummy_private_key; + auto rsa = Crypto::PK::RSA(certificate_public_key, dummy_private_key); + + auto signature_verify_buffer_result = ByteBuffer::create_uninitialized(signature_length); + if (signature_verify_buffer_result.is_error()) { + dbgln("verify_rsa_server_key_exchange failed: Not enough memory"); + return (i8)Error::OutOfMemory; + } + auto signature_verify_buffer = signature_verify_buffer_result.release_value(); + auto signature_verify_bytes = signature_verify_buffer.bytes(); + rsa.verify(signature, signature_verify_bytes); + + auto message_result = ByteBuffer::create_uninitialized(64 + server_key_info_buffer.size()); + if (message_result.is_error()) { + dbgln("verify_rsa_server_key_exchange failed: Not enough memory"); + return (i8)Error::OutOfMemory; + } + auto message = message_result.release_value(); + message.overwrite(0, m_context.local_random, 32); + message.overwrite(32, m_context.remote_random, 32); + message.overwrite(64, server_key_info_buffer.data(), server_key_info_buffer.size()); + + Crypto::Hash::HashKind hash_kind; + switch ((HashAlgorithm)signature_hash) { + case HashAlgorithm::SHA1: + hash_kind = Crypto::Hash::HashKind::SHA1; + break; + case HashAlgorithm::SHA256: + hash_kind = Crypto::Hash::HashKind::SHA256; + break; + case HashAlgorithm::SHA384: + hash_kind = Crypto::Hash::HashKind::SHA384; + break; + case HashAlgorithm::SHA512: + hash_kind = Crypto::Hash::HashKind::SHA512; + break; + default: + dbgln("verify_rsa_server_key_exchange failed: Hash algorithm is not SHA1/256/384/512, instead {}", signature_hash); + return (i8)Error::NotUnderstood; + } + + auto pkcs1 = Crypto::PK::EMSA_PKCS1_V1_5(hash_kind); + auto verification = pkcs1.verify(message, signature_verify_bytes, signature_length * 8); + + if (verification == Crypto::VerificationConsistency::Inconsistent) { + dbgln("verify_rsa_server_key_exchange failed: Verification of signature inconsistent"); + return (i8)Error::NotSafe; + } return 0; } - } diff --git a/Userland/Libraries/LibTLS/TLSv12.h b/Userland/Libraries/LibTLS/TLSv12.h index 757b735742..850bd5c4fb 100644 --- a/Userland/Libraries/LibTLS/TLSv12.h +++ b/Userland/Libraries/LibTLS/TLSv12.h @@ -482,6 +482,8 @@ private: void pseudorandom_function(Bytes output, ReadonlyBytes secret, const u8* label, size_t label_length, ReadonlyBytes seed, ReadonlyBytes seed_b); + ssize_t verify_rsa_server_key_exchange(ReadonlyBytes server_key_info_buffer, ReadonlyBytes signature_buffer); + size_t key_length() const { switch (m_context.cipher) {