diff --git a/Makefile b/Makefile index f16f089..f8e514e 100644 --- a/Makefile +++ b/Makefile @@ -483,7 +483,29 @@ $(BUILD)/tools/lha: # MIDGE_VERSION; the release workflow's tag-vs-source check # (scripts/verify-version.sh) separately confirms the tag matches # src/version.h, closing the loop: tag == src/version.h == the binaries. -dist: build guide $(LHA) +# +# Unlike `build`/`library`/`test-host` (which feature-detect AmiSSL and +# degrade gracefully - the shared CI image has no SDK, see M68K_HAS_AMISSL +# above), a release MUST ship a TLS-capable mqtt.library: the readme/docs +# advertise TLS unconditionally. So `dist` fetches the SDK itself if +# missing and re-invokes `build` as a sub-make ($(MAKE), not a plain +# prerequisite) - M68K_HAS_AMISSL is a `:=` immediate-expansion variable, +# fixed when the OUTER make parses this file, before any recipe (including +# the fetch below) has run; only a fresh sub-make process re-parses the +# Makefile and sees the now-populated .cache/amissl-sdk/. The `rm -f` +# before it is required too - `make` has no idea M68K_HAS_AMISSL changed, +# only that mqtt.library's sources didn't, and would otherwise report +# "Nothing to be done" and reuse the stale non-TLS build. The grep after +# is a mechanical, permanent guard against this whole class of regression +# (a silently-degraded release), not just a one-time fix. +dist: guide $(LHA) + @if [ ! -f "$(AMISSL_SDK_DIR)/include/openssl/ssl.h" ]; then \ + echo "dist: fetching AmiSSL SDK (a release must ship TLS support)"; \ + $(MAKE) fetch-amissl-sdk; \ + fi + rm -f $(BUILD)/mqtt.library + $(MAKE) build + @grep -aq "amisslmaster.library" $(BUILD)/mqtt.library || { echo "dist: $(BUILD)/mqtt.library has no AmiSSL/TLS support - refusing to release (see M68K_HAS_AMISSL / make fetch-amissl-sdk)"; exit 1; } @v=$$(sed -n 's/^#define MIDGE_VERSION[[:space:]]*"\(.*\)"$$/\1/p' src/version.h); \ for b in mqtt_pub mqtt_sub mqtt_pub-static mqtt_sub-static; do \ grep -aqF "\$$VER: $$b $$v (" $(BUILD)/$$b || { echo "dist: $(BUILD)/$$b lacks \"\$$VER: $$b $$v (...)\" - stale build/?"; exit 1; }; \ diff --git a/README.md b/README.md index 4cce828..c90a8a0 100644 --- a/README.md +++ b/README.md @@ -8,14 +8,16 @@ own telemetry. It ships as command-line tools, a shared library (`mqtt.library`) any other AmigaOS program can call, and — eventually — a ReAction dashboard application. -> **Status:** the protocol core, `mqtt_pub`/`mqtt_sub` CLI tools, and +> **Status:** the protocol core, `mqtt_pub`/`mqtt_sub` CLI tools, > `mqtt.library` (subprocess-per-connection, QoS 0/1, MsgPort dispatch, -> opt-in auto-reconnect with backoff) are implemented, cross-build to real -> AmigaOS binaries, and are verified end-to-end against a real Mosquitto -> broker on real 68020 codegen via Copperline in CI. Not yet tagged for -> release. TLS via AmiSSL and a ReAction dashboard with Home Assistant MQTT -> discovery are next — see [`docs/ARCHITECTURE.md`](docs/ARCHITECTURE.md) -> for the full roadmap. +> opt-in auto-reconnect with backoff), and TLS via AmiSSL (`mco_TLS`, +> optional private-CA trust) are implemented, cross-build to real AmigaOS +> binaries, and are verified end-to-end against a real Mosquitto broker on +> real 68020 codegen via Copperline in CI. Not yet tagged for release. A +> ReAction dashboard with Home Assistant MQTT discovery is next — see the +> [issue tracker](https://github.com/sidick/midge/issues) for what's +> planned (`docs/ARCHITECTURE.md`'s own roadmap section was retired once +> it matched shipped history). ## AI-assisted development @@ -55,6 +57,11 @@ implementation of its own. `mco_AutoReconnect` mode reconnects with exponential backoff and auto-resubscribes after an unexpected drop. Two example programs and a hand-written autodoc ship in the release archive's `developer/` tree. +- **TLS via AmiSSL** — opt-in (`TLS`/`TLSINSECURE`/`CAFILE` on the Amiga + tools, `-s`/`-S`/`-c` on the host builds), certificate and hostname + verification on by default, with an optional private-CA trust anchor for + self-hosted brokers. The statically linked tools have no AmiSSL + dependency and don't support it. - **Portable, testable core** — the packet codec and connection state machine are plain C99 with zero OS dependencies, so protocol-level tests run on the host with no emulator. @@ -68,8 +75,9 @@ implementation of its own. dependencies beyond C99 itself (no sockets, no timers, no allocation — caller buffers only). All platform code lives in `src/host/`/`src/amiga/`; the transport vtable (`src/core/mqtt_transport.h`) is the only seam - between them, so TLS (Phase 3) drops in as another transport - implementation with no change to the client state machine. + between them - TLS (`src/host/transport_openssl.c`, + `src/amiga/transport_amissl.c`) is exactly this: another transport + implementation, no change to the client state machine. - **Real on-target verification, not just host tests** — every networking claim is proven against a real broker on real 68020 codegen (Copperline's HostSocket board), not just mocked. `volamos` gives a much faster local diff --git a/scripts/verify-version.sh b/scripts/verify-version.sh index ebc8130..2018e62 100755 --- a/scripts/verify-version.sh +++ b/scripts/verify-version.sh @@ -13,7 +13,29 @@ tag="${tag_ref#v}" src=$(sed -n 's/^#define MIDGE_VERSION[[:space:]]*"\(.*\)"$/\1/p' src/version.h) readme=$(sed -n 's/^Version:[[:space:]]*\(.*\)$/\1/p' midge.readme) +date=$(sed -n 's/^#define MIDGE_VERSION_DATE[[:space:]]*"\(.*\)"$/\1/p' src/version.h) -echo "tag=$tag src/version.h=$src midge.readme=$readme" +echo "tag=$tag src/version.h=$src ($date) midge.readme=$readme" [ "$tag" = "$src" ] || { echo "::error file=src/version.h::Tag v$tag does not match MIDGE_VERSION \"$src\""; exit 1; } [ "$tag" = "$readme" ] || { echo "::error file=midge.readme::Tag v$tag does not match Version: \"$readme\""; exit 1; } + +case "$date" in + [0-9][0-9].[0-9][0-9].[0-9][0-9][0-9][0-9]) ;; + *) echo "::error file=src/version.h::MIDGE_VERSION_DATE \"$date\" is not DD.MM.YYYY"; exit 1 ;; +esac + +# issue #9: MIDGE_VERSION_DATE isn't covered by the checks above, or by +# `make dist`'s own $VER grep (it only matches the version number) - a +# release PR that bumps MIDGE_VERSION but forgets the date would otherwise +# ship binaries whose $VER string still carries the previous release's +# date. Compare against the previous release tag's own src/version.h +# (skipped gracefully if there isn't one, e.g. this is the first release). +prev_tag=$(git tag -l 'v*' --sort=-v:refname | grep -vFx "v$tag" | head -n1 || true) +if [ -n "$prev_tag" ]; then + prev_date=$(git show "$prev_tag:src/version.h" 2>/dev/null | \ + sed -n 's/^#define MIDGE_VERSION_DATE[[:space:]]*"\(.*\)"$/\1/p') + if [ -n "$prev_date" ] && [ "$date" = "$prev_date" ]; then + echo "::error file=src/version.h::MIDGE_VERSION_DATE \"$date\" is unchanged since $prev_tag - bump it too" + exit 1 + fi +fi diff --git a/src/amiga/transport_amissl.c b/src/amiga/transport_amissl.c index 80bd3de..42abf9e 100644 --- a/src/amiga/transport_amissl.c +++ b/src/amiga/transport_amissl.c @@ -67,12 +67,33 @@ #include #include +#include "tool_clock.h" + /* WaitSelect() poll interval per gated retry - matches * transport_bsdsocket.c's MQTT_BSDSOCKET_POLL_SECS and * tests/copperline/amissl-spike/amisslspike.c's SPIKE_POLL_SECS, so all * three Amiga-side network waits share one wakeup cadence. */ #define MQTT_AMISSL_POLL_SECS 1 +/* Overall handshake budget. Without this, a peer that accepts the TCP + * connect but never completes (or never finishes) the TLS handshake - + * a tarpit, a firewall black hole, the wrong service on the port, a wedged + * broker - makes the loop below spin in wait_gate() forever: unlike + * amissl_send()/recv() (bounded by core's own keepalive/PUBACK/SUBACK + * timeouts once a connection exists), this loop runs entirely inside + * transport_amissl_connect(), before mqtt_client_connect() is even + * called, so nothing above it can time it out. Worse inside mqtt.library: + * the connection child subprocess isn't reachable by the caller's + * MQTT_Disconnect()/MQTT_DeleteClient() (cmd_port) until this call + * returns, and wait_gate() deliberately treats a break_sigmask wake as a + * retry-me timeout, not an abort (see its own comment) - so this was a + * real "only a reboot recovers" hang, not just a slow retry. 30s is + * generous next to every real handshake observed in testing (even on a + * stock 68020, see userdocs/CLI-Reference.md's CPU-speed note) while + * still bounding the worst case to something a caller/reconnect loop can + * recover from. */ +#define MQTT_AMISSL_HANDSHAKE_TIMEOUT_MS 30000u + /* Single bounded wait for readability/writability, mirroring * amisslspike.c's wait_gate() exactly (same fd_set/timeval/sigmask shape as * transport_bsdsocket_recv). Returns 0 if the fd may now be ready (timeout @@ -312,26 +333,26 @@ int transport_amissl_connect(mqtt_transport *out, amissl_ctx *ctx, SSL_set_connect_state(ctx->ssl); /* --- 4. handshake, WaitSelect-gated (see amisslspike.c) -------------- */ - for (;;) { - int rc; - ERR_clear_error(); - rc = SSL_do_handshake(ctx->ssl); - if (rc == 1) - break; - - { - int err = SSL_get_error(ctx->ssl, rc); - if (err == SSL_ERROR_WANT_READ) { - if (wait_gate(ctx, 0) < 0) - goto fail_ssl; - continue; - } - if (err == SSL_ERROR_WANT_WRITE) { - if (wait_gate(ctx, 1) < 0) + { + uint32_t handshake_start = tool_now_ms(); + + for (;;) { + int rc; + ERR_clear_error(); + rc = SSL_do_handshake(ctx->ssl); + if (rc == 1) + break; + + { + int err = SSL_get_error(ctx->ssl, rc); + if (err != SSL_ERROR_WANT_READ && err != SSL_ERROR_WANT_WRITE) + goto fail_ssl; /* handshake failed outright */ + if (tool_now_ms() - handshake_start > + MQTT_AMISSL_HANDSHAKE_TIMEOUT_MS) + goto fail_ssl; /* MQTT_AMISSL_HANDSHAKE_TIMEOUT_MS - see its comment */ + if (wait_gate(ctx, err == SSL_ERROR_WANT_WRITE) < 0) goto fail_ssl; - continue; } - goto fail_ssl; /* handshake failed outright */ } } diff --git a/src/host/transport_openssl.c b/src/host/transport_openssl.c index ee5b959..1ee95e1 100644 --- a/src/host/transport_openssl.c +++ b/src/host/transport_openssl.c @@ -23,6 +23,14 @@ #include #include +#include "tool_clock.h" + +/* Overall handshake budget - see the retry loop in + * transport_openssl_connect() below. Same value and same rationale as + * transport_amissl.c's MQTT_AMISSL_HANDSHAKE_TIMEOUT_MS: bounds a peer + * that accepts the TCP connect but never completes the handshake. */ +#define MQTT_OPENSSL_HANDSHAKE_TIMEOUT_MS 30000u + static int openssl_send(void *ctx, const uint8_t *buf, size_t len) { openssl_ctx *c = (openssl_ctx *)ctx; @@ -186,25 +194,60 @@ int transport_openssl_connect(mqtt_transport *out, openssl_ctx *ctx, return -1; } - ERR_clear_error(); - if (SSL_connect(ssl) != 1) { - /* SSL_get_verify_result() MUST be read before SSL_free() - after - * free it silently reports X509_V_OK regardless of what actually - * happened, which would misreport every handshake failure as a - * verification success. */ - long verify_result = SSL_get_verify_result(ssl); - - fprintf(stderr, "mqtt: TLS handshake with %s:%u failed", host, - (unsigned)port); - if (verify_result != X509_V_OK) - fprintf(stderr, " (%s)", - X509_verify_cert_error_string(verify_result)); - fprintf(stderr, "\n"); + /* A single SSL_connect() call is not actually enough: this socket is + * blocking with a 1s SO_RCVTIMEO (tcp_connect() above), and a read + * that hits that timeout makes the BIO layer set its retry flag and + * SSL_connect() return SSL_ERROR_WANT_READ - blocking mode only + * affects whether the underlying recv()/send() call itself blocks, + * not whether OpenSSL surfaces a retry indication when one comes + * back empty. One lost packet mid-handshake (TCP's own initial RTO is + * >=1s) or a slow broker would otherwise be reported as a fatal + * handshake failure instead of retried - a full connect() attempt is + * only actually done once every 1s of real elapsed time here, so this + * loop just keeps re-issuing SSL_connect() until it succeeds, fails + * for a real reason, or the overall budget below is spent. */ + { + uint32_t handshake_start = tool_now_ms(); + int rc, err; - SSL_free(ssl); - SSL_CTX_free(ssl_ctx); - close(fd); - return -1; + for (;;) { + ERR_clear_error(); + rc = SSL_connect(ssl); + if (rc == 1) + break; + err = SSL_get_error(ssl, rc); + if (err != SSL_ERROR_WANT_READ && err != SSL_ERROR_WANT_WRITE) + break; /* real failure - fall through to the report below */ + if (tool_now_ms() - handshake_start > + MQTT_OPENSSL_HANDSHAKE_TIMEOUT_MS) { + fprintf(stderr, "mqtt: TLS handshake with %s:%u timed out\n", + host, (unsigned)port); + SSL_free(ssl); + SSL_CTX_free(ssl_ctx); + close(fd); + return -1; + } + } + + if (rc != 1) { + /* SSL_get_verify_result() MUST be read before SSL_free() - + * after free it silently reports X509_V_OK regardless of what + * actually happened, which would misreport every handshake + * failure as a verification success. */ + long verify_result = SSL_get_verify_result(ssl); + + fprintf(stderr, "mqtt: TLS handshake with %s:%u failed", host, + (unsigned)port); + if (verify_result != X509_V_OK) + fprintf(stderr, " (%s)", + X509_verify_cert_error_string(verify_result)); + fprintf(stderr, "\n"); + + SSL_free(ssl); + SSL_CTX_free(ssl_ctx); + close(fd); + return -1; + } } ctx->fd = fd; diff --git a/userdocs/CLI-Reference.md b/userdocs/CLI-Reference.md index 4633377..77e68b1 100644 --- a/userdocs/CLI-Reference.md +++ b/userdocs/CLI-Reference.md @@ -19,7 +19,8 @@ Publish a single message to a broker topic and exit. ``` mqtt_pub HOST/A,PORT/N/K,TOPIC/A,MESSAGE/K,FILE/K,QOS/N/K,CLIENTID/K,USER/K, - PASSWORD/K,KEEPALIVE/N/K,RETAIN/S,VERBOSE/S + PASSWORD/K,KEEPALIVE/N/K,RETAIN/S,VERBOSE/S,TLS/S,TLSINSECURE/S, + CAFILE/K ``` | Argument | Meaning | @@ -52,7 +53,7 @@ with Ctrl-C. ``` mqtt_sub HOST/A,PORT/N/K,TOPIC/A,QOS/N/K,CLIENTID/K,USER/K,PASSWORD/K, - KEEPALIVE/N/K,COUNT/N/K,VERBOSE/S + KEEPALIVE/N/K,COUNT/N/K,VERBOSE/S,TLS/S,TLSINSECURE/S,CAFILE/K ``` | Argument | Meaning | @@ -81,8 +82,8 @@ mqtt_sub HOST 192.168.1.10 TOPIC home/# The repo also builds host-native `mqtt_pub-host` / `mqtt_sub-host` (via `make cli`), used for development and by the CI broker smoke tests. They take getopt-style flags mirroring the Amiga arguments above (`-h HOST`, -`-p PORT`, `-t TOPIC`, and so on), plus two flags that only exist on the -host builds so far: +`-p PORT`, `-t TOPIC`, and so on), including the TLS switches - as short +flags rather than the Amiga tools' `TLS`/`TLSINSECURE`/`CAFILE` keywords: | Flag | Meaning | |---|---| diff --git a/userdocs/mqtt-library.md b/userdocs/mqtt-library.md index b5ba9ff..eb9d027 100644 --- a/userdocs/mqtt-library.md +++ b/userdocs/mqtt-library.md @@ -14,6 +14,9 @@ every function. - AmigaOS 3.1+ (3.2 is the reference platform), 68020 or better. - A TCP/IP stack providing `bsdsocket.library` (Roadshow, AmiTCP, Miami, or an emulator-provided stack) - the same requirement as the CLI tools. +- For `mco_TLS`: [AmiSSL](https://github.com/jens-maus/amissl) 5.x, + installed separately - see [Installation](Installation.md#installing-amissl-needed-for-tls). + Everything else in this page works identically without it. ## Installing @@ -132,6 +135,30 @@ The default (a zeroed `MqttConnectOpts`, or `mco_AutoReconnect = FALSE`) is today's plain behaviour: an unexpected drop leaves the client disconnected until the caller makes a fresh `MQTT_Connect()` call. +## TLS + +Setting `mco_TLS = TRUE` in `MqttConnectOpts` connects via AmiSSL instead +of a plain TCP transport, with certificate and hostname verification on +by default: + +- `mco_TLSInsecure = TRUE` skips certificate/hostname verification + (`SSL_VERIFY_NONE`) - for testing against self-signed or otherwise + untrusted brokers only, never for production use. Unlike the CLI tools' + `TLSINSECURE`/`-S` (which implies `TLS`/`-s`), the library's + `mco_TLSInsecure` is simply **ignored** unless `mco_TLS` is also set - + setting it alone does not turn TLS on. +- `mco_CAFile` names a PEM file trusted as an extra CA, alongside + AmiSSL's bundled trust store - for a broker behind a private CA that + isn't in it. Ignored unless `mco_TLS` is set, and ignored if + `mco_TLSInsecure` is also set (nothing to verify against then). +- If this build of `mqtt.library` has no AmiSSL support, or AmiSSL isn't + installed, `MQTT_Connect()` fails with `MQTTERR_NOTCONNECTED` - the + same as any other connect failure, not a distinct error code. +- AmiSSL is CPU-intensive: a genuinely stock, unaccelerated 68020 has been + found to intermittently fail under it, while any real accelerator (or a + 68030 or better) is reliable - see + [CLI Reference](CLI-Reference.md#a-note-on-tls-and-cpu-speed). + ## Error codes Every function that returns a status returns 0 on success and a negative @@ -141,10 +168,11 @@ code on failure: |---|---|---| | `MQTTERR_OK` | 0 | Success. | | `MQTTERR_NOMEM` | -200 | Out of memory creating the handle, a message port, or the subprocess. | -| `MQTTERR_NOSTACK` | -201 | `CreateNewProcTags()` failed to start the connection subprocess. | -| `MQTTERR_NOTCONNECTED` | -202 | Called before `MQTT_Connect()` succeeded, after `MQTT_Disconnect()`, or while auto-reconnect is mid-reconnect. | +| `MQTTERR_NOSTACK` | -201 | Reserved for a `CreateNewProcTags()` failure - never actually returned today: `MQTT_CreateClient()` has no error-code channel and returns a bare `NULL` for every creation failure, including this one. | +| `MQTTERR_NOTCONNECTED` | -202 | Called before `MQTT_Connect()` succeeded, after `MQTT_Disconnect()`, or while auto-reconnect is mid-reconnect. Also returned when `mco_TLS` was requested but this build/install has no AmiSSL support. | | `MQTTERR_TIMEOUT` | -203 | QoS 1 publish: no `PUBACK` within the retry budget. Subscribe: no `SUBACK` within ~10 seconds. | | `MQTTERR_REFUSED` | -204 | Subscribe: the broker's `SUBACK` refused the subscription. | +| `MQTTERR_STATE` | -205 | `MQTT_Connect()` called on a handle that's already connected (a prior `MQTT_Connect()` succeeded and neither `MQTT_Disconnect()` nor an unexpected drop has happened since). | Other negative values are passed straight through from the portable MQTT codec below the library (a malformed packet, a rejected `CONNECT`, and so