From 575a9eb849c9302411224dda586730899acf29ac Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Wed, 15 Oct 2014 18:22:56 +0200 Subject: [PATCH] Implement error handling for the "pki sign-csr" command refs #7247 --- lib/base/tlsutility.cpp | 60 +++++++++++++++++------------------ lib/base/tlsutility.hpp | 1 + lib/cli/pkisigncsrcommand.cpp | 51 +++++++++++++++++++++++++++-- 3 files changed, 79 insertions(+), 33 deletions(-) diff --git a/lib/base/tlsutility.cpp b/lib/base/tlsutility.cpp index ab7f6c901..4c1000495 100644 --- a/lib/base/tlsutility.cpp +++ b/lib/base/tlsutility.cpp @@ -48,7 +48,7 @@ static unsigned long OpenSSLIDCallback(void) /** * Initializes the OpenSSL library. */ -static void InitializeOpenSSL(void) +void InitializeOpenSSL(void) { if (l_SSLInitialized) return; @@ -85,37 +85,37 @@ shared_ptr MakeSSLContext(const String& pubkey, const String& privkey, SSL_CTX_set_mode(sslContext.get(), SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); if (!SSL_CTX_use_certificate_chain_file(sslContext.get(), pubkey.CStr())) { - msgbuf << "Error with public key file '" << pubkey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error with public key file '" << pubkey << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_CTX_use_certificate_chain_file") - << errinfo_openssl_error(ERR_get_error()) + << errinfo_openssl_error(ERR_peek_error()) << boost::errinfo_file_name(pubkey)); } if (!SSL_CTX_use_PrivateKey_file(sslContext.get(), privkey.CStr(), SSL_FILETYPE_PEM)) { - msgbuf << "Error with private key file '" << privkey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error with private key file '" << privkey << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_CTX_use_PrivateKey_file") - << errinfo_openssl_error(ERR_get_error()) + << errinfo_openssl_error(ERR_peek_error()) << boost::errinfo_file_name(privkey)); } if (!SSL_CTX_check_private_key(sslContext.get())) { - msgbuf << "Error checking private key '" << privkey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error checking private key '" << privkey << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_CTX_check_private_key") - << errinfo_openssl_error(ERR_get_error())); + << errinfo_openssl_error(ERR_peek_error())); } if (!SSL_CTX_load_verify_locations(sslContext.get(), cakey.CStr(), NULL)) { - msgbuf << "Error loading and verifying locations in ca key file '" << cakey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error loading and verifying locations in ca key file '" << cakey << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_CTX_load_verify_locations") - << errinfo_openssl_error(ERR_get_error()) + << errinfo_openssl_error(ERR_peek_error()) << boost::errinfo_file_name(cakey)); } @@ -123,11 +123,11 @@ shared_ptr MakeSSLContext(const String& pubkey, const String& privkey, cert_names = SSL_load_client_CA_file(cakey.CStr()); if (cert_names == NULL) { - msgbuf << "Error loading client ca key file '" << cakey << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error loading client ca key file '" << cakey << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SSL_load_client_CA_file") - << errinfo_openssl_error(ERR_get_error()) + << errinfo_openssl_error(ERR_peek_error()) << boost::errinfo_file_name(cakey)); } @@ -152,19 +152,19 @@ void AddCRLToSSLContext(const shared_ptr& context, const String& crlPat lookup = X509_STORE_add_lookup(x509_store, X509_LOOKUP_file()); if (!lookup) { - msgbuf << "Error adding X509 store lookup: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error adding X509 store lookup: " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("X509_STORE_add_lookup") - << errinfo_openssl_error(ERR_get_error())); + << errinfo_openssl_error(ERR_peek_error())); } if (X509_LOOKUP_load_file(lookup, crlPath.CStr(), X509_FILETYPE_PEM) != 0) { - msgbuf << "Error loading crl file '" << crlPath << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error loading crl file '" << crlPath << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("X509_LOOKUP_load_file") - << errinfo_openssl_error(ERR_get_error()) + << errinfo_openssl_error(ERR_peek_error()) << boost::errinfo_file_name(crlPath)); } @@ -190,11 +190,11 @@ String GetCertificateCN(const shared_ptr& certificate) NID_commonName, buffer, sizeof(buffer)); if (rc == -1) { - msgbuf << "Error with x509 NAME getting text by NID: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error with x509 NAME getting text by NID: " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("X509_NAME_get_text_by_NID") - << errinfo_openssl_error(ERR_get_error())); + << errinfo_openssl_error(ERR_peek_error())); } return buffer; @@ -214,29 +214,29 @@ shared_ptr GetX509Certificate(const String& pemfile) BIO *fpcert = BIO_new(BIO_s_file()); if (fpcert == NULL) { - msgbuf << "Error creating new BIO: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error creating new BIO: " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("BIO_new") - << errinfo_openssl_error(ERR_get_error())); + << errinfo_openssl_error(ERR_peek_error())); } if (BIO_read_filename(fpcert, pemfile.CStr()) < 0) { - msgbuf << "Error reading pem file '" << pemfile << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error reading pem file '" << pemfile << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("BIO_read_filename") - << errinfo_openssl_error(ERR_get_error()) + << errinfo_openssl_error(ERR_peek_error()) << boost::errinfo_file_name(pemfile)); } cert = PEM_read_bio_X509_AUX(fpcert, NULL, NULL, NULL); if (cert == NULL) { - msgbuf << "Error on bio X509 AUX reading pem file '" << pemfile << "': " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error on bio X509 AUX reading pem file '" << pemfile << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("PEM_read_bio_X509_AUX") - << errinfo_openssl_error(ERR_get_error()) + << errinfo_openssl_error(ERR_peek_error()) << boost::errinfo_file_name(pemfile)); } @@ -296,7 +296,7 @@ int MakeX509CSR(const String& cn, const String& keyfile, const String& csrfile, X509_REQ_sign(req, key, EVP_sha1()); - Log(LogInformation, "base", "Writing certificate signing request to '" + certfile + "'."); + Log(LogInformation, "base", "Writing certificate signing request to '" + csrfile + "'."); bio = BIO_new(BIO_s_file()); BIO_write_filename(bio, const_cast(csrfile.CStr())); @@ -348,27 +348,27 @@ String SHA256(const String& s) unsigned char digest[SHA256_DIGEST_LENGTH]; if (!SHA256_Init(&context)) { - msgbuf << "Error on SHA256 Init: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error on SHA256 Init: " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SHA256_Init") - << errinfo_openssl_error(ERR_get_error())); + << errinfo_openssl_error(ERR_peek_error())); } if (!SHA256_Update(&context, (unsigned char*)s.CStr(), s.GetLength())) { - msgbuf << "Error on SHA256 Update: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error on SHA256 Update: " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SHA256_Update") - << errinfo_openssl_error(ERR_get_error())); + << errinfo_openssl_error(ERR_peek_error())); } if (!SHA256_Final(digest, &context)) { - msgbuf << "Error on SHA256 Final: " << ERR_get_error() << ", \"" << ERR_error_string(ERR_get_error(), errbuf) << "\""; + msgbuf << "Error on SHA256 Final: " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; Log(LogCritical, "SSL", msgbuf.str()); BOOST_THROW_EXCEPTION(openssl_error() << boost::errinfo_api_function("SHA256_Final") - << errinfo_openssl_error(ERR_get_error())); + << errinfo_openssl_error(ERR_peek_error())); } int i; diff --git a/lib/base/tlsutility.hpp b/lib/base/tlsutility.hpp index cb1472d23..a2e8b6500 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -34,6 +34,7 @@ namespace icinga { +void I2_BASE_API InitializeOpenSSL(void); shared_ptr I2_BASE_API MakeSSLContext(const String& pubkey, const String& privkey, const String& cakey); void I2_BASE_API AddCRLToSSLContext(const shared_ptr& context, const String& crlPath); String I2_BASE_API GetCertificateCN(const shared_ptr& certificate); diff --git a/lib/cli/pkisigncsrcommand.cpp b/lib/cli/pkisigncsrcommand.cpp index 2d1586687..f18d79dd8 100644 --- a/lib/cli/pkisigncsrcommand.cpp +++ b/lib/cli/pkisigncsrcommand.cpp @@ -52,9 +52,20 @@ void PKISignCSRCommand::InitParameters(boost::program_options::options_descripti */ int PKISignCSRCommand::Run(const boost::program_options::variables_map& vm, const std::vector& ap) const { + std::stringstream msgbuf; + char errbuf[120]; + + InitializeOpenSSL(); + BIO *csrbio = BIO_new_fp(stdin, BIO_NOCLOSE); - X509_REQ *req; - PEM_read_bio_X509_REQ(csrbio, &req, NULL, NULL); + X509_REQ *req = PEM_read_bio_X509_REQ(csrbio, NULL, NULL, NULL); + + if (!req) { + msgbuf << "Could not parse X509 certificate request: " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); + return 1; + } + BIO_free(csrbio); String cadir = Application::GetLocalStateDir() + "/lib/icinga2/ca"; @@ -64,13 +75,41 @@ int PKISignCSRCommand::Run(const boost::program_options::variables_map& vm, cons RSA *rsa; BIO *cakeybio = BIO_new_file(const_cast(cakeyfile.CStr()), "r"); + + if (!cakeybio) { + msgbuf << "Could not open CA key file '" << cakeyfile << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); + return 1; + } + rsa = PEM_read_bio_RSAPrivateKey(cakeybio, NULL, NULL, NULL); + + if (!rsa) { + msgbuf << "Could not read RSA key from CA key file '" << cakeyfile << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); + return 1; + } + BIO_free(cakeybio); String cacertfile = cadir + "/ca.crt"; BIO *cacertbio = BIO_new_file(const_cast(cacertfile.CStr()), "r"); + + if (!cacertbio) { + msgbuf << "Could not open CA certificate file '" << cakeyfile << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); + return 1; + } + X509 *cacert = PEM_read_bio_X509(cacertbio, NULL, NULL, NULL); + + if (!cacert) { + msgbuf << "Could not read X509 certificate from CA certificate file '" << cakeyfile << "': " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); + return 1; + } + BIO_free(cacertbio); EVP_PKEY *privkey = EVP_PKEY_new(); @@ -83,7 +122,13 @@ int PKISignCSRCommand::Run(const boost::program_options::variables_map& vm, cons X509_free(cacert); BIO *certbio = BIO_new_fp(stdout, BIO_NOCLOSE); - PEM_write_bio_X509(certbio, cert); + + if (!PEM_write_bio_X509(certbio, cert)) { + msgbuf << "Could not write X509 certificate: " << ERR_peek_error() << ", \"" << ERR_error_string(ERR_peek_error(), errbuf) << "\""; + Log(LogCritical, "SSL", msgbuf.str()); + return 1; + } + BIO_free(certbio); return 0; -- 2.40.0