Skip to content

Commit 5cd13fa

Browse files
committed
fix: continue when the Mbed TLS stream cannot install the client credential
Reverses the half of #785 that failed Open when mbedtls_ssl_conf_own_cert returned an error. That drew the line between a configuration mistake and a resource fault; the contract draws it between our own credential and the peer's identity, so both now report and continue. The same fault is indistinguishable on the OpenSSL side - a failed PEM load and a failed allocation surface as one return code - so leaving this one blocking would have left the two adapters differing on an event neither can tell apart. Continuing is safe: ssl_append_key_cert returns MBEDTLS_ERR_SSL_ALLOC_FAILED before the key_cert node is appended (library/ssl_tls.c, 3.6.2), so nothing is installed and the connection continues server-authenticated, exactly as the OpenSSL adapter's does. ApplyTlsPolicy returns void again and leaves Open's chain. The severity moves with it, to the WARNING and CAT_BAD_CONFIG that docs/error-severity.md gives a component that was built and is delivering. Rating a resource fault BAD_CONFIG is the one cost of collapsing the two paths, and the detail code still separates them for a handler that wants to retry one and not the other. docs/tls.md states the rule both adapters now share, and its client-credential obligation covers the third failure mode rather than two. The divergence note comes off the OpenSSL platform page, and its setup page no longer says a half-supplied credential is rejected at Open. Part of #782.
1 parent 8d50c3c commit 5cd13fa

6 files changed

Lines changed: 49 additions & 49 deletions

