From 474158dfeff442f0ffc3877e24b9a2f08064c928 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Mon, 14 Jan 2019 11:59:17 +0100 Subject: [PATCH] ssl: Dump OpenSSL error stack on errors Bugs such as https://bugzilla.redhat.com/show_bug.cgi?id=1651882 can be quite tricky to figure out without the detailed OpenSSL error. This commit adds a detailed dump of the OpenSSL error stack when an OpenSSL failure happens. In the bug above, this would have displayed: (process:13154): Spice-WARNING **: 05:43:10.139: reds.c:2816:reds_init_ssl: Could not load certificates from /etc/pki/libvirt-spice/server-cert.pem (process:13154): Spice-WARNING **: 05:43:10.140: error:140AB18F:SSL routines:SSL_CTX_use_certificate:ee key too small Signed-off-by: Christophe Fergeau Acked-by: Uri Lublin --- server/red-stream.c | 2 +- server/reds.c | 20 +++++++++++++++----- server/utils.c | 14 ++++++++++++++ server/utils.h | 2 ++ 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/server/red-stream.c b/server/red-stream.c index 55ad170f..4fb2d2bf 100644 --- a/server/red-stream.c +++ b/server/red-stream.c @@ -500,7 +500,7 @@ RedStreamSslStatus red_stream_ssl_accept(RedStream *stream) } } - ERR_print_errors_fp(stderr); + red_dump_openssl_errors(); spice_warning("SSL_accept failed, error=%d", ssl_error); SSL_free(stream->priv->ssl); stream->priv->ssl = NULL; diff --git a/server/reds.c b/server/reds.c index 24047bda..1a909ae3 100644 --- a/server/reds.c +++ b/server/reds.c @@ -1705,11 +1705,13 @@ static bool reds_send_link_ack(RedsState *reds, RedLinkInfo *link) || !red_link_info_test_capability(link, SPICE_COMMON_CAP_AUTH_SASL)) { if (!(link->tiTicketing.rsa = RSA_new())) { spice_warning("RSA new failed"); + red_dump_openssl_errors(); return FALSE; } if (!(bio = BIO_new(BIO_s_mem()))) { spice_warning("BIO new failed"); + red_dump_openssl_errors(); return FALSE; } @@ -1717,9 +1719,9 @@ static bool reds_send_link_ack(RedsState *reds, RedLinkInfo *link) SPICE_TICKET_KEY_PAIR_LENGTH, link->tiTicketing.bn, NULL) != 1) { - spice_warning("Failed to generate %d bits RSA key: %s", - SPICE_TICKET_KEY_PAIR_LENGTH, - ERR_error_string(ERR_get_error(), NULL)); + spice_warning("Failed to generate %d bits RSA key", + SPICE_TICKET_KEY_PAIR_LENGTH); + red_dump_openssl_errors(); goto end; } link->tiTicketing.rsa_size = RSA_size(link->tiTicketing.rsa); @@ -2019,6 +2021,7 @@ static void openssl_init(RedLinkInfo *link) link->tiTicketing.bn = BN_new(); if (!link->tiTicketing.bn) { + red_dump_openssl_errors(); spice_error("OpenSSL BIGNUMS alloc failed"); } @@ -2217,8 +2220,8 @@ static void reds_handle_ticket(void *opaque) link->tiTicketing.rsa, RSA_PKCS1_OAEP_PADDING); if (password_size == -1) { - spice_warning("failed to decrypt RSA encrypted password: %s", - ERR_error_string(ERR_get_error(), NULL)); + spice_warning("failed to decrypt RSA encrypted password"); + red_dump_openssl_errors(); goto error; } password[password_size] = '\0'; @@ -2817,6 +2820,7 @@ static int load_dh_params(SSL_CTX *ctx, char *file) if ((bio = BIO_new_file(file, "r")) == NULL) { spice_warning("Could not open DH file"); + red_dump_openssl_errors(); return -1; } @@ -2824,12 +2828,14 @@ static int load_dh_params(SSL_CTX *ctx, char *file) BIO_free(bio); if (ret == 0) { spice_warning("Could not read DH params"); + red_dump_openssl_errors(); return -1; } if (SSL_CTX_set_tmp_dh(ctx, ret) < 0) { spice_warning("Could not set DH params"); + red_dump_openssl_errors(); return -1; } @@ -2877,6 +2883,7 @@ static void openssl_thread_setup(void) * don't do it twice to avoid possible races. */ if (CRYPTO_get_locking_callback() != NULL) { + red_dump_openssl_errors(); return; } @@ -2924,6 +2931,7 @@ static int reds_init_ssl(RedsState *reds) reds->ctx = SSL_CTX_new(ssl_method); if (!reds->ctx) { spice_warning("Could not allocate new SSL context"); + red_dump_openssl_errors(); return -1; } @@ -2936,6 +2944,7 @@ static int reds_init_ssl(RedsState *reds) spice_debug("Loaded certificates from %s", reds->config->ssl_parameters.certs_file); } else { spice_warning("Could not load certificates from %s", reds->config->ssl_parameters.certs_file); + red_dump_openssl_errors(); return -1; } @@ -2957,6 +2966,7 @@ static int reds_init_ssl(RedsState *reds) spice_debug("Loaded CA certificates from %s", reds->config->ssl_parameters.ca_certificate_file); } else { spice_warning("Could not use CA file %s", reds->config->ssl_parameters.ca_certificate_file); + red_dump_openssl_errors(); return -1; } diff --git a/server/utils.c b/server/utils.c index b8a40b1d..b440c064 100644 --- a/server/utils.c +++ b/server/utils.c @@ -20,6 +20,7 @@ #include #include +#include #include #include "utils.h" @@ -114,3 +115,16 @@ int red_channel_name_to_type(const char *name) } return -1; } + +void red_dump_openssl_errors(void) +{ + int ssl_error; + + ssl_error = ERR_get_error(); + while (ssl_error != 0) { + char error_str[256]; + ERR_error_string_n(ssl_error, error_str, sizeof(error_str)); + g_warning("%s", error_str); + ssl_error = ERR_get_error(); + } +} diff --git a/server/utils.h b/server/utils.h index 57d89bee..ea0de152 100644 --- a/server/utils.h +++ b/server/utils.h @@ -73,4 +73,6 @@ int rgb32_data_has_alpha(int width, int height, size_t stride, const char *red_channel_type_to_str(int type); int red_channel_name_to_type(const char *name); +void red_dump_openssl_errors(void); + #endif /* UTILS_H_ */