LibCrypto: Cleanup `Crypto::PK::RSA` constructors to avoid pitfalls

- Removed the constructor taking a (n, d, e) tuple and moved
  it to `RSAPrivateKey`
- Removed default constructor with key generation because it was always
  misused and the default key size is quite small
- Added utility constructors to accept a key pair, public key, private
  key or both
- Made constructor parameters const
- Updated test to use generated random keys where possible

Libraries/LibCrypto/PK/PK.h

@@ -126,7 +126,7 @@ public:
     using PublicKeyType = PubKeyT;
     using PrivateKeyType = PrivKeyT;
 
-    PKSystem(PublicKeyType& pubkey, PrivateKeyType& privkey)
+    PKSystem(PublicKeyType const& pubkey, PrivateKeyType const& privkey)
         : m_public_key(pubkey)
         , m_private_key(privkey)
     {

Libraries/LibCrypto/PK/RSA.h

@@ -7,7 +7,6 @@
 
 #pragma once
 
-#include <AK/Span.h>
 #include <LibCrypto/ASN1/DER.h>
 #include <LibCrypto/BigInt/UnsignedBigInteger.h>
 #include <LibCrypto/NumberTheory/ModularFunctions.h>
@@ -64,6 +63,14 @@ private:
 template<typename Integer = UnsignedBigInteger>
 class RSAPrivateKey {
 public:
+    RSAPrivateKey(Integer n, Integer d, Integer e)
+        : m_modulus(move(n))
+        , m_private_exponent(move(d))
+        , m_public_exponent(move(e))
+        , m_length(m_modulus.trimmed_length() * sizeof(u32))
+    {
+    }
+
     RSAPrivateKey(Integer n, Integer d, Integer e, Integer p, Integer q)
         : m_modulus(move(n))
         , m_private_exponent(move(d))
@@ -175,17 +182,27 @@ public:
         return keys;
     }
 
-    RSA(IntegerType n, IntegerType d, IntegerType e)
+    RSA(KeyPairType const& pair)
+        : PKSystem<RSAPrivateKey<IntegerType>, RSAPublicKey<IntegerType>>(pair.public_key, pair.private_key)
     {
-        m_public_key.set(n, e);
-        m_private_key = { n, d, e, 0, 0, 0, 0, 0 };
     }
 
-    RSA(PublicKeyType& pubkey, PrivateKeyType& privkey)
+    RSA(PublicKeyType const& pubkey, PrivateKeyType const& privkey)
         : PKSystem<RSAPrivateKey<IntegerType>, RSAPublicKey<IntegerType>>(pubkey, privkey)
     {
     }
 
+    RSA(PrivateKeyType const& privkey)
+    {
+        m_private_key = privkey;
+        m_public_key.set(m_private_key.modulus(), m_private_key.public_exponent());
+    }
+
+    RSA(PublicKeyType const& pubkey)
+    {
+        m_public_key = pubkey;
+    }
+
     RSA(ByteBuffer const& publicKeyPEM, ByteBuffer const& privateKeyPEM)
     {
         import_public_key(publicKeyPEM);
@@ -198,14 +215,6 @@ public:
         m_public_key.set(m_private_key.modulus(), m_private_key.public_exponent());
     }
 
-    // create our own keys
-    RSA()
-    {
-        auto pair = generate_key_pair();
-        m_public_key = pair.public_key;
-        m_private_key = pair.private_key;
-    }
-
     virtual void encrypt(ReadonlyBytes in, Bytes& out) override;
     virtual void decrypt(ReadonlyBytes in, Bytes& out) override;
 

Libraries/LibTLS/HandshakeClient.cpp

@@ -189,7 +189,7 @@ void TLSv12::build_rsa_pre_master_secret(PacketBuilder& builder)
         print_buffer(m_context.premaster_key);
     }
 
-    Crypto::PK::RSA_PKCS1_EME rsa(certificate.public_key.rsa.modulus(), 0, certificate.public_key.rsa.public_exponent());
+    Crypto::PK::RSA_PKCS1_EME rsa(certificate.public_key.rsa);
 
     Vector<u8, 32> out;
     out.resize(rsa.output_size());

Libraries/LibWeb/Crypto/CryptoAlgorithms.cpp

