Implement channel binding support on Unix - #130758
Conversation
…Unix Adds a new `NetSecurityNative_AcceptSecContextEx` native shim that takes a channel binding token (the application data portion of the `SecChannelBindings` buffer) and forwards it to `gss_accept_sec_context` via a `gss_channel_bindings_struct`. The pre-existing `NetSecurityNative_AcceptSecContext` shim now delegates to the new entry point with NULL bindings to preserve backward compatibility for in-place servicing. The managed `NegotiateAuthenticationPal.Unix` server path now passes `_channelBinding` into `AcceptSecurityContext`, which dispatches to the cbt-aware overload when bindings are present. GSSAPI surfaces a binding mismatch as `GSS_S_BAD_BINDINGS` so no additional validation is required on the managed side, matching the existing client (initiator) behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds four functional tests using KerberosExecutor (in-process KDC) that
verify GSSAPI server-side channel binding token enforcement:
* ChannelBindings_Matching_{Endpoint,Unique}_Succeeds confirm authentication
succeeds when client and server present matching bindings.
* ChannelBindings_Mismatched_{Endpoint,Unique}_FailsWithBadBinding confirm
the server returns NegotiateAuthenticationStatusCode.BadBinding when the
bindings differ.
Without the prior implementation change the mismatch tests fail with
status=Completed (no enforcement); with the fix they observe BadBinding.
The new file is conditionally compiled for non-Windows targets only,
matching the visibility of SafeChannelBindingHandle. The companion sources
(SafeChannelBindingHandle.cs, SecChannelBindings.cs) are added as Compile
items to the test csproj for the same target conditions.
Co-authored-by: Copilot <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 |
There was a problem hiding this comment.
Pull request overview
This PR extends Unix (GSSAPI) Negotiate/Kerberos server-side authentication to support TLS channel bindings by plumbing a channel binding token (CBT) from managed NegotiateAuthenticationPal.Unix into a new native shim (NetSecurityNative_AcceptSecContextEx) that forwards bindings to gss_accept_sec_context.
Changes:
- Add a new native export
NetSecurityNative_AcceptSecContextExand keep the existingNetSecurityNative_AcceptSecContextas a compatibility wrapper. - Update the Unix managed server path to pass
_channelBindingintoAcceptSecurityContext, dispatching to the CBT-aware native entrypoint when present. - Add Unix-only functional tests validating matching vs. mismatched channel bindings for Kerberos (endpoint/unique).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Net.Security.Native/pal_gssapi.h | Declares the new CBT-aware AcceptSecContext native shim. |
| src/native/libs/System.Net.Security.Native/pal_gssapi.c | Implements NetSecurityNative_AcceptSecContextEx and makes the legacy entrypoint delegate to it with no bindings. |
| src/native/libs/System.Net.Security.Native/entrypoints.c | Exposes the new native export via the resolver table. |
| src/libraries/Common/src/Interop/Unix/System.Net.Security.Native/Interop.NetSecurityNative.cs | Adds LibraryImport for NetSecurityNative_AcceptSecContextEx and an internal wrapper overload. |
| src/libraries/System.Net.Security/src/System/Net/NegotiateAuthenticationPal.Unix.cs | Plumbs server-side channel binding into AcceptSecurityContext; maps GSS_S_BAD_BINDINGS to BadBinding. |
| src/libraries/System.Net.Security/tests/FunctionalTests/System.Net.Security.Tests.csproj | Includes the new Unix-only channel binding test file and links production CBT helper types for tests. |
| src/libraries/System.Net.Security/tests/FunctionalTests/NegotiateAuthenticationKerberosTest.cs | Makes the Kerberos test class partial to allow adding a new test file. |
| src/libraries/System.Net.Security/tests/FunctionalTests/NegotiateAuthenticationKerberosTest.ChannelBindings.cs | Adds functional tests for matching/mismatched channel bindings (endpoint/unique). |
- Protect the ChannelBinding SafeHandle with DangerousAddRef/DangerousRelease around the native AcceptSecContext call to avoid a use-after-free if the binding is disposed concurrently. - Fail fast with GSS_S_BAD_BINDINGS in NetSecurityNative_AcceptSecContextEx when cbtSize is negative, preventing a negative-to-size_t cast that could make gss_accept_sec_context read past the buffer in Release builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 331f6905-5e70-4f82-9c67-63a939a13117
Instead of adding a separate NetSecurityNative_AcceptSecContextEx export, add the cbt/cbtSize parameters directly to NetSecurityNative_AcceptSecContext. Callers that don't use a channel binding token pass NULL/0. These native shims are only consumed by the in-repo managed interop layer, so there is no ABI compatibility concern that would require a separate entry point. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 331f6905-5e70-4f82-9c67-63a939a13117
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
src/native/libs/System.Net.Security.Native/pal_gssapi.h:156
NetSecurityNative_AcceptSecContexthas its exported signature changed (addscbt/cbtSize). This contradicts the PR description (which says a new...AcceptSecContextExshim is added and the existing entry point delegates for backwards compatibility) and can break in-place servicing scenarios where older managed code calls the old signature against a newer native library. Consider restoring the originalNetSecurityNative_AcceptSecContextsignature and introducing a newNetSecurityNative_AcceptSecContextExexport for channel bindings, with the old entry point delegating to the new one with NULL/0 bindings.
PALEXPORT uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
void* cbt,
int32_t cbtSize,
uint8_t* inputBytes,
uint32_t inputLength,
PAL_GssBuffer* outBuffer,
uint32_t* retFlags,
int32_t* isNtlmUsed);
src/native/libs/System.Net.Security.Native/pal_gssapi.c:414
- This implementation updates the
NetSecurityNative_AcceptSecContextexport to takecbt/cbtSize. If the intent is to preserve backward compatibility (as described in the PR), the existing export should keep its original signature and delegate to a newNetSecurityNative_AcceptSecContextExexport instead. As-is, a managed/native mismatch would be a hard runtime failure.
uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
void* cbt,
int32_t cbtSize,
uint8_t* inputBytes,
uint32_t inputLength,
PAL_GssBuffer* outBuffer,
uint32_t* retFlags,
int32_t* isNtlmUsed)
src/libraries/Common/src/Interop/Unix/System.Net.Security.Native/Interop.NetSecurityNative.cs:173
- The managed
LibraryImportforNetSecurityNative_AcceptSecContextnow includescbt/cbtSize, which relies on the native export’s signature having changed. If the goal is in-place servicing compatibility, consider keeping the oldNetSecurityNative_AcceptSecContextP/Invoke signature and adding a newAcceptSecContextExP/Invoke that includes channel bindings, mirroring the existingInitSecContext/InitSecContextExpattern.
[LibraryImport(Interop.Libraries.NetSecurityNative, EntryPoint = "NetSecurityNative_AcceptSecContext")]
private static partial Status AcceptSecContext(
out Status minorStatus,
SafeGssCredHandle acceptorCredHandle,
ref SafeGssContextHandle acceptContextHandle,
IntPtr cbt,
int cbtSize,
ref byte inputBytes,
int inputLength,
ref GssBuffer token,
out uint retFlags,
[MarshalAs(UnmanagedType.Bool)] out bool isNtlmUsed);
…ive path - Reject a malformed ChannelBinding (Size smaller than the SecChannelBindings header) in both the initiator and acceptor managed paths before doing pointer arithmetic, returning BadBinding instead of passing a negative size to interop. - Mirror the acceptor's cbtSize < 0 fail-fast guard in NetSecurityNative_InitSecContextEx so the initiator native path is equally protected against an out-of-bounds read on malformed input. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 331f6905-5e70-4f82-9c67-63a939a13117
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/native/libs/System.Net.Security.Native/pal_gssapi.h:156
- The PR description says a new
NetSecurityNative_AcceptSecContextExentry point is added and the existingNetSecurityNative_AcceptSecContextdelegates to it to preserve backward compatibility. In this header, the existing exportedNetSecurityNative_AcceptSecContextsignature is being extended withcbt/cbtSize, and there is no...AcceptSecContextExentry point. Extending an existing export changes the native ABI and does not preserve the old signature for in-place servicing / mixed-version scenarios.
Consider introducing a new export (e.g. NetSecurityNative_AcceptSecContextEx) and keeping NetSecurityNative_AcceptSecContext with its original signature as a wrapper that passes NULL/0 for CBT.
PALEXPORT uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
void* cbt,
int32_t cbtSize,
uint8_t* inputBytes,
uint32_t inputLength,
PAL_GssBuffer* outBuffer,
uint32_t* retFlags,
int32_t* isNtlmUsed);
filipnavara
left a comment
There was a problem hiding this comment.
Generally LGTM, there was just one small thing I noticed which could be pre-existing bug.
…nt binding in initiator - Initialize outBuffer/retFlags/isNtlmUsed to safe defaults on the cbtSize<0 fast-fail path in both NetSecurityNative_InitSecContextEx and NetSecurityNative_AcceptSecContext so no uninitialized data propagates back to managed code. - Bracket the initiator InitSecContext interop call with DangerousAddRef/ DangerousRelease on the ChannelBinding handle, mirroring the acceptor path, to avoid a use-after-free on concurrent dispose. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a829d7e2-feca-4d23-ac34-8e07b2bd71a5
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/native/libs/System.Net.Security.Native/pal_gssapi.h:152
- The PR description says a new
NetSecurityNative_AcceptSecContextExshim was added, but the code here changes the existing exportedNetSecurityNative_AcceptSecContextsignature to include CBT parameters. Given the existingInitSecContext/InitSecContextExpattern in this same header, consider keeping the oldAcceptSecContextABI (no CBT) and adding anAcceptSecContextExentrypoint instead (with the old one forwardingcbt = NULL, cbtSize = 0). This avoids native/managed version-skew issues and keeps the shim surface consistent.
PALEXPORT uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
void* cbt,
int32_t cbtSize,
uint8_t* inputBytes,
src/libraries/System.Net.Security/src/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicy.cs:154
OSSupportsExtendedProtectionis now true on Linux, but the comment still says "Managed implements only client-side", which is misleading now that the Unix server path flows channel bindings intoAcceptSecContext. Please update the comment to match the actual supported platforms/behavior (and fix the macOS casing/punctuation while here).
// ExtendedProtection is supported on all Windows versions supported by current .NET version.
// Linux is supported via GSSAPI (Managed implements only client-side).
// MacOS's Heimdal-derived GSS.framework does not reject a channel binding mismatch (the exchange completes successfully)
return OperatingSystem.IsWindows() || OperatingSystem.IsLinux();
src/libraries/System.Net.Security/tests/UnitTests/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicyTest.cs:60
- The unit tests validating
ExtendedProtectionPolicy.OSSupportsExtendedProtectionwere removed, but the property’s semantics changed (Linux is now supported). Please keep a small platform-gated assertion in this test file so regressions in the platform matrix get caught (Windows/Linux => true; everything else => false).
[Fact]
public void ExtendedProtectionPolicy_Properties()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/libraries/System.Net.Security/tests/UnitTests/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicyTest.cs:61
- Unit coverage for
ExtendedProtectionPolicy.OSSupportsExtendedProtectionwas removed, but the property’s semantics changed (Linux now returns true). The remaining tests only use the property as a condition, so they won’t catch accidental regressions in its value. Add a cross-platform assertion that verifies the property matches the intended OS matrix (Windows/Linux => true; other OSes => false).
[Fact]
public void ExtendedProtectionPolicy_Properties()
{
src/native/libs/System.Net.Security.Native/pal_gssapi.h:152
- The PR description says a new
NetSecurityNative_AcceptSecContextExshim was added, but the code changes the existingNetSecurityNative_AcceptSecContextexport signature instead. If the intent is to keep the native ABI additive (and match the description), consider keeping the old export as a wrapper (no CBT) and adding a new...Exentrypoint that takes CBT/size, updating managed interop to call the new symbol.
PALEXPORT uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
void* cbt,
int32_t cbtSize,
uint8_t* inputBytes,
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/libraries/System.Net.Security/src/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicy.cs:154
- The comment on
OSSupportsExtendedProtectionlooks outdated/misleading now that Linux server-side channel binding support exists in the Unix PAL. Consider updating the wording (and macOS capitalization/punctuation) to describe the actual supported platforms without referencing managed-only behavior.
// ExtendedProtection is supported on all Windows versions supported by current .NET version.
// Linux is supported via GSSAPI (Managed implements only client-side).
// MacOS's Heimdal-derived GSS.framework does not reject a channel binding mismatch (the exchange completes successfully)
return OperatingSystem.IsWindows() || OperatingSystem.IsLinux();
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/libraries/System.Net.Security/tests/FunctionalTests/NegotiateAuthenticationKerberosTest.ChannelBindings.cs:83
RunMismatchis missing braces, so theusingstatements are not inside a method body and this file will not compile. Wrap the body in{ ... }.
private static void RunMismatch(ChannelBindingKind kind)
using SafeChannelBindingHandle clientBinding = CreateChannelBinding(kind, hashSeed: 0x42);
using SafeChannelBindingHandle serverBinding = CreateChannelBinding(kind, hashSeed: 0x99);
src/libraries/System.Net.Security/src/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicy.cs:154
- The comment says Linux support is client-side only, but this PR adds server-side channel binding support on Unix via
AcceptSecContext. Please update the comment to avoid misleading future readers, and use the standard "macOS" casing / add punctuation.
// ExtendedProtection is supported on all Windows versions supported by current .NET version.
// Linux is supported via GSSAPI (Managed implements only client-side).
// MacOS's Heimdal-derived GSS.framework does not reject a channel binding mismatch (the exchange completes successfully)
return OperatingSystem.IsWindows() || OperatingSystem.IsLinux();
src/libraries/System.Net.Security/tests/UnitTests/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicyTest.cs:61
- The unit tests no longer cover the platform contract of
ExtendedProtectionPolicy.OSSupportsExtendedProtection. Since this PR changes the behavior to include Linux, it would be good to keep a simple regression test here to prevent accidental reverts.
[Fact]
public void ExtendedProtectionPolicy_Properties()
{
src/native/libs/System.Net.Security.Native/pal_gssapi.h:152
- The PR description mentions adding a new
NetSecurityNative_AcceptSecContextExshim, but the native export being changed here isNetSecurityNative_AcceptSecContext(signature extended with CBT pointer/size). Please update the PR description (or rename/add an...Exexport) so the implementation matches the stated approach.
/*
Shims the gss_accept_sec_context method.
*/
PALEXPORT uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
void* cbt,
int32_t cbtSize,
uint8_t* inputBytes,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/libraries/System.Net.Security/tests/UnitTests/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicyTest.cs:58
- The PR changes
ExtendedProtectionPolicy.OSSupportsExtendedProtectionsemantics (Linux now supported), but the unit tests that validatedOSSupportsExtendedProtectionwere removed rather than updated. This loses direct coverage of a public, behavior-affecting property and makes it easier for future changes to accidentally regress platform gating.
src/native/libs/System.Net.Security.Native/pal_gssapi.h:152 - The PR description says a new
NetSecurityNative_AcceptSecContextExshim is added, but the actual change extends the existingNetSecurityNative_AcceptSecContextexport/signature. This mismatch makes it harder to understand the intended compatibility story and can confuse future maintainers when searching for the "Ex" entrypoint.
PALEXPORT uint32_t NetSecurityNative_AcceptSecContext(uint32_t* minorStatus,
GssCredId* acceptorCredHandle,
GssCtxId** contextHandle,
void* cbt,
int32_t cbtSize,
uint8_t* inputBytes,
|
/ba-g Test failures are unrelated |
Adds a new
NetSecurityNative_AcceptSecContextExnative shim that takes achannel binding token (the application data portion of the
SecChannelBindingsbuffer) and forwards it to
gss_accept_sec_contextvia agss_channel_bindings_struct.The managed
NegotiateAuthenticationPal.Unixserver path now passes_channelBindingintoAcceptSecurityContext, which dispatches to thecbt-aware overload when bindings are present. GSSAPI surfaces a binding
mismatch as
GSS_S_BAD_BINDINGSso no additional validation is requiredon the managed side, matching the existing client (initiator) behavior.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com