Bug 1702663 Need to support RSA PSS with Hashing PKCS #11 Mechanisms.

FIPS requires that we supply a hash and sign interface for our supported
signing algorithms to be validated. We already have those interfaces in
softoken for RSA PKCS1, DSA, and ECDSA. However, we don't test those
interfaces, now do we supply a way for an application to access those
interfaces (usually applications use the VFY_ and SGN_ interfaces which
handles the hashing an verify/sign operations).

We also have a generic pk11_signature_tests class in pk11_gtest, but only ecdsa
and some rsa pss tests uses it.

This patch rectifies all of these deficiencies:

lib/softokn
1) Hash and sign/verify mechanisms have been added to softoken to support PSS
hash and sign.
2) The rsa, dsa, and ecdsa hash and sign algorithms were also cleaned up by
creating a fake CKM_SHA1 which matches CKM_SHA_1 so that we can fully use the
same macros for all the hash types.

1&2 was sufficient to provide the goals of this task, however we wanted to be
able to add tests for this functionality..

lib/pk11wrap
3) Two new functions were added: PK11_CreateContextByPubKey and
PK11_CreateContextByPrivKey. These allow you to create multipart contexts with
Public and Private keys. This is a requirement to support hash and sign, as
they are multi-part operations (rather then just signing a hash, which is a
single part operation). With these functions, you can now use the PK11_DigestOp
and PK11_DigestFinal to complete a signature or verify optiation.

gtests/pk11_gtest
4) Add hash and sign/hash and verify support to the generic
pk11_signature_tests.h.
5) pk11_dsa_unittest.cc, pk11_rsa_unittest.cc, and the remainder of
pk11_rsapss_unittest.cc (Wycheproof tests) were moved to use the
pk11_signature_tests common implementation rather then their own.
6) pk11_ecdsa_unittest.cc was updated to support the hash&sign/verify combo
mechanism.
7) With multiple functions using pk11_signature_tests.h, The large functions
are moved to pk11_signature_tests.cpp.
8) The test vectors themselves were not changes, now just test against the
traditional hash first then verify interfaces and the hash and verify
interfaces.

Differential Revision: https://phabricator.services.mozilla.com/D110641

8 files changed, 380 insertions, 201 deletions

diff --git a/gtests/pk11_gtest/manifest.mn b/gtests/pk11_gtest/manifest.mn
index 80530675b..a1a1af346 100644
--- a/gtests/pk11_gtest/manifest.mn
+++ b/gtests/pk11_gtest/manifest.mn
@@ -38,6 +38,7 @@ CPPSRCS = \
       pk11_rsaoaep_unittest.cc \
       pk11_rsapkcs1_unittest.cc \
       pk11_rsapss_unittest.cc \
+      pk11_signature_test.cc \
       pk11_seed_cbc_unittest.cc \
       $(NULL)
 
diff --git a/gtests/pk11_gtest/pk11_dsa_unittest.cc b/gtests/pk11_gtest/pk11_dsa_unittest.cc
index 0c776c9f1..634d496bf 100644
--- a/gtests/pk11_gtest/pk11_dsa_unittest.cc
+++ b/gtests/pk11_gtest/pk11_dsa_unittest.cc
@@ -6,12 +6,14 @@
 
 #include <memory>
 #include "nss.h"
+#include "prerror.h"
 #include "pk11pub.h"
 #include "sechash.h"
 #include "cryptohi.h"
 
 #include "cpputil.h"
 #include "databuffer.h"
+#include "pk11_signature_test.h"
 
 #include "gtest/gtest.h"
 #include "nss_scoped_ptrs.h"
