Implement TLS1.3 delayed client cert requests on Linux by rzikm · Pull Request #64268 · dotnet/runtime

rzikm · 2022-01-25T14:57:00Z

This PR adds the NegotiateClientCertificateAsync support for TLS1.3 on Linux

The changes in this PR depend on changes made in #63945, so this PR will need to stay in draft state until that one is merged.
https://github.com/dotnet/runtime/pull/64268/files#diff-5d47fb42fd96c09e13b52482e126d7a282513646feb5f672108dbbd7dd0d5f4aR543-R545

The changes also should contribute to #58927, I was able to reproduce the hang in SslStream_NegotiateClientCertificateAsyncTls13_Succeeds when using OpenSSL and Linux.
https://github.com/dotnet/runtime/pull/64268/files#diff-1d28e92dd65ace08e351f991ffd5da78f606a6bc4f212160df97d5012bd7cdc9R577-R579

ghost · 2022-01-25T14:57:08Z

Tagging subscribers to this area: @dotnet/ncl, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #55757

This PR adds the NegotiateClientCertificateAsync support for TLS1.3 on Linux

The changes in this PR depend on changes made in #63945, so this PR will need to stay in draft state until that one is merged.

The changes also should contribute to #58927, I was able to reproduce the hang in SslStream_NegotiateClientCertificateAsyncTls13_Succeeds when using OpenSSL and Linux.

Author:	rzikm
Assignees:	-
Labels:	`area-System.Net.Security`
Milestone:	-

wfurt

looks like good start. I left few early comments

rzikm · 2022-01-28T09:12:52Z

Looks like we have test failures on Ubuntu 18.04, possibly becuase we are calling SSL_renegotiate on TLS 1.3 session. The only way this can happen IMO is that we are compiling against older openssl headers and thus the branch calling SSL_verify_client_post_handshake is left out due to preprocessor define.

CI Log

===========================================================================================================
/datadisks/disk1/work/9FBA08A2/w/A8980923/e /datadisks/disk1/work/9FBA08A2/w/A8980923/e
Discovering: System.Net.Security.Tests (method display = ClassAndMethod, method display options = None)
Discovered: System.Net.Security.Tests (found 290 of 475 test cases)
Starting: System.Net.Security.Tests (parallel test collections = on, max threads = 2)
System.Net.Security.Tests.SslStreamEKUTest.SslStream_SelfSignedClientEKUClientAuth_Ok [SKIP]
Condition(s) not met: "IsRootCertificateInstalled"
System.Net.Security.Tests.SslStreamEKUTest.SslStream_ServerEKUClientAuth_Fails [SKIP]
Condition(s) not met: "IsRootCertificateInstalled"
System.Net.Security.Tests.SslStreamEKUTest.SslStream_ClientEKUServerAuth_Fails [SKIP]
Condition(s) not met: "IsRootCertificateInstalled"
System.Net.Security.Tests.SslStreamEKUTest.SslStream_NoEKUServerAuth_Ok [SKIP]
Condition(s) not met: "IsRootCertificateInstalled"
System.Net.Security.Tests.SslStreamEKUTest.SslStream_NoEKUClientAuth_Ok [SKIP]
Condition(s) not met: "IsRootCertificateInstalled"
System.Net.Security.Tests.SslStreamNetworkStreamTest.SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(sendClientCertificate: True) [FAIL]
Interop+Crypto+OpenSslCryptographicException : error:1420410A:SSL routines:SSL_renegotiate:wrong ssl version
Stack Trace:
/_/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs(298,0): at System.Net.Security.SslStream.<RenegotiateAsync>d__172`1[[System.Net.Security.AsyncReadWriteAdapter, System.Net.Security, Version=7.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]].MoveNext()
/_/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs(463,0): at System.Net.Security.Tests.SslStreamNetworkStreamTest.SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(Boolean sendClientCertificate)
--- End of stack trace from previous location ---
System.Net.Security.Tests.SslStreamNetworkStreamTest.SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(sendClientCertificate: False) [FAIL]
Interop+Crypto+OpenSslCryptographicException : error:1420410A:SSL routines:SSL_renegotiate:wrong ssl version
Stack Trace:
/_/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs(298,0): at System.Net.Security.SslStream.<RenegotiateAsync>d__172`1[[System.Net.Security.AsyncReadWriteAdapter, System.Net.Security, Version=7.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]].MoveNext()
/_/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs(463,0): at System.Net.Security.Tests.SslStreamNetworkStreamTest.SslStream_NegotiateClientCertificateAsyncTls13_Succeeds(Boolean sendClientCertificate)
--- End of stack trace from previous location ---
Finished: System.Net.Security.Tests
=== TEST EXECUTION SUMMARY ===

rzikm · 2022-02-02T14:53:07Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2022-02-02T14:53:22Z

Azure Pipelines successfully started running 1 pipeline(s).

rzikm · 2022-02-03T08:17:07Z

CI failures are unrelated:

System.Net.Sockets.Tests.SocketAsyncEventArgsTest.AcceptAsync_WithTooSmallReceiveBuffer_Failure
System.Net.Security.Tests.TelemetryTest.EventSource_SuccessfulHandshake_LogsStartStop

bartonjs · 2022-02-08T17:12:53Z

