Improve OpenSSL integration

Ensure the error stack is cleared after errors are processed.

Also handle NULL returns safely.

Makes the debug mode spew more data, though almost none of
the contexts is in reaction to network peer data.
This commit is contained in:
Přemysl Eric Janouch 2020-10-20 01:55:46 +02:00
parent 13c85aa361
commit 383f6af344
Signed by: p
GPG Key ID: A0420B94F92B9493
4 changed files with 37 additions and 15 deletions

View File

@ -50,6 +50,30 @@ init_openssl (void)
// --- To be moved to liberty -------------------------------------------------- // --- To be moved to liberty --------------------------------------------------
// FIXME: in xssl_get_error() we rely on error reasons never being NULL (i.e.,
// all loaded), which isn't very robust.
// TODO: check all places where this is used and see if we couldn't gain better
// information by piecing together some other subset of data from the error
// stack. Most often, this is used in an error_set() context, which would
// allow us to allocate memory instead of returning static strings.
static const char *
xerr_describe_error (void)
{
unsigned long err = ERR_get_error ();
if (!err)
return "undefined error";
const char *reason = ERR_reason_error_string (err);
do
// Not thread-safe, not a concern right now--need a buffer
print_debug ("%s", ERR_error_string (err, NULL));
while ((err = ERR_get_error ()));
if (!reason)
return "cannot retrieve error description";
return reason;
}
static ssize_t static ssize_t
strv_find (const struct strv *v, const char *s) strv_find (const struct strv *v, const char *s)
{ {

View File

@ -5320,13 +5320,13 @@ transport_tls_init_ca_set (SSL_CTX *ssl_ctx, const char *file, const char *path,
return error_set (e, "%s: %s", return error_set (e, "%s: %s",
"Failed to set locations for the CA certificate bundle", "Failed to set locations for the CA certificate bundle",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
} }
if (!SSL_CTX_set_default_verify_paths (ssl_ctx)) if (!SSL_CTX_set_default_verify_paths (ssl_ctx))
return error_set (e, "%s: %s", return error_set (e, "%s: %s",
"Couldn't load the default CA certificate bundle", "Couldn't load the default CA certificate bundle",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
return true; return true;
} }
@ -5416,7 +5416,7 @@ transport_tls_init_cert (struct server *s, SSL *ssl, struct error **e)
else if (!SSL_use_certificate_file (ssl, path, SSL_FILETYPE_PEM) else if (!SSL_use_certificate_file (ssl, path, SSL_FILETYPE_PEM)
|| !SSL_use_PrivateKey_file (ssl, path, SSL_FILETYPE_PEM)) || !SSL_use_PrivateKey_file (ssl, path, SSL_FILETYPE_PEM))
error_set (e, "%s: %s", "Setting the TLS client certificate failed", error_set (e, "%s: %s", "Setting the TLS client certificate failed",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
else else
result = true; result = true;
free (path); free (path);
@ -5474,7 +5474,7 @@ error_ssl_2:
error_ssl_1: error_ssl_1:
if (!error) if (!error)
error_set (&error, "%s: %s", "Could not initialize TLS", error_set (&error, "%s: %s", "Could not initialize TLS",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
error_propagate (e, error); error_propagate (e, error);
return false; return false;

8
kike.c
View File

@ -3261,7 +3261,7 @@ error_ssl_3:
SSL_free (c->ssl); SSL_free (c->ssl);
c->ssl = NULL; c->ssl = NULL;
error_ssl_2: error_ssl_2:
error_info = ERR_reason_error_string (ERR_get_error ()); error_info = xerr_describe_error ();
error_ssl_1: error_ssl_1:
print_debug ("could not initialize TLS for %s: %s", c->address, error_info); print_debug ("could not initialize TLS for %s: %s", c->address, error_info);
return false; return false;
@ -3540,7 +3540,7 @@ irc_initialize_ssl_ctx (struct server_context *ctx,
if (!ctx->ssl_ctx) if (!ctx->ssl_ctx)
{ {
error_set (e, "%s: %s", "could not initialize TLS", error_set (e, "%s: %s", "could not initialize TLS",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
return false; return false;
} }
SSL_CTX_set_verify (ctx->ssl_ctx, SSL_CTX_set_verify (ctx->ssl_ctx,
@ -3570,11 +3570,11 @@ irc_initialize_ssl_ctx (struct server_context *ctx,
error_set (e, "failed to select any cipher from the cipher list"); error_set (e, "failed to select any cipher from the cipher list");
else if (!SSL_CTX_use_certificate_chain_file (ctx->ssl_ctx, cert_path)) else if (!SSL_CTX_use_certificate_chain_file (ctx->ssl_ctx, cert_path))
error_set (e, "%s: %s", "setting the TLS certificate failed", error_set (e, "%s: %s", "setting the TLS certificate failed",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
else if (!SSL_CTX_use_PrivateKey_file else if (!SSL_CTX_use_PrivateKey_file
(ctx->ssl_ctx, key_path, SSL_FILETYPE_PEM)) (ctx->ssl_ctx, key_path, SSL_FILETYPE_PEM))
error_set (e, "%s: %s", "setting the TLS private key failed", error_set (e, "%s: %s", "setting the TLS private key failed",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
else else
// TODO: SSL_CTX_check_private_key()? It has probably already been // TODO: SSL_CTX_check_private_key()? It has probably already been
// checked by SSL_CTX_use_PrivateKey_file() above. // checked by SSL_CTX_use_PrivateKey_file() above.

View File

@ -280,7 +280,7 @@ irc_send (struct bot_context *ctx, const char *format, ...)
if (SSL_write (ctx->ssl, str.str, str.len) != (int) str.len) if (SSL_write (ctx->ssl, str.str, str.len) != (int) str.len)
{ {
print_debug ("%s: %s: %s", __func__, "SSL_write", print_debug ("%s: %s: %s", __func__, "SSL_write",
ERR_error_string (ERR_get_error (), NULL)); xerr_describe_error ());
result = false; result = false;
} }
} }
@ -320,13 +320,13 @@ irc_initialize_ca_set (SSL_CTX *ssl_ctx, const char *file, const char *path,
return error_set (e, "%s: %s", return error_set (e, "%s: %s",
"failed to set locations for the CA certificate bundle", "failed to set locations for the CA certificate bundle",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
} }
if (!SSL_CTX_set_default_verify_paths (ssl_ctx)) if (!SSL_CTX_set_default_verify_paths (ssl_ctx))
return error_set (e, "%s: %s", return error_set (e, "%s: %s",
"couldn't load the default CA certificate bundle", "couldn't load the default CA certificate bundle",
ERR_reason_error_string (ERR_get_error ())); xerr_describe_error ());
return true; return true;
} }
@ -407,7 +407,7 @@ irc_initialize_tls (struct bot_context *ctx, struct error **e)
else if (!SSL_use_certificate_file (ctx->ssl, path, SSL_FILETYPE_PEM) else if (!SSL_use_certificate_file (ctx->ssl, path, SSL_FILETYPE_PEM)
|| !SSL_use_PrivateKey_file (ctx->ssl, path, SSL_FILETYPE_PEM)) || !SSL_use_PrivateKey_file (ctx->ssl, path, SSL_FILETYPE_PEM))
print_error ("%s: %s", "setting the TLS client certificate failed", print_error ("%s: %s", "setting the TLS client certificate failed",
ERR_error_string (ERR_get_error (), NULL)); xerr_describe_error ());
free (path); free (path);
} }
@ -434,10 +434,8 @@ error_ssl_2:
SSL_CTX_free (ctx->ssl_ctx); SSL_CTX_free (ctx->ssl_ctx);
ctx->ssl_ctx = NULL; ctx->ssl_ctx = NULL;
error_ssl_1: error_ssl_1:
// XXX: these error strings are really nasty; also there could be
// multiple errors on the OpenSSL stack.
if (!error_info) if (!error_info)
error_info = ERR_error_string (ERR_get_error (), NULL); error_info = xerr_describe_error ();
return error_set (e, "%s: %s", "could not initialize TLS", error_info); return error_set (e, "%s: %s", "could not initialize TLS", error_info);
} }