Ticket #58 (closed defect: fixed)

Opened 20 months ago

Last modified 20 months ago

c2s/s2s: improper use of OpenSSL's SSL_CTX_use_certificate_chain_file() and SSL_CTX_load_verify_locations()

Reported by: smoku Owned by: smoku
Priority: major Component: General
Version: 2.1 Keywords:
Cc: Tracforge_linkmap:
Blocking: Blocked By:

Description

jabberd2's SSL initialization (in sx/ssl.c) is flawed, unfortunately. It will work with self-signed end-entity (i.e. server) certificates or end-entity certificates issued by a self-signed root, but as soon as a chain with intermediate CA certificates is involved ("cachain" config statement), things will break.

The v2_0 branch currently has it like this (sx/ssl.c):

    /* load the certificate */
    ret = SSL_CTX_use_certificate_file(ctx, pemfile, SSL_FILETYPE_PEM);
    if(ret != 1) {
        _sx_debug(ZONE, "couldn't load certificate from %s", pemfile);
        SSL_CTX_free(ctx);
        return 1;    }

    /* load the private key */
    ret = SSL_CTX_use_PrivateKey_file(ctx, pemfile, SSL_FILETYPE_PEM);
    if(ret != 1) {
        _sx_debug(ZONE, "couldn't load private key from %s", pemfile);
        SSL_CTX_free(ctx);
        return 1;
    }

    /* Load the CA chain, if configured */
    if (cachain != NULL) {
        ret = SSL_CTX_use_certificate_chain_file(ctx, cachain);
        if(ret != 1) {
            _sx_debug(ZONE, "WARNING: couldn't load CA chain");
        }
    }

In trunk, the last code block has been changed to use SSL_CTX_load_verify_locations() instead:

    /* Load the CA chain, if configured */
    if (cachain != NULL) {
        ret = SSL_CTX_load_verify_locations (ctx, cachain, NULL);
        if(ret != 1) {
            _sx_debug(ZONE, "WARNING: couldn't load CA chain: %s", cachain);
        }
    }

However, both cases reflect improper usage of these OpenSSL functions and probably come from a misunderstanding when reading OpenSSL's man page(s). Specifically, SSL_CTX_use_certificate(3) says:

it is recommended to use the SSL_CTX_use_certificate_chain_file() instead of the SSL_CTX_use_certificate_file() function in order to allow the use of complete certificate chains even when no trusted CA storage is used or when the CA issuing the certificate shall not be added to the trusted CA storage.

This is exactly the idea behind the "cachain" statement, as far as I see, but the current code doesn't achive that (both v2_0 and trunk).

The patch proposed in this message

http://mail.jabber.org/pipermail/jabberd/2006-April/003320.html

will "somehow" address the problem, but it's not the proper fix - which consists of replacing SSL_CTX_use_certificate() by SSL_CTX_use_certificate_chain_file(), making it possible to get rid of the "cachain" statement and use "pemfile" for both the server certificate plus private key *and* the certificate chain.

I will be attaching two patches (one for the V2_0 branch, the other for trunk) which

  • modify sx_ssl_init() to use SSL_CTX_use_certificate_chain_file() instead of SSL_CTX_use_certificate_file() [sx/ssl.c]
  • completely remove the "cachain" statement (both in the code handling the configuration file and in the config templates) [c2s/c2s.h, c2s/main.c, etc/*.xml.dist.in, s2s/main.c, s2s/s2s.h]
  • add a detailed explanation of the "pemfile" statement wherever applicable [etc/*.xml.dist.in]

Would be nice to see them applied soon. Let me know if you have further questions about the proposed changes (which are backward compatible, in my opinion - "pemfile"s with only the end-entity certificate and the private key will continue to work, and "cachain" was never really usable, I guess).

Attachments

cachain-trunk.diff (15.2 KB) - added by smoku 20 months ago.
Patch by Kaspar Brand <brand@…>
cachain-v2_0.diff (12.2 KB) - added by smoku 20 months ago.
Patch by Kaspar Brand <brand@…>

Change History

Changed 20 months ago by smoku

Patch by Kaspar Brand <brand@…>

Changed 20 months ago by smoku

Patch by Kaspar Brand <brand@…>

Changed 20 months ago by smoku

  • status changed from new to assigned

This is almost all true. With one exception.

jabberd does not need global cachain for presenting it's cert to clients, but it needs global cachain to verify incoming certificates on s2s connections, so there is a need to support it.

Changed 20 months ago by smoku

  • status changed from assigned to closed
  • resolution set to fixed

(In [162]) Integrated CA-chain chandling patch by Kaspar Brand. Fixes #58

Note: See TracTickets for help on using tickets.