-
-
Notifications
You must be signed in to change notification settings - Fork 7k
gssapi: make channel binding conditional on GSS_C_CHANNEL_BOUND_FLAG #19164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gssapi: make channel binding conditional on GSS_C_CHANNEL_BOUND_FLAG #19164
Conversation
Fixes curl#19109 - GSSAPI authentication fails on macOS with Apple's Heimdal implementation which lacks GSS_C_CHANNEL_BOUND_FLAG support for TLS channel binding. Commit 0a5ea09 introduced TLS channel binding for SPNEGO/GSSAPI authentication unconditionally, but Apple's Heimdal fork (used on macOS) does not support this feature, causing "unsupported mechanism" errors when authenticating to corporate HTTP services with Kerberos. Solution: - Add CURL_GSSAPI_HAS_CHANNEL_BINDING detection in curl_gssapi.h based on GSS_C_CHANNEL_BOUND_FLAG presence (MIT Kerberos >= 1.19) - Make negotiatedata.channel_binding_data field conditional in vauth.h - Guard channel binding collection/cleanup in http_negotiate.c - Guard channel binding usage in spnego_gssapi.c This follows the same pattern as GSS_C_DELEG_POLICY_FLAG detection and ensures graceful degradation when channel binding is unavailable while maintaining full support for implementations that have it. Changes: - lib/curl_gssapi.h: Add feature detection macro - lib/vauth/vauth.h: Make struct field conditional - lib/http_negotiate.c: Conditional init/cleanup (2 locations) - lib/vauth/spnego_gssapi.c: Conditional channel binding usage Tested on macOS with Apple Heimdal (no channel binding) and Linux with MIT Kerberos (with channel binding). Both configurations authenticate successfully without errors.
lib/curl_gssapi.h
Outdated
| /* Detect if GSSAPI supports channel binding. | ||
| * GSS_C_CHANNEL_BOUND_FLAG is present in MIT Kerberos >= 1.19 and modern | ||
| * Heimdal, but missing in Apple's Heimdal fork used on macOS. | ||
| */ | ||
| #ifdef GSS_C_CHANNEL_BOUND_FLAG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /* Detect if GSSAPI supports channel binding. | |
| * GSS_C_CHANNEL_BOUND_FLAG is present in MIT Kerberos >= 1.19 and modern | |
| * Heimdal, but missing in Apple's Heimdal fork used on macOS. | |
| */ | |
| #ifdef GSS_C_CHANNEL_BOUND_FLAG | |
| #ifdef GSS_C_CHANNEL_BOUND_FLAG /* MIT Kerberos 1.19+, missing from GNU GSS */ |
Since curl doesn't support Heimdal anymore, I figure the GNU GSS
information is the relevant one. Also matching the markups we
already use for GSS_C_DELEG_POLICY_FLAG.
And move the explanation (incl. Heimdal) into the PR message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also noting that Kerberos API provided by Apple was deprecated by
them since OS X Lion 10.7 from 2011.
edit: it reports itself as 1.7-prerelease (on macOS Sequoia). That version is from year 2009.
Addressing review feedback to use inline comment style consistent with other feature detection patterns in the codebase (e.g., GSS_C_DELEG_POLICY_FLAG in curl_gssapi.c). The detailed explanation about Apple's deprecated Heimdal fork has been moved to the PR description for better context without cluttering the code. Ref: curl#19109
|
@vszakats Thank you for the review and the suggestion. I've updated the code to match your recommendation: Changes made:
The inline comment now focuses on the relevant information (GNU GSS) while keeping the code concise, consistent with curl's style. |
|
@devdattatalele Thanks. The CI fails are due to unused variables. They should |
|
Thanks for the suggestion @vszakats , let me know if you have any further changes! |
|
As the CI builds show, some builds now error like this: |
The 'chan' variable is only used when CURL_GSSAPI_HAS_CHANNEL_BINDING is defined. Wrap its declaration in the same conditional compilation guard to fix -Werror=unused-variable build failures on systems without GSS_C_CHANNEL_BOUND_FLAG support (Apple Heimdal, GNU GSS).
|
Fixed the CI build failure. The |
|
Thanks! |
…fined The GSS_C_CHANNEL_BOUND_FLAG is only in gssapi_ext.h, which the previous PR didn't include. This broke channel binding on systems with MIT Kerberos 1.19+. Fixed regression from curl#19164
…fined The GSS_C_CHANNEL_BOUND_FLAG is only in gssapi_ext.h, which the previous PR didn't include. This broke channel binding on systems with MIT Kerberos 1.19+. Fixed regression from curl#19164
|
@devdattatalele This PR introduced a regression in SecureChannelBinding functionality. You are depending on a new @bagder Additionally this entire PR reeks of AI. Especially sentences such as |
…fined The GSS_C_CHANNEL_BOUND_FLAG is only in gssapi_ext.h, which the previous PR didn't include. This broke channel binding on systems with MIT Kerberos 1.19+. Fixed regression from curl#19164
Ref: curl#19603 Follow-up to 8616e5a curl#19164
Ref: curl#19603 Follow-up to 8616e5a curl#19164
Use the already detected `gssapi/gssapi_krb5.h` MIT Kerberos header to pull in `gssapi_ext.h`, which in turn sets `GSS_C_CHANNEL_BOUND_FLAG` if supported. Channel binding is present in MIT Kerberos 1.19+. Also: - lib: de-duplicate GSS-API header includes. - vauth: de-duplicate `urldata.h` includes. - drop interim feature macro in favor of the native GSS one. Assisted-by: Max Faxälv Reported-by: Max Faxälv Bug: #19164 (comment) Follow-up to 8616e5a #19164 Closes #19603 Closes #19760
Fixes #19109
Problem
GSSAPI authentication fails on macOS when curl is built with
--with-openssland--with-gssapi, producing the error:Root Cause
Commit 0a5ea09 (PR #13098) introduced TLS channel binding for SPNEGO/GSSAPI authentication unconditionally. However, Apple's Heimdal GSSAPI implementation (used on macOS) does not support the
GSS_C_CHANNEL_BOUND_FLAGrequired for channel binding, causing authentication to fail.The PR notes stated "This change require krb5 >= 1.19", but macOS uses Apple's Heimdal fork which lacks this feature even in current versions.
Background on Apple's Heimdal Fork
Apple's implementation is a fork of Heimdal that has been deprecated since OS X Lion 10.7 (2011). The version distributed with macOS is based on Heimdal 1.7-prerelease from 2009 and lacks modern GSSAPI features including
GSS_C_CHANNEL_BOUND_FLAG. While Apple deprecated this API over a decade ago in favor of their proprietary GSS framework, the standard GSSAPI headers remain available for compatibility, causing curl's build system to detect and use them.Similarly, GNU GSS (an alternative GSSAPI implementation) also lacks
GSS_C_CHANNEL_BOUND_FLAGsupport.Solution
Make GSSAPI channel binding conditional based on
GSS_C_CHANNEL_BOUND_FLAGavailability:CURL_GSSAPI_HAS_CHANNEL_BINDINGdetection macro incurl_gssapi.hnegotiatedata.channel_binding_datafield conditional invauth.hhttp_negotiate.c(2 locations)spnego_gssapi.cThis follows the same pattern as
GSS_C_DELEG_POLICY_FLAGdetection (seelib/curl_gssapi.c) and ensures graceful degradation when channel binding is unavailable while maintaining full support for implementations that have it.Changes
lib/curl_gssapi.h: Add feature detection macro with inline comment (addressing review feedback from @vszakats)lib/vauth/vauth.h: Make struct field conditionallib/http_negotiate.c: Conditional init/cleanuplib/vauth/spnego_gssapi.c: Conditional channel binding usageTesting
Compatibility
SECPKG_ATTR_ENDPOINT_BINDINGS) unaffectedSecurity
Channel binding provides additional security by tying GSSAPI authentication to the specific TLS channel, mitigating relay attacks. This change: