1
0
mirror of https://github.com/samba-team/samba.git synced 2024-12-23 17:34:34 +03:00

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 e1f29b0970)
This commit is contained in:
Samuel Cabrero 2022-04-13 11:01:00 +02:00 committed by Jule Anger
parent 41a2825ea5
commit 621b80645a
3 changed files with 17 additions and 15 deletions

View File

@ -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;

View File

@ -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);

View File

@ -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;
}