Re: [spice-server v2] ssl: Dump OpenSSL error stack on errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ping?

On Mon, Jan 14, 2019 at 11:59:17AM +0100, Christophe Fergeau wrote:
> 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 <cfergeau@xxxxxxxxxx>
> ---
> Changes since v1:
> - forgot to squash 2 changes in the previous patch
> 
> 
>  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 fd5b8cd12..0baa7eb88 100644
> --- a/server/red-stream.c
> +++ b/server/red-stream.c
> @@ -498,7 +498,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 221989266..38e8cc2d2 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1575,11 +1575,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;
>          }
>  
> @@ -1587,9 +1589,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);
> @@ -1889,6 +1891,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");
>      }
>  
> @@ -2087,8 +2090,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';
> @@ -2687,6 +2690,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;
>      }
>  
> @@ -2694,12 +2698,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;
>      }
>  
> @@ -2747,6 +2753,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;
>      }
>  
> @@ -2794,6 +2801,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;
>      }
>  
> @@ -2806,6 +2814,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;
>      }
>  
> @@ -2827,6 +2836,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 b8a40b1d9..b440c0648 100644
> --- a/server/utils.c
> +++ b/server/utils.c
> @@ -20,6 +20,7 @@
>  
>  #include <glib.h>
>  #include <spice/enums.h>
> +#include <openssl/err.h>
>  #include <common/macros.h>
>  
>  #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 57d89bee0..ea0de1529 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_ */
> -- 
> 2.20.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]