Skip to content

Commit

Permalink
tls openssl: added pointer guards and error messages
Browse files Browse the repository at this point in the history 
- avoid to dereference null-pointers in openssl_ctx_ and openssl_
  • Loading branch information
@franku
franku committed Sep 30, 2018
1 parent 0d06188 commit d1e0149
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 61 deletions.
39 changes: 25 additions & 14 deletions core/src/lib/tls_openssl.cc
View file
Expand Up @@ -78,12 +78,17 @@ TlsOpenSsl::~TlsOpenSsl()

bool TlsOpenSsl::init()
{
if (!d_->openssl_ctx_) {
OpensslPostErrors(M_FATAL, _("Error initializing TlsOpenSsl (no SSL_CTX)\n"));
return false;
}

if (d_->cipherlist_.empty()) {
d_->cipherlist_ = TLS_DEFAULT_CIPHERS;
}

if (SSL_CTX_set_cipher_list(d_->openssl_ctx_, d_->cipherlist_.c_str()) != 1) {
Dmsg0(100, _("Error setting cipher list, no valid ciphers available\n"));
OpensslPostErrors(M_FATAL, _("Error setting cipher list, no valid ciphers available\n"));
return false;
}

Expand All @@ -107,7 +112,7 @@ bool TlsOpenSsl::init()
}
} else if (d_->verify_peer_) {
/* At least one CA is required for peer verification */
Dmsg0(100, _("Either a certificate file or a directory must be"
OpensslPostErrors(M_WARNING, _("Either a certificate file or a directory must be"
" specified as a verification store\n"));
// return false; Ueb: do not return compatibility ?
}
Expand Down Expand Up @@ -190,26 +195,32 @@ bool TlsOpenSsl::init()

void TlsOpenSsl::SetTlsPskClientContext(const PskCredentials &credentials)
{
Dmsg1(50, "Preparing TLS_PSK CLIENT context for identity %s\n", credentials.get_identity().c_str());

if (d_->openssl_ctx_) {
if (!d_->openssl_ctx_) {
Dmsg0(50, "Could not set TLS_PSK CLIENT context (no SSL_CTX)\n");
} else {
Dmsg1(50, "Preparing TLS_PSK CLIENT context for identity %s\n", credentials.get_identity().c_str());
d_->ClientContextInsertCredentials(credentials);
SSL_CTX_set_psk_client_callback(d_->openssl_ctx_, TlsOpenSslPrivate::psk_client_cb);
}
}

void TlsOpenSsl::SetTlsPskServerContext(ConfigurationParser *config, GetTlsPskByFullyQualifiedResourceNameCb_t cb)
{
Dmsg0(50, "Preparing TLS_PSK SERVER callback\n");

SSL_CTX_set_ex_data(d_->openssl_ctx_,
TlsOpenSslPrivate::SslCtxExDataIndex::kGetTlsPskByFullyQualifiedResourceNameCb,
(void *)cb);
SSL_CTX_set_ex_data(d_->openssl_ctx_,
TlsOpenSslPrivate::SslCtxExDataIndex::kConfigurationParserPtr,
(void *)config);
if (!d_->openssl_ctx_) {
Dmsg0(50, "Could not prepare TLS_PSK SERVER callback (no SSL_CTX)\n");
} else if (!config) {
Dmsg0(50, "Could not prepare TLS_PSK SERVER callback (no config)\n");
} else if (!cb) {
Dmsg0(50, "Could not prepare TLS_PSK SERVER callback (no callback)\n");
} else {
Dmsg0(50, "Preparing TLS_PSK SERVER callback\n");
SSL_CTX_set_ex_data(d_->openssl_ctx_,
TlsOpenSslPrivate::SslCtxExDataIndex::kGetTlsPskByFullyQualifiedResourceNameCb,
(void *)cb);
SSL_CTX_set_ex_data(d_->openssl_ctx_,
TlsOpenSslPrivate::SslCtxExDataIndex::kConfigurationParserPtr,
(void *)config);

if (d_->openssl_ctx_) {
SSL_CTX_set_psk_server_callback(d_->openssl_ctx_, TlsOpenSslPrivate::psk_server_cb);
}
}
Expand Down
107 changes: 60 additions & 47 deletions core/src/lib/tls_openssl_private.cc
View file
Expand Up @@ -42,7 +42,7 @@ TlsOpenSslPrivate::TlsOpenSslPrivate()
Dmsg0(100, "Construct TlsOpenSslPrivate\n");
}

TlsOpenSslPrivate::~TlsOpenSslPrivate() // FreeTlsContext
TlsOpenSslPrivate::~TlsOpenSslPrivate()
{
Dmsg0(100, "Destruct TlsOpenSslPrivate\n");
if (openssl_ctx_) {
Expand All @@ -57,9 +57,9 @@ TlsOpenSslPrivate::~TlsOpenSslPrivate() // FreeTlsContext
/*
* report any errors that occured
*/
int TlsOpenSslPrivate::OpensslVerifyPeer(int ok, X509_STORE_CTX *store)
int TlsOpenSslPrivate::OpensslVerifyPeer(int preverify_ok, X509_STORE_CTX *store)
{ /* static */
if (!ok) {
if (!preverify_ok) {
X509 *cert = X509_STORE_CTX_get_current_cert(store);
int depth = X509_STORE_CTX_get_error_depth(store);
int err = X509_STORE_CTX_get_error(store);
Expand All @@ -75,7 +75,7 @@ int TlsOpenSslPrivate::OpensslVerifyPeer(int ok, X509_STORE_CTX *store)
depth, issuer, subject, err, X509_verify_cert_error_string(err));
}

return ok;
return preverify_ok;
}

int TlsOpenSslPrivate::OpensslBsockReadwrite(BareosSocket *bsock, char *ptr, int nbytes, bool write)
Expand Down Expand Up @@ -231,8 +231,12 @@ int TlsOpenSslPrivate::tls_pem_callback_dispatch(char *buf, int size, int rwflag

void TlsOpenSslPrivate::ClientContextInsertCredentials(const PskCredentials &credentials)
{
TlsOpenSslPrivate::psk_client_credentials.insert(
std::pair<const SSL_CTX *, PskCredentials>(openssl_ctx_, credentials));
if (!openssl_ctx_) { /* do not register nullptr */
Dmsg0(100, "Psk Server Callback: No SSL_CTX\n");
} else {
TlsOpenSslPrivate::psk_client_credentials.insert(
std::pair<const SSL_CTX *, PskCredentials>(openssl_ctx_, credentials));
}
}

unsigned int TlsOpenSslPrivate::psk_server_cb(SSL *ssl,
Expand All @@ -244,28 +248,37 @@ unsigned int TlsOpenSslPrivate::psk_server_cb(SSL *ssl,

SSL_CTX *openssl_ctx = SSL_get_SSL_CTX(ssl);

if (!openssl_ctx) {
Dmsg0(100, "Psk Server Callback: No SSL_CTX\n");
return result;
}

Dmsg1(100, "psk_server_cb. identitiy: %s.\n", identity);

if (openssl_ctx) {
std::string configured_psk;
GetTlsPskByFullyQualifiedResourceNameCb_t GetTlsPskByFullyQualifiedResourceNameCb =
reinterpret_cast<GetTlsPskByFullyQualifiedResourceNameCb_t>(SSL_CTX_get_ex_data(
openssl_ctx, TlsOpenSslPrivate::SslCtxExDataIndex::kGetTlsPskByFullyQualifiedResourceNameCb));
std::string configured_psk;
GetTlsPskByFullyQualifiedResourceNameCb_t GetTlsPskByFullyQualifiedResourceNameCb =
reinterpret_cast<GetTlsPskByFullyQualifiedResourceNameCb_t>(SSL_CTX_get_ex_data(
openssl_ctx, TlsOpenSslPrivate::SslCtxExDataIndex::kGetTlsPskByFullyQualifiedResourceNameCb));

ConfigurationParser *config = reinterpret_cast<ConfigurationParser*>(SSL_CTX_get_ex_data(
openssl_ctx, TlsOpenSslPrivate::SslCtxExDataIndex::kConfigurationParserPtr));
if (!GetTlsPskByFullyQualifiedResourceNameCb) {
Dmsg0(100, "Callback not set: kGetTlsPskByFullyQualifiedResourceNameCb\n");
return result;
}

if (!GetTlsPskByFullyQualifiedResourceNameCb || !config) {
Dmsg0(100, "GetTlsPskByFullyQualifiedResourceNameCb not set for psk server callback\n");
return result;
}
if (GetTlsPskByFullyQualifiedResourceNameCb(config, identity, configured_psk)) {
int psklen = Bsnprintf((char *)psk_output, max_psk_len, "%s", configured_psk.c_str());
Dmsg1(100, "psk_server_cb. psk: %s.\n", psk_output);
result = (psklen < 0) ? 0 : psklen;
} else {
Dmsg0(100, "Error, TLS-PSK credentials not found.\n");
}
ConfigurationParser *config = reinterpret_cast<ConfigurationParser*>(SSL_CTX_get_ex_data(
openssl_ctx, TlsOpenSslPrivate::SslCtxExDataIndex::kConfigurationParserPtr));

if (!config) {
Dmsg0(100, "Config not set: kConfigurationParserPtr\n");
return result;
}

if (!GetTlsPskByFullyQualifiedResourceNameCb(config, identity, configured_psk)) {
Dmsg0(100, "Error, TLS-PSK credentials not found.\n");
} else {
int psklen = Bsnprintf((char *)psk_output, max_psk_len, "%s", configured_psk.c_str());
Dmsg1(100, "psk_server_cb. psk: %s.\n", psk_output);
result = (psklen < 0) ? 0 : psklen;
}
return result;
}
Expand All @@ -279,32 +292,32 @@ unsigned int TlsOpenSslPrivate::psk_client_cb(SSL *ssl,
{
const SSL_CTX *openssl_ctx = SSL_get_SSL_CTX(ssl);

if (openssl_ctx) {
if (psk_client_credentials.find(openssl_ctx) != psk_client_credentials.end()) {
const PskCredentials &credentials = TlsOpenSslPrivate::psk_client_credentials.at(openssl_ctx);
int ret = Bsnprintf(identity, max_identity_len, "%s", credentials.get_identity().c_str());

if (ret < 0 || (unsigned int)ret > max_identity_len) {
Dmsg0(100, "Error, identify too long\n");
return 0;
}
Dmsg1(100, "psk_client_cb. identity: %s.\n", identity);

ret = Bsnprintf((char *)psk, max_psk_len, "%s", credentials.get_psk().c_str());
if (ret < 0 || (unsigned int)ret > max_psk_len) {
Dmsg0(100, "Error, psk too long\n");
return 0;
}
Dmsg1(100, "psk_client_cb. psk: %s.\n", psk);

return ret;
} else {
Dmsg0(100, "Error, TLS-PSK CALLBACK not set.\n");
if (!openssl_ctx) {
Dmsg0(100, "Psk Client Callback: No SSL_CTX\n");
return 0;
}
}

Dmsg0(100, "Error, SSL_CTX not set.\n");
if (psk_client_credentials.find(openssl_ctx) == psk_client_credentials.end()) {
Dmsg0(100, "Error, TLS-PSK CALLBACK not set because SSL_CTX is not registered.\n");
} else {
const PskCredentials &credentials = TlsOpenSslPrivate::psk_client_credentials.at(openssl_ctx);
int ret = Bsnprintf(identity, max_identity_len, "%s", credentials.get_identity().c_str());

if (ret < 0 || (unsigned int)ret > max_identity_len) {
Dmsg0(100, "Error, identify too long\n");
return 0;
}
Dmsg1(100, "psk_client_cb. identity: %s.\n", identity);

ret = Bsnprintf((char *)psk, max_psk_len, "%s", credentials.get_psk().c_str());
if (ret < 0 || (unsigned int)ret > max_psk_len) {
Dmsg0(100, "Error, psk too long\n");
return 0;
}
Dmsg1(100, "psk_client_cb. psk: %s.\n", psk);

return ret;
}
return 0;
}

Expand Down

0 comments on commit d1e0149

Please sign in to comment.