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
commit c5ec833aec58425d203fbc452058ea837ad2ae75
Author: Joe McDonnell <joemcdonn...@cloudera.com>
AuthorDate: Tue Jun 13 21:38:35 2023 -0700
IMPALA-12211: Adapt FIPS detection to handle OpenSSL 3
Various pieces of our code detect FIPS and enable/disable
code and tests accordingly (e.g. old hashing algorithms
like MD5 are not allowed). The detection logic currently
uses FIPS_mode(), but FIPS_mode() was removed in OpenSSL 3.
The OpenSSL 3 equivalent is EVP_default_properties_is_fips_enabled().
This adds a new IsFIPSMode() function in util/openssl-util.h
that uses OPENSSL_VERSION_NUMBER to dispatch to either
FIPS_mode() or EVP_default_properties_is_fips_enabled().
This also pulls in the Kudu changes to handle this for KRPC.
The original upstream Kudu commits were:
c24629083e : Follow-up on OpenSSL 3 FIPS_mode removal
acac73ecda : Fix OpenSSL3 FIPS_mode() issue on RHEL9.1
Testing:
- Built on Ubuntu 22 / Redhat 9 with OpenSSL 3
Change-Id: I78571c8086bea06bf7b355cfe86a16f190cde067
Reviewed-on: http://gerrit.cloudera.org:8080/20072
Reviewed-by: Michael Smith <michael.sm...@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
---
be/src/exprs/expr-test.cc | 10 +++++-----
be/src/exprs/mask-functions-ir.cc | 4 ++--
be/src/exprs/utility-functions-ir.cc | 9 ++++-----
be/src/kudu/util/openssl_util.cc | 6 +++++-
be/src/kudu/util/openssl_util.h | 4 ++++
be/src/util/openssl-util.h | 16 ++++++++++++----
be/src/util/webserver-test.cc | 6 +++---
be/src/util/webserver.cc | 3 ++-
8 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc
index a0be31492..5edd34e65 100644
--- a/be/src/exprs/expr-test.cc
+++ b/be/src/exprs/expr-test.cc
@@ -29,7 +29,6 @@
#include <boost/scoped_ptr.hpp>
#include <boost/unordered_map.hpp>
-#include <openssl/crypto.h>
#include <openssl/sha.h>
#include "codegen/llvm-codegen.h"
@@ -71,6 +70,7 @@
#include "util/debug-util.h"
#include "util/decimal-util.h"
#include "util/metrics.h"
+#include "util/openssl-util.h"
#include "util/string-parser.h"
#include "util/string-util.h"
#include "util/test-info.h"
@@ -5706,7 +5706,7 @@ TEST_P(ExprTest, SHAFunctions) {
unsigned char sha384[SHA384_DIGEST_LENGTH];
unsigned char sha512[SHA512_DIGEST_LENGTH];
- if (FIPS_mode()) {
+ if (IsFIPSMode()) {
TestError(sha1fn);
TestError(sha2fn + ", 224)");
TestError(sha2fn + ", 256)");
@@ -5733,7 +5733,7 @@ TEST_P(ExprTest, SHAFunctions) {
TestStringValue(sha2fn + ", 512)", expected);
// Test empty strings. Empty string is valid input and produces hash.
- if (!FIPS_mode()) {
+ if (!IsFIPSMode()) {
SHA1(input, 0, sha1);
expected = ToHex(sha1, SHA_DIGEST_LENGTH);
TestStringValue("sha1('')", expected);
@@ -5764,7 +5764,7 @@ TEST_P(ExprTest, SHAFunctions) {
}
TEST_P(ExprTest, MD5Function) {
- if (FIPS_mode()) {
+ if (IsFIPSMode()) {
TestError("md5('foo')");
} else {
std::string expected("1fdf956bdad98101171512d18eec3bcf");
@@ -10787,7 +10787,7 @@ TEST_P(ExprTest, MaskTest) {
}
TEST_P(ExprTest, MaskHashTest) {
- if (FIPS_mode()) {
+ if (IsFIPSMode()) {
TestStringValue("mask_hash('TestString-123')",
"f3a58111be6ecec11449ac44654e72376b7759883ea11723b6e51354d50436de"
"645bd061cb5c2b07b68e15b7a7c342cac41f69b9c4efe19e810bbd7abf639a1c");
diff --git a/be/src/exprs/mask-functions-ir.cc
b/be/src/exprs/mask-functions-ir.cc
index c96be187d..b6725ea5c 100644
--- a/be/src/exprs/mask-functions-ir.cc
+++ b/be/src/exprs/mask-functions-ir.cc
@@ -20,13 +20,13 @@
#include <boost/locale/generator.hpp>
#include <boost/locale/utf8_codecvt.hpp>
#include <gutil/strings/substitute.h>
-#include <openssl/crypto.h>
#include <openssl/err.h>
#include <openssl/sha.h>
#include "exprs/anyval-util.h"
#include "exprs/math-functions.h"
#include "exprs/string-functions.h"
+#include "util/openssl-util.h"
#include "util/ubsan.h"
#include "common/names.h"
@@ -943,7 +943,7 @@ TimestampVal MaskFunctions::Mask(FunctionContext* ctx,
const TimestampVal& val,
StringVal MaskFunctions::MaskHash(FunctionContext* ctx, const StringVal& val) {
// Hive hash the value by sha256 and encoding it into a lower case hex
string in
// non FIPS mode. In FIPS enabled mode, it's required to use sha512 for mask
hash.
- if (FIPS_mode()) {
+ if (IsFIPSMode()) {
StringVal sha512_hash(ctx, SHA512_DIGEST_LENGTH);
if (UNLIKELY(sha512_hash.is_null)) return StringVal::null();
discard_result(SHA512(val.ptr, val.len, sha512_hash.ptr));
diff --git a/be/src/exprs/utility-functions-ir.cc
b/be/src/exprs/utility-functions-ir.cc
index ce2925350..2d6ecfaec 100644
--- a/be/src/exprs/utility-functions-ir.cc
+++ b/be/src/exprs/utility-functions-ir.cc
@@ -22,13 +22,12 @@
#include "exprs/math-functions.h"
#include "exprs/string-functions.h"
-#include <openssl/crypto.h>
-#include <openssl/evp.h>
#include <openssl/md5.h>
#include <openssl/sha.h>
#include "runtime/runtime-state.h"
#include "udf/udf-internal.h"
#include "util/debug-util.h"
+#include "util/openssl-util.h"
#include "util/time.h"
#include "common/names.h"
@@ -247,7 +246,7 @@ StringVal UtilityFunctions::Sha1(FunctionContext* ctx,
const StringVal& input_st
}
// SHA-1 is not supported for FIPS.
- if (FIPS_mode()) {
+ if (IsFIPSMode()) {
ctx->SetError("sha1 is not supported in FIPS mode.");
return StringVal::null();
} else {
@@ -266,7 +265,7 @@ StringVal UtilityFunctions::Sha2(FunctionContext* ctx,
const StringVal& input_st
}
// SHA-224 and SHA-256 are deprecated for FIPS mode.
- if (FIPS_mode() && (bit_len.val == 224 || bit_len.val == 256)) {
+ if (IsFIPSMode() && (bit_len.val == 224 || bit_len.val == 256)) {
ctx->SetError("Only bit lengths 384 and 512 are supported in FIPS mode.");
return StringVal::null();
}
@@ -308,7 +307,7 @@ StringVal UtilityFunctions::Md5(FunctionContext* ctx, const
StringVal& input_str
if (input_str.is_null) {
return StringVal::null();
}
- if (UNLIKELY(FIPS_mode())) {
+ if (UNLIKELY(IsFIPSMode())) {
ctx->SetError("MD5 is not supported in FIPS mode.");
return StringVal::null();
} else {
diff --git a/be/src/kudu/util/openssl_util.cc b/be/src/kudu/util/openssl_util.cc
index b210f3d66..33e985b62 100644
--- a/be/src/kudu/util/openssl_util.cc
+++ b/be/src/kudu/util/openssl_util.cc
@@ -95,7 +95,11 @@ void ThreadIdCB(CRYPTO_THREADID* tid) {
#endif
void CheckFIPSMode() {
- auto fips_mode = FIPS_mode();
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
+ int fips_mode = FIPS_mode();
+#else
+ int fips_mode = EVP_default_properties_is_fips_enabled(NULL);
+#endif
// If the environment variable KUDU_REQUIRE_FIPS_MODE is set to "1", we
// check if FIPS approved mode is enabled. If not, we crash the process.
// As this is used in clients as well, we can't use gflags to set this.
diff --git a/be/src/kudu/util/openssl_util.h b/be/src/kudu/util/openssl_util.h
index d1e4b16a3..b62eca59a 100644
--- a/be/src/kudu/util/openssl_util.h
+++ b/be/src/kudu/util/openssl_util.h
@@ -17,9 +17,13 @@
#pragma once
+#include <openssl/crypto.h>
#include <openssl/err.h>
#include <openssl/pem.h>
#include <openssl/ssl.h>
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+#include <openssl/types.h>
+#endif
#include <openssl/x509.h>
#include <functional>
diff --git a/be/src/util/openssl-util.h b/be/src/util/openssl-util.h
index c7a30e400..0864e5fb6 100644
--- a/be/src/util/openssl-util.h
+++ b/be/src/util/openssl-util.h
@@ -15,10 +15,10 @@
// specific language governing permissions and limitations
// under the License.
-#ifndef IMPALA_UTIL_OPENSSL_UTIL_H
-#define IMPALA_UTIL_OPENSSL_UTIL_H
+#pragma once
#include <openssl/aes.h>
+#include <openssl/crypto.h>
#include <openssl/evp.h>
#include <openssl/sha.h>
#include <openssl/ssl.h>
@@ -59,6 +59,16 @@ bool IsExternalTlsConfigured();
/// and again periodically to add new entropy.
void SeedOpenSSLRNG();
+/// Returns true if OpenSSL's FIPS mode is enabled. This calculation varies by
the OpenSSL
+/// version, so it is useful to have a shared function.
+inline bool IsFIPSMode() {
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
+ return FIPS_mode() > 0;
+#else
+ return EVP_default_properties_is_fips_enabled(nullptr) > 0;
+#endif
+};
+
enum AES_CIPHER_MODE {
AES_256_CFB,
AES_256_CTR,
@@ -200,5 +210,3 @@ class EncryptionKey {
};
}
-
-#endif
diff --git a/be/src/util/webserver-test.cc b/be/src/util/webserver-test.cc
index 29256cbea..2ed6d567f 100644
--- a/be/src/util/webserver-test.cc
+++ b/be/src/util/webserver-test.cc
@@ -24,7 +24,6 @@
#include <boost/lexical_cast.hpp>
#include <gutil/strings/substitute.h>
#include <map>
-#include <openssl/crypto.h>
#include <openssl/ssl.h>
#include <regex>
@@ -35,6 +34,7 @@
#include "util/default-path-handlers.h"
#include "util/kudu-status-util.h"
#include "util/metrics.h"
+#include "util/openssl-util.h"
#include "util/os-util.h"
#include "util/webserver.h"
@@ -662,7 +662,7 @@ TEST(Webserver, StartWithPasswordFileTest) {
MetricGroup metrics("webserver-test");
Webserver webserver("", FLAGS_webserver_port, &metrics);
- if (FIPS_mode()) {
+ if (IsFIPSMode()) {
ASSERT_FALSE(webserver.Start().ok());
} else {
ASSERT_OK(webserver.Start());
@@ -701,7 +701,7 @@ TEST(Webserver, StartWithPasswordFileTest) {
}
TEST(Webserver, StartWithMissingPasswordFileTest) {
- if (FIPS_mode()) return;
+ if (IsFIPSMode()) return;
stringstream password_file;
password_file << getenv("IMPALA_HOME") << "/be/src/testutil/doesntexist";
auto password =
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index d5a754911..a4436f6f8 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -55,6 +55,7 @@
#include "util/jwt-util.h"
#include "util/mem-info.h"
#include "util/metrics.h"
+#include "util/openssl-util.h"
#include "util/os-info.h"
#include "util/os-util.h"
#include "util/pretty-printer.h"
@@ -450,7 +451,7 @@ Status Webserver::Start() {
}
if (!FLAGS_webserver_password_file.empty()) {
- if (FIPS_mode()) {
+ if (IsFIPSMode()) {
return Status("HTTP digest authorization is not supported in FIPS
approved mode.");
} else {
// Squeasel doesn't log anything if it can't stat the password file (but
will if it