@@ -658,8 +658,7 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> RSAOAEP::encrypt(AlgorithmParams c
     auto ciphertext = TRY_OR_THROW_OOM(vm, ByteBuffer::create_uninitialized(public_key.length()));
     auto ciphertext_bytes = ciphertext.bytes();
 
-    auto rsa = ::Crypto::PK::RSA {};
-    rsa.set_public_key(public_key);
+    auto rsa = ::Crypto::PK::RSA { public_key };
     rsa.encrypt(padding, ciphertext_bytes);
 
     // 6. Return the result of creating an ArrayBuffer containing ciphertext.
@@ -687,8 +686,7 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> RSAOAEP::decrypt(AlgorithmParams c
     // 3. Perform the decryption operation defined in Section 7.1 of [RFC3447] with the key represented by key as the recipient's RSA private key,
     //    the contents of ciphertext as the ciphertext to be decrypted, C, and label as the label, L, and with the hash function specified by the hash attribute
     //    of the [[algorithm]] internal slot of key as the Hash option and MGF1 (defined in Section B.2.1 of [RFC3447]) as the MGF option.
-    auto rsa = ::Crypto::PK::RSA {};
-    rsa.set_private_key(private_key);
+    auto rsa = ::Crypto::PK::RSA { private_key };
     u32 private_key_length = private_key.length();
 
     auto padding = TRY_OR_THROW_OOM(vm, ByteBuffer::create_uninitialized(private_key_length));

Tests/LibCrypto/TestRSA.cpp

@@ -21,10 +21,10 @@ TEST_CASE(test_RSA_raw_encrypt)
 {
     ByteBuffer data { "hellohellohellohellohellohellohellohellohellohellohellohello123-"_b };
     u8 result[] { 0x6f, 0x7b, 0xe2, 0xd3, 0x95, 0xf8, 0x8d, 0x87, 0x6d, 0x10, 0x5e, 0xc3, 0xcd, 0xf7, 0xbb, 0xa6, 0x62, 0x8e, 0x45, 0xa0, 0xf1, 0xe5, 0x0f, 0xdf, 0x69, 0xcb, 0xb6, 0xd5, 0x42, 0x06, 0x7d, 0x72, 0xa9, 0x5e, 0xae, 0xbf, 0xbf, 0x0f, 0xe0, 0xeb, 0x31, 0x31, 0xca, 0x8a, 0x81, 0x1e, 0xb9, 0xec, 0x6d, 0xcc, 0xb8, 0xa4, 0xac, 0xa3, 0x31, 0x05, 0xa9, 0xac, 0xc9, 0xd3, 0xe6, 0x2a, 0x18, 0xfe };
-    Crypto::PK::RSA rsa(
+    Crypto::PK::RSA rsa(Crypto::PK::RSAPublicKey<> {
         "8126832723025844890518845777858816391166654950553329127845898924164623511718747856014227624997335860970996746552094406240834082304784428582653994490504519"_bigint,
-        "4234603516465654167360850580101327813936403862038934287300450163438938741499875303761385527882335478349599685406941909381269804396099893549838642251053393"_bigint,
-        "65537"_bigint);
+        "65537"_bigint,
+    });
     u8 buffer[rsa.output_size()];
     auto buf = Bytes { buffer, sizeof(buffer) };
     rsa.encrypt(data, buf);
@@ -35,10 +35,7 @@ TEST_CASE(test_RSA_raw_encrypt)
 TEST_CASE(test_RSA_PKCS_1_encrypt)
 {
     ByteBuffer data { "hellohellohellohellohellohellohellohellohello123-"_b };
-    Crypto::PK::RSA_PKCS1_EME rsa(
-        "8126832723025844890518845777858816391166654950553329127845898924164623511718747856014227624997335860970996746552094406240834082304784428582653994490504519"_bigint,
-        "4234603516465654167360850580101327813936403862038934287300450163438938741499875303761385527882335478349599685406941909381269804396099893549838642251053393"_bigint,
-        "65537"_bigint);
+    Crypto::PK::RSA_PKCS1_EME rsa(Crypto::PK::RSA::generate_key_pair(1024));
     u8 buffer[rsa.output_size()];
     auto buf = Bytes { buffer, sizeof(buffer) };
     rsa.encrypt(data, buf);
@@ -153,10 +150,7 @@ c8yGzl89pYST
 
 TEST_CASE(test_RSA_encrypt_decrypt)
 {
-    Crypto::PK::RSA rsa(
-        "9527497237087650398000977129550904920919162360737979403539302312977329868395261515707123424679295515888026193056908173564681660256268221509339074678416049"_bigint,
-        "39542231845947188736992321577701849924317746648774438832456325878966594812143638244746284968851807975097653255909707366086606867657273809465195392910913"_bigint,
-        "65537"_bigint);
+    Crypto::PK::RSA rsa(Crypto::PK::RSA::generate_key_pair(1024));
 
     u8 enc_buffer[rsa.output_size()];
     u8 dec_buffer[rsa.output_size()];