Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Restrict public keys prior to TLS handshake #3501

Merged
merged 4 commits into from
Oct 31, 2024
Merged

Conversation

yacovm
Copy link
Contributor

@yacovm yacovm commented Oct 27, 2024

Why this should be merged

This commit further restricts the public keys allowed in the TLS certificates of clients:

- No ed25519
- For ECDSA, only P256 is supported

How this works

Expands ValidateCertificate (formally ValidateRSACertificate) by adding extra checks

How this was tested

Added some test cases in the corresponding unit test.

Need to be documented in RELEASES.md?

I don't think so, because anyone that uses avalanchego from the official release shouldn't have non P256 EC keys.

This commit makes validation of TLS certificates with either too big RSA keys, or the wrong exponent, fail as soon as the remote node presents its TLS certificate, in contrast to after the TLS handshake.

Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
@yacovm yacovm self-assigned this Oct 28, 2024
@yacovm yacovm added the enhancement New feature or request label Oct 28, 2024
network/peer/tls_config.go Outdated Show resolved Hide resolved
network/peer/tls_config_test.go Outdated Show resolved Hide resolved
network/peer/tls_config_test.go Show resolved Hide resolved
network/peer/upgrader_test.go Outdated Show resolved Hide resolved
network/peer/upgrader_test.go Outdated Show resolved Hide resolved
network/peer/tls_config_test.go Outdated Show resolved Hide resolved
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
This commit further restricts the public keys allowed in the TLS certificates of clients:

- No ed25519
- For ECDSA, only P256 is supported

Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
@StephenButtolph StephenButtolph added this to the v1.11.13 milestone Oct 30, 2024
@StephenButtolph StephenButtolph changed the title Further restrict public keys in TLS handshake Restrict public keys prior to TLS handshake Oct 30, 2024
@marun marun added this pull request to the merge queue Oct 31, 2024
Merged via the queue into ava-labs:master with commit 739544e Oct 31, 2024
23 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants