The Samba-Bugzilla – Attachment 17270 Details for
Bug 15046
PAM Kerberos authentication incorrectly fails with a clock skew error
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for 4.16
bso15046-v416.patch (text/plain), 12.46 KB, created by
Samuel Cabrero
on 2022-04-13 16:38:06 UTC
(
hide
)
Description:
Patch for 4.16
Filename:
MIME Type:
Creator:
Samuel Cabrero
Created:
2022-04-13 16:38:06 UTC
Size:
12.46 KB
patch
obsolete
>From 14411772657c46e5c0bbcae8a6985ebc7d449c3f Mon Sep 17 00:00:00 2001 >From: Samuel Cabrero <scabrero@samba.org> >Date: Wed, 13 Apr 2022 11:01:00 +0200 >Subject: [PATCH 1/4] s3:winbind: Do not use domain's private data to store the > SAMR pipes > >The domain's private_data pointer is also used to store a ADS_STRUCT, >which is not allocated using talloc and there are many places casting >this pointer directly. > >The recently added samba.tests.pam_winbind_setcred was randomly failing >and after debugging it the problem was that kerberos authentication was >failing because the time_offset passed to kerberos_return_pac() was >wrong. This time_offset was retrieved from ads->auth.time_offset, where >the ads pointer was directly casted from domain->private_data but >private_data was pointing to a winbind_internal_pipes struct. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15046 > >Signed-off-by: Samuel Cabrero <scabrero@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit e1f29b0970f4cac52a9cd517be6862cf69a1433a) >--- > source3/winbindd/winbindd.h | 6 ++++++ > source3/winbindd/winbindd_ndr.c | 3 +++ > source3/winbindd/winbindd_samr.c | 23 ++++++++--------------- > 3 files changed, 17 insertions(+), 15 deletions(-) > >diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h >index dac4a1fa927..762844502e5 100644 >--- a/source3/winbindd/winbindd.h >+++ b/source3/winbindd/winbindd.h >@@ -43,6 +43,8 @@ > > #define WB_REPLACE_CHAR '_' > >+struct winbind_internal_pipes; >+ > struct winbindd_cli_state { > struct winbindd_cli_state *prev, *next; /* Linked list pointers */ > int sock; /* Open socket from client */ >@@ -157,6 +159,10 @@ struct winbindd_domain { > > void *private_data; > >+ struct { >+ struct winbind_internal_pipes *samr_pipes; >+ } backend_data; >+ > /* A working DC */ > char *dcname; > const char *ping_dcname; >diff --git a/source3/winbindd/winbindd_ndr.c b/source3/winbindd/winbindd_ndr.c >index 157ce1bff27..36901776b98 100644 >--- a/source3/winbindd/winbindd_ndr.c >+++ b/source3/winbindd/winbindd_ndr.c >@@ -144,6 +144,9 @@ void ndr_print_winbindd_domain(struct ndr_print *ndr, > ndr_print_bool(ndr, "startup", r->startup); > ndr_print_winbindd_methods(ndr, "backend", r->backend); > ndr_print_ptr(ndr, "private_data", r->private_data); >+ ndr_print_ptr(ndr, >+ "backend_data.samr_pipes", >+ r->backend_data.samr_pipes); > ndr_print_string(ndr, "dcname", r->dcname); > ndr_print_sockaddr_storage(ndr, "dcaddr", &r->dcaddr); > ndr_print_time_t(ndr, "last_seq_check", r->last_seq_check); >diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c >index 6b7a2c3be6a..d5c4e8e1f4f 100644 >--- a/source3/winbindd/winbindd_samr.c >+++ b/source3/winbindd/winbindd_samr.c >@@ -131,8 +131,7 @@ static void cached_internal_pipe_close( > struct winbindd_domain *domain = talloc_get_type_abort( > private_data, struct winbindd_domain); > /* >- * domain->private_data is the struct winbind_internal_pipes * >- * pointer so freeing it closes the cached pipes. >+ * Freeing samr_pipes closes the cached pipes. > * > * We can do a hard close because at the time of this commit > * we only use sychronous calls to external pipes. So we can't >@@ -141,7 +140,7 @@ static void cached_internal_pipe_close( > * get nested event loops. Once we start to get async in > * winbind children, we need to check for outstanding calls > */ >- TALLOC_FREE(domain->private_data); >+ TALLOC_FREE(domain->backend_data.samr_pipes); > } > > static NTSTATUS open_cached_internal_pipe_conn( >@@ -153,7 +152,7 @@ static NTSTATUS open_cached_internal_pipe_conn( > { > struct winbind_internal_pipes *internal_pipes = NULL; > >- if (domain->private_data == NULL) { >+ if (domain->backend_data.samr_pipes == NULL) { > TALLOC_CTX *frame = talloc_stackframe(); > NTSTATUS status; > >@@ -190,14 +189,14 @@ static NTSTATUS open_cached_internal_pipe_conn( > return NT_STATUS_NO_MEMORY; > } > >- domain->private_data = talloc_move(domain, &internal_pipes); >+ domain->backend_data.samr_pipes = >+ talloc_move(domain, &internal_pipes); > > TALLOC_FREE(frame); > > } > >- internal_pipes = talloc_get_type_abort( >- domain->private_data, struct winbind_internal_pipes); >+ internal_pipes = domain->backend_data.samr_pipes; > > if (samr_domain_hnd) { > *samr_domain_hnd = internal_pipes->samr_domain_hnd; >@@ -226,23 +225,17 @@ static bool reset_connection_on_error(struct winbindd_domain *domain, > struct rpc_pipe_client *p, > NTSTATUS status) > { >- struct winbind_internal_pipes *internal_pipes = NULL; > struct dcerpc_binding_handle *b = p->binding_handle; > >- internal_pipes = talloc_get_type_abort( >- domain->private_data, struct winbind_internal_pipes); >- > if (NT_STATUS_EQUAL(status, NT_STATUS_IO_TIMEOUT) || > NT_STATUS_EQUAL(status, NT_STATUS_IO_DEVICE_ERROR)) > { >- TALLOC_FREE(internal_pipes); >- domain->private_data = NULL; >+ TALLOC_FREE(domain->backend_data.samr_pipes); > return true; > } > > if (!dcerpc_binding_handle_is_connected(b)) { >- TALLOC_FREE(internal_pipes); >- domain->private_data = NULL; >+ TALLOC_FREE(domain->backend_data.samr_pipes); > return true; > } > >-- >2.35.1 > > >From e8b968d461f6f5c6b169ccc88b3a507654620b2c Mon Sep 17 00:00:00 2001 >From: Samuel Cabrero <scabrero@samba.org> >Date: Wed, 13 Apr 2022 11:15:35 +0200 >Subject: [PATCH 2/4] s3:winbind: Simplify open_cached_internal_pipe_conn() > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15046 > >Signed-off-by: Samuel Cabrero <scabrero@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit 91395e660a2b1b69bf74ca0b77aee416e2ac1db3) >--- > source3/winbindd/winbindd_samr.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > >diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c >index d5c4e8e1f4f..ebf9c24b9e4 100644 >--- a/source3/winbindd/winbindd_samr.c >+++ b/source3/winbindd/winbindd_samr.c >@@ -150,9 +150,10 @@ static NTSTATUS open_cached_internal_pipe_conn( > struct rpc_pipe_client **lsa_pipe, > struct policy_handle *lsa_hnd) > { >- struct winbind_internal_pipes *internal_pipes = NULL; >+ struct winbind_internal_pipes *internal_pipes = >+ domain->backend_data.samr_pipes; > >- if (domain->backend_data.samr_pipes == NULL) { >+ if (internal_pipes == NULL) { > TALLOC_CTX *frame = talloc_stackframe(); > NTSTATUS status; > >@@ -190,14 +191,11 @@ static NTSTATUS open_cached_internal_pipe_conn( > } > > domain->backend_data.samr_pipes = >- talloc_move(domain, &internal_pipes); >+ talloc_steal(domain, internal_pipes); > > TALLOC_FREE(frame); >- > } > >- internal_pipes = domain->backend_data.samr_pipes; >- > if (samr_domain_hnd) { > *samr_domain_hnd = internal_pipes->samr_domain_hnd; > } >-- >2.35.1 > > >From 428bea9f1a55535569c4a8330da1a637320a904c Mon Sep 17 00:00:00 2001 >From: Samuel Cabrero <scabrero@samba.org> >Date: Wed, 13 Apr 2022 11:31:45 +0200 >Subject: [PATCH 3/4] s3:winbind: Do not use domain's private data to store the > ADS_STRUCT > >The ADS_STRUCT is not allocated using talloc and there are many places >casting this pointer directly so use a typed pointer. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15046 > >Signed-off-by: Samuel Cabrero <scabrero@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit 3cb256439e9ceece26c2de82293c43486543e0cb) >--- > source3/winbindd/winbindd.h | 2 ++ > source3/winbindd/winbindd_ads.c | 10 +++++----- > source3/winbindd/winbindd_ndr.c | 3 +++ > source3/winbindd/winbindd_pam.c | 6 ++---- > 4 files changed, 12 insertions(+), 9 deletions(-) > >diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h >index 762844502e5..3cc88367b90 100644 >--- a/source3/winbindd/winbindd.h >+++ b/source3/winbindd/winbindd.h >@@ -44,6 +44,7 @@ > #define WB_REPLACE_CHAR '_' > > struct winbind_internal_pipes; >+struct ads_struct; > > struct winbindd_cli_state { > struct winbindd_cli_state *prev, *next; /* Linked list pointers */ >@@ -161,6 +162,7 @@ struct winbindd_domain { > > struct { > struct winbind_internal_pipes *samr_pipes; >+ struct ads_struct *ads_conn; > } backend_data; > > /* A working DC */ >diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c >index 6f01ef6e334..d350f160223 100644 >--- a/source3/winbindd/winbindd_ads.c >+++ b/source3/winbindd/winbindd_ads.c >@@ -269,10 +269,10 @@ static ADS_STRUCT *ads_cached_connection(struct winbindd_domain *domain) > } > > DEBUG(10,("ads_cached_connection\n")); >- ads_cached_connection_reuse((ADS_STRUCT **)&domain->private_data); >+ ads_cached_connection_reuse(&domain->backend_data.ads_conn); > >- if (domain->private_data) { >- return (ADS_STRUCT *)domain->private_data; >+ if (domain->backend_data.ads_conn != NULL) { >+ return domain->backend_data.ads_conn; > } > > /* the machine acct password might have change - fetch it every time */ >@@ -303,7 +303,7 @@ static ADS_STRUCT *ads_cached_connection(struct winbindd_domain *domain) > } > > status = ads_cached_connection_connect( >- (ADS_STRUCT **)&domain->private_data, >+ &domain->backend_data.ads_conn, > domain->alt_name, > domain->name, NULL, > password, realm, >@@ -322,7 +322,7 @@ static ADS_STRUCT *ads_cached_connection(struct winbindd_domain *domain) > return NULL; > } > >- return (ADS_STRUCT *)domain->private_data; >+ return domain->backend_data.ads_conn; > } > > /* Query display info for a realm. This is the basic user list fn */ >diff --git a/source3/winbindd/winbindd_ndr.c b/source3/winbindd/winbindd_ndr.c >index 36901776b98..94ce9d73747 100644 >--- a/source3/winbindd/winbindd_ndr.c >+++ b/source3/winbindd/winbindd_ndr.c >@@ -147,6 +147,9 @@ void ndr_print_winbindd_domain(struct ndr_print *ndr, > ndr_print_ptr(ndr, > "backend_data.samr_pipes", > r->backend_data.samr_pipes); >+ ndr_print_ptr(ndr, >+ "backend_data.ads_conn", >+ r->backend_data.ads_conn); > ndr_print_string(ndr, "dcname", r->dcname); > ndr_print_sockaddr_storage(ndr, "dcaddr", &r->dcaddr); > ndr_print_time_t(ndr, "last_seq_check", r->last_seq_check); >diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c >index c2fcc399ab8..84c3720c19f 100644 >--- a/source3/winbindd/winbindd_pam.c >+++ b/source3/winbindd/winbindd_pam.c >@@ -677,7 +677,6 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx, > fstring name_namespace, name_domain, name_user; > time_t ticket_lifetime = 0; > time_t renewal_until = 0; >- ADS_STRUCT *ads; > time_t time_offset = 0; > const char *user_ccache_file; > struct PAC_LOGON_INFO *logon_info = NULL; >@@ -716,9 +715,8 @@ static NTSTATUS winbindd_raw_kerberos_login(TALLOC_CTX *mem_ctx, > /* 2nd step: > * get kerberos properties */ > >- if (domain->private_data) { >- ads = (ADS_STRUCT *)domain->private_data; >- time_offset = ads->auth.time_offset; >+ if (domain->backend_data.ads_conn != NULL) { >+ time_offset = domain->backend_data.ads_conn->auth.time_offset; > } > > >-- >2.35.1 > > >From 8544f6a0f5ba8a1d64a159864f1a2a67f1f69e8b Mon Sep 17 00:00:00 2001 >From: Samuel Cabrero <scabrero@samba.org> >Date: Wed, 13 Apr 2022 11:34:18 +0200 >Subject: [PATCH 4/4] s3:winbind: Remove no longer used domain's private_data > pointer > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15046 > >Signed-off-by: Samuel Cabrero <scabrero@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: Andreas Schneider <asn@samba.org> >(cherry picked from commit a6d6ae3cfcd64a85f82ec5b12253ca0e237d95bb) >--- > source3/winbindd/winbindd.h | 4 ---- > source3/winbindd/winbindd_ndr.c | 1 - > 2 files changed, 5 deletions(-) > >diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h >index 3cc88367b90..fe286a9a686 100644 >--- a/source3/winbindd/winbindd.h >+++ b/source3/winbindd/winbindd.h >@@ -156,10 +156,6 @@ struct winbindd_domain { > */ > struct winbindd_methods *backend; > >- /* Private data for the backends (used for connection cache) */ >- >- void *private_data; >- > struct { > struct winbind_internal_pipes *samr_pipes; > struct ads_struct *ads_conn; >diff --git a/source3/winbindd/winbindd_ndr.c b/source3/winbindd/winbindd_ndr.c >index 94ce9d73747..b393586a692 100644 >--- a/source3/winbindd/winbindd_ndr.c >+++ b/source3/winbindd/winbindd_ndr.c >@@ -143,7 +143,6 @@ void ndr_print_winbindd_domain(struct ndr_print *ndr, > ndr_print_time_t(ndr, "startup_time", r->startup_time); > ndr_print_bool(ndr, "startup", r->startup); > ndr_print_winbindd_methods(ndr, "backend", r->backend); >- ndr_print_ptr(ndr, "private_data", r->private_data); > ndr_print_ptr(ndr, > "backend_data.samr_pipes", > r->backend_data.samr_pipes); >-- >2.35.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
asn
:
review+
scabrero
:
review?
(
metze
)
Actions:
View
Attachments on
bug 15046
: 17270 |
17271