ldap: support empty username and password - #22196
Conversation
|
I'm not entirely sure my use of Curl_creds_user/passwd was correct because if credentials doesn't exist then an empty username and password would be used. But if there are no credentials, then the user didn't specify an empty user pass right? Lines 80 to 81 in 68720b4 To really get the old behavior there doesn't seem to be a macro for that, so I'm changing my proposal: diff --git a/lib/ldap.c b/lib/ldap.c
index e385245..ae12727 100644
--- a/lib/ldap.c
+++ b/lib/ldap.c
@@ -282,8 +282,8 @@ static CURLcode ldap_do(struct Curl_easy *data, bool *done)
#else
char *host = NULL;
#endif
- const char *user = Curl_creds_user(data->state.creds);
- const char *passwd = Curl_creds_passwd(data->state.creds);
+ const char *user = data->state.creds ? data->state.creds->user : NULL;
+ const char *passwd = data->state.creds ? data->state.creds->passwd : NULL;
struct ip_quadruple ipquad;
bool is_ipv6;
BerElement *ber = NULL; |
|
Originally, the was how It's a funny world... I'll update CREDS.md that still describes the old behaviour. |
| data->state.creds->user : NULL; | ||
| const char *passwd = Curl_creds_has_passwd(data->state.creds) ? | ||
| data->state.creds->passwd : NULL; | ||
| const char *user = data->state.creds ? data->state.creds->user : NULL; |
There was a problem hiding this comment.
Hmm, user is alwas passed to ldap_win_bind() and the reporter says that passing NULL does not work. (probably the reason why they use -u : in the first place?
Maybe always using Curl_creds_user() is best here?
There was a problem hiding this comment.
Maybe always using
Curl_creds_user()is best here?
Ref: https://github.com/curl/curl/blob/curl-8_21_0/lib/ldap.c#L153-L231
For Windows native LDAP builds of curl the behavior of the NULL, NULL seems to be dependent on whether or not it was also built with Windows SSPI support. If it was then the NULL, NULL will cause the current user's Windows credentials to be used:
Lines 197 to 201 in 68720b4
I think we'd have to keep the old behavior and not use Curl_creds_user, since if there is no creds struct then we'd still want to pass on NULL to get to that block?
There was a problem hiding this comment.
Whatever works. You have a better idea of this.
There was a problem hiding this comment.
I think that the behavior is not entirely correct for the SSPI case but it would need a different fix. My plan is this PR would restore the old behavior and then I'd fix the -u : case for SSPI with some auth (negotiate, etc) separately. It's a little more complicated than I thought, see #22230
Prior to this change an empty username or password was passed to the LDAP bind function as NULL instead of an empty string. Regression since 8f71d0f. Reported-by: Yoshiro Yoneya Fixes curl#22162 Closes #xxxx
5373059 to
9466b72
Compare
2 participants
Prior to this change an empty username or password was passed to the LDAP bind function as NULL instead of an empty string.
Regression since 8f71d0f.
Reported-by: Yoshiro Yoneya
Fixes #22162
Closes #xxxx