Fix To-header signing during SCT renewal (#5883)#5943
Open
afifi-ins wants to merge 2 commits into
Open
Conversation
WSSecurityOneDotZeroSendSecurityHeader.CompletePrimarySignatureCore added a To-header reference whenever ShouldSignToHeader was true and either signing-key field was set. On NetFx the equivalent guard is 'signatureKey is AsymmetricSecurityKey' -- only asymmetric primary signatures (e.g. the X.509 client cert in the initial SCT Issue request) reference the <a:To> header. Symmetric primary signatures driven by an SCT-derived KeyedHashAlgorithm (e.g. the RST/SCT/Renew exchange secured by the current session key) skip the reference entirely. In the .NET (Core) port asymmetric setup leaves _signingKey null and stores the AsymmetricAlgorithm on _signedXml.SigningKey, while symmetric setup populates _signingKey (KeyedHashAlgorithm) and leaves _signedXml.SigningKey null (see StartPrimarySignatureCore / GetSigningAlgorithm). The fix replaces the OR-ed check with '_signedXml.SigningKey != null' to mirror NetFx exactly. Two side-effects of the previous behavior surfaced when a .NET (Core) client renewed an SCT against a .NET Framework 4.8 server over NetTcpBinding+TransportWithMessageCredential+Certificate (the scenario reported in dotnet#5883): 1. The extra To reference made the renew message a shape the FX server rejects with 'The security protocol cannot verify the incoming message.' 2. _toHeaderStream was consumed once by the first signature on the renew message and the second signature on the same outgoing message then hashed zero bytes, surfacing as XmlException 'Root element is missing.' from inside SignedXml.ComputeSignature and a CommunicationObjectFaultedException to the caller. With the corrected gate the symmetric primary signature no longer references _toHeaderStream at all, so the stream-position issue cannot trigger. Adds reflection-based regression tests covering both gate states (asymmetric primary signs To header; symmetric primary does not). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Server SessionKeyRenewalInterval drops from 30s to 10s, SessionKeyRolloverInterval from 10s to 3s. The test loop is correspondingly shortened from 75s/5s-step to 25s/2s-step. The test still reliably crosses the renewal boundary (fails at ~6s without the WSSecurityOneDotZeroSendSecurityHeader fix) but now completes in ~25s when the fix is in place. Add E2E regression test for SCT renewal Replaces the reflection-based unit test from PR dotnet#5943 with a full end-to-end regression test that hosts a NetTcp + TransportWithMessageCredential + Certificate service with SessionKeyRenewalInterval = 30s, then drives an IWcfService.Echo loop across the renewal boundary. The test fails (channel faults) without the WSSecurityOneDotZeroSendSecurityHeader fix and passes with it. The new test host compiles for both SelfHostedWcfService (net471 / System.ServiceModel) and SelfHostedCoreWcfService (net10.0 / CoreWCF), matching the existing TcpTransportSecurityMessageCredentialsCert host pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
A .NET (Core) WCF client using NetTcpBinding with TransportWithMessageCredential + ClientCredentialType=Certificate against a .NET Framework 4.8 server faults at the first SCT renewal (~30 s after open).
The client throws CommunicationObjectFaultedException wrapping XmlException: Root element is missing.; the FX server logs MessageSecurityException: The security protocol cannot verify the incoming message.
Root cause
WSSecurityOneDotZeroSendSecurityHeader.CompletePrimarySignatureCore (.NET Core port) added the <a:To> reference to SignedInfo whenever ShouldSignToHeader was true and either _signingKey != null or
_signedXml.SigningKey != null.
On .NET Framework the same code gates that block on signatureKey is AsymmetricSecurityKey — i.e. only asymmetric primary signatures (e.g. the X.509 client cert on the initial SCT Issue request) reference
<a:To>. Symmetric primary signatures driven by an SCT-derived KeyedHashAlgorithm (e.g. the RST/SCT/Renew exchange secured by the current session key) skip the reference entirely.
In the .NET (Core) port the equivalent of NetFx's signatureKey is AsymmetricSecurityKey is _signedXml.SigningKey != null:
So the OR-ed check let the symmetric renew signature attach an unexpected To reference; the FX server then rejected the renew message. As a secondary effect, on messages with two signatures the shared
_toHeaderStream was consumed by the first signature and the second hashed zero bytes, surfacing as the Root element is missing. XmlException inside SignedXml.ComputeSignature.
Fix
Replace the OR-ed gate with _signedXml.SigningKey != null to mirror NetFx exactly. With the corrected gate, symmetric primary signatures no longer touch _toHeaderStream, so the stream-position issue cannot
trigger either.
// before
if (isPrimarySignature && (ShouldSignToHeader) && (_signingKey != null || _signedXml.SigningKey != null) && ...)
// after
if ((ShouldSignToHeader) && (_signedXml.SigningKey != null) && ...