Repository navigation
Conversation
Track the peer Finished across post-handshake authentication so unrelated records cannot complete certificate negotiation. Reject interrupting application data, preserve shutdown errors, and leave trailing application records for normal reads. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 4 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
|
is there some other way how to address this? I really don't like tracking OpenSSL internals. (or TLS protocols details) And what are the symptoms one would observer? NegotiateClientCertificateAsync would return nothing in cases the client may actually send certificate? |
yes, that is exactly the symptom.
not that I am aware of, apparently, OpenSSL does not expose PHA negotiation state on public API, so it finishing can only be inferred by the change in the stored Finished message. |
TLS 1.3 post-handshake client authentication can complete prematurely when an unrelated record, such as KeyUpdate or application data, arrives while
SslStream.NegotiateClientCertificateAsyncis waiting for the client's certificate flight. OpenSSL can report handshake success in this state even though the peer has not answered the CertificateRequest.This prototype tracks the peer's verified Finished message across post-handshake authentication and keeps negotiation pending until a new Finished has been processed. The completion guard is shared by the buffered SslStream path and socket-bound TlsSession path. An actual empty-Certificate response remains distinguishable from an unanswered request and can still be accepted by the application's validation callback.
Application data received before the authentication flight completes now fails explicitly with
AuthenticationException, rather than completing with no certificate. This is intentionally fail-closed rather than buffering interleaved application data. The change also preserves shutdown/error statuses fromSSL_peekand processes renegotiation records individually so application data following Finished remains available to normal reads.Regression coverage adds application-data and shutdown interruption cases and extends the TLS 1.3 success/decline cases to cover application data coalesced with the authentication flight. No public API is added.
Validation
build.cmd clr+libs -rc release(from the shortS:\subst path): passed..dotnet\dotnet.exe build src\libraries\System.Net.Security\tests\FunctionalTests\System.Net.Security.Tests.csproj /t:test /p:RuntimeConfiguration=Release: passed; 5,392 total, 36 skipped, zero failures..dotnet\dotnet.exe build src\libraries\System.Net.Security\tests\UnitTests\System.Net.Security.Unit.Tests.csproj /t:test /p:RuntimeConfiguration=Release: passed; 121 total, 4 skipped, zero failures../src/native/libs/build-native.sh debug x64 -outconfig net12.0-linux-Debug-x64under WSL: passed. After the shutdown-status changes,cmake --build artifacts/obj/native/net12.0-linux-Debug-x64 --target install -j 4also passed.Linux managed validation is pending.
./build.sh clr+libs -rc releaseunder WSL failed before compilation because the shared.dotnetdirectory has the Windows host but no Linuxdotnetexecutable. The Linux managed assembly and Linux-only interruption tests have not been built/run; the regression tests have not been verified to fail against the unfixed managed implementation. This PR preserves the prototype at the requested validation boundary.Note
This pull request description and code changes were generated with GitHub Copilot.