Test mbedtls_ssl_conf_own_cert #10217
Draft
+2,081
−1,430
Conversation
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
Regexp replacement then `code_style.py --fix`. Signed-off-by: Gilles Peskine <[email protected]>
No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
A future commit will test it on its own instead of as part of every positive test. Signed-off-by: Gilles Peskine <[email protected]>
No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
Link the ciphersuite list that's passed to mbedtls_ssl_conf_ciphersuites(), and needs to survive in memory as long as the configuration object is live, in the endpoint structure. This way it doesn't have to be a local variable in mbedtls_test_ssl_do_handshake_with_endpoints(). Signed-off-by: Gilles Peskine <[email protected]>
This reflects the fact that the library will not modify the list, and allows the list to be read from a const buffer. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
This is a step towards making mbedtls_test_ssl_endpoint_init() and mbedtls_test_ssl_endpoint_free() more self-contained. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
Create an auxiliary function to perform some endpoint setup that involves both the client and the server. This is only needed for DTLS. The code that will eventually be in this function is currently mostly in mbedtls_test_ssl_endpoint_init(). This commit adds the new function to the control flow; a subsequent commit will move the relevant code. Signed-off-by: Gilles Peskine <[email protected]>
This allows mbedtls_test_ssl_endpoint_init() to no longer interact with the other endpoint. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
The DTLS context and the queues now conveyed inside the endpoint object. Remove the unused parameters. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
This is made possible by the endpoint init simplification. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
This will facilitate future refactoring that breaks out code into auxiliary functions. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
Stop calling mbedtls_test_set_step() in mbedtls_test_ssl_perform_handshake(). This leaves the caller free to use the test step as they wish. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
Split mbedtls_test_ssl_perform_connection() out of mbedtls_test_ssl_perform_handshake(). No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
Applying SSL configuration settings recorded in the `mbedtls_test_handshake_test_options` structure to an `mbedtls_test_ssl_endpoint` object was split between `mbedtls_test_ssl_endpoint_init()` and `mbedtls_test_ssl_perform_handshake()`. This was surprising, and made it harder to use `mbedtls_test_ssl_endpoint_init()` for custom behavior. It also meant some code duplication in `mbedtls_test_ssl_perform_handshake()`. Move most configuration setup from `mbedtls_test_ssl_perform_handshake()` to `mbedtls_test_ssl_endpoint_init()`. This changes the behavior in two ways: * `mbedtls_test_ssl_endpoint_init()` now takes some options into account that it previously ignored. This is ok because we don't set these options in any of the existing tests. * When calling `mbedtls_test_ssl_perform_handshake()`, some SSL configuration settings are now set (calls to `mbedtls_ssl_conf_xxx()`) before the call to `mbedtls_ssl_setup()` instead of after. This should be ok since it is forbidden to change the configuration after `mbedtls_ssl_setup()`, although the previous test code was getting away with it. This commit does not move all configuration before `mbedtls_ssl_setup()`, that would be out of scope of the current series of patches. Thus there are some internal behavior changes, but they should not affect any relevant aspect of the tests' behavior. Signed-off-by: Gilles Peskine <[email protected]>
There was a discrepancy between how `mbedtls_test_ssl_endpoint_init()` and `mbedtls_test_ssl_perform_handshake()` handled client authentication: `mbedtls_test_ssl_endpoint_init()` defaulted to `MBEDTLS_SSL_VERIFY_REQUIRED` on both sides, whereas `mbedtls_test_ssl_perform_handshake()` obeyed `options->srv_auth_mode` which defaulted to no verification of the client certificate. Make this more uniform. Now `mbedtls_test_ssl_endpoint_init()` obeys `options->srv_auth_mode` on servers (still forcing verification on clients, which is the library default anyway). Also, `options->srv_auth_mode` is now enabled by default. Thus: * Tests that call `mbedtls_test_ssl_perform_handshake()` now perform client certificate verification, unless they disable it explicitly. * Tests that call `mbedtls_test_ssl_endpoint_init()` on a server are unchanged. (They would change if they were setting `options->srv_auth_mode` explicitly, which previously was ignored, but no test function did this.) This means that a few test functions now perform client certificate verification whereas they previously don't. This is harmless except in `handshake_ciphersuite_select`, where one test case `Handshake, select ECDH-RSA-WITH-AES-256-CBC-SHA384, opaque` fails with client authentication because the test code doesn't deal with the weirdness of static ECDH correctly with respect to client authentication. So keep the previous behavior in `handshake_ciphersuite_select`, by explicitly turning off client authentication. Signed-off-by: Gilles Peskine <[email protected]>
No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
In `mbedtls_test_ssl_endpoint_init()`, don't change the SSL configuration object (`mbedtls_ssl_config`) after setting up an SSL context by calling `mbedtls_ssl_setup()`. This works in practice, but is officially forbidden. No intended behavior change. The test code calls the library slightly differently, but this shouldn't make any difference in practice. If it does make a difference, it fixes a bug in the test code. Signed-off-by: Gilles Peskine <[email protected]>
This will allow splitting the configuration and setup stages of `mbedtls_test_ssl_endpoint_init()`, while still checking that the value is carried over from the configuration to the session context. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
Split `mbedtls_test_ssl_endpoint_init()` into two separate stages: constructing the SSL configuration, and setting up an SSL session context with that configuration. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
This will allow more direct intervention on the server key(s). Signed-off-by: Gilles Peskine <[email protected]>
Track the need to free a PSA key independently of its usage in a PK context. This fixes a memory leak if the call to `mbedtls_pk_import_into_psa()` in `mbedtls_test_ssl_endpoint_certificate_init()` fails. Other than that, no behavior change. Signed-off-by: Gilles Peskine <[email protected]>
Expose `mbedtls_test_cert_load_rsa()` and `mbedtls_test_cert_load_ecc()` as separate functions. No behavior change. Signed-off-by: Gilles Peskine <[email protected]>
If `options->pk_alg == MBEDTLS_PK_NONE`, don't load a key and certificate into the SSL endpoint. This affects `mbedtls_test_ssl_endpoint_certificate_init()`, `mbedtls_test_ssl_endpoint_init_conf()`, `mbedtls_test_ssl_endpoint_init()`, `mbedtls_test_ssl_perform_handshake()`. This does not affect any existing test case since they all either explicitly pass one of `MBEDTLS_PK_RSA` or `MBEDTLS_PK_ECDSA`, or rely on the default which is `MBEDTLS_PK_RSA`. Signed-off-by: Gilles Peskine <[email protected]>
Rework how debug logs are handled in SSL unit tests. The big change is that test endpoints now automatically set up the new function `mbedtls_test_ssl_debug_handler()` a debug callback, and they set the debug level to high so that the handler is always called. Test functions no longer call `mbedtls_ssl_conf_dbg()` or `mbedtls_debug_set_threshold()`. As part of this change, the interface for setting a pattern that must be present in the logs has changed, and is now a bit simpler: just set `options.debug_threshold`, `options.cli_log_pattern` and `options.srv_log_pattern`. The endpoint structure now owns its own counter. This refactoring will make it a lot easier to enable debug logs temporarily when debugging tests. This commit changes how the test helpers work, but there is no intended change to what the test functions actually test for. Signed-off-by: Gilles Peskine <[email protected]>
Use a runtime variable rather than a compile-time edit to determine whether to print SSL debug logs to stdout. This makes it easier to selectively enable it, and allows controlling the level. Also fix bitrot in the code that was guarded by `#if 0`. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Test RSA keys of various kinds: built-in or PSA, with different policies. Test `mbedtls_ssl_conf_own_cert(ssl, NULL, NULL)`. Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
added
needs-work
component-tls
size-s
github-project-automation
bot
moved this to In Development
in Roadmap pull requests (new board)
Comment out test cases failing due to a bug in TLS 1.2: it calls mbedtls_pk_sign() which uses the key's primary algorithm rather than the algorithm required by the protocol. Mbed-TLS#10208 Signed-off-by: Gilles Peskine <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Test
mbedtls_ssl_conf_own_cert(). This also tests how various ways of constructing a private key in ambedtls_pk_contextare understood by TLS. Resolves #10160.Status: work in progress. Expect rebases! At the moment the tests are failing on #10208.
Preceding PR: #10197
PR checklist