From c1b041e761b99679a3998dbd81821febdecd2df9 Mon Sep 17 00:00:00 2001 From: Michiel Visser Date: Sat, 5 Mar 2022 18:13:39 +0100 Subject: [PATCH] LibCrypto+LibTLS: Generalize the elliptic curve interface These changes generalize the interface with an elliptic curve implementation. This allows LibTLS to support elliptic curves generally without needing the specifics of elliptic curve implementations. This should allow for easier addition of other elliptic curves. --- Tests/LibCrypto/TestCurves.cpp | 39 +++------ .../LibCrypto/Curves/EllipticCurve.h | 24 ++++++ .../Libraries/LibCrypto/Curves/X25519.cpp | 22 +++++ Userland/Libraries/LibCrypto/Curves/X25519.h | 9 ++- Userland/Libraries/LibCrypto/Curves/X448.cpp | 22 +++++ Userland/Libraries/LibCrypto/Curves/X448.h | 9 ++- Userland/Libraries/LibTLS/CipherSuite.h | 17 ---- Userland/Libraries/LibTLS/HandshakeClient.cpp | 80 +++++-------------- Userland/Libraries/LibTLS/HandshakeServer.cpp | 27 +++++-- Userland/Libraries/LibTLS/TLSv12.h | 6 +- 10 files changed, 133 insertions(+), 122 deletions(-) create mode 100644 Userland/Libraries/LibCrypto/Curves/EllipticCurve.h diff --git a/Tests/LibCrypto/TestCurves.cpp b/Tests/LibCrypto/TestCurves.cpp index 91189803d0..c7a40c351c 100644 --- a/Tests/LibCrypto/TestCurves.cpp +++ b/Tests/LibCrypto/TestCurves.cpp @@ -48,30 +48,24 @@ TEST_CASE(test_x25519) 0x76, 0xf0, 0x9b, 0x3c, 0x1e, 0x16, 0x17, 0x42 }; - u8 coordinate_data[32] { - 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 - }; - - ReadonlyBytes coordinate { coordinate_data, 32 }; ReadonlyBytes alice_public_key { alice_public_key_data, 32 }; ReadonlyBytes alice_private_key { alice_private_key_data, 32 }; ReadonlyBytes bob_public_key { bob_public_key_data, 32 }; ReadonlyBytes bob_private_key { bob_private_key_data, 32 }; ReadonlyBytes shared_secret { shared_secret_data, 32 }; - auto generated_alice_public = MUST(Crypto::Curves::X25519::compute_coordinate(alice_private_key, coordinate)); + Crypto::Curves::X25519 curve; + + auto generated_alice_public = MUST(curve.generate_public_key(alice_private_key)); EXPECT_EQ(alice_public_key, generated_alice_public); - auto generated_bob_public = MUST(Crypto::Curves::X25519::compute_coordinate(bob_private_key, coordinate)); + auto generated_bob_public = MUST(curve.generate_public_key(bob_private_key)); EXPECT_EQ(bob_public_key, generated_bob_public); - auto shared_alice = MUST(Crypto::Curves::X25519::compute_coordinate(alice_private_key, bob_public_key)); + auto shared_alice = MUST(curve.compute_coordinate(alice_private_key, bob_public_key)); EXPECT_EQ(shared_alice, shared_secret); - auto shared_bob = MUST(Crypto::Curves::X25519::compute_coordinate(bob_private_key, alice_public_key)); + auto shared_bob = MUST(curve.compute_coordinate(bob_private_key, alice_public_key)); EXPECT_EQ(shared_bob, shared_secret); EXPECT_EQ(shared_alice, shared_bob); @@ -130,33 +124,24 @@ TEST_CASE(test_x448) 0x44, 0x9a, 0x50, 0x37, 0x51, 0x4a, 0x87, 0x9d }; - u8 coordinate_data[56] { - 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 - }; - - ReadonlyBytes coordinate { coordinate_data, 56 }; ReadonlyBytes alice_public_key { alice_public_key_data, 56 }; ReadonlyBytes alice_private_key { alice_private_key_data, 56 }; ReadonlyBytes bob_public_key { bob_public_key_data, 56 }; ReadonlyBytes bob_private_key { bob_private_key_data, 56 }; ReadonlyBytes shared_secret { shared_secret_data, 56 }; - auto generated_alice_public = MUST(Crypto::Curves::X448::compute_coordinate(alice_private_key, coordinate)); + Crypto::Curves::X448 curve; + + auto generated_alice_public = MUST(curve.generate_public_key(alice_private_key)); EXPECT_EQ(alice_public_key, generated_alice_public); - auto generated_bob_public = MUST(Crypto::Curves::X448::compute_coordinate(bob_private_key, coordinate)); + auto generated_bob_public = MUST(curve.generate_public_key(bob_private_key)); EXPECT_EQ(bob_public_key, generated_bob_public); - auto shared_alice = MUST(Crypto::Curves::X448::compute_coordinate(alice_private_key, bob_public_key)); + auto shared_alice = MUST(curve.compute_coordinate(alice_private_key, bob_public_key)); EXPECT_EQ(shared_alice, shared_secret); - auto shared_bob = MUST(Crypto::Curves::X448::compute_coordinate(bob_private_key, alice_public_key)); + auto shared_bob = MUST(curve.compute_coordinate(bob_private_key, alice_public_key)); EXPECT_EQ(shared_bob, shared_secret); EXPECT_EQ(shared_alice, shared_bob); diff --git a/Userland/Libraries/LibCrypto/Curves/EllipticCurve.h b/Userland/Libraries/LibCrypto/Curves/EllipticCurve.h new file mode 100644 index 0000000000..ceb756c410 --- /dev/null +++ b/Userland/Libraries/LibCrypto/Curves/EllipticCurve.h @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2022, Michiel Visser + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace Crypto::Curves { + +class EllipticCurve { +public: + virtual size_t key_size() = 0; + virtual ErrorOr generate_private_key() = 0; + virtual ErrorOr generate_public_key(ReadonlyBytes a) = 0; + virtual ErrorOr compute_coordinate(ReadonlyBytes scalar_bytes, ReadonlyBytes point_bytes) = 0; + virtual ErrorOr derive_premaster_key(ReadonlyBytes shared_point) = 0; + + virtual ~EllipticCurve() = default; +}; + +} diff --git a/Userland/Libraries/LibCrypto/Curves/X25519.cpp b/Userland/Libraries/LibCrypto/Curves/X25519.cpp index 60240952f4..f4c0374dfd 100644 --- a/Userland/Libraries/LibCrypto/Curves/X25519.cpp +++ b/Userland/Libraries/LibCrypto/Curves/X25519.cpp @@ -6,6 +6,7 @@ #include #include +#include #include namespace Crypto::Curves { @@ -259,6 +260,19 @@ void X25519::modular_multiply_inverse(u32* state, u32* value) modular_multiply(state, u, value); } +ErrorOr X25519::generate_private_key() +{ + auto buffer = TRY(ByteBuffer::create_uninitialized(BYTES)); + fill_with_random(buffer.data(), buffer.size()); + return buffer; +} + +ErrorOr X25519::generate_public_key(ReadonlyBytes a) +{ + u8 generator[BYTES] { 9 }; + return compute_coordinate(a, { generator, BYTES }); +} + // https://datatracker.ietf.org/doc/html/rfc7748#section-5 ErrorOr X25519::compute_coordinate(ReadonlyBytes input_k, ReadonlyBytes input_u) { @@ -334,4 +348,12 @@ ErrorOr X25519::compute_coordinate(ReadonlyBytes input_k, ReadonlyBy // Encode state for export return export_state(u); } + +ErrorOr X25519::derive_premaster_key(ReadonlyBytes shared_point) +{ + VERIFY(shared_point.size() == BYTES); + ByteBuffer premaster_key = TRY(ByteBuffer::copy(shared_point)); + return premaster_key; +} + } diff --git a/Userland/Libraries/LibCrypto/Curves/X25519.h b/Userland/Libraries/LibCrypto/Curves/X25519.h index b0342211d2..1950f48531 100644 --- a/Userland/Libraries/LibCrypto/Curves/X25519.h +++ b/Userland/Libraries/LibCrypto/Curves/X25519.h @@ -7,10 +7,11 @@ #pragma once #include +#include namespace Crypto::Curves { -class X25519 { +class X25519 : public EllipticCurve { static constexpr u8 BITS = 255; static constexpr u8 BYTES = 32; @@ -18,7 +19,11 @@ class X25519 { static constexpr u32 A24 = 121666; public: - static ErrorOr compute_coordinate(ReadonlyBytes a, ReadonlyBytes b); + size_t key_size() override { return BYTES; } + ErrorOr generate_private_key() override; + ErrorOr generate_public_key(ReadonlyBytes a) override; + ErrorOr compute_coordinate(ReadonlyBytes a, ReadonlyBytes b) override; + ErrorOr derive_premaster_key(ReadonlyBytes shared_point) override; private: static void import_state(u32* state, ReadonlyBytes data); diff --git a/Userland/Libraries/LibCrypto/Curves/X448.cpp b/Userland/Libraries/LibCrypto/Curves/X448.cpp index a590141bfa..d648ab6463 100644 --- a/Userland/Libraries/LibCrypto/Curves/X448.cpp +++ b/Userland/Libraries/LibCrypto/Curves/X448.cpp @@ -6,6 +6,7 @@ #include #include +#include #include namespace Crypto::Curves { @@ -282,6 +283,19 @@ void X448::modular_multiply_inverse(u32* state, u32* value) modular_multiply(state, u, value); } +ErrorOr X448::generate_private_key() +{ + auto buffer = TRY(ByteBuffer::create_uninitialized(BYTES)); + fill_with_random(buffer.data(), buffer.size()); + return buffer; +} + +ErrorOr X448::generate_public_key(ReadonlyBytes a) +{ + u8 generator[BYTES] { 5 }; + return compute_coordinate(a, { generator, BYTES }); +} + // https://datatracker.ietf.org/doc/html/rfc7748#section-5 ErrorOr X448::compute_coordinate(ReadonlyBytes input_k, ReadonlyBytes input_u) { @@ -353,4 +367,12 @@ ErrorOr X448::compute_coordinate(ReadonlyBytes input_k, ReadonlyByte // Encode state for export return export_state(u); } + +ErrorOr X448::derive_premaster_key(ReadonlyBytes shared_point) +{ + VERIFY(shared_point.size() == BYTES); + ByteBuffer premaster_key = TRY(ByteBuffer::copy(shared_point)); + return premaster_key; +} + } diff --git a/Userland/Libraries/LibCrypto/Curves/X448.h b/Userland/Libraries/LibCrypto/Curves/X448.h index b2c9a1f8ac..f6d18a5cc4 100644 --- a/Userland/Libraries/LibCrypto/Curves/X448.h +++ b/Userland/Libraries/LibCrypto/Curves/X448.h @@ -7,10 +7,11 @@ #pragma once #include +#include namespace Crypto::Curves { -class X448 { +class X448 : public EllipticCurve { static constexpr u16 BITS = 448; static constexpr u8 BYTES = 56; @@ -18,7 +19,11 @@ class X448 { static constexpr u32 A24 = 39082; public: - static ErrorOr compute_coordinate(ReadonlyBytes a, ReadonlyBytes b); + size_t key_size() override { return BYTES; } + ErrorOr generate_private_key() override; + ErrorOr generate_public_key(ReadonlyBytes a) override; + ErrorOr compute_coordinate(ReadonlyBytes a, ReadonlyBytes b) override; + ErrorOr derive_premaster_key(ReadonlyBytes shared_point) override; private: static void import_state(u32* state, ReadonlyBytes data); diff --git a/Userland/Libraries/LibTLS/CipherSuite.h b/Userland/Libraries/LibTLS/CipherSuite.h index ac39e7999c..91f9b4e733 100644 --- a/Userland/Libraries/LibTLS/CipherSuite.h +++ b/Userland/Libraries/LibTLS/CipherSuite.h @@ -197,23 +197,6 @@ enum class NamedCurve : u16 { x448 = 30, }; -constexpr size_t named_curve_key_size(NamedCurve group) -{ - switch (group) { - case NamedCurve::secp256r1: - case NamedCurve::secp384r1: - case NamedCurve::secp521r1: - // FIXME: Add the correct key size for these elliptic curves - return 0; - case NamedCurve::x25519: - return 256; - case NamedCurve::x448: - return 448; - default: - return 0; - } -} - enum class ECPointFormat : u8 { Uncompressed = 0, }; diff --git a/Userland/Libraries/LibTLS/HandshakeClient.cpp b/Userland/Libraries/LibTLS/HandshakeClient.cpp index 38c8e34efe..1420e7a1ef 100644 --- a/Userland/Libraries/LibTLS/HandshakeClient.cpp +++ b/Userland/Libraries/LibTLS/HandshakeClient.cpp @@ -10,8 +10,6 @@ #include #include #include -#include -#include #include #include #include @@ -295,86 +293,44 @@ void TLSv12::build_dhe_rsa_pre_master_secret(PacketBuilder& builder) builder.append(dh_Yc_bytes); } -ErrorOr TLSv12::named_curve_multiply(NamedCurve curve, ReadonlyBytes a, ReadonlyBytes b) -{ - switch (curve) { - case NamedCurve::x25519: - return Crypto::Curves::X25519::compute_coordinate(a, b); - case NamedCurve::x448: - return Crypto::Curves::X448::compute_coordinate(a, b); - default: - dbgln("No known handler for multiplying curve {}", static_cast(curve)); - TODO(); - } -} - -ErrorOr TLSv12::named_curve_generator_point(NamedCurve curve) -{ - auto key_size = named_curve_key_size(curve) / 8; - - auto generator_point_data_result = ByteBuffer::create_zeroed(key_size); - if (generator_point_data_result.is_error()) { - dbgln("Failed to generate curve multiplication point: not enough memory"); - return AK::Error::from_string_literal("Failed to generate curve multiplication point: not enough memory"); - } - auto generator_point_data = generator_point_data_result.release_value(); - - switch (curve) { - case NamedCurve::x25519: - ByteReader::store(generator_point_data.offset_pointer(0), 9); - break; - case NamedCurve::x448: - ByteReader::store(generator_point_data.offset_pointer(0), 5); - break; - default: - dbgln("No known handler for generator point of curve {}", static_cast(curve)); - TODO(); - } - - return generator_point_data; -} - void TLSv12::build_ecdhe_rsa_pre_master_secret(PacketBuilder& builder) { - size_t const key_size = named_curve_key_size(m_context.server_curve_choice) / 8; - auto generator_point_result = named_curve_generator_point(m_context.server_curve_choice); - if (generator_point_result.is_error()) { - dbgln("Failed to build ECDHE_RSA premaster secret: not enough memory"); - return; - } - auto generator_point = generator_point_result.release_value(); - ReadonlyBytes generator_point_bytes = generator_point; - // Create a random private key - auto private_key_result = ByteBuffer::create_uninitialized(key_size); + auto private_key_result = m_context.server_key_exchange_curve->generate_private_key(); if (private_key_result.is_error()) { dbgln("Failed to build ECDHE_RSA premaster secret: not enough memory"); return; } auto private_key = private_key_result.release_value(); - fill_with_random(private_key.data(), key_size); - ReadonlyBytes private_key_bytes = private_key; - // Calculate the public key by multiplying the private key with the generator point - auto public_key_result = named_curve_multiply(m_context.server_curve_choice, private_key_bytes, generator_point_bytes); + // Calculate the public key from the private key + auto public_key_result = m_context.server_key_exchange_curve->generate_public_key(private_key); if (public_key_result.is_error()) { dbgln("Failed to build ECDHE_RSA premaster secret: not enough memory"); return; } auto public_key = public_key_result.release_value(); - // Calculate the pre master secret by multiplying the client private key and the server public key + // Calculate the shared point by multiplying the client private key and the server public key ReadonlyBytes server_public_key_bytes = m_context.server_diffie_hellman_params.p; - auto pre_master_secret_result = named_curve_multiply(m_context.server_curve_choice, private_key_bytes, server_public_key_bytes); - if (pre_master_secret_result.is_error()) { + auto shared_point_result = m_context.server_key_exchange_curve->compute_coordinate(private_key, server_public_key_bytes); + if (shared_point_result.is_error()) { dbgln("Failed to build ECDHE_RSA premaster secret: not enough memory"); return; } - m_context.premaster_key = pre_master_secret_result.release_value(); + auto shared_point = shared_point_result.release_value(); + + // Derive the premaster key from the shared point + auto premaster_key_result = m_context.server_key_exchange_curve->derive_premaster_key(shared_point); + if (premaster_key_result.is_error()) { + dbgln("Failed to build ECDHE_RSA premaster secret: not enough memory"); + return; + } + m_context.premaster_key = premaster_key_result.release_value(); if constexpr (TLS_DEBUG) { dbgln("Build ECDHE_RSA pre master secret"); - dbgln("client private key: {:hex-dump}", private_key_bytes); + dbgln("client private key: {:hex-dump}", (ReadonlyBytes)private_key); dbgln("client public key: {:hex-dump}", (ReadonlyBytes)public_key); dbgln("premaster key: {:hex-dump}", (ReadonlyBytes)m_context.premaster_key); } @@ -384,8 +340,8 @@ void TLSv12::build_ecdhe_rsa_pre_master_secret(PacketBuilder& builder) return; } - builder.append_u24(key_size + 1); - builder.append((u8)key_size); + builder.append_u24(public_key.size() + 1); + builder.append((u8)public_key.size()); builder.append(public_key); } diff --git a/Userland/Libraries/LibTLS/HandshakeServer.cpp b/Userland/Libraries/LibTLS/HandshakeServer.cpp index 1fc590fbe3..1e0866bcec 100644 --- a/Userland/Libraries/LibTLS/HandshakeServer.cpp +++ b/Userland/Libraries/LibTLS/HandshakeServer.cpp @@ -11,6 +11,9 @@ #include #include +#include +#include +#include #include #include #include @@ -290,8 +293,7 @@ ssize_t TLSv12::handle_dhe_rsa_server_key_exchange(ReadonlyBytes buffer) ssize_t TLSv12::handle_ecdhe_rsa_server_key_exchange(ReadonlyBytes buffer) { - size_t size_required_for_curve_name = 6; - if (buffer.size() < size_required_for_curve_name) + if (buffer.size() < 7) return (i8)Error::NeedMoreData; auto curve_type = buffer[3]; @@ -302,14 +304,23 @@ ssize_t TLSv12::handle_ecdhe_rsa_server_key_exchange(ReadonlyBytes buffer) if (!m_context.options.elliptic_curves.contains_slow(curve)) return (i8)Error::NotUnderstood; - m_context.server_curve_choice = curve; - auto curve_key_size_bytes = named_curve_key_size(curve) / 8; - if (buffer.size() < curve_key_size_bytes + 1) - return (i8)Error::NeedMoreData; + switch ((NamedCurve)curve) { + case NamedCurve::x25519: + m_context.server_key_exchange_curve = make(); + break; + case NamedCurve::x448: + m_context.server_key_exchange_curve = make(); + break; + default: + return (i8)Error::NotUnderstood; + } auto server_public_key_length = buffer[6]; - if (server_public_key_length != curve_key_size_bytes) - return (i8)Error::FeatureNotSupported; + if (server_public_key_length != m_context.server_key_exchange_curve->key_size()) + return (i8)Error::NotUnderstood; + + if (buffer.size() < 7u + server_public_key_length) + return (i8)Error::NeedMoreData; auto server_public_key = buffer.slice(7, server_public_key_length); auto server_public_key_copy_result = ByteBuffer::copy(server_public_key); diff --git a/Userland/Libraries/LibTLS/TLSv12.h b/Userland/Libraries/LibTLS/TLSv12.h index b94fae81fc..6440fd5290 100644 --- a/Userland/Libraries/LibTLS/TLSv12.h +++ b/Userland/Libraries/LibTLS/TLSv12.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -334,7 +335,7 @@ struct Context { ByteBuffer Ys; } server_diffie_hellman_params; - NamedCurve server_curve_choice; + OwnPtr server_key_exchange_curve; }; class TLSv12 final : public Core::Stream::Socket { @@ -465,9 +466,6 @@ private: void build_dhe_rsa_pre_master_secret(PacketBuilder&); void build_ecdhe_rsa_pre_master_secret(PacketBuilder&); - static ErrorOr named_curve_multiply(NamedCurve curve, ReadonlyBytes a, ReadonlyBytes b); - static ErrorOr named_curve_generator_point(NamedCurve curve); - ErrorOr flush(); void write_into_socket(); ErrorOr read_from_socket();