From ead2b6e03dfcb18849c820c48b1efda1414dbaec Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Thu, 7 Jul 2022 11:22:05 +0200 Subject: [PATCH 1/2] s3:winbind: Create service principal inside add_ccache_to_list() The function can build the service principal itself, there is no need to do it in the caller. This removes code duplication. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14979 Signed-off-by: Samuel Cabrero Reviewed-by: Andreas Schneider (cherry picked from commit 8bef8e3de9fc96ff45319f80529e878977563f3a) --- source3/winbindd/winbindd_cred_cache.c | 16 +++++++++------- source3/winbindd/winbindd_pam.c | 14 -------------- source3/winbindd/winbindd_proto.h | 1 - 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/source3/winbindd/winbindd_cred_cache.c b/source3/winbindd/winbindd_cred_cache.c index 6c65db6a73f..2fbef1ec8a9 100644 --- a/source3/winbindd/winbindd_cred_cache.c +++ b/source3/winbindd/winbindd_cred_cache.c @@ -493,7 +493,6 @@ bool ccache_entry_identical(const char *username, NTSTATUS add_ccache_to_list(const char *princ_name, const char *ccname, - const char *service, const char *username, const char *pass, const char *realm, @@ -613,12 +612,6 @@ NTSTATUS add_ccache_to_list(const char *princ_name, goto no_mem; } } - if (service) { - entry->service = talloc_strdup(entry, service); - if (!entry->service) { - goto no_mem; - } - } if (canon_principal != NULL) { entry->canon_principal = talloc_strdup(entry, canon_principal); if (entry->canon_principal == NULL) { @@ -642,6 +635,15 @@ NTSTATUS add_ccache_to_list(const char *princ_name, goto no_mem; } + entry->service = talloc_asprintf(entry, + "%s/%s@%s", + KRB5_TGS_NAME, + realm, + realm); + if (entry->service == NULL) { + goto no_mem; + } + entry->create_time = create_time; entry->renew_until = renew_until; entry->uid = uid; diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c index 5505220335f..d574834ba94 100644 --- a/source3/winbindd/winbindd_pam.c +++ b/source3/winbindd/winbindd_pam.c @@ -672,7 +672,6 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx, krb5_error_code krb5_ret; const char *cc = NULL; const char *principal_s = NULL; - const char *service = NULL; char *realm = NULL; fstring name_namespace, name_domain, name_user; time_t ticket_lifetime = 0; @@ -755,11 +754,6 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - service = talloc_asprintf(mem_ctx, "%s/%s@%s", KRB5_TGS_NAME, realm, realm); - if (service == NULL) { - return NT_STATUS_NO_MEMORY; - } - local_service = talloc_asprintf(mem_ctx, "%s$@%s", lp_netbios_name(), lp_realm()); if (local_service == NULL) { @@ -848,7 +842,6 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx, result = add_ccache_to_list(principal_s, cc, - service, user, pass, realm, @@ -1180,7 +1173,6 @@ static NTSTATUS winbindd_dual_pam_auth_cached(struct winbindd_domain *domain, const char *cc = NULL; char *realm = NULL; const char *principal_s = NULL; - const char *service = NULL; const char *user_ccache_file; if (domain->alt_name == NULL) { @@ -1215,11 +1207,6 @@ static NTSTATUS winbindd_dual_pam_auth_cached(struct winbindd_domain *domain, return NT_STATUS_NO_MEMORY; } - service = talloc_asprintf(state->mem_ctx, "%s/%s@%s", KRB5_TGS_NAME, realm, realm); - if (service == NULL) { - return NT_STATUS_NO_MEMORY; - } - if (user_ccache_file != NULL) { fstrcpy(state->response->data.auth.krb5ccname, @@ -1227,7 +1214,6 @@ static NTSTATUS winbindd_dual_pam_auth_cached(struct winbindd_domain *domain, result = add_ccache_to_list(principal_s, cc, - service, state->request->data.auth.user, state->request->data.auth.pass, realm, diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h index 16c23f3de40..c685fab2606 100644 --- a/source3/winbindd/winbindd_proto.h +++ b/source3/winbindd/winbindd_proto.h @@ -228,7 +228,6 @@ void ccache_remove_all_after_fork(void); void ccache_regain_all_now(void); NTSTATUS add_ccache_to_list(const char *princ_name, const char *ccname, - const char *service, const char *username, const char *password, const char *realm, -- 2.37.0 From d3a97f652d6c7cabfe4e29cab452930ae0039c0a Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Thu, 7 Jul 2022 11:32:39 +0200 Subject: [PATCH 2/2] s3:winbind: Use the canonical realm name to renew the credentials Consider the following AD topology where all trusts are parent-child trusts: ADOM.AFOREST.AD | ACHILD.ADOM.AFOREST.AD | AGRANDCHILD.ACHILD.ADOM.AFOREST.AD <-- Samba joined When logging into the Samba machine using pam_winbind with kerberos enabled with user ACHILD\user1, the ccache content is: Default principal: user1@ACHILD.ADOM.AFOREST.AD Valid starting Expires Service principal 07/06/2022 16:09:23 07/06/2022 16:14:23 krbtgt/ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD renew until 07/13/2022 16:09:23 --> 07/06/2022 16:09:23 07/06/2022 16:14:23 krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD <-- NOTE this TGT ticket renew until 07/13/2022 16:09:23 07/06/2022 16:09:23 07/06/2022 16:14:23 SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD renew until 07/13/2022 16:09:23 But when logging in with user ADOM\user1, the ccache content is: Default principal: user1@ADOM.AFOREST.AD Valid starting Expires Service principal 07/06/2022 16:04:37 07/06/2022 16:09:37 krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD renew until 07/13/2022 16:04:37 07/06/2022 16:04:37 07/06/2022 16:09:37 SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD renew until 07/13/2022 16:04:37 MIT does not store the intermediate TGTs when there is more than one hop: ads_krb5_cli_get_ticket: Getting ticket for service [SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD] using creds from [FILE:/tmp/krb5cc_11105] and impersonating [(null)] Getting credentials user1@ADOM.AFOREST.AD -> SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD using ccache FILE:/tmp/krb5cc_11105 Starting with TGT for client realm: user1@ADOM.AFOREST.AD -> krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD Requesting TGT krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ADOM.AFOREST.AD using TGT krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD Sending request to ADOM.AFOREST.AD Received answer from stream 192.168.101.32:88 TGS reply is for user1@ADOM.AFOREST.AD -> krbtgt/ACHILD.ADOM.AFOREST.AD@ADOM.AFOREST.AD with session key rc4-hmac/D88B --> Received TGT for offpath realm ACHILD.ADOM.AFOREST.AD <-- NOTE this TGT ticket is not stored Requesting TGT krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD using TGT krbtgt/ACHILD.ADOM.AFOREST.AD@ADOM.AFOREST.AD Sending request (1748 bytes) to ACHILD.ADOM.AFOREST.AD Received answer (1628 bytes) from stream 192.168.101.33:88 TGS reply is for user1@ADOM.AFOREST.AD -> krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD with session key rc4-hmac/D015 --> Received TGT for service realm: krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD <-- NOTE this TGT is not stored Requesting tickets for SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD, referrals on Sending request (1721 bytes) to AGRANDCHILD.ACHILD.ADOM.AFOREST.AD Received answer (1647 bytes) from stream 192.168.101.34:88 TGS reply is for user1@ADOM.AFOREST.AD -> SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD with session key aes256-cts/345A Received creds for desired service SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD Storing user1@ADOM.AFOREST.AD -> SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD in FILE:/tmp/krb5cc_11105 In the case of ACHILD\user1: ads_krb5_cli_get_ticket: Getting ticket for service [SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD] using creds from [FILE:/tmp/krb5cc_2000] and impersonating [(null)] Getting credentials user1@ACHILD.ADOM.AFOREST.AD -> SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD using ccache FILE:/tmp/krb5cc_2000 Starting with TGT for client realm: user1@ACHILD.ADOM.AFOREST.AD -> krbtgt/ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD Requesting TGT krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD using TGT krbtgt/ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD Sending request to ACHILD.ADOM.AFOREST.AD Received answer from stream 192.168.101.33:88 TGS reply is for user1@ACHILD.ADOM.AFOREST.AD -> krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD with session key rc4-hmac/0F60 --> Storing user1@ACHILD.ADOM.AFOREST.AD -> krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD in FILE:/tmp/krb5cc_2000 <-- NOTE this TGT is stored Received TGT for service realm: krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ACHILD.ADOM.AFOREST.AD Requesting tickets for SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD, referrals on Sending request (1745 bytes) to AGRANDCHILD.ACHILD.ADOM.AFOREST.AD Received answer (1675 bytes) from stream 192.168.101.34:88 TGS reply is for user1@ACHILD.ADOM.AFOREST.AD -> SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD with session key aes256-cts/3576 Received creds for desired service SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD Storing user1@ACHILD.ADOM.AFOREST.AD -> SAMBA$@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD in FILE:/tmp/krb5cc_2000 The result is that winbindd can't refresh the tickets for ADOM\user1 because the local realm is used to build the TGT service name. smb_krb5_renew_ticket: Using FILE:/tmp/krb5cc_11105 as ccache for client 'user1@ADOM.AFOREST.AD' and service 'krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@AGRANDCHILD.ACHILD.ADOM.AFOREST.AD' Retrieving user1@ADOM.AFOREST.AD -> krbtgt/AGRANDCHILD.ACHILD.ADOM.AFOREST.AD@ADOM.AFOREST.AD from FILE:/tmp/krb5cc_11105 with result: -1765328243/Matching credential not found (filename: /tmp/krb5cc_11105) The canonical realm name must be used instead: smb_krb5_renew_ticket: Using FILE:/tmp/krb5cc_11105 as ccache for client 'user1@ADOM.AFOREST.AD' and service 'krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD' Retrieving user1@ADOM.AFOREST.AD -> krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD from FILE:/tmp/krb5cc_11105 with result: 0/Success Get cred via TGT krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD after requesting krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD (canonicalize off) Sending request to ADOM.AFOREST.AD Received answer from stream 192.168.101.32:88 TGS reply is for user1@ADOM.AFOREST.AD -> krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD with session key aes256-cts/8C7B Storing user1@ADOM.AFOREST.AD -> krbtgt/ADOM.AFOREST.AD@ADOM.AFOREST.AD in FILE:/tmp/krb5cc_11105 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14979 Signed-off-by: Samuel Cabrero Reviewed-by: Andreas Schneider Autobuild-User(master): Andreas Schneider Autobuild-Date(master): Tue Jul 12 12:38:55 UTC 2022 on sn-devel-184 (cherry picked from commit 116af0df4f74aa450cbb77c79f8cac4bfc288631) --- source3/winbindd/winbindd_cred_cache.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/winbindd/winbindd_cred_cache.c b/source3/winbindd/winbindd_cred_cache.c index 2fbef1ec8a9..9d27cbe8f78 100644 --- a/source3/winbindd/winbindd_cred_cache.c +++ b/source3/winbindd/winbindd_cred_cache.c @@ -638,8 +638,8 @@ NTSTATUS add_ccache_to_list(const char *princ_name, entry->service = talloc_asprintf(entry, "%s/%s@%s", KRB5_TGS_NAME, - realm, - realm); + canon_realm, + canon_realm); if (entry->service == NULL) { goto no_mem; } -- 2.37.0