-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Fix 4 problems in SASL kerberos #7008
Conversation
Ref: 7b29c28 |
@monnerat have you thought anything about how we can add tests for this? We have too much gss/kerberos code that we can't test... 😢 |
@jay: I have bcc'd captain-caveman2k on https://curl.se/mail/lib-2021-04/0103.html without reaction. Is he still active? @bagder: I've had some thinking about gss tests and gave up. I even doubt about an easy feasibility since it requires to have a valid ticket in the kerberos cache (obtained externally before use). |
If we'd have our own test server, such a ticket would be no problem. If we'd write stubs for all the gss functions to use when testing we could do other shortcuts. |
You mean something similar to libhostname.so, but for |
Sure, it is. But having this much functionality in curl and not even the smallest way to test and verify it is... crazy. I presume you've verified the functionality of your changes here by running curl code against some actual real servers? |
Sure. As part of implementing SASL (binary) in openldap, I've started to test the available authentication methods. |
Note that we already got some stubs at |
e648e57
to
6b797e0
Compare
I think these changes should've been done as separate PRs so that we could review and discuss the changes separately. Consider splitting things up in the future. Is this series ready for merge in your opinion? What does "auth: we do not support a security layer after kerberos authentication" mean for someone not into kerberos specifics? |
SASL GSSAPI features an optional transmission layer assuring data integrity/confidentiality. If negotiated, this layer is active after the negotiation succeeds, in the same way STARTTLS adds an encryption layer. There's a similar sequence in Lines 339 to 350 in a0709f9
|
Unless someone gripes about this, yes. This works for me and I don't plan other changes around it. |
7c9b63e
to
a62cf5d
Compare
Congratulations 🎉. DeepCode analyzed your code in 2.956 seconds and we found no issues. Enjoy a moment of no bugs ☀️. 👉 View analysis in DeepCode’s Dashboard | Configure the bot👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin. |
... instead of deriving it from active ticket.
RFC4752 Section 3.1 states "The authorization identity is not terminated with a zero-valued (%x00) octet". Although a comment in code said it may be needed anyway, nothing confirms it. In addition, servers may consider it as part of the identity, causing a failure.
... instead of deriving it from active ticket. Closes #7008
RFC4752 Section 3.1 states "The authorization identity is not terminated with a zero-valued (%x00) octet". Although a comment in code said it may be needed anyway, nothing confirms it. In addition, servers may consider it as part of the identity, causing a failure. Closes #7008
Thanks! |
Thanks for pulling! |
These are all in
Curl_auth_create_gssapi_security_message
, both in gssapi and sspi versions: