Merge pull request #128 from cgzones/tls

(D)TLS fixes, especially certificate verification

Author: ssahani

README.md

@@ -71,7 +71,7 @@ systemd-netlogd reads configuration files named `/etc/systemd/netlogd.conf` and
         Specifies the minimum delay before subsequent attempts to contact a Log server are made. Takes a time span value. The default unit is seconds, but other units may be specified, see systemd.time(5). Defaults to 30 seconds and must not be smaller than 1 second.
 
     TLSCertificateAuthMode=
-        Specifies whether to validate the certificate. Takes one of no, allow, deny, warn. Defaults to 'no' which disables certificate validation.
+        Specifies whether to validate the certificate. Takes one of no, allow, deny, warn. Defaults to 'deny' which rejects certificates failed to validate.
 
     KeepAlive=
         Takes a boolean argument. If true, the TCP/IP stack will send a keep alive message after 2h (depending on the configuration of /proc/sys/net/ipv4/tcp_keepalive_time) for all TCP streams accepted on this socket. This controls the SO_KEEPALIVE socket option (see socket(7) and the TCP Keepalive HOWTO for details.) Defaults to false.

conf/netlogd.conf.in

@@ -1,7 +1,10 @@
 [Network]
 #Address=239.0.0.1:6000
 #Protocol=udp
+#TLSCertificateAuthMode=deny
 #LogFormat=rfc5424
+#Directory=
+#Namespace=
 #StructuredData=
 #UseSysLogStructuredData=no
 #UseSysLogMsgId=no
@@ -10,5 +13,5 @@
 #KeepAliveTimeSec=
 #KeepAliveIntervalSec=
 #KeepAliveProbes=
-#NoDelay=
+#NoDelay=no
 #SendBuffer=

src/meson.build

@@ -120,6 +120,8 @@ systemd_netlogd_sources = files('''
                         netlog/netlog-protocol.h
                         netlog/netlog-dtls.c
                         netlog/netlog-dtls.h
+                        netlog/netlog-ssl.c
+                        netlog/netlog-ssl.h
                         netlog/netlog-tls.c
                         netlog/netlog-tls.h
                         '''.split())

src/netlog/netlog-dtls.c

@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
+#include "netlog-dtls.h"
+
 #include <arpa/inet.h>
 #include <netinet/in.h>
 #include <openssl/bio.h>
@@ -11,21 +13,28 @@
 #include "fd-util.h"
 #include "io-util.h"
 #include "iovec-util.h"
-#include "netlog-dtls.h"
+
+#include "netlog-ssl.h"
 
 static int dtls_write(DTLSManager *m, const char *buf, size_t count) {
         int r;
 
         assert(m);
         assert(m->ssl);
+        assert(m->pretty_address);
         assert(buf);
         assert(count > 0);
         assert(count < INT_MAX);
 
         ERR_clear_error();
         r = SSL_write(m->ssl, buf, count);
-        if (r <= 0)
-                return log_error_errno(r, "DTLS: Failed to invoke SSL_write: %s", TLS_ERROR_STRING(SSL_get_error(m->ssl, r)));
+        if (r <= 0) {
+                int error = SSL_get_error(m->ssl, r);
+                if (IN_SET(error, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE))
+                        return log_info_errno(SYNTHETIC_ERRNO(EAGAIN), "DTLS: Failed to invoke SSL_write to %s: %s", m->pretty_address, TLS_ERROR_STRING(error));
+                else
+                        return log_error_errno(SYNTHETIC_ERRNO(EPIPE), "DTLS: Failed to invoke SSL_write to %s: %s", m->pretty_address, TLS_ERROR_STRING(error));
+        }
 
         return log_debug("DTLS: Successful SSL_write: %d bytes", r);
 }
@@ -56,14 +65,15 @@ int dtls_connect(DTLSManager *m, SocketAddress *address) {
         _cleanup_free_ char *pretty = NULL;
         const SSL_CIPHER *cipher;
         socklen_t salen;
-        SSL_CTX *ctx;
         struct timeval timeout = {
                 .tv_sec = 3,
                 .tv_usec = 0,
         };
-        int fd, r;
+        _cleanup_close_ int fd = -1;
+        int r;
 
         assert(m);
+        assert(m->ctx);
         assert(address);
 
         switch (address->sockaddr.sa.sa_family) {
@@ -91,39 +101,36 @@ int dtls_connect(DTLSManager *m, SocketAddress *address) {
 
         log_debug("DTLS: Connected to remote server: '%s'", pretty);
 
-        ctx = SSL_CTX_new(DTLS_method());
-        if (!ctx)
-                return log_error_errno(SYNTHETIC_ERRNO(ENOMEM),
-                                       "DTLS: Failed to allocate memory for SSL CTX: %m");
-
-        ssl = SSL_new(ctx);
-        if (!ssl)
-                return log_error_errno(SYNTHETIC_ERRNO(ENOMEM),
-                                       "DTLS: Failed to allocate memory for ssl: %s",
-                                       ERR_error_string(ERR_get_error(), NULL));
-
         /* Create BIO from socket array! */
         bio = BIO_new_dgram(fd, BIO_NOCLOSE);
         if (!bio)
                 return log_error_errno(SYNTHETIC_ERRNO(ENOMEM),
                                        "DTLS: Failed to allocate memory for bio: %m");
 
         BIO_ctrl(bio, BIO_CTRL_DGRAM_SET_CONNECTED, 0, &address);
+        /* Set and activate timeouts */
+        BIO_ctrl(bio, BIO_CTRL_DGRAM_SET_RECV_TIMEOUT, 0, &timeout);
+
+        ssl = SSL_new(m->ctx);
+        if (!ssl)
+                return log_error_errno(SYNTHETIC_ERRNO(ENOMEM),
+                                       "DTLS: Failed to allocate memory for ssl: %s",
+                                       ERR_error_string(ERR_get_error(), NULL));
+
         SSL_set_bio(ssl, bio, bio);
-        m->bio = TAKE_PTR(bio);
+        bio = NULL;
 
         /* Certification verification  */
-        if (m->auth_mode != OPEN_SSL_CERTIFICATE_AUTH_MODE_NONE && m->auth_mode != OPEN_SSL_CERTIFICATE_AUTH_MODE_INVALID) {
+        if (m->auth_mode != OPEN_SSL_CERTIFICATE_AUTH_MODE_NONE) {
                 log_debug("DTLS: enable certificate verification");
 
-                SSL_set_ex_data(ssl, 0, m);
-                SSL_set_ex_data(ssl, 1, address);
+                SSL_set_ex_data(ssl, EX_DATA_TLSMANAGER, m);
+                SSL_set_ex_data(ssl, EX_DATA_PRETTYADDRESS, pretty);
                 SSL_set_verify(ssl, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, ssl_verify_certificate_validity);
         } else {
                 log_debug("DTLS: disable certificate verification");
-                SSL_CTX_set_verify(ctx, SSL_VERIFY_NONE, NULL);
+                SSL_set_verify(ssl, SSL_VERIFY_NONE, NULL);
         }
-        SSL_CTX_set_default_verify_paths(ctx);
 
         r = SSL_connect(ssl);
         if (r <= 0)
@@ -150,12 +157,9 @@ int dtls_connect(DTLSManager *m, SocketAddress *address) {
                         log_debug("DTLS: No certificates.");
         }
 
-        /* Set and activate timeouts */
-        BIO_ctrl(bio, BIO_CTRL_DGRAM_SET_RECV_TIMEOUT, 0, &timeout);
-
         m->ssl = TAKE_PTR(ssl);
-        m->ctx = ctx;
-        m->fd = fd;
+        m->fd = TAKE_FD(fd);
+        m->pretty_address = TAKE_PTR(pretty);
 
         m->connected = true;
         return 0;
@@ -173,6 +177,7 @@ void dtls_disconnect(DTLSManager *m) {
                 m->ssl = NULL;
         }
 
+        m->pretty_address = mfree(m->pretty_address);
         m->fd = safe_close(m->fd);
         m->connected = false;
 }
@@ -189,13 +194,24 @@ void dtls_manager_free(DTLSManager *m) {
 
 int dtls_manager_init(OpenSSLCertificateAuthMode auth_mode, DTLSManager **ret) {
         _cleanup_(dtls_manager_freep) DTLSManager *m = NULL;
+        _cleanup_(SSL_CTX_freep) SSL_CTX *ctx = NULL;
+
+        ctx = SSL_CTX_new(DTLS_method());
+        if (!ctx)
+                return log_error_errno(SYNTHETIC_ERRNO(ENOMEM),
+                                       "DTLS: Failed to allocate memory for SSL CTX: %m");
+
+        SSL_CTX_set_default_verify_paths(ctx);
+        SSL_CTX_set_verify_depth(ctx, VERIFICATION_DEPTH + 1);
 
         m = new(DTLSManager, 1);
         if (!m)
                 return log_oom();
 
         *m = (DTLSManager) {
            .auth_mode = auth_mode,
+           .ctx = TAKE_PTR(ctx),
+           .fd = -1,
         };
 
         *ret = TAKE_PTR(m);

src/netlog/netlog-dtls.h

@@ -2,7 +2,6 @@
 #pragma once
 
 #include <openssl/ssl.h>
-#include <openssl/bio.h>
 #include <stdbool.h>
 
 #include "socket-util.h"
@@ -13,9 +12,9 @@ typedef struct DTLSManager DTLSManager;
 
 struct DTLSManager {
         SSL_CTX *ctx;
-        BIO *bio;
         SSL *ssl;
 
+        char *pretty_address;
         int fd;
         bool connected;
 

src/netlog/netlog-manager.c

@@ -634,7 +634,7 @@ int manager_new(const char *state_file, const char *cursor, Manager **ret) {
                 .state_file = strdup(state_file),
                 .protocol = SYSLOG_TRANSMISSION_PROTOCOL_UDP,
                 .log_format = SYSLOG_TRANSMISSION_LOG_FORMAT_RFC_5424,
-                .auth_mode = OPEN_SSL_CERTIFICATE_AUTH_MODE_INVALID,
+                .auth_mode = OPEN_SSL_CERTIFICATE_AUTH_MODE_DENY,
                 .connection_retry_usec = DEFAULT_CONNECTION_RETRY_USEC,
                 .ratelimit = (const RateLimit) {
                         RATELIMIT_INTERVAL_USEC,

src/netlog/netlog-ssl.c

@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "netlog-ssl.h"
+
+#include "alloc-util.h"
+#include "openssl-util.h"
+#include "socket-util.h"
+
+#include "netlog-dtls.h"
+#include "netlog-tls.h"
+
+static_assert(offsetof(TLSManager, auth_mode) == offsetof(DTLSManager, auth_mode));
+
+/* inspired by SSL_set_verify(3) */
+int ssl_verify_certificate_validity(int preverify_ok, X509_STORE_CTX *store) {
+        const SSL* ssl = X509_STORE_CTX_get_ex_data(store, SSL_get_ex_data_X509_STORE_CTX_idx());
+        const char *pretty = (const char *) SSL_get_ex_data(ssl, EX_DATA_PRETTYADDRESS);
+        const TLSManager *m = (const TLSManager *) SSL_get_ex_data(ssl, EX_DATA_TLSMANAGER);
+        const X509 *error_cert = X509_STORE_CTX_get_current_cert(store);
+        int depth = X509_STORE_CTX_get_error_depth(store);
+        int error = X509_STORE_CTX_get_error(store);
+        char subject_buf[256];
+        char issuer_buf[256];
+        int log_level;
+
+        assert(store);
+        assert(pretty);
+        assert(m);
+
+        X509_NAME_oneline(X509_get_subject_name(error_cert), subject_buf, sizeof(subject_buf));
+        X509_NAME_oneline(X509_get_issuer_name(error_cert), issuer_buf, sizeof(issuer_buf));
+
+        log_debug("TLS: Verifying SSL certificates of server %s: certificate: subject='%s' issuer='%s' depth=%d preverify_ok=%d error=%d/%s ...",
+                  pretty, subject_buf, issuer_buf, depth, preverify_ok, error, X509_verify_cert_error_string(error));
+
+        if (depth > VERIFICATION_DEPTH) {
+                /*
+                 * From man:SSL_set_verif(3):
+                 *
+                 *   Catch a too long certificate chain. The depth limit set using
+                 *   SSL_CTX_set_verify_depth() is by purpose set to "limit+1" so
+                 *   that whenever the "depth>verify_depth" condition is met, we
+                 *   have violated the limit and want to log this error condition.
+                 *   We must do it here, because the CHAIN_TOO_LONG error would not
+                 *   be found explicitly; only errors introduced by cutting off the
+                 *   additional certificates would be logged.
+                 */
+                preverify_ok = 0;
+                error = X509_V_ERR_CERT_CHAIN_TOO_LONG;
+                X509_STORE_CTX_set_error(store, error);
+        }
+
+        if (preverify_ok) {
+                log_debug("TLS: Verified SSL certificate of server=%s (certificate: subject='%s' issuer='%s' depth=%d): %s",
+                          pretty, subject_buf, issuer_buf, depth, X509_verify_cert_error_string(error));
+                return preverify_ok;
+        }
+
+        switch (m->auth_mode) {
+                case OPEN_SSL_CERTIFICATE_AUTH_MODE_DENY:
+                        log_level = LOG_ERR;
+                        break;
+                case OPEN_SSL_CERTIFICATE_AUTH_MODE_WARN:
+                        log_level = LOG_WARNING;
+                        preverify_ok = 1;
+                        break;
+                case OPEN_SSL_CERTIFICATE_AUTH_MODE_ALLOW:
+                        log_level = LOG_DEBUG;
+                        preverify_ok = 1;
+                        break;
+                default:
+                        assert_not_reached("Invalid certificate authentication mode"); /* mode NO does not set this callback */
+        }
+
+        log_full(log_level, "TLS: Failed to verify certificate of server=%s (certificate: subject='%s' issuer='%s' depth=%d)%s: %s",
+                            pretty, subject_buf, issuer_buf, depth,
+                            preverify_ok ? ", ignoring" : "",
+                            X509_verify_cert_error_string(error));
+
+        return preverify_ok;
+}

src/netlog/netlog-ssl.h

@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+#include <openssl/ssl.h>
+
+#include "macro.h"
+
+#define VERIFICATION_DEPTH 20
+
+#define EX_DATA_TLSMANAGER 0
+#define EX_DATA_PRETTYADDRESS 1
+
+int ssl_verify_certificate_validity(int preverify_ok, X509_STORE_CTX *store);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(SSL_CTX*, SSL_CTX_free);