fix(tls): AEAD-only 1.2 suites, server preference, TOCTOU key load, IP SAN

C5: restrict the TLS 1.2 and below cipher list to ECDHE AEAD suites
(ECDHE+AESGCM:ECDHE+CHACHA20, minus NULL/eNULL/MD5/RC4/3DES) instead of
HIGH (which includes CBC), and set SSL_OP_CIPHER_SERVER_PREFERENCE so the
server's order decides the negotiated cipher.  Client and server share
create_ssl_ctx, so both are updated.

C7: load the private key through an O_RDONLY|O_NOFOLLOW|O_CLOEXEC fd,
fstat that fd and validate owner/mode (now also rejecting group/other
execute bits), then load from the fd via BIO_new_fd.  This removes the
stat-to-load TOCTOU race while keeping the exact-owner/0600 policy.

C8: verify an IP-literal client hostname against the certificate IP SAN
with X509_VERIFY_PARAM_set1_ip_asc instead of SSL_set1_host (a DNS
check), falling back to SSL_set1_host for real names.

Unit tests assert the server-preference option, the absence of CBC/RC4/
3DES suites, and that context creation still succeeds.
This commit is contained in:
2026-09-14 16:09:21 +02:00
parent 0d6c1f784f
commit 551c187005
2 changed files with 87 additions and 15 deletions
+76 -15
View File
@@ -4,7 +4,9 @@
#include "transport_tcp.h" #include "transport_tcp.h"
#include "utils.h" #include "utils.h"
#include <arpa/inet.h> #include <arpa/inet.h>
#include <fcntl.h>
#include <openssl/err.h> #include <openssl/err.h>
#include <openssl/pem.h>
#include <openssl/ssl.h> #include <openssl/ssl.h>
#include <signal.h> #include <signal.h>
#include <stdio.h> #include <stdio.h>
@@ -34,6 +36,59 @@ static void log_ssl_errors(void) {
} }
} }
/* Load the TLS private key through an already-opened, no-follow descriptor so
* the owner/mode policy is checked on the SAME file object that is loaded: an
* attacker cannot swap the path between a stat() and a later open() (TOCTOU).
* The exact-owner / 0600 policy is preserved and group/other execute bits are
* rejected as well. Ownership of the descriptor passes to the BIO and is
* released exactly once by BIO_free() (BIO_CLOSE). */
static bool load_private_key_secure(SSL_CTX* ctx, const char* key) {
int fd = open(key, O_RDONLY | O_NOFOLLOW | O_CLOEXEC);
if (fd < 0) {
char* escaped = output_escape(key, false);
log_message(LOG_LEVEL_ERROR, "Failed to open private key: %s",
escaped ? escaped : "<allocation failed>");
free(escaped);
return false;
}
struct stat key_stat;
if (fstat(fd, &key_stat) != 0 || !S_ISREG(key_stat.st_mode) || key_stat.st_uid != geteuid() ||
(key_stat.st_mode & (S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH | S_IXGRP | S_IXOTH))) {
log_message(LOG_LEVEL_ERROR,
"TLS private key must be a regular file owned by the current user and private "
"(mode 0600)");
close(fd);
return false;
}
BIO* bio = BIO_new_fd(fd, BIO_CLOSE);
if (!bio) {
close(fd);
log_message(LOG_LEVEL_ERROR, "Failed to read private key");
return false;
}
EVP_PKEY* pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);
BIO_free(bio); /* releases fd via BIO_CLOSE */
if (!pkey) {
char* escaped = output_escape(key, false);
log_message(LOG_LEVEL_ERROR, "Failed to load private key: %s",
escaped ? escaped : "<allocation failed>");
free(escaped);
log_ssl_errors();
return false;
}
int use_ok = SSL_CTX_use_PrivateKey(ctx, pkey);
EVP_PKEY_free(pkey);
if (use_ok != 1) {
char* escaped = output_escape(key, false);
log_message(LOG_LEVEL_ERROR, "Failed to use private key: %s",
escaped ? escaped : "<allocation failed>");
free(escaped);
log_ssl_errors();
return false;
}
return true;
}
static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key, static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key,
const char* ca_path) { const char* ca_path) {
if (!is_server && !ca_path) { if (!is_server && !ca_path) {
@@ -56,12 +111,21 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key
#ifdef SSL_OP_NO_RENEGOTIATION #ifdef SSL_OP_NO_RENEGOTIATION
SSL_CTX_set_options(ctx, SSL_OP_NO_RENEGOTIATION); SSL_CTX_set_options(ctx, SSL_OP_NO_RENEGOTIATION);
#endif #endif
/* Let the server's own preference order decide the negotiated cipher rather
* than the client's, so a client cannot steer both peers into a weaker (but
* still offered) suite. */
SSL_CTX_set_options(ctx, SSL_OP_CIPHER_SERVER_PREFERENCE);
if (SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) != 1) { if (SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) != 1) {
SSL_CTX_free(ctx); SSL_CTX_free(ctx);
return NULL; return NULL;
} }
if (SSL_CTX_set_cipher_list(ctx, "HIGH:!aNULL:!eNULL:!MD5:!RC4:!3DES") != 1) { /* TLS 1.2 and below: an AEAD-only suite list. "HIGH" still includes CBC
* suites (Lucky13/POODLE-adjacent MAC-then-encrypt constructions), so restrict
* the list to ECDHE key agreement with an AEAD record cipher (AES-GCM or
* ChaCha20-Poly1305). A NULL/weak/3DES cipher is never selectable. */
if (SSL_CTX_set_cipher_list(ctx, "ECDHE+AESGCM:ECDHE+CHACHA20:!aNULL:!eNULL:!MD5:!RC4:!3DES") !=
1) {
SSL_CTX_free(ctx); SSL_CTX_free(ctx);
return NULL; return NULL;
} }
@@ -79,13 +143,6 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key
#endif #endif
if (cert && key) { if (cert && key) {
struct stat key_stat;
if (stat(key, &key_stat) != 0 || !S_ISREG(key_stat.st_mode) || key_stat.st_uid != geteuid() ||
(key_stat.st_mode & (S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH))) {
log_message(LOG_LEVEL_ERROR, "TLS private key must be owned by the current user and private");
SSL_CTX_free(ctx);
return NULL;
}
if (SSL_CTX_use_certificate_file(ctx, cert, SSL_FILETYPE_PEM) <= 0) { if (SSL_CTX_use_certificate_file(ctx, cert, SSL_FILETYPE_PEM) <= 0) {
char* escaped = output_escape(cert, false); char* escaped = output_escape(cert, false);
log_message(LOG_LEVEL_ERROR, "Failed to load certificate: %s", log_message(LOG_LEVEL_ERROR, "Failed to load certificate: %s",
@@ -95,12 +152,7 @@ static SSL_CTX* create_ssl_ctx(bool is_server, const char* cert, const char* key
SSL_CTX_free(ctx); SSL_CTX_free(ctx);
return NULL; return NULL;
} }
if (SSL_CTX_use_PrivateKey_file(ctx, key, SSL_FILETYPE_PEM) <= 0) { if (!load_private_key_secure(ctx, key)) {
char* escaped = output_escape(key, false);
log_message(LOG_LEVEL_ERROR, "Failed to load private key: %s",
escaped ? escaped : "<allocation failed>");
free(escaped);
log_ssl_errors();
SSL_CTX_free(ctx); SSL_CTX_free(ctx);
return NULL; return NULL;
} }
@@ -144,7 +196,16 @@ static SSL* wrap_fd_with_ssl(int fd, SSL_CTX* ctx, bool is_server, const char* h
// Enable hostname verification for client connections when a hostname is provided. // Enable hostname verification for client connections when a hostname is provided.
// Must be done before SSL_connect to take effect during the handshake. // Must be done before SSL_connect to take effect during the handshake.
if (!is_server && hostname) { if (!is_server && hostname) {
if (SSL_set1_host(ssl, hostname) != 1) { /* An IP-literal host must be verified against the certificate's IP SAN
* (X509_check_ip_asc), not as a DNS name: SSL_set1_host would look for a
* DNS SAN that a legitimate IP-SAN certificate never carries. */
struct in_addr ipv4;
struct in6_addr ipv6;
bool is_ip_literal =
inet_pton(AF_INET, hostname, &ipv4) == 1 || inet_pton(AF_INET6, hostname, &ipv6) == 1;
int set_ok = is_ip_literal ? X509_VERIFY_PARAM_set1_ip_asc(SSL_get0_param(ssl), hostname)
: SSL_set1_host(ssl, hostname);
if (set_ok != 1) {
SSL_free(ssl); SSL_free(ssl);
return NULL; return NULL;
} }
+11
View File
@@ -24,6 +24,17 @@ static void test_server_create_tls_without_certs() {
#ifdef SSL_OP_NO_RENEGOTIATION #ifdef SSL_OP_NO_RENEGOTIATION
EXPECT_TRUE((SSL_CTX_get_options(ctx) & SSL_OP_NO_RENEGOTIATION) != 0); EXPECT_TRUE((SSL_CTX_get_options(ctx) & SSL_OP_NO_RENEGOTIATION) != 0);
#endif #endif
/* C5: the server's preference order decides the cipher and the TLS 1.2 list is
* AEAD-only (no CBC/RC4/3DES legacy suites). */
EXPECT_TRUE((SSL_CTX_get_options(ctx) & SSL_OP_CIPHER_SERVER_PREFERENCE) != 0);
STACK_OF(SSL_CIPHER)* ciphers = SSL_CTX_get_ciphers(ctx);
EXPECT_NOT_NULL(ciphers);
for (int i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
const char* name = SSL_CIPHER_get_name(sk_SSL_CIPHER_value(ciphers, i));
EXPECT_TRUE(name != NULL && strstr(name, "CBC") == NULL);
EXPECT_TRUE(name != NULL && strstr(name, "RC4") == NULL);
EXPECT_TRUE(name != NULL && strstr(name, "3DES") == NULL);
}
server_delete(&s); server_delete(&s);
EXPECT_NULL(s); EXPECT_NULL(s);
} }