This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch branch-1.17.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 214cb41bd9707f88740f06c591807a8153548810
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Fri Apr 28 16:52:12 2023 -0700
[security] handle BIO_new
This patch adds verification of the result returned by the OpenSSL's
BIO_new() function. It's a rare case when BIO_new() returns nullptr,
but if it does so, let's handle the results as documented.
Change-Id: I7a18034fde219c181f13d2d4c1ee9acb7e8e1a46
Reviewed-on: http://gerrit.cloudera.org:8080/19819
Tested-by: Kudu Jenkins
Reviewed-by: Yuqi Du <shenxingwuy...@gmail.com>
Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
(cherry picked from commit a7684eacf26bf84c415b8bb32228d026b639f7a8)
Conflicts:
src/kudu/util/openssl_util_bio.h
Reviewed-on: http://gerrit.cloudera.org:8080/20055
Reviewed-by: Yingchun Lai <laiyingc...@apache.org>
---
src/kudu/security/cert.cc | 3 +++
src/kudu/security/cert.h | 1 -
src/kudu/security/crypto.cc | 6 ++----
src/kudu/security/tls_handshake.cc | 10 ++--------
src/kudu/util/openssl_util.h | 5 +++++
src/kudu/util/openssl_util_bio.h | 3 +++
6 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/kudu/security/cert.cc b/src/kudu/security/cert.cc
index ff5cf0e71..b84e2bada 100644
--- a/src/kudu/security/cert.cc
+++ b/src/kudu/security/cert.cc
@@ -59,6 +59,7 @@ string X509NameToString(X509_NAME* name) {
SCOPED_OPENSSL_NO_PENDING_ERRORS;
CHECK(name);
auto bio = ssl_make_unique(BIO_new(BIO_s_mem()));
+ OPENSSL_CHECK(bio, "could not create memory BIO");
OPENSSL_CHECK_OK(X509_NAME_print_ex(bio.get(), name, 0, XN_FLAG_ONELINE));
BUF_MEM* membuf;
@@ -213,7 +214,9 @@ Status Cert::GetServerEndPointChannelBindings(string*
channel_bindings) const {
// Create a digest BIO. All data written to the BIO will be sent through the
// digest (hash) function. The digest BIO requires a null BIO to
writethrough to.
auto null_bio = ssl_make_unique(BIO_new(BIO_s_null()));
+ OPENSSL_RET_IF_NULL(null_bio, "could not create null BIO");
auto md_bio = ssl_make_unique(BIO_new(BIO_f_md()));
+ OPENSSL_RET_IF_NULL(md_bio, "could not create message digest BIO");
OPENSSL_RET_NOT_OK(BIO_set_md(md_bio.get(), md), "failed to set digest for
BIO");
BIO_push(md_bio.get(), null_bio.get());
diff --git a/src/kudu/security/cert.h b/src/kudu/security/cert.h
index 55bd44fcd..3a00ef4be 100644
--- a/src/kudu/security/cert.h
+++ b/src/kudu/security/cert.h
@@ -20,7 +20,6 @@
#include <openssl/ssl.h>
#include <openssl/x509.h>
-#include <memory>
#include <optional>
#include <string>
#include <vector>
diff --git a/src/kudu/security/crypto.cc b/src/kudu/security/crypto.cc
index 1e0f07a44..149eda35c 100644
--- a/src/kudu/security/crypto.cc
+++ b/src/kudu/security/crypto.cc
@@ -206,14 +206,12 @@ Status PrivateKey::GetPublicKey(PublicKey* public_key)
const {
return Status::RuntimeError(GetOpenSSLErrors());
}
auto tmp = ssl_make_unique(BIO_new(BIO_s_mem()));
- CHECK(tmp);
+ OPENSSL_RET_IF_NULL(tmp, "could not create memory BIO");
// Export public key in DER format into the temporary buffer.
OPENSSL_RET_NOT_OK(i2d_RSA_PUBKEY_bio(tmp.get(), rsa.get()),
"error extracting public RSA key");
// Read the public key into the result placeholder.
- RETURN_NOT_OK(public_key->FromBIO(tmp.get(), DataFormat::DER));
-
- return Status::OK();
+ return public_key->FromBIO(tmp.get(), DataFormat::DER);
}
// Modeled after code in $OPENSSL_ROOT/apps/dgst.c
diff --git a/src/kudu/security/tls_handshake.cc
b/src/kudu/security/tls_handshake.cc
index f3d2e4503..b28df06ad 100644
--- a/src/kudu/security/tls_handshake.cc
+++ b/src/kudu/security/tls_handshake.cc
@@ -101,15 +101,9 @@ Status TlsHandshake::Init(c_unique_ptr<SSL> s) {
}
auto rbio = ssl_make_unique(BIO_new(BIO_s_mem()));
- if (!rbio) {
- return Status::RuntimeError(
- "failed to create memory-based read BIO", GetOpenSSLErrors());
- }
+ OPENSSL_RET_IF_NULL(rbio, "failed to create memory read BIO");
auto wbio = ssl_make_unique(BIO_new(BIO_s_mem()));
- if (!wbio) {
- return Status::RuntimeError(
- "failed to create memory-based write BIO", GetOpenSSLErrors());
- }
+ OPENSSL_RET_IF_NULL(wbio, "failed to create memory write BIO");
ssl_ = std::move(s);
auto* ssl = ssl_.get();
SSL_set_bio(ssl, rbio.release(), wbio.release());
diff --git a/src/kudu/util/openssl_util.h b/src/kudu/util/openssl_util.h
index d1e4b16a3..2a077933b 100644
--- a/src/kudu/util/openssl_util.h
+++ b/src/kudu/util/openssl_util.h
@@ -56,6 +56,11 @@ typedef struct x509_st X509;
return Status::RuntimeError((msg), security::GetOpenSSLErrors()); \
}
+#define OPENSSL_CHECK(call, msg) \
+ if ((call) == nullptr) { \
+ LOG(FATAL) << #msg ": " << security::GetOpenSSLErrors(); \
+ }
+
#define OPENSSL_RET_IF_NULL(call, msg) \
if ((call) == nullptr) { \
return Status::RuntimeError((msg), security::GetOpenSSLErrors()); \
diff --git a/src/kudu/util/openssl_util_bio.h b/src/kudu/util/openssl_util_bio.h
index 4eff7682e..31be38896 100644
--- a/src/kudu/util/openssl_util_bio.h
+++ b/src/kudu/util/openssl_util_bio.h
@@ -96,6 +96,7 @@ Status FromString(const std::string& data, DataFormat format,
data.size()));
RETURN_NOT_OK_PREPEND((FromBIO<Type, Traits>(bio.get(), format, ret)),
"unable to load data from memory");
+ OPENSSL_RET_IF_NULL(bio, "could not create memory BIO");
return Status::OK();
}
@@ -105,6 +106,7 @@ Status ToString(std::string* data, DataFormat format, Type*
obj) {
auto bio = ssl_make_unique(BIO_new(BIO_s_mem()));
RETURN_NOT_OK_PREPEND((ToBIO<Type, Traits>(bio.get(), format, obj)),
"error serializing data");
+ OPENSSL_RET_IF_NULL(bio, "could not create memory BIO");
BUF_MEM* membuf;
OPENSSL_CHECK_OK(BIO_get_mem_ptr(bio.get(), &membuf));
data->assign(membuf->data, membuf->length);
@@ -115,6 +117,7 @@ template<typename Type, typename Traits =
SslTypeTraits<Type>>
Status FromFile(const std::string& fpath, DataFormat format,
c_unique_ptr<Type>* ret, const PasswordCallback& cb =
PasswordCallback()) {
auto bio = ssl_make_unique(BIO_new(BIO_s_file()));
+ OPENSSL_RET_IF_NULL(bio, "could not create file BIO");
OPENSSL_RET_NOT_OK(BIO_read_filename(bio.get(), fpath.c_str()),
strings::Substitute("could not read data from file '$0'", fpath));
RETURN_NOT_OK_PREPEND((FromBIO<Type, Traits>(bio.get(), format, ret, cb)),