If no special #if is required for these prototypes, they should be sorted alphabetically in this block.

Should be fixed now

rzikm · 2022-02-08T19:29:18Z

I think the code is ready now for (hopefully last) round of reviews

wfurt

The SslStream part look good to me. I'll deferrer final judgement on the PAL to @bartonjs.

rzikm · 2022-02-09T07:56:19Z

Failing test is #64964
build runtime-libraries enterprise-linux failure seems unrelated

 [ 45%] Building ASM object vm/wks/CMakeFiles/cee_wks_core.dir/__/amd64/jithelpers_fastwritebarriers.S.o
/repo/src/coreclr/vm/amd64/jithelpers_fast.S:456:9: error: invalid operand for instruction
mov r10, 0xCDCDCDCDCDCDCDCD
^

rzikm · 2022-02-09T07:56:29Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2022-02-09T07:56:43Z

Azure Pipelines successfully started running 1 pipeline(s).

Fix native shims on older OpenSSL versions Remove unneeded hacks Remove redundant PlatformSpecificAttribute Merge CryptoNative_SslVerifyClientPostHandshake to CryptoNative_SslRenegotiate Reenable test on Windows 11 Revert unwanted changes Advertise post-handshake auth if client provided a cert Fix compilation against older OpenSSL versions Fix trailing whitespace Fix compilation on older versions Add missing define

ghost added the area-System.Net.Security label Jan 25, 2022

ghost assigned rzikm Jan 25, 2022

rzikm changed the title ~~Use correct API on TLS1.3 for delayed client cert requests (WIP)~~Implement TLS1.3 delayed client cert requests on LinuxJan 25, 2022

rzikm commented Jan 25, 2022
View reviewed changes

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs Outdated

rzikm added the os-linux Linux OS (any supported distro) label Jan 25, 2022

wfurt reviewed Jan 25, 2022
View reviewed changes

rzikm force-pushed the 55757_nego-tls-1.3 branch 3 times, most recently from 709984b to 6f64c53Compare January 27, 2022 13:24

rzikm commented Jan 27, 2022
View reviewed changes

Comment threadsrc/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs Outdated

rzikm requested a review from wfurt January 27, 2022 13:56

rzikm marked this pull request as ready for review January 27, 2022 19:33

rzikm force-pushed the 55757_nego-tls-1.3 branch 2 times, most recently from 0698b85 to 0996fcdCompare February 2, 2022 12:01

wfurt reviewed Feb 5, 2022
View reviewed changes

Comment threadsrc/libraries/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs Outdated

wfurt reviewed Feb 5, 2022
View reviewed changes

Comment threadsrc/native/libs/System.Security.Cryptography.Native/opensslshim.h Outdated

wfurt reviewed Feb 5, 2022
View reviewed changes

Comment threadsrc/native/libs/System.Security.Cryptography.Native/opensslshim.h Outdated

wfurt reviewed Feb 5, 2022
View reviewed changes

Comment threadsrc/native/libs/System.Security.Cryptography.Native/pal_ssl.c Outdated

stephentoub reviewed Feb 7, 2022
View reviewed changes

Comment threadsrc/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs Outdated

bartonjs reviewed Feb 8, 2022
View reviewed changes

rzikm requested a review from bartonjs February 8, 2022 19:29

wfurt approved these changes Feb 9, 2022
View reviewed changes

rzikmand others added 7 commits February 9, 2022 11:40

Fix compilation
1e36e7c

Redisable TLS13 renego tests on Windows
b5e049e

Improve wording of a comment.
25fe070

Use NEED_OPENSSL_1_1 instead of NEED_OPENSSL_1_3
bdc29d0

Move function prototypes to osslcompat_111.h
a014271

Sort the function prototypes alphabetically
72eb1bb

rzikm force-pushed the 55757_nego-tls-1.3 branch from 5e2225d to 72eb1bbCompare February 9, 2022 10:40

runfoappBot mentioned this pull request Feb 9, 2022
ThreadPoolTests.CooperativeBlockingCanCreateThreadsFaster timed out #64964
Closed

bartonjs approved these changes Feb 9, 2022
View reviewed changes

bartonjs merged commit aaaa2a7 into dotnet:mainFeb 9, 2022

rzikm removed their assignment Feb 16, 2022

ghost locked as resolved and limited conversation to collaborators Mar 18, 2022

karelz added this to the 7.0.0 milestone Apr 8, 2022

Uh oh!

Conversation

rzikm commented Jan 25, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jan 25, 2022

Uh oh!

Uh oh!

wfurt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rzikm commented Jan 28, 2022

Uh oh!

rzikm commented Feb 2, 2022

Uh oh!

azure-pipelinesBot commented Feb 2, 2022

Uh oh!

rzikm commented Feb 3, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bartonjsFeb 8, 2022

Choose a reason for hiding this comment

Uh oh!

rzikmFeb 8, 2022

Choose a reason for hiding this comment

Uh oh!

rzikm commented Feb 8, 2022

Uh oh!

wfurt left a comment

Choose a reason for hiding this comment

Uh oh!

rzikm commented Feb 9, 2022

Uh oh!

rzikm commented Feb 9, 2022

Uh oh!

azure-pipelinesBot commented Feb 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rzikm commented Jan 25, 2022•
edited
Loading