diff --git a/tink/jwt/BUILD.bazel b/tink/jwt/BUILD.bazel index 3e230d23..d46e5c44 100644 --- a/tink/jwt/BUILD.bazel +++ b/tink/jwt/BUILD.bazel @@ -484,12 +484,13 @@ cc_library( "//tink:key", "//tink:partial_key_access_token", "//tink:restricted_big_integer", - "//tink/internal:bn_util", + "//tink/internal:rsa_util", "//tink/internal:ssl_unique_ptr", "//tink/util:status", "//tink/util:statusor", "@boringssl//:crypto", "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/types:optional", ], ) @@ -588,12 +589,13 @@ cc_library( "//tink:key", "//tink:partial_key_access_token", "//tink:restricted_big_integer", - "//tink/internal:bn_util", + "//tink/internal:rsa_util", "//tink/internal:ssl_unique_ptr", "//tink/util:status", "//tink/util:statusor", "@boringssl//:crypto", "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/types:optional", ], ) diff --git a/tink/jwt/CMakeLists.txt b/tink/jwt/CMakeLists.txt index 016e1fbf..44020ff2 100644 --- a/tink/jwt/CMakeLists.txt +++ b/tink/jwt/CMakeLists.txt @@ -440,6 +440,7 @@ tink_cc_library( tink::jwt::jwt_rsa_ssa_pkcs1_public_key tink::jwt::jwt_signature_private_key absl::status + absl::statusor absl::optional crypto tink::core::big_integer @@ -447,7 +448,7 @@ tink_cc_library( tink::core::key tink::core::partial_key_access_token tink::core::restricted_big_integer - tink::internal::bn_util + tink::internal::rsa_util tink::internal::ssl_unique_ptr tink::util::status tink::util::statusor @@ -540,6 +541,7 @@ tink_cc_library( tink::jwt::jwt_rsa_ssa_pss_public_key tink::jwt::jwt_signature_private_key absl::status + absl::statusor absl::optional crypto tink::core::big_integer @@ -547,7 +549,7 @@ tink_cc_library( tink::core::key tink::core::partial_key_access_token tink::core::restricted_big_integer - tink::internal::bn_util + tink::internal::rsa_util tink::internal::ssl_unique_ptr tink::util::status tink::util::statusor diff --git a/tink/jwt/jwt_rsa_ssa_pkcs1_private_key.cc b/tink/jwt/jwt_rsa_ssa_pkcs1_private_key.cc index a4ab03da..12aae79d 100644 --- a/tink/jwt/jwt_rsa_ssa_pkcs1_private_key.cc +++ b/tink/jwt/jwt_rsa_ssa_pkcs1_private_key.cc @@ -16,14 +16,17 @@ #include "tink/jwt/jwt_rsa_ssa_pkcs1_private_key.h" +#include + #include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "tink/internal/rsa_util.h" #ifdef OPENSSL_IS_BORINGSSL #include "openssl/base.h" #endif #include "openssl/rsa.h" #include "tink/big_integer.h" #include "tink/insecure_secret_key_access.h" -#include "tink/internal/bn_util.h" #include "tink/internal/ssl_unique_ptr.h" #include "tink/jwt/jwt_rsa_ssa_pkcs1_public_key.h" #include "tink/key.h" @@ -41,88 +44,18 @@ util::Status ValidateKeyPair( const RestrictedBigInteger& p, const RestrictedBigInteger& q, const RestrictedBigInteger& d, const RestrictedBigInteger& dp, const RestrictedBigInteger& dq, const RestrictedBigInteger& q_inv) { - internal::SslUniquePtr rsa(RSA_new()); - if (rsa.get() == nullptr) { - return util::Status(absl::StatusCode::kInternal, - "Internal RSA allocation error"); - } - - util::StatusOr> n = - internal::StringToBignum(modulus.GetValue()); - if (!n.ok()) { - return n.status(); - } - - util::StatusOr> e = - internal::StringToBignum(public_exponent.GetValue()); - if (!e.ok()) { - return e.status(); - } - - util::StatusOr> d_bn = - internal::StringToBignum(d.GetSecret(InsecureSecretKeyAccess::Get())); - if (!d_bn.ok()) { - return d_bn.status(); - } - - util::StatusOr> p_bn = - internal::StringToBignum(p.GetSecret(InsecureSecretKeyAccess::Get())); - if (!p_bn.ok()) { - return p_bn.status(); - } - util::StatusOr> q_bn = - internal::StringToBignum(q.GetSecret(InsecureSecretKeyAccess::Get())); - if (!q_bn.ok()) { - return q_bn.status(); - } - - util::StatusOr> dp_bn = - internal::StringToBignum(dp.GetSecret(InsecureSecretKeyAccess::Get())); - if (!dp_bn.ok()) { - return dp_bn.status(); - } - util::StatusOr> dq_bn = - internal::StringToBignum(dq.GetSecret(InsecureSecretKeyAccess::Get())); - if (!dq_bn.ok()) { - return dq_bn.status(); - } - util::StatusOr> q_inv_bn = - internal::StringToBignum(q_inv.GetSecret(InsecureSecretKeyAccess::Get())); - if (!q_inv_bn.ok()) { - return q_inv_bn.status(); - } - - // Build RSA key from the given values. The RSA object takes ownership of the - // given values after the call. - if (RSA_set0_key(rsa.get(), n->release(), e->release(), d_bn->release()) != - 1 || - RSA_set0_factors(rsa.get(), p_bn->release(), q_bn->release()) != 1 || - RSA_set0_crt_params(rsa.get(), dp_bn->release(), dq_bn->release(), - q_inv_bn->release()) != 1) { - return util::Status(absl::StatusCode::kInternal, - "Internal RSA key loading error"); - } - - // Validate key. - int check_key_status = RSA_check_key(rsa.get()); - if (check_key_status == 0) { - return util::Status(absl::StatusCode::kInvalidArgument, - "RSA key pair is not valid"); - } - - if (check_key_status == -1) { - return util::Status(absl::StatusCode::kInternal, - "An error ocurred while checking the key"); - } - -#ifdef OPENSSL_IS_BORINGSSL - if (RSA_check_fips(rsa.get()) == 0) { - return util::Status(absl::StatusCode::kInvalidArgument, - "RSA key pair is not valid in FIPS mode"); - } -#endif - - return util::OkStatus(); + absl::StatusOr> rsa = + internal::RsaPrivateKeyToRsa(internal::RsaPrivateKey{ + /*n=*/std::string(modulus.GetValue()), + /*e=*/std::string(public_exponent.GetValue()), + /*d=*/d.GetSecretData(InsecureSecretKeyAccess::Get()), + /*p=*/p.GetSecretData(InsecureSecretKeyAccess::Get()), + /*q=*/q.GetSecretData(InsecureSecretKeyAccess::Get()), + /*dp=*/dp.GetSecretData(InsecureSecretKeyAccess::Get()), + /*dq=*/dq.GetSecretData(InsecureSecretKeyAccess::Get()), + /*crt=*/q_inv.GetSecretData(InsecureSecretKeyAccess::Get()), + }); + return rsa.status(); } } // namespace diff --git a/tink/jwt/jwt_rsa_ssa_pkcs1_private_key_test.cc b/tink/jwt/jwt_rsa_ssa_pkcs1_private_key_test.cc index f7d1d9b6..cd2b99fd 100644 --- a/tink/jwt/jwt_rsa_ssa_pkcs1_private_key_test.cc +++ b/tink/jwt/jwt_rsa_ssa_pkcs1_private_key_test.cc @@ -372,7 +372,7 @@ TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesModulus) { EXPECT_THAT(private_key_modified_modulus.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Modulus size is"))); } TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrimeP) { @@ -397,7 +397,7 @@ TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrimeP) { EXPECT_THAT(private_key_modified_prime_p.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrimeQ) { @@ -422,7 +422,7 @@ TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrimeQ) { EXPECT_THAT(private_key_modified_prime_q.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrimeExponentP) { @@ -448,7 +448,7 @@ TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrimeExponentP) { EXPECT_THAT(private_key_modified_prime_exponent_p.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrimeExponentQ) { @@ -474,7 +474,7 @@ TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrimeExponentQ) { EXPECT_THAT(private_key_modified_prime_exponent_q.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrivateExponent) { @@ -500,7 +500,7 @@ TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesPrivateExponent) { EXPECT_THAT(private_key_modified_private_exponent.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesCrtCoefficient) { @@ -526,7 +526,7 @@ TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPrivateKeyValidatesCrtCoefficient) { EXPECT_THAT(private_key_modified_crt_coefficient.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPkcs1PrivateKeyTest, BuildPublicKeyNotSetFails) { @@ -727,7 +727,7 @@ TEST(JwtRsaSsaPkcs1PrivateKeyTest, CreateMismatchedKeyPairFails) { EXPECT_THAT(private_key.status(), StatusIs(absl::StatusCode ::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST_P(JwtRsaSsaPkcs1PrivateKeyTest, PrivateKeyEquals) { diff --git a/tink/jwt/jwt_rsa_ssa_pss_private_key.cc b/tink/jwt/jwt_rsa_ssa_pss_private_key.cc index 09598ad3..b6e7e22f 100644 --- a/tink/jwt/jwt_rsa_ssa_pss_private_key.cc +++ b/tink/jwt/jwt_rsa_ssa_pss_private_key.cc @@ -16,14 +16,17 @@ #include "tink/jwt/jwt_rsa_ssa_pss_private_key.h" +#include + #include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "tink/internal/rsa_util.h" #ifdef OPENSSL_IS_BORINGSSL #include "openssl/base.h" #endif #include "openssl/rsa.h" #include "tink/big_integer.h" #include "tink/insecure_secret_key_access.h" -#include "tink/internal/bn_util.h" #include "tink/internal/ssl_unique_ptr.h" #include "tink/jwt/jwt_rsa_ssa_pss_public_key.h" #include "tink/key.h" @@ -41,94 +44,23 @@ util::Status ValidateKeyPair( const RestrictedBigInteger& p, const RestrictedBigInteger& q, const RestrictedBigInteger& d, const RestrictedBigInteger& dp, const RestrictedBigInteger& dq, const RestrictedBigInteger& q_inv) { - internal::SslUniquePtr rsa(RSA_new()); - if (rsa.get() == nullptr) { - return util::Status(absl::StatusCode::kInternal, - "Internal RSA allocation error"); - } - - util::StatusOr> n = - internal::StringToBignum(modulus.GetValue()); - if (!n.ok()) { - return n.status(); - } - - util::StatusOr> e = - internal::StringToBignum(public_exponent.GetValue()); - if (!e.ok()) { - return e.status(); - } - - util::StatusOr> d_bn = - internal::StringToBignum(d.GetSecret(InsecureSecretKeyAccess::Get())); - if (!d_bn.ok()) { - return d_bn.status(); - } - - util::StatusOr> p_bn = - internal::StringToBignum(p.GetSecret(InsecureSecretKeyAccess::Get())); - if (!p_bn.ok()) { - return p_bn.status(); - } - util::StatusOr> q_bn = - internal::StringToBignum(q.GetSecret(InsecureSecretKeyAccess::Get())); - if (!q_bn.ok()) { - return q_bn.status(); - } - - util::StatusOr> dp_bn = - internal::StringToBignum(dp.GetSecret(InsecureSecretKeyAccess::Get())); - if (!dp_bn.ok()) { - return dp_bn.status(); - } - util::StatusOr> dq_bn = - internal::StringToBignum(dq.GetSecret(InsecureSecretKeyAccess::Get())); - if (!dq_bn.ok()) { - return dq_bn.status(); - } - util::StatusOr> q_inv_bn = - internal::StringToBignum(q_inv.GetSecret(InsecureSecretKeyAccess::Get())); - if (!q_inv_bn.ok()) { - return q_inv_bn.status(); - } - - // Build RSA key from the given values. The RSA object takes ownership of the - // given values after the call. - if (RSA_set0_key(rsa.get(), n->release(), e->release(), d_bn->release()) != - 1 || - RSA_set0_factors(rsa.get(), p_bn->release(), q_bn->release()) != 1 || - RSA_set0_crt_params(rsa.get(), dp_bn->release(), dq_bn->release(), - q_inv_bn->release()) != 1) { - return util::Status(absl::StatusCode::kInternal, - "Internal RSA key loading error"); - } - - // Validate key. - int check_key_status = RSA_check_key(rsa.get()); - if (check_key_status == 0) { - return util::Status(absl::StatusCode::kInvalidArgument, - "RSA key pair is not valid"); - } - - if (check_key_status == -1) { - return util::Status(absl::StatusCode::kInternal, - "An error ocurred while checking the key"); - } - -#ifdef OPENSSL_IS_BORINGSSL - if (RSA_check_fips(rsa.get()) == 0) { - return util::Status(absl::StatusCode::kInvalidArgument, - "RSA key pair is not valid in FIPS mode"); - } -#endif - - return util::OkStatus(); + absl::StatusOr> rsa = + internal::RsaPrivateKeyToRsa(internal::RsaPrivateKey{ + /*n=*/std::string(modulus.GetValue()), + /*e=*/std::string(public_exponent.GetValue()), + /*d=*/d.GetSecretData(InsecureSecretKeyAccess::Get()), + /*p=*/p.GetSecretData(InsecureSecretKeyAccess::Get()), + /*q=*/q.GetSecretData(InsecureSecretKeyAccess::Get()), + /*dp=*/dp.GetSecretData(InsecureSecretKeyAccess::Get()), + /*dq=*/dq.GetSecretData(InsecureSecretKeyAccess::Get()), + /*crt=*/q_inv.GetSecretData(InsecureSecretKeyAccess::Get()), + }); + return rsa.status(); } } // namespace -JwtRsaSsaPssPrivateKey::Builder& -JwtRsaSsaPssPrivateKey::Builder::SetPublicKey( +JwtRsaSsaPssPrivateKey::Builder& JwtRsaSsaPssPrivateKey::Builder::SetPublicKey( const JwtRsaSsaPssPublicKey& public_key) { public_key_ = public_key; return *this; @@ -174,8 +106,8 @@ JwtRsaSsaPssPrivateKey::Builder::SetCrtCoefficient( return *this; } -util::StatusOr -JwtRsaSsaPssPrivateKey::Builder::Build(PartialKeyAccessToken token) { +util::StatusOr JwtRsaSsaPssPrivateKey::Builder::Build( + PartialKeyAccessToken token) { if (!public_key_.has_value()) { return util::Status(absl::StatusCode::kInvalidArgument, "Cannot build without setting the public key"); @@ -210,7 +142,7 @@ JwtRsaSsaPssPrivateKey::Builder::Build(PartialKeyAccessToken token) { } return JwtRsaSsaPssPrivateKey(*public_key_, *p_, *q_, *dp_, *dq_, *d_, - *q_inv_); + *q_inv_); } bool JwtRsaSsaPssPrivateKey::operator==(const Key& other) const { diff --git a/tink/jwt/jwt_rsa_ssa_pss_private_key_test.cc b/tink/jwt/jwt_rsa_ssa_pss_private_key_test.cc index b63760be..954d5775 100644 --- a/tink/jwt/jwt_rsa_ssa_pss_private_key_test.cc +++ b/tink/jwt/jwt_rsa_ssa_pss_private_key_test.cc @@ -367,7 +367,7 @@ TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesModulus) { EXPECT_THAT(private_key_modified_modulus.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Modulus size is"))); } TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrimeP) { @@ -392,7 +392,7 @@ TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrimeP) { EXPECT_THAT(private_key_modified_prime_p.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrimeQ) { @@ -417,7 +417,7 @@ TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrimeQ) { EXPECT_THAT(private_key_modified_prime_q.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrimeExponentP) { @@ -442,7 +442,7 @@ TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrimeExponentP) { EXPECT_THAT(private_key_modified_prime_exponent_p.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrimeExponentQ) { @@ -467,7 +467,7 @@ TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrimeExponentQ) { EXPECT_THAT(private_key_modified_prime_exponent_q.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrivateExponent) { @@ -492,7 +492,7 @@ TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesPrivateExponent) { EXPECT_THAT(private_key_modified_private_exponent.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesCrtCoefficient) { @@ -517,7 +517,7 @@ TEST(JwtRsaSsaPssPrivateKeyTest, BuildPrivateKeyValidatesCrtCoefficient) { EXPECT_THAT(private_key_modified_crt_coefficient.status(), StatusIs(absl::StatusCode::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST(JwtRsaSsaPssPrivateKeyTest, BuildPublicKeyNotSetFails) { @@ -718,7 +718,7 @@ TEST(JwtRsaSsaPssPrivateKeyTest, CreateMismatchedKeyPairFails) { EXPECT_THAT(private_key.status(), StatusIs(absl::StatusCode ::kInvalidArgument, - HasSubstr("RSA key pair is not valid"))); + HasSubstr("Could not load RSA key"))); } TEST_P(JwtRsaSsaPssPrivateKeyTest, PrivateKeyEquals) {