Julien00859/config certchain truststore#78
Draft
Julien00859 wants to merge 2 commits intotrailofbits:mainfrom
Draft
Julien00859/config certchain truststore#78Julien00859 wants to merge 2 commits intotrailofbits:mainfrom
Julien00859 wants to merge 2 commits intotrailofbits:mainfrom
Conversation
TLS mandates[^1][^4] that the server send both a Certificate (with a non-empty list of certificates) and a CertificateVerify message during the handshake unless the client and server agree to use (TLS 1.3) pre-shared keys or (TLS 1.2) anonymous sessions. The same isn't true for the client, as the server may not require the client to authenticate itself, or at least not via TLS. It means that running a TLS server that lack an appropriate certificate chain is most likely a configuration error, and the current default None value makes it more likely that developpers will misconfigure their server. TLS 1.3 Pre-shared keys are used in two cases: 1. Session resumption. When a recent (<10 min) handshake using certificates was successful, the server can offer a session token to the client. The client can use that session token to bootstrap another secure TLS connection and skip the (lengty) certificate verification step. 2. Pre-shared key. When a secret value was exchanged between the two peers out of band (e.g. IRL handshake) and is used to bootstrap the TLS connection. The first case require that a prior handshake using certificates was succesful. Hence a certificate chain was required for establishing that prior connection. The second case can work without a certificate chain, but at the cost of loosing forward secrecy[^2]. At the moment tlslib doesn't provide a way to configure a pre-shared secret, a subclass might do it, and so it might override `__init__` to loosen the exception. Note that it is also possible to setup a dummy self-signed certificate chain, for the sake of instantiating the configuration object. TLS 1.2 anonymous session are insecure[^5] and should not be used. They use the `TLS_(EC)?DH_anon_.*` ciphers[^6]. We could had listed those and only raise the exception when the configured ciphers only list those anon ciphers. We decided it was not worth it. [^1]: https://datatracker.ietf.org/doc/html/rfc8446#section-4.4.2 [^2]: https://datatracker.ietf.org/doc/html/rfc8446#section-2.2 [^3]: https://datatracker.ietf.org/doc/html/rfc7250 [^4]: https://datatracker.ietf.org/doc/html/rfc5246#section-7.4.2 [^5]: https://datatracker.ietf.org/doc/html/rfc5246#appendix-F.1.1.1 [^6]: https://ciphersuite.info/search/?singlepage=true&q=_anon_ Closes trailofbits#72
Instantiating a TLSServerConfiguration with `truststore=None` means that client-authentication is disabled. Instantiating a TLSClientConfiguration with the same `truststore=None` means instead to use the system trust store. Instantiating a TLSClientConfiguration with `truststore=TrustStore.system()` is another (more explicit) way to use the system trust store. We believe that there should only be one way to instantiate a client configuration so it uses the system trust store, and that `None` is a bad value as it has a totally different meaning in the server configuration. We used the opportunity of this work to introduce `is_system()`: a new property on `TrustStore` that should be called to test if a given truststore object is the system one. This way the implementation (all three attributes `_buffer`, `_path` and `_id` set `None`) becomes an implementation detail.
Julien00859
commented
Jan 28, 2026
| """ | ||
| return cls() | ||
|
|
||
| def is_system(self): |
Contributor
Author
There was a problem hiding this comment.
and return value annotation
Comment on lines
+382
to
+385
| if not certificate_chain: | ||
| e = "certificate_chain cannot be empty" | ||
| raise ValueError(e) | ||
|
|
Contributor
Author
multiple tests failing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following our discussions on the forum.