Kcer-over-NSS: Implement SignRsaPkcs1Raw

Bug: b:244408716
Test: Kcer* unit tests
Change-Id: I42271f0bfd69c8d04c1b19708abc2c93393cc9d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4527096
Commit-Queue: Michael Ershov <miersh@google.com>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1147112}

chrome/browser/chromeos/kcer_nss/kcer_nss_unittest.cc

@@ -20,6 +20,7 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/browser_task_environment.h"
 #include "crypto/scoped_test_nss_db.h"
+#include "crypto/secure_hash.h"
 #include "crypto/signature_verifier.h"
 #include "net/cert/pem.h"
 #include "net/test/cert_builder.h"
@@ -159,6 +160,27 @@ absl::optional<std::vector<uint8_t>> ReadPemFileReturnDer(
   return StrToBytes(tokenizer.data());
 }
 
+// Returns |hash| prefixed with DER-encoded PKCS#1 DigestInfo with
+// AlgorithmIdentifier=id-sha256.
+// This is useful for testing Kcer::SignRsaPkcs1Raw which only
+// appends PKCS#1 v1.5 padding before signing.
+std::vector<uint8_t> PrependSHA256DigestInfo(base::span<const uint8_t> hash) {
+  // DER-encoded PKCS#1 DigestInfo "prefix" with
+  // AlgorithmIdentifier=id-sha256.
+  // The encoding is taken from https://tools.ietf.org/html/rfc3447#page-43
+  const std::vector<uint8_t> kDigestInfoSha256DerData = {
+      0x30, 0x31, 0x30, 0x0d, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01,
+      0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04, 0x20};
+
+  std::vector<uint8_t> result;
+  result.reserve(kDigestInfoSha256DerData.size() + hash.size());
+
+  result.insert(result.end(), kDigestInfoSha256DerData.begin(),
+                kDigestInfoSha256DerData.end());
+  result.insert(result.end(), hash.begin(), hash.end());
+  return result;
+}
+
 // A helper class to work with tokens (that exist on the IO thread) from the UI
 // thread.
 class TokenHolder {
@@ -363,6 +385,10 @@ TEST_F(KcerNssTest, QueueTasksFailInitializationThenGetErrors) {
   base::test::TestFuture<base::expected<Signature, Error>> sign_waiter;
   kcer->Sign(PrivateKeyHandle(PublicKeySpki()), SigningScheme::kRsaPkcs1Sha512,
              DataToSign({1, 2, 3}), sign_waiter.GetCallback());
+  base::test::TestFuture<base::expected<Signature, Error>> sign_digest_waiter;
+  kcer->SignRsaPkcs1Raw(PrivateKeyHandle(PublicKeySpki()),
+                        DigestWithPrefix({1, 2, 3}),
+                        sign_digest_waiter.GetCallback());
   base::test::TestFuture<base::expected<TokenInfo, Error>>
       get_token_info_waiter;
   kcer->GetTokenInfo(Token::kUser, get_token_info_waiter.GetCallback());
@@ -416,6 +442,9 @@ TEST_F(KcerNssTest, QueueTasksFailInitializationThenGetErrors) {
             Error::kTokenInitializationFailed);
   ASSERT_FALSE(sign_waiter.Get().has_value());
   EXPECT_EQ(sign_waiter.Get().error(), Error::kTokenInitializationFailed);
+  ASSERT_FALSE(sign_digest_waiter.Get().has_value());
+  EXPECT_EQ(sign_digest_waiter.Get().error(),
+            Error::kTokenInitializationFailed);
   ASSERT_FALSE(get_token_info_waiter.Get().has_value());
   EXPECT_EQ(get_token_info_waiter.Get().error(),
             Error::kTokenInitializationFailed);
@@ -542,6 +571,56 @@ TEST_F(KcerNssTest, SignEcc) {
   }
 }
 
+// Test that `Kcer::SignRsaPkcs1Raw()` produces a correct signature.
+TEST_F(KcerNssTest, SignRsaPkcs1Raw) {
+  TokenHolder user_token(Token::kUser);
+  user_token.Initialize();
+
+  std::unique_ptr<Kcer> kcer = internal::CreateKcer(
+      IOTaskRunner(), user_token.GetWeakPtr(), /*device_token=*/nullptr);
+
+  base::test::TestFuture<base::expected<PublicKey, Error>> generate_key_waiter;
+  kcer->GenerateRsaKey(Token::kUser, /*modulus_length_bits=*/2048,
+                       /*hardware_backed=*/true,
+                       generate_key_waiter.GetCallback());
+  ASSERT_TRUE(generate_key_waiter.Get().has_value());
+  const PublicKey& public_key = generate_key_waiter.Get().value();
+
+  DataToSign data_to_sign({1, 2, 3, 4, 5, 6, 7, 8, 9, 10});
+
+  // A caller would need to hash the data themself before calling
+  // `SignRsaPkcs1Raw`, do that here.
+  auto hasher = crypto::SecureHash::Create(crypto::SecureHash::SHA256);
+  hasher->Update(data_to_sign->data(), data_to_sign->size());
+  std::vector<uint8_t> hash(hasher->GetHashLength());
+  hasher->Finish(hash.data(), hash.size());
+  DigestWithPrefix digest_with_prefix(PrependSHA256DigestInfo(hash));
+
+  // Generate the signature.
+  base::test::TestFuture<base::expected<Signature, Error>> sign_waiter;
+  kcer->SignRsaPkcs1Raw(PrivateKeyHandle(public_key),
+                        std::move(digest_with_prefix),
+                        sign_waiter.GetCallback());
+  ASSERT_TRUE(sign_waiter.Get().has_value());
+  const Signature& signature = sign_waiter.Get().value();
+
+  // Verify the signature.
+  crypto::SignatureVerifier signature_verifier;
+  ASSERT_TRUE(signature_verifier.VerifyInit(
+      crypto::SignatureVerifier::SignatureAlgorithm::RSA_PKCS1_SHA256,
+      signature.value(), public_key.GetSpki().value()));
+  signature_verifier.VerifyUpdate(data_to_sign.value());
+  EXPECT_TRUE(signature_verifier.VerifyFinal());
+
+  // Verify that manual hashing + `SignRsaPkcs1Raw` produces the same
+  // signature as just `Sign`.
+  base::test::TestFuture<base::expected<Signature, Error>> normal_sign_waiter;
+  kcer->Sign(PrivateKeyHandle(public_key), SigningScheme::kRsaPkcs1Sha256,
+             data_to_sign, normal_sign_waiter.GetCallback());
+  ASSERT_TRUE(normal_sign_waiter.Get().has_value());
+  EXPECT_EQ(sign_waiter.Get().value(), normal_sign_waiter.Get().value());
+}
+
 // Test that Kcer::GetTokenInfo() method returns meaningful values.
 TEST_F(KcerNssTest, GetTokenInfo) {
   TokenHolder user_token(Token::kUser);
@@ -640,8 +719,8 @@ TEST_F(KcerNssTest, GetKeyInfoGeneric) {
   // Hardware- vs software-backed indicators on real devices are provided by
   // Chaps and are wrong in unit tests.
   expected_key_info.is_hardware_backed = true;
-  // NSS sets an empty nickname by default, this doesn't have to be like this in
-  // general.
+  // NSS sets an empty nickname by default, this doesn't have to be like this
+  // in general.
   expected_key_info.nickname = "";
   // Custom attributes are stored differently in tests and have empty values by
   // default.
@@ -655,8 +734,8 @@ TEST_F(KcerNssTest, GetKeyInfoGeneric) {
     ASSERT_TRUE(key_info_waiter.Get().has_value());
     const KeyInfo& key_info = key_info_waiter.Get().value();
 
-    // Copy some fields, their values are covered by dedicated tests, this test
-    // only checks that they don't change when they shouldn't.
+    // Copy some fields, their values are covered by dedicated tests, this
+    // test only checks that they don't change when they shouldn't.
     expected_key_info.key_type = key_info.key_type;
     expected_key_info.supported_signing_schemes =
         key_info.supported_signing_schemes;

chrome/browser/chromeos/kcer_nss/kcer_token_impl_nss.cc

@@ -464,6 +464,83 @@ void SignOnWorkerThread(crypto::ScopedPK11Slot slot,
   return std::move(callback).Run(Signature(std::move(signature)));
 }
 
+// Checks whether |input_length| is lower or equal to the maximum input length
+// for a RSA PKCS#1 v1.5 signature generated using |private_key| with PK11_Sign.
+// Returns false if |input_length| is too large.
+// If the maximum input length can not be determined (which is possible because
+// it queries the PKCS#11 module), returns true and logs a warning.
+bool CheckRsaPkcs1SignDigestInputLength(SECKEYPrivateKey* private_key,
+                                        size_t input_length) {
+  // For RSA keys, PK11_Sign will perform PKCS#1 v1.5 padding, which needs at
+  // least 11 bytes. RSA Sign can process an input of max. modulus length.
+  // Thus the maximum input length for the sign operation is
+  // |modulus_length - 11|.
+  int modulus_length_bytes = PK11_GetPrivateModulusLen(private_key);
+  if (modulus_length_bytes <= 0) {
+    LOG(WARNING) << "Could not determine modulus length";
+    return true;
+  }
+  size_t max_input_length_after_padding =
+      static_cast<size_t>(modulus_length_bytes);
+  // PKCS#1 v1.5 Padding needs at least this many bytes.
+  size_t kMinPaddingLength = 11u;
+  return input_length + kMinPaddingLength <= max_input_length_after_padding;
+}
+
+// Performs "raw" PKCS1 v1.5 padding + signing by calling PK11_Sign on |key|.
+// TODO(b/244408117): This method is mostly a copy of
+// `PlatformKeysService::SignRSAPKCS1Raw` from platform_keys_service_nss.cc .
+// The original method should be replaced during the upcoming refactoring to
+// remove direct usages of NSS.
+void SignRsaPkcs1RawOnWorkerThread(crypto::ScopedPK11Slot slot,
+                                   PrivateKeyHandle key,
+                                   DigestWithPrefix digest_with_prefix,
+                                   Kcer::SignCallback callback) {
+  base::expected<crypto::ScopedSECKEYPrivateKey, Error> private_key =
+      GetSECKEYPrivateKey(slot, key);
+  if (!private_key.has_value()) {
+    return std::move(callback).Run(base::unexpected(private_key.error()));
+  }
+  const crypto::ScopedSECKEYPrivateKey& sec_private_key = private_key.value();
+  if (sec_private_key->keyType != ::KeyType::rsaKey) {
+    return std::move(callback).Run(
+        base::unexpected(Error::kKeyDoesNotSupportSigningScheme));
+  }
+
+  static_assert(
+      sizeof(*digest_with_prefix->data()) == sizeof(char),
+      "Can't reinterpret data if its characters are not 8 bit large.");
+  SECItem input = {siBuffer,
+                   const_cast<unsigned char*>(digest_with_prefix->data()),
+                   static_cast<unsigned int>(digest_with_prefix->size())};
+
+  // Compute signature of hash.
+  int signature_len = PK11_SignatureLen(sec_private_key.get());
+  if (signature_len <= 0) {
+    return std::move(callback).Run(
+        base::unexpected(Error::kFailedToSignBadSignatureLength));
+  }
+
+  std::vector<unsigned char> signature(signature_len);
+  SECItem signature_output = {siBuffer, signature.data(),
+                              static_cast<unsigned int>(signature.size())};
+  if (PK11_Sign(sec_private_key.get(), &signature_output, &input) !=
+      SECSuccess) {
+    // Input size is checked after a failure - obtaining max input size
+    // involves extracting key modulus length which is not a free operation, so
+    // don't bother if signing succeeded.
+    // Note: It would be better if this could be determined from some library
+    // return code (e.g. PORT_GetError), but this was not possible with
+    // NSS+chaps at this point.
+    if (!CheckRsaPkcs1SignDigestInputLength(sec_private_key.get(),
+                                            digest_with_prefix->size())) {
+      return std::move(callback).Run(base::unexpected(Error::kInputTooLong));
+    }
+    return std::move(callback).Run(base::unexpected(Error::kFailedToSign));
+  }
+  return std::move(callback).Run(Signature(std::move(signature)));
+}
+
 std::vector<SigningScheme> GetSigningSchemes(bool supports_pss,
                                              KeyType key_type) {
   std::vector<SigningScheme> result;
@@ -981,10 +1058,29 @@ void KcerTokenImplNss::Sign(PrivateKeyHandle key,
 }
 
 void KcerTokenImplNss::SignRsaPkcs1Raw(PrivateKeyHandle key,
-                                       SigningScheme signing_scheme,
                                        DigestWithPrefix digest_with_prefix,
                                        Kcer::SignCallback callback) {
-  // TODO(244408716): Implement.
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  if (UNLIKELY(state_ == State::kInitializationFailed)) {
+    return HandleInitializationFailed(std::move(callback));
+  }
+  if (is_blocked_) {
+    return task_queue_.push(base::BindOnce(
+        &KcerTokenImplNss::SignRsaPkcs1Raw, weak_factory_.GetWeakPtr(),
+        std::move(key), std::move(digest_with_prefix), std::move(callback)));
+  }
+
+  // Block task queue, attach unblocking task to the callback.
+  auto unblocking_callback = std::move(callback).Then(BlockQueueGetUnblocker());
+
+  base::ThreadPool::PostTask(
+      FROM_HERE,
+      {base::MayBlock(), base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
+      base::BindOnce(&SignRsaPkcs1RawOnWorkerThread,
+                     crypto::ScopedPK11Slot(PK11_ReferenceSlot(slot_.get())),
+                     std::move(key), std::move(digest_with_prefix),
+                     std::move(unblocking_callback)));
 }
 
 void KcerTokenImplNss::GetTokenInfo(Kcer::GetTokenInfoCallback callback) {

chrome/browser/chromeos/kcer_nss/kcer_token_impl_nss.h

@@ -94,7 +94,6 @@ class KcerTokenImplNss : public KcerToken, public net::CertDatabase::Observer {
             DataToSign data,
             Kcer::SignCallback callback) override;
   void SignRsaPkcs1Raw(PrivateKeyHandle key,
-                       SigningScheme signing_scheme,
                        DigestWithPrefix digest_with_prefix,
                        Kcer::SignCallback callback) override;
   void GetTokenInfo(Kcer::GetTokenInfoCallback callback) override;

chromeos/components/kcer/kcer.h

@@ -72,6 +72,7 @@ enum class COMPONENT_EXPORT(KCER) Error {
   kFailedToSign = 23,
   kFailedToSignBadSignatureLength = 24,
   kFailedToDerEncode = 25,
+  kInputTooLong = 26,
 };
 
 // Handles for tokens on ChromeOS.

chromeos/components/kcer/kcer_impl.cc

@@ -258,7 +258,37 @@ void KcerImpl::SignWithToken(
 void KcerImpl::SignRsaPkcs1Raw(PrivateKeyHandle key,
                                DigestWithPrefix digest_with_prefix,
                                SignCallback callback) {
-  // TODO(244408716): Implement.
+  if (key.GetTokenInternal().has_value()) {
+    return SignRsaPkcs1RawWithToken(std::move(digest_with_prefix),
+                                    std::move(callback), std::move(key));
+  }
+
+  auto on_find_key_done = base::BindOnce(
+      &KcerImpl::SignRsaPkcs1RawWithToken, weak_factory_.GetWeakPtr(),
+      std::move(digest_with_prefix), std::move(callback));
+  return PopulateTokenForKey(std::move(key), std::move(on_find_key_done));
+}
+
+void KcerImpl::SignRsaPkcs1RawWithToken(
+    DigestWithPrefix digest_with_prefix,
+    SignCallback callback,
+    base::expected<PrivateKeyHandle, Error> key_or_error) {
+  if (!key_or_error.has_value()) {
+    return std::move(callback).Run(base::unexpected(key_or_error.error()));
+  }
+  PrivateKeyHandle key = std::move(key_or_error).value();
+
+  const base::WeakPtr<KcerToken>& kcer_token =
+      GetToken(key.GetTokenInternal().value());
+  if (!kcer_token.MaybeValid()) {
+    return std::move(callback).Run(
+        base::unexpected(Error::kTokenIsNotAvailable));
+  }
+  token_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(&KcerToken::SignRsaPkcs1Raw, kcer_token, std::move(key),
+                     std::move(digest_with_prefix),
+                     base::BindPostTaskToCurrentDefault(std::move(callback))));
 }
 
 base::flat_set<Token> KcerImpl::GetAvailableTokens() {

chromeos/components/kcer/kcer_impl.h

@@ -123,6 +123,11 @@ class KcerImpl : public Kcer {
                      SignCallback callback,
                      base::expected<PrivateKeyHandle, Error> key_or_error);
 
+  void SignRsaPkcs1RawWithToken(
+      DigestWithPrefix digest_with_prefix,
+      SignCallback callback,
+      base::expected<PrivateKeyHandle, Error> key_or_error);
+
   void GetKeyInfoWithToken(
       GetKeyInfoCallback callback,
       base::expected<PrivateKeyHandle, Error> key_or_error);

chromeos/components/kcer/kcer_token.h

@@ -66,7 +66,6 @@ class COMPONENT_EXPORT(KCER) KcerToken {
                     DataToSign data,
                     Kcer::SignCallback callback) = 0;
   virtual void SignRsaPkcs1Raw(PrivateKeyHandle key,
-                               SigningScheme signing_scheme,
                                DigestWithPrefix digest_with_prefix,
                                Kcer::SignCallback callback) = 0;
   virtual void GetTokenInfo(Kcer::GetTokenInfoCallback callback) = 0;