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
base: master
Are you sure you want to change the base?
Conversation
a0d34ca
to
7acadc0
Compare
"Linux / hyper" CI failed during |
7acadc0
to
5b8c0eb
Compare
Rebased to bring PR up-to-date with master due to time passed. Minor changes
|
d916f49
to
d776382
Compare
return CURLE_SSL_INVALIDCERTSTATUS; | ||
} | ||
} | ||
|
||
#if OPENSSL_VERSION_NUMBER >= 0x30000000L | ||
algo_type = EVP_MD_fetch(NULL, algo_name, NULL); |
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.
It’s best to cache the result of this, for performance reasons.
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.
Hi @DemiMarie! Thank you for continued review.
Where would it be best to cache this? I am honestly not too familiar with this level of curl internals. I am primarily working this as a continuation of the great work done by @steffe-kiess (#7196) to add support for SPNEGO GSSAPI, as it is something we critically need from command-line usage due to some web servers now enforcing this. Any pointer in right direction of caching architecture would be greatly appreciated.
I have found little material online regarding the performance impact of repeatedly calling EVP_MD_fetch
too much. The explicit fetching will only be done once per connection, during initial connect, so for most cases this function will only be called once. This is a also a completely new feature, as curl has been unable to ever connect to servers requiring secure channel binding. Therefore wondering if caching is something which could be done in a future patch if it turns out to be too much work.
Finally, regarding Explicit Fetching, I have not found any other usage of EVP_*_fetch
in the entire curl source code... All other places use EVP_sha256()
and similar implicit functions. I agree it could be a good idea to support explicit fetching, but I'd ideally not want this change to be a testing ground for this migration. Rather reverting back to the implicit-only implementation and then adding OpenSSL 3.x Explicit Fetching support in a separate PR as it touches many more areas outside of this scope. Thoughts?
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.
Yup, separate PR.
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.
OK! Do you wanna do explicit+caching in a separate PR, or only caching?
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.
I’m not a curl developer, just someone who noticed this PR while reading about the mess that OpenSSL 3.x is causing for HAProxy.
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>
a1a6fad
to
3d924fa
Compare
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.