From 148ae663cf5b1499a521a03e2fb20042d2f71ec2 Mon Sep 17 00:00:00 2001 From: Matzman Date: Tue, 6 Oct 2026 16:29:32 +0200 Subject: [PATCH] TLS for the MQTT connection (iteration 2, step 5) ca_file or the system's CAs (MOSQ_OPT_TLS_USE_OS_CERTS), client certificate, broker certificate and host name verified, settings errors reported before connecting. The client thread blocks SIGPIPE: with TLS, a write to a connection the broker closed killed the process. The reason of a failed handshake (libmosquitto's log) is appended to the error. tst_mqtt covers TLS with test certificates in tests/data (D44). Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- docs/DESIGN.md | 25 ++++-- docs/PROGRESS.md | 16 +++- src/mqtt/MosquittoClient.cpp | 73 ++++++++++++++++-- src/mqtt/MosquittoClient.h | 6 ++ tests/CMakeLists.txt | 2 + tests/data/README.md | 29 +++++++ tests/data/ca.crt | 11 +++ tests/data/client.crt | 11 +++ tests/data/client.key | 5 ++ tests/data/other-ca.crt | 11 +++ tests/data/server.crt | 11 +++ tests/data/server.key | 5 ++ tests/tst_mqtt.cpp | 145 ++++++++++++++++++++++++++++++++++- 14 files changed, 336 insertions(+), 16 deletions(-) create mode 100644 tests/data/README.md create mode 100644 tests/data/ca.crt create mode 100644 tests/data/client.crt create mode 100644 tests/data/client.key create mode 100644 tests/data/other-ca.crt create mode 100644 tests/data/server.crt create mode 100644 tests/data/server.key diff --git a/README.md b/README.md index 42c938a..fbe71e6 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ FrameAssistant reads its settings from `~/.config/frameassistant/config.json` | `mqtt.username`, `mqtt.password` | no | no login | | | `mqtt.tls.enabled` | no | `false` | | | `mqtt.tls.ca_file` | no | system certificates | CA of the broker's certificate | -| `mqtt.tls.cert_file`, `mqtt.tls.key_file` | no | | Client certificate | +| `mqtt.tls.cert_file`, `mqtt.tls.key_file` | no | | Client certificate (needs `ca_file`) | | `homeassistant.discovery_prefix` | no | `homeassistant` | | | `homeassistant.url` | no | | Home Assistant's address; reserved for the dashboard, not used yet | | `notifications.timeout_s` | no | 10 | How long notifications stay | diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 39e1232..e377188 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -315,11 +315,19 @@ MQTT (D17). - **Normal exit:** `disconnectFromBroker(timeout)` publishes the will payload itself, waits up to the timeout for all acknowledgements (`waitForPublished()`), sends DISCONNECT and joins the thread. After that no callback can arrive. -- **TLS:** `mosquitto_tls_set()` with `ca_file`; with an empty `ca_file`, - `MOSQ_OPT_TLS_USE_OS_CERTS` (libmosquitto ≥ 2.0). Client certificate if configured. - (Step 5; until then a TLS setting is reported as an error.) -- **Errors** are reported in English regardless of the locale (`strerrordesc_np()`). A DNS - failure has no reason: libmosquitto overwrites `getaddrinfo()`'s code in `errno` before it +- **TLS:** `mosquitto_tls_set()` with `ca_file` (and the client certificate, if configured); + the broker's certificate and host name are verified. With an empty `ca_file`, only + `MOSQ_OPT_TLS_USE_OS_CERTS`: libmosquitto's `tls_set()` insists on a CA file or directory, + so this option alone enables TLS, and **a client certificate needs `ca_file`**. Unreadable + files and a certificate without key are reported before connecting. +- **SIGPIPE:** with TLS, OpenSSL writes with `write()`, not `send(MSG_NOSIGNAL)`, so writing + to a connection the broker closed raised SIGPIPE and ended the process (found by + `tst_mqtt`). The client thread blocks SIGPIPE (`pthread_sigmask`); only it writes to the + socket. +- **Errors** are reported in English regardless of the locale (`strerrordesc_np()`). A failed + TLS handshake returns only "TLS error" (2.0) or "Protocol error" (2.1); libmosquitto's + error log line of the attempt is appended, e.g. "Protocol error: certificate verify + failed". A DNS failure has no reason: libmosquitto overwrites `getaddrinfo()`'s code in `errno` before it returns. The log line "connecting to host:port" names the host. - **Logging:** every connection state change and error (`qInfo`/`qWarning`, so also in the log file); repeated failures with the same reason are reported once. Retries after a failure @@ -401,12 +409,13 @@ delete). |---|---| | `tst_haentity` | discovery component (optional keys only when set), topics, throttling (several values within an interval → one state with the newest value; unchanged or reverted values not emitted), first value → state then available, cleared → unavailable | | `tst_hadevice` (`tests/FakeMqttClient.h`) | topics, discovery prefix, last will, discovery payload, what is published after a connect and in which order, entity without value unavailable, changes while connected, nothing buffered and current state after reconnect, birth message (delayed; cancelled by a disconnect; other messages ignored), `removeFromHa()` | -| `tst_mqtt` (integration; real `mosquitto` on a free port, QSKIP without it) | connect and messages; retained message for a later subscriber; nothing published while disconnected; first connect retried until the broker runs; reconnect after a broker restart; unresolvable host; last will after `kill -9` of a helper process (the test binary itself, `--will-helper`); `disconnectFromBroker()` publishes the will; login right and wrong; TLS with a test CA (step 5) | +| `tst_mqtt` (integration; real `mosquitto` on a free port, QSKIP without it) | connect and messages; retained message for a later subscriber; nothing published while disconnected; first connect retried until the broker runs; reconnect after a broker restart; unresolvable host; last will after `kill -9` of a helper process (the test binary itself, `--will-helper`); `disconnectFromBroker()` publishes the will; login right and wrong; TLS: connect with the test CA, wrong CA rejected with reason, system CAs (`ca_file` empty), retry after a TLS error (the case that ends libmosquitto's own loop), client certificate required (with/without), settings errors | | `tst_surfaces` | `--desktop-novr`; app command: hook called, no surface created, exit code returned | | Manual | from the PC against the user's HA: device appears, online/offline, broker restart, two devices, `--ha-remove`, UI-1 in `--desktop`. On the Frame: the package with libmosquitto runs | -- TLS test certificates (CA, server certificate and key, 100-year validity) are committed to - `tests/data/` (D44). +- TLS test certificates (CA, server and client certificates with keys, a second CA; EC P-256, + 100-year validity) are committed to `tests/data/` (D44), with the commands that made them + in `tests/data/README.md`. ### Implementation order diff --git a/docs/PROGRESS.md b/docs/PROGRESS.md index e4c29b9..1c46086 100644 --- a/docs/PROGRESS.md +++ b/docs/PROGRESS.md @@ -25,7 +25,7 @@ CLAUDE.md ("Working rules"). - **Implementation** follows the order in DESIGN.md (steps 1–7). Steps 1 (libmosquitto in CMake and the containers), 2 (`MqttClient`, `MosquittoClient`, `tst_mqtt`) 3 (`HaEntity`, `HaSensor`, `HaDevice`, unit tests) and 4 (app wiring, `--desktop-novr`, - UI-1) done; next: step 5 (TLS). + UI-1) and 5 (TLS) done; next: step 6 (app commands, `--ha-remove`, `uninstall.sh`). ## Next steps @@ -164,6 +164,20 @@ Known limitation: the bundled-Qt package does not support WebEngine in this iter `libmosquitto.so.1`, at most `GLIBC_2.34`, private Qt symbols only `QVulkan*`. - Not checked yet: the graceful exit through `aboutToQuit` in the app (only in `tst_mqtt`); it needs a real window close or "Quit app" in SteamVR (manual test). +- Step 5: TLS in `MosquittoClient` (`ca_file` or the system's CAs, client certificate, + checks before connecting); test certificates in `tests/data/` (D44); 9 TLS cases in + `tst_mqtt`. Findings: + - **SIGPIPE crash:** with TLS, a write to a connection the broker closed killed the + process (OpenSSL uses `write()`). Fixed by blocking SIGPIPE in the client thread. + - libmosquitto's `tls_set()` needs a CA file or directory, so with system CAs only + `MOSQ_OPT_TLS_USE_OS_CERTS` is set, and a client certificate needs `ca_file` + (documented, reported as a settings error). + - A failed handshake returns only "TLS error"/"Protocol error"; the reason is in + libmosquitto's log, which is now appended. + - Checked: host 3 × 21/21; `container-test.sh` 12/12 (libmosquitto 2.0.21), no Vulkan + validation messages. + - Still open: which CA store the Frame's OpenSSL uses by default (for `ca_file` empty); + check on the device. ### 2026-10-05: requirements approved diff --git a/src/mqtt/MosquittoClient.cpp b/src/mqtt/MosquittoClient.cpp index ad534e5..07e3b44 100644 --- a/src/mqtt/MosquittoClient.cpp +++ b/src/mqtt/MosquittoClient.cpp @@ -1,8 +1,10 @@ #include "mqtt/MosquittoClient.h" #include +#include #include +#include #include #include #include @@ -61,6 +63,7 @@ MosquittoClient::MosquittoClient(Settings settings, QObject* parent) mosquitto_disconnect_callback_set(m_mosq, &MosquittoClient::onDisconnect); mosquitto_publish_callback_set(m_mosq, &MosquittoClient::onPublish); mosquitto_message_callback_set(m_mosq, &MosquittoClient::onMessage); + mosquitto_log_callback_set(m_mosq, &MosquittoClient::onLog); const MqttConfig& broker = m_settings.broker; int rc = MOSQ_ERR_SUCCESS; @@ -76,9 +79,43 @@ MosquittoClient::MosquittoClient(Settings settings, QObject* parent) int(will.payload.size()), will.payload.constData(), 1, will.retain); } - if (rc == MOSQ_ERR_SUCCESS && broker.tls.enabled) - m_initError = QStringLiteral("TLS is not supported yet"); - if (rc != MOSQ_ERR_SUCCESS) m_initError = errorText(rc, errno); + if (rc != MOSQ_ERR_SUCCESS) + m_initError = errorText(rc, errno); + else if (broker.tls.enabled) + m_initError = setUpTls(); +} + +QString MosquittoClient::setUpTls() { + const MqttTlsConfig& tls = m_settings.broker.tls; + for (const QString& file : {tls.caFile, tls.certFile, tls.keyFile}) { + if (!file.isEmpty() && !QFileInfo(file).isReadable()) + return QStringLiteral("TLS: cannot read %1").arg(file); + } + if (tls.certFile.isEmpty() != tls.keyFile.isEmpty()) + return QStringLiteral("TLS: cert_file and key_file go together"); + if (tls.caFile.isEmpty()) { + // The system's CA certificates. mosquitto_tls_set() insists on a CA + // file or directory, so this option alone enables TLS, and a client + // certificate can't be set without ca_file. + if (!tls.certFile.isEmpty()) + return QStringLiteral("TLS: a client certificate needs ca_file"); + const int rc = mosquitto_int_option(m_mosq, MOSQ_OPT_TLS_USE_OS_CERTS, 1); + return rc == MOSQ_ERR_SUCCESS + ? QString() + : QStringLiteral("TLS: %1").arg(errorText(rc, errno)); + } + const QByteArray ca = QFile::encodeName(tls.caFile); + const QByteArray cert = QFile::encodeName(tls.certFile); + const QByteArray key = QFile::encodeName(tls.keyFile); + // Verifies the broker's certificate and host name (libmosquitto's + // default). + const int rc = + mosquitto_tls_set(m_mosq, ca.constData(), nullptr, + cert.isEmpty() ? nullptr : cert.constData(), + key.isEmpty() ? nullptr : key.constData(), nullptr); + return rc == MOSQ_ERR_SUCCESS + ? QString() + : QStringLiteral("TLS: %1").arg(errorText(rc, errno)); } MosquittoClient::~MosquittoClient() { @@ -150,10 +187,20 @@ bool MosquittoClient::waitForPublished(int timeoutMs) { } void MosquittoClient::run() { + // With TLS, OpenSSL writes with write(), not send(MSG_NOSIGNAL): writing + // to a connection the broker closed raises SIGPIPE, which would end the + // whole process. Blocked in this thread, the write fails with EPIPE + // instead. Only this thread writes to the socket (threaded mode). + sigset_t pipe; + sigemptyset(&pipe); + sigaddset(&pipe, SIGPIPE); + pthread_sigmask(SIG_BLOCK, &pipe, nullptr); + const QByteArray host = m_settings.broker.host.toUtf8(); int delayMs = m_settings.reconnectMinMs; while (!m_stop) { m_lastError.clear(); + m_lastLogError.clear(); // Blocks for the DNS lookup; the TCP connect itself completes in the // loop below. int rc = @@ -174,8 +221,13 @@ void MosquittoClient::run() { if (m_stop) break; // A connection that came up starts the backoff again. if (m_state == State::Connected) delayMs = m_settings.reconnectMinMs; - setState(State::Disconnected, - m_lastError.isEmpty() ? errorText(rc, err) : m_lastError); + QString error = m_lastError.isEmpty() ? errorText(rc, err) : m_lastError; + // libmosquitto logs the reason of a failed TLS handshake (e.g. + // "certificate verify failed"), but returns only "TLS error" (2.0) or + // "Protocol error" (2.1). + if (!m_lastLogError.isEmpty()) + error += QStringLiteral(": ") + m_lastLogError; + setState(State::Disconnected, error); if (!sleepUnlessStopped(delayMs)) break; delayMs = std::min(delayMs * 2, m_settings.reconnectMaxMs); } @@ -253,3 +305,14 @@ void MosquittoClient::onMessage(mosquitto*, void* self, QByteArray(static_cast(message->payload), message->payloadlen)); } + +void MosquittoClient::onLog(mosquitto*, void* self, int level, + const char* text) { + if (!(level & MOSQ_LOG_ERR)) return; + // E.g. "OpenSSL Error[0]: error:0A000086:SSL routines::certificate verify + // failed": keep the reason at the end. + QString line = QString::fromUtf8(text); + const qsizetype colon = line.lastIndexOf(QLatin1Char(':')); + if (colon >= 0) line = line.mid(colon + 1).trimmed(); + static_cast(self)->m_lastLogError = line; +} diff --git a/src/mqtt/MosquittoClient.h b/src/mqtt/MosquittoClient.h index 2eb09e1..a51d2d8 100644 --- a/src/mqtt/MosquittoClient.h +++ b/src/mqtt/MosquittoClient.h @@ -44,6 +44,8 @@ class MosquittoClient : public MqttClient { State state() const override { return m_state; } private: + // Sets up TLS from the settings; returns an error text or nothing. + QString setUpTls(); // Thread body: connect, loop, back off, until m_stop. void run(); // Sleeps up to `ms`; returns early (false) when stopping. @@ -58,6 +60,7 @@ class MosquittoClient : public MqttClient { static void onPublish(mosquitto* mosq, void* self, int mid); static void onMessage(mosquitto* mosq, void* self, const mosquitto_message* message); + static void onLog(mosquitto* mosq, void* self, int level, const char* text); const Settings m_settings; mosquitto* m_mosq = nullptr; @@ -67,6 +70,9 @@ class MosquittoClient : public MqttClient { std::atomic m_stop{false}; // Why the last attempt failed (our thread only). QString m_lastError; + // libmosquitto's last error log line of this attempt (our thread only): + // the details of a TLS error. + QString m_lastLogError; // The error last reported by setState(), to report repeats only once. QString m_reportedError; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 165f5e3..c46c3c8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -55,4 +55,6 @@ add_test(NAME hadevice COMMAND tst_hadevice) # MosquittoClient against a local mosquitto broker (D30 level 2); skips without one. qt_add_executable(tst_mqtt tst_mqtt.cpp) target_link_libraries(tst_mqtt PRIVATE frameassistant_lib Qt6::Test) +# TLS test certificates (D44). +target_compile_definitions(tst_mqtt PRIVATE TEST_DATA_DIR="${CMAKE_CURRENT_SOURCE_DIR}/data") add_test(NAME mqtt COMMAND tst_mqtt) diff --git a/tests/data/README.md b/tests/data/README.md new file mode 100644 index 0000000..3ff8eb9 --- /dev/null +++ b/tests/data/README.md @@ -0,0 +1,29 @@ +# Test certificates + +For the TLS cases in `tst_mqtt` (docs/DESIGN.md, D44). EC P-256, valid until 2126. Test use +only: the private keys are public. + +| File | What | +|---|---| +| `ca.crt` | Test CA (its key was deleted after signing) | +| `server.crt`, `server.key` | Broker certificate from the test CA, for `localhost` and `127.0.0.1` | +| `client.crt`, `client.key` | Client certificate from the test CA | +| `other-ca.crt` | A second CA that signed nothing here: the wrong CA | + +Made with: + +```sh +openssl req -x509 -newkey ec -pkeyopt ec_paramgen_curve:prime256v1 -nodes \ + -keyout ca.key -out ca.crt -days 36500 -subj "/CN=FrameAssistant test CA" +openssl req -newkey ec -pkeyopt ec_paramgen_curve:prime256v1 -nodes \ + -keyout server.key -out server.csr -subj "/CN=localhost" +openssl x509 -req -in server.csr -CA ca.crt -CAkey ca.key -CAcreateserial -out server.crt \ + -days 36500 -extfile <(printf "subjectAltName=DNS:localhost,IP:127.0.0.1\nextendedKeyUsage=serverAuth") +openssl req -newkey ec -pkeyopt ec_paramgen_curve:prime256v1 -nodes \ + -keyout client.key -out client.csr -subj "/CN=frameassistant-test-client" +openssl x509 -req -in client.csr -CA ca.crt -CAkey ca.key -CAcreateserial -out client.crt \ + -days 36500 -extfile <(printf "extendedKeyUsage=clientAuth") +openssl req -x509 -newkey ec -pkeyopt ec_paramgen_curve:prime256v1 -nodes \ + -keyout other-ca.key -out other-ca.crt -days 36500 -subj "/CN=Other test CA" +rm server.csr client.csr ca.srl ca.key other-ca.key +``` diff --git a/tests/data/ca.crt b/tests/data/ca.crt new file mode 100644 index 0000000..777a224 --- /dev/null +++ b/tests/data/ca.crt @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBmTCCAT+gAwIBAgIUDYqH6JFwdNkUywBv5SyHxuuFc3owCgYIKoZIzj0EAwIw +ITEfMB0GA1UEAwwWRnJhbWVBc3Npc3RhbnQgdGVzdCBDQTAgFw0yNjEwMDYxNDE4 +NDBaGA8yMTI2MDkxMjE0MTg0MFowITEfMB0GA1UEAwwWRnJhbWVBc3Npc3RhbnQg +dGVzdCBDQTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABEYTVW0Vs72geJmDYYTO +qCxisH73KxOpqQIFd8dZguzP3Ymu64T1xi6/7BbnPmtvKDP3KFDb7EzHTFQ9l32s +qWajUzBRMB0GA1UdDgQWBBRLda21NY4WNcdtMQUU20dpFjzLxTAfBgNVHSMEGDAW +gBRLda21NY4WNcdtMQUU20dpFjzLxTAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49 +BAMCA0gAMEUCIQCcWQf7cejgQw+xzZ/oDgUVSQ2s73wZ/N+b/XxD2goUswIgS/Y5 +HeqcBO4HHqOVLbwRMhCWYbQlAhiLpaX26wyzIyM= +-----END CERTIFICATE----- diff --git a/tests/data/client.crt b/tests/data/client.crt new file mode 100644 index 0000000..7c22fca --- /dev/null +++ b/tests/data/client.crt @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBojCCAUegAwIBAgIUMl5Z4QsUdsD5cj2ZNcEWB1L4awAwCgYIKoZIzj0EAwIw +ITEfMB0GA1UEAwwWRnJhbWVBc3Npc3RhbnQgdGVzdCBDQTAgFw0yNjEwMDYxNDE4 +NDBaGA8yMTI2MDkxMjE0MTg0MFowJTEjMCEGA1UEAwwaZnJhbWVhc3Npc3RhbnQt +dGVzdC1jbGllbnQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQXzJWn1Vn4oghl +m077IKOEEX3VBFDMP7IGVBR6V19oHGgCbth9gEQXqPMr+8LYtK3UWYhk/VuFVP3X +abqlipIpo1cwVTATBgNVHSUEDDAKBggrBgEFBQcDAjAdBgNVHQ4EFgQUQjEJ7EHL +s4y78P14VB3keg8WCFIwHwYDVR0jBBgwFoAUS3WttTWOFjXHbTEFFNtHaRY8y8Uw +CgYIKoZIzj0EAwIDSQAwRgIhAJH+jXs/7SovwLZfT0BhlteFfCyhXuz6e39FbRjk +dkGmAiEA2IpZZlW+T3r71sHE+CJe8RpHdjDPjmij2nctps8Pdm4= +-----END CERTIFICATE----- diff --git a/tests/data/client.key b/tests/data/client.key new file mode 100644 index 0000000..7aa899c --- /dev/null +++ b/tests/data/client.key @@ -0,0 +1,5 @@ +-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQggKrDK1CB3lOzi23P +XkvdV45GpZG4YMrAH0BVaB24liGhRANCAAQXzJWn1Vn4oghlm077IKOEEX3VBFDM +P7IGVBR6V19oHGgCbth9gEQXqPMr+8LYtK3UWYhk/VuFVP3XabqlipIp +-----END PRIVATE KEY----- diff --git a/tests/data/other-ca.crt b/tests/data/other-ca.crt new file mode 100644 index 0000000..1939a60 --- /dev/null +++ b/tests/data/other-ca.crt @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBhzCCAS2gAwIBAgIUD73YYi99elTmyJ3drpvRqNus3McwCgYIKoZIzj0EAwIw +GDEWMBQGA1UEAwwNT3RoZXIgdGVzdCBDQTAgFw0yNjEwMDYxNDE4NDBaGA8yMTI2 +MDkxMjE0MTg0MFowGDEWMBQGA1UEAwwNT3RoZXIgdGVzdCBDQTBZMBMGByqGSM49 +AgEGCCqGSM49AwEHA0IABP6HT0Ey1ohM72ykQVIiNpd215blibaafbhlNbyyU/uD +4BCCuEWKkDF1wQwrLH/O+N8gjVUpymd8/dK9zGzjGPKjUzBRMB0GA1UdDgQWBBTB +dmiTSLe4jI9TKsp/+fFk/GoXWDAfBgNVHSMEGDAWgBTBdmiTSLe4jI9TKsp/+fFk +/GoXWDAPBgNVHRMBAf8EBTADAQH/MAoGCCqGSM49BAMCA0gAMEUCIQCZzxaVM3+1 +OcS0fovnWedcs2s4QRAQ/PGzr1BGt5wxVwIgGxzzOfsh7yz8QveA0oUkPkOJLsOq +K739i67tvYbRNl4= +-----END CERTIFICATE----- diff --git a/tests/data/server.crt b/tests/data/server.crt new file mode 100644 index 0000000..3d0de11 --- /dev/null +++ b/tests/data/server.crt @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBrDCCAVKgAwIBAgIUMl5Z4QsUdsD5cj2ZNcEWB1L4av8wCgYIKoZIzj0EAwIw +ITEfMB0GA1UEAwwWRnJhbWVBc3Npc3RhbnQgdGVzdCBDQTAgFw0yNjEwMDYxNDE4 +NDBaGA8yMTI2MDkxMjE0MTg0MFowFDESMBAGA1UEAwwJbG9jYWxob3N0MFkwEwYH +KoZIzj0CAQYIKoZIzj0DAQcDQgAEkIJRd0MoLtpP0X755ZbjYvV0Vp2rDECQZyC6 +4bbwufvk2vm1+JUBusnnnSIfqRU5S3wH6pzSyWxZiuc1TXscrqNzMHEwGgYDVR0R +BBMwEYIJbG9jYWxob3N0hwR/AAABMBMGA1UdJQQMMAoGCCsGAQUFBwMBMB0GA1Ud +DgQWBBR7nqpN0AFa5jGDs9gq3vK3OvxpSTAfBgNVHSMEGDAWgBRLda21NY4WNcdt +MQUU20dpFjzLxTAKBggqhkjOPQQDAgNIADBFAiAwIGSpsMJmq3ExrtLq3PpNWbe/ +HMnp7V7rmS8K8uWJpwIhAI8osCchJeKH7xg4yqfZXBxrU1FwagojEGQDk3HKEoIe +-----END CERTIFICATE----- diff --git a/tests/data/server.key b/tests/data/server.key new file mode 100644 index 0000000..a8d526e --- /dev/null +++ b/tests/data/server.key @@ -0,0 +1,5 @@ +-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgP5sIs6digdS0/HXU +a8G16oZk3HZwfYHTjnAIm0APWpuhRANCAASQglF3Qygu2k/RfvnlluNi9XRWnasM +QJBnILrhtvC5++Ta+bX4lQG6yeedIh+pFTlLfAfqnNLJbFmK5zVNexyu +-----END PRIVATE KEY----- diff --git a/tests/tst_mqtt.cpp b/tests/tst_mqtt.cpp index 5000164..887684b 100644 --- a/tests/tst_mqtt.cpp +++ b/tests/tst_mqtt.cpp @@ -1,6 +1,7 @@ // MosquittoClient against a real broker (docs/DESIGN.md, D30 level 2): a // local mosquitto on a free port. Connect, messages, retained messages, -// reconnect, last will, login. Skips when mosquitto isn't installed. +// reconnect, last will, login, TLS (certificates in tests/data/, D44). Skips +// when mosquitto isn't installed. // // The binary is also its own helper process for the last-will test: // `tst_mqtt --will-helper ` connects with a will and waits to be @@ -24,6 +25,33 @@ namespace { constexpr int kTimeoutMs = 10000; +QString data(const char* file) { + return QStringLiteral(TEST_DATA_DIR "/") + QLatin1String(file); +} + +// Broker configuration for a TLS listener with the test CA's server +// certificate. +QString tlsListener(bool requireClientCertificate = false) { + return QStringLiteral( + "allow_anonymous true\ncafile %1\ncertfile %2\nkeyfile %3\n" + "require_certificate %4\n") + .arg(data("ca.crt"), data("server.crt"), data("server.key"), + requireClientCertificate ? QStringLiteral("true") + : QStringLiteral("false")); +} + +MosquittoClient::Settings tlsSettings(int port, const char* caFile) { + MosquittoClient::Settings s; + s.broker.host = QStringLiteral("127.0.0.1"); + s.broker.port = port; + s.broker.tls.enabled = true; + if (caFile) s.broker.tls.caFile = data(caFile); + s.clientId = QStringLiteral("tls"); + s.reconnectMinMs = 100; + s.reconnectMaxMs = 500; + return s; +} + QString findTool(const QString& name) { QString path = QStandardPaths::findExecutable(name); if (path.isEmpty()) @@ -350,6 +378,121 @@ class TestMqtt : public QObject { } } + void tlsConnects() { + Broker broker(m_dir.path()); + QVERIFY(broker.start(tlsListener())); + MosquittoClient a(tlsSettings(broker.port(), "ca.crt")); + Recorder ra(&a); + QVERIFY(connectClient(a, ra, QStringLiteral("test/#"))); + QVERIFY(a.publish(QStringLiteral("test/x"), "secret", false)); + QTRY_COMPARE_WITH_TIMEOUT(ra.messages.size(), 1, kTimeoutMs); + QVERIFY(ra.errors.isEmpty()); + } + + // The broker's certificate isn't from the given CA: no connection, and + // the reason says why. + void tlsWrongCaRejected() { + Broker broker(m_dir.path()); + QVERIFY(broker.start(tlsListener())); + MosquittoClient a(tlsSettings(broker.port(), "other-ca.crt")); + Recorder ra(&a); + a.connectToBroker(); + QTRY_VERIFY_WITH_TIMEOUT(!ra.errors.isEmpty(), kTimeoutMs); + QVERIFY2(ra.errors.first().contains(QStringLiteral("verify failed")), + qPrintable(ra.errors.first())); + QTest::qWait(300); + QVERIFY(!ra.states.contains(MqttClient::State::Connected)); + } + + // Without ca_file the system's CAs are used (they don't know the test + // CA). Fails the same way, which shows that TLS is on. + void tlsWithSystemCertificates() { + Broker broker(m_dir.path()); + QVERIFY(broker.start(tlsListener())); + MosquittoClient a(tlsSettings(broker.port(), nullptr)); + Recorder ra(&a); + a.connectToBroker(); + QTRY_VERIFY_WITH_TIMEOUT(!ra.errors.isEmpty(), kTimeoutMs); + QVERIFY2(ra.errors.first().contains(QStringLiteral("verify failed")), + qPrintable(ra.errors.first())); + } + + // A TLS error ends libmosquitto's own loop for good (D45); ours retries. + // First a plain listener on the port (handshake fails), then TLS. + void retriesAfterTlsError() { + Broker broker(m_dir.path()); + QVERIFY(broker.start()); + MosquittoClient a(tlsSettings(broker.port(), "ca.crt")); + Recorder ra(&a); + a.connectToBroker(); + QTRY_VERIFY_WITH_TIMEOUT(!ra.errors.isEmpty(), kTimeoutMs); + broker.stop(); + QVERIFY(broker.start(tlsListener())); + QTRY_COMPARE_WITH_TIMEOUT(a.state(), MqttClient::State::Connected, + kTimeoutMs); + } + + void tlsClientCertificate_data() { + QTest::addColumn("withCertificate"); + QTest::newRow("with") << true; + QTest::newRow("without") << false; + } + + void tlsClientCertificate() { + QFETCH(bool, withCertificate); + Broker broker(m_dir.path()); + QVERIFY(broker.start(tlsListener(true))); + MosquittoClient::Settings s = tlsSettings(broker.port(), "ca.crt"); + if (withCertificate) { + s.broker.tls.certFile = data("client.crt"); + s.broker.tls.keyFile = data("client.key"); + } + MosquittoClient a(s); + Recorder ra(&a); + a.connectToBroker(); + if (withCertificate) { + QTRY_COMPARE_WITH_TIMEOUT(a.state(), MqttClient::State::Connected, + kTimeoutMs); + } else { + QTRY_VERIFY_WITH_TIMEOUT(!ra.errors.isEmpty(), kTimeoutMs); + QVERIFY(!ra.states.contains(MqttClient::State::Connected)); + } + } + + // Settings errors show up at once, without a connection attempt. + void tlsSettingsErrors_data() { + QTest::addColumn("caFile"); + QTest::addColumn("certFile"); + QTest::addColumn("keyFile"); + QTest::addColumn("error"); + QTest::newRow("missing CA file") + << QStringLiteral("/no/such/ca.crt") << QString() << QString() + << QStringLiteral("TLS: cannot read /no/such/ca.crt"); + QTest::newRow("certificate without key") + << data("ca.crt") << data("client.crt") << QString() + << QStringLiteral("TLS: cert_file and key_file go together"); + QTest::newRow("client certificate without CA file") + << QString() << data("client.crt") << data("client.key") + << QStringLiteral("TLS: a client certificate needs ca_file"); + } + + void tlsSettingsErrors() { + QFETCH(QString, caFile); + QFETCH(QString, certFile); + QFETCH(QString, keyFile); + QFETCH(QString, error); + MosquittoClient::Settings s = tlsSettings(freePort(), nullptr); + s.broker.tls.caFile = caFile; + s.broker.tls.certFile = certFile; + s.broker.tls.keyFile = keyFile; + MosquittoClient a(s); + Recorder ra(&a); + a.connectToBroker(); + QTRY_COMPARE_WITH_TIMEOUT(ra.errors.size(), 1, kTimeoutMs); + QCOMPARE(ra.errors.first(), error); + QCOMPARE(a.state(), MqttClient::State::Disconnected); + } + private: QTemporaryDir m_dir; };