Repository: kudu Updated Branches: refs/heads/master bbf753061 -> ff38c6b5a
Use Traits to avoid duplicate code in SSL wrappers cert_management.cc contained some duplicate code for each of the wrappers. In addition, having to always remember the correct 'free' function for each SSL type was a bit difficult. This uses a "traits" pattern to try to clean up the code. It doesn't save a ton of lines of code, but it replaces a lot of imperative code with more "declarative" lists of types and functions. Change-Id: If1c44857358fc82f2694dff021b9f2ac3641313a Reviewed-on: http://gerrit.cloudera.org:8080/5767 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> Reviewed-by: Dan Burkert <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a1a778c0 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a1a778c0 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a1a778c0 Branch: refs/heads/master Commit: a1a778c031c73c74eaa6ff5111ea6ef15385fc9a Parents: bbf7530 Author: Todd Lipcon <[email protected]> Authored: Fri Jan 20 14:58:47 2017 -0800 Committer: Alexey Serbin <[email protected]> Committed: Tue Jan 24 04:33:39 2017 +0000 ---------------------------------------------------------------------- src/kudu/security/ca/cert_management.cc | 259 +++++++++++++-------------- 1 file changed, 123 insertions(+), 136 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/a1a778c0/src/kudu/security/ca/cert_management.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/ca/cert_management.cc b/src/kudu/security/ca/cert_management.cc index c9c3b57..e34a7b4 100644 --- a/src/kudu/security/ca/cert_management.cc +++ b/src/kudu/security/ca/cert_management.cc @@ -65,6 +65,104 @@ namespace kudu { namespace security { namespace ca { + +namespace { + +// Writing the private key from an EVP_PKEY has a different +// signature than the rest of the write functions, so we +// have to provide this wrapper. +int PemWritePrivateKey(BIO* bio, EVP_PKEY* key); + +// For each SSL type, the Traits class provides the important OpenSSL +// API functions. +template<class SSL_TYPE> +struct SslTypeTraits {}; + +template<> struct SslTypeTraits<X509> { + static constexpr auto free = &X509_free; + static constexpr auto read_pem = &PEM_read_bio_X509; + static constexpr auto read_der = &d2i_X509_bio; + static constexpr auto write_pem = &PEM_write_bio_X509; + static constexpr auto write_der = &i2d_X509_bio; +}; +template<> struct SslTypeTraits<X509_REQ> { + static constexpr auto free = &X509_REQ_free; + static constexpr auto read_pem = &PEM_read_bio_X509_REQ; + static constexpr auto read_der = &d2i_X509_REQ_bio; + static constexpr auto write_pem = &PEM_write_bio_X509_REQ; + static constexpr auto write_der = &i2d_X509_REQ_bio; +}; +template<> struct SslTypeTraits<EVP_PKEY> { + static constexpr auto free = &EVP_PKEY_free; + static constexpr auto read_pem = &PEM_read_bio_PrivateKey; + static constexpr auto read_der = &d2i_PrivateKey_bio; + static constexpr auto write_pem = &PemWritePrivateKey; + static constexpr auto write_der = &i2d_PrivateKey_bio; +}; +template<> struct SslTypeTraits<ASN1_INTEGER> { + static constexpr auto free = &ASN1_INTEGER_free; +}; +template<> struct SslTypeTraits<BIO> { + static constexpr auto free = &BIO_free; +}; +template<> struct SslTypeTraits<BIGNUM> { + static constexpr auto free = &BN_free; +}; +template<> struct SslTypeTraits<RSA> { + static constexpr auto free = &RSA_free; +}; +template<> struct SslTypeTraits<X509_EXTENSION> { + static constexpr auto free = &X509_EXTENSION_free; +}; + +template<class SSL_TYPE> +static c_unique_ptr<SSL_TYPE> make_ssl_unique(SSL_TYPE* d) { + return {d, SslTypeTraits<SSL_TYPE>::free}; +} + +int PemWritePrivateKey(BIO* bio, EVP_PKEY* key) { + auto rsa = make_ssl_unique(EVP_PKEY_get1_RSA(key)); + return PEM_write_bio_RSAPrivateKey( + bio, rsa.get(), nullptr, nullptr, 0, nullptr, nullptr); +} + +template<class TYPE> +Status ToBIO(BIO* bio, DataFormat format, TYPE* obj) { + using Traits = SslTypeTraits<TYPE>; + CHECK(bio); + CHECK(obj); + switch (format) { + case DataFormat::DER: + CERT_RET_NOT_OK(Traits::write_der(bio, obj), "error exporting DER format"); + break; + case DataFormat::PEM: + CERT_RET_NOT_OK(Traits::write_pem(bio, obj), "error exporting PEM format"); + break; + } + CERT_RET_NOT_OK(BIO_flush(bio), "error flushing BIO"); + return Status::OK(); +} + +template<class TYPE> +Status FromBIO(BIO* bio, DataFormat format, c_unique_ptr<TYPE>* ret) { + using Traits = SslTypeTraits<TYPE>; + CHECK(bio); + switch (format) { + case DataFormat::DER: + *ret = make_ssl_unique(Traits::read_der(bio, nullptr)); + break; + case DataFormat::PEM: + *ret = make_ssl_unique(Traits::read_pem(bio, nullptr, nullptr, nullptr)); + break; + } + if (PREDICT_FALSE(!*ret)) { + return Status::RuntimeError(GetOpenSSLErrors()); + } + return Status::OK(); +} +} // anonymous namespace + + const string& DataFormatToString(DataFormat fmt) { static const string kStrFormatUnknown = "UNKNOWN"; static const string kStrFormatDer = "DER"; @@ -80,7 +178,7 @@ const string& DataFormatToString(DataFormat fmt) { } Status BasicWrapper::FromFile(const string& fpath, DataFormat format) { - c_unique_ptr<BIO> bio(BIO_new(BIO_s_file()), BIO_free); + auto bio = make_ssl_unique(BIO_new(BIO_s_file())); CERT_RET_NOT_OK(BIO_read_filename(bio.get(), fpath.c_str()), Substitute("$0: could not read from file", fpath)); RETURN_NOT_OK_PREPEND(FromBIO(bio.get(), format), @@ -91,13 +189,13 @@ Status BasicWrapper::FromFile(const string& fpath, DataFormat format) { Status BasicWrapper::FromString(const string& data, DataFormat format) { const void* mdata = reinterpret_cast<const void*>(data.data()); - c_unique_ptr<BIO> bio(BIO_new_mem_buf( + auto bio = make_ssl_unique(BIO_new_mem_buf( #if OPENSSL_VERSION_NUMBER < 0x10002000L const_cast<void*>(mdata), #else mdata, #endif - data.size()), BIO_free); + data.size())); RETURN_NOT_OK_PREPEND(FromBIO(bio.get(), format), "unable to load data from memory"); return Status::OK(); @@ -105,7 +203,7 @@ Status BasicWrapper::FromString(const string& data, DataFormat format) { Status BasicWrapper::ToString(std::string* data, DataFormat format) const { CHECK(data); - c_unique_ptr<BIO> bio(BIO_new(BIO_s_mem()), BIO_free); + auto bio = make_ssl_unique(BIO_new(BIO_s_mem())); RETURN_NOT_OK_PREPEND(ToBIO(bio.get(), format), "error serializing data"); BUF_MEM* membuf; CERT_CHECK_OK(BIO_get_mem_ptr(bio.get(), &membuf)); @@ -114,165 +212,56 @@ Status BasicWrapper::ToString(std::string* data, DataFormat format) const { } void Key::AdoptRawData(RawDataType* data) { - c_unique_ptr<RawDataType> d(data, EVP_PKEY_free); - data_.swap(d); + data_ = make_ssl_unique(data); } Status Key::FromBIO(BIO* bio, DataFormat format) { - CHECK(bio); - c_unique_ptr<EVP_PKEY> key(nullptr, EVP_PKEY_free); - switch (format) { - case DataFormat::DER: - key.reset(d2i_PrivateKey_bio(bio, nullptr)); - break; - case DataFormat::PEM: - key.reset(PEM_read_bio_PrivateKey(bio, nullptr, nullptr, nullptr)); - break; - default: - return Status::InvalidArgument(Substitute("$0: unrecognized format", - static_cast<uint16_t>(format))); - } - if (PREDICT_FALSE(!key)) { - return Status::RuntimeError( - Substitute("error reading private key: $0", GetOpenSSLErrors())); - } - key.swap(data_); + RETURN_NOT_OK_PREPEND(ca::FromBIO(bio, format, &data_), + "unable to read private key"); return Status::OK(); } Status Key::ToBIO(BIO* bio, DataFormat format) const { - CHECK(bio); - CHECK(data_); - switch (format) { - case DataFormat::DER: - CERT_RET_NOT_OK(i2d_PrivateKey_bio(bio, data_.get()), - "error exporting private key in DER format"); - break; - case DataFormat::PEM: - { - c_unique_ptr<RSA> rsa(EVP_PKEY_get1_RSA(data_.get()), RSA_free); - CERT_RET_NOT_OK(PEM_write_bio_RSAPrivateKey(bio, rsa.get(), - nullptr, nullptr, 0, - nullptr, nullptr), - "error exporting private key in PEM format"); - } - break; - default: - return Status::InvalidArgument(Substitute("$0: unrecognized format", - static_cast<uint16_t>(format))); - } - CERT_RET_NOT_OK(BIO_flush(bio), "error flushing BIO"); + RETURN_NOT_OK_PREPEND(ca::ToBIO(bio, format, data_.get()), "could not export cert"); return Status::OK(); } void Cert::AdoptRawData(RawDataType* data) { - c_unique_ptr<RawDataType> d(data, X509_free); - data_.swap(d); + data_ = make_ssl_unique(data); } Status Cert::FromBIO(BIO* bio, DataFormat format) { - CHECK(bio); - c_unique_ptr<X509> cert(nullptr, X509_free); - switch (format) { - case DataFormat::DER: - cert.reset(d2i_X509_bio(bio, nullptr)); - break; - case DataFormat::PEM: - cert.reset(PEM_read_bio_X509(bio, nullptr, nullptr, nullptr)); - break; - default: { - string err_msg = Substitute("$0: unrecognized data format", - static_cast<uint16_t>(format)); - LOG(DFATAL) << err_msg; - return Status::InvalidArgument(err_msg); - } - } - if (PREDICT_FALSE(!cert)) { - return Status::RuntimeError( - Substitute("error loading X509 certificate: $0", GetOpenSSLErrors())); - } - cert.swap(data_); + RETURN_NOT_OK_PREPEND(ca::FromBIO(bio, format, &data_), "could not read cert"); return Status::OK(); } Status Cert::ToBIO(BIO* bio, DataFormat format) const { - CHECK(data_); - CHECK(bio); - switch (format) { - case DataFormat::DER: - CERT_RET_NOT_OK(i2d_X509_bio(bio, data_.get()), - "error exporting X509 certificate in DER format"); - break; - case DataFormat::PEM: - CERT_RET_NOT_OK(PEM_write_bio_X509(bio, data_.get()), - "error exporting X509 certificate in PEM format"); - break; - default: - return Status::InvalidArgument(Substitute("$0: unrecognized format", - static_cast<uint16_t>(format))); - } - CERT_RET_NOT_OK(BIO_flush(bio), "error flushing BIO"); + RETURN_NOT_OK_PREPEND(ca::ToBIO(bio, format, data_.get()), "could not export cert"); return Status::OK(); } void CertSignRequest::AdoptRawData(RawDataType* data) { - c_unique_ptr<RawDataType> d(data, X509_REQ_free); - data_.swap(d); + data_ = make_ssl_unique(data); } Status CertSignRequest::FromBIO(BIO* bio, DataFormat format) { - CHECK(bio); - c_unique_ptr<X509_REQ> req(nullptr, X509_REQ_free); - switch (format) { - case DataFormat::DER: - req.reset(d2i_X509_REQ_bio(bio, nullptr)); - break; - case DataFormat::PEM: - req.reset(PEM_read_bio_X509_REQ(bio, nullptr, nullptr, nullptr)); - break; - default: { - string err_msg = Substitute("$0: unrecognized data format", - static_cast<uint16_t>(format)); - LOG(DFATAL) << err_msg; - return Status::InvalidArgument(err_msg); - } - } - if (PREDICT_FALSE(!req)) { - return Status::RuntimeError( - Substitute("error loading X509 CSR: $0", GetOpenSSLErrors())); - } - req.swap(data_); + RETURN_NOT_OK_PREPEND(ca::FromBIO(bio, format, &data_), "could not read X509 CSR"); return Status::OK(); } Status CertSignRequest::ToBIO(BIO* bio, DataFormat format) const { - CHECK(bio); - CHECK(data_); - switch (format) { - case DataFormat::DER: - CERT_RET_NOT_OK(i2d_X509_REQ_bio(bio, data_.get()), - "error exporting X509 CSR in DER format"); - break; - case DataFormat::PEM: - CERT_RET_NOT_OK(PEM_write_bio_X509_REQ(bio, data_.get()), - "error exporting X509 CSR in PEM format"); - break; - default: - return Status::InvalidArgument(Substitute("$0: unrecognized format", - static_cast<uint16_t>(format))); - } - CERT_RET_NOT_OK(BIO_flush(bio), "error flushing BIO"); + RETURN_NOT_OK_PREPEND(ca::ToBIO(bio, format, data_.get()), "could not export X509 CSR"); return Status::OK(); } Status GeneratePrivateKey(int num_bits, Key* ret) { CHECK(ret); InitializeOpenSSL(); - c_unique_ptr<EVP_PKEY> key(EVP_PKEY_new(), EVP_PKEY_free); + auto key = make_ssl_unique(EVP_PKEY_new()); { - c_unique_ptr<BIGNUM> bn(BN_new(), BN_free); + auto bn = make_ssl_unique(BN_new()); CERT_CHECK_OK(BN_set_word(bn.get(), RSA_F4)); - c_unique_ptr<RSA> rsa(RSA_new(), RSA_free); + auto rsa = make_ssl_unique(RSA_new()); CERT_RET_NOT_OK(RSA_generate_key_ex(rsa.get(), num_bits, bn.get(), nullptr), "error generating RSA key"); CERT_RET_NOT_OK(EVP_PKEY_set1_RSA(key.get(), rsa.get()), @@ -295,7 +284,7 @@ Status CertRequestGeneratorBase::GenerateRequest(const Key& key, CertSignRequest* ret) const { CHECK(ret); CHECK(Initialized()); - c_unique_ptr<X509_REQ> req(X509_REQ_new(), X509_REQ_free); + auto req = make_ssl_unique(X509_REQ_new()); CERT_RET_NOT_OK(X509_REQ_set_pubkey(req.get(), key.GetRawData()), "error setting X509 public key"); X509_NAME* name = X509_REQ_get_subject_name(req.get()); @@ -332,9 +321,8 @@ Status CertRequestGeneratorBase::GenerateRequest(const Key& key, Status CertRequestGeneratorBase::PushExtension(stack_st_X509_EXTENSION* st, int32_t nid, const char* value) { - c_unique_ptr<X509_EXTENSION> ex( - X509V3_EXT_conf_nid(nullptr, nullptr, nid, const_cast<char*>(value)), - X509_EXTENSION_free); + auto ex = make_ssl_unique( + X509V3_EXT_conf_nid(nullptr, nullptr, nid, const_cast<char*>(value))); if (!ex) { return Status::RuntimeError("error configuring extension"); } @@ -538,7 +526,7 @@ const Key& CertSigner::ca_private_key() const { Status CertSigner::Sign(const CertSignRequest& req, Cert* ret) const { CHECK(ret); CHECK(Initialized()); - c_unique_ptr<X509> x509(X509_new(), X509_free); + auto x509 = make_ssl_unique(X509_new()); RETURN_NOT_OK(FillCertTemplateFromRequest(req.GetRawData(), x509.get())); RETURN_NOT_OK(DoSign(EVP_sha256(), config_.exp_interval_sec, x509.get())); ret->AdoptRawData(x509.release()); @@ -562,8 +550,7 @@ Status CertSigner::CopyExtensions(X509_REQ* req, X509* x) { if (idx != -1) { // If extension exits, delete all extensions of same type. do { - c_unique_ptr<X509_EXTENSION> tmpext(X509_get_ext(x, idx), - X509_EXTENSION_free); + auto tmpext = make_ssl_unique(X509_get_ext(x, idx)); X509_delete_ext(x, idx); idx = X509_get_ext_by_OBJ(x, obj, -1); } while (idx != -1); @@ -582,7 +569,7 @@ Status CertSigner::FillCertTemplateFromRequest(X509_REQ* req, X509* tmpl) { !req->req_info->pubkey->public_key->data) { return Status::RuntimeError("corrupted CSR: no public key"); } - c_unique_ptr<EVP_PKEY> pub_key(X509_REQ_get_pubkey(req), EVP_PKEY_free); + auto pub_key = make_ssl_unique(X509_REQ_get_pubkey(req)); if (!pub_key) { return Status::RuntimeError("error unpacking public key from CSR"); } @@ -607,10 +594,10 @@ Status CertSigner::DigestSign(const EVP_MD* md, EVP_PKEY* pkey, X509* x) { } Status CertSigner::GenerateSerial(c_unique_ptr<ASN1_INTEGER>* ret) { - c_unique_ptr<BIGNUM> btmp(BN_new(), BN_free); + auto btmp = make_ssl_unique(BN_new()); CERT_RET_NOT_OK(BN_pseudo_rand(btmp.get(), 64, 0, 0), "error generating random number"); - c_unique_ptr<ASN1_INTEGER> serial(ASN1_INTEGER_new(), ASN1_INTEGER_free); + auto serial = make_ssl_unique(ASN1_INTEGER_new()); CERT_RET_IF_NULL(BN_to_ASN1_INTEGER(btmp.get(), serial.get()), "error converting number into ASN1 representation"); if (ret) {
