From: Stefan Metzmacher Date: Tue, 14 May 2024 07:02:07 +0000 (+0200) Subject: s3:winbindd: don't use ads_kdestroy(NULL) in winbindd_raw_kerberos_login() X-Git-Tag: tdb-1.4.11~799 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4959f932279105e1de7c0bdf11ea503e1967a341;p=thirdparty%2Fsamba.git s3:winbindd: don't use ads_kdestroy(NULL) in winbindd_raw_kerberos_login() This fixes a problem introduced in the commit: commit e6c693b705686a590d2fa8f434ff015d8926a349 Author: Stefan Metzmacher Date: Wed Feb 28 17:28:43 2024 +0100 s3:winbindd: pass a NULL ccache to kerberos_return_pac() for a MEMORY ccache It means kerberos_return_pac() will use smb_krb5_cc_new_unique_memory(). ... Before that commit cc was never NULL as generate_krb5_ccache() returned "MEMORY:winbindd_pam_ccache" as fallback. So we called ads_kdestroy("MEMORY:winbindd_pam_ccache"). Now we have cc == NULL if user_ccache_file == NULL. and kerberos_return_pac() uses smb_krb5_cc_new_unique_memory() and krb5_cc_destroy() internally. It means unless user_ccache_file != NULL we should not call ads_kdestroy(cc) as cc is NULL and means we would destroy any global default krb5 ccache. Review with: git show -U25 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider --- diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c index ec55cf0accb..9764c874f77 100644 --- a/source3/winbindd/winbindd_pam.c +++ b/source3/winbindd/winbindd_pam.c @@ -739,7 +739,6 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx, { #ifdef HAVE_KRB5 NTSTATUS result = NT_STATUS_UNSUCCESSFUL; - krb5_error_code krb5_ret; const char *cc = NULL; const char *principal_s = NULL; char *realm = NULL; @@ -851,6 +850,11 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx, DEBUG(10,("winbindd_raw_kerberos_login: uid is %d\n", uid)); } + /* + * Note cc can be NULL, it means + * kerberos_return_pac() will use + * a temporary krb5 ccache internally. + */ result = kerberos_return_pac(mem_ctx, principal_s, pass, @@ -939,18 +943,8 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx, DEBUG(10,("winbindd_raw_kerberos_login: failed to add ccache to list: %s\n", nt_errstr(result))); } - } else { - - /* need to delete the memory cred cache, it is not used anymore */ - - krb5_ret = ads_kdestroy(cc); - if (krb5_ret) { - DEBUG(3,("winbindd_raw_kerberos_login: " - "could not destroy krb5 credential cache: " - "%s\n", error_message(krb5_ret))); - } - } + *info6 = info6_copy; return NT_STATUS_OK; @@ -969,13 +963,6 @@ failed: * local host and therefore didn't get the PAC, we need to remove that * cache entirely now */ - krb5_ret = ads_kdestroy(cc); - if (krb5_ret) { - DEBUG(3,("winbindd_raw_kerberos_login: " - "could not destroy krb5 credential cache: " - "%s\n", error_message(krb5_ret))); - } - if (!NT_STATUS_IS_OK(remove_ccache(user))) { DEBUG(3,("winbindd_raw_kerberos_login: " "could not remove ccache for user %s\n",