summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPřemysl Eric Janouch <p@janouch.name>2020-10-20 01:55:46 +0200
committerPřemysl Eric Janouch <p@janouch.name>2020-10-20 01:55:46 +0200
commit383f6af344b07a4bc8f510310aaed5eb54f61033 (patch)
treedc3ee6ed541edf7b1b8d19b9e049b0fa59fcb630
parent13c85aa3614ace6aff2e1614a4ca9da5fab33b81 (diff)
downloadxK-383f6af344b07a4bc8f510310aaed5eb54f61033.tar.gz
xK-383f6af344b07a4bc8f510310aaed5eb54f61033.tar.xz
xK-383f6af344b07a4bc8f510310aaed5eb54f61033.zip
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.
-rw-r--r--common.c24
-rw-r--r--degesch.c8
-rw-r--r--kike.c8
-rw-r--r--zyklonb.c12
4 files changed, 37 insertions, 15 deletions
diff --git a/common.c b/common.c
index 0837188..24077a6 100644
--- a/common.c
+++ b/common.c
@@ -50,6 +50,30 @@ init_openssl (void)
// --- 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
strv_find (const struct strv *v, const char *s)
{
diff --git a/degesch.c b/degesch.c
index ac9cd8a..6d120c2 100644
--- a/degesch.c
+++ b/degesch.c
@@ -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",
"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))
return error_set (e, "%s: %s",
"Couldn't load the default CA certificate bundle",
- ERR_reason_error_string (ERR_get_error ()));
+ xerr_describe_error ());
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)
|| !SSL_use_PrivateKey_file (ssl, path, SSL_FILETYPE_PEM))
error_set (e, "%s: %s", "Setting the TLS client certificate failed",
- ERR_reason_error_string (ERR_get_error ()));
+ xerr_describe_error ());
else
result = true;
free (path);
@@ -5474,7 +5474,7 @@ error_ssl_2:
error_ssl_1:
if (!error)
error_set (&error, "%s: %s", "Could not initialize TLS",
- ERR_reason_error_string (ERR_get_error ()));
+ xerr_describe_error ());
error_propagate (e, error);
return false;
diff --git a/kike.c b/kike.c
index 1cb6091..1e344a8 100644
--- a/kike.c
+++ b/kike.c
@@ -3261,7 +3261,7 @@ error_ssl_3:
SSL_free (c->ssl);
c->ssl = NULL;
error_ssl_2:
- error_info = ERR_reason_error_string (ERR_get_error ());
+ error_info = xerr_describe_error ();
error_ssl_1:
print_debug ("could not initialize TLS for %s: %s", c->address, error_info);
return false;
@@ -3540,7 +3540,7 @@ irc_initialize_ssl_ctx (struct server_context *ctx,
if (!ctx->ssl_ctx)
{
error_set (e, "%s: %s", "could not initialize TLS",
- ERR_reason_error_string (ERR_get_error ()));
+ xerr_describe_error ());
return false;
}
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");
else if (!SSL_CTX_use_certificate_chain_file (ctx->ssl_ctx, cert_path))
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
(ctx->ssl_ctx, key_path, SSL_FILETYPE_PEM))
error_set (e, "%s: %s", "setting the TLS private key failed",
- ERR_reason_error_string (ERR_get_error ()));
+ xerr_describe_error ());
else
// TODO: SSL_CTX_check_private_key()? It has probably already been
// checked by SSL_CTX_use_PrivateKey_file() above.
diff --git a/zyklonb.c b/zyklonb.c
index d8417c5..0d27758 100644
--- a/zyklonb.c
+++ b/zyklonb.c
@@ -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)
{
print_debug ("%s: %s: %s", __func__, "SSL_write",
- ERR_error_string (ERR_get_error (), NULL));
+ xerr_describe_error ());
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",
"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))
return error_set (e, "%s: %s",
"couldn't load the default CA certificate bundle",
- ERR_reason_error_string (ERR_get_error ()));
+ xerr_describe_error ());
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)
|| !SSL_use_PrivateKey_file (ctx->ssl, path, SSL_FILETYPE_PEM))
print_error ("%s: %s", "setting the TLS client certificate failed",
- ERR_error_string (ERR_get_error (), NULL));
+ xerr_describe_error ());
free (path);
}
@@ -434,10 +434,8 @@ error_ssl_2:
SSL_CTX_free (ctx->ssl_ctx);
ctx->ssl_ctx = NULL;
error_ssl_1:
- // XXX: these error strings are really nasty; also there could be
- // multiple errors on the OpenSSL stack.
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);
}