Don't unset ALPN list pointer during ALPN selection callback.

[rzikm]

Fixes #99289.

When server issues HelloRetryRequest and client sends a second ClientHello message, we need to do a second round of ALPN selection, so we need to keep it alpn list handle still set in the Ssl ex data slot.

The ALPN list handle is going to be alive until the end of the SSL object lifetime, so there is no risk of dangling pointer as the comment claims.

runtime/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs

Lines 423 to 427 in 961257b

	if (AlpnHandle.IsAllocated)
	{
	Interop.Ssl.SslSetData(handle, IntPtr.Zero);
	AlpnHandle.Free();
	}

[ghost]

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

Issue Details

---

Fixes #99289.

When server issues HelloRetryRequest and client sends a second ClientHello message, we need to do a second round of ALPN selection, so we need to keep it around and not set the ptr to null.

The ALPN list handle is going to be alive until the end of the SSL object lifetime

runtime/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs

Lines 423 to 427 in 961257b

	if (AlpnHandle.IsAllocated)
	{
	Interop.Ssl.SslSetData(handle, IntPtr.Zero);
	AlpnHandle.Free();
	}

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

[rzikm]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[wfurt]

Can we craft test using NegotiateClientCertificateAsync? It does not need to send cert IMHO just a way how to trigger renegotiation. Perhaps do Theory for 1.2 and 1.3 separately....?

[rzikm]

Can we craft test using NegotiateClientCertificateAsync? It does not need to send cert IMHO just a way how to trigger renegotiation. Perhaps do Theory for 1.2 and 1.3 separately....?

I don't think renegotiation triggers this, its triggered by server sending HelloRetryRequest in response to ClientHello, and I don't think we have public API which can trigger this (I had to manipulate the openssl.cnf to reproduce the issue).

[wfurt]

Can we craft test using NegotiateClientCertificateAsync? It does not need to send cert IMHO just a way how to trigger renegotiation. Perhaps do Theory for 1.2 and 1.3 separately....?

I don't think renegotiation triggers this, its triggered by server sending HelloRetryRequest, and I don't think we have public API which can trigger this (I had to manipulate the openssl.cnf to reproduce the issue).

Wouldn't the handshake logic be triggered again ... including the callback? I think it does it for certificate validation as far as I remember.

[rzikm]

Classical renegotiation after the first handshake was completed does not trigger the ALPN selection callback (Otherwise, we would've noticed already because we test this code path for late client cert negotiation).

I have found some discussion at openssl/openssl#2767 and apparently OpenSSL ignores ALPN extension once the application protocol has been established by ServerHello.

[rzikm]

Test failures are unrelated.

[rzikm]

/azp run runtime-libraries-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[wfurt]

LGTM

[rzikm]

CI failures are unrelated

[rzikm]

/backport to release/8.0-staging

[github-actions]

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/8262729469