Backport: feat(remote): custom certificate validation with single execution path - fixes mTLS asymmetry bug (#7915) #7921
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
…h - fixes mTLS asymmetry bug (akkadotnet#7915) * feat(remote): add CertificateValidationCallback delegate and CertificateValidation helper factory - Define public CertificateValidationCallback delegate for custom certificate validation - Add CertificateValidation factory class with 7 helper methods: * ValidateChain() - CA chain validation * ValidateHostname() - CN/SAN matching * PinnedCertificate() - Certificate pinning by thumbprint * ValidateSubject() - Subject DN matching (with wildcard support) * ValidateIssuer() - Issuer DN matching * Combine() - Compose multiple validators * ChainPlusThen() - Chain validation + custom logic - Add CustomValidator property to DotNettySslSetup with overloaded constructors - Maintain full backward compatibility with existing config-based validation Relates to akkadotnet#7914 * feat(remote): implement single execution path for certificate validation with hostname validation asymmetry fix - Integrate custom certificate validators into DotNettyTransport pipelines (client and server) - Implement single execution path: compose validator from config when custom not provided - Add ComposeValidatorFromSettings() to build validators from SuppressValidation and ValidateCertificateHostname settings - Add CustomValidator property to SslSettings with updated constructors for seamless integration - Fix asymmetry bug: server-side now applies hostname validation like client-side - Replace dual-path logic (custom vs config-based) with unified composition pattern - Add hostname matching helper with reflection-based SAN support for multi-framework compatibility - Eliminates need for TlsValidationCallbacks on each pipeline setup call * fix: use TlsValidationCallbacks for config-based validation in single execution path - Revert to using proven TlsValidationCallbacks logic for configuration-based validation - This maintains compatibility with existing validation behavior while enabling single execution path - CertificateValidation helpers remain available for custom user validators - Reduces test failures from 9 to 2 by using well-tested validation logic * fix: reject missing client certificates in server-side mutual TLS validation When the server requires mutual TLS authentication (RequireMutualAuthentication=true), it must reject TLS handshakes where the client fails to provide a certificate. Previously, the validation callback would pass a null certificate to the composed validator without pre-checking it. This allowed connections from clients without certificates to succeed when they should fail. Now we explicitly check if the certificate is null when mutual auth is required and immediately reject the connection with a warning log message. This fixes the failing test: Mutual_TLS_should_fail_when_client_has_no_certificate Fixes: All 329 tests now pass (324 passed, 5 skipped, 0 failed) * docs: add programmatic certificate validation examples and consolidate security documentation Adds comprehensive programmatic certificate validation examples to TlsConfigurationSample: - ProgrammaticMutualTlsSetup: Basic mutual TLS with custom validators - CertificatePinningExample: Certificate pinning by thumbprint - CustomValidationLogicExample: Chain validation + custom business logic - HostnameValidationExample: Programmatic hostname validation setup - SubjectValidationExample: Subject DN validation Consolidates security.md documentation: - Merged "Hostname Validation" and "Mutual TLS Authentication" into unified "Validation Strategies: HOCON vs Programmatic" section with decision matrix - Added examples for both P2P clusters and client-server architectures - Cross-referenced sections to reduce duplication - Clarified when to use programmatic vs HOCON configuration Follows documentation guidelines (security.md:70): - Uses !code references with #region tags for live code examples - Organizes content for discoverability - Provides decision matrix for choosing validation strategy * fix: correct compilation errors in TlsConfigurationSample documentation examples - Add Akka.Remote reference to Akka.Docs.Tests project for DotNettySslSetup types - Wrap new programmatic examples with #if NET6_0_OR_GREATER for framework compatibility - Convert example methods to void to simplify documentation-only code - Fix API usage: use params instead of arrays, correct delegate signatures - Remove BootstrapSetup complexity from examples to focus on core TLS setup patterns * fix: wire CustomValidator through SslSettings and add comprehensive integration tests CRITICAL FIXES: - Fixed DotNettySslSetup.Settings to pass CustomValidator to SslSettings constructor (Line 114 was creating SslSettings without the CustomValidator parameter) - Removed unused ValidateCertificateHostnameMatch method (489-542) (Hostname validation is already handled by TlsValidationCallbacks.Create) NEW TESTS - CustomValidator Functionality: - CustomValidator_that_accepts_should_allow_connection * Verifies CustomValidator callback is invoked during TLS handshake * Verifies acceptance allows successful connection - CustomValidator_that_rejects_should_prevent_connection * Verifies CustomValidator rejection prevents connection * Verifies callback was invoked even when rejecting - DotNettySslSetup_should_pass_CustomValidator_to_SslSettings * Unit test verifying CustomValidator is wired through to SslSettings Addresses PR review comments akkadotnet#7915: - CustomValidator now properly wired to SslSettings - Removed dead code (ValidateCertificateHostnameMatch) - Added real integration tests that validate CustomValidator actually works * Add warning when both DotNettySslSetup and HOCON SSL certificate config are present - Logs warning when DotNettySslSetup is used alongside explicit HOCON certificate configuration - Only warns when HOCON has actual certificate.path or certificate.thumbprint configured - Avoids false positives from default/empty config sections - Adds test verifying DotNettySslSetup precedence behavior - Addresses PR feedback: implement Option 1 from review comment * Remove unnecessary NET6_0_OR_GREATER conditional compilation directives - All types used (X509Certificate2, DotNettySslSetup, CertificateValidation) are available in .NET Standard 2.0 - Conditional directives were added during troubleshooting but are not needed - Verified compilation on both net8.0 and net48 targets * Consolidate TlsValidationCallbacks into public CertificateValidation API Removes internal TlsValidationCallbacks class and related enums (~145 lines) and refactors ComposeValidatorFromSettings() to use the public CertificateValidation helpers instead. Changes: - Remove ChainValidationMode and HostnameValidationMode enums - Remove TlsValidationCallbacks internal class - Refactor ComposeValidatorFromSettings() to handle all 4 combinations of SuppressValidation and ValidateCertificateHostname flags: * suppressChain=true, validateHostname=false → Accept all * suppressChain=true, validateHostname=true → Validate hostname only * suppressChain=false, validateHostname=true → Chain + hostname * suppressChain=false, validateHostname=false → Chain only (default) Benefits: - Eliminates code duplication between internal and public APIs - Simplifies maintenance by having a single validation implementation - Makes the public CertificateValidation API the canonical approach - All 43 DotNetty tests pass including edge case validations * cleaned up `CertificateValidation` composition code for default settings * Add comprehensive test coverage for CertificateValidation helpers Added 11 new tests to achieve 100% coverage of previously untested CertificateValidation helper methods: PinnedCertificate tests: - Accept connections with matching thumbprint - Reject connections with non-matching thumbprint ValidateSubject tests: - Accept certificates with matching subject - Reject certificates with non-matching subject - Support wildcard pattern matching (CN=Akka-Node-*) ValidateIssuer tests: - Accept certificates with matching issuer Combine/ChainPlusThen tests: - Verify composability of validators CustomValidator precedence tests: - Verify CustomValidator overrides validateCertificateHostname setting Also removed obsolete Mono checks from all new tests per maintainer guidance (Mono is no longer supported). Test results: 18/18 passing (7 existing + 11 new) * Remove unnecessary Combine() wrapper for single validators in tests Simplified two test cases that were unnecessarily wrapping single CertificateValidationCallback delegates in Combine(): - CustomValidator_that_accepts_should_allow_connection - CustomValidator_that_rejects_should_prevent_connection Changed from: var validator = CertificateValidation.Combine((cert, ...) => true); To cleaner direct delegate assignment: CertificateValidationCallback validator = (cert, ...) => true; Combine() is only needed when composing multiple validators. These tests verify single custom validators, so direct assignment is clearer. All tests still pass (4/4 CustomValidator tests verified). * added `nullability` annotations to DotNettyTransport * Remove obsolete Mono and NET471 workarounds from SSL tests - Removed #if !NET471 conditional compilation directives (10 instances) Project now targets net48, making NET471 conditionals meaningless - Removed if (IsMono) runtime checks (7 instances) Modern .NET uses CoreCLR cross-platform, not Mono - All SSL tests now run unconditionally on supported platforms - Tests verified passing: 27/27 on net8.0, 26/26 on net48 * Fix null certificate handling in SSL validation methods - Added explicit null checks to all CertificateValidation helper methods - PinnedCertificate: Check for null cert and filter empty thumbprints - ValidateSubject/ValidateIssuer: Check for null cert and empty values - ValidateHostname: Check for null cert before accessing properties - ValidateChain: Check for null cert before chain validation - Improved error messages to distinguish null cert from other failures - Added comprehensive unit test coverage for edge cases - Prevents potential NullReferenceException in TLS handshake scenarios * Add documentation explaining why case-insensitive thumbprint comparison is safe Added detailed comment explaining: - Thumbprints are hexadecimal SHA hash representations - Hex values are inherently case-insensitive (2A8B == 2a8b) - Different tools display differently (Windows vs OpenSSL) - Case-insensitive comparison improves usability without compromising security * Improve certificate validation tests with EventFilter - Replaced ExpectMsg with EventFilter for proper log assertion pattern - EventFilter is the idiomatic way to assert log messages in Akka.NET tests - Added test for rejecting non-matching thumbprint with EventFilter - Updated Combine test to clearly document short-circuit behavior - All tests now properly verify both result AND expected log messages * Use EventFilter to assert SSL validation errors in multi-actor system tests Updated SSL integration tests to use EventFilter for asserting specific validation errors instead of just checking connection failure. This provides better test precision by verifying the exact reason for connection failure. With mTLS enabled, validation errors occur on the server side (_sys2) when it validates the client certificate, since the client (Sys) has suppressValidation enabled. The EventFilter assertions are correctly targeted to the system where the validation errors occur. Changes: - Added EventFilter assertions to PinnedCertificate rejection test - Added EventFilter assertions to CustomValidator rejection test - Added EventFilter assertions to ValidateSubject rejection test - Modified custom validator to log error for EventFilter detection - Added comments explaining the mTLS validation flow * Revert "Use EventFilter to assert SSL validation errors in multi-actor system tests" This reverts commit 2022d63. * remove unnecessary project reference * added API approvals * Fix incorrect bitwise AND check with SslPolicyErrors.None The condition `(errors & SslPolicyErrors.None) != SslPolicyErrors.None` was always false because SslPolicyErrors.None equals 0, and any value bitwise AND with 0 always results in 0. Changed to simple equality check `errors != SslPolicyErrors.None` to correctly detect when SSL policy errors are present. This bug prevented the TlsErrorMessageBuilder from ever building detailed error messages when SSL validation failed, making debugging harder. --------- Co-authored-by: Gregorius Soedharmo <arkatufus@yahoo.com>
This was referenced Oct 27, 2025
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.
Summary
Backport of #7915 (commit e720f43) from
devtov1.5.This PR adds programmatic certificate validation helpers and fixes an mTLS asymmetry bug where server-side hostname validation was not being applied consistently with client-side validation.
Key Features
New
CertificateValidationCallbackdelegate andCertificateValidationhelper factory with 7 helper methods:ValidateChain()- CA chain validationValidateHostname()- CN/SAN matchingPinnedCertificate()- Certificate pinning by thumbprintValidateSubject()- Subject DN matching (with wildcard support)ValidateIssuer()- Issuer DN matchingCombine()- Compose multiple validatorsChainPlusThen()- Chain validation + custom logicSingle execution path for certificate validation (eliminates dual-path config vs custom logic)
Fixes mTLS asymmetry bug where server-side hostname validation wasn't applied
Full backward compatibility with existing HOCON-based validation
Comprehensive test coverage (18 new tests)
Test Plan
All 329 Akka.Remote SSL tests pass (324 passed, 5 skipped, 0 failed)
Related Issues
Fixes the mTLS asymmetry bug discovered in #7914