Improve TLS certificate loading, handling and validation #478
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
1. Improved Certificate Loading: - Now fails explicitly when no certificates can be loaded - Properly reports when CA certificates are unavailable 2. Enhanced Error Handling: - Added new error types for specific TLS/certificate validation issues - Improved error messages for better diagnostics 3. Better Certificate Validation: - Implemented certificate validation checks during connection establishment - Added basic validation of certificate data to ensure certificates aren't empty - Added proper error handling for invalid DNS names 4. Proper TLS Connection Validation: - Added explicit error handling for TLS connection failures - Added logging for certificate validation failures for easier debugging
cceckman-at-fastly
approved these changes
May 28, 2025
Contributor
cceckman-at-fastly
left a comment
cceckman-at-fastly
left a comment
There was a problem hiding this comment.
Patched with some syntactic changes - map_err and use of the ?, mostly - to cut out some of the error-handling boilerplate.
@jedisct1 If this looks good to you, it's good to merge IMO!
lib/src/upstream.rs
Outdated
| }; | ||
|
|
||
| let remote_addr = tcp.peer_addr()?; | ||
| let remote_addr = match tcp.peer_addr() { |
Contributor
There was a problem hiding this comment.
I don't think this hunk does anything-
The ? operator applies one or more from/into conversions between the error types. crate::error::Error implements From<std::io::Error> (it's hidden in the little #[from] note on the IoError declaration), so this is just doing (more verbosely) what ? does.
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.
Improved Certificate Loading:
Better Certificate Validation:
Proper TLS Connection Validation: