V1.5 TLS Patches#7898
Merged
Merged
Conversation
…kkadotnet#7891) * Improve TLS/SSL certificate error messages during handshake failures (akkadotnet#7890) Added TlsErrorMessageBuilder helper class to provide human-readable error messages for TLS certificate validation failures. Enhanced error messages now include: - Detailed SSL policy error interpretations - X509 chain status diagnostics with actionable suggestions - Certificate details (subject, issuer, thumbprint, validity dates) - Role-specific troubleshooting guidance (client vs server) Updated certificate validation callback in mutual TLS to use enhanced error messages. Added TLS exception handling in TcpHandlers to detect and report AuthenticationException and CryptographicException with detailed diagnostics. All existing TLS tests continue to pass. * Enhance TLS error logging across all handshake scenarios Upgraded mutual TLS validation errors from Warning to Error level for better visibility. Enhanced error messages now cover all TLS failure scenarios: Server-side mutual TLS validation: - No client certificate provided: detailed error with troubleshooting steps - Client certificate validation failures: comprehensive chain validation diagnostics Client-side and general handshake failures: - Added enhanced error diagnostics to UserEventTriggered for TlsHandshakeCompletionEvent - Improved client-side troubleshooting guidance including certificate trust chain requirements - Both client and server TLS exceptions now include role-specific troubleshooting All error messages provide actionable suggestions and certificate details to aid in diagnosis.
…dotnet#7897) * Fix TLS hostname validation bug and add configurable validation (akkadotnet#7893) ## Summary This commit addresses GitHub issue akkadotnet#7893 by fixing a critical TLS hostname validation bug and introducing a configurable, type-safe validation system. ## Changes ### Bug Fix - **Fixed DotNettyTransport.cs:355** - TLS client was incorrectly validating against the client's own certificate DNS name instead of the remote server address - Changed from `certificate.GetNameInfo(X509NameType.DnsName, false)` to `remoteAddress.Host` ### New Configuration - Added `validate-certificate-hostname` config option to Remote.conf - Default: `false` (disabled for backward compatibility and mutual TLS flexibility) - When enabled: Traditional TLS hostname validation (CN/SAN must match target hostname) - When disabled: Only validates certificate chain, ignores hostname mismatches - Useful for: Mutual TLS with per-node certificates, IP-based connections, dynamic service discovery ### Type-Safe Validation System - Introduced enums to prevent primitive confusion in security-critical code: - `ChainValidationMode` enum (ValidateChain, IgnoreChainErrors) - `HostnameValidationMode` enum (ValidateHostname, IgnoreHostnameMismatch) - Created `TlsValidationCallbacks` static factory class with: - Main `Create()` method accepting enum parameters and logging adapter - Convenience methods: `ValidateFull()`, `ValidateChainOnly()`, `ValidateHostnameOnly()`, `AcceptAll()` - Detailed error logging with filtered SslPolicyErrors - Makes validation flags independent and composable - Replaces ~35 lines of inline callback code with 3 lines of self-documenting factory calls ### Updated SslSettings - Added `ValidateCertificateHostname` property - Updated all constructors to accept the new property - Updated `Create()` method to read from HOCON config ### Test Coverage - Extended `DotNettyMutualTlsSpec` with 4 new test cases: - `Hostname_validation_disabled_should_allow_different_certificates()` - Different certs work with validation disabled - `Hostname_validation_enabled_should_reject_different_certificates()` - Different certs fail with validation enabled - `Same_certificate_should_connect_in_mutual_tls()` - Typical mutual TLS scenario - `Hostname_validation_default_should_be_disabled()` - Verifies backward compatibility ## Technical Details Chain validation and hostname validation are now fully independent: - `suppressValidation=true` disables chain/CA validation (for self-signed certs) - `validateCertificateHostname=true/false` controls hostname matching (for per-node certs, IPs) This allows testing hostname validation with self-signed certificates by using `suppressValidation=true, validateCertificateHostname=true`. Fixes akkadotnet#7893 * Extend DotNettySslSetup to expose all SSL/TLS configuration options ## Summary Extended `DotNettySslSetup` programmatic API to expose the full SSL/TLS configuration, including the newly added hostname validation setting and the existing RequireMutualAuthentication setting that was previously only available via HOCON. ## Changes ### DotNettySslSetup API - Added `RequireMutualAuthentication` property (was missing from programmatic API) - Added `ValidateCertificateHostname` property (new setting from akkadotnet#7893) - Added comprehensive XML documentation for all properties and constructors - Added backward-compatible constructors: - 2-parameter: Defaults to RequireMutualAuthentication=true, ValidateCertificateHostname=false - 3-parameter: Defaults to ValidateCertificateHostname=false - 4-parameter: Full control over all settings - Updated `Settings` property to pass all 4 parameters to `SslSettings` constructor ### Integration Tests - Added 3 integration tests in `DotNettySslSetupSpec`: - Verify 2-parameter setup configures effective DotNettyTransportSettings with expected defaults - Verify 3-parameter setup configures effective settings with specified RequireMutualAuth - Verify 4-parameter setup configures effective settings with all specified values - Tests verify the actual consumption path: ActorSystem → DotNettyTransportSettings.Create() → setup.Value.Settings - Tests validate that setup values correctly override HOCON defaults ## Backward Compatibility All existing code using the 2-parameter constructor continues to work with the same defaults: - RequireMutualAuthentication: true (matches previous HOCON-only behavior) - ValidateCertificateHostname: false (matches new HOCON default) The setup is properly consumed in `DotNettyTransportSettings.Create(ActorSystem)` which retrieves the setup via `system.Settings.Setup.Get<DotNettySslSetup>()` and calls `setup.Value.Settings` to get the fully configured `SslSettings` object. * Update security documentation for hostname validation feature ## Changes - Explained the new independent validation system (chain vs hostname) - Added details about default certificate stores used (Windows, Linux, macOS) - Documented the `validate-certificate-hostname` setting with use cases - Added validation mode combination table - Included configuration examples for both P2P and client-server scenarios - Added comprehensive troubleshooting for hostname validation errors - Documented enhanced TLS error messages from v1.5.52 - Reduced emoji usage for more professional tone - Added links to Microsoft documentation and RFC specifications ## Key Documentation Updates ### New Sections - Certificate Validation: Independent Control - Hostname Validation setting explanation - Validation Mode Combinations table - Configuration with Hostname Validation Enabled - Enhanced error message examples ### Troubleshooting Additions - RemoteCertificateNameMismatch errors (hostname validation failures) - UntrustedRoot errors (chain validation failures) - Understanding TLS Error Messages section with real examples - Multiple fix options for each error scenario ### Technical Details - Explained which OS certificate stores are used by default - Referenced RFC 5280 and RFC 6125 for validation standards - Clarified that suppress-validation only controls chain validation - Clarified that hostname validation is separate and optional * Fix markdown linting and spellcheck issues in security docs - Add blank lines around all fenced code blocks - Add language specifiers to code blocks (text, hocon, bash, powershell) - Change dash lists to asterisk lists for consistency - Add 'hostnames' to spellcheck dictionary - Emphasize that hostname validation defaults to false (disabled) * Improve documentation heading structure and title case - Remove technical setting names from headings - Use descriptive section titles instead - Change subheadings to 'Enabled/Disabled' pattern - Move technical details into content body - Fix title case linting issues This makes the documentation more scannable and separates conceptual sections from implementation details. * added api approvals
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
1 participant
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.
Changes
Backports #7891 and #7897 from the
devbranch