diff --git a/lib/logwriter.c b/lib/logwriter.c index 236b046837..146eace4bf 100644 --- a/lib/logwriter.c +++ b/lib/logwriter.c @@ -1741,6 +1741,12 @@ log_writer_steal_proto(LogWriter *self) return proto; } +LogProtoClient * +log_writer_get_proto(LogWriter *self) +{ + return self->proto; +} + /* run in the main thread in reaction to a log_writer_reopen to change * the destination LogProtoClient instance. It needs to be ran in the main diff --git a/lib/logwriter.h b/lib/logwriter.h index ee2da062b0..aac2bb82ef 100644 --- a/lib/logwriter.h +++ b/lib/logwriter.h @@ -86,6 +86,7 @@ gboolean log_writer_has_pending_writes(LogWriter *self); gboolean log_writer_opened(LogWriter *self); void log_writer_reopen(LogWriter *self, LogProtoClient *proto); LogProtoClient *log_writer_steal_proto(LogWriter *self); +LogProtoClient *log_writer_get_proto(LogWriter *self); void log_writer_set_queue(LogWriter *self, LogQueue *queue); LogQueue *log_writer_get_queue(LogWriter *s); LogWriter *log_writer_new(guint32 flags, GlobalConfig *cfg); diff --git a/lib/transport/transport-adapter.c b/lib/transport/transport-adapter.c index 7b1e472e39..3a70a84391 100644 --- a/lib/transport/transport-adapter.c +++ b/lib/transport/transport-adapter.c @@ -27,7 +27,7 @@ gssize log_transport_adapter_read_method(LogTransport *s, gpointer buf, gsize buflen, LogTransportAuxData *aux) { LogTransportAdapter *self = (LogTransportAdapter *) s; - LogTransport *transport = log_transport_stack_get_transport(s->stack, self->base_index); + LogTransport *transport = log_transport_stack_get_or_create_transport(s->stack, self->base_index); return log_transport_read(transport, buf, buflen, aux); } @@ -36,7 +36,7 @@ gssize log_transport_adapter_write_method(LogTransport *s, const gpointer buf, gsize count) { LogTransportAdapter *self = (LogTransportAdapter *) s; - LogTransport *transport = log_transport_stack_get_transport(s->stack, self->base_index); + LogTransport *transport = log_transport_stack_get_or_create_transport(s->stack, self->base_index); return log_transport_write(transport, buf, count); } @@ -45,7 +45,7 @@ gssize log_transport_adapter_writev_method(LogTransport *s, struct iovec *iov, gint iov_count) { LogTransportAdapter *self = (LogTransportAdapter *) s; - LogTransport *transport = log_transport_stack_get_transport(s->stack, self->base_index); + LogTransport *transport = log_transport_stack_get_or_create_transport(s->stack, self->base_index); return log_transport_writev(transport, iov, iov_count); } diff --git a/lib/transport/transport-stack.c b/lib/transport/transport-stack.c index 2c505b1916..0d22261c4a 100644 --- a/lib/transport/transport-stack.c +++ b/lib/transport/transport-stack.c @@ -59,7 +59,7 @@ log_transport_stack_switch(LogTransportStack *self, gint index) { g_assert(index < LOG_TRANSPORT__MAX); LogTransport *active_transport = log_transport_stack_get_active(self); - LogTransport *requested_transport = log_transport_stack_get_transport(self, index); + LogTransport *requested_transport = log_transport_stack_get_or_create_transport(self, index); if (!requested_transport) return FALSE; diff --git a/lib/transport/transport-stack.h b/lib/transport/transport-stack.h index a819ae065b..05c0aec421 100644 --- a/lib/transport/transport-stack.h +++ b/lib/transport/transport-stack.h @@ -108,7 +108,7 @@ struct _LogTransportStack }; static inline LogTransport * -log_transport_stack_get_transport(LogTransportStack *self, gint index) +log_transport_stack_get_or_create_transport(LogTransportStack *self, gint index) { g_assert(index < LOG_TRANSPORT__MAX); @@ -127,7 +127,16 @@ log_transport_stack_get_transport(LogTransportStack *self, gint index) static inline LogTransport * log_transport_stack_get_active(LogTransportStack *self) { - return log_transport_stack_get_transport(self, self->active_transport); + // TODO - Change it to log_transport_stack_get_transport after checking call sites + return log_transport_stack_get_or_create_transport(self, self->active_transport); +} + +static inline LogTransport * +log_transport_stack_get_transport(LogTransportStack *self, gint index) +{ + g_assert(index < LOG_TRANSPORT__MAX); + + return self->transports[index]; } void log_transport_stack_add_factory(LogTransportStack *self, LogTransportFactory *); diff --git a/lib/transport/transport-tls.c b/lib/transport/transport-tls.c index 53bebe8753..3a76151059 100644 --- a/lib/transport/transport-tls.c +++ b/lib/transport/transport-tls.c @@ -237,6 +237,14 @@ log_transport_tls_write_method(LogTransport *s, const gpointer buf, gsize buflen return -1; } +TLSSession * +log_tansport_tls_get_session(LogTransport *s) +{ + LogTransportTLS *self = (LogTransportTLS *)s; + + return self->tls_session; +} + static void log_transport_tls_free_method(LogTransport *s); diff --git a/lib/transport/transport-tls.h b/lib/transport/transport-tls.h index 682667e1ab..2a67765ab3 100644 --- a/lib/transport/transport-tls.h +++ b/lib/transport/transport-tls.h @@ -28,5 +28,6 @@ #include "transport/tls-context.h" LogTransport *log_transport_tls_new(TLSSession *tls_session, gint fd); +TLSSession *log_tansport_tls_get_session(LogTransport *s); #endif diff --git a/modules/afsocket/afinet-dest.c b/modules/afsocket/afinet-dest.c index 62016830e2..e2acf2f532 100644 --- a/modules/afsocket/afinet-dest.c +++ b/modules/afsocket/afinet-dest.c @@ -28,6 +28,8 @@ #include "gprocess.h" #include "compat/openssl_support.h" #include "afsocket-signals.h" +#include "transport/transport-tls.h" +#include "transport/transport-stack.h" #include #include @@ -228,6 +230,12 @@ afinet_dd_setup_tls_verifier(AFInetDestDriver *self) transport_mapper_inet_set_tls_verifier(transport_mapper_inet, verifier); } +static AFInetDestDriverTLSVerifyData * +_get_tls_verify_data (TLSVerifier *verifier) +{ + return (AFInetDestDriverTLSVerifyData *)verifier->verify_data; +} + void afinet_dd_enable_failover(LogDriver *s) { @@ -697,6 +705,31 @@ afinet_dd_free(LogPipe *s) afsocket_dd_free(s); } +static void +afinet_dd_update_tls_verifier(AFSocketDestDriver *s, ReloadStoreItem *rsi) +{ + AFInetDestDriver *self = (AFInetDestDriver *) s; + + LogWriter *writer = rsi->writer; + + if (!writer) + return; + + LogProtoClient *proto = log_writer_get_proto(writer); + + if (!proto) + return; + + LogTransport *transport = log_transport_stack_get_transport(&proto->transport_stack, LOG_TRANSPORT_TLS); + + if (transport) + { + TLSSession *session = log_tansport_tls_get_session(transport); + AFInetDestDriverTLSVerifyData *verify_data = _get_tls_verify_data (session->verifier); + verify_data->signal_connector = self->super.super.super.super.signal_slot_connector; + } +} + static AFInetDestDriver * afinet_dd_new_instance(TransportMapper *transport_mapper, gchar *hostname, GlobalConfig *cfg) { @@ -710,6 +743,7 @@ afinet_dd_new_instance(TransportMapper *transport_mapper, gchar *hostname, Globa self->super.construct_writer = afinet_dd_construct_writer; self->super.setup_addresses = afinet_dd_setup_addresses; self->super.get_dest_name = afinet_dd_get_dest_name; + self->super.on_connection_restore = afinet_dd_update_tls_verifier; self->primary = g_strdup(hostname); diff --git a/modules/afsocket/afsocket-dest.c b/modules/afsocket/afsocket-dest.c index 974179af42..49bff28acc 100644 --- a/modules/afsocket/afsocket-dest.c +++ b/modules/afsocket/afsocket-dest.c @@ -37,13 +37,6 @@ #include #include -typedef struct _ReloadStoreItem -{ - LogProtoClientFactory *proto_factory; - GSockAddr *dest_addr; - LogWriter *writer; -} ReloadStoreItem; - static ReloadStoreItem * _reload_store_item_new(AFSocketDestDriver *afsocket_dd) { @@ -453,6 +446,15 @@ afsocket_dd_setup_addresses_method(AFSocketDestDriver *self) return TRUE; } +static void +_on_connection_restore(AFSocketDestDriver *self, ReloadStoreItem *item) +{ + if (self->on_connection_restore) + { + self->on_connection_restore(self, item); + } +} + static gboolean _afsocket_dd_try_to_restore_connection_state(AFSocketDestDriver *self) { @@ -470,7 +472,10 @@ _afsocket_dd_try_to_restore_connection_state(AFSocketDestDriver *self) return FALSE; if (_is_protocol_compatible_with_writer_after_reload(self, item)) - self->writer = _reload_store_item_release_writer(item); + { + _on_connection_restore(self, item); + self->writer = _reload_store_item_release_writer(item); + } self->dest_addr = g_sockaddr_ref(item->dest_addr); _reload_store_item_free(item); diff --git a/modules/afsocket/afsocket-dest.h b/modules/afsocket/afsocket-dest.h index 83e9997a90..bf061306d4 100644 --- a/modules/afsocket/afsocket-dest.h +++ b/modules/afsocket/afsocket-dest.h @@ -32,6 +32,13 @@ #include +typedef struct _ReloadStoreItem +{ + LogProtoClientFactory *proto_factory; + GSockAddr *dest_addr; + LogWriter *writer; +} ReloadStoreItem; + typedef struct _AFSocketDestDriver AFSocketDestDriver; struct _AFSocketDestDriver @@ -62,6 +69,7 @@ struct _AFSocketDestDriver LogWriter *(*construct_writer)(AFSocketDestDriver *self); gboolean (*setup_addresses)(AFSocketDestDriver *s); const gchar *(*get_dest_name)(const AFSocketDestDriver *s); + void (*on_connection_restore)(AFSocketDestDriver *s, ReloadStoreItem *ri); }; static inline LogWriter * diff --git a/news/bugfix-418.md b/news/bugfix-418.md new file mode 100644 index 0000000000..f243bcab62 --- /dev/null +++ b/news/bugfix-418.md @@ -0,0 +1,4 @@ +`network(), syslog()`: Fixed a potential crash for TLS destinations during reload + +In case of a TLS connection, if the handshake didn't happen before reloading AxoSyslog, +it crashed on the first message sent to that destination. \ No newline at end of file diff --git a/tests/copyright/policy b/tests/copyright/policy index caeb3e1ebf..100aab0bb7 100644 --- a/tests/copyright/policy +++ b/tests/copyright/policy @@ -277,6 +277,7 @@ tests/light/functional_tests/source_drivers/syslog_source/auto/test_auto_proto\. tests/light/src/syslog_ng_ctl/prometheus_stats_handler.py tests/light/src/syslog_ng_config/statements/template/template\.py tests/light/src/syslog_ng_config/statements/__init__\.py +tests/light/functional_tests/destination_drivers/network_destination/test_kept_alive_tls_connection_doing_handshake_after_reload\.py modules/correlation/id-counter\.[ch]$ modules/correlation/group-lines.h modules/xml/windows-eventlog-xml-parser\.h diff --git a/tests/light/functional_tests/destination_drivers/network_destination/test_kept_alive_tls_connection_doing_handshake_after_reload.py b/tests/light/functional_tests/destination_drivers/network_destination/test_kept_alive_tls_connection_doing_handshake_after_reload.py new file mode 100644 index 0000000000..f88f6672d4 --- /dev/null +++ b/tests/light/functional_tests/destination_drivers/network_destination/test_kept_alive_tls_connection_doing_handshake_after_reload.py @@ -0,0 +1,47 @@ +#!/usr/bin/env python +############################################################################# +# Copyright (c) 2024 Axoflow +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License version 2 as published +# by the Free Software Foundation, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +# +# As an additional exemption you are allowed to compile & link against the +# OpenSSL libraries as published by the OpenSSL project. See the file +# COPYING for details. +# +############################################################################# +from src.common.file import copy_shared_file + + +def test_kept_alive_tls_connection_doing_handshake_after_reload(config, syslog_ng, port_allocator, testcase_parameters): + ca = copy_shared_file(testcase_parameters, "valid-ca.crt") + + network_source = config.create_network_source(port=port_allocator()) + tls_network_destination = config.create_network_destination( + ip="localhost", port=port_allocator(), transport="tls", + keep_alive="yes", + tls={ + "ca-file": ca, + "peer-verify": "yes", + }, + ) + + config.create_logpath(statements=[network_source, tls_network_destination]) + + tls_network_destination.start_listener() + syslog_ng.start(config) + + syslog_ng.reload(config) + network_source.write_log("test msg") + + assert tls_network_destination.read_until_logs(["test msg"]) diff --git a/tests/light/shared_files/valid-ca.crt b/tests/light/shared_files/valid-ca.crt new file mode 100644 index 0000000000..7511001b28 --- /dev/null +++ b/tests/light/shared_files/valid-ca.crt @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIBrzCCAWGgAwIBAgIUcnaPg4elxLxduWuz285K04ySXm8wBQYDK2VwMEwxCzAJ +BgNVBAYTAkhVMQswCQYDVQQIDAJCUDERMA8GA1UEBwwIQnVkYXBlc3QxEDAOBgNV +BAoMB0F4b2Zsb3cxCzAJBgNVBAMMAkNBMCAXDTI0MTIxNDEyMDgxMVoYDzIxMjQx +MTIwMTIwODExWjBMMQswCQYDVQQGEwJIVTELMAkGA1UECAwCQlAxETAPBgNVBAcM +CEJ1ZGFwZXN0MRAwDgYDVQQKDAdBeG9mbG93MQswCQYDVQQDDAJDQTAqMAUGAytl +cAMhABy7FT1AbxGnqrainAD583ToCDPgewE9KEhcOyoOjx+fo1MwUTAdBgNVHQ4E +FgQUvb+QlzW+T0PrjRBlQ8xFaqzdFpYwHwYDVR0jBBgwFoAUvb+QlzW+T0PrjRBl +Q8xFaqzdFpYwDwYDVR0TAQH/BAUwAwEB/zAFBgMrZXADQQA6zlO5N4zE1EveBX8p +0qt1pszyettEDG6SOMGQsZmo0DvBo/d8IUN0pvmupP8ODCbRtBm2BFYNpwkaMnv5 +iHkH +-----END CERTIFICATE----- diff --git a/tests/light/shared_files/valid-ca.key b/tests/light/shared_files/valid-ca.key new file mode 100644 index 0000000000..c46257b0e1 --- /dev/null +++ b/tests/light/shared_files/valid-ca.key @@ -0,0 +1,3 @@ +-----BEGIN PRIVATE KEY----- +MC4CAQAwBQYDK2VwBCIEIIJdHC0MTNkrdOUeEMsbdsjB8XFE18fsTW85Gi79tdfy +-----END PRIVATE KEY----- diff --git a/tests/light/shared_files/valid-localhost.crt b/tests/light/shared_files/valid-localhost.crt new file mode 100644 index 0000000000..b8a3d868cb --- /dev/null +++ b/tests/light/shared_files/valid-localhost.crt @@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBXDCCAQ4CFHPirqN81POlHqbX64hq/v7yULDPMAUGAytlcDBMMQswCQYDVQQG +EwJIVTELMAkGA1UECAwCQlAxETAPBgNVBAcMCEJ1ZGFwZXN0MRAwDgYDVQQKDAdB +eG9mbG93MQswCQYDVQQDDAJDQTAgFw0yNDEyMTQxMjA5NDNaGA8yMTI0MTEyMDEy +MDk0M1owUzELMAkGA1UEBhMCSFUxCzAJBgNVBAgMAkJQMREwDwYDVQQHDAhCdWRh +cGVzdDEQMA4GA1UECgwHQXhvZmxvdzESMBAGA1UEAwwJbG9jYWxob3N0MCowBQYD +K2VwAyEAiDEFvid9kcpBbPgxCBaVWuj8YJZ3qZB1nBdsBuBMwEkwBQYDK2VwA0EA +vuyKnc/DhVcZgn1BInCRMMDU/15kpgwoI6y36IO3ftCUmSVR79zn//YiotaeSBf9 +2HRhksjez10aoLLJvDErDg== +-----END CERTIFICATE----- diff --git a/tests/light/shared_files/valid-localhost.key b/tests/light/shared_files/valid-localhost.key new file mode 100644 index 0000000000..5b6e1e24bb --- /dev/null +++ b/tests/light/shared_files/valid-localhost.key @@ -0,0 +1,3 @@ +-----BEGIN PRIVATE KEY----- +MC4CAQAwBQYDK2VwBCIEIGfwI2Ogf0umSj1p5yOosbwOnDqhOOP9rolfsky9gytI +-----END PRIVATE KEY----- diff --git a/tests/light/src/common/file.py b/tests/light/src/common/file.py index 51c3c74a80..edf1386e85 100644 --- a/tests/light/src/common/file.py +++ b/tests/light/src/common/file.py @@ -45,6 +45,11 @@ def copy_shared_file(testcase_parameters, shared_file_name): return Path(Path.cwd(), shared_file_name) +def get_shared_file(shared_file_name): + absolute_framework_dir = Path(__file__).parents[2].resolve() + return absolute_framework_dir / "shared_files" / shared_file_name + + def delete_session_file(shared_file_name): shared_file_name = Path(shared_file_name) shared_file_name.unlink() diff --git a/tests/light/src/common/network.py b/tests/light/src/common/network.py index 9fd5e4a51e..93e9edddb1 100644 --- a/tests/light/src/common/network.py +++ b/tests/light/src/common/network.py @@ -48,7 +48,10 @@ async def stop(self): async def close_client(self): """After a client connection is accepted, new connections will be rejected until this method is called""" - await self._client.close() + try: + await self._client.close() + except ConnectionResetError: # remove this after fixing two way shutdown (SSL_shutdown) in AxoSyslog + logger.warning("Client closed the connection prematurely") async def _create_client(self): return self.Client() diff --git a/tests/light/src/driver_io/network/network_io.py b/tests/light/src/driver_io/network/network_io.py index 5a27c4941a..e455045f4d 100644 --- a/tests/light/src/driver_io/network/network_io.py +++ b/tests/light/src/driver_io/network/network_io.py @@ -21,6 +21,7 @@ # ############################################################################# import socket +import ssl from enum import Enum from enum import IntEnum from pathlib import Path @@ -28,6 +29,7 @@ from src.common.asynchronous import BackgroundEventLoop from src.common.blocking import DEFAULT_TIMEOUT from src.common.file import File +from src.common.file import get_shared_file from src.common.network import SingleConnectionTCPServer from src.common.network import UDPServer from src.common.random_id import get_unique_id @@ -84,8 +86,12 @@ def construct(self, port, host=None, ip_proto_version=None): def _construct(server, reader_class): return reader_class(server), server + tls = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + tls.load_cert_chain(get_shared_file("valid-localhost.crt"), get_shared_file("valid-localhost.key")) + transport_mapping = { NetworkIO.Transport.TCP: lambda: _construct(SingleConnectionTCPServer(port, host, ip_proto_version), message_readers.SingleLineStreamReader), + NetworkIO.Transport.TLS: lambda: _construct(SingleConnectionTCPServer(port, host, ip_proto_version, tls), message_readers.SingleLineStreamReader), NetworkIO.Transport.UDP: lambda: _construct(UDPServer(port, host, ip_proto_version), message_readers.DatagramReader), } return transport_mapping[self]() diff --git a/tests/light/src/syslog_ng_config/statements/destinations/network_destination.py b/tests/light/src/syslog_ng_config/statements/destinations/network_destination.py index 1cb8c53a09..be16e80274 100644 --- a/tests/light/src/syslog_ng_config/statements/destinations/network_destination.py +++ b/tests/light/src/syslog_ng_config/statements/destinations/network_destination.py @@ -28,6 +28,7 @@ def map_transport(transport): mapping = { "tcp": NetworkIO.Transport.TCP, "udp": NetworkIO.Transport.UDP, + "tls": NetworkIO.Transport.TLS, } transport = transport.replace("_", "-").replace("'", "").replace('"', "").lower()