creds: add sasl service name#21585
Closed
icing wants to merge 2 commits into
Closed
Conversation
The SASL service name, used in authentication, is part of curl's credentials when authenticating to a server/proxy. Make it part of `struct Curl_creds`. Change code to use `creds` to obtain a service name. By tying creds used to the connection, connection reuse is also only allowed when the service name matches.
bagder
approved these changes
May 13, 2026
There was a problem hiding this comment.
Pull request overview
This PR makes the SASL “service name” part of struct Curl_creds so that authentication (and connection reuse decisions) can consistently key off the service name tied to the connection’s credentials rather than looking at data->set.* each time.
Changes:
- Extends
struct Curl_credswithsasl_serviceand updatesCurl_creds_create()/comparison logic accordingly. - Updates NTLM/Digest/Negotiate (SPNEGO) and SOCKS5 GSSAPI code paths to derive the effective service name from
creds(falling back to a provided default). - Wires service-name options into credential creation for transfers and proxies, and adjusts several protocols to use
conn->creds.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/unit1304.c | Updates unit test helper to new Curl_creds_create() signature. |
| lib/creds.h | Adds sasl_service to Curl_creds and updates API/macros. |
| lib/creds.c | Implements storing/merging/comparing sasl_service in credentials. |
| lib/url.c | Passes service-name options into created creds and adjusts proxy/server credential creation paths. |
| lib/curl_sasl.c | Uses creds-carried service name when selecting SASL GSSAPI/NTLM service. |
| lib/vauth/vauth.h | Renames service parameter to default_service for updated call semantics. |
| lib/vauth/digest.c, lib/vauth/digest_sspi.c | Derives digest service from creds (fallback to default). |
| lib/vauth/ntlm.c, lib/vauth/ntlm_sspi.c | Derives NTLM service from creds (fallback to default). |
| lib/vauth/krb5_gssapi.c, lib/vauth/krb5_sspi.c | Derives Kerberos service from creds (fallback to default). |
| lib/vauth/spnego_gssapi.c, lib/vauth/spnego_sspi.c | Derives SPNEGO service from creds (fallback to default). |
| lib/http_ntlm.c | Stops reading service name from data->set.*; relies on creds + default. |
| lib/http_negotiate.c | Stops reading service name from data->set.*; relies on creds + default. |
| lib/socks.h, lib/socks.c | Extends SOCKS5 GSSAPI negotiate API to accept creds. |
| lib/socks_gssapi.c, lib/socks_sspi.c | Uses creds-carried service name (fallback to "rcmd"). |
| lib/pop3.c | Switches login credential presence check to conn->creds. |
| lib/imap.c | Switches login credential presence check to conn->creds. |
| lib/openldap.c | Switches bind credential source to conn->creds. |
| lib/netrc.c | Updates call to Curl_creds_create() for new signature. |
Comments suppressed due to low confidence (1)
lib/openldap.c:352
- This function now uses
conn->credsfor the bind DN/password, but the error mapping below still checksdata->state.credsto decide betweenCURLE_LOGIN_DENIEDandCURLE_LDAP_CANNOT_BIND. On connection reuse it’s plausible forconn->credsto be set whiledata->state.credsis NULL, leading to the wrong error code. Useconn->credsconsistently for this decision.
if(conn->creds) {
binddn = Curl_creds_user(conn->creds);
passwd.bv_val = CURL_UNCONST(Curl_creds_passwd(conn->creds));
passwd.bv_len = strlen(passwd.bv_val);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bagder
approved these changes
May 13, 2026
outcast36
pushed a commit
to greearb/curl
that referenced
this pull request
Jun 3, 2026
The SASL service name, used in authentication, is part of curl's credentials when authenticating to a server/proxy. Make it part of `struct Curl_creds`. Change code to use `creds` to obtain a service name. By tying creds used to the connection, connection reuse is also only allowed when the service name matches. Closes curl#21585
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SASL service name, used in authentication, is part of curl's credentials when authenticating to a server/proxy. Make it part of
struct Curl_creds.Change code to use
credsto obtain a service name. By tying creds used to the connection, connection reuse is also only allowed when the service name matches.