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

spnego_gssapi: implement TLS channel bindings for openssl #13098

Closed
wants to merge 10 commits into from

Conversation

Foorack
Copy link

@Foorack Foorack commented Mar 11, 2024

Channel Bindings are used to tie the session context to a specific TLS channel. This is to provide additional proof of valid identity, mitigating authentication relay attacks.

Major web servers have the ability to require (None/Accept/Require) GSSAPI channel binding, rendering Curl unable to connect to such websites unless support for channel bindings is implemented.

IIS calls this feature Extended Protection (EPA), which is used in Enterprise environments using Kerberos for authentication.

This change require krb5 >= 1.19, otherwise channel bindings won't be forwarded through SPNEGO.

@Foorack
Copy link
Author

Foorack commented Mar 12, 2024

"Linux / hyper" CI failed during install hyper step, due to compile error in Hyper and was fixed in hyperium/hyper@4d0c184

lib/vtls/openssl.c Outdated Show resolved Hide resolved
@Foorack Foorack force-pushed the openssl-spnego-channel-binding branch from 7acadc0 to 5b8c0eb Compare April 23, 2024 13:11
@Foorack
Copy link
Author

Foorack commented Apr 23, 2024

Rebased to bring PR up-to-date with master due to time passed.

Minor changes

  • ossl_ssl_backend_data had in upstream been renamed to ossl_ctx, and ->handle to ->ssl
  • changed u_char* to unsigned char* to be in line with rest of project

@Foorack Foorack force-pushed the openssl-spnego-channel-binding branch 2 times, most recently from d916f49 to d776382 Compare April 24, 2024 08:24
lib/vtls/openssl.c Outdated Show resolved Hide resolved
@Foorack
Copy link
Author

Foorack commented Jun 18, 2024

Reverted Explicit Fetching implementation, for now, in order to be in-line with the rest of CURL source code. Potential migration from Implicit to Explicit Fetching is outside the scope of this specific PR.

Also resolved merge conflicts to keep branch up-to-date.

@Foorack
Copy link
Author

Foorack commented Jun 18, 2024

@bagder We've been running this change in Production on hundreds of clients for several months, and it's been working great. What is the next step for this to be merged?

lib/vtls/vtls.c Outdated Show resolved Hide resolved
lib/vtls/openssl.c Outdated Show resolved Hide resolved
SGA-max-faxalv and others added 10 commits August 7, 2024 07:26
Channel Bindings are used to tie the session context to a specific
TLS channel. This is to provide additional proof of valid identity,
mitigating authentication relay attacks.

Major web servers have the ability to require (None/Accept/Require)
GSSAPI channel binding, rendering Curl unable to connect to such
websites unless support for channel bindings is implemented.

IIS calls this feature Extended Protection (EPA), which is used in
Enterprise environments using Kerberos for authentication.

This change require krb5 >= 1.19, otherwise channel bindings won't be
forwarded through SPNEGO.

Co-Authored-By: Steffen Kieß <947515+steffen-kiess@users.noreply.github.com>
@Foorack Foorack force-pushed the openssl-spnego-channel-binding branch from 3c771e4 to be3512c Compare August 7, 2024 05:26
@Foorack
Copy link
Author

Foorack commented Aug 7, 2024

Some tests failed because Launchpad was temporarily down.

@Foorack
Copy link
Author

Foorack commented Aug 7, 2024

All tests pass now!

Copy link
Member

@MarcelRaad MarcelRaad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks! It's a pity that nothing can be shared with the existing SSPI implementation as that's based on SSPI-specific types. I would appreciate more reviews from others interested in the topic (not necessarily curl maintainers).

@bagder bagder closed this in 0a5ea09 Aug 12, 2024
@bagder
Copy link
Member

bagder commented Aug 12, 2024

Thanks!

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

Successfully merging this pull request may close these issues.

5 participants