Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve quality of TLS logging #1926

Merged
merged 1 commit into from Jul 16, 2021
Merged

Conversation

matt335672
Copy link
Member

This PR makes some improvements to the TLS logging.

Example PRs where this could help are #1922 and #1776.

The following improvements are made:-

  1. The contents of the SSL error stack are dumped if an error occurs. Although the SSL stack contents are quite technical, they can help by providing more information on an error. Here's an example from Does xrdp support ECC? #1776 (which would now be logged if Does xrdp support ECC? #1776 hadn't been addressed):-
    [20210621-18:01:26] [ERROR] [ssl_tls_accept(ssl_calls.c:799)] Error loading TLS private key from /etc/xrdp/ec-key.pem
    [20210621-18:01:26] [ERROR] [dump_ssl_error_stack(ssl_calls.c:629)] SSL: error:0607907F:digital envelope routines:EVP_PKEY_get0_RSA:expecting an rsa key
    [20210621-18:01:26] [ERROR] [dump_ssl_error_stack(ssl_calls.c:629)] SSL: error:140B3009:SSL routines:SSL_CTX_use_RSAPrivateKey_file:PEM lib
    
  2. Only the first TLS error is logged. This prevents the log file from being polluted with additional errors related to SSL_shutdown(). The intent is to make it easier for users to report problems.
  3. A basic check is made that the certificate and private key match. This check works for a chain as well as a self-signed cert.
  4. An error is logged if the private key file has been encrypted, in which case we can't read it.

Other checks could be made on our end, but things like the certificate chain validity are best checked (and logged) by the client.

@matt335672 matt335672 merged commit 2710703 into neutrinolabs:devel Jul 16, 2021
@matt335672 matt335672 deleted the ssl_error branch September 9, 2021 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants