This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new a7684eacf [security] handle BIO_new
a7684eacf is described below
commit a7684eacf26bf84c415b8bb32228d026b639f7a8
Author: Alexey Serbin
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
Reviewed-by: Abhishek Chennaka
---
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
#include
-#include
#include
#include
#include
diff --git a/src/kudu/security/crypto.cc b/src/kudu/security/crypto.cc
index be2ebf0f0..ea9fc22f3 100644
--- a/src/kudu/security/crypto.cc
+++ b/src/kudu/security/crypto.cc
@@ -268,14 +268,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 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 97daf7121..2115c13a5 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/u