s3:winbindd: don't use ads_kdestroy(NULL) in winbindd_raw_kerberos_login()
authorStefan Metzmacher <metze@samba.org>
Tue, 14 May 2024 07:02:07 +0000 (09:02 +0200)
committerStefan Metzmacher <metze@samba.org>
Tue, 14 May 2024 10:18:31 +0000 (10:18 +0000)
This fixes a problem introduced in the commit:

commit e6c693b705686a590d2fa8f434ff015d8926a349
Author: Stefan Metzmacher <metze@samba.org>
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 <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
source3/winbindd/winbindd_pam.c

index ec55cf0accb78c3fdb83ffa0a0dce56f808431b9..9764c874f77627a714ca3f370da7eea82d3333e6 100644 (file)
@@ -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",