Skip to content

Fix enabling SASL_SEC_NONSTD_CBIND#882

Open
flowerysong wants to merge 1 commit intocyrusimap:masterfrom
flowerysong:nonstd_cbind
Open

Fix enabling SASL_SEC_NONSTD_CBIND#882
flowerysong wants to merge 1 commit intocyrusimap:masterfrom
flowerysong:nonstd_cbind

Conversation

@flowerysong
Copy link
Contributor

Fixes #853

This security flag is supposed to allow explicitly enabling channel bindings for the GSSAPI mechanism. However, if I set this flag in the test programs it results in the mechanism being rejected when the library checks to see if it's suitable. Adding it to the flags defined in the plugins fixed the tests, but also appeared to revert to channel bindings being enabled by default.

The only way I was able to get the tests working and still require explicit enablement was to ignore this flag while doing the comparisons, which feels wrong but it's been over a year and no one has offered an alternative solution so maybe it's correct.

This security flag is supposed to allow explicitly enabling channel
bindings for the GSSAPI mechanism. However, if I set this flag in the
test programs it results in the mechanism being rejected when the
library checks to see if it's suitable. Adding it to the flags defined
in the plugins fixed the tests, but also appeared to revert to channel
bindings being enabled by default.

The only way I was able to get the tests working and still require
explicit enablement was to ignore this flag while doing the
comparisons, which feels wrong but it's been over a year and no one
has offered an alternative solution.

Signed-off-by: Paul Arthur <paul.arthur@flowerysong.com>
@GuidoKiener
Copy link
Contributor

@flowerysong Did you see my proposal (#853 (comment)) for an alternative solution?

@flowerysong
Copy link
Contributor Author

flowerysong commented Nov 8, 2025

@GuidoKiener I have an interest in fixing the test suite, which is currently broken because GSSAPI channel bindings are no longer enabled by default, and cannot be enabled by setting the flag that is supposed to enable them. This change fixes the existing, broken functionality. You appear to be proposing a more comprehensive overhaul of how channel bindings work, which is not something I have an opinion on. That would require me to know what they are and care how they are used.

@GuidoKiener
Copy link
Contributor

GuidoKiener commented Nov 9, 2025

(English text is revised by Chatgpt. Thanks!)

@flowerysong, @rjbs

Why Channel Binding Is Important

Channel binding helps detect “man-in-the-middle” (MITM) attacks. Some organizations or tools can intercept encrypted connections between a client and a server by using forged certificates. Channel binding mitigates such attacks because the client and server incorporate shared, cryptographically bound secrets into the authentication process — secrets that a MITM attacker cannot reproduce without brute force.


Test Suite and GSSAPI

I assume the test suite currently works with Cyrus SASL version 2.1.28, but not version 2.2.0.
In plugins/gssapi.c, the feature flag SASL_FEAT_CHANNEL_BINDING is not present — which is correct, since channel binding is not defined for GSSAPI or GSS-SPNEGO. See:

Enabling SASL_FEAT_CHANNEL_BINDING for GSSAPI or GSS-SPNEGO would incorrectly advertise the mechanisms GSSAPI-PLUS or GSS-SPNEGO-PLUS, which are not valid SASL mechanisms. Instead GS2-KRB5-PLUS is a valid SASL mechanism for GSSAPI with channel binding.


OpenLDAP

Channel binding support for GSSAPI was introduced by 975edbb from @simo5 on the main branch, sparking a discussion about its correctness.
While I am not an OpenLDAP expert, I know that this implementation violates RFC 4752. The feature may be useful for LDAP interoperability with Microsoft servers, but if enabled for other services (e.g., IMAP), it can conflict with other mail clients or servers.

A related article from Microsoft provides further context:
LDAP Channel Binding and LDAP Signing Requirements (KB4520412)

Recommendation: Remove the SASL_FEAT_CHANNEL_BINDING flags from GSSAPI and GSS-SPNEGO to prevent the advertisement of invalid mechanisms like GSSAPI-PLUS or GSS-SPNEGO-PLUS.


SASL_SEC_NONSTD_CBIND

A later fix by @hyc 8735185 disabled channel binding for GSSAPI by default.
However, this patch does not prevent the announcement of GSSAPI-PLUS or GSS-SPNEGO-PLUS, leading to inconsistencies — for example, GSSAPI-PLUS is advertised when channel binding is set, but if SASL_SEC_NONSTD_CBIND is not enabled then it will cause authentication failures due to missing channel binding data in a *-PLUS mechanism. How can a server find out wich channel binding type (none, tls-exporter, tls-unique, tls-server-end-point) was selected by the client? How does a client find out which types are supported by the server?

Recommendation:
Since SASL_SEC_XXX flags are intended to increase security and limit the set of allowed SASL mechanisms, introducing the SASL_SEC_NONSTD_CBIND flag to enable channel binding for GSSAPI is not advisable. It may interfere with applications that rely on these flags to restrict available mechanisms.
Please consider reverting this commit.


How to Proceed

If you agree that channel binding provides value, we can discuss a cleaner proposal and define the next steps together.

@flowerysong
Copy link
Contributor Author

@GuidoKiener I'm not going to read that, since you couldn't be bothered to write it. I don't think any part of my answer was unclear or invited you to vomit text at me, but I'll restate: I don't have a position on whether channel binding support should be reworked, and I will not be putting in the effort to form an opinion.

There is currently a feature in cyrus-sasl that is covered by the test suite and does not work. This PR fixes that feature, and your proposal, while it would have implications for that feature, is not an alternative code fix for that feature so it is not relevant to my interests.

@GuidoKiener
Copy link
Contributor

@flowerysong : Hmm, it's a pity. I spent some time to write this summary for you and the community. Chatgpt only does a nice rewording of my text. Nevertheless, it's not my project and I wish you good luck!

@rjbs
Copy link
Contributor

rjbs commented Nov 17, 2025

The issue here is not thinking that channel binding is without value. It is these facts together:

  • there is no stable release of cyrus-sasl that includes the features required
  • the current state of cyrus-sasl is that the tests do not pass and its "safe to release" status is unknown
  • the developers of cyrus-imapd are not the same people as the developers of cyrus-sasl

So I am trying to figure out how a safe and responsible release of cyrus-sasl can get made, but until that happens, this ticket is effectively blocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SASL_SEC_NONSTD_CBIND can't be enabled?

3 participants