diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9f9890c9..14aacb26 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,6 +61,47 @@ jobs: retention-days: 1 compression-level: 0 + openssl-integration: + runs-on: ubuntu-latest + permissions: + contents: read + checks: write + container: + image: ghcr.io/davidcozens/cpputest:sha-18f19e1 + options: --user root + env: + GIT_CONFIG_COUNT: 1 + GIT_CONFIG_KEY_0: safe.directory + GIT_CONFIG_VALUE_0: '*' + + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + + - name: Configure + run: cmake --preset debug + + - name: Build integration tests + run: cmake --build --preset debug --target OpenSslIntegrationTests + + - name: Run integration tests + run: cd build/debug && ./Tests/OpenSslIntegration/OpenSslIntegrationTests -v -ojunit -k OpenSslIntegrationTests + + - name: Test Report + uses: dorny/test-reporter@v3 + if: success() || failure() + with: + name: Test Results (OpenSSL Integration) + path: build/debug/cpputest_*.xml + reporter: java-junit + + - name: Upload JUnit XML + if: success() || failure() + uses: actions/upload-artifact@v4 + with: + name: junit-openssl-integration + path: build/debug/cpputest_*.xml + retention-days: 1 + clang-build-and-test: runs-on: ubuntu-latest permissions: @@ -508,7 +549,7 @@ jobs: quality-summary: if: always() && github.event_name == 'pull_request' - needs: [build-and-test, clang-build-and-test, sanitize, coverage, tidy, cppcheck, format, bdd, windows-build-and-test, bdd-windows] + needs: [build-and-test, clang-build-and-test, sanitize, coverage, tidy, cppcheck, format, bdd, windows-build-and-test, bdd-windows, openssl-integration] runs-on: ubuntu-latest permissions: contents: read @@ -563,6 +604,11 @@ jobs: "name": "Unit Tests (Sanitize)", "pattern": "**/junit-sanitize/cpputest_*.xml" }, + { + "id": "junit", + "name": "Integration Tests (OpenSSL)", + "pattern": "**/junit-openssl-integration/cpputest_*.xml" + }, { "id": "junit", "name": "BDD Tests (Linux)", diff --git a/CMakeLists.txt b/CMakeLists.txt index 6020f6b8..74931f1c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -88,8 +88,13 @@ endif() # OpenSSL. find_package(OpenSSL QUIET) option(SOLIDSYSLOG_OPENSSL "Include OpenSSL-based TLS support" ${OpenSSL_FOUND}) -if(SOLIDSYSLOG_OPENSSL AND NOT OpenSSL_FOUND) - message(FATAL_ERROR "SOLIDSYSLOG_OPENSSL=ON but OpenSSL was not found; install OpenSSL or pass -DSOLIDSYSLOG_OPENSSL=OFF") +if(SOLIDSYSLOG_OPENSSL) + if(NOT OpenSSL_FOUND) + message(FATAL_ERROR "SOLIDSYSLOG_OPENSSL=ON but OpenSSL was not found; install OpenSSL or pass -DSOLIDSYSLOG_OPENSSL=OFF") + endif() + if(OPENSSL_VERSION VERSION_LESS 3.0) + message(FATAL_ERROR "SOLIDSYSLOG_OPENSSL requires OpenSSL 3.0 or later; found ${OPENSSL_VERSION}. The integration harness uses EVP_RSA_gen (3.0+).") + endif() endif() add_subdirectory(Core/Source) diff --git a/DEVLOG.md b/DEVLOG.md index faf70774..23c0392b 100644 --- a/DEVLOG.md +++ b/DEVLOG.md @@ -841,3 +841,115 @@ S3.9 / S3.10 / S3.11 as placeholder issues under E3. because it rides on the already-reliable auto-load. Worth recording: separating "conventions that must be read" from "conventions the tool auto-loads" is a real gap, and signposting from CLAUDE.md bridges it. + +## 2026-04-21/22 — S03.08 TLS hardening: cert validation, cipher pinning, lifecycle + +### What went well + +- **Planning turn before coding paid off.** Before any C, I laid out a + phase plan and six API-shape surprises — integration harness must + live in its own binary (fake interposes libssl), cert generation + programmatic not fixtures, cipher policy at caller not library, BIO + lifecycle per-Open, the `OpenSslFake` per-BIO aliasing issue. The + user corrected one call (BDD scenarios for negative cases → + integration harness instead, much faster feedback), parked three + (cipher shape, BIO lifecycle, `SSL_VERIFY_FAIL_IF_NO_PEER_CERT`), + and greenlit the rest. Every surprise came up once, got a decision, + then didn't recur. + +- **Composable test-support trio.** `TlsTestCert` + `TlsTestServer` + + `BioPairStream` under `Tests/OpenSslIntegration/` each do one thing; + new scenarios are a single test body with a cert config and an + assertion. The fourth scenario (unsupported cipher) was a + seven-line addition. This is the scaffolding paying for itself + instantly, not in six months. + +- **Pump-on-read pattern.** `BioPairStream::Read` drives the server + side cooperatively when the BIO pair is empty, so + `SolidSyslogTlsStream`'s synchronous `SSL_connect` works unchanged + over non-blocking in-memory BIOs — no threads in the harness, no + production changes to accommodate testing. + +### What didn't go well / process refinements + +- **BDD-first plan got reversed.** My Phase 1 was "BDD pending + scenarios for SL4 failure modes." User pushed back: integration + harness covers it faster with no PO-visibility loss. Should have + surfaced this trade-off *myself* in the opening surprises list + rather than defaulting to BDD. Memory entry + `feedback_integration_over_bdd.md` captures the pattern: for + crypto/TLS failure modes, integration harness beats BDD. + +- **Phase 2 / Phase 4 coupling I didn't spot up front.** Plan + separated "return-value checks" (Phase 2) from "resource lifecycle" + (Phase 4). In practice, every new early-return path made a + pre-existing leak worse until `Destroy` covered partial state. + Ended up interleaving — not a real problem but the phase plan was + cleaner than reality. + +- **Commit count.** 17 commits on the branch is a lot of tight red/green + pairs. Each is a clear atomic behaviour change and reads well in + `git log --oneline`, but the PR-review surface is larger than a + single "TLS hardening" story arguably needed. Next time, consider + batching several return-value checks into one commit when they + share a test shape. + +- **Characterisation vs red/green mismatch.** Three of the integration + tests went green on first write — OpenSSL defaults already rejected + expired / wrong-hostname / wrong-CA. Under strict "only write + production code driven by a failing test" this feels off, but the + tests still earn their keep as regression protection. I flagged + this explicitly rather than pretending it was red/green, and the + user accepted the framing. Calling out "this is characterisation, + not TDD" is probably the right move whenever it happens. + +- **One stray production branch.** The `if (ctx == NULL) return NULL;` + inside `CreateSslContext` wasn't strictly driven by a failing test — + it was the natural implementation alongside the `OpenReturnsFalseWhenCtxNewFails` + fix. User reminded me mid-session to "only write a new branch of code + when you have a failing test." Tightened up for the rest of the session. + +### Decisions captured this session + +- **Integration test executable is its own CI job** (option C of A/B/C) + — runs in parallel with `build-and-test`, self-contained configure + + build of only `OpenSslIntegrationTests`. Clang and sanitize + presets don't run it; unit tests remain the coverage engine. +- **Cipher list is caller policy.** `SolidSyslogTlsStreamConfig.cipherList` + is forwarded to `SSL_CTX_set_cipher_list` if non-NULL; library ships + no SL4 default. `docs/iec62443.md` names + `ECDHE+AESGCM:ECDHE+CHACHA20` as a reasonable starting point, not a + baked-in list. +- **BIO_METHOD lifecycle is per-Open.** Symmetric with SSL/SSL_CTX — + each Open allocates a fresh set, each Close releases. Destroy + covers partial-init state idempotently. +- **Coverage stays unit-test-only.** Integration tests are supplementary + evidence of real-libssl acceptance, not the coverage engine. Memory + entry `project_coverage_integration.md` records the trigger for + revisiting (multi-platform, or a real-libssl-only code path). + +### Deferred + +- **`OpenSslFake` per-BIO data refactor.** Listed as S03.08 acceptance + but no production code path or test requires distinct BIO data yet + (the library creates exactly one BIO per Open). Forward-looking + for mTLS (S03.09) and future cases. Suggest a short follow-up + story rather than doing it here without a test that fails without it. +- **TLS 1.3 cipher suite control.** `SSL_CTX_set_cipher_list` governs + TLS 1.2 only; TLS 1.3 uses `SSL_CTX_set_ciphersuites`. Out of the + ticket's acceptance. Follow-up. +- **`max_proto_version` control.** Library pins min but not max. No + current need; might emerge with an SL4 policy that mandates TLS 1.3. +- **Formal Planned → Available promotion** for TLS in + `docs/iec62443.md` — S03.11 scope. This session added the SL4 + substrate narrative without touching the headline status. + +### Open questions for the blog source + +- **When is characterisation a legitimate TDD outcome?** Three tests + went green on first write because OpenSSL already did the right + thing. Keeping them is evidence and regression protection; rejecting + them would be dogmatic. The useful lens: if the test protects against + a realistic future regression, it's load-bearing — TDD discipline is + about "production code driven by tests", not "every test must have + been red at some point." diff --git a/Platform/OpenSsl/Interface/SolidSyslogTlsStream.h b/Platform/OpenSsl/Interface/SolidSyslogTlsStream.h index f6855aca..dc21b951 100644 --- a/Platform/OpenSsl/Interface/SolidSyslogTlsStream.h +++ b/Platform/OpenSsl/Interface/SolidSyslogTlsStream.h @@ -10,6 +10,7 @@ EXTERN_C_BEGIN struct SolidSyslogStream* transport; /* underlying byte stream — caller owns */ const char* caBundlePath; /* PEM file of trust anchors */ const char* serverName; /* SNI + cert hostname check; NULL to skip */ + const char* cipherList; /* TLS 1.2 cipher list; NULL = OpenSSL default */ }; struct SolidSyslogStream* SolidSyslogTlsStream_Create(const struct SolidSyslogTlsStreamConfig* config); diff --git a/Platform/OpenSsl/Source/SolidSyslogTlsStream.c b/Platform/OpenSsl/Source/SolidSyslogTlsStream.c index ac5e8ce9..91a7be53 100644 --- a/Platform/OpenSsl/Source/SolidSyslogTlsStream.c +++ b/Platform/OpenSsl/Source/SolidSyslogTlsStream.c @@ -9,105 +9,205 @@ struct SolidSyslogTlsStream struct SolidSyslogTlsStreamConfig config; SSL_CTX* ctx; SSL* ssl; + BIO_METHOD* bioMethod; }; -static bool Open(struct SolidSyslogStream* self, const struct SolidSyslogAddress* addr); -static bool Send(struct SolidSyslogStream* self, const void* buffer, size_t size); -static SolidSyslogSsize Read(struct SolidSyslogStream* self, void* buffer, size_t size); -static void Close(struct SolidSyslogStream* self); -static SSL_CTX* CreateSslContext(const char* caBundlePath); -static BIO* CreateTransportBio(void); -static int TransportBioCreate(BIO* bio); -static int TransportBioRead(BIO* bio, char* buffer, int size); -static int TransportBioWrite(BIO* bio, const char* buffer, int size); -static long TransportBioCtrl(BIO* bio, int cmd, long larg, void* parg); +static inline bool TlsStream_AttachTransportBio(struct SolidSyslogTlsStream* stream); +static inline void TlsStream_Close(struct SolidSyslogStream* self); +static inline bool TlsStream_ConfigureCipherList(SSL_CTX* ctx, const char* cipherList); +static inline bool TlsStream_ConfigureExpectedHostname(struct SolidSyslogTlsStream* stream); +static inline bool TlsStream_ConfigureProtocolFloor(SSL_CTX* ctx); +static inline bool TlsStream_ConfigureSslContext(SSL_CTX* ctx, const struct SolidSyslogTlsStreamConfig* config); +static inline bool TlsStream_ConfigureTrustAnchors(SSL_CTX* ctx, const char* caBundlePath); +static inline SSL_CTX* TlsStream_CreateSslContext(const struct SolidSyslogTlsStreamConfig* config); +static inline BIO* TlsStream_CreateTransportBio(struct SolidSyslogTlsStream* stream); +static inline BIO_METHOD* TlsStream_CreateTransportBioMethod(void); +static inline bool TlsStream_InitSslContext(struct SolidSyslogTlsStream* stream); +static inline bool TlsStream_InitSslSession(struct SolidSyslogTlsStream* stream); +static inline bool TlsStream_Open(struct SolidSyslogStream* self, const struct SolidSyslogAddress* addr); +static inline bool TlsStream_PerformHandshake(struct SolidSyslogTlsStream* stream); +static inline SolidSyslogSsize TlsStream_Read(struct SolidSyslogStream* self, void* buffer, size_t size); +static inline void TlsStream_ReleaseBioMethod(struct SolidSyslogTlsStream* stream); +static inline void TlsStream_ReleaseHandshakeState(struct SolidSyslogTlsStream* stream); +static inline void TlsStream_ReleaseSsl(struct SolidSyslogTlsStream* stream); +static inline void TlsStream_ReleaseSslContext(struct SolidSyslogTlsStream* stream); +static inline bool TlsStream_Send(struct SolidSyslogStream* self, const void* buffer, size_t size); +static inline int TlsStream_TransportBioCreate(BIO* bio); +static inline long TlsStream_TransportBioCtrl(BIO* bio, int cmd, long larg, void* parg); +static inline int TlsStream_TransportBioRead(BIO* bio, char* buffer, int size); +static inline int TlsStream_TransportBioWrite(BIO* bio, const char* buffer, int size); + +static const struct SolidSyslogStream VTABLE = { + .Open = TlsStream_Open, + .Send = TlsStream_Send, + .Read = TlsStream_Read, + .Close = TlsStream_Close, +}; static struct SolidSyslogTlsStream instance; struct SolidSyslogStream* SolidSyslogTlsStream_Create(const struct SolidSyslogTlsStreamConfig* config) { - instance.config = *config; - instance.base.Open = Open; - instance.base.Send = Send; - instance.base.Read = Read; - instance.base.Close = Close; + instance.config = *config; + instance.base = VTABLE; return &instance.base; } void SolidSyslogTlsStream_Destroy(void) { - if (instance.ctx != NULL) + TlsStream_ReleaseHandshakeState(&instance); + TlsStream_ReleaseSslContext(&instance); +} + +static inline void TlsStream_ReleaseHandshakeState(struct SolidSyslogTlsStream* stream) +{ + TlsStream_ReleaseSsl(stream); + TlsStream_ReleaseBioMethod(stream); +} + +static inline void TlsStream_ReleaseSsl(struct SolidSyslogTlsStream* stream) +{ + if (stream->ssl != NULL) { - SSL_CTX_free(instance.ctx); - instance.ctx = NULL; + SSL_free(stream->ssl); + stream->ssl = NULL; } } -static bool Open(struct SolidSyslogStream* self, const struct SolidSyslogAddress* addr) +static inline void TlsStream_ReleaseBioMethod(struct SolidSyslogTlsStream* stream) { - struct SolidSyslogTlsStream* stream = (struct SolidSyslogTlsStream*) self; - if (!SolidSyslogStream_Open(stream->config.transport, addr)) + if (stream->bioMethod != NULL) { - return false; + BIO_meth_free(stream->bioMethod); + stream->bioMethod = NULL; } - stream->ctx = CreateSslContext(stream->config.caBundlePath); - stream->ssl = SSL_new(stream->ctx); - BIO* bio = CreateTransportBio(); - BIO_set_data(bio, stream->config.transport); - SSL_set_bio(stream->ssl, bio, bio); - if (stream->config.serverName != NULL) +} + +static inline void TlsStream_ReleaseSslContext(struct SolidSyslogTlsStream* stream) +{ + if (stream->ctx != NULL) { - SSL_set_tlsext_host_name(stream->ssl, stream->config.serverName); - SSL_set1_host(stream->ssl, stream->config.serverName); + SSL_CTX_free(stream->ctx); + stream->ctx = NULL; } - return SSL_connect(stream->ssl) > 0; } -static bool Send(struct SolidSyslogStream* self, const void* buffer, size_t size) +static inline bool TlsStream_Open(struct SolidSyslogStream* self, const struct SolidSyslogAddress* addr) { struct SolidSyslogTlsStream* stream = (struct SolidSyslogTlsStream*) self; - return SSL_write(stream->ssl, buffer, (int) size) > 0; + return SolidSyslogStream_Open(stream->config.transport, addr) && TlsStream_InitSslContext(stream) && TlsStream_InitSslSession(stream) && + TlsStream_AttachTransportBio(stream) && TlsStream_ConfigureExpectedHostname(stream) && TlsStream_PerformHandshake(stream); } -static SolidSyslogSsize Read(struct SolidSyslogStream* self, void* buffer, size_t size) +static inline bool TlsStream_InitSslContext(struct SolidSyslogTlsStream* stream) { - struct SolidSyslogTlsStream* stream = (struct SolidSyslogTlsStream*) self; - return (SolidSyslogSsize) SSL_read(stream->ssl, buffer, (int) size); + stream->ctx = TlsStream_CreateSslContext(&stream->config); + return stream->ctx != NULL; } -static void Close(struct SolidSyslogStream* self) +static inline SSL_CTX* TlsStream_CreateSslContext(const struct SolidSyslogTlsStreamConfig* config) { - struct SolidSyslogTlsStream* stream = (struct SolidSyslogTlsStream*) self; - SSL_shutdown(stream->ssl); - SSL_free(stream->ssl); - SolidSyslogStream_Close(stream->config.transport); + SSL_CTX* ctx = SSL_CTX_new(TLS_client_method()); + if (ctx != NULL && !TlsStream_ConfigureSslContext(ctx, config)) + { + SSL_CTX_free(ctx); + ctx = NULL; + } + return ctx; +} + +static inline bool TlsStream_ConfigureSslContext(SSL_CTX* ctx, const struct SolidSyslogTlsStreamConfig* config) +{ + return TlsStream_ConfigureTrustAnchors(ctx, config->caBundlePath) && TlsStream_ConfigureProtocolFloor(ctx) && + TlsStream_ConfigureCipherList(ctx, config->cipherList); +} + +static inline bool TlsStream_ConfigureTrustAnchors(SSL_CTX* ctx, const char* caBundlePath) +{ + bool ok = SSL_CTX_load_verify_locations(ctx, caBundlePath, NULL) == 1; + if (ok) + { + SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL); + } + return ok; +} + +static inline bool TlsStream_ConfigureProtocolFloor(SSL_CTX* ctx) +{ + return SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) == 1; +} + +static inline bool TlsStream_ConfigureCipherList(SSL_CTX* ctx, const char* cipherList) +{ + bool ok = true; + if (cipherList != NULL) + { + ok = SSL_CTX_set_cipher_list(ctx, cipherList) == 1; + } + return ok; } -static BIO* CreateTransportBio(void) +static inline bool TlsStream_InitSslSession(struct SolidSyslogTlsStream* stream) +{ + stream->ssl = SSL_new(stream->ctx); + return stream->ssl != NULL; +} + +static inline bool TlsStream_AttachTransportBio(struct SolidSyslogTlsStream* stream) +{ + BIO* bio = TlsStream_CreateTransportBio(stream); + bool ok = bio != NULL; + if (ok) + { + BIO_set_data(bio, stream->config.transport); + SSL_set_bio(stream->ssl, bio, bio); + } + return ok; +} + +static inline BIO* TlsStream_CreateTransportBio(struct SolidSyslogTlsStream* stream) +{ + stream->bioMethod = TlsStream_CreateTransportBioMethod(); + BIO* bio = NULL; + if (stream->bioMethod != NULL) + { + bio = BIO_new(stream->bioMethod); + if (bio == NULL) + { + TlsStream_ReleaseBioMethod(stream); + } + } + return bio; +} + +static inline BIO_METHOD* TlsStream_CreateTransportBioMethod(void) { BIO_METHOD* method = BIO_meth_new(BIO_TYPE_SOURCE_SINK, "SolidSyslog transport BIO"); - BIO_meth_set_create(method, TransportBioCreate); - BIO_meth_set_read(method, TransportBioRead); - BIO_meth_set_write(method, TransportBioWrite); - BIO_meth_set_ctrl(method, TransportBioCtrl); - return BIO_new(method); + if (method != NULL) + { + BIO_meth_set_create(method, TlsStream_TransportBioCreate); + BIO_meth_set_read(method, TlsStream_TransportBioRead); + BIO_meth_set_write(method, TlsStream_TransportBioWrite); + BIO_meth_set_ctrl(method, TlsStream_TransportBioCtrl); + } + return method; } /* Called when BIO_new instantiates a BIO from our method. Marking init=1 tells * OpenSSL the BIO is ready for I/O; without it SSL_connect bails early. */ -static int TransportBioCreate(BIO* bio) +static inline int TlsStream_TransportBioCreate(BIO* bio) { BIO_set_init(bio, 1); return 1; } -static int TransportBioRead(BIO* bio, char* buffer, int size) +static inline int TlsStream_TransportBioRead(BIO* bio, char* buffer, int size) { struct SolidSyslogStream* transport = (struct SolidSyslogStream*) BIO_get_data(bio); return (int) SolidSyslogStream_Read(transport, buffer, (size_t) size); } -static int TransportBioWrite(BIO* bio, const char* buffer, int size) +static inline int TlsStream_TransportBioWrite(BIO* bio, const char* buffer, int size) { struct SolidSyslogStream* transport = (struct SolidSyslogStream*) BIO_get_data(bio); return SolidSyslogStream_Send(transport, buffer, (size_t) size) ? size : -1; @@ -117,28 +217,57 @@ static int TransportBioWrite(BIO* bio, const char* buffer, int size) * during normal operation; returning 1 for the common lifecycle commands lets * SSL_connect / SSL_write / SSL_shutdown proceed. Unknown commands return 0. */ // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- signature fixed by OpenSSL BIO_ctrl_fn contract -static long TransportBioCtrl(BIO* bio, int cmd, long larg, void* parg) +static inline long TlsStream_TransportBioCtrl(BIO* bio, int cmd, long larg, void* parg) { (void) bio; (void) larg; (void) parg; + long result = 0; switch (cmd) { case BIO_CTRL_FLUSH: case BIO_CTRL_PUSH: case BIO_CTRL_POP: case BIO_CTRL_DUP: - return 1; + result = 1; + break; default: - return 0; + break; } + return result; } -static SSL_CTX* CreateSslContext(const char* caBundlePath) +static inline bool TlsStream_ConfigureExpectedHostname(struct SolidSyslogTlsStream* stream) { - SSL_CTX* ctx = SSL_CTX_new(TLS_client_method()); - SSL_CTX_load_verify_locations(ctx, caBundlePath, NULL); - SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL); - SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION); - return ctx; + bool ok = true; + if (stream->config.serverName != NULL) + { + ok = (SSL_set_tlsext_host_name(stream->ssl, stream->config.serverName) == 1) && (SSL_set1_host(stream->ssl, stream->config.serverName) == 1); + } + return ok; +} + +static inline bool TlsStream_PerformHandshake(struct SolidSyslogTlsStream* stream) +{ + return SSL_connect(stream->ssl) > 0; +} + +static inline bool TlsStream_Send(struct SolidSyslogStream* self, const void* buffer, size_t size) +{ + struct SolidSyslogTlsStream* stream = (struct SolidSyslogTlsStream*) self; + return SSL_write(stream->ssl, buffer, (int) size) > 0; +} + +static inline SolidSyslogSsize TlsStream_Read(struct SolidSyslogStream* self, void* buffer, size_t size) +{ + struct SolidSyslogTlsStream* stream = (struct SolidSyslogTlsStream*) self; + return (SolidSyslogSsize) SSL_read(stream->ssl, buffer, (int) size); +} + +static inline void TlsStream_Close(struct SolidSyslogStream* self) +{ + struct SolidSyslogTlsStream* stream = (struct SolidSyslogTlsStream*) self; + SSL_shutdown(stream->ssl); + TlsStream_ReleaseHandshakeState(stream); + SolidSyslogStream_Close(stream->config.transport); } diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt index 50fa1399..0c60327d 100644 --- a/Tests/CMakeLists.txt +++ b/Tests/CMakeLists.txt @@ -112,6 +112,10 @@ if(SOLIDSYSLOG_WINSOCK) target_link_libraries(${PROJECT_NAME}Tests PRIVATE WinsockFakes) endif() +if(SOLIDSYSLOG_OPENSSL) + add_subdirectory(OpenSslIntegration) +endif() + # Standard CppUTest console output via ctest -V add_test(NAME ${PROJECT_NAME}Tests COMMAND ${PROJECT_NAME}Tests) diff --git a/Tests/OpenSslFakeTest.cpp b/Tests/OpenSslFakeTest.cpp index 8822a368..3037de89 100644 --- a/Tests/OpenSslFakeTest.cpp +++ b/Tests/OpenSslFakeTest.cpp @@ -179,6 +179,19 @@ TEST(OpenSslFake, BioGetDataReturnsPreviouslySetData) POINTERS_EQUAL(&sentinel, BIO_get_data(bio)); } +TEST(OpenSslFake, BioDataIsStoredPerInstance) +{ + BIO_METHOD* method = BIO_meth_new(0, "fake"); + BIO* bio1 = BIO_new(method); + BIO* bio2 = BIO_new(method); + int a = 0; + int b = 0; + BIO_set_data(bio1, &a); + BIO_set_data(bio2, &b); + POINTERS_EQUAL(&a, BIO_get_data(bio1)); + POINTERS_EQUAL(&b, BIO_get_data(bio2)); +} + // NOLINTNEXTLINE(readability-non-const-parameter) -- signature fixed by OpenSSL BIO_meth_set_read contract static int DummyRead(BIO* bio, char* buf, int size) { diff --git a/Tests/OpenSslIntegration/BioPairStream.c b/Tests/OpenSslIntegration/BioPairStream.c new file mode 100644 index 00000000..79e3bae7 --- /dev/null +++ b/Tests/OpenSslIntegration/BioPairStream.c @@ -0,0 +1,90 @@ +#include "BioPairStream.h" +#include "SolidSyslogStreamDefinition.h" + +#include +#include +#include + +struct BioPairStream +{ + struct SolidSyslogStream base; + BIO* bio; + BioPairStreamPumpFunction pump; + void* pumpContext; +}; + +static bool Open(struct SolidSyslogStream* self, const struct SolidSyslogAddress* addr); +static bool Send(struct SolidSyslogStream* self, const void* buffer, size_t size); +static SolidSyslogSsize Read(struct SolidSyslogStream* self, void* buffer, size_t size); +static void Close(struct SolidSyslogStream* self); + +struct SolidSyslogStream* BioPairStream_Create(BIO* bio) +{ + struct BioPairStream* stream = (struct BioPairStream*) calloc(1, sizeof(struct BioPairStream)); + stream->base.Open = Open; + stream->base.Send = Send; + stream->base.Read = Read; + stream->base.Close = Close; + stream->bio = bio; + return &stream->base; +} + +void BioPairStream_Destroy(struct SolidSyslogStream* self) +{ + free(self); +} + +void BioPairStream_SetPump(struct SolidSyslogStream* self, BioPairStreamPumpFunction pump, void* context) +{ + struct BioPairStream* stream = (struct BioPairStream*) self; + stream->pump = pump; + stream->pumpContext = context; +} + +static bool Open(struct SolidSyslogStream* self, const struct SolidSyslogAddress* addr) +{ + (void) self; + (void) addr; + return true; +} + +static bool Send(struct SolidSyslogStream* self, const void* buffer, size_t size) +{ + struct BioPairStream* stream = (struct BioPairStream*) self; + int written = BIO_write(stream->bio, buffer, (int) size); + return written == (int) size; +} + +/* Reads from the BIO, driving the paired peer via the pump callback whenever the + * read would otherwise block. Models a blocking stream on top of a non-blocking + * BIO pair so SolidSyslogTlsStream's synchronous SSL_connect / SSL_read path + * works unchanged. */ +static SolidSyslogSsize Read(struct SolidSyslogStream* self, void* buffer, size_t size) +{ + struct BioPairStream* stream = (struct BioPairStream*) self; + SolidSyslogSsize result = -1; + bool done = false; + while (!done) + { + int bytesRead = BIO_read(stream->bio, buffer, (int) size); + if (bytesRead > 0) + { + result = (SolidSyslogSsize) bytesRead; + done = true; + } + else if (!BIO_should_retry(stream->bio) || stream->pump == NULL) + { + done = true; + } + else + { + stream->pump(stream->pumpContext); + } + } + return result; +} + +static void Close(struct SolidSyslogStream* self) +{ + (void) self; +} diff --git a/Tests/OpenSslIntegration/BioPairStream.h b/Tests/OpenSslIntegration/BioPairStream.h new file mode 100644 index 00000000..a2783e25 --- /dev/null +++ b/Tests/OpenSslIntegration/BioPairStream.h @@ -0,0 +1,18 @@ +#ifndef BIOPAIRSTREAM_H +#define BIOPAIRSTREAM_H + +#include "ExternC.h" +#include "SolidSyslogStream.h" +#include + +EXTERN_C_BEGIN + + typedef void (*BioPairStreamPumpFunction)(void* context); + + struct SolidSyslogStream* BioPairStream_Create(BIO * bio); + void BioPairStream_Destroy(struct SolidSyslogStream * self); + void BioPairStream_SetPump(struct SolidSyslogStream * self, BioPairStreamPumpFunction pump, void* context); + +EXTERN_C_END + +#endif /* BIOPAIRSTREAM_H */ diff --git a/Tests/OpenSslIntegration/CMakeLists.txt b/Tests/OpenSslIntegration/CMakeLists.txt new file mode 100644 index 00000000..c4d0d41e --- /dev/null +++ b/Tests/OpenSslIntegration/CMakeLists.txt @@ -0,0 +1,27 @@ +# OpenSSL-backed integration tests: links the real libssl and exercises +# SolidSyslogTlsStream end-to-end against an in-process server. Separate binary +# from SolidSyslogTests because unit tests link OpenSslFakes in place of libssl +# — the two cannot share one executable without fake/real symbol collision. +add_executable(OpenSslIntegrationTests + BioPairStream.c + TlsTestCert.c + TlsTestServer.c + SolidSyslogTlsStreamIntegrationTest.cpp + main.cpp +) + +target_link_libraries(OpenSslIntegrationTests PRIVATE + ${PROJECT_NAME} + CppUTest + CppUTestExt + OpenSSL::SSL + OpenSSL::Crypto + Threads::Threads +) + +target_include_directories(OpenSslIntegrationTests PRIVATE + ${CMAKE_SOURCE_DIR}/Core/Source + ${CMAKE_CURRENT_SOURCE_DIR} +) + +add_test(NAME OpenSslIntegrationTests COMMAND OpenSslIntegrationTests) diff --git a/Tests/OpenSslIntegration/SolidSyslogTlsStreamIntegrationTest.cpp b/Tests/OpenSslIntegration/SolidSyslogTlsStreamIntegrationTest.cpp new file mode 100644 index 00000000..37625609 --- /dev/null +++ b/Tests/OpenSslIntegration/SolidSyslogTlsStreamIntegrationTest.cpp @@ -0,0 +1,137 @@ +#include "BioPairStream.h" +#include "CppUTest/TestHarness.h" +#include "SolidSyslogAddress.h" +#include "SolidSyslogStream.h" +#include "SolidSyslogTlsStream.h" +#include "TlsTestCert.h" +#include "TlsTestServer.h" + +#include +#include +#include +#include +#include + +// clang-format off +TEST_GROUP(TlsStreamIntegration) +{ + struct TlsTestCert cert = {}; + struct TlsTestServer* server = nullptr; + struct SolidSyslogStream* transport = nullptr; + struct SolidSyslogTlsStreamConfig tlsConfig = {}; + struct SolidSyslogStream* tlsStream = nullptr; + SolidSyslogAddressStorage addrStorage = {}; + struct SolidSyslogAddress* addr = nullptr; + char caPath[64] = {}; + + void setup() override + { + addr = SolidSyslogAddress_FromStorage(&addrStorage); + } + + void teardown() override + { + if (tlsStream != nullptr) { SolidSyslogTlsStream_Destroy(); } + if (transport != nullptr) { BioPairStream_Destroy(transport); } + if (server != nullptr) { TlsTestServer_Destroy(server); } + if (cert.cert != nullptr) { TlsTestCert_Destroy(&cert); } + if (caPath[0] != '\0') { unlink(caPath); } + } + + void buildScenario(const struct TlsTestCertConfig& certConfig, + const char* clientServerName = "localhost") + { + TlsTestCert_Create(&certConfig, &cert); + std::strcpy(caPath, "/tmp/solidsyslog_tls_ca_XXXXXX"); + int fd = mkstemp(caPath); + CHECK_TRUE(fd >= 0); + if (fd < 0) + { + return; + } + close(fd); + TlsTestCert_WritePemToFile(&cert, caPath); + + struct TlsTestServerConfig serverConfig = {}; + serverConfig.serverCert = &cert; + server = TlsTestServer_Create(&serverConfig); + + transport = BioPairStream_Create(TlsTestServer_ClientSideBio(server)); + BioPairStream_SetPump(transport, TlsTestServer_Pump, server); + + tlsConfig.transport = transport; + tlsConfig.caBundlePath = caPath; + tlsConfig.serverName = clientServerName; + tlsStream = SolidSyslogTlsStream_Create(&tlsConfig); + } +}; + +// clang-format on + +static const char* const LOCALHOST_SANS[] = {"localhost", nullptr}; + +TEST(TlsStreamIntegration, HandshakeSucceedsAgainstTrustedServerCert) +{ + struct TlsTestCertConfig certConfig = {}; + certConfig.commonName = "localhost"; + certConfig.subjectAltDnsNames = LOCALHOST_SANS; + buildScenario(certConfig); + + CHECK_TRUE(SolidSyslogStream_Open(tlsStream, addr)); +} + +TEST(TlsStreamIntegration, HandshakeRejectedWhenServerCertIsExpired) +{ + struct TlsTestCertConfig certConfig = {}; + certConfig.commonName = "localhost"; + certConfig.subjectAltDnsNames = LOCALHOST_SANS; + certConfig.notBefore = std::time(nullptr) - 7200; + certConfig.notAfter = std::time(nullptr) - 3600; + buildScenario(certConfig); + + CHECK_FALSE(SolidSyslogStream_Open(tlsStream, addr)); +} + +TEST(TlsStreamIntegration, HandshakeRejectedWhenServerCertHostnameDoesNotMatch) +{ + static const char* const otherSans[] = {"someone-else.example", nullptr}; + struct TlsTestCertConfig certConfig = {}; + certConfig.commonName = "someone-else.example"; + certConfig.subjectAltDnsNames = otherSans; + buildScenario(certConfig); /* client.serverName defaults to "localhost" */ + + CHECK_FALSE(SolidSyslogStream_Open(tlsStream, addr)); +} + +TEST(TlsStreamIntegration, HandshakeRejectedWhenClientDoesNotTrustServerCert) +{ + struct TlsTestCertConfig certConfig = {}; + certConfig.commonName = "localhost"; + certConfig.subjectAltDnsNames = LOCALHOST_SANS; + buildScenario(certConfig); + + /* Overwrite the client's trust file with an unrelated self-signed cert + * so the server's cert is no longer anchored in the trust store. The CA + * file is loaded on Open, so this replacement takes effect for the next + * handshake attempt. */ + struct TlsTestCertConfig untrustedConfig = {}; + untrustedConfig.commonName = "some-other-entity.example"; + struct TlsTestCert untrusted = {}; + TlsTestCert_Create(&untrustedConfig, &untrusted); + TlsTestCert_WritePemToFile(&untrusted, caPath); + + CHECK_FALSE(SolidSyslogStream_Open(tlsStream, addr)); + + TlsTestCert_Destroy(&untrusted); +} + +TEST(TlsStreamIntegration, HandshakeRejectedWhenCipherListIsUnsupported) +{ + struct TlsTestCertConfig certConfig = {}; + certConfig.commonName = "localhost"; + certConfig.subjectAltDnsNames = LOCALHOST_SANS; + tlsConfig.cipherList = "NOT-A-REAL-CIPHER"; + buildScenario(certConfig); + + CHECK_FALSE(SolidSyslogStream_Open(tlsStream, addr)); +} diff --git a/Tests/OpenSslIntegration/TlsTestCert.c b/Tests/OpenSslIntegration/TlsTestCert.c new file mode 100644 index 00000000..e4f7466c --- /dev/null +++ b/Tests/OpenSslIntegration/TlsTestCert.c @@ -0,0 +1,102 @@ +#include "TlsTestCert.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +enum +{ + RSA_BITS = 2048, + DEFAULT_VALIDITY_SECONDS = 3600, +}; + +static void SetValidity(X509* cert, const struct TlsTestCertConfig* config); +static void SetSubject(X509* cert, const char* commonName); +static void AddSubjectAltNames(X509* cert, const char* const * dnsNames); + +void TlsTestCert_Create(const struct TlsTestCertConfig* config, struct TlsTestCert* out) +{ + EVP_PKEY* key = EVP_RSA_gen(RSA_BITS); + X509* cert = X509_new(); + + X509_set_version(cert, 2); /* X.509 v3 */ + ASN1_INTEGER_set(X509_get_serialNumber(cert), 1); + + SetValidity(cert, config); + SetSubject(cert, config->commonName); + AddSubjectAltNames(cert, config->subjectAltDnsNames); + + X509* issuerCert = (config->issuer != NULL) ? config->issuer->cert : cert; + EVP_PKEY* issuerKey = (config->issuer != NULL) ? config->issuer->key : key; + X509_set_issuer_name(cert, X509_get_subject_name(issuerCert)); + X509_set_pubkey(cert, key); + X509_sign(cert, issuerKey, EVP_sha256()); + + out->cert = cert; + out->key = key; +} + +void TlsTestCert_Destroy(struct TlsTestCert* cert) +{ + if (cert->cert != NULL) + { + X509_free(cert->cert); + cert->cert = NULL; + } + if (cert->key != NULL) + { + EVP_PKEY_free(cert->key); + cert->key = NULL; + } +} + +void TlsTestCert_WritePemToFile(const struct TlsTestCert* cert, const char* path) +{ + FILE* file = fopen(path, "w"); + if (file == NULL) + { + return; + } + PEM_write_X509(file, cert->cert); + fclose(file); +} + +static void SetValidity(X509* cert, const struct TlsTestCertConfig* config) +{ + time_t now = time(NULL); + time_t start = (config->notBefore != 0) ? config->notBefore : now; + time_t end = (config->notAfter != 0) ? config->notAfter : now + DEFAULT_VALIDITY_SECONDS; + X509_time_adj_ex(X509_getm_notBefore(cert), 0, 0, &start); + X509_time_adj_ex(X509_getm_notAfter(cert), 0, 0, &end); +} + +static void SetSubject(X509* cert, const char* commonName) +{ + X509_NAME* name = X509_get_subject_name(cert); + X509_NAME_add_entry_by_txt(name, "CN", MBSTRING_ASC, (const unsigned char*) commonName, -1, -1, 0); +} + +static void AddSubjectAltNames(X509* cert, const char* const * dnsNames) +{ + if (dnsNames == NULL) + { + return; + } + STACK_OF(GENERAL_NAME)* sans = sk_GENERAL_NAME_new_null(); + for (int i = 0; dnsNames[i] != NULL; i++) + { + GENERAL_NAME* gen = GENERAL_NAME_new(); + ASN1_IA5STRING* str = ASN1_IA5STRING_new(); + ASN1_STRING_set(str, dnsNames[i], -1); + GENERAL_NAME_set0_value(gen, GEN_DNS, str); + sk_GENERAL_NAME_push(sans, gen); + } + X509_add1_ext_i2d(cert, NID_subject_alt_name, sans, 0, 0); + sk_GENERAL_NAME_pop_free(sans, GENERAL_NAME_free); +} diff --git a/Tests/OpenSslIntegration/TlsTestCert.h b/Tests/OpenSslIntegration/TlsTestCert.h new file mode 100644 index 00000000..f34e1f89 --- /dev/null +++ b/Tests/OpenSslIntegration/TlsTestCert.h @@ -0,0 +1,32 @@ +#ifndef TLSTESTCERT_H +#define TLSTESTCERT_H + +#include "ExternC.h" +#include +#include +#include + +EXTERN_C_BEGIN + + struct TlsTestCert + { + X509* cert; + EVP_PKEY* key; + }; + + struct TlsTestCertConfig + { + const char* commonName; + const char* const * subjectAltDnsNames; /* NULL-terminated array; NULL if no SAN */ + time_t notBefore; /* 0 = now */ + time_t notAfter; /* 0 = now + 3600 */ + const struct TlsTestCert* issuer; /* NULL = self-signed */ + }; + + void TlsTestCert_Create(const struct TlsTestCertConfig* config, struct TlsTestCert* out); + void TlsTestCert_Destroy(struct TlsTestCert * cert); + void TlsTestCert_WritePemToFile(const struct TlsTestCert* cert, const char* path); + +EXTERN_C_END + +#endif /* TLSTESTCERT_H */ diff --git a/Tests/OpenSslIntegration/TlsTestServer.c b/Tests/OpenSslIntegration/TlsTestServer.c new file mode 100644 index 00000000..059ef81f --- /dev/null +++ b/Tests/OpenSslIntegration/TlsTestServer.c @@ -0,0 +1,75 @@ +#include "TlsTestServer.h" + +#include +#include +#include +#include + +struct TlsTestServer +{ + SSL_CTX* ctx; + SSL* ssl; + BIO* serverBio; + BIO* clientBio; + bool handshakeComplete; +}; + +struct TlsTestServer* TlsTestServer_Create(const struct TlsTestServerConfig* config) +{ + struct TlsTestServer* self = (struct TlsTestServer*) calloc(1, sizeof(struct TlsTestServer)); + + self->ctx = SSL_CTX_new(TLS_server_method()); + SSL_CTX_use_certificate(self->ctx, config->serverCert->cert); + SSL_CTX_use_PrivateKey(self->ctx, config->serverCert->key); + if (config->cipherList != NULL) + { + SSL_CTX_set_cipher_list(self->ctx, config->cipherList); + } + + self->ssl = SSL_new(self->ctx); + BIO_new_bio_pair(&self->serverBio, 0, &self->clientBio, 0); + SSL_set_bio(self->ssl, self->serverBio, self->serverBio); + self->serverBio = NULL; /* ownership transferred to SSL via SSL_set_bio */ + SSL_set_accept_state(self->ssl); + + return self; +} + +void TlsTestServer_Destroy(struct TlsTestServer* self) +{ + if (self == NULL) + { + return; + } + if (self->ssl != NULL) + { + SSL_free(self->ssl); /* also frees serverBio */ + } + if (self->clientBio != NULL) + { + BIO_free(self->clientBio); + } + if (self->ctx != NULL) + { + SSL_CTX_free(self->ctx); + } + free(self); +} + +BIO* TlsTestServer_ClientSideBio(struct TlsTestServer* self) +{ + return self->clientBio; +} + +void TlsTestServer_Pump(void* context) +{ + struct TlsTestServer* self = (struct TlsTestServer*) context; + if (!self->handshakeComplete) + { + int ret = SSL_accept(self->ssl); + if (ret == 1) + { + self->handshakeComplete = true; + } + } +} diff --git a/Tests/OpenSslIntegration/TlsTestServer.h b/Tests/OpenSslIntegration/TlsTestServer.h new file mode 100644 index 00000000..32d538ca --- /dev/null +++ b/Tests/OpenSslIntegration/TlsTestServer.h @@ -0,0 +1,32 @@ +#ifndef TLSTESTSERVER_H +#define TLSTESTSERVER_H + +#include "ExternC.h" +#include "TlsTestCert.h" +#include + +EXTERN_C_BEGIN + + struct TlsTestServer; + + struct TlsTestServerConfig + { + const struct TlsTestCert* serverCert; /* includes matching private key */ + const char* cipherList; /* NULL = server default */ + }; + + struct TlsTestServer* TlsTestServer_Create(const struct TlsTestServerConfig* config); + void TlsTestServer_Destroy(struct TlsTestServer * self); + + /* Returns the client-facing end of the internal BIO pair. Pass to + * BioPairStream_Create and use as the transport for SolidSyslogTlsStream. */ + BIO* TlsTestServer_ClientSideBio(struct TlsTestServer * self); + + /* Advances the server's state machine one step. Used as a pump callback + * so client-side reads can make cooperative progress. Context is the + * TlsTestServer pointer. */ + void TlsTestServer_Pump(void* context); + +EXTERN_C_END + +#endif /* TLSTESTSERVER_H */ diff --git a/Tests/OpenSslIntegration/main.cpp b/Tests/OpenSslIntegration/main.cpp new file mode 100644 index 00000000..a185088c --- /dev/null +++ b/Tests/OpenSslIntegration/main.cpp @@ -0,0 +1,6 @@ +#include "CppUTest/CommandLineTestRunner.h" + +int main(int argc, char** argv) +{ + return CommandLineTestRunner::RunAllTests(argc, argv); +} diff --git a/Tests/SolidSyslogTlsStreamTest.cpp b/Tests/SolidSyslogTlsStreamTest.cpp index 56fb3445..7239a86e 100644 --- a/Tests/SolidSyslogTlsStreamTest.cpp +++ b/Tests/SolidSyslogTlsStreamTest.cpp @@ -79,6 +79,40 @@ TEST(SolidSyslogTlsStream, OpenSetsTls12Floor) LONGS_EQUAL(TLS1_2_VERSION, OpenSslFake_LastMinProtoVersion()); } +TEST(SolidSyslogTlsStream, OpenPassesCipherListToSslCtx) +{ + SolidSyslogTlsStream_Destroy(); + config.cipherList = "ECDHE+AESGCM"; + stream = SolidSyslogTlsStream_Create(&config); + SolidSyslogStream_Open(stream, addr); + STRCMP_EQUAL("ECDHE+AESGCM", OpenSslFake_LastCipherList()); +} + +TEST(SolidSyslogTlsStream, OpenSkipsCipherListSetupWhenNotConfigured) +{ + SolidSyslogStream_Open(stream, addr); + LONGS_EQUAL(0, OpenSslFake_SetCipherListCallCount()); +} + +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenCipherListRejected) +{ + SolidSyslogTlsStream_Destroy(); + config.cipherList = "not-a-real-cipher"; + stream = SolidSyslogTlsStream_Create(&config); + OpenSslFake_SetCipherListFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, CipherListFailureFreesCtx) +{ + SolidSyslogTlsStream_Destroy(); + config.cipherList = "not-a-real-cipher"; + stream = SolidSyslogTlsStream_Create(&config); + OpenSslFake_SetCipherListFails(true); + SolidSyslogStream_Open(stream, addr); + LONGS_EQUAL(1, OpenSslFake_CtxFreeCallCount()); +} + TEST(SolidSyslogTlsStream, OpenCreatesSslSession) { SolidSyslogStream_Open(stream, addr); @@ -271,6 +305,13 @@ TEST(SolidSyslogTlsStream, CloseClosesTransport) LONGS_EQUAL(1, StreamFake_CloseCallCount(transport)); } +TEST(SolidSyslogTlsStream, CloseFreesBioMethod) +{ + SolidSyslogStream_Open(stream, addr); + SolidSyslogStream_Close(stream); + LONGS_EQUAL(1, OpenSslFake_BioMethFreeCallCount()); +} + TEST(SolidSyslogTlsStream, DestroyFreesSslContext) { SolidSyslogStream_Open(stream, addr); @@ -279,6 +320,37 @@ TEST(SolidSyslogTlsStream, DestroyFreesSslContext) /* teardown re-Destroys safely */ } +TEST(SolidSyslogTlsStream, DestroyFreesBioMethodWhenCloseNotCalled) +{ + SolidSyslogStream_Open(stream, addr); + SolidSyslogTlsStream_Destroy(); + LONGS_EQUAL(1, OpenSslFake_BioMethFreeCallCount()); + /* teardown re-Destroys safely */ +} + +TEST(SolidSyslogTlsStream, DestroyAfterCloseDoesNotDoubleFreeBioMethod) +{ + SolidSyslogStream_Open(stream, addr); + SolidSyslogStream_Close(stream); + SolidSyslogTlsStream_Destroy(); + LONGS_EQUAL(1, OpenSslFake_BioMethFreeCallCount()); +} + +TEST(SolidSyslogTlsStream, DestroyFreesSslWhenCloseNotCalled) +{ + SolidSyslogStream_Open(stream, addr); + SolidSyslogTlsStream_Destroy(); + LONGS_EQUAL(1, OpenSslFake_FreeCallCount()); +} + +TEST(SolidSyslogTlsStream, DestroyAfterCloseDoesNotDoubleFreeSsl) +{ + SolidSyslogStream_Open(stream, addr); + SolidSyslogStream_Close(stream); + SolidSyslogTlsStream_Destroy(); + LONGS_EQUAL(1, OpenSslFake_FreeCallCount()); +} + /* ------------------------------------------------------------------------- * Pointer-chain assertions: each OpenSSL call must receive the handle * returned by the preceding call, not some stale or NULL pointer. @@ -401,6 +473,82 @@ TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenHandshakeFails) CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); } +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenSet1HostFails) +{ + SolidSyslogTlsStream_Destroy(); + config.serverName = "logs.example"; + stream = SolidSyslogTlsStream_Create(&config); + OpenSslFake_SetSet1HostFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenSniHostnameSetupFails) +{ + SolidSyslogTlsStream_Destroy(); + config.serverName = "logs.example"; + stream = SolidSyslogTlsStream_Create(&config); + OpenSslFake_SetSniHostnameFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenCtxNewFails) +{ + OpenSslFake_SetCtxNewFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenSslNewFails) +{ + OpenSslFake_SetSslNewFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenLoadVerifyLocationsFails) +{ + OpenSslFake_SetLoadVerifyLocationsFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, LoadVerifyLocationsFailureFreesCtx) +{ + OpenSslFake_SetLoadVerifyLocationsFails(true); + SolidSyslogStream_Open(stream, addr); + LONGS_EQUAL(1, OpenSslFake_CtxFreeCallCount()); +} + +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenMinProtoVersionFails) +{ + OpenSslFake_SetMinProtoVersionFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, MinProtoVersionFailureFreesCtx) +{ + OpenSslFake_SetMinProtoVersionFails(true); + SolidSyslogStream_Open(stream, addr); + LONGS_EQUAL(1, OpenSslFake_CtxFreeCallCount()); +} + +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenBioMethNewFails) +{ + OpenSslFake_SetBioMethNewFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, OpenReturnsFalseWhenBioNewFails) +{ + OpenSslFake_SetBioNewFails(true); + CHECK_FALSE(SolidSyslogStream_Open(stream, addr)); +} + +TEST(SolidSyslogTlsStream, BioNewFailureFreesBioMethodInline) +{ + OpenSslFake_SetBioNewFails(true); + SolidSyslogStream_Open(stream, addr); + LONGS_EQUAL(1, OpenSslFake_BioMethFreeCallCount()); + /* teardown re-Destroys safely — bioMethod already cleared */ +} + TEST(SolidSyslogTlsStream, SendReturnsTrueOnHappyPath) { SolidSyslogStream_Open(stream, addr); diff --git a/Tests/Support/OpenSslFake.c b/Tests/Support/OpenSslFake.c index b083a9bd..38a5220f 100644 --- a/Tests/Support/OpenSslFake.c +++ b/Tests/Support/OpenSslFake.c @@ -17,15 +17,34 @@ static char fakeCtxStorage; static char fakeMethodStorage; static char fakeSslStorage; static char fakeBioMethStorage; -static char fakeBioStorage; + +/* Pool of fake BIOs. Each slot is independent so callers can exercise + * multiple BIOs without aliasing. The address of `slot` is the BIO handle + * returned from BIO_new; `data` is the per-BIO storage that backs + * BIO_set_data / BIO_get_data. */ +typedef struct +{ + char slot; + void* data; +} FakeBio; + +enum +{ + FAKE_BIO_POOL_SIZE = 4 +}; + +static FakeBio fakeBios[FAKE_BIO_POOL_SIZE]; +static int fakeBioCount; /* SSL_CTX_new */ static int ctxNewCallCount; static const SSL_METHOD* lastCtxNewMethodArg; +static bool ctxNewFails; /* SSL_CTX_load_verify_locations */ static SSL_CTX* lastLoadVerifyLocationsCtxArg; static const char* lastCaBundlePath; +static bool loadVerifyLocationsFails; /* SSL_CTX_set_verify */ static SSL_CTX* lastSetVerifyCtxArg; @@ -34,10 +53,25 @@ static int lastVerifyMode; /* SSL_CTX_ctrl (SET_MIN_PROTO_VERSION) */ static SSL_CTX* lastSslCtxCtrlCtxArg; static long lastMinProtoVersion; +static bool minProtoVersionFails; + +/* SSL_CTX_set_cipher_list */ +static int setCipherListCallCount; +static SSL_CTX* lastSetCipherListCtxArg; +static const char* lastCipherList; +static bool setCipherListFails; /* SSL_new */ static int sslNewCallCount; static SSL_CTX* lastSslNewCtxArg; +static bool sslNewFails; + +/* BIO_meth_new */ +static bool bioMethNewFails; + +/* BIO_meth_free */ +static int bioMethFreeCallCount; +static BIO_METHOD* lastBioMethFreeArg; /* BIO_meth_set_read / BIO_meth_set_write */ static BIO_METHOD* lastBioMethSetReadMethodArg; @@ -51,6 +85,7 @@ static int (*lastBioWriteCallback)(BIO*, const char*, int); /* BIO_new */ static int bioNewCallCount; static const BIO_METHOD* lastBioNewMethodArg; +static bool bioNewFails; /* BIO_set_data / BIO_get_data */ static BIO* lastSetDataBioArg; @@ -66,10 +101,12 @@ static BIO* lastSetBioWriteBioArg; /* SSL_ctrl (SET_TLSEXT_HOSTNAME) */ static SSL* lastSslCtrlSslArg; static const char* lastSniHostname; +static bool sniHostnameFails; /* SSL_set1_host */ static SSL* lastSet1HostSslArg; static const char* lastSet1Host; +static bool set1HostFails; /* SSL_connect */ static int connectCallCount; @@ -109,14 +146,25 @@ void OpenSslFake_Reset(void) { ctxNewCallCount = 0; lastCtxNewMethodArg = NULL; + ctxNewFails = false; lastLoadVerifyLocationsCtxArg = NULL; lastCaBundlePath = NULL; + loadVerifyLocationsFails = false; lastSetVerifyCtxArg = NULL; lastVerifyMode = 0; lastSslCtxCtrlCtxArg = NULL; lastMinProtoVersion = 0; + minProtoVersionFails = false; + setCipherListCallCount = 0; + lastSetCipherListCtxArg = NULL; + lastCipherList = NULL; + setCipherListFails = false; sslNewCallCount = 0; lastSslNewCtxArg = NULL; + sslNewFails = false; + bioMethNewFails = false; + bioMethFreeCallCount = 0; + lastBioMethFreeArg = NULL; lastBioMethSetReadMethodArg = NULL; lastBioReadCallback = NULL; lastBioCtrlCallback = NULL; @@ -126,35 +174,43 @@ void OpenSslFake_Reset(void) lastBioWriteCallback = NULL; bioNewCallCount = 0; lastBioNewMethodArg = NULL; - lastSetDataBioArg = NULL; - lastSetDataArg = NULL; - lastGetDataBioArg = NULL; - setBioCallCount = 0; - lastSetBioSslArg = NULL; - lastSetBioReadBioArg = NULL; - lastSetBioWriteBioArg = NULL; - lastSslCtrlSslArg = NULL; - lastSniHostname = NULL; - lastSet1HostSslArg = NULL; - lastSet1Host = NULL; - connectCallCount = 0; - lastConnectSslArg = NULL; - connectFails = false; - writeCallCount = 0; - lastWriteSslArg = NULL; - lastWriteBuf = NULL; - lastWriteSize = 0; - writeFails = false; - sslReadCallCount = 0; - lastSslReadSslArg = NULL; - lastSslReadBuf = NULL; - lastSslReadSize = 0; - shutdownCallCount = 0; - lastShutdownSslArg = NULL; - freeCallCount = 0; - lastFreeSslArg = NULL; - ctxFreeCallCount = 0; - lastCtxFreeCtxArg = NULL; + bioNewFails = false; + fakeBioCount = 0; + for (int i = 0; i < FAKE_BIO_POOL_SIZE; i++) + { + fakeBios[i].data = NULL; + } + lastSetDataBioArg = NULL; + lastSetDataArg = NULL; + lastGetDataBioArg = NULL; + setBioCallCount = 0; + lastSetBioSslArg = NULL; + lastSetBioReadBioArg = NULL; + lastSetBioWriteBioArg = NULL; + lastSslCtrlSslArg = NULL; + lastSniHostname = NULL; + sniHostnameFails = false; + lastSet1HostSslArg = NULL; + lastSet1Host = NULL; + set1HostFails = false; + connectCallCount = 0; + lastConnectSslArg = NULL; + connectFails = false; + writeCallCount = 0; + lastWriteSslArg = NULL; + lastWriteBuf = NULL; + lastWriteSize = 0; + writeFails = false; + sslReadCallCount = 0; + lastSslReadSslArg = NULL; + lastSslReadBuf = NULL; + lastSslReadSize = 0; + shutdownCallCount = 0; + lastShutdownSslArg = NULL; + freeCallCount = 0; + lastFreeSslArg = NULL; + ctxFreeCallCount = 0; + lastCtxFreeCtxArg = NULL; } /* ------------------------------------------------------------------------- @@ -273,7 +329,7 @@ const BIO_METHOD* OpenSslFake_LastBioNewMethodArg(void) BIO* OpenSslFake_LastBioReturned(void) { - return (BIO*) &fakeBioStorage; + return fakeBioCount > 0 ? (BIO*) &fakeBios[fakeBioCount - 1].slot : NULL; } BIO* OpenSslFake_LastSetDataBioArg(void) @@ -428,7 +484,12 @@ SSL_CTX* SSL_CTX_new(const SSL_METHOD* method) { ctxNewCallCount++; lastCtxNewMethodArg = method; - return (SSL_CTX*) &fakeCtxStorage; + return ctxNewFails ? NULL : (SSL_CTX*) &fakeCtxStorage; +} + +void OpenSslFake_SetCtxNewFails(bool fails) +{ + ctxNewFails = fails; } // NOLINTNEXTLINE(bugprone-easily-swappable-parameters) -- signature fixed by OpenSSL API @@ -437,7 +498,12 @@ int SSL_CTX_load_verify_locations(SSL_CTX* ctx, const char* CAfile, const char* (void) CApath; lastLoadVerifyLocationsCtxArg = ctx; lastCaBundlePath = CAfile; - return 1; + return loadVerifyLocationsFails ? 0 : 1; +} + +void OpenSslFake_SetLoadVerifyLocationsFails(bool fails) +{ + loadVerifyLocationsFails = fails; } void SSL_CTX_set_verify(SSL_CTX* ctx, int mode, SSL_verify_cb verify_callback) @@ -457,22 +523,82 @@ long SSL_CTX_ctrl(SSL_CTX* ctx, int cmd, long larg, void* parg) if (cmd == SSL_CTRL_SET_MIN_PROTO_VERSION) { lastMinProtoVersion = larg; + return minProtoVersionFails ? 0 : 1; } return 1; } +void OpenSslFake_SetMinProtoVersionFails(bool fails) +{ + minProtoVersionFails = fails; +} + +int SSL_CTX_set_cipher_list(SSL_CTX* ctx, const char* str) +{ + setCipherListCallCount++; + lastSetCipherListCtxArg = ctx; + lastCipherList = str; + return setCipherListFails ? 0 : 1; +} + +void OpenSslFake_SetCipherListFails(bool fails) +{ + setCipherListFails = fails; +} + +int OpenSslFake_SetCipherListCallCount(void) +{ + return setCipherListCallCount; +} + +SSL_CTX* OpenSslFake_LastSetCipherListCtxArg(void) +{ + return lastSetCipherListCtxArg; +} + +const char* OpenSslFake_LastCipherList(void) +{ + return lastCipherList; +} + SSL* SSL_new(SSL_CTX* ctx) { sslNewCallCount++; lastSslNewCtxArg = ctx; - return (SSL*) &fakeSslStorage; + return sslNewFails ? NULL : (SSL*) &fakeSslStorage; +} + +void OpenSslFake_SetSslNewFails(bool fails) +{ + sslNewFails = fails; } BIO_METHOD* BIO_meth_new(int type, const char* name) { (void) type; (void) name; - return (BIO_METHOD*) &fakeBioMethStorage; + return bioMethNewFails ? NULL : (BIO_METHOD*) &fakeBioMethStorage; +} + +void OpenSslFake_SetBioMethNewFails(bool fails) +{ + bioMethNewFails = fails; +} + +void BIO_meth_free(BIO_METHOD* biom) +{ + bioMethFreeCallCount++; + lastBioMethFreeArg = biom; +} + +int OpenSslFake_BioMethFreeCallCount(void) +{ + return bioMethFreeCallCount; +} + +BIO_METHOD* OpenSslFake_LastBioMethFreeArg(void) +{ + return lastBioMethFreeArg; } int BIO_meth_set_read(BIO_METHOD* biom, int (*read)(BIO*, char*, int)) @@ -513,19 +639,45 @@ BIO* BIO_new(const BIO_METHOD* type) { bioNewCallCount++; lastBioNewMethodArg = type; - return (BIO*) &fakeBioStorage; + BIO* bio = NULL; + if (!bioNewFails && fakeBioCount < FAKE_BIO_POOL_SIZE) + { + bio = (BIO*) &fakeBios[fakeBioCount].slot; + fakeBioCount++; + } + return bio; +} + +void OpenSslFake_SetBioNewFails(bool fails) +{ + bioNewFails = fails; } void BIO_set_data(BIO* a, void* ptr) { lastSetDataBioArg = a; lastSetDataArg = ptr; + for (int i = 0; i < fakeBioCount; i++) + { + if ((BIO*) &fakeBios[i].slot == a) + { + fakeBios[i].data = ptr; + } + } } void* BIO_get_data(BIO* a) { lastGetDataBioArg = a; - return lastSetDataArg; + void* result = NULL; + for (int i = 0; i < fakeBioCount; i++) + { + if ((BIO*) &fakeBios[i].slot == a) + { + result = fakeBios[i].data; + } + } + return result; } void SSL_set_bio(SSL* ssl, BIO* rbio, BIO* wbio) @@ -546,15 +698,26 @@ long SSL_ctrl(SSL* ssl, int cmd, long larg, void* parg) if (cmd == SSL_CTRL_SET_TLSEXT_HOSTNAME) { lastSniHostname = (const char*) parg; + return sniHostnameFails ? 0 : 1; } return 1; } +void OpenSslFake_SetSniHostnameFails(bool fails) +{ + sniHostnameFails = fails; +} + int SSL_set1_host(SSL* ssl, const char* hostname) { lastSet1HostSslArg = ssl; lastSet1Host = hostname; - return 1; + return set1HostFails ? 0 : 1; +} + +void OpenSslFake_SetSet1HostFails(bool fails) +{ + set1HostFails = fails; } int SSL_connect(SSL* ssl) diff --git a/Tests/Support/OpenSslFake.h b/Tests/Support/OpenSslFake.h index dc1e5b96..ccf1a9b5 100644 --- a/Tests/Support/OpenSslFake.h +++ b/Tests/Support/OpenSslFake.h @@ -19,6 +19,15 @@ EXTERN_C_BEGIN /* Failure-mode switches */ void OpenSslFake_SetConnectFails(bool fails); void OpenSslFake_SetWriteFails(bool fails); + void OpenSslFake_SetSet1HostFails(bool fails); + void OpenSslFake_SetSniHostnameFails(bool fails); + void OpenSslFake_SetCtxNewFails(bool fails); + void OpenSslFake_SetSslNewFails(bool fails); + void OpenSslFake_SetLoadVerifyLocationsFails(bool fails); + void OpenSslFake_SetMinProtoVersionFails(bool fails); + void OpenSslFake_SetBioMethNewFails(bool fails); + void OpenSslFake_SetBioNewFails(bool fails); + void OpenSslFake_SetCipherListFails(bool fails); /* SSL_CTX_new */ int OpenSslFake_CtxNewCallCount(void); @@ -37,6 +46,11 @@ EXTERN_C_BEGIN struct ssl_ctx_st* OpenSslFake_LastSslCtxCtrlCtxArg(void); long OpenSslFake_LastMinProtoVersion(void); + /* SSL_CTX_set_cipher_list */ + int OpenSslFake_SetCipherListCallCount(void); + struct ssl_ctx_st* OpenSslFake_LastSetCipherListCtxArg(void); + const char* OpenSslFake_LastCipherList(void); + /* SSL_new */ int OpenSslFake_SslNewCallCount(void); struct ssl_ctx_st* OpenSslFake_LastSslNewCtxArg(void); @@ -45,6 +59,10 @@ EXTERN_C_BEGIN /* BIO_meth_new */ struct bio_method_st* OpenSslFake_LastBioMethReturned(void); + /* BIO_meth_free */ + int OpenSslFake_BioMethFreeCallCount(void); + struct bio_method_st* OpenSslFake_LastBioMethFreeArg(void); + /* BIO_meth_set_read */ struct bio_method_st* OpenSslFake_LastBioMethSetReadMethodArg(void); int (*OpenSslFake_LastBioReadCallback(void))(struct bio_st*, char*, int); diff --git a/docs/iec62443.md b/docs/iec62443.md index e60955dc..aa42122b 100644 --- a/docs/iec62443.md +++ b/docs/iec62443.md @@ -152,6 +152,35 @@ for SL4 extension but some components are not yet implemented. - Full RFC 5424 character compliance — PRINTUSASCII validation on header fields, UTF-8 safe truncation on message body +**TLS hardening (SL4 substrate in place).** `SolidSyslogTlsStream` — the +OpenSSL-backed client-side TLS substrate — ships with the SL4 controls that +depend on the client: + +- **Hostname verification.** `SolidSyslogTlsStreamConfig.serverName` is + forwarded to both SNI (`SSL_set_tlsext_host_name`) and the cert check + (`SSL_set1_host`). Both return values are checked; handshake is aborted on + any setup failure, preventing a silent "handshake succeeded without checking + the name" bypass. +- **Trust chain.** `caBundlePath` loads trust anchors via + `SSL_CTX_load_verify_locations`; `SSL_VERIFY_PEER` is pinned on the CTX. +- **TLS 1.2 floor.** `SSL_CTX_set_min_proto_version(TLS1_2_VERSION)` is + return-checked; Open fails if libssl refuses the floor. +- **Cipher pinning.** `SolidSyslogTlsStreamConfig.cipherList` is forwarded to + `SSL_CTX_set_cipher_list`. The library ships no baked-in list — the + SL4-appropriate cipher policy lives with the caller, which understands its + security profile. A reasonable starting point is + `"ECDHE+AESGCM:ECDHE+CHACHA20"` (TLS 1.2 AEAD with forward secrecy); tune to + match the libssl build on the target platform. +- **Resource lifecycle.** `Close`/`Destroy` release `SSL_CTX`, `SSL`, and + `BIO_METHOD` idempotently — no leaks on partial Open failure, no double + frees if both are called. + +Remaining E3 work for a full SL4 TLS posture: mutual TLS +([S03.09](https://github.com/DavidCozens/solid-syslog/issues/173)), cert +rotation ([S03.10](https://github.com/DavidCozens/solid-syslog/issues/174)), +and the formal doc promotion from Planned to Available +([S03.11](https://github.com/DavidCozens/solid-syslog/issues/175)). + ## Architecture for Security SolidSyslog's architecture directly supports IEC 62443-4-1 (secure development @@ -221,6 +250,6 @@ the collector can alert on missing sequence numbers without polling the device. | CR 2.10 Response to audit failures | `SolidSyslogStoreFullCallback`, halt policy | Callback on storage full, optional halt | | CR 2.11 Timestamps | `SolidSyslogClockFunction`, `SolidSyslogTimeQualitySd` | Injected clock with quality metadata | | CR 2.12 Non-repudiation | `SolidSyslogMetaSd` (sequenceId), `SolidSyslogCrc16Policy` | Sequence gaps detectable by SIEM | -| CR 3.9 Audit information protection | `SolidSyslogCrc16Policy` (SL3), TLS + encryption (SL4) | SL4: [E3](https://github.com/DavidCozens/solid-syslog/issues/5), [E17](https://github.com/DavidCozens/solid-syslog/issues/105) | +| CR 3.9 Audit information protection | `SolidSyslogCrc16Policy` (SL3), TLS + encryption (SL4) | SL4 TLS substrate hardened by [S03.08](https://github.com/DavidCozens/solid-syslog/issues/172) (hostname verify, TLS 1.2 floor, cipher pinning); remaining TLS work in [E3](https://github.com/DavidCozens/solid-syslog/issues/5); integrity/encryption at rest in [E17](https://github.com/DavidCozens/solid-syslog/issues/105) | | SR 6.1 Audit log accessibility | `SolidSyslog_Service`, sender vtable | Non-blocking Log, background Service | | SR 6.2 Continuous monitoring | TCP/TLS transport, store-and-forward | Assured delivery via replay on reconnect |