ssl_fork_server RNG testing - #10861
Open
gilles-peskine-arm wants to merge 13 commits into
Open
Conversation
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
`ssl_fork_server` reseeds its RNG, so in a build with a nonvolatile seed, it re-reads the seed file midway. If the client also rewrites the seed file while the server is running, that causes the test conditions to not reflect real-world conditions for the server. Fix this by running `ssl_fork_server` in its own directory when we're asserting properties of the random generator. This was a relatively easy way to separate the client seed file from the server seed file, since the seed file name `"seedfile"` is not configurable at runtime. This mostly matters for builds with no actual entropy source, only an NV seed. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Assert that when two clients connect to ssl_fork_server, they see different random values from the server. This validates that each child process has its own random generator state. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Assert that when two clients connect to ssl_fork_server, they see different ephemeral public keys from the server. This validates that each child process has its own random generator state. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Run some of `ssl-opt.sh` on builds with only an NV seed as the entropy source. There isn't much impact outside of the crypto unit tests, but do test that it works in a "real" program. In particular, run `ssl_fork_server` which has a peculiar challenge since its RNG state is replicated. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
…ient random values Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Fix a bug introduced in "Fix temporary directory left behind on normal exit" that caused the exit code of `ssl-opt.sh` to be 1 on success. Also, when exiting on a trapped signal, re-raise the signal rather than exiting with status 1. Also, on failure, where we report the number of failures through the exit code, clamp the exit code to 125 to avoid confusion with standard values. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm
added
needs-review
gilles-peskine-arm
added
component-tls
size-s
github-project-automation
Bot
moved this to In Development
in Roadmap pull requests (new board)
This was referenced Aug 6, 2026
bensze01
previously approved these changes
Aug 7, 2026
bensze01
left a comment
bensze01
left a comment
Contributor
There was a problem hiding this comment.
LGTM - faithful forward port minus the differences discussed in the description.
The test case passes even if the two clients have different RNG seeds, but in that case, it doesn't do what it claims, but instead it duplicates the "2 successive clients, TLS 1.3" test case above. So run "2 successive clients with same seed, TLS 1.3" only when there is no entropy, like "2 successive clients with same seed, TLS 1.3". Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
bjwtaylor
previously approved these changes
Aug 13, 2026
| # Check that the library seems to refer to the seedfile, but not to | ||
| # platform entropy sources. | ||
| grep seedfile tf-psa-crypto/platform/CMakeFiles/platform.dir/platform.c.o | ||
| not grep getrandom tf-psa-crypto/drivers/builtin/CMakeFiles/builtin.dir/src/entropy*.o platform/CMakeFiles/platform.dir/platform*.o |
Contributor
There was a problem hiding this comment.
So, I may have not understood the operation of this. However as far as I can tell platform/CMakeFiles/platform.dir/platform*.o doesn't exist. In which case these chacks fail open? Or have I missed something somewhere?
Contributor
There was a problem hiding this comment.
Accidentally approved instead of commented, so have re requested the review. Let me know what you think on the above?
Contributor
Author
There was a problem hiding this comment.
In which case these chacks fail open?
Yes, they do. We've had a fix waiting for review for a while: Mbed-TLS/mbedtls-framework#268 . And it keeps breaking when I re-run the CI because we add new failures.
I'll fix it here.
bjwtaylor
self-requested a review
August 13, 2026 14:22
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
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
3 participants
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 that
ssl_fork_serverhas a different RNG state in each client. Fixes #10664.Forward port of #10666. Differences:
component_test_entropy_nv_seed_onlyalready exists in crypto but we now add it to mbedtls so that we can runssl-opt.sh. Also, in 4.x, there's only one variant becauseMBEDTLS_USE_PSA_CRYPTOis always enabled.PR checklist