From ef244d6e7f810deb5f6fc6d952bfcb3ca4991ce8 Mon Sep 17 00:00:00 2001 From: Sebastian Reimers Date: Thu, 6 Jun 2024 15:28:36 +0200 Subject: [PATCH] tls: refactoring SNI ctx usage for libressl support (#1136) * tls: refactoring sni ctx usage for libressl support * fix load verify locations * fix cleanup * fix ctx uninitialized goto * improve sni callback error handling * cleanup tls.h * refactor cert_store verify * check cert_store pointer * refactor goto error handling --- src/tls/openssl/sni.c | 51 +++------ src/tls/openssl/tls.c | 258 ++++++++++++++++-------------------------- src/tls/openssl/tls.h | 5 +- 3 files changed, 114 insertions(+), 200 deletions(-) diff --git a/src/tls/openssl/sni.c b/src/tls/openssl/sni.c index 1f02f707b..8298e40fd 100644 --- a/src/tls/openssl/sni.c +++ b/src/tls/openssl/sni.c @@ -19,12 +19,10 @@ #include "tls.h" -#define DEBUG_MODULE "tls" +#define DEBUG_MODULE "tls/sni" #define DEBUG_LEVEL 5 #include -#if !defined(LIBRESSL_VERSION_NUMBER) - struct tls_conn; @@ -161,48 +159,33 @@ static int ssl_set_verify_client(SSL *ssl, const char *host) } -static int ssl_use_cert(SSL *ssl, struct tls_cert *uc) -{ - int err; - long r; - - SSL_certs_clear(ssl); - r = SSL_clear_chain_certs(ssl); - if (r != 1) - return EINVAL; - - r = SSL_use_cert_and_key(ssl, tls_cert_x509(uc), tls_cert_pkey(uc), - tls_cert_chain(uc), 1); - if (r != 1) { - ERR_clear_error(); - return EINVAL; - } - - err = ssl_set_verify_client(ssl, tls_cert_host(uc)); - return err; -} - - static int ssl_servername_handler(SSL *ssl, int *al, void *arg) { - struct tls *tls = arg; + struct tls *tls = arg; struct tls_cert *uc = NULL; const char *sni; - (void)al; sni = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); - if (!str_isset(sni)) - goto out; + if (!str_isset(sni)) { + *al = SSL_AD_UNRECOGNIZED_NAME; + return SSL_TLSEXT_ERR_ALERT_FATAL; + } /* find and apply matching certificate */ uc = tls_cert_for_sni(tls, sni); - if (!uc) - goto out; + if (!uc) { + *al = SSL_AD_UNRECOGNIZED_NAME; + return SSL_TLSEXT_ERR_ALERT_FATAL; + } DEBUG_INFO("found cert for sni %s\n", sni); - (void)ssl_use_cert(ssl, uc); + if (SSL_set_SSL_CTX(ssl, tls_cert_ctx(uc)) == NULL) { + *al = SSL_AD_INTERNAL_ERROR; + return SSL_TLSEXT_ERR_ALERT_FATAL; + } + + (void)ssl_set_verify_client(ssl, tls_cert_host(uc)); -out: return SSL_TLSEXT_ERR_OK; } @@ -218,5 +201,3 @@ void tls_enable_sni(struct tls *tls) ssl_servername_handler); SSL_CTX_set_tlsext_servername_arg(tls_ssl_ctx(tls), tls); } - -#endif /* !defined(LIBRESSL_VERSION_NUMBER) */ diff --git a/src/tls/openssl/tls.c b/src/tls/openssl/tls.c index b53f70283..b182b74ef 100644 --- a/src/tls/openssl/tls.c +++ b/src/tls/openssl/tls.c @@ -58,9 +58,7 @@ struct tls { */ struct tls_cert { struct le le; - X509 *x509; - EVP_PKEY *pkey; - STACK_OF(X509) *chain; + SSL_CTX *ctx; char *host; }; @@ -164,7 +162,6 @@ static int keytype2int(enum tls_keytype type) } -#if !defined(LIBRESSL_VERSION_NUMBER) /** * OpenSSL verify handler for debugging purposes. Prints only warnings in the * default build @@ -207,7 +204,6 @@ int tls_verify_handler(int ok, X509_STORE_CTX *ctx) return ok; } -#endif static int tls_verify_idx = -1; @@ -223,95 +219,112 @@ static void tls_init_verify_idx(void) } -/** - * Allocate a new TLS context - * - * @param tlsp Pointer to allocated TLS context - * @param method TLS method - * @param keyfile Optional private key file - * @param pwd Optional password - * - * @return 0 if success, otherwise errorcode - */ -int tls_alloc(struct tls **tlsp, enum tls_method method, const char *keyfile, - const char *pwd) +static int tls_ctx_alloc(SSL_CTX **ctxp, enum tls_method method, + const char *certf, const char *pwd, struct tls *tls) { - struct tls *tls; - int r, err; + int err = 0; + int r; + SSL_CTX *ctx; int min_proto = 0; - if (!tlsp) - return EINVAL; - - tls = mem_zalloc(sizeof(*tls), destructor); - if (!tls) - return ENOMEM; - - tls->verify_server = true; switch (method) { case TLS_METHOD_TLS: case TLS_METHOD_SSLV23: - tls->ctx = SSL_CTX_new(TLS_method()); + ctx = SSL_CTX_new(TLS_method()); min_proto = TLS1_2_VERSION; break; case TLS_METHOD_DTLS: case TLS_METHOD_DTLSV1: case TLS_METHOD_DTLSV1_2: - tls->ctx = SSL_CTX_new(DTLS_method()); + ctx = SSL_CTX_new(DTLS_method()); break; default: DEBUG_WARNING("tls method %d not supported\n", method); - err = ENOSYS; + return ENOSYS; + } + + if (!ctx) { + ERR_clear_error(); + return ENOMEM; + } + + SSL_CTX_set_min_proto_version(ctx, min_proto); + + if (!certf) goto out; + + /* Load our keys and certificates */ + if (pwd && tls) { + err = str_dup(&tls->pass, pwd); + if (err) + goto out; + + SSL_CTX_set_default_passwd_cb(ctx, password_cb); + SSL_CTX_set_default_passwd_cb_userdata(ctx, tls); } - if (!tls->ctx) { + r = SSL_CTX_use_certificate_chain_file(ctx, certf); + if (r <= 0) { + DEBUG_WARNING("Can't read certificate file: %s (%d)\n", certf, + r); ERR_clear_error(); - err = ENOMEM; + err = EINVAL; goto out; } - err = tls_set_min_proto_version(tls, min_proto); - if (err) + r = SSL_CTX_use_PrivateKey_file(ctx, certf, SSL_FILETYPE_PEM); + if (r <= 0) { + DEBUG_WARNING("Can't read key file: %s (%d)\n", certf, r); + ERR_clear_error(); + err = EINVAL; goto out; + } -#if defined(TRACE_SSL) - SSL_CTX_set_keylog_callback(tls->ctx, tls_keylogger_cb); -#endif +out: + if (err) + SSL_CTX_free(ctx); + else + *ctxp = ctx; - /* Load our keys and certificates */ - if (keyfile) { - if (pwd) { - err = str_dup(&tls->pass, pwd); - if (err) - goto out; + return err; +} - SSL_CTX_set_default_passwd_cb(tls->ctx, password_cb); - SSL_CTX_set_default_passwd_cb_userdata(tls->ctx, tls); - } - r = SSL_CTX_use_certificate_chain_file(tls->ctx, keyfile); - if (r <= 0) { - DEBUG_WARNING("Can't read certificate file: %s (%d)\n", - keyfile, r); - ERR_clear_error(); - err = EINVAL; - goto out; - } +/** + * Allocate a new TLS context + * + * @param tlsp Pointer to allocated TLS context + * @param method TLS method + * @param keyfile Optional private key file + * @param pwd Optional password + * + * @return 0 if success, otherwise errorcode + */ +int tls_alloc(struct tls **tlsp, enum tls_method method, const char *keyfile, + const char *pwd) +{ + struct tls *tls; + int err; - r = SSL_CTX_use_PrivateKey_file(tls->ctx, keyfile, - SSL_FILETYPE_PEM); - if (r <= 0) { - DEBUG_WARNING("Can't read key file: %s (%d)\n", - keyfile, r); - ERR_clear_error(); - err = EINVAL; - goto out; - } - } + if (!tlsp) + return EINVAL; + + tls = mem_zalloc(sizeof(*tls), destructor); + if (!tls) + return ENOMEM; + + err = tls_ctx_alloc(&tls->ctx, method, keyfile, pwd, tls); + if (err) + goto out; + + tls->verify_server = true; + +#if defined(TRACE_SSL) + SSL_CTX_set_keylog_callback(tls->ctx, tls_keylogger_cb); +#endif err = hash_alloc(&tls->reuse.ht_sessions, 256); if (err) @@ -1405,7 +1418,6 @@ int tls_set_ciphers(struct tls *tls, const char *cipherv[], size_t count) */ int tls_set_verify_server(struct tls_conn *tc, const char *host) { -#if !defined(LIBRESSL_VERSION_NUMBER) struct sa sa; if (!tc || !host) @@ -1434,12 +1446,6 @@ int tls_set_verify_server(struct tls_conn *tc, const char *host) SSL_set_verify(tc->ssl, SSL_VERIFY_PEER, tls_verify_handler); return 0; -#else - (void)tc; - (void)host; - - return ENOSYS; -#endif } @@ -1947,17 +1953,14 @@ SSL_CTX *tls_ssl_ctx(const struct tls *tls) } -#if !defined(LIBRESSL_VERSION_NUMBER) static void tls_cert_destructor(void *arg) { struct tls_cert *uc = arg; mem_deref(uc->host); - X509_free(uc->x509); - EVP_PKEY_free(uc->pkey); - sk_X509_pop_free(uc->chain, X509_free); + if (uc->ctx) + SSL_CTX_free(uc->ctx); } -#endif /** @@ -1973,11 +1976,8 @@ static void tls_cert_destructor(void *arg) */ int tls_add_certf(struct tls *tls, const char *certf, const char *host) { -#if !defined(LIBRESSL_VERSION_NUMBER) struct tls_cert *uc; - BIO *bio = NULL; int err = 0; - int ret; if (!tls || !certf) return EINVAL; @@ -1989,73 +1989,30 @@ int tls_add_certf(struct tls *tls, const char *certf, const char *host) if (str_isset(host)) { err = str_dup(&uc->host, host); if (err) - goto out; + goto err; } - bio = BIO_new_file(certf, "r"); - if (!bio) { - err = EIO; - goto out; - } - - uc->x509 = PEM_read_bio_X509(bio, NULL, NULL, NULL); - if (!uc->x509) { - DEBUG_WARNING("Can't read certificate from file: %s\n", certf); - err = ENOTSUP; - goto out; - } - - while (1) { - X509 *ca = PEM_read_bio_X509(bio, NULL, 0, NULL); - if (!ca) - break; - - if (!uc->chain) - uc->chain = sk_X509_new_null(); - - if (!uc->chain) { - err = ENOMEM; - goto out; - } + err = tls_ctx_alloc(&uc->ctx, TLS_METHOD_TLS, certf, NULL, NULL); + if (err) + goto err; - if (!sk_X509_push(uc->chain, ca)) { - err = ENOMEM; - goto out; - } + X509_STORE *ca = SSL_CTX_get_cert_store(tls->ctx); + if (ca) { + X509_STORE_up_ref(ca); + SSL_CTX_set_cert_store(uc->ctx, ca); } - ret = BIO_reset(bio); - if (ret < 0 || !bio) { - err = EIO; - goto out; - } + list_append(&tls->certs, &uc->le, uc); + if (list_count(&tls->certs) == 1) + tls_enable_sni(tls); - uc->pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); - if (!uc->pkey) { - DEBUG_WARNING("Can't read private key from file: %s\n", certf); - err = ENOTSUP; - goto out; - } + return 0; -out: - BIO_free(bio); - if (err) { - ERR_clear_error(); - mem_deref(uc); - } - else { - list_append(&tls->certs, &uc->le, uc); - if (list_count(&tls->certs) == 1) - tls_enable_sni(tls); - } +err: + ERR_clear_error(); + mem_deref(uc); return err; -#else - (void)tls; - (void)certf; - (void)host; - return ENOSYS; -#endif } @@ -2068,36 +2025,15 @@ int tls_add_certf(struct tls *tls, const char *certf, const char *host) */ X509 *tls_cert_x509(struct tls_cert *hc) { - return hc ? hc->x509 : NULL; + return hc ? SSL_CTX_get0_certificate(hc->ctx) : NULL; } -/** - * Returns the private key of the TLS certificate - * - * @param hc TLS certificate - * - * @return The OpenSSL EVP_PKEY - */ -EVP_PKEY *tls_cert_pkey(struct tls_cert *hc) -{ - return hc ? hc->pkey : NULL; -} - +SSL_CTX *tls_cert_ctx(struct tls_cert *hc) { -/* - * Returns the certificate chain of the TLS certificate - * - * @param hc TLS certificate - * - * @return The OpenSSL stack of X509 - */ -struct stack_st_X509 *tls_cert_chain(struct tls_cert *hc) -{ - return hc ? hc->chain : NULL; + return hc ? hc->ctx : NULL; } - /** * Returns the host name of the TLS certificate * diff --git a/src/tls/openssl/tls.h b/src/tls/openssl/tls.h index 7094c9d69..ac780f0ae 100644 --- a/src/tls/openssl/tls.h +++ b/src/tls/openssl/tls.h @@ -27,14 +27,11 @@ struct tls_cert; void tls_flush_error(void); SSL_CTX *tls_ssl_ctx(const struct tls *tls); X509 *tls_cert_x509(struct tls_cert *hc); -EVP_PKEY *tls_cert_pkey(struct tls_cert *hc); +SSL_CTX *tls_cert_ctx(struct tls_cert *hc); -struct stack_st_X509 *tls_cert_chain(struct tls_cert *hc); const char *tls_cert_host(struct tls_cert *hc); const struct list *tls_certs(const struct tls *tls); struct tls_cert *tls_cert_for_sni(const struct tls *tls, const char *sni); -#if !defined(LIBRESSL_VERSION_NUMBER) int tls_verify_handler(int ok, X509_STORE_CTX *ctx); void tls_enable_sni(struct tls *tls); -#endif