@@ -19,59 +21,59 @@
 #include "testvectors/dsa-vectors.h"
 
 namespace nss_test {
-
-class Pkcs11DsaTest : public ::testing::TestWithParam<DsaTestVector> {
+CK_MECHANISM_TYPE
+DsaHashToComboMech(SECOidTag hash) {
+  switch (hash) {
+    case SEC_OID_SHA1:
+      return CKM_DSA_SHA1;
+    case SEC_OID_SHA224:
+      return CKM_DSA_SHA224;
+    case SEC_OID_SHA256:
+      return CKM_DSA_SHA256;
+    case SEC_OID_SHA384:
+      return CKM_DSA_SHA384;
+    case SEC_OID_SHA512:
+      return CKM_DSA_SHA512;
+    default:
+      break;
+  }
+  return CKM_INVALID_MECHANISM;
+}
+
+class Pkcs11DsaTestBase : public Pk11SignatureTest {
  protected:
-  void Derive(const uint8_t* sig, size_t sig_len, const uint8_t* spki,
-              size_t spki_len, const uint8_t* data, size_t data_len,
-              bool expect_success, const uint32_t test_id,
-              const SECOidTag hash_oid) {
-    std::stringstream s;
-    s << "Test with original ID #" << test_id << " failed.\n";
-    s << "Expected Success: " << expect_success << "\n";
-    std::string msg = s.str();
-
-    SECItem spki_item = {siBuffer, toUcharPtr(spki),
-                         static_cast<unsigned int>(spki_len)};
-
-    ScopedCERTSubjectPublicKeyInfo cert_spki(
-        SECKEY_DecodeDERSubjectPublicKeyInfo(&spki_item));
-    ASSERT_TRUE(cert_spki) << msg;
-
-    ScopedSECKEYPublicKey pub_key(SECKEY_ExtractPublicKey(cert_spki.get()));
-    ASSERT_TRUE(pub_key) << msg;
-
-    SECItem sig_item = {siBuffer, toUcharPtr(sig),
-                        static_cast<unsigned int>(sig_len)};
-    ScopedSECItem decoded_sig_item(
-        DSAU_DecodeDerSigToLen(&sig_item, SECKEY_SignatureLen(pub_key.get())));
-    if (!decoded_sig_item) {
-      ASSERT_FALSE(expect_success) << msg;
+  Pkcs11DsaTestBase(SECOidTag hashOid)
+      : Pk11SignatureTest(CKM_DSA, hashOid, DsaHashToComboMech(hashOid)) {}
+
+  void Verify(const DsaTestVector vec) {
+    /* DSA vectors encode the signature in DER, we need to unwrap it before
+     * we can send the raw signatures to PKCS #11. */
+    DataBuffer pubKeyBuffer(vec.public_key.data(), vec.public_key.size());
+    ScopedSECKEYPublicKey nssPubKey(ImportPublicKey(pubKeyBuffer));
+    SECItem sigItem = {siBuffer, toUcharPtr(vec.sig.data()),
+                       static_cast<unsigned int>(vec.sig.size())};
+    ScopedSECItem decodedSigItem(
+        DSAU_DecodeDerSigToLen(&sigItem, SECKEY_SignatureLen(nssPubKey.get())));
+    if (!decodedSigItem) {
+      ASSERT_FALSE(vec.valid) << "Failed to decode DSA signature Error: "
+                              << PORT_ErrorToString(PORT_GetError()) << "\n";
       return;
     }
 
-    DataBuffer hash;
-    hash.Allocate(static_cast<size_t>(HASH_ResultLenByOidTag(hash_oid)));
-    SECStatus rv = PK11_HashBuf(hash_oid, toUcharPtr(hash.data()),
-                                toUcharPtr(data), data_len);
-    ASSERT_EQ(SECSuccess, rv) << msg;
-
-    // Verify.
-    SECItem hash_item = {siBuffer, toUcharPtr(hash.data()),
-                         static_cast<unsigned int>(hash.len())};
-    rv = PK11_VerifyWithMechanism(pub_key.get(), CKM_DSA, nullptr,
-                                  decoded_sig_item.get(), &hash_item, nullptr);
-    EXPECT_EQ(expect_success ? SECSuccess : SECFailure, rv);
-  };
+    Pkcs11SignatureTestParams params = {
+        DataBuffer(), pubKeyBuffer, DataBuffer(vec.msg.data(), vec.msg.size()),
+        DataBuffer(decodedSigItem.get()->data, decodedSigItem.get()->len)};
+    Pk11SignatureTest::Verify(params, (bool)vec.valid);
+  }
+};
 
-  void Derive(const DsaTestVector vector) {
-    Derive(vector.sig.data(), vector.sig.size(), vector.public_key.data(),
-           vector.public_key.size(), vector.msg.data(), vector.msg.size(),
-           vector.valid, vector.id, vector.hash_oid);
-  };
+class Pkcs11DsaTest : public Pkcs11DsaTestBase,
+                      public ::testing::WithParamInterface<DsaTestVector> {
+ public:
+  Pkcs11DsaTest() : Pkcs11DsaTestBase(GetParam().hash_oid) {}
 };
 
-TEST_P(Pkcs11DsaTest, WycheproofVectors) { Derive(GetParam()); }
+TEST_P(Pkcs11DsaTest, WycheproofVectors) { Verify(GetParam()); }
 
 INSTANTIATE_TEST_SUITE_P(DsaTest, Pkcs11DsaTest,
                          ::testing::ValuesIn(kDsaWycheproofVectors));
diff --git a/gtests/pk11_gtest/pk11_ecdsa_unittest.cc b/gtests/pk11_gtest/pk11_ecdsa_unittest.cc
index c127004c8..9f4bf6a2b 100644
--- a/gtests/pk11_gtest/pk11_ecdsa_unittest.cc
+++ b/gtests/pk11_gtest/pk11_ecdsa_unittest.cc
@@ -8,6 +8,7 @@
 #include "sechash.h"
 #include "cryptohi.h"
 
+#include "cpputil.h"
 #include "gtest/gtest.h"
 #include "nss_scoped_ptrs.h"
 
@@ -19,10 +20,29 @@
 
 namespace nss_test {
 
+CK_MECHANISM_TYPE
+EcHashToComboMech(SECOidTag hash) {
+  switch (hash) {
+    case SEC_OID_SHA1:
+      return CKM_ECDSA_SHA1;
+    case SEC_OID_SHA224:
+      return CKM_ECDSA_SHA224;
+    case SEC_OID_SHA256:
+      return CKM_ECDSA_SHA256;
+    case SEC_OID_SHA384:
+      return CKM_ECDSA_SHA384;
+    case SEC_OID_SHA512:
+      return CKM_ECDSA_SHA512;
+    default:
+      break;
+  }
+  return CKM_INVALID_MECHANISM;
+}
+
 class Pkcs11EcdsaTestBase : public Pk11SignatureTest {
  protected:
   Pkcs11EcdsaTestBase(SECOidTag hash_oid)
-      : Pk11SignatureTest(CKM_ECDSA, hash_oid) {}
+      : Pk11SignatureTest(CKM_ECDSA, hash_oid, EcHashToComboMech(hash_oid)) {}
 };
 
 struct Pkcs11EcdsaTestParams {
@@ -88,7 +108,8 @@ TEST_F(Pkcs11EcdsaSha256Test, ImportOnlyAlgorithmParams) {
                sizeof(kP256Pkcs8OnlyAlgorithmParams));
   DataBuffer data(kP256Data, sizeof(kP256Data));
   DataBuffer sig;
-  EXPECT_TRUE(ImportPrivateKeyAndSignHashedData(k, data, &sig));
+  DataBuffer sig2;
+  EXPECT_TRUE(ImportPrivateKeyAndSignHashedData(k, data, &sig, &sig2));
 };
 
 // Importing a private key in PKCS#8 format must succeed when the outer AlgID
@@ -99,7 +120,8 @@ TEST_F(Pkcs11EcdsaSha256Test, ImportMatchingCurveOIDAndAlgorithmParams) {
                sizeof(kP256Pkcs8MatchingCurveOIDAndAlgorithmParams));
   DataBuffer data(kP256Data, sizeof(kP256Data));
   DataBuffer sig;
-  EXPECT_TRUE(ImportPrivateKeyAndSignHashedData(k, data, &sig));
+  DataBuffer sig2;
+  EXPECT_TRUE(ImportPrivateKeyAndSignHashedData(k, data, &sig, &sig2));
 };
 
 // Importing a private key in PKCS#8 format must succeed when the outer AlgID
@@ -110,7 +132,8 @@ TEST_F(Pkcs11EcdsaSha256Test, ImportDissimilarCurveOIDAndAlgorithmParams) {
                sizeof(kP256Pkcs8DissimilarCurveOIDAndAlgorithmParams));
   DataBuffer data(kP256Data, sizeof(kP256Data));
   DataBuffer sig;
-  EXPECT_TRUE(ImportPrivateKeyAndSignHashedData(k, data, &sig));
+  DataBuffer sig2;
+  EXPECT_TRUE(ImportPrivateKeyAndSignHashedData(k, data, &sig, &sig2));
 };
 
 // Importing a private key in PKCS#8 format must fail when the outer ASN.1
diff --git a/gtests/pk11_gtest/pk11_gtest.gyp b/gtests/pk11_gtest/pk11_gtest.gyp
index 4171ea382..44778f51b 100644
--- a/gtests/pk11_gtest/pk11_gtest.gyp
+++ b/gtests/pk11_gtest/pk11_gtest.gyp
@@ -44,6 +44,7 @@
         'pk11_rsapkcs1_unittest.cc',
         'pk11_rsapss_unittest.cc',
         'pk11_seed_cbc_unittest.cc',
+        'pk11_signature_test.cc',
         '<(DEPTH)/gtests/common/gtests.cc'
       ],
       'dependencies': [
diff --git a/gtests/pk11_gtest/pk11_rsapkcs1_unittest.cc b/gtests/pk11_gtest/pk11_rsapkcs1_unittest.cc
index db31f0dac..7ae718982 100644
--- a/gtests/pk11_gtest/pk11_rsapkcs1_unittest.cc
+++ b/gtests/pk11_gtest/pk11_rsapkcs1_unittest.cc
@@ -15,6 +15,7 @@
 #include "pk11pub.h"
 #include "secerr.h"
 #include "sechash.h"
+#include "pk11_signature_test.h"
 
 #include "testvectors/rsa_signature_2048_sha224-vectors.h"
 #include "testvectors/rsa_signature_2048_sha256-vectors.h"
@@ -28,10 +29,46 @@
 
 namespace nss_test {
 
+CK_MECHANISM_TYPE RsaHashToComboMech(SECOidTag hash) {
+  switch (hash) {
+    case SEC_OID_SHA1:
+      return CKM_SHA1_RSA_PKCS;
+    case SEC_OID_SHA224:
+      return CKM_SHA224_RSA_PKCS;
+    case SEC_OID_SHA256:
+      return CKM_SHA256_RSA_PKCS;
+    case SEC_OID_SHA384:
+      return CKM_SHA384_RSA_PKCS;
+    case SEC_OID_SHA512:
+      return CKM_SHA512_RSA_PKCS;
+    default:
+      break;
+  }
+  return CKM_INVALID_MECHANISM;
+}
+
+class Pkcs11RsaBaseTest : public Pk11SignatureTest {
+ protected:
+  Pkcs11RsaBaseTest(SECOidTag hashOid)
+      : Pk11SignatureTest(CKM_RSA_PKCS, hashOid, RsaHashToComboMech(hashOid)) {}
+
+  void Verify(const RsaSignatureTestVector vec) {
+    Pkcs11SignatureTestParams params = {
+        DataBuffer(), DataBuffer(vec.public_key.data(), vec.public_key.size()),
+        DataBuffer(vec.msg.data(), vec.msg.size()),
+        DataBuffer(vec.sig.data(), vec.sig.size())};
+    Pk11SignatureTest::Verify(params, (bool)vec.valid);
+  }
+};
+
 class Pkcs11RsaPkcs1WycheproofTest
-    : public ::testing::TestWithParam<RsaSignatureTestVector> {
+    : public Pkcs11RsaBaseTest,
+      public ::testing::WithParamInterface<RsaSignatureTestVector> {
+ public:
+  Pkcs11RsaPkcs1WycheproofTest() : Pkcs11RsaBaseTest(GetParam().hash_oid) {}
+
  protected:
-  void Derive(const RsaSignatureTestVector vec) {
+  void Verify1(const RsaSignatureTestVector vec) {
     SECItem spki_item = {siBuffer, toUcharPtr(vec.public_key.data()),
                          static_cast<unsigned int>(vec.public_key.size())};
 
@@ -210,7 +247,13 @@ TEST(RsaPkcs1Test, RequireNullParameter) {
 #endif
 }
 
-TEST_P(Pkcs11RsaPkcs1WycheproofTest, Verify) { Derive(GetParam()); }
+TEST_P(Pkcs11RsaPkcs1WycheproofTest, Verify) {
+  /* Using VFY_ interface */
+  Verify1(GetParam());
+  /* Using PKCS #11 interface */
+  setSkipRaw(true);
+  Verify(GetParam());
+}
 
 INSTANTIATE_TEST_SUITE_P(
     Wycheproof2048RsaSignatureSha224Test, Pkcs11RsaPkcs1WycheproofTest,
diff --git a/gtests/pk11_gtest/pk11_rsapss_unittest.cc b/gtests/pk11_gtest/pk11_rsapss_unittest.cc
index 06c3ae32a..e8428f794 100644
--- a/gtests/pk11_gtest/pk11_rsapss_unittest.cc
+++ b/gtests/pk11_gtest/pk11_rsapss_unittest.cc
@@ -9,7 +9,6 @@
 #include "pk11pub.h"
 #include "sechash.h"
 
-#include "cpputil.h"
 #include "databuffer.h"
 
 #include "gtest/gtest.h"
@@ -28,83 +27,68 @@
 
 namespace nss_test {
 
-class Pkcs11RsaPssTestWycheproof
-    : public ::testing::TestWithParam<RsaPssTestVector> {
- protected:
-  void TestPss(const RsaPssTestVector& vec) {
-    SECItem spki_item = {siBuffer, toUcharPtr(vec.public_key.data()),
-                         static_cast<unsigned int>(vec.public_key.size())};
-
-    ScopedCERTSubjectPublicKeyInfo cert_spki(
-        SECKEY_DecodeDERSubjectPublicKeyInfo(&spki_item));
-    ASSERT_TRUE(cert_spki);
-
-    ScopedSECKEYPublicKey pub_key(SECKEY_ExtractPublicKey(cert_spki.get()));
-    ASSERT_TRUE(pub_key);
-
-    DataBuffer hash;
-    hash.Allocate(static_cast<size_t>(HASH_ResultLenByOidTag(vec.hash_oid)));
-    SECStatus rv = PK11_HashBuf(vec.hash_oid, toUcharPtr(hash.data()),
-                                toUcharPtr(vec.msg.data()), vec.msg.size());
-    ASSERT_EQ(rv, SECSuccess);
-
-    // Verify.
-    SECItem hash_item = {siBuffer, toUcharPtr(hash.data()),
-                         static_cast<unsigned int>(hash.len())};
-    SECItem sig_item = {siBuffer, toUcharPtr(vec.sig.data()),
-                        static_cast<unsigned int>(vec.sig.size())};
-    CK_MECHANISM_TYPE hash_mech = 0;
-    switch (vec.hash_oid) {
-      case SEC_OID_SHA1:
-        hash_mech = CKM_SHA_1;
-        break;
-      case SEC_OID_SHA224:
-        hash_mech = CKM_SHA224;
-        break;
-      case SEC_OID_SHA256:
-        hash_mech = CKM_SHA256;
-        break;
-      case SEC_OID_SHA384:
-        hash_mech = CKM_SHA384;
-        break;
-      case SEC_OID_SHA512:
-        hash_mech = CKM_SHA512;
-        break;
-      default:
-        ASSERT_TRUE(hash_mech);
-        return;
-    }
-
-    CK_RSA_PKCS_PSS_PARAMS pss_params = {hash_mech, vec.mgf_hash, vec.sLen};
-    SECItem params = {siBuffer, reinterpret_cast<unsigned char*>(&pss_params),
-                      sizeof(pss_params)};
-
-    rv = PK11_VerifyWithMechanism(pub_key.get(), CKM_RSA_PKCS_PSS, &params,
-                                  &sig_item, &hash_item, nullptr);
-    EXPECT_EQ(vec.valid ? SECSuccess : SECFailure, rv);
-  };
-};
+CK_MECHANISM_TYPE RsaPssMapCombo(SECOidTag hashOid) {
+  switch (hashOid) {
+    case SEC_OID_SHA1:
+      return CKM_SHA1_RSA_PKCS_PSS;
+    case SEC_OID_SHA224:
+      return CKM_SHA224_RSA_PKCS_PSS;
+    case SEC_OID_SHA256:
+      return CKM_SHA256_RSA_PKCS_PSS;
+    case SEC_OID_SHA384:
+      return CKM_SHA384_RSA_PKCS_PSS;
+    case SEC_OID_SHA512:
+      return CKM_SHA512_RSA_PKCS_PSS;
+    default:
+      break;
+  }
+  return CKM_INVALID_MECHANISM;
+}
 
-class Pkcs11RsaPssTest : public Pk11SignatureTest {
- public:
-  Pkcs11RsaPssTest() : Pk11SignatureTest(CKM_RSA_PKCS_PSS, SEC_OID_SHA1) {
-    pss_params_.hashAlg = CKM_SHA_1;
-    pss_params_.mgf = CKG_MGF1_SHA1;
-    pss_params_.sLen = HASH_ResultLenByOidTag(SEC_OID_SHA1);
+class Pkcs11RsaPssTestBase : public Pk11SignatureTest {
+ protected:
+  Pkcs11RsaPssTestBase(SECOidTag hashOid, CK_RSA_PKCS_MGF_TYPE mgf, int sLen)
+      : Pk11SignatureTest(CKM_RSA_PKCS_PSS, hashOid, RsaPssMapCombo(hashOid)) {
+    pss_params_.hashAlg = PK11_AlgtagToMechanism(hashOid);
+    pss_params_.mgf = mgf;
+    pss_params_.sLen = sLen;
 
     params_.type = siBuffer;
     params_.data = reinterpret_cast<unsigned char*>(&pss_params_);
     params_.len = sizeof(pss_params_);
   }
 
- protected:
   const SECItem* parameters() const { return &params_; }
 
+  void Verify(const RsaPssTestVector& vec) {
+    Pkcs11SignatureTestParams params = {
+        DataBuffer(), DataBuffer(vec.public_key.data(), vec.public_key.size()),
+        DataBuffer(vec.msg.data(), vec.msg.size()),
+        DataBuffer(vec.sig.data(), vec.sig.size())};
+
+    Pk11SignatureTest::Verify(params, vec.valid);
+  }
+
  private:
   CK_RSA_PKCS_PSS_PARAMS pss_params_;
   SECItem params_;
 };
 
+class Pkcs11RsaPssTest : public Pkcs11RsaPssTestBase {
+ public:
+  Pkcs11RsaPssTest()
+      : Pkcs11RsaPssTestBase(SEC_OID_SHA1, CKG_MGF1_SHA1, SHA1_LENGTH) {}
+};
+
+class Pkcs11RsaPssTestWycheproof
+    : public Pkcs11RsaPssTestBase,
+      public ::testing::WithParamInterface<RsaPssTestVector> {
+ public:
+  Pkcs11RsaPssTestWycheproof()
+      : Pkcs11RsaPssTestBase(GetParam().hash_oid, GetParam().mgf_hash,
+                             GetParam().sLen) {}
+};
+
 TEST_F(Pkcs11RsaPssTest, GenerateAndSignAndVerify) {
   // Sign data with a 1024-bit RSA key, using PSS/SHA-256.
   SECOidTag hashOid = SEC_OID_SHA256;
@@ -179,7 +163,9 @@ class Pkcs11RsaPssVectorTest
     : public Pkcs11RsaPssTest,
       public ::testing::WithParamInterface<Pkcs11SignatureTestParams> {};
 
-TEST_P(Pkcs11RsaPssVectorTest, Verify) { Verify(GetParam()); }
+TEST_P(Pkcs11RsaPssVectorTest, Verify) {
+  Pk11SignatureTest::Verify(GetParam());
+}
 
 TEST_P(Pkcs11RsaPssVectorTest, SignAndVerify) { SignAndVerify(GetParam()); }
 
@@ -227,7 +213,7 @@ static const Pkcs11SignatureTestParams kRsaPssVectors[] = {
 INSTANTIATE_TEST_SUITE_P(RsaPssSignVerify, Pkcs11RsaPssVectorTest,
                          ::testing::ValuesIn(kRsaPssVectors));
 
-TEST_P(Pkcs11RsaPssTestWycheproof, Verify) { TestPss(GetParam()); }
+TEST_P(Pkcs11RsaPssTestWycheproof, Verify) { Verify(GetParam()); }
 
 INSTANTIATE_TEST_SUITE_P(
     Wycheproof2048RsaPssSha120Test, Pkcs11RsaPssTestWycheproof,
diff --git a/gtests/pk11_gtest/pk11_signature_test.cc b/gtests/pk11_gtest/pk11_signature_test.cc
new file mode 100644
index 000000000..7ef51812c
--- /dev/null
+++ b/gtests/pk11_gtest/pk11_signature_test.cc
@@ -0,0 +1,181 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include <memory>
+#include "nss.h"
+#include "pk11pub.h"
+#include "sechash.h"
+#include "prerror.h"
+
+#include "cpputil.h"
+#include "nss_scoped_ptrs.h"
+#include "databuffer.h"
+
+#include "gtest/gtest.h"
+#include "pk11_signature_test.h"
+
+namespace nss_test {
+
+ScopedSECKEYPrivateKey Pk11SignatureTest::ImportPrivateKey(
+    const DataBuffer& pkcs8) {
+  ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
+  if (!slot) {
+    ADD_FAILURE() << "No slot";
+    return nullptr;
+  }
+
+  SECItem pkcs8Item = {siBuffer, toUcharPtr(pkcs8.data()),
+                       static_cast<unsigned int>(pkcs8.len())};
+
+  SECKEYPrivateKey* key = nullptr;
+  SECStatus rv = PK11_ImportDERPrivateKeyInfoAndReturnKey(
+      slot.get(), &pkcs8Item, nullptr, nullptr, false, false, KU_ALL, &key,
+      nullptr);
+
+  if (rv != SECSuccess) {
+    return nullptr;
+  }
+
+  return ScopedSECKEYPrivateKey(key);
+}
+
+ScopedSECKEYPublicKey Pk11SignatureTest::ImportPublicKey(
+    const DataBuffer& spki) {
+  SECItem spkiItem = {siBuffer, toUcharPtr(spki.data()),
+                      static_cast<unsigned int>(spki.len())};
+
+  ScopedCERTSubjectPublicKeyInfo certSpki(
+      SECKEY_DecodeDERSubjectPublicKeyInfo(&spkiItem));
+  if (!certSpki) {
+    return nullptr;
+  }
+
+  return ScopedSECKEYPublicKey(SECKEY_ExtractPublicKey(certSpki.get()));
+}
+
+bool Pk11SignatureTest::SignHashedData(ScopedSECKEYPrivateKey& privKey,
+                                       const DataBuffer& hash,
+                                       DataBuffer* sig) {
+  SECItem hashItem = {siBuffer, toUcharPtr(hash.data()),
+                      static_cast<unsigned int>(hash.len())};
+  unsigned int sigLen = PK11_SignatureLen(privKey.get());
+  EXPECT_LT(0, (int)sigLen);
+  sig->Allocate(static_cast<size_t>(sigLen));
+  SECItem sigItem = {siBuffer, toUcharPtr(sig->data()),
+                     static_cast<unsigned int>(sig->len())};
+  SECStatus rv = PK11_SignWithMechanism(privKey.get(), mechanism_, parameters(),
+                                        &sigItem, &hashItem);
+  EXPECT_EQ(sigLen, sigItem.len);
+  return rv == SECSuccess;
+}
+
+bool Pk11SignatureTest::SignData(ScopedSECKEYPrivateKey& privKey,
+                                 const DataBuffer& data, DataBuffer* sig) {
+  unsigned int sigLen = PK11_SignatureLen(privKey.get());
+  bool result = true;
+  EXPECT_LT(0, (int)sigLen);
+  sig->Allocate(static_cast<size_t>(sigLen));
+
+  // test the hash and verify interface */
+  PK11Context* context = PK11_CreateContextByPrivKey(
+      combo_, CKA_SIGN, privKey.get(), parameters());
+  if (context == NULL) {
+    ADD_FAILURE() << "Failed to sign data: couldn't create context"
+                  << "\n"
+                  << "mech=0x" << std::hex << combo_ << "\n"
+                  << "Error: " << PORT_ErrorToString(PORT_GetError());
+    return false;
+  }
+  SECStatus rv = PK11_DigestOp(context, data.data(), data.len());
+  if (rv != SECSuccess) {
+    ADD_FAILURE() << "Failed to sign data: Update failed\n"
+                  << "Error: " << PORT_ErrorToString(PORT_GetError());
+    PK11_DestroyContext(context, PR_TRUE);
+    return false;
+  }
+  unsigned int len = sigLen;
+  rv = PK11_DigestFinal(context, sig->data(), &len, sigLen);
+  if (rv != SECSuccess) {
+    ADD_FAILURE() << "Failed to sign data: final failed\n"
+                  << "Error: " << PORT_ErrorToString(PORT_GetError());
+    result = false;
+  }
+  if (len != sigLen) {
+    ADD_FAILURE() << "sign data: unexpected len " << len << "expected"
+                  << sigLen;
+    result = false;
+  }
+  PK11_DestroyContext(context, PR_TRUE);
+  return result;
+}
+
+bool Pk11SignatureTest::ImportPrivateKeyAndSignHashedData(
+    const DataBuffer& pkcs8, const DataBuffer& data, DataBuffer* sig,
+    DataBuffer* sig2) {
+  ScopedSECKEYPrivateKey privKey(ImportPrivateKey(pkcs8));
+  if (!privKey) {
+    return false;
+  }
+
+  DataBuffer hash;
+  if (!ComputeHash(data, &hash)) {
+    ADD_FAILURE() << "Failed to compute hash";
+    return false;
+  }
+  if (!SignHashedData(privKey, hash, sig)) {
+    ADD_FAILURE() << "Failed to sign hashed data";
+    return false;
+  }
+  if (!SignData(privKey, data, sig2)) {
+    /* failure was already added by SignData, with an error message */
+    return false;
+  }
+  return true;
+}
+
+void Pk11SignatureTest::Verify(const Pkcs11SignatureTestParams& params,
+                               const DataBuffer& sig, bool valid) {
+  ScopedSECKEYPublicKey pubKey(ImportPublicKey(params.spki_));
+  ASSERT_TRUE(pubKey);
+
+  SECStatus rv;
+  DataBuffer hash;
+
+  SECItem sigItem = {siBuffer, toUcharPtr(sig.data()),
+                     static_cast<unsigned int>(sig.len())};
+
+  /* RSA single shot requires encoding the hash before calling
+   * VerifyWithMechanism. We already check that mechanism
+   * with the VFY_ interface, so just do the combined hash/Verify
+   * in that case */
+  if (!skip_raw_) {
+    ASSERT_TRUE(ComputeHash(params.data_, &hash));
+
+    // Verify.
+    SECItem hashItem = {siBuffer, toUcharPtr(hash.data()),
+                        static_cast<unsigned int>(hash.len())};
+    rv = PK11_VerifyWithMechanism(pubKey.get(), mechanism_, parameters(),
+                                  &sigItem, &hashItem, nullptr);
+    EXPECT_EQ(rv, valid ? SECSuccess : SECFailure);
+  }
+
+  // test the hash and verify interface */
+  PK11Context* context = PK11_CreateContextByPubKey(
+      combo_, CKA_VERIFY, pubKey.get(), parameters(), NULL);
+  /* we assert here because we'll crash if we try to continue
+   * without a context. */
+  ASSERT_NE((void*)context, (void*)NULL)
+      << "CreateContext failed Error:" << PORT_ErrorToString(PORT_GetError())
+      << "\n";
+  rv = PK11_DigestOp(context, params.data_.data(), params.data_.len());
+  /* expect success unconditionally here */
+  EXPECT_EQ(rv, SECSuccess);
+  unsigned int len;
+  rv = PK11_DigestFinal(context, sigItem.data, &len, sigItem.len);
+  EXPECT_EQ(rv, valid ? SECSuccess : SECFailure)
+      << "verify failed Error:" << PORT_ErrorToString(PORT_GetError()) << "\n";
+  PK11_DestroyContext(context, PR_TRUE);
+}
+
+}  // namespace nss_test
diff --git a/gtests/pk11_gtest/pk11_signature_test.h b/gtests/pk11_gtest/pk11_signature_test.h
index cd46f17d7..82ef03896 100644
--- a/gtests/pk11_gtest/pk11_signature_test.h
+++ b/gtests/pk11_gtest/pk11_signature_test.h
@@ -7,7 +7,6 @@
 #include "pk11pub.h"
 #include "sechash.h"
 
-#include "cpputil.h"
 #include "nss_scoped_ptrs.h"
 #include "databuffer.h"
 
@@ -25,46 +24,18 @@ struct Pkcs11SignatureTestParams {
 
 class Pk11SignatureTest : public ::testing::Test {
  protected:
-  Pk11SignatureTest(CK_MECHANISM_TYPE mech, SECOidTag hash_oid)
-      : mechanism_(mech), hash_oid_(hash_oid) {}
+  Pk11SignatureTest(CK_MECHANISM_TYPE mech, SECOidTag hash_oid,
+                    CK_MECHANISM_TYPE combo)
+      : mechanism_(mech), hash_oid_(hash_oid), combo_(combo) {
+    skip_raw_ = false;
+  }
 
   virtual const SECItem* parameters() const { return nullptr; }
   CK_MECHANISM_TYPE mechanism() const { return mechanism_; }
+  void setSkipRaw(bool skip_raw) { skip_raw_ = true; }
 
-  ScopedSECKEYPrivateKey ImportPrivateKey(const DataBuffer& pkcs8) {
-    ScopedPK11SlotInfo slot(PK11_GetInternalSlot());
-    if (!slot) {
-      ADD_FAILURE() << "No slot";
-      return nullptr;
-    }
-
-    SECItem pkcs8Item = {siBuffer, toUcharPtr(pkcs8.data()),
-                         static_cast<unsigned int>(pkcs8.len())};
-
-    SECKEYPrivateKey* key = nullptr;
-    SECStatus rv = PK11_ImportDERPrivateKeyInfoAndReturnKey(
-        slot.get(), &pkcs8Item, nullptr, nullptr, false, false, KU_ALL, &key,
-        nullptr);
-
-    if (rv != SECSuccess) {
-      return nullptr;
-    }
-
-    return ScopedSECKEYPrivateKey(key);
-  }
-
-  ScopedSECKEYPublicKey ImportPublicKey(const DataBuffer& spki) {
-    SECItem spkiItem = {siBuffer, toUcharPtr(spki.data()),
-                        static_cast<unsigned int>(spki.len())};
-
-    ScopedCERTSubjectPublicKeyInfo certSpki(
-        SECKEY_DecodeDERSubjectPublicKeyInfo(&spkiItem));
-    if (!certSpki) {
-      return nullptr;
-    }
-
-    return ScopedSECKEYPublicKey(SECKEY_ExtractPublicKey(certSpki.get()));
-  }
+  ScopedSECKEYPrivateKey ImportPrivateKey(const DataBuffer& pkcs8);
+  ScopedSECKEYPublicKey ImportPublicKey(const DataBuffer& spki);
 
   bool ComputeHash(const DataBuffer& data, DataBuffer* hash) {
     hash->Allocate(static_cast<size_t>(HASH_ResultLenByOidTag(hash_oid_)));
@@ -74,66 +45,37 @@ class Pk11SignatureTest : public ::testing::Test {
   }
 
   bool SignHashedData(ScopedSECKEYPrivateKey& privKey, const DataBuffer& hash,
-                      DataBuffer* sig) {
-    SECItem hashItem = {siBuffer, toUcharPtr(hash.data()),
-                        static_cast<unsigned int>(hash.len())};
-    int sigLen = PK11_SignatureLen(privKey.get());
-    EXPECT_LT(0, sigLen);
-    sig->Allocate(static_cast<size_t>(sigLen));
-    SECItem sigItem = {siBuffer, toUcharPtr(sig->data()),
-                       static_cast<unsigned int>(sig->len())};
-    SECStatus rv = PK11_SignWithMechanism(privKey.get(), mechanism_,
-                                          parameters(), &sigItem, &hashItem);
-    return rv == SECSuccess;
-  }
-
+                      DataBuffer* sig);
+  bool SignData(ScopedSECKEYPrivateKey& privKey, const DataBuffer& data,
+                DataBuffer* sig);
   bool ImportPrivateKeyAndSignHashedData(const DataBuffer& pkcs8,
                                          const DataBuffer& data,
-                                         DataBuffer* sig) {
-    ScopedSECKEYPrivateKey privKey(ImportPrivateKey(pkcs8));
-    if (!privKey) {
-      return false;
-    }
-
-    DataBuffer hash;
-    if (!ComputeHash(data, &hash)) {
-      ADD_FAILURE() << "Failed to compute hash";
-      return false;
-    }
-    return SignHashedData(privKey, hash, sig);
-  }
-
-  void Verify(const Pkcs11SignatureTestParams& params, const DataBuffer& sig) {
-    ScopedSECKEYPublicKey pubKey(ImportPublicKey(params.spki_));
-    ASSERT_TRUE(pubKey);
-
-    DataBuffer hash;
-    ASSERT_TRUE(ComputeHash(params.data_, &hash));
+                                         DataBuffer* sig, DataBuffer* sig2);
+  void Verify(const Pkcs11SignatureTestParams& params, const DataBuffer& sig,
+              bool valid);
 
-    // Verify.
-    SECItem hashItem = {siBuffer, toUcharPtr(hash.data()),
-                        static_cast<unsigned int>(hash.len())};
-    SECItem sigItem = {siBuffer, toUcharPtr(sig.data()),
-                       static_cast<unsigned int>(sig.len())};
-    SECStatus rv = PK11_VerifyWithMechanism(
-        pubKey.get(), mechanism_, parameters(), &sigItem, &hashItem, nullptr);
-    EXPECT_EQ(rv, SECSuccess);
+  void Verify(const Pkcs11SignatureTestParams& params, bool valid) {
+    Verify(params, params.signature_, valid);
   }
 
   void Verify(const Pkcs11SignatureTestParams& params) {
-    Verify(params, params.signature_);
+    Verify(params, params.signature_, true);
   }
 
   void SignAndVerify(const Pkcs11SignatureTestParams& params) {
     DataBuffer sig;
-    ASSERT_TRUE(
-        ImportPrivateKeyAndSignHashedData(params.pkcs8_, params.data_, &sig));
-    Verify(params, sig);
+    DataBuffer sig2;
+    ASSERT_TRUE(ImportPrivateKeyAndSignHashedData(params.pkcs8_, params.data_,
+                                                  &sig, &sig2));
+    Verify(params, sig, true);
+    Verify(params, sig2, true);
   }
 
  private:
   CK_MECHANISM_TYPE mechanism_;
   SECOidTag hash_oid_;
+  CK_MECHANISM_TYPE combo_;
+  bool skip_raw_;
 };
 
 }  // namespace nss_test