diff --git a/cc/signature/ecdsa_sign_key_manager.cc b/cc/signature/ecdsa_sign_key_manager.cc index 0234a6b4a1..5d066efbb8 100644 --- a/cc/signature/ecdsa_sign_key_manager.cc +++ b/cc/signature/ecdsa_sign_key_manager.cc @@ -214,7 +214,8 @@ EcdsaSignKeyManager::GetPrimitiveImpl( ec_key.pub_y = public_key.y(); ec_key.priv = ecdsa_private_key.key_value(); auto ecdsa_result = subtle::EcdsaSignBoringSsl::New( - ec_key, Enums::ProtoToSubtle(public_key.params().hash_type())); + ec_key, Enums::ProtoToSubtle(public_key.params().hash_type()), + Enums::ProtoToSubtle(public_key.params().encoding())); if (!ecdsa_result.ok()) return ecdsa_result.status(); std::unique_ptr ecdsa(ecdsa_result.ValueOrDie().release()); return std::move(ecdsa); diff --git a/cc/signature/ecdsa_sign_key_manager_test.cc b/cc/signature/ecdsa_sign_key_manager_test.cc index ac90eb2c00..bfaf798f26 100644 --- a/cc/signature/ecdsa_sign_key_manager_test.cc +++ b/cc/signature/ecdsa_sign_key_manager_test.cc @@ -133,18 +133,6 @@ TEST_F(EcdsaSignKeyManagerTest, testKeyMessageErrors) { result.status().error_message()); } - { // Bad encoding. - EcdsaPrivateKey key; - auto public_key = key.mutable_public_key(); - public_key->mutable_params()->set_encoding( - EcdsaSignatureEncoding::IEEE_P1363); - auto result = key_manager.GetPrimitive(key); - EXPECT_FALSE(result.ok()); - EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); - EXPECT_PRED_FORMAT2(testing::IsSubstring, "Only DER encoding", - result.status().error_message()); - } - { // Bad elliptic curve. EcdsaPrivateKey key; auto public_key = key.mutable_public_key(); @@ -198,28 +186,32 @@ TEST_F(EcdsaSignKeyManagerTest, testKeyMessageErrors) { } TEST_F(EcdsaSignKeyManagerTest, testPrimitives) { - std::string message = "some message to sign"; - EcdsaSignKeyManager sign_key_manager; - EcdsaPrivateKey key = test::GetEcdsaTestPrivateKey( - EllipticCurveType::NIST_P256, HashType::SHA256); - - { // Using Key proto. - auto result = sign_key_manager.GetPrimitive(key); - EXPECT_TRUE(result.ok()) << result.status(); - auto sign = std::move(result.ValueOrDie()); - auto signing_result = sign->Sign(message); - EXPECT_TRUE(signing_result.ok()) << signing_result.status(); - } - - { // Using KeyData proto. - KeyData key_data; - key_data.set_type_url(ecdsa_sign_key_type_); - key_data.set_value(key.SerializeAsString()); - auto result = sign_key_manager.GetPrimitive(key_data); - EXPECT_TRUE(result.ok()) << result.status(); - auto sign = std::move(result.ValueOrDie()); - auto signing_result = sign->Sign(message); - EXPECT_TRUE(signing_result.ok()) << signing_result.status(); + EcdsaSignatureEncoding encodings[2] = {EcdsaSignatureEncoding::DER, + EcdsaSignatureEncoding::IEEE_P1363}; + for (EcdsaSignatureEncoding encoding : encodings) { + std::string message = "some message to sign"; + EcdsaSignKeyManager sign_key_manager; + EcdsaPrivateKey key = test::GetEcdsaTestPrivateKey( + EllipticCurveType::NIST_P256, HashType::SHA256, encoding); + + { // Using Key proto. + auto result = sign_key_manager.GetPrimitive(key); + EXPECT_TRUE(result.ok()) << result.status(); + auto sign = std::move(result.ValueOrDie()); + auto signing_result = sign->Sign(message); + EXPECT_TRUE(signing_result.ok()) << signing_result.status(); + } + + { // Using KeyData proto. + KeyData key_data; + key_data.set_type_url(ecdsa_sign_key_type_); + key_data.set_value(key.SerializeAsString()); + auto result = sign_key_manager.GetPrimitive(key_data); + EXPECT_TRUE(result.ok()) << result.status(); + auto sign = std::move(result.ValueOrDie()); + auto signing_result = sign->Sign(message); + EXPECT_TRUE(signing_result.ok()) << signing_result.status(); + } } } @@ -332,10 +324,9 @@ TEST_F(EcdsaSignKeyManagerTest, testNewKeyErrors) { auto result = key_factory.NewKey(key_format); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); - EXPECT_PRED_FORMAT2(testing::IsSubstring, "Only DER encoding", + EXPECT_PRED_FORMAT2(testing::IsSubstring, "Unsupported signature encoding", result.status().error_message()); } - // Wrong curve params->set_encoding(EcdsaSignatureEncoding::DER); { diff --git a/cc/signature/ecdsa_verify_key_manager.cc b/cc/signature/ecdsa_verify_key_manager.cc index 8b74a7ead6..9b008b312f 100644 --- a/cc/signature/ecdsa_verify_key_manager.cc +++ b/cc/signature/ecdsa_verify_key_manager.cc @@ -144,7 +144,8 @@ EcdsaVerifyKeyManager::GetPrimitiveImpl( ec_key.pub_x = ecdsa_public_key.x(); ec_key.pub_y = ecdsa_public_key.y(); auto ecdsa_result = subtle::EcdsaVerifyBoringSsl::New( - ec_key, Enums::ProtoToSubtle(ecdsa_public_key.params().hash_type())); + ec_key, Enums::ProtoToSubtle(ecdsa_public_key.params().hash_type()), + Enums::ProtoToSubtle(ecdsa_public_key.params().encoding())); if (!ecdsa_result.ok()) return ecdsa_result.status(); std::unique_ptr ecdsa(ecdsa_result.ValueOrDie().release()); return std::move(ecdsa); @@ -152,9 +153,13 @@ EcdsaVerifyKeyManager::GetPrimitiveImpl( // static Status EcdsaVerifyKeyManager::Validate(const EcdsaParams& params) { - if (params.encoding() != EcdsaSignatureEncoding::DER) { - return Status(util::error::INVALID_ARGUMENT, - "Only DER encoding is supported."); + switch (params.encoding()) { + case EcdsaSignatureEncoding::DER: // fall through + case EcdsaSignatureEncoding::IEEE_P1363: + break; + default: + return ToStatusF(util::error::INVALID_ARGUMENT, + "Unsupported signature encoding: %d", params.encoding()); } switch (params.curve()) { case EllipticCurveType::NIST_P256: diff --git a/cc/signature/ecdsa_verify_key_manager_test.cc b/cc/signature/ecdsa_verify_key_manager_test.cc index 5a1fd55c26..7a0520fb86 100644 --- a/cc/signature/ecdsa_verify_key_manager_test.cc +++ b/cc/signature/ecdsa_verify_key_manager_test.cc @@ -115,16 +115,6 @@ TEST_F(EcdsaVerifyKeyManagerTest, testKeyMessageErrors) { result.status().error_message()); } - { // Bad encoding. - EcdsaPublicKey key; - key.mutable_params()->set_encoding(EcdsaSignatureEncoding::IEEE_P1363); - auto result = key_manager.GetPrimitive(key); - EXPECT_FALSE(result.ok()); - EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); - EXPECT_PRED_FORMAT2(testing::IsSubstring, "Only DER encoding", - result.status().error_message()); - } - { // Bad elliptic curve. EcdsaPublicKey key; key.mutable_params()->set_encoding(EcdsaSignatureEncoding::DER); @@ -174,33 +164,49 @@ TEST_F(EcdsaVerifyKeyManagerTest, testKeyMessageErrors) { } TEST_F(EcdsaVerifyKeyManagerTest, testPrimitives) { - std::string message = "some message to sign"; - EcdsaSignKeyManager sign_key_manager; - EcdsaVerifyKeyManager verify_key_manager; - EcdsaPrivateKey private_key = test::GetEcdsaTestPrivateKey( - EllipticCurveType::NIST_P256, HashType::SHA256); - EcdsaPublicKey key = private_key.public_key(); - auto sign = std::move( - sign_key_manager.GetPrimitive(private_key).ValueOrDie()); - std::string signature = sign->Sign(message).ValueOrDie(); - - { // Using Key proto. - auto result = verify_key_manager.GetPrimitive(key); - EXPECT_TRUE(result.ok()) << result.status(); - auto verify = std::move(result.ValueOrDie()); - auto verify_status = verify->Verify(signature, message); - EXPECT_TRUE(verify_status.ok()) << verify_status; - } + EcdsaSignatureEncoding encodings[2] = {EcdsaSignatureEncoding::DER, + EcdsaSignatureEncoding::IEEE_P1363}; + for (EcdsaSignatureEncoding encoding : encodings) { + std::string message = "some message to sign"; + EcdsaSignKeyManager sign_key_manager; + EcdsaVerifyKeyManager verify_key_manager; + EcdsaPrivateKey private_key = test::GetEcdsaTestPrivateKey( + EllipticCurveType::NIST_P256, HashType::SHA256, encoding); + EcdsaPublicKey key = private_key.public_key(); + auto sign = + std::move(sign_key_manager.GetPrimitive(private_key).ValueOrDie()); + std::string signature = sign->Sign(message).ValueOrDie(); - { // Using KeyData proto. - KeyData key_data; - key_data.set_type_url(ecdsa_verify_key_type_); - key_data.set_value(key.SerializeAsString()); - auto result = verify_key_manager.GetPrimitive(key_data); - EXPECT_TRUE(result.ok()) << result.status(); - auto verify = std::move(result.ValueOrDie()); - auto verify_status = verify->Verify(signature, message); - EXPECT_TRUE(verify_status.ok()) << verify_status; + { // Using Key proto. + auto result = verify_key_manager.GetPrimitive(key); + EXPECT_TRUE(result.ok()) << result.status(); + auto verify = std::move(result.ValueOrDie()); + auto verify_status = verify->Verify(signature, message); + EXPECT_TRUE(verify_status.ok()) << verify_status; + } + + { // Using KeyData proto. + KeyData key_data; + key_data.set_type_url(ecdsa_verify_key_type_); + key_data.set_value(key.SerializeAsString()); + auto result = verify_key_manager.GetPrimitive(key_data); + EXPECT_TRUE(result.ok()) << result.status(); + auto verify = std::move(result.ValueOrDie()); + auto verify_status = verify->Verify(signature, message); + EXPECT_TRUE(verify_status.ok()) << verify_status; + } + + { // Using Key proto with wrong encoding. + auto params = key.mutable_params(); + params->set_encoding(encoding == EcdsaSignatureEncoding::DER + ? EcdsaSignatureEncoding::IEEE_P1363 + : EcdsaSignatureEncoding::DER); + auto result = verify_key_manager.GetPrimitive(key); + EXPECT_TRUE(result.ok()) << result.status(); + auto verify = std::move(result.ValueOrDie()); + auto verify_status = verify->Verify(signature, message); + EXPECT_FALSE(verify_status.ok()) << verify_status; + } } } diff --git a/cc/signature/public_key_sign_factory_test.cc b/cc/signature/public_key_sign_factory_test.cc index 2ecf7aa822..b63a9bb4f1 100644 --- a/cc/signature/public_key_sign_factory_test.cc +++ b/cc/signature/public_key_sign_factory_test.cc @@ -33,6 +33,7 @@ using crypto::tink::KeysetUtil; using crypto::tink::test::AddTinkKey; using google::crypto::tink::EcdsaPrivateKey; +using google::crypto::tink::EcdsaSignatureEncoding; using google::crypto::tink::EllipticCurveType; using google::crypto::tink::HashType; using google::crypto::tink::KeyData; @@ -52,8 +53,9 @@ class PublicKeySignFactoryTest : public ::testing::Test { }; EcdsaPrivateKey GetNewEcdsaPrivateKey() { - return test::GetEcdsaTestPrivateKey( - EllipticCurveType::NIST_P256, HashType::SHA256); + return test::GetEcdsaTestPrivateKey(EllipticCurveType::NIST_P256, + HashType::SHA256, + EcdsaSignatureEncoding::DER); } TEST_F(PublicKeySignFactoryTest, testBasic) { diff --git a/cc/signature/public_key_verify_factory_test.cc b/cc/signature/public_key_verify_factory_test.cc index a3a0ded23e..b2501efa86 100644 --- a/cc/signature/public_key_verify_factory_test.cc +++ b/cc/signature/public_key_verify_factory_test.cc @@ -33,6 +33,7 @@ using crypto::tink::KeysetUtil; using crypto::tink::test::AddTinkKey; using google::crypto::tink::EcdsaPublicKey; +using google::crypto::tink::EcdsaSignatureEncoding; using google::crypto::tink::EllipticCurveType; using google::crypto::tink::HashType; using google::crypto::tink::KeyData; @@ -52,8 +53,9 @@ class PublicKeyVerifyFactoryTest : public ::testing::Test { }; EcdsaPublicKey GetNewEcdsaPublicKey() { - auto ecdsa_key = test::GetEcdsaTestPrivateKey( - EllipticCurveType::NIST_P256, HashType::SHA256); + auto ecdsa_key = test::GetEcdsaTestPrivateKey(EllipticCurveType::NIST_P256, + HashType::SHA256, + EcdsaSignatureEncoding::DER); return ecdsa_key.public_key(); } diff --git a/cc/subtle/BUILD.bazel b/cc/subtle/BUILD.bazel index ec540dc544..7c2c8b64f5 100644 --- a/cc/subtle/BUILD.bazel +++ b/cc/subtle/BUILD.bazel @@ -476,6 +476,7 @@ cc_test( copts = ["-Iexternal/gtest/include"], deps = [ ":common_enums", + ":ec_util", ":ecdsa_sign_boringssl", ":ecdsa_verify_boringssl", "//cc:public_key_sign", diff --git a/cc/subtle/common_enums.h b/cc/subtle/common_enums.h index ef8ee5ffa2..a1d1277ea4 100644 --- a/cc/subtle/common_enums.h +++ b/cc/subtle/common_enums.h @@ -47,6 +47,12 @@ enum HashType { SHA512 = 4, }; +enum EcdsaSignatureEncoding { + UNKNOWN_ENCODING = 0, + IEEE_P1363 = 1, + DER = 2, +}; + std::string EnumToString(EllipticCurveType type); std::string EnumToString(EcPointFormat format); std::string EnumToString(HashType type); diff --git a/cc/subtle/ecdsa_sign_boringssl.cc b/cc/subtle/ecdsa_sign_boringssl.cc index 8a7c6c3f67..c9bb931f51 100644 --- a/cc/subtle/ecdsa_sign_boringssl.cc +++ b/cc/subtle/ecdsa_sign_boringssl.cc @@ -31,10 +31,45 @@ namespace crypto { namespace tink { namespace subtle { +namespace { + +// Transforms ECDSA DER signature encoding to IEEE_P1363 encoding. +// +// The IEEE_P1363 signature's format is r || s, where r and s are zero-padded +// and have the same size in bytes as the order of the curve. For example, for +// NIST P-256 curve, r and s are zero-padded to 32 bytes. +// +// The DER signature is encoded using ASN.1 +// (https://tools.ietf.org/html/rfc5480#appendix-A): ECDSA-Sig-Value :: = +// SEQUENCE { r INTEGER, s INTEGER }. In particular, the encoding is: 0x30 || +// totalLength || 0x02 || r's length || r || 0x02 || s's length || s. +crypto::tink::util::StatusOr DerToIeee(absl::string_view der, + const EC_KEY* key) { + size_t field_size_in_bytes = + (EC_GROUP_get_degree(EC_KEY_get0_group(key)) + 7) / 8; + bssl::UniquePtr ecdsa(ECDSA_SIG_from_bytes( + reinterpret_cast(der.data()), der.size())); + if (ecdsa.get() == nullptr) { + return util::Status(util::error::INTERNAL, + "Internal BoringSSL ECDSA_SIG_from_bytes's error"); + } + auto status_or_r = SubtleUtilBoringSSL::bn2str(ecdsa->r, field_size_in_bytes); + if (!status_or_r.ok()) { + return status_or_r.status(); + } + auto status_or_s = SubtleUtilBoringSSL::bn2str(ecdsa->s, field_size_in_bytes); + if (!status_or_s.ok()) { + return status_or_s.status(); + } + return status_or_r.ValueOrDie() + status_or_s.ValueOrDie(); +} + +} // namespace + // static -util::StatusOr> -EcdsaSignBoringSsl::New(const SubtleUtilBoringSSL::EcKey& ec_key, - HashType hash_type) { +util::StatusOr> EcdsaSignBoringSsl::New( + const SubtleUtilBoringSSL::EcKey& ec_key, HashType hash_type, + EcdsaSignatureEncoding encoding) { // Check hash. auto hash_status = SubtleUtilBoringSSL::ValidateSignatureHash(hash_type); if (!hash_status.ok()) { @@ -74,12 +109,13 @@ EcdsaSignBoringSsl::New(const SubtleUtilBoringSSL::EcKey& ec_key, // Sign. std::unique_ptr sign( - new EcdsaSignBoringSsl(key.release(), hash)); + new EcdsaSignBoringSsl(key.release(), hash, encoding)); return std::move(sign); } -EcdsaSignBoringSsl::EcdsaSignBoringSsl(EC_KEY* key, const EVP_MD* hash) - : key_(key), hash_(hash) {} +EcdsaSignBoringSsl::EcdsaSignBoringSsl(EC_KEY* key, const EVP_MD* hash, + EcdsaSignatureEncoding encoding) + : key_(key), hash_(hash), encoding_(encoding) {} util::StatusOr EcdsaSignBoringSsl::Sign( absl::string_view data) const { @@ -99,6 +135,15 @@ util::StatusOr EcdsaSignBoringSsl::Sign( return util::Status(util::error::INTERNAL, "Signing failed."); } + if (encoding_ == subtle::EcdsaSignatureEncoding::IEEE_P1363) { + auto status_or_sig = DerToIeee( + std::string(reinterpret_cast(buffer.data()), sig_length), key_.get()); + if (!status_or_sig.ok()) { + return status_or_sig.status(); + } + return status_or_sig.ValueOrDie(); + } + return std::string(reinterpret_cast(buffer.data()), sig_length); } diff --git a/cc/subtle/ecdsa_sign_boringssl.h b/cc/subtle/ecdsa_sign_boringssl.h index 6f1625247a..86105c03e0 100644 --- a/cc/subtle/ecdsa_sign_boringssl.h +++ b/cc/subtle/ecdsa_sign_boringssl.h @@ -35,8 +35,9 @@ namespace subtle { class EcdsaSignBoringSsl : public PublicKeySign { public: public: - static crypto::tink::util::StatusOr> - New(const SubtleUtilBoringSSL::EcKey& ec_key, HashType hash_type); + static crypto::tink::util::StatusOr> New( + const SubtleUtilBoringSSL::EcKey& ec_key, HashType hash_type, + EcdsaSignatureEncoding encoding); // Computes the signature for 'data'. crypto::tink::util::StatusOr Sign( @@ -45,10 +46,12 @@ class EcdsaSignBoringSsl : public PublicKeySign { virtual ~EcdsaSignBoringSsl() {} private: - EcdsaSignBoringSsl(EC_KEY* key, const EVP_MD* hash); + EcdsaSignBoringSsl(EC_KEY* key, const EVP_MD* hash, + EcdsaSignatureEncoding encoding); bssl::UniquePtr key_; const EVP_MD* hash_; // Owned by BoringSSL. + EcdsaSignatureEncoding encoding_; }; } // namespace subtle diff --git a/cc/subtle/ecdsa_sign_boringssl_test.cc b/cc/subtle/ecdsa_sign_boringssl_test.cc index cb6d526748..6a8304cc13 100644 --- a/cc/subtle/ecdsa_sign_boringssl_test.cc +++ b/cc/subtle/ecdsa_sign_boringssl_test.cc @@ -18,15 +18,16 @@ #include +#include "gtest/gtest.h" #include "tink/public_key_sign.h" #include "tink/public_key_verify.h" -#include "tink/subtle/ecdsa_verify_boringssl.h" #include "tink/subtle/common_enums.h" +#include "tink/subtle/ec_util.h" +#include "tink/subtle/ecdsa_verify_boringssl.h" #include "tink/subtle/subtle_util_boringssl.h" #include "tink/util/status.h" #include "tink/util/statusor.h" #include "tink/util/test_util.h" -#include "gtest/gtest.h" namespace crypto { namespace tink { @@ -37,40 +38,102 @@ class EcdsaSignBoringSslTest : public ::testing::Test { }; TEST_F(EcdsaSignBoringSslTest, testBasicSigning) { - auto ec_key = SubtleUtilBoringSSL::GetNewEcKey( - EllipticCurveType::NIST_P256).ValueOrDie(); - auto signer_result = EcdsaSignBoringSsl::New(ec_key, HashType::SHA256); - ASSERT_TRUE(signer_result.ok()) << signer_result.status(); - auto signer = std::move(signer_result.ValueOrDie()); - - auto verifier_result = EcdsaVerifyBoringSsl::New(ec_key, HashType::SHA256); - ASSERT_TRUE(verifier_result.ok()) << verifier_result.status(); - auto verifier = std::move(verifier_result.ValueOrDie()); - - std::string message = "some data to be signed"; - std::string signature = signer->Sign(message).ValueOrDie(); - EXPECT_NE(signature, message); - auto status = verifier->Verify(signature, message); - EXPECT_TRUE(status.ok()) << status; - - status = verifier->Verify("some bad signature", message); - EXPECT_FALSE(status.ok()); - - status = verifier->Verify(signature, "some bad message"); - EXPECT_FALSE(status.ok()); - - // Message is a null string_view. - const absl::string_view empty_message; - signature = signer->Sign(empty_message).ValueOrDie(); - EXPECT_NE(signature, empty_message); - status = verifier->Verify(signature, empty_message); - EXPECT_TRUE(status.ok()) << status; + subtle::EcdsaSignatureEncoding encodings[2] = { + EcdsaSignatureEncoding::DER, EcdsaSignatureEncoding::IEEE_P1363}; + for (EcdsaSignatureEncoding encoding : encodings) { + auto ec_key = SubtleUtilBoringSSL::GetNewEcKey(EllipticCurveType::NIST_P256) + .ValueOrDie(); + auto signer_result = + EcdsaSignBoringSsl::New(ec_key, HashType::SHA256, encoding); + ASSERT_TRUE(signer_result.ok()) << signer_result.status(); + auto signer = std::move(signer_result.ValueOrDie()); + + auto verifier_result = + EcdsaVerifyBoringSsl::New(ec_key, HashType::SHA256, encoding); + ASSERT_TRUE(verifier_result.ok()) << verifier_result.status(); + auto verifier = std::move(verifier_result.ValueOrDie()); + + std::string message = "some data to be signed"; + std::string signature = signer->Sign(message).ValueOrDie(); + EXPECT_NE(signature, message); + auto status = verifier->Verify(signature, message); + EXPECT_TRUE(status.ok()) << status; + + status = verifier->Verify("some bad signature", message); + EXPECT_FALSE(status.ok()); + + status = verifier->Verify(signature, "some bad message"); + EXPECT_FALSE(status.ok()); + + // Message is a null string_view. + const absl::string_view empty_message; + signature = signer->Sign(empty_message).ValueOrDie(); + EXPECT_NE(signature, empty_message); + status = verifier->Verify(signature, empty_message); + EXPECT_TRUE(status.ok()) << status; + } +} + +TEST_F(EcdsaSignBoringSslTest, testEncodingsMismatch) { + subtle::EcdsaSignatureEncoding encodings[2] = { + EcdsaSignatureEncoding::DER, EcdsaSignatureEncoding::IEEE_P1363}; + for (EcdsaSignatureEncoding encoding : encodings) { + auto ec_key = SubtleUtilBoringSSL::GetNewEcKey(EllipticCurveType::NIST_P256) + .ValueOrDie(); + auto signer_result = + EcdsaSignBoringSsl::New(ec_key, HashType::SHA256, encoding); + ASSERT_TRUE(signer_result.ok()) << signer_result.status(); + auto signer = std::move(signer_result.ValueOrDie()); + + auto verifier_result = + EcdsaVerifyBoringSsl::New(ec_key, HashType::SHA256, + encoding == EcdsaSignatureEncoding::DER + ? EcdsaSignatureEncoding::IEEE_P1363 + : EcdsaSignatureEncoding::DER); + ASSERT_TRUE(verifier_result.ok()) << verifier_result.status(); + auto verifier = std::move(verifier_result.ValueOrDie()); + + std::string message = "some data to be signed"; + std::string signature = signer->Sign(message).ValueOrDie(); + EXPECT_NE(signature, message); + auto status = verifier->Verify(signature, message); + EXPECT_FALSE(status.ok()) << status; + } +} + +TEST_F(EcdsaSignBoringSslTest, testSignatureSizesWithIEEE_P1364Encoding) { + EllipticCurveType curves[3] = {EllipticCurveType::NIST_P256, + EllipticCurveType::NIST_P384, + EllipticCurveType::NIST_P521}; + for (EllipticCurveType curve : curves) { + auto ec_key = SubtleUtilBoringSSL::GetNewEcKey(curve).ValueOrDie(); + auto signer_result = EcdsaSignBoringSsl::New( + ec_key, HashType::SHA256, EcdsaSignatureEncoding::IEEE_P1363); + ASSERT_TRUE(signer_result.ok()) << signer_result.status(); + auto signer = std::move(signer_result.ValueOrDie()); + + auto verifier_result = EcdsaVerifyBoringSsl::New( + ec_key, HashType::SHA256, EcdsaSignatureEncoding::IEEE_P1363); + ASSERT_TRUE(verifier_result.ok()) << verifier_result.status(); + auto verifier = std::move(verifier_result.ValueOrDie()); + + std::string message = "some data to be signed"; + std::string signature = signer->Sign(message).ValueOrDie(); + EXPECT_NE(signature, message); + auto status = verifier->Verify(signature, message); + EXPECT_TRUE(status.ok()) << status; + + // Check signature size. + auto field_size_in_bytes = EcUtil::FieldSizeInBytes(curve); + EXPECT_EQ(2 * field_size_in_bytes, signature.size()); + } } TEST_F(EcdsaSignBoringSslTest, testNewErrors) { auto ec_key = SubtleUtilBoringSSL::GetNewEcKey(EllipticCurveType::NIST_P256) .ValueOrDie(); - auto signer_result = EcdsaSignBoringSsl::New(ec_key, HashType::SHA1); + auto signer_result = EcdsaSignBoringSsl::New(ec_key, HashType::SHA1, + EcdsaSignatureEncoding::DER); EXPECT_FALSE(signer_result.ok()) << signer_result.status(); } diff --git a/cc/subtle/ecdsa_verify_boringssl.cc b/cc/subtle/ecdsa_verify_boringssl.cc index 727dbbfb41..0131877440 100644 --- a/cc/subtle/ecdsa_verify_boringssl.cc +++ b/cc/subtle/ecdsa_verify_boringssl.cc @@ -17,22 +17,72 @@ #include "tink/subtle/ecdsa_verify_boringssl.h" #include "absl/strings/str_cat.h" -#include "tink/subtle/common_enums.h" -#include "tink/subtle/subtle_util_boringssl.h" -#include "tink/util/errors.h" #include "openssl/bn.h" #include "openssl/ec.h" #include "openssl/ecdsa.h" #include "openssl/evp.h" +#include "openssl/mem.h" +#include "tink/subtle/common_enums.h" +#include "tink/subtle/subtle_util_boringssl.h" +#include "tink/util/errors.h" namespace crypto { namespace tink { namespace subtle { +namespace { + +// Transforms ECDSA IEEE_P1363 signature encoding to DER encoding. +// +// The IEEE_P1363 signature's format is r || s, where r and s are zero-padded +// and have the same size in bytes as the order of the curve. For example, for +// NIST P-256 curve, r and s are zero-padded to 32 bytes. +// +// The DER signature is encoded using ASN.1 +// (https://tools.ietf.org/html/rfc5480#appendix-A): ECDSA-Sig-Value :: = +// SEQUENCE { r INTEGER, s INTEGER }. In particular, the encoding is: 0x30 || +// totalLength || 0x02 || r's length || r || 0x02 || s's length || s. +crypto::tink::util::StatusOr IeeeToDer(absl::string_view ieee, + const EC_KEY* key) { + size_t field_size_in_bytes = + (EC_GROUP_get_degree(EC_KEY_get0_group(key)) + 7) / 8; + if (ieee.size() != field_size_in_bytes * 2) { + return util::Status(util::error::INVALID_ARGUMENT, + "Signature is not valid."); + } + bssl::UniquePtr ecdsa(ECDSA_SIG_new()); + auto status_or_r = + SubtleUtilBoringSSL::str2bn(ieee.substr(0, ieee.size() / 2)); + if (!status_or_r.ok()) { + return status_or_r.status(); + } + auto status_or_s = SubtleUtilBoringSSL::str2bn( + ieee.substr(ieee.size() / 2, ieee.size() / 2)); + if (!status_or_s.ok()) { + return status_or_s.status(); + } + if (1 != ECDSA_SIG_set0(ecdsa.get(), status_or_r.ValueOrDie().get(), + status_or_s.ValueOrDie().get())) { + return util::Status(util::error::INTERNAL, "ECDSA_SIG_set0 error."); + } + // ECDSA_SIG_set0 takes ownership of s and r's pointers. + status_or_r.ValueOrDie().release(); + status_or_s.ValueOrDie().release(); + uint8_t* der = nullptr; + bssl::UniquePtr unique(der); + size_t der_len; + if (!ECDSA_SIG_to_bytes(&der, &der_len, ecdsa.get())) { + return util::Status(util::error::INVALID_ARGUMENT, + "ECDSA_SIG_to_bytes error"); + } + return std::string(reinterpret_cast(der), der_len); +} +} // namespace + // static -util::StatusOr> -EcdsaVerifyBoringSsl::New(const SubtleUtilBoringSSL::EcKey& ec_key, - HashType hash_type) { +util::StatusOr> EcdsaVerifyBoringSsl::New( + const SubtleUtilBoringSSL::EcKey& ec_key, HashType hash_type, + EcdsaSignatureEncoding encoding) { // Check hash. auto hash_status = SubtleUtilBoringSSL::ValidateSignatureHash(hash_type); if (!hash_status.ok()) { @@ -60,12 +110,13 @@ EcdsaVerifyBoringSsl::New(const SubtleUtilBoringSSL::EcKey& ec_key, SubtleUtilBoringSSL::GetErrors())); } std::unique_ptr verify( - new EcdsaVerifyBoringSsl(key.release(), hash)); + new EcdsaVerifyBoringSsl(key.release(), hash, encoding)); return std::move(verify); } -EcdsaVerifyBoringSsl::EcdsaVerifyBoringSsl(EC_KEY* key, const EVP_MD* hash) - : key_(key), hash_(hash) {} +EcdsaVerifyBoringSsl::EcdsaVerifyBoringSsl(EC_KEY* key, const EVP_MD* hash, + EcdsaSignatureEncoding encoding) + : key_(key), hash_(hash), encoding_(encoding) {} util::Status EcdsaVerifyBoringSsl::Verify( absl::string_view signature, @@ -79,10 +130,19 @@ util::Status EcdsaVerifyBoringSsl::Verify( return util::Status(util::error::INTERNAL, "Could not compute digest."); } + std::string derSig(signature); + if (encoding_ == subtle::EcdsaSignatureEncoding::IEEE_P1363) { + auto status_or_der = IeeeToDer(signature, key_.get()); + if (!status_or_der.ok()) { + return status_or_der.status(); + } + derSig = status_or_der.ValueOrDie(); + } + // Verify the signature. if (1 != ECDSA_verify(0 /* unused */, digest, digest_size, - reinterpret_cast(signature.data()), - signature.size(), key_.get())) { + reinterpret_cast(derSig.data()), + derSig.size(), key_.get())) { // signature is invalid return util::Status(util::error::UNKNOWN, "Signature is not valid."); } diff --git a/cc/subtle/ecdsa_verify_boringssl.h b/cc/subtle/ecdsa_verify_boringssl.h index 8fc2b20506..7df731708c 100644 --- a/cc/subtle/ecdsa_verify_boringssl.h +++ b/cc/subtle/ecdsa_verify_boringssl.h @@ -35,7 +35,8 @@ namespace subtle { class EcdsaVerifyBoringSsl : public PublicKeyVerify { public: static crypto::tink::util::StatusOr> - New(const SubtleUtilBoringSSL::EcKey& ec_key, HashType hash_type); + New(const SubtleUtilBoringSSL::EcKey& ec_key, HashType hash_type, + EcdsaSignatureEncoding encoding); // Verifies that 'signature' is a digital signature for 'data'. crypto::tink::util::Status Verify( @@ -45,10 +46,12 @@ class EcdsaVerifyBoringSsl : public PublicKeyVerify { virtual ~EcdsaVerifyBoringSsl() {} private: - EcdsaVerifyBoringSsl(EC_KEY* key, const EVP_MD* hash); + EcdsaVerifyBoringSsl(EC_KEY* key, const EVP_MD* hash, + EcdsaSignatureEncoding encoding); bssl::UniquePtr key_; const EVP_MD* hash_; // Owned by BoringSSL. + EcdsaSignatureEncoding encoding_; }; } // namespace subtle diff --git a/cc/subtle/ecdsa_verify_boringssl_test.cc b/cc/subtle/ecdsa_verify_boringssl_test.cc index 2f0ebc273b..092ae14b13 100644 --- a/cc/subtle/ecdsa_verify_boringssl_test.cc +++ b/cc/subtle/ecdsa_verify_boringssl_test.cc @@ -39,41 +39,80 @@ namespace { class EcdsaVerifyBoringSslTest : public ::testing::Test {}; TEST_F(EcdsaVerifyBoringSslTest, testBasicSigning) { - auto ec_key_result = SubtleUtilBoringSSL::GetNewEcKey( - EllipticCurveType::NIST_P256); - ASSERT_TRUE(ec_key_result.ok()) << ec_key_result.status(); - auto ec_key = std::move(ec_key_result.ValueOrDie()); - - auto signer_result = EcdsaSignBoringSsl::New(ec_key, HashType::SHA256); - ASSERT_TRUE(signer_result.ok()) << signer_result.status(); - auto signer = std::move(signer_result.ValueOrDie()); - - auto verifier_result = EcdsaVerifyBoringSsl::New(ec_key, HashType::SHA256); - ASSERT_TRUE(verifier_result.ok()) << verifier_result.status(); - auto verifier = std::move(verifier_result.ValueOrDie()); - - std::string message = "some data to be signed"; - auto sign_result = signer->Sign(message); - ASSERT_TRUE(sign_result.ok()) << sign_result.status(); - std::string signature = sign_result.ValueOrDie(); - EXPECT_NE(signature, message); - auto status = verifier->Verify(signature, message); - EXPECT_TRUE(status.ok()) << status; - - status = verifier->Verify(signature + "some trailing data", message); - EXPECT_FALSE(status.ok()) << status; - - status = verifier->Verify("some bad signature", message); - EXPECT_FALSE(status.ok()); - - status = verifier->Verify(signature, "some bad message"); - EXPECT_FALSE(status.ok()); + subtle::EcdsaSignatureEncoding encodings[2] = { + EcdsaSignatureEncoding::DER, EcdsaSignatureEncoding::IEEE_P1363}; + for (EcdsaSignatureEncoding encoding : encodings) { + auto ec_key_result = + SubtleUtilBoringSSL::GetNewEcKey(EllipticCurveType::NIST_P256); + ASSERT_TRUE(ec_key_result.ok()) << ec_key_result.status(); + auto ec_key = std::move(ec_key_result.ValueOrDie()); + + auto signer_result = + EcdsaSignBoringSsl::New(ec_key, HashType::SHA256, encoding); + ASSERT_TRUE(signer_result.ok()) << signer_result.status(); + auto signer = std::move(signer_result.ValueOrDie()); + + auto verifier_result = + EcdsaVerifyBoringSsl::New(ec_key, HashType::SHA256, encoding); + ASSERT_TRUE(verifier_result.ok()) << verifier_result.status(); + auto verifier = std::move(verifier_result.ValueOrDie()); + + std::string message = "some data to be signed"; + auto sign_result = signer->Sign(message); + ASSERT_TRUE(sign_result.ok()) << sign_result.status(); + std::string signature = sign_result.ValueOrDie(); + EXPECT_NE(signature, message); + auto status = verifier->Verify(signature, message); + EXPECT_TRUE(status.ok()) << status; + + status = verifier->Verify(signature + "some trailing data", message); + EXPECT_FALSE(status.ok()) << status; + + status = verifier->Verify("some bad signature", message); + EXPECT_FALSE(status.ok()); + + status = verifier->Verify(signature, "some bad message"); + EXPECT_FALSE(status.ok()); + } +} + +TEST_F(EcdsaVerifyBoringSslTest, testEncodingsMismatch) { + subtle::EcdsaSignatureEncoding encodings[2] = { + EcdsaSignatureEncoding::DER, EcdsaSignatureEncoding::IEEE_P1363}; + for (EcdsaSignatureEncoding encoding : encodings) { + auto ec_key_result = + SubtleUtilBoringSSL::GetNewEcKey(EllipticCurveType::NIST_P256); + ASSERT_TRUE(ec_key_result.ok()) << ec_key_result.status(); + auto ec_key = std::move(ec_key_result.ValueOrDie()); + + auto signer_result = + EcdsaSignBoringSsl::New(ec_key, HashType::SHA256, encoding); + ASSERT_TRUE(signer_result.ok()) << signer_result.status(); + auto signer = std::move(signer_result.ValueOrDie()); + + auto verifier_result = + EcdsaVerifyBoringSsl::New(ec_key, HashType::SHA256, + encoding == EcdsaSignatureEncoding::DER + ? EcdsaSignatureEncoding::IEEE_P1363 + : EcdsaSignatureEncoding::DER); + ASSERT_TRUE(verifier_result.ok()) << verifier_result.status(); + auto verifier = std::move(verifier_result.ValueOrDie()); + + std::string message = "some data to be signed"; + auto sign_result = signer->Sign(message); + ASSERT_TRUE(sign_result.ok()) << sign_result.status(); + std::string signature = sign_result.ValueOrDie(); + EXPECT_NE(signature, message); + auto status = verifier->Verify(signature, message); + EXPECT_FALSE(status.ok()) << status; + } } TEST_F(EcdsaVerifyBoringSslTest, testNewErrors) { auto ec_key = SubtleUtilBoringSSL::GetNewEcKey(EllipticCurveType::NIST_P256) .ValueOrDie(); - auto verifier_result = EcdsaVerifyBoringSsl::New(ec_key, HashType::SHA1); + auto verifier_result = EcdsaVerifyBoringSsl::New( + ec_key, HashType::SHA1, EcdsaSignatureEncoding::IEEE_P1363); EXPECT_FALSE(verifier_result.ok()) << verifier_result.status(); } @@ -106,7 +145,8 @@ GetVerifier(const rapidjson::Value &test_group) { key.pub_y = GetInteger(test_group["key"]["wy"]); key.curve = WycheproofUtil::GetEllipticCurveType(test_group["key"]["curve"]); HashType md = WycheproofUtil::GetHashType(test_group["sha"]); - auto result = EcdsaVerifyBoringSsl::New(key, md); + auto result = + EcdsaVerifyBoringSsl::New(key, md, subtle::EcdsaSignatureEncoding::DER); if (!result.ok()) { std::cout << "Failed: " << result.status() << "\n"; } diff --git a/cc/subtle/subtle_util_boringssl.cc b/cc/subtle/subtle_util_boringssl.cc index 4af7e9cdd2..4de9e0191c 100644 --- a/cc/subtle/subtle_util_boringssl.cc +++ b/cc/subtle/subtle_util_boringssl.cc @@ -27,14 +27,6 @@ namespace subtle { namespace { -util::StatusOr bn2str(const BIGNUM* bn, size_t len) { - std::unique_ptr res(new uint8_t[len]); - if (1 != BN_bn2bin_padded(res.get(), len, bn)) { - return util::Status(util::error::INTERNAL, "Value too large"); - } - return std::string(reinterpret_cast(res.get()), len); -} - size_t ScalarSizeInBytes(const EC_GROUP* group) { return BN_num_bytes(EC_GROUP_get0_order(group)); } @@ -46,6 +38,16 @@ size_t FieldElementSizeInBytes(const EC_GROUP* group) { } // namespace +// static +util::StatusOr SubtleUtilBoringSSL::bn2str(const BIGNUM *bn, + size_t len) { + std::unique_ptr res(new uint8_t[len]); + if (1 != BN_bn2bin_padded(res.get(), len, bn)) { + return util::Status(util::error::INTERNAL, "Value too large"); + } + return std::string(reinterpret_cast(res.get()), len); +} + // static util::StatusOr> SubtleUtilBoringSSL::str2bn( absl::string_view s) { diff --git a/cc/subtle/subtle_util_boringssl.h b/cc/subtle/subtle_util_boringssl.h index 102c73d8f7..01205a7e2f 100644 --- a/cc/subtle/subtle_util_boringssl.h +++ b/cc/subtle/subtle_util_boringssl.h @@ -65,6 +65,9 @@ class SubtleUtilBoringSSL { // representation. static util::StatusOr> str2bn(absl::string_view s); + // Returns a std::string of size 'len' that holds BIGNUM 'bn'. + static util::StatusOr bn2str(const BIGNUM *bn, size_t len); + // Returns BoringSSL error strings accumulated in the error queue, // thus emptying the queue. static std::string GetErrors(); diff --git a/cc/subtle/subtle_util_boringssl_test.cc b/cc/subtle/subtle_util_boringssl_test.cc index aee6e3a7b7..4690dcba1b 100644 --- a/cc/subtle/subtle_util_boringssl_test.cc +++ b/cc/subtle/subtle_util_boringssl_test.cc @@ -128,6 +128,21 @@ TEST(SubtleUtilBoringSSLTest, testEcPointDecode) { } } +TEST(SubtleUtilBoringSSLTest, testBn2strAndStr2bn) { + int len = 8; + std::string bn_str[6] = {"0000000000000000", "0000000000000001", + "1000000000000000", "ffffffffffffffff", + "0fffffffffffffff", "00ffffffffffffff"}; + for (const std::string& s : bn_str) { + auto status_or_bn = SubtleUtilBoringSSL::str2bn(test::HexDecodeOrDie(s)); + EXPECT_TRUE(status_or_bn.ok()); + auto status_or_str = + SubtleUtilBoringSSL::bn2str(status_or_bn.ValueOrDie().get(), len); + EXPECT_TRUE(status_or_str.ok()); + EXPECT_EQ(test::HexDecodeOrDie(s), status_or_str.ValueOrDie()); + } +} + TEST(SubtleUtilBoringSSLTest, testValidateSignatureHash) { EXPECT_TRUE( SubtleUtilBoringSSL::ValidateSignatureHash(HashType::SHA256).ok()); diff --git a/cc/util/BUILD.bazel b/cc/util/BUILD.bazel index 766bba18b5..5dd2f7c61a 100644 --- a/cc/util/BUILD.bazel +++ b/cc/util/BUILD.bazel @@ -22,6 +22,7 @@ cc_library( deps = [ "//cc/subtle:common_enums", "//proto:common_cc_proto", + "//proto:ecdsa_cc_proto", "//proto:tink_cc_proto", "@com_google_absl//absl/strings", ], diff --git a/cc/util/enums.cc b/cc/util/enums.cc index f5c1bc6d0c..6ea7e9da81 100644 --- a/cc/util/enums.cc +++ b/cc/util/enums.cc @@ -16,6 +16,7 @@ #include "tink/util/enums.h" #include "proto/common.pb.h" +#include "proto/ecdsa.pb.h" #include "proto/tink.pb.h" namespace pb = google::crypto::tink; @@ -108,6 +109,32 @@ subtle::HashType Enums::ProtoToSubtle(pb::HashType type) { } } +// static +subtle::EcdsaSignatureEncoding Enums::ProtoToSubtle( + pb::EcdsaSignatureEncoding encoding) { + switch (encoding) { + case pb::EcdsaSignatureEncoding::DER: + return subtle::EcdsaSignatureEncoding::DER; + case pb::EcdsaSignatureEncoding::IEEE_P1363: + return subtle::EcdsaSignatureEncoding::IEEE_P1363; + default: + return subtle::EcdsaSignatureEncoding::UNKNOWN_ENCODING; + } +} + +// static +pb::EcdsaSignatureEncoding Enums::SubtleToProto( + subtle::EcdsaSignatureEncoding encoding) { + switch (encoding) { + case subtle::EcdsaSignatureEncoding::DER: + return pb::EcdsaSignatureEncoding::DER; + case subtle::EcdsaSignatureEncoding::IEEE_P1363: + return pb::EcdsaSignatureEncoding::IEEE_P1363; + default: + return pb::EcdsaSignatureEncoding::UNKNOWN_ENCODING; + } +} + // static const char* Enums::KeyStatusName(pb::KeyStatusType key_status_type) { switch (key_status_type) { diff --git a/cc/util/enums.h b/cc/util/enums.h index e1a3638975..a62b6209a5 100644 --- a/cc/util/enums.h +++ b/cc/util/enums.h @@ -20,6 +20,7 @@ #include "absl/strings/string_view.h" #include "tink/subtle/common_enums.h" #include "proto/common.pb.h" +#include "proto/ecdsa.pb.h" #include "proto/tink.pb.h" namespace crypto { @@ -52,6 +53,13 @@ class Enums { static crypto::tink::subtle::HashType ProtoToSubtle( google::crypto::tink::HashType type); + // EcdsaSignatureEncoding. + static google::crypto::tink::EcdsaSignatureEncoding SubtleToProto( + crypto::tink::subtle::EcdsaSignatureEncoding encoding); + + static crypto::tink::subtle::EcdsaSignatureEncoding ProtoToSubtle( + google::crypto::tink::EcdsaSignatureEncoding encoding); + // Printable names for common enums. static const char* KeyStatusName( google::crypto::tink::KeyStatusType key_status_type); diff --git a/cc/util/enums_test.cc b/cc/util/enums_test.cc index dd31a20e16..b404da837a 100644 --- a/cc/util/enums_test.cc +++ b/cc/util/enums_test.cc @@ -147,6 +147,34 @@ TEST_F(EnumsTest, testEcPointFormat) { EXPECT_EQ(4, count); } +TEST_F(EnumsTest, testEcdsaSignatureEncoding) { + EXPECT_EQ( + pb::EcdsaSignatureEncoding::UNKNOWN_ENCODING, + Enums::SubtleToProto(subtle::EcdsaSignatureEncoding::UNKNOWN_ENCODING)); + EXPECT_EQ(pb::EcdsaSignatureEncoding::IEEE_P1363, + Enums::SubtleToProto(subtle::EcdsaSignatureEncoding::IEEE_P1363)); + EXPECT_EQ(pb::EcdsaSignatureEncoding::DER, + Enums::SubtleToProto(subtle::EcdsaSignatureEncoding::DER)); + EXPECT_EQ(subtle::EcdsaSignatureEncoding::UNKNOWN_ENCODING, + Enums::ProtoToSubtle(pb::EcdsaSignatureEncoding::UNKNOWN_ENCODING)); + EXPECT_EQ(subtle::EcdsaSignatureEncoding::IEEE_P1363, + Enums::ProtoToSubtle(pb::EcdsaSignatureEncoding::IEEE_P1363)); + EXPECT_EQ(subtle::EcdsaSignatureEncoding::DER, + Enums::ProtoToSubtle(pb::EcdsaSignatureEncoding::DER)); + // Check that enum conversion covers the entire range of the proto-enum. + int count = 0; + for (int int_encoding = (int)pb::EcdsaSignatureEncoding_MIN; + int_encoding <= (int)pb::EcdsaSignatureEncoding_MAX; int_encoding++) { + if (pb::EcdsaSignatureEncoding_IsValid(int_encoding)) { + pb::EcdsaSignatureEncoding encoding = + (pb::EcdsaSignatureEncoding)int_encoding; + EXPECT_EQ(encoding, Enums::SubtleToProto(Enums::ProtoToSubtle(encoding))); + count++; + } + } + EXPECT_EQ(3, count); +} + TEST_F(EnumsTest, testKeyStatusName) { EXPECT_EQ("ENABLED", std::string(Enums::KeyStatusName(pb::KeyStatusType::ENABLED))); diff --git a/cc/util/test_util.cc b/cc/util/test_util.cc index 77d18a96db..98cbcc7b88 100644 --- a/cc/util/test_util.cc +++ b/cc/util/test_util.cc @@ -178,16 +178,17 @@ EciesAeadHkdfPrivateKey GetEciesAesGcmHkdfTestKey( } EcdsaPrivateKey GetEcdsaTestPrivateKey( - subtle::EllipticCurveType curve_type, - subtle::HashType hash_type) { - return GetEcdsaTestPrivateKey( - Enums::SubtleToProto(curve_type), - Enums::SubtleToProto(hash_type)); + subtle::EllipticCurveType curve_type, subtle::HashType hash_type, + subtle::EcdsaSignatureEncoding encoding) { + return GetEcdsaTestPrivateKey(Enums::SubtleToProto(curve_type), + Enums::SubtleToProto(hash_type), + Enums::SubtleToProto(encoding)); } EcdsaPrivateKey GetEcdsaTestPrivateKey( google::crypto::tink::EllipticCurveType curve_type, - google::crypto::tink::HashType hash_type) { + google::crypto::tink::HashType hash_type, + google::crypto::tink::EcdsaSignatureEncoding encoding) { auto test_key = subtle::SubtleUtilBoringSSL::GetNewEcKey( Enums::ProtoToSubtle(curve_type)).ValueOrDie(); EcdsaPrivateKey ecdsa_key; @@ -200,7 +201,7 @@ EcdsaPrivateKey GetEcdsaTestPrivateKey( auto params = public_key->mutable_params(); params->set_hash_type(hash_type); params->set_curve(curve_type); - params->set_encoding(EcdsaSignatureEncoding::DER); + params->set_encoding(encoding); return ecdsa_key; } diff --git a/cc/util/test_util.h b/cc/util/test_util.h index 0898206bee..3fae82f920 100644 --- a/cc/util/test_util.h +++ b/cc/util/test_util.h @@ -103,17 +103,18 @@ google::crypto::tink::EciesAeadHkdfPrivateKey GetEciesAesGcmHkdfTestKey( google::crypto::tink::HashType hash_type, uint32_t aes_gcm_key_size); -// Generates a fresh test key for EC DSA for the given 'curve_type' -// and 'hash_type'. The resulting signatures will use DER-encoding. +// Generates a fresh test key for EC DSA for the given 'curve_type', 'hash_type' +// and 'encoding'. google::crypto::tink::EcdsaPrivateKey GetEcdsaTestPrivateKey( - subtle::EllipticCurveType curve_type, - subtle::HashType hash_type); + subtle::EllipticCurveType curve_type, subtle::HashType hash_type, + subtle::EcdsaSignatureEncoding encoding); -// Generates a fresh test key for EC DSA for the given 'curve_type' -// and 'hash_type'. The resulting signatures will use DER-encoding. +// Generates a fresh test key for EC DSA for the given 'curve_type', 'hash_type' +// and 'encoding'. google::crypto::tink::EcdsaPrivateKey GetEcdsaTestPrivateKey( google::crypto::tink::EllipticCurveType curve_type, - google::crypto::tink::HashType hash_type); + google::crypto::tink::HashType hash_type, + google::crypto::tink::EcdsaSignatureEncoding encoding); // A dummy implementation of Aead-interface. // An instance of DummyAead can be identified by a name specified diff --git a/objc/Tests/UnitTests/signature/TINKPublicKeySignFactory.mm b/objc/Tests/UnitTests/signature/TINKPublicKeySignFactory.mm index 6896e51ac6..1892daebbb 100644 --- a/objc/Tests/UnitTests/signature/TINKPublicKeySignFactory.mm +++ b/objc/Tests/UnitTests/signature/TINKPublicKeySignFactory.mm @@ -45,13 +45,15 @@ using crypto::tink::test::AddTinkKey; using google::crypto::tink::EcdsaPrivateKey; using google::crypto::tink::EllipticCurveType; +using google::crypto::tink::EcdsaSignatureEncoding; using google::crypto::tink::HashType; using google::crypto::tink::KeyData; using google::crypto::tink::Keyset; using google::crypto::tink::KeyStatusType; static EcdsaPrivateKey GetNewEcdsaPrivateKey() { - return crypto::tink::test::GetEcdsaTestPrivateKey(EllipticCurveType::NIST_P256, HashType::SHA256); + return crypto::tink::test::GetEcdsaTestPrivateKey(EllipticCurveType::NIST_P256, HashType::SHA256, + EcdsaSignatureEncoding::DER); } @interface TINKPublicKeySignFactoryTest : XCTestCase diff --git a/objc/Tests/UnitTests/signature/TINKPublicKeyVerifyFactory.mm b/objc/Tests/UnitTests/signature/TINKPublicKeyVerifyFactory.mm index 13c005e6b9..732acfa196 100644 --- a/objc/Tests/UnitTests/signature/TINKPublicKeyVerifyFactory.mm +++ b/objc/Tests/UnitTests/signature/TINKPublicKeyVerifyFactory.mm @@ -42,6 +42,7 @@ #include "proto/tink.pb.h" using crypto::tink::EcdsaSignKeyManager; +using google::crypto::tink::EcdsaSignatureEncoding; using crypto::tink::KeyFactory; using crypto::tink::KeysetUtil; using crypto::tink::test::AddRawKey; @@ -55,7 +56,8 @@ using google::crypto::tink::KeyStatusType; static EcdsaPrivateKey GetNewEcdsaPrivateKey() { - return crypto::tink::test::GetEcdsaTestPrivateKey(EllipticCurveType::NIST_P256, HashType::SHA256); + return crypto::tink::test::GetEcdsaTestPrivateKey(EllipticCurveType::NIST_P256, HashType::SHA256, + EcdsaSignatureEncoding::DER); } static EcdsaPublicKey GetEcdsaPublicKeyFromPrivate(EcdsaPrivateKey &privateKey) {