This is an automated email from the ASF dual-hosted git repository. dbecker pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit eddf075a7af8c5d2e76e9613142260333e3d65b1 Author: Joe McDonnell <[email protected]> AuthorDate: Tue May 20 10:20:13 2025 -0700 IMPALA-14066 (Part 6): Re-applying IMPALA-14038: Pull in KUDU-3663 to handle certs with RSASSA-PSS This commit re-applies IMPALA-14038 to the Kudu files after the Kudu rebase to v1.17.1. Testing: - exhaustive tests have passed The original commit message is below: The existing KRPC code to determine the hash algorithm for a certificate does not handle RSASSA-PSS signatures as the hash algorithm is configurable for RSASSA-PSS. This was addressed in Kudu with KUDU-3663. That fix uses OpenSSL 1.1.1's x509_get_signature_info() function, which is able to determine the hash algorithm even for RSASSA-PSS. This is similar to the fix that Postgres did in a similar situation. It does not support RSASSA-PSS on OpenSSL 1.0.2, but it improves the error message in that case. Testing: - Kudu added a unit test that passes Change-Id: I162efac2d68c2bfb34a5086557182f68670d8c2a Reviewed-on: http://gerrit.cloudera.org:8080/22923 Reviewed-by: Jason Fehr <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Reviewed-on: http://gerrit.cloudera.org:8080/23015 Reviewed-by: Daniel Becker <[email protected]> Tested-by: Daniel Becker <[email protected]> --- be/src/kudu/security/cert-test.cc | 26 +++++++++++++++ be/src/kudu/security/cert.cc | 41 ++++++++++++++++++++--- be/src/kudu/security/cert.h | 6 ++++ be/src/kudu/security/test/test_certs.cc | 59 +++++++++++++++++++++++++++++++++ be/src/kudu/security/test/test_certs.h | 5 +++ 5 files changed, 132 insertions(+), 5 deletions(-) diff --git a/be/src/kudu/security/cert-test.cc b/be/src/kudu/security/cert-test.cc index 17e29d600..683117ce2 100644 --- a/be/src/kudu/security/cert-test.cc +++ b/be/src/kudu/security/cert-test.cc @@ -17,6 +17,7 @@ #include "kudu/security/cert.h" +#include <openssl/crypto.h> #include <openssl/obj_mac.h> #include <optional> @@ -57,9 +58,13 @@ class CertTest : public KuduTest { ASSERT_OK(ca_exp_cert_.FromString(kCaExpiredCert, DataFormat::PEM)); ASSERT_OK(ca_exp_private_key_.FromString(kCaExpiredPrivateKey, DataFormat::PEM)); + ASSERT_OK(ca_rsassapss_cert_.FromString(kCaRsassaPssCert, DataFormat::PEM)); + ASSERT_OK(ca_rsassapss_private_key_.FromString(kCaRsassaPssPrivateKey, + DataFormat::PEM)); // Sanity checks. ASSERT_OK(ca_cert_.CheckKeyMatch(ca_private_key_)); ASSERT_OK(ca_exp_cert_.CheckKeyMatch(ca_exp_private_key_)); + ASSERT_OK(ca_rsassapss_cert_.CheckKeyMatch(ca_rsassapss_private_key_)); } protected: @@ -69,6 +74,9 @@ class CertTest : public KuduTest { Cert ca_exp_cert_; PrivateKey ca_exp_private_key_; + + Cert ca_rsassapss_cert_; + PrivateKey ca_rsassapss_private_key_; }; // Regression test to make sure that GetKuduKerberosPrincipalOidNid is thread @@ -164,5 +172,23 @@ TEST_F(CertTest, DnsHostnameInSanField) { EXPECT_EQ(hostname_too_long, hostnames[2]); } +TEST_F(CertTest, SignatureHashAlgorithm) { + int digest_nid = NID_undef; + Status status = ca_cert_.GetSignatureHashAlgorithm(&digest_nid); + EXPECT_OK(status); + EXPECT_EQ(digest_nid, NID_sha256); + + digest_nid = NID_undef; + status = ca_rsassapss_cert_.GetSignatureHashAlgorithm(&digest_nid); +#if OPENSSL_VERSION_NUMBER >= 0x10101000L + EXPECT_OK(status); + EXPECT_EQ(digest_nid, NID_sha256); +#else + EXPECT_FALSE(status.ok()); + EXPECT_TRUE(status.IsNotSupported()); +#endif + +} + } // namespace security } // namespace kudu diff --git a/be/src/kudu/security/cert.cc b/be/src/kudu/security/cert.cc index b84e2bada..e9a9b468c 100644 --- a/be/src/kudu/security/cert.cc +++ b/be/src/kudu/security/cert.cc @@ -34,6 +34,8 @@ #include <glog/logging.h> +#include "kudu/gutil/port.h" +#include "kudu/gutil/strings/substitute.h" #include "kudu/gutil/macros.h" #include "kudu/security/crypto.h" #include "kudu/util/openssl_util.h" @@ -44,6 +46,7 @@ using std::nullopt; using std::optional; using std::string; using std::vector; +using strings::Substitute; namespace kudu { namespace security { @@ -171,7 +174,7 @@ Status Cert::CheckKeyMatch(const PrivateKey& key) const { return Status::OK(); } -Status Cert::GetServerEndPointChannelBindings(string* channel_bindings) const { +Status Cert::GetSignatureHashAlgorithm(int* digest_nid_out) const { SCOPED_OPENSSL_NO_PENDING_ERRORS; // Find the signature type of the certificate. This corresponds to the digest // (hash) algorithm, and the public key type which signed the cert. @@ -186,9 +189,22 @@ Status Cert::GetServerEndPointChannelBindings(string* channel_bindings) const { #endif // Retrieve the digest algorithm type. + // + // Prefer OpenSSL 1.1.1's X509_get_signature_info when available, as it has extra + // handling to determine the hash algorithm for signature types that store the hash + // algorithm separately (e.g. RSASSA-PSS). int digest_nid; - int public_key_nid; - OBJ_find_sigid_algs(signature_nid, &digest_nid, &public_key_nid); +#if OPENSSL_VERSION_NUMBER >= 0x10101000L + X509_get_signature_info(GetTopOfChainX509(), &digest_nid, nullptr, nullptr, nullptr); +#else + // Return a better error message for RSASSA-PSS, because we know that + // OBJ_find_sigid_algs() won't handle it properly. + if (PREDICT_FALSE(signature_nid == NID_rsassaPss)) { + return Status::NotSupported("Server certificate uses an RSASSA-PSS signature. " + "RSASSA-PSS signatures are not supported for OpenSSL < 1.1.1"); + } + OBJ_find_sigid_algs(signature_nid, &digest_nid, nullptr); +#endif // RFC 5929: if the certificate's signatureAlgorithm uses no hash functions or // uses multiple hash functions, then this channel binding type's channel @@ -197,10 +213,25 @@ Status Cert::GetServerEndPointChannelBindings(string* channel_bindings) const { // // TODO(dan): can the multiple hash function scenario actually happen? What // does OBJ_find_sigid_algs do in that scenario? - if (digest_nid == NID_undef) { - return Status::NotSupported("server certificate has no signature digest (hash) algorithm"); + if (PREDICT_FALSE(digest_nid == NID_undef)) { + std::string signature_type(OBJ_nid2ln(signature_nid)); + return Status::NotSupported(Substitute( + "server certificate using '$0' signature algorithm has no signature " + "digest (hash) algorithm", signature_type)); } + *digest_nid_out = digest_nid; + return Status::OK(); +} + +Status Cert::GetServerEndPointChannelBindings(string* channel_bindings) const { + SCOPED_OPENSSL_NO_PENDING_ERRORS; + + // Get the hash algorithm used for the signature + int digest_nid; + RETURN_NOT_OK(GetSignatureHashAlgorithm(&digest_nid)); + DCHECK_NE(digest_nid, NID_undef); + // RFC 5929: if the certificate's signatureAlgorithm uses a single hash // function, and that hash function is either MD5 [RFC1321] or SHA-1 // [RFC3174], then use SHA-256 [FIPS-180-3]; diff --git a/be/src/kudu/security/cert.h b/be/src/kudu/security/cert.h index 3a00ef4be..d729403d2 100644 --- a/be/src/kudu/security/cert.h +++ b/be/src/kudu/security/cert.h @@ -72,6 +72,12 @@ class Cert : public RawDataWrapper<STACK_OF(X509)> { // Return Status::OK() if key match the end-user certificate. Status CheckKeyMatch(const PrivateKey& key) const WARN_UNUSED_RESULT; + // Determine the digest (hash) algorithm used in the signature of the certificate + // as needed to implement RFC 5929. If the hash algorithm can be determined, + // return the NID in digest_nid_out and return Status::OK(). Otherwise, return + // an error. + Status GetSignatureHashAlgorithm(int* digest_nid_out) const WARN_UNUSED_RESULT; + // Returns the 'tls-server-end-point' channel bindings for the end-user certificate as // specified in RFC 5929. Status GetServerEndPointChannelBindings(std::string* channel_bindings) const WARN_UNUSED_RESULT; diff --git a/be/src/kudu/security/test/test_certs.cc b/be/src/kudu/security/test/test_certs.cc index 8268b1ed1..e47c4ae53 100644 --- a/be/src/kudu/security/test/test_certs.cc +++ b/be/src/kudu/security/test/test_certs.cc @@ -270,6 +270,65 @@ an4ys5seqeHuK2WzP3NAx7LOwe/R1kHpEAX/Al6xyLIY3h7BBzurpgfrO6hTTECF -----END CERTIFICATE----- )***"; +// This uses a similar process as kCaCert_, except it specifies +// RSASSA-PSS via '-sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:64' +const char kCaRsassaPssCert[] = R"***( +-----BEGIN CERTIFICATE----- +MIID3zCCApegAwIBAgIUPm3WcVM4xPYuHt88YFMJaTxadMUwPQYJKoZIhvcNAQEK +MDCgDTALBglghkgBZQMEAgGhGjAYBgkqhkiG9w0BAQgwCwYJYIZIAWUDBAIBogMC +AUAwTjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQswCQYDVQQHDAJTRjERMA8G +A1UECgwIQ2xvdWRlcmExEjAQBgNVBAMMCWxvY2FsaG9zdDAgFw0yNTA1MTYyMTIx +MjdaGA8yMDUyMTAwMTIxMjEyN1owTjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNB +MQswCQYDVQQHDAJTRjERMA8GA1UECgwIQ2xvdWRlcmExEjAQBgNVBAMMCWxvY2Fs +aG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKSUIlO7untRydez +wAmpzDHMWE0wa/2RU+UDUDvVyyY6CyoDWTkNnAPWQKDgxDE2TboakKKivuiDseRV +ztZjUZE93O0oJZtRgEsvH5UTsdxD1ggv3q5cbN7lKOpjd56p1rWrmW6lBghxt1ID +eW9YDqZUQJH4ea36WEAlZiQwDexHhq1ed7yA3Ht7p+fyJvEy0/amF1n4rh00nxht +ofzGdMzTQ5dqf8I9qr9mqKcs5hslc2Vs4+zXckUncbmAB2m/mVItyCev7Uy/FfG/ +k6nc+AaXQcTn7xQwD4MJRnAgDDybdNCZ9Idobml1YCmrVuE6BD7qsT8SoEoAr5Ri +Lw6J+YsCAwEAAaNTMFEwHQYDVR0OBBYEFLGZLVEQNR1qeA3fN3TgzgAF75cmMB8G +A1UdIwQYMBaAFLGZLVEQNR1qeA3fN3TgzgAF75cmMA8GA1UdEwEB/wQFMAMBAf8w +PQYJKoZIhvcNAQEKMDCgDTALBglghkgBZQMEAgGhGjAYBgkqhkiG9w0BAQgwCwYJ +YIZIAWUDBAIBogMCAUADggEBAIuAX+VP1+4H3POcQYtegEkDOw+mV5JrYpu1K2Qw +K9AMtfdfafWfIGGYBPZ/tcPsfXxjy7KX9l5B8RAfCiqbCbQ1N5MbukqgsKkw2eNr +vgFzMcRfy9tif3bP5ytPLhoPla8gsa5x0Lng2P5MaWeA9pJdO8OIbhtLUwCC/KTe +Z84T2499hu3RJgDIfYi0QIFKVEltQRH4+5zP8Ay7buJICjMCkmKYDoHJAVDYlArI +VHqg4qNVcY76E7urByeLx3NJ45RIT01u6GbC/V7Rhq+lSa2ON0xAsyhqHj5Hq+1Q +j3Om2w+ByZEDJE0e/YKeFHk7mWjyFtigMO388sMgU007PXE= +-----END CERTIFICATE----- +)***"; + +// See comment on kCaRsassaPssCert +const char kCaRsassaPssPrivateKey[] = R"***( +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEApJQiU7u6e1HJ17PACanMMcxYTTBr/ZFT5QNQO9XLJjoLKgNZ +OQ2cA9ZAoODEMTZNuhqQoqK+6IOx5FXO1mNRkT3c7Sglm1GASy8flROx3EPWCC/e +rlxs3uUo6mN3nqnWtauZbqUGCHG3UgN5b1gOplRAkfh5rfpYQCVmJDAN7EeGrV53 +vIDce3un5/Im8TLT9qYXWfiuHTSfGG2h/MZ0zNNDl2p/wj2qv2aopyzmGyVzZWzj +7NdyRSdxuYAHab+ZUi3IJ6/tTL8V8b+Tqdz4BpdBxOfvFDAPgwlGcCAMPJt00Jn0 +h2huaXVgKatW4ToEPuqxPxKgSgCvlGIvDon5iwIDAQABAoIBAEi3vzcaInpsl+97 +16UtZjC2pmlstLp0JQpyXVgizcEVMmuc0SZ5Ue8MEsBCr81CvjM1m6SQniOkVMyb +8WketyKin+QVshAfgb02lBDNg+/b9UzmwdBuvBf8TwjJbEgpqNnaeU+/EJxYinRt +XpGI6egqH+GfVTw++hFVtPzWUsCL4D5XnwC+ggTTIL022IYM7oRcsWREN0lNSj0I +EIivp9I7+t6hfObo49KJlKuvcx+4DytaLzF5J0Om6aChYVRgbMrfv+QXOd8w5GWy +vZlpM3uEK9FfA/qYJMCev9PFj2BT+dfDjGz2SyP6PdH6JxjXYqrSX9+4aUAjl7kh +ZLXFLvECgYEA2tIdZVetwoUQ4zCoZUg64e/5LEcJzz4Ds+qcHoc3pMIByPFfy1Zs +L6vMgRx0G9uKNUchbw8ATkxgisfxNOA1Q3h/hfZmOllFikX4/N2XsGNXoRfTYmuS +eERvSG5X3+YFADiggGGo61BJN8BXnRplVJUkqH/kyetee1doE+OUxgMCgYEAwIqw +rHmfTArLJ03rZu3MVEmtXw4uJbv5N676cODWiGFsaU59P7q0pRDrfKzoYZT8/IIG +BFGWqur5nmRc89DXvjQHHlPGPz/McIifuQTYEBXTtxUcNhOK7xHxV9mvmoVTSopv +lNX43p+nXRLSMzQkSIaa2+EiNk3ULoVNOk2kC9kCgYAhBYhOHNcp/a64ukUPU8Ef +C3nMxsOiNLeWVRdOPBWXlXdzfYl5RAd7gi+QZFzZP14yABP5kIf5SOlgyB+MXTFs +hyinbLGsqIAoB7s1XbNgeP1mYBQUTCuEXr90bMJyFWI30FPYS+ST7j++XBZcrPkR +tJgdnX9HQW+2qVAZgESZRQKBgCts3E36HEhtQsZ5l3ceePAlsdl3fEb8b0f0yf09 +aIVX27iggDUoaee0ujfjU4H2tVxKAwtkT2P7HRNxNVm0J4R5fYWEhXjsbbKPzd5P +zl9KXPa05yj3HWWwGUukCCwEl/V+5Y2e+MNVJM0kGo572xcUbMbcrveqdAmN/Q4C +RtZ5AoGBAI5vyaO13QgVzsAfuX4uhWLa/X16ht1FV7prDAZ13T+wfNzHGf4LFd8D +HI3kpN7QlkdSGHa5Izr5d3Nv/N6AEDB2RIVxV/p3HXSQYxFWyPhHVR5/8i5V/JTj +dN0jgPAyvyrffMc9J0mGh5Gmbg9HiuT51fMRz65Qpp+BIDkztR2z +-----END RSA PRIVATE KEY----- +)***"; + // // The reference signatures were obtained by using the following sequence: // 0. The reference private key was saved into /tmp/ca.pkey.pem file. diff --git a/be/src/kudu/security/test/test_certs.h b/be/src/kudu/security/test/test_certs.h index 8c571ce34..a68aa17e7 100644 --- a/be/src/kudu/security/test/test_certs.h +++ b/be/src/kudu/security/test/test_certs.h @@ -46,6 +46,11 @@ extern const char kCaExpiredPublicKey[]; // Certificate with multiple DNS hostnames in the SAN field. extern const char kCertDnsHostnamesInSan[]; +// Valid root CA certificate that uses RSASSA-PSS signature (PEM format) +extern const char kCaRsassaPssCert[]; +// The private key for the certificate using RSASSA-PSS +extern const char kCaRsassaPssPrivateKey[]; + extern const char kDataTiny[]; extern const char kSignatureTinySHA512[];
