fix: Increase received signature scheme limit #4544
Merged
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.
Resolved issues:
Temporary workaround for #4543
Description of changes:
Currently, the maximum number of received signature schemes is set to 64:
s2n-tls/tls/s2n_tls_parameters.h
Line 160 in eb168f2
Some clients have been found to send more than 64 signature schemes, which causes the connection to fail (see open-quantum-safe/oqs-provider#399). This PR increases the maximum number of received signature schemes, allowing these clients to connect successfully.
Note that the reason a limit exists at all is to allocate a separate buffer for received signature schemes, which isn't actually necessary. As a long-term solution for this issue, we should read the signature schemes straight from the TLS message rather than copy them into a separate buffer. See #4543.
Call-outs:
Lindsay also had the idea to remove the separate client and server signature scheme buffers in order to avoid increasing the memory used per connection, so this PR also does this refactor.
Testing:
A new test was added to ensure that the new limit is respected.
I reproduced this issue with the oqs-provider client connecting to an s2n-tls server, and confirmed that this fix resolved the issue:
Test results
Before fix:
After fix:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.