This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new df8cb46f7 IMPALA-14038: Pull in KUDU-3663 to handle certs with
RSASSA-PSS
df8cb46f7 is described below
commit df8cb46f75105ee27d1abf2c4a7c512ba1570643
Author: Joe McDonnell <[email protected]>
AuthorDate: Tue May 20 10:20:13 2025 -0700
IMPALA-14038: Pull in KUDU-3663 to handle certs with RSASSA-PSS
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: I1df2ce4cac2ed13ea0668ffeaadff10dc83a3d38
Reviewed-on: http://gerrit.cloudera.org:8080/22923
Reviewed-by: Jason Fehr <[email protected]>
Tested-by: Impala Public Jenkins <[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 83e5fb9ce..a7f6517e6 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 <string>
@@ -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 f6e9c8e6a..a1c592800 100644
--- a/be/src/kudu/security/cert.cc
+++ b/be/src/kudu/security/cert.cc
@@ -34,6 +34,8 @@
#include <boost/optional/optional.hpp>
#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"
@@ -42,6 +44,7 @@
using std::string;
using std::vector;
+using strings::Substitute;
namespace kudu {
namespace security {
@@ -164,7 +167,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.
@@ -179,9 +182,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
@@ -190,10 +206,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 4f9bb016f..673723b16 100644
--- a/be/src/kudu/security/cert.h
+++ b/be/src/kudu/security/cert.h
@@ -77,6 +77,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 0c5b29e4a..9b4ee1663 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 7767cb249..d3a4ae938 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[];
