Skip to content

Commit

Permalink
Support EcdsaSignatureEncoding::IEEE_P1363 in C++.
Browse files Browse the repository at this point in the history 
 + Add EcdsaSignatureEncoding parameter into constructor of ecdsa_sign/verify_boringssl
 + str2bn/bn2str subtle_util_boringssl.
 + Methods transform back and forth between IEEE_P1363 and DER encoding.

The rests of changes are just consequences of constructor change.

PiperOrigin-RevId: 206673897
GitOrigin-RevId: b41a79aed3c317ed6104e945dc1834bb4df0b358
  • Loading branch information
@cryptosubtlety @chuckx
cryptosubtlety authored and chuckx committed Aug 1, 2018
1 parent 09ecacb commit ae7e9f9
Show file tree
Hide file tree
Showing 25 changed files with 507 additions and 189 deletions.
3 changes: 2 additions & 1 deletion cc/signature/ecdsa_sign_key_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<PublicKeySign> ecdsa(ecdsa_result.ValueOrDie().release());
return std::move(ecdsa);
Expand Down
63 changes: 27 additions & 36 deletions cc/signature/ecdsa_sign_key_manager_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
}
}

Expand Down Expand Up @@ -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);
{
Expand Down
13 changes: 9 additions & 4 deletions cc/signature/ecdsa_verify_key_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,17 +144,22 @@ 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<PublicKeyVerify> ecdsa(ecdsa_result.ValueOrDie().release());
return std::move(ecdsa);
}

// 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:
Expand Down
78 changes: 42 additions & 36 deletions cc/signature/ecdsa_verify_key_manager_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions cc/signature/public_key_sign_factory_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions cc/signature/public_key_verify_factory_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}

Expand Down
1 change: 1 addition & 0 deletions cc/subtle/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions cc/subtle/common_enums.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
57 changes: 51 additions & 6 deletions cc/subtle/ecdsa_sign_boringssl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> 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_SIG> ecdsa(ECDSA_SIG_from_bytes(
reinterpret_cast<const uint8_t*>(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<std::unique_ptr<EcdsaSignBoringSsl>>
EcdsaSignBoringSsl::New(const SubtleUtilBoringSSL::EcKey& ec_key,
HashType hash_type) {
util::StatusOr<std::unique_ptr<EcdsaSignBoringSsl>> 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()) {
Expand Down Expand Up @@ -74,12 +109,13 @@ EcdsaSignBoringSsl::New(const SubtleUtilBoringSSL::EcKey& ec_key,

// Sign.
std::unique_ptr<EcdsaSignBoringSsl> 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<std::string> EcdsaSignBoringSsl::Sign(
absl::string_view data) const {
Expand All @@ -99,6 +135,15 @@ util::StatusOr<std::string> 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<char*>(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<char*>(buffer.data()), sig_length);
}

Expand Down
9 changes: 6 additions & 3 deletions cc/subtle/ecdsa_sign_boringssl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ namespace subtle {
class EcdsaSignBoringSsl : public PublicKeySign {
public:
public:
static crypto::tink::util::StatusOr<std::unique_ptr<EcdsaSignBoringSsl>>
New(const SubtleUtilBoringSSL::EcKey& ec_key, HashType hash_type);
static crypto::tink::util::StatusOr<std::unique_ptr<EcdsaSignBoringSsl>> New(
const SubtleUtilBoringSSL::EcKey& ec_key, HashType hash_type,
EcdsaSignatureEncoding encoding);

// Computes the signature for 'data'.
crypto::tink::util::StatusOr<std::string> Sign(
Expand All @@ -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<EC_KEY> key_;
const EVP_MD* hash_; // Owned by BoringSSL.
EcdsaSignatureEncoding encoding_;
};

} // namespace subtle
Expand Down
Loading

0 comments on commit ae7e9f9

Please sign in to comment.