This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 90508b45596052c377415e673bb55dae116dc522
Author: jasonmfehr <jf...@cloudera.com>
AuthorDate: Fri Jul 11 09:05:20 2025 -0700
IMPALA-14217: Fixes Incompatibility with OpenSSL 1.0
A new function `ValidatePemBundle` was introduced in openssl-utl.cc
under IMPALA-13237. This function used the X509_get0_notBefore and
X509_get0_notAfter functions which were introduced in OpenSSL 1.1.0.
Fixes compilation errors with OpenSSL 1.0.x by reverting to the older
X509_get_notBefore and X509_get_notAfter functions. These functions
were deprecated in OpenSSL 1.1.1 but are drop-in replacements for
X509_get0_notBefore and X509_get0_notAfter differing only in the
function return being mutable vs const.
Fixes flaky tests failing because different versions of OpenSSL set
different error codes in the BIO_new_mem_buf() function.
Tested locally by compiling and running openssl-util-test ctests.
Also built and ran tests using Jenkins on RHEL 8.6 and CentOS 7.4.
Change-Id: If58a12f14a5509d62b7cfe291372b53b440da511
Reviewed-on: http://gerrit.cloudera.org:8080/23161
Reviewed-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
---
be/src/testutil/gtest-util.h | 10 ++++++++++
be/src/util/openssl-util-test.cc | 21 ++++++++++-----------
be/src/util/openssl-util.cc | 4 ++--
3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/be/src/testutil/gtest-util.h b/be/src/testutil/gtest-util.h
index 88fa7d3e9..e5d641ab2 100644
--- a/be/src/testutil/gtest-util.h
+++ b/be/src/testutil/gtest-util.h
@@ -18,6 +18,8 @@
#ifndef IMPALA_TESTUTIL_GTEST_UTIL_H_
#define IMPALA_TESTUTIL_GTEST_UTIL_H_
+#include <regex>
+
#include <gtest/gtest.h>
#include "common/init.h"
#include "common/status.h"
@@ -65,5 +67,13 @@ namespace impala {
return RUN_ALL_TESTS(); \
} \
+// Asserts a string matches the provided regex pattern string.
+#define ASSERT_REGEX_MATCH(actual_str, regex_str)
\
+ do {
\
+ ASSERT_TRUE(std::regex_search(actual_str, std::regex(regex_str)))
\
+ << "Expected regex '" << regex_str << "' to match string '"
\
+ << actual_str << "'";
\
+ } while (0)
+
}
#endif // IMPALA_TESTUTIL_GTEST_UTIL_H_
diff --git a/be/src/util/openssl-util-test.cc b/be/src/util/openssl-util-test.cc
index 384536f9f..29f26437e 100644
--- a/be/src/util/openssl-util-test.cc
+++ b/be/src/util/openssl-util-test.cc
@@ -17,6 +17,7 @@
#include <random>
+#include <glog/logging.h>
#include <gtest/gtest.h>
#include <openssl/err.h>
#include <openssl/rand.h>
@@ -261,7 +262,7 @@ static const string& VALID_CERT_2 = "wildcardCA";
/// Constants and functions defining expected error messages.
static const string& MSG_NEW_BIO_ERR = "OpenSSL error in PEM_read_bio_X509
unexpected "
- "error '$0' while reading PEM bundle";
+ "error '\\d+' while reading PEM bundle";
static const string& MSG_NONE_VALID = "PEM bundle contains no valid
certificates";
static string _msg_time(int invalid_notbefore_cnt, int invalid_notafter_cnt) {
return Substitute("PEM bundle contains $0 invalid certificate(s) with
notBefore in the "
@@ -279,9 +280,9 @@ static string read_cert(const string& cert_name) {
Substitute("$0/be/src/testutil/$1.pem", getenv("IMPALA_HOME"),
cert_name),
&contents);
- EXPECT_TRUE(s.ok())<< "Certificate '" << cert_name << "' could not be read: "
+ CHECK(s.ok())<< "Certificate '" << cert_name << "' could not be read: "
<< s.ToString();
- EXPECT_FALSE(contents.ToString().empty()) << "Certificate '" << cert_name
+ CHECK(!contents.ToString().empty()) << "Certificate '" << cert_name
<< "' is empty. Please check the test data.";
string cert = contents.ToString();
@@ -387,14 +388,12 @@ TEST_F(OpenSSLUtilTest,
ValidatePemBundleThreeCertsOneInvalid) {
read_cert(VALID_CERT_1), read_cert(VALID_CERT_2))), MSG_NONE_VALID);
// Invalid cert is in the middle of the bundle.
- EXPECT_STR_CONTAINS(ValidatePemBundle(Substitute("$0$1$2",
read_cert(VALID_CERT_1),
- INVALID_CERT_TEXT, read_cert(VALID_CERT_2))).GetDetail(),
- Substitute(MSG_NEW_BIO_ERR, 123));
+ ASSERT_REGEX_MATCH(ValidatePemBundle(Substitute("$0$1$2",
read_cert(VALID_CERT_1),
+ INVALID_CERT_TEXT, read_cert(VALID_CERT_2))).GetDetail(),
MSG_NEW_BIO_ERR);
// Invalid cert is last in the bundle.
- EXPECT_STR_CONTAINS(ValidatePemBundle(Substitute("$0$1$2",
read_cert(VALID_CERT_1),
- read_cert(VALID_CERT_2), INVALID_CERT_TEXT)).GetDetail(),
- Substitute(MSG_NEW_BIO_ERR, 112));
+ ASSERT_REGEX_MATCH(ValidatePemBundle(Substitute("$0$1$2",
read_cert(VALID_CERT_1),
+ read_cert(VALID_CERT_2), INVALID_CERT_TEXT)).GetDetail(),
MSG_NEW_BIO_ERR);
}
/// Asserts a bundle is invalid if it contains one expired and one non-expired
certificate
@@ -429,8 +428,8 @@ TEST_F(OpenSSLUtilTest,
ValidatePemBundleTwoCertsNotCertFirst) {
read_cert(VALID_CERT_1))), MSG_NONE_VALID);
// Invalid cert is last in the bundle.
- EXPECT_STR_CONTAINS(ValidatePemBundle(Substitute("$0$1",
read_cert(VALID_CERT_1),
- INVALID_CERT_TEXT)).GetDetail(), Substitute(MSG_NEW_BIO_ERR, 112));
+ ASSERT_REGEX_MATCH(ValidatePemBundle(Substitute("$0$1",
read_cert(VALID_CERT_1),
+ INVALID_CERT_TEXT)).GetDetail(), MSG_NEW_BIO_ERR);
}
/// Asserts a bundle is invalid if it contains two valid, one expired, and one
future
diff --git a/be/src/util/openssl-util.cc b/be/src/util/openssl-util.cc
index bb65e21e8..cad078565 100644
--- a/be/src/util/openssl-util.cc
+++ b/be/src/util/openssl-util.cc
@@ -179,11 +179,11 @@ Status ValidatePemBundle(const string& bundle) {
while ((cert = PEM_read_bio_X509(bio, nullptr, nullptr, nullptr)) !=
nullptr) {
cert_count++;
// Check certificate validity (notBefore and notAfter)
- if (UNLIKELY(X509_cmp_current_time(X509_get0_notBefore(cert)) > 0)) {
+ if (UNLIKELY(X509_cmp_current_time(X509_get_notBefore(cert)) > 0)) {
invalid_notbefore_cnt++;
}
- if (UNLIKELY(X509_cmp_current_time(X509_get0_notAfter(cert)) < 0)) {
+ if (UNLIKELY(X509_cmp_current_time(X509_get_notAfter(cert)) < 0)) {
invalid_notafter_cnt++;
}