File tree

Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ static inline uint32_t MbedTlsStream_ResolveHandshakeTimeoutMs(struct SolidSyslo
3636
static inline struct SolidSyslogMbedTlsStream* MbedTlsStream_SelfFromBase(struct SolidSyslogStream* base);
3737
static inline bool MbedTlsStream_Open(struct SolidSyslogStream* base, const struct SolidSyslogAddress* addr);
3838
static inline bool MbedTlsStream_ApplySslConfigDefaults(struct SolidSyslogMbedTlsStream* self);
39-
static inline bool MbedTlsStream_ApplyTlsPolicy(struct SolidSyslogMbedTlsStream* self);
39+
static inline void MbedTlsStream_ApplyTlsPolicy(struct SolidSyslogMbedTlsStream* self);
4040
static inline bool MbedTlsStream_HasClientCredential(const struct SolidSyslogMbedTlsStreamConfig* config);
4141
static inline bool MbedTlsStream_HasHalfOfClientCredential(const struct SolidSyslogMbedTlsStreamConfig* config);
4242
static inline bool MbedTlsStream_BindContextToConfig(struct SolidSyslogMbedTlsStream* self);
@@ -133,8 +133,8 @@ static inline bool MbedTlsStream_Open(struct SolidSyslogStream* base, const stru
133133
bool ok = SolidSyslogStream_Open(self->Config.Transport, addr) && MbedTlsStream_ApplySslConfigDefaults(self);
134134
if (ok)
135135
{
136-
ok = MbedTlsStream_ApplyTlsPolicy(self) && MbedTlsStream_BindContextToConfig(self) &&
137-
MbedTlsStream_ConfigureExpectedHostname(self);
136+
MbedTlsStream_ApplyTlsPolicy(self);
137+
ok = MbedTlsStream_BindContextToConfig(self) && MbedTlsStream_ConfigureExpectedHostname(self);
138138
}
139139
if (ok)
140140
{
@@ -169,9 +169,13 @@ static inline bool MbedTlsStream_ApplySslConfigDefaults(struct SolidSyslogMbedTl
169169

170170
/* TLS policy owned by the library - set per-ssl_config so it cannot leak
171171
* into the integrator's other ssl_configs (per coexistence contract). */
172-
static inline bool MbedTlsStream_ApplyTlsPolicy(struct SolidSyslogMbedTlsStream* self)
172+
/* No fault in our own credential stops delivery: the collector is the
173+
* enforcement point for it, and one that requires a client certificate refuses
174+
* the handshake anyway. Every failure here leaves nothing installed, so the
175+
* connection continues server-authenticated rather than half-presenting a
176+
* credential. */
177+
static inline void MbedTlsStream_ApplyTlsPolicy(struct SolidSyslogMbedTlsStream* self)
173178
{
174-
bool ok = true;
175179
mbedtls_ssl_conf_authmode(&self->SslConfig, MBEDTLS_SSL_VERIFY_REQUIRED);
176180
/* Pin the floor at TLS 1.2 rather than inheriting MBEDTLS_SSL_PRESET_DEFAULT,
177181
* which can negotiate down to TLS 1.0/1.1 on permissive integrator builds.
@@ -184,15 +188,13 @@ static inline bool MbedTlsStream_ApplyTlsPolicy(struct SolidSyslogMbedTlsStream*
184188
mbedtls_ssl_conf_rng(&self->SslConfig, mbedtls_ctr_drbg_random, self->Config.Rng);
185189
if (MbedTlsStream_HasClientCredential(&self->Config))
186190
{
187-
ok = mbedtls_ssl_conf_own_cert(&self->SslConfig, self->Config.ClientCertChain, self->Config.ClientKey) == 0;
188-
if (!ok)
191+
/* Only MBEDTLS_ERR_SSL_ALLOC_FAILED, which returns before the key_cert
192+
* node is appended, so nothing is left half-configured. */
193+
if (mbedtls_ssl_conf_own_cert(&self->SslConfig, self->Config.ClientCertChain, self->Config.ClientKey) != 0)
189194
{
190-
/* Only MBEDTLS_ERR_SSL_ALLOC_FAILED. Failing Open rather than
191-
* connecting without the certificate: a device configured for
192-
* mutual TLS must not present itself as one that was not. */
193195
MbedTlsStream_Report(
194-
SOLIDSYSLOG_SEVERITY_ERROR,
195-
SOLIDSYSLOG_CAT_TLS_STREAM_INIT_FAILED,
196+
SOLIDSYSLOG_SEVERITY_WARNING,
197+
SOLIDSYSLOG_CAT_BAD_CONFIG,
196198
SOLIDSYSLOG_MBEDTLS_STREAM_ERROR_CLIENT_CREDENTIAL_NOT_INSTALLED
197199
);
198200
}
@@ -209,7 +211,6 @@ static inline bool MbedTlsStream_ApplyTlsPolicy(struct SolidSyslogMbedTlsStream*
209211
{
210212
/* Neither supplied - server-authenticated TLS is the deliberate case. */
211213
}
212-
return ok;
213214
}
214215

215216
static inline bool MbedTlsStream_HasClientCredential(const struct SolidSyslogMbedTlsStreamConfig* config)

Tests/MbedTls/SolidSyslogMbedTlsStreamTest.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -888,22 +888,23 @@ TEST(SolidSyslogMbedTlsStream, OpenReportsIncompleteClientCredentialWhenClientCe
888888
CHECK_INCOMPLETE_CREDENTIAL_REPORTED();
889889
}
890890

891-
TEST(SolidSyslogMbedTlsStream, OpenFailsAndReportsWhenClientCredentialCannotBeInstalled)
892-
891+
TEST(SolidSyslogMbedTlsStream, OpenReportsClientCredentialNotInstalledAndStillConnects)
893892
{
894893
/* Both halves supplied, but mbedTLS cannot take them - the only documented
895-
* failure is MBEDTLS_ERR_SSL_ALLOC_FAILED. Discarding it is the silent
896-
* downgrade to server-authenticated TLS this stops. */
894+
* failure is MBEDTLS_ERR_SSL_ALLOC_FAILED, which returns before anything is
895+
* appended to the config. Nothing is presented, so the connection continues
896+
* server-authenticated and the collector decides whether to accept it. */
897897
static mbedtls_x509_crt clientCertMarker;
898898
static mbedtls_pk_context clientKeyMarker;
899899
WireClientCredential(&clientCertMarker, &clientKeyMarker);
900900
MbedTlsFake_SetSslConfOwnCertReturn(MBEDTLS_ERR_SSL_ALLOC_FAILED);
901901

902-
CHECK_FALSE(SolidSyslogStream_Open(handle, addr));
902+
CHECK_TRUE(SolidSyslogStream_Open(handle, addr));
903903

904-
CHECK_OPEN_UNWOUND_WITH_ERROR(
905-
transport,
906-
SOLIDSYSLOG_CAT_TLS_STREAM_INIT_FAILED,
904+
CHECK_ERROR_REPORTED_ONCE(
905+
SOLIDSYSLOG_SEVERITY_WARNING,
906+
&MbedTlsStreamErrorSource,
907+
SOLIDSYSLOG_CAT_BAD_CONFIG,
907908
SOLIDSYSLOG_MBEDTLS_STREAM_ERROR_CLIENT_CREDENTIAL_NOT_INSTALLED
908909
);
909910
}

docs/platforms/openssl/index.md

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,6 @@ an encrypted store has to be written to a readable file before this adapter can
5959
use it. Tracked under
6060
[E39](https://github.com/cososo-ltd/solid-syslog/issues/782).
6161

62-
### A half-supplied client credential stops delivery
63-
64-
A certificate without its key, or a key without its certificate, is rejected when
65-
the stream opens, so nothing is delivered until the configuration is corrected.
66-
The contract asks for it to be reported with delivery continuing, on the grounds
67-
that the collector is the enforcement point for our own credential.
68-
69-
This adapter is stricter than the contract rather than weaker, and the stricter
70-
behaviour is safe. Tracked as
71-
[#734](https://github.com/cososo-ltd/solid-syslog/issues/734).
72-
7362
### The cipher policy does not bind a TLS 1.3 connection
7463

7564
The cipher list is passed to OpenSSL unchanged and pins nothing of the library's

docs/platforms/openssl/setup.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@ itself, and this page is the wiring.
99
## What you need
1010

1111
OpenSSL 3.0 or later on the include and link path, and a platform supplying the
12-
TCP stream underneath the [capability matrix](../index.md) shows which fill
12+
TCP stream underneath - the [capability matrix](../index.md) shows which fill
1313
that role.
1414

1515
```cmake
1616
set(SOLIDSYSLOG_PLATFORMS "OpenSsl;<Network>")
1717
```
1818

1919
`<Network>` is whichever platform the [capability matrix](../index.md) says
20-
fills that role on your target see
20+
fills that role on your target - see
2121
[naming your platforms](../../build-integration.md#cmake) for how the list is
2222
read.
2323

@@ -48,7 +48,7 @@ struct SolidSyslogStream* transport = CreateTcpStream();
4848
static struct SolidSyslogOpenSslStreamConfig tlsConfig;
4949
tlsConfig = (struct SolidSyslogOpenSslStreamConfig) {0};
5050
tlsConfig.Transport = transport;
51-
tlsConfig.Sleep = MySleep; /* required no fallback */
51+
tlsConfig.Sleep = MySleep; /* required - no fallback */
5252
tlsConfig.CaBundlePath = "/etc/ssl/collector-ca.pem";
5353
tlsConfig.ServerName = "collector.example.net";
5454

@@ -57,15 +57,15 @@ struct SolidSyslogStream* tls = SolidSyslogOpenSslStream_Create(&tlsConfig);
5757
5858
Zero-initialise the config before filling it.
5959
60-
For mutual TLS, add the client credential both fields or neither, since
61-
supplying one without the other is rejected at `Open`:
60+
For mutual TLS, add the client credential - both fields, since one without the
61+
other is reported and leaves the connection server-authenticated:
6262
6363
```c
6464
tlsConfig.ClientCertChainPath = "/etc/ssl/device-chain.pem";
6565
tlsConfig.ClientKeyPath = "/etc/ssl/device-key.pem";
6666
```
6767

68-
Then the sender, unchanged from the plain-TCP case it sees a Stream and does
68+
Then the sender, unchanged from the plain-TCP case - it sees a Stream and does
6969
not know or care that it is a TLS one:
7070

7171
```c
@@ -85,5 +85,5 @@ created.
8585
8686
Failures report through the error handler rather than silently. Install one
8787
before you start, and read [error severity](../../error-severity.md) for what
88-
each level is telling you a `CRITICAL` at create time means the stream fell
88+
each level is telling you - a `CRITICAL` at create time means the stream fell
8989
back to the Null object, and nothing will be delivered.

docs/tls.md

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ Where the key must not be in application memory at all, that is a property of th
211211
credential source rather than of the window: a source backed by a secure element
212212
or a hardware key store never hands the key over in the first place.
213213

214-
### Report a partially configured client credential
214+
### Report a client credential that will not be presented
215215

216216
Mutual TLS is all-or-nothing: a certificate without its key, or a key without its
217217
certificate, is a configuration error and is reported as one. It is never
@@ -225,10 +225,19 @@ it is reported at the same point. Left to the handshake, it comes back as a
225225
rejection from the collector, which sends the integrator looking at the collector
226226
for a fault that is on the device.
227227

228-
Delivery continues. The receiver is the enforcement point for our credential - a
229-
collector that requires a client certificate will refuse the handshake, and one
230-
that does not was never going to check. Blocking here would deny the audit trail
231-
without changing what the collector decides.
228+
A credential the TLS library will not take is reported on the same terms: a file
229+
that does not load, memory it cannot allocate. The cause differs and the reports
230+
distinguish it where the library does, but the consequence is the one that
231+
matters - what the integrator configured is not in force.
232+
233+
Delivery continues in every one of those cases. The receiver is the enforcement
234+
point for our credential - a collector that requires a client certificate will
235+
refuse the handshake, and one that does not was never going to check. Blocking
236+
here would deny the audit trail without changing what the collector decides.
237+
238+
The rule stops at our own credential, and the line is the one drawn under
239+
*Delivery is preferred to silence*: a fault in the material we present never
240+
stops delivery, and a failed check on the peer always does.
232241

233242
### Permit the cryptographic level to be chosen
234243

misra_suppressions.txt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ misra-c2012-11.3:Platform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256Policy.c:83
5959
misra-c2012-11.3:Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicy.c:85
6060
misra-c2012-11.3:Platform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicy.c:89
6161
misra-c2012-11.3:Platform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256Policy.c:84
62-
misra-c2012-11.3:Platform/OpenSsl/Source/SolidSyslogOpenSslStream.c:93
62+
misra-c2012-11.3:Platform/OpenSsl/Source/SolidSyslogOpenSslStream.c:99
6363
misra-c2012-11.3:Platform/Posix/Source/SolidSyslogPosixAddress.c:18
6464
misra-c2012-11.3:Platform/Posix/Source/SolidSyslogPosixAddressPrivate.h:27
6565
misra-c2012-11.3:Platform/Posix/Source/SolidSyslogPosixAddressPrivate.h:32
@@ -86,8 +86,8 @@ misra-c2012-11.5:Platform/LwipRaw/Source/SolidSyslogLwipRawDnsResolver.c:158
8686
misra-c2012-11.5:Platform/LwipRaw/Source/SolidSyslogLwipRawDnsResolver.c:220
8787
misra-c2012-11.5:Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c:151
8888
misra-c2012-11.5:Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c:159
89-
misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:344
90-
misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:356
89+
misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:345
90+
misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:357
9191

9292
# D.003 — Rule 5.7: repeating struct tags (no-typedef-struct convention)
9393
# See docs/misra-deviations.md#d003
@@ -196,8 +196,8 @@ misra-c2012-8.9:Core/Source/SolidSyslogFileBlockDevice.c:24
196196

197197
# D.013 — Rule 11.5: void* <-> a byte pointer at third-party byte-buffer API boundaries
198198
# See docs/misra-deviations.md#d013
199-
misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:381
200-
misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:399
199+
misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:382
200+
misra-c2012-11.5:Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c:400
201201
misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockDatagram.c:146
202202
misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:358
203203
misra-c2012-11.5:Platform/Windows/Source/SolidSyslogWinsockTcpStream.c:378

0 commit comments

Comments
 (0)