Skip to content
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

Add callback SASL_CB_SERVER_CHANNEL_BINDING #824

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

GuidoKiener
Copy link
Contributor

Provide a callback function to change the channel binding type of servers (e.g. "tls-server-end-point" instead of "tls-unique") during authentication of secure mechanisms like GS2-KRB5-PLUS or SCRAM-SHA-256-PLUS.

The callback is used by the plugins SCRAM and GS2 when the desired binding type of the client does not match the binding type set with property SASL_CHANNEL_BINDING.
A server can check the requested type of channel binding and overwrite the current channel binding data with the property SASL_CHANNEL_BINDING before the authentication proceeds.

Issue #823

This patch is required to support channel binding for "tls-server-end-point" in cyrus-imapd.
See example: GuidoKiener/cyrus-imapd@5e0af18
Note that the example is not complete and requires more discussion about handling of certificates with signature algorithm SHA224, ED25519, and ED448.

@Neustradamus
Copy link
Contributor

@GuidoKiener: I only discover this PR today! :)

No three possibilities?

cc: @cyrusimap team, @aamelnikov.

@GuidoKiener
Copy link
Contributor Author

@GuidoKiener: I only discover this PR today! :)

No three possibilities?
Which three possibilities? With the callback you have unlimited possibilities.

@quanah quanah requested a review from hyc July 22, 2024 16:41
@quanah quanah added this to the 2.2.0 milestone Jul 22, 2024
@hyc
Copy link
Contributor

hyc commented Jul 22, 2024

The patches look OK but I think the example should be attached in a doc or README or something. The usage is not at all obvious without the example.

@GuidoKiener
Copy link
Contributor Author

The patches look OK but I think the example should be attached in a doc or README or something. The usage is not at all obvious without the example.

I agree that an example or good README is always helpful. I also did not find a real SCRAM example in this repo with openssl and real channel binding like "tls-unique". Since I got no feedback about this patch for five months, I thought that nobody is interested in this security extension.
If you are willing to merge this branch, I can extend the simple example in

if (cb_flag) {
and sample/server.c to show how a user can deal with two different types of channel binding. A real implementation can only be provided in cyurs-imapd (see example GuidoKiener/cyrus-imapd@5e0af18) which is driven by future security updates and discussion.
Does it work for you when I provide this example within two weeks?

@quanah
Copy link
Contributor

quanah commented Jul 24, 2024

Does it work for you when I provide this example within two weeks?

That would be great, thank you very much!

Provide a callback function to change the channel binding type
of servers (e.g. "tls-server-end-point" instead of "tls-unique")
during authentication of secure mechanisms like GS2-KRB5-PLUS or
SCRAM-SHA-256-PLUS.

The callback is used by the plugins SCRAM and GS2 when the desired
binding type of the client does not match the binding type set with
property SASL_CHANNEL_BINDING.
A server can check the requested type of channel binding and
overwrite the current channel binding data with the property
SASL_CHANNEL_BINDING before the authentication proceeds.

Issue cyrusimap#823

Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
With the callback SASL_CB_SERVER_CHANNEL_BINDING
a server can change the channel binding data to the
desired channel binding type of the client.

sample/client.c
Select the option -d or -D to use the channel
binding type "tls-server-end-point".
Or select the option -c or -C to use the channel
binding type "tls-unique".
Options -C or -D force client and server to use
channel binding.

sample/server.c
select option -c to activate channel binding
type "tls-unique".
Option -C forces clients to use channel binding.
Via callback "my_select_binding" the server changes
the binding type to "tls-server-end-point" if
required.

Note that client/server must use the binding type:
- "tls-exporter" for TLS 1.3
- "tls-unique" for TLS 1.2
- "tls-server-end-point" works for TLS 1.2 and 1.3

Issue cyrusimap#823

Signed-off-by: Guido Kiener <guido@kiener-muenchen.de>
@GuidoKiener
Copy link
Contributor Author

GuidoKiener commented Aug 9, 2024

The patches look OK but I think the example should be attached in a doc or README or something. The usage is not at all obvious without the example.

@hyc , @quanah : A sample code is now provided. Here is an output when the client selects option -D with type 'tls-server-end-point':

$ ./client -D -m SCRAM-SHA-256-PLUS localhost
receiving capability list... recv: {92}
SCRAM-SHA-512-PLUS SCRAM-SHA-384-PLUS SCRAM-SHA-256-PLUS SCRAM-SHA-224-PLUS SCRAM-SHA-1-PLUS
SCRAM-SHA-512-PLUS SCRAM-SHA-384-PLUS SCRAM-SHA-256-PLUS SCRAM-SHA-224-PLUS SCRAM-SHA-1-PLUS
please enter an authentication id: guido
please enter an authorization id: guido
Password: 
send: {13}
SCRAM-SHA-256
send: {1}
Y
send: {73}
p=tls-server-end-point,a=guido,n=guido,r=FfiOkfowrbkAVZ1SpiCMVDaS+aLZDqm+
recv: {120}
r=FfiOkfowrbkAVZ1SpiCMVDaS+aLZDqm+BaiACgb6wd/Uq8A0k9P4+79mMR81ISwM,s=dcTGeYUJKRw2rzkiXZu2QM9QeDSCYDaJLb3zhHPJcR0=,i=4096
send: {184}
c=cD10bHMtc2VydmVyLWVuZC1wb2ludCxhPWd1aWRvLHVzZSBYNTA5X2RpZ2VzdCgpAA==,r=FfiOkfowrbkAVZ1SpiCMVDaS+aLZDqm+BaiACgb6wd/Uq8A0k9P4+79mMR81ISwM,p=696gLecQi1RVU+2jZOES7IA3uihIQ7wT1VEq1rc9c+Y=
recv: {46}
v=cHhVu6M81oUGC2/NnZe7AxNpUx8dssMDRPO33Xc9+3g=
send: {0}

successful authentication
closing connection

Does it solve your issue #800 ?
I just looked at /docsrc/sasl/... The documentation is out of sync with the current implemenation. I could give my 5 cents, but do not want to do the whole job if there is much extra work. Is there someone who feels responsible for the documentation?

@quanah
Copy link
Contributor

quanah commented Aug 29, 2024

Thank you very much! We will look at this as soon as we can, real life having some impact atm.

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.

5 participants