[SCM] Samba Shared Repository - branch master updated

Stefan Metzmacher metze at samba.org
Wed May 20 14:17:03 MDT 2015


The branch, master has been updated
       via  435ddd8 s3:winbindd: make sure we remove pending io requests before closing client sockets
       via  33ca017 tevent: add a note to tevent_add_fd()
       via  eb029b3 s4: libcli/finddcs_cldap: continue processing CLDAP until all addresses are used
       via  47a3f9c heimdal:lib/krb5: verify_logonname() to handle multi component principal
      from  88d1b44 autobuild: Add test for cross-compilation infrastructure

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 435ddd8223eaa6fafb62cead0399bdd042d998e8
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon May 18 13:17:40 2015 +0200

    s3:winbindd: make sure we remove pending io requests before closing client sockets
    
    This avoids a crash inside the tevent epoll backend.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=11141
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
    Autobuild-Date(master): Wed May 20 22:16:54 CEST 2015 on sn-devel-104

commit 33ca0179ac091c8bb1c2b3fa7999cc5047d09a80
Author: Stefan Metzmacher <metze at samba.org>
Date:   Mon May 18 13:25:33 2015 +0200

    tevent: add a note to tevent_add_fd()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=11141
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit eb029b32e95c9e7382488f3a1b033cdbe3237c1c
Author: Alexander Bokovoy <ab at samba.org>
Date:   Wed May 20 11:17:38 2015 +0300

    s4: libcli/finddcs_cldap: continue processing CLDAP until all addresses are used
    
    This is a subtle bug that causes CLDAP pings to fail if SRV records
    discovered cannot be resolved or connection to them cannot be
    established. The code that fires up CLDAP ping will silently cancel
    the whole tevent request without going to the next server in the queue.
    
    This may happen, for example, when connection to IPv6 addresses couldn't
    be established, or when IPv4 address is not online or blocked by
    firewall.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=11284
    
    Signed-off-by: Alexander Bokovoy <ab at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 47a3f9cc5a1e3de5b7eadeae5c001863c2adca2b
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed May 20 13:40:58 2015 +0000

    heimdal:lib/krb5: verify_logonname() to handle multi component principal
    
    FreeIPA can generate tickets with a client principal of
    'host/hostname.example.com'.
    
    verify_logonname() should just verify the principal name
    in the PAC_LOGON_NAME is the same as the principal of
    the client principal (without realm) of the ticket.
    
    Samba commit b7cc8c1187ff967e44587cd0d09185330378f366
    break this. We try to compare ['host']['hostname.example.com']
    with ['host/hostname.example.com]' (as we interpret it as enterprise principal)
    this fail if we don't compare them as strings.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=11142
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 lib/tevent/tevent.h            |  5 +++++
 source3/winbindd/winbindd.c    | 26 ++++++++++++++++++++++++++
 source3/winbindd/winbindd.h    |  1 +
 source4/heimdal/lib/krb5/pac.c | 34 ++++++++++++++++++++--------------
 source4/libcli/finddcs_cldap.c | 42 ++++++++++++++++++++++++++++++++----------
 5 files changed, 84 insertions(+), 24 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/tevent/tevent.h b/lib/tevent/tevent.h
index c54cbe2..6861ffb 100644
--- a/lib/tevent/tevent.h
+++ b/lib/tevent/tevent.h
@@ -176,6 +176,11 @@ void tevent_set_default_backend(const char *backend);
  *
  * @note To cancel the monitoring of a file descriptor, call talloc_free()
  * on the object returned by this function.
+ *
+ * @note The caller should avoid closing the file descriptor before
+ * calling talloc_free()! Otherwise the behaviour is undefined which
+ * might result in crashes. See https://bugzilla.samba.org/show_bug.cgi?id=11141
+ * for an example.
  */
 struct tevent_fd *tevent_add_fd(struct tevent_context *ev,
 				TALLOC_CTX *mem_ctx,
diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 87bc532..4ffe79c 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -824,6 +824,7 @@ static void request_finished(struct winbindd_cli_state *state)
 		return;
 	}
 	tevent_req_set_callback(req, winbind_client_response_written, state);
+	state->io_req = req;
 }
 
 static void winbind_client_response_written(struct tevent_req *req)
@@ -833,6 +834,8 @@ static void winbind_client_response_written(struct tevent_req *req)
 	ssize_t ret;
 	int err;
 
+	state->io_req = NULL;
+
 	ret = wb_resp_write_recv(req, &err);
 	TALLOC_FREE(req);
 	if (ret == -1) {
@@ -859,6 +862,7 @@ static void winbind_client_response_written(struct tevent_req *req)
 		return;
 	}
 	tevent_req_set_callback(req, winbind_client_request_read, state);
+	state->io_req = req;
 }
 
 void request_error(struct winbindd_cli_state *state)
@@ -929,6 +933,7 @@ static void new_connection(int listen_sock, bool privileged)
 		return;
 	}
 	tevent_req_set_callback(req, winbind_client_request_read, state);
+	state->io_req = req;
 
 	/* Add to connection list */
 
@@ -942,6 +947,8 @@ static void winbind_client_request_read(struct tevent_req *req)
 	ssize_t ret;
 	int err;
 
+	state->io_req = NULL;
+
 	ret = wb_req_read_recv(req, state, &state->request, &err);
 	TALLOC_FREE(req);
 	if (ret == -1) {
@@ -973,6 +980,25 @@ static void remove_client(struct winbindd_cli_state *state)
 		return;
 	}
 
+	/*
+	 * We need to remove a pending wb_req_read_*
+	 * or wb_resp_write_* request before closing the
+	 * socket.
+	 *
+	 * This is important as they might have used tevent_add_fd() and we
+	 * use the epoll * backend on linux. So we must remove the tevent_fd
+	 * before closing the fd.
+	 *
+	 * Otherwise we might hit a race with close_conns_after_fork() (via
+	 * winbindd_reinit_after_fork()) where a file description
+	 * is still open in a child, which means it's still active in
+	 * the parents epoll queue, but the related tevent_fd is already
+	 * already gone in the parent.
+	 *
+	 * See bug #11141.
+	 */
+	TALLOC_FREE(state->io_req);
+
 	if (state->sock != -1) {
 		/* tell client, we are closing ... */
 		nwritten = write(state->sock, &c, sizeof(c));
diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h
index f676b1e..b2105e3 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -67,6 +67,7 @@ struct winbindd_cli_state {
 	struct winbindd_request *request;         /* Request from client */
 	struct tevent_queue *out_queue;
 	struct winbindd_response *response;        /* Respose to client */
+	struct tevent_req *io_req; /* wb_req_read_* or wb_resp_write_* */
 
 	struct getpwent_state *pwent_state; /* State for getpwent() */
 	struct getgrent_state *grent_state; /* State for getgrent() */
diff --git a/source4/heimdal/lib/krb5/pac.c b/source4/heimdal/lib/krb5/pac.c
index 9328647..7c8ba50 100644
--- a/source4/heimdal/lib/krb5/pac.c
+++ b/source4/heimdal/lib/krb5/pac.c
@@ -595,11 +595,12 @@ verify_logonname(krb5_context context,
 		 krb5_const_principal principal)
 {
     krb5_error_code ret;
-    krb5_principal p2;
     uint32_t time1, time2;
     krb5_storage *sp;
     uint16_t len;
-    char *s;
+    char *s = NULL;
+    char *principal_string = NULL;
+    char *logon_string = NULL;
 
     sp = krb5_storage_from_readonly_mem((const char *)data->data + logon_name->offset_lo,
 					logon_name->buffersize);
@@ -664,31 +665,36 @@ verify_logonname(krb5_context context,
 	    return ret;
 	}
 	u8len += 1; /* Add space for NUL */
-	s = malloc(u8len);
-	if (s == NULL) {
+	logon_string = malloc(u8len);
+	if (logon_string == NULL) {
 	    free(ucs2);
 	    return krb5_enomem(context);
 	}
-	ret = wind_ucs2utf8(ucs2, ucs2len, s, &u8len);
+	ret = wind_ucs2utf8(ucs2, ucs2len, logon_string, &u8len);
 	free(ucs2);
 	if (ret) {
-	    free(s);
+	    free(logon_string);
 	    krb5_set_error_message(context, ret, "Failed to convert to UTF-8");
 	    return ret;
 	}
     }
-    ret = krb5_parse_name_flags(context, s,
-				KRB5_PRINCIPAL_PARSE_NO_REALM |
-				KRB5_PRINCIPAL_PARSE_ENTERPRISE, &p2);
-    free(s);
-    if (ret)
+    ret = krb5_unparse_name_flags(context, principal,
+				  KRB5_PRINCIPAL_UNPARSE_NO_REALM |
+				  KRB5_PRINCIPAL_UNPARSE_DISPLAY,
+				  &principal_string);
+    if (ret) {
+	free(logon_string);
 	return ret;
+    }
 
-    if (krb5_principal_compare_any_realm(context, principal, p2) != TRUE) {
+    ret = strcmp(logon_string, principal_string);
+    if (ret != 0) {
 	ret = EINVAL;
-	krb5_set_error_message(context, ret, "PAC logon name mismatch");
+	krb5_set_error_message(context, ret, "PAC logon name [%s] mismatch principal name [%s]",
+			       logon_string, principal_string);
     }
-    krb5_free_principal(context, p2);
+    free(logon_string);
+    free(principal_string);
     return ret;
 out:
     return ret;
diff --git a/source4/libcli/finddcs_cldap.c b/source4/libcli/finddcs_cldap.c
index ce0e1c7..b385b1c 100644
--- a/source4/libcli/finddcs_cldap.c
+++ b/source4/libcli/finddcs_cldap.c
@@ -42,6 +42,7 @@ struct finddcs_cldap_state {
 	uint32_t srv_address_index;
 	struct cldap_socket *cldap;
 	struct cldap_netlogon *netlogon;
+	NTSTATUS status;
 };
 
 static void finddcs_cldap_srv_resolved(struct composite_context *ctx);
@@ -245,10 +246,15 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
 	struct tevent_req *subreq;
 	struct tsocket_address *dest;
 	int ret;
-	NTSTATUS status;
+
+	TALLOC_FREE(state->cldap);
 
 	if (state->srv_addresses[state->srv_address_index] == NULL) {
-		tevent_req_nterror(state->req, NT_STATUS_OBJECT_NAME_NOT_FOUND);
+		if (NT_STATUS_IS_OK(state->status)) {
+			tevent_req_nterror(state->req, NT_STATUS_OBJECT_NAME_NOT_FOUND);
+		} else {
+			tevent_req_nterror(state->req, state->status);
+		}
 		DEBUG(2,("finddcs: No matching CLDAP server found\n"));
 		return;
 	}
@@ -259,22 +265,29 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
 						389,
 						&dest);
 	if (ret == 0) {
-		status = NT_STATUS_OK;
+		state->status = NT_STATUS_OK;
 	} else {
-		status = map_nt_error_from_unix_common(errno);
+		state->status = map_nt_error_from_unix_common(errno);
 	}
-	if (tevent_req_nterror(state->req, status)) {
+	if (!NT_STATUS_IS_OK(state->status)) {
+		state->srv_address_index++;
+		finddcs_cldap_next_server(state);
 		return;
 	}
 
-	status = cldap_socket_init(state, NULL, dest, &state->cldap);
-	if (tevent_req_nterror(state->req, status)) {
+	state->status = cldap_socket_init(state, NULL, dest, &state->cldap);
+	if (!NT_STATUS_IS_OK(state->status)) {
+		state->srv_address_index++;
+		finddcs_cldap_next_server(state);
 		return;
 	}
 
 	TALLOC_FREE(state->netlogon);
 	state->netlogon = talloc_zero(state, struct cldap_netlogon);
-	if (tevent_req_nomem(state->netlogon, state->req)) {
+	if (state->netlogon == NULL) {
+		state->status = NT_STATUS_NO_MEMORY;
+		state->srv_address_index++;
+		finddcs_cldap_next_server(state);
 		return;
 	}
 
@@ -283,7 +296,10 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
 	}
 	if (state->domain_sid) {
 		state->netlogon->in.domain_sid = dom_sid_string(state, state->domain_sid);
-		if (tevent_req_nomem(state->netlogon->in.domain_sid, state->req)) {
+		if (state->netlogon->in.domain_sid == NULL) {
+			state->status = NT_STATUS_NO_MEMORY;
+			state->srv_address_index++;
+			finddcs_cldap_next_server(state);
 			return;
 		}
 	}
@@ -299,7 +315,10 @@ static void finddcs_cldap_next_server(struct finddcs_cldap_state *state)
 
 	subreq = cldap_netlogon_send(state, state->ev,
 				     state->cldap, state->netlogon);
-	if (tevent_req_nomem(subreq, state->req)) {
+	if (subreq == NULL) {
+		state->status = NT_STATUS_NO_MEMORY;
+		state->srv_address_index++;
+		finddcs_cldap_next_server(state);
 		return;
 	}
 
@@ -321,6 +340,7 @@ static void finddcs_cldap_netlogon_replied(struct tevent_req *subreq)
 	TALLOC_FREE(subreq);
 	TALLOC_FREE(state->cldap);
 	if (!NT_STATUS_IS_OK(status)) {
+		state->status = status;
 		state->srv_address_index++;
 		finddcs_cldap_next_server(state);
 		return;
@@ -364,6 +384,7 @@ static void finddcs_cldap_name_resolved(struct composite_context *ctx)
 
 	state->srv_address_index = 0;
 
+	state->status = NT_STATUS_OK;
 	finddcs_cldap_next_server(state);
 }
 
@@ -416,6 +437,7 @@ static void finddcs_cldap_srv_resolved(struct composite_context *ctx)
 
 	state->srv_address_index = 0;
 
+	state->status = NT_STATUS_OK;
 	finddcs_cldap_next_server(state);
 }
 


-- 
Samba Shared Repository


More information about the samba-cvs mailing list