r335: added much better handling of servers that die unexpectedly during a

request (a dead socket). I discovered this when testing against Sun's
PC-NetLink.

cleaned up the naming of some of the samr requests

add IDL and test code for samr_QueryGroupMember(),
samr_SetMemberAttributesOfGroup() and samr_Shutdown().  (actually, I
didn't leave the samr_Shutdown() test in, as its fatal to windows
servers due to doing exactly what it says it does).

source/lib/util_sock.c

@@ -187,6 +187,11 @@ ssize_t read_data(int fd, char *buffer, size_t N)
 	ssize_t ret;
 	size_t total=0;  
  
+	if (fd == -1) {
+		errno = EIO;
+		return -1;
+	}
+
 	while (total < N) {
 		ret = sys_read(fd,buffer + total,N - total);
 		if (ret == 0) {
@@ -209,6 +214,11 @@ ssize_t write_data(int fd, const char *buffer, size_t N)
 	size_t total=0;
 	ssize_t ret;
 
+	if (fd == -1) {
+		errno = EIO;
+		return -1;
+	}
+
 	while (total < N) {
 		ret = sys_write(fd, buffer + total, N - total);
 		if (ret == -1) {

source/libcli/raw/clisocket.c

@@ -74,15 +74,25 @@ BOOL cli_sock_connect(struct cli_socket *sock, struct in_addr *ip, int port)
 }
 
 
+/****************************************************************************
+ mark the socket as dead
+****************************************************************************/
+void cli_sock_dead(struct cli_socket *sock)
+{
+	if (sock->fd != -1) {
+		close(sock->fd);
+		sock->fd = -1;
+	}
+}
+
 /****************************************************************************
  reduce socket reference count - if it becomes zero then close
 ****************************************************************************/
 void cli_sock_close(struct cli_socket *sock)
 {
 	sock->reference_count--;
-	if (sock->reference_count <= 0 && sock->fd != -1) {
+	if (sock->reference_count <= 0) {
-		close(sock->fd);
+		cli_sock_dead(sock);
-		sock->fd = -1;
 	}
 }
 
@@ -99,6 +109,11 @@ void cli_sock_set_options(struct cli_socket *sock, const char *options)
 ****************************************************************************/
 ssize_t cli_sock_write(struct cli_socket *sock, const char *data, size_t len)
 {
+	if (sock->fd == -1) {
+		errno = EIO;
+		return -1;
+	}
+
 	return write_data(sock->fd, data, len);
 }
 
@@ -108,6 +123,11 @@ ssize_t cli_sock_write(struct cli_socket *sock, const char *data, size_t len)
 ****************************************************************************/
 ssize_t cli_sock_read(struct cli_socket *sock, char *data, size_t len)
 {
+	if (sock->fd == -1) {
+		errno = EIO;
+		return -1;
+	}
+
 	return read_data(sock->fd, data, len);
 }
 

source/libcli/raw/clitransport.c

@@ -60,6 +60,14 @@ void cli_transport_close(struct cli_transport *transport)
 	}
 }
 
+/*
+  mark the transport as dead
+*/
+void cli_transport_dead(struct cli_transport *transport)
+{
+	cli_sock_dead(transport->socket);
+}
+
 
 
 /****************************************************************************

source/libcli/raw/rawrequest.c

@@ -40,9 +40,11 @@ NTSTATUS cli_request_destroy(struct cli_request *req)
 	   _send() call fails completely */
 	if (!req) return NT_STATUS_UNSUCCESSFUL;
 
+	if (req->transport) {
 		/* remove it from the list of pending requests (a null op if
 		   its not in the list) */
 		DLIST_REMOVE(req->transport->pending_requests, req);
+	}
 
 	/* ahh, its so nice to destroy a complex structure in such a
 	   simple way! */
@@ -306,11 +308,12 @@ BOOL cli_request_receive(struct cli_request *req)
 	/* keep receiving packets until this one is replied to */
 	while (!req->in.buffer) {
 		if (!cli_transport_select(req->transport)) {
+			req->status = NT_STATUS_UNSUCCESSFUL;
 			return False;
 		}
 
 		if (!cli_request_receive_next(req->transport)) {
-			cli_transport_close(req->transport);
+			cli_transport_dead(req->transport);
 			req->status = NT_STATUS_UNEXPECTED_NETWORK_ERROR;
 			return False;
 		}

source/libcli/raw/rawtrans.c

@@ -64,7 +64,6 @@ NTSTATUS smb_raw_trans2_recv(struct cli_request *req,
 	parms->out.params.data = NULL;
 
 	if (!cli_request_receive(req)) {
-		req->status = NT_STATUS_UNSUCCESSFUL;
 		return cli_request_destroy(req);
 	}
 	

source/librpc/idl/samr.idl

@@ -54,7 +54,13 @@
 
 	/******************/
 	/* Function: 0x04 */
-	NTSTATUS samr_Shutdown ();
+
+	/*
+	  shutdown the SAM - once you call this the SAM will be dead
+	*/
+	NTSTATUS samr_Shutdown (
+		[in,ref]   policy_handle *handle
+		);
 
 	/******************/
 	/* Function: 0x05 */
@@ -401,13 +407,44 @@
 		[in]                      uint32 rid
 		);
 
+
 	/************************/
 	/* Function    0x19     */
-	NTSTATUS samr_QUERY_GROUPMEM();
+	/*
+	  this isn't really valid IDL, but it does work. I suspect
+	  I need to do some more pidl work to get this really right
+	*/
+	typedef struct {
+		uint32 count;
+		uint32 v[count];
+	} samr_intArray;
+
+	typedef struct {
+		samr_intArray *rids;
+		samr_intArray *unknown7;
+	} samr_ridArray;
+
+	NTSTATUS samr_QueryGroupMember(
+		[in,ref]  policy_handle *handle,
+		[out]     uint32 *count,
+		[out]     samr_ridArray rids
+		);
+
 
 	/************************/
 	/* Function    0x1a     */
-	NTSTATUS samr_SET_MEMBER_ATTRIBUTES_OF_GROUP();
+
+	/*
+	  win2003 seems to accept any data at all for the two integers
+	  below, and doesn't seem to do anything with them that I can
+	  see. Weird. I really expected the first integer to be a rid
+	  and the second to be the attributes for that rid member.
+	*/
+	NTSTATUS samr_SetMemberAttributesOfGroup(
+		[in,ref]  policy_handle *handle,
+		[in]      uint32 unknown1,
+		[in]      uint32 unknown2
+		);
 
 
 	/************************/
@@ -457,14 +494,14 @@
 
 	/************************/
 	/* Function    0x1f     */
-	NTSTATUS samr_AddAliasMem(
+	NTSTATUS samr_AddAliasMember(
 		[in,ref]  policy_handle *handle,
 		[in,ref]  dom_sid2      *sid
 		);
 
 	/************************/
 	/* Function    0x20     */
-	NTSTATUS samr_DelAliasMem(
+	NTSTATUS samr_DeleteAliasMember(
 		[in,ref] policy_handle *handle,
 		[in,ref] dom_sid2      *sid
 		);
@@ -1093,12 +1130,17 @@
 		[out]       dom_sid2      *sid
 		);
 
+
 	/************************/
 	/* Function    0x42     */
 	NTSTATUS samr_SET_DSRM_PASSWORD();
 
 	/************************/
 	/* Function    0x43     */
-	NTSTATUS samr_VALIDATE_PASSWORD();
 
+	/*
+	  I haven't been able to work out the format of this one yet.
+	  Seems to start with a switch level for a union?
+	*/
+	NTSTATUS samr_ValidatePassword();
 }

source/librpc/rpc/dcerpc_smb.c

@@ -75,6 +75,7 @@ static NTSTATUS dcerpc_raw_recv(struct dcerpc_pipe *p,
 	DATA_BLOB payload;
 
 	status = smb_raw_trans_recv(req, mem_ctx, &trans);
+
 	/* STATUS_BUFFER_OVERFLOW means that there is more data
 	   available via SMBreadX */
 	if (!NT_STATUS_IS_OK(status) && 

source/librpc/rpc/dcerpc_tcp.c

@@ -29,6 +29,18 @@ struct tcp_private {
 	uint32 port;
 };
 
+
+/*
+  mark the socket dead
+*/
+static void tcp_sock_dead(struct tcp_private *tcp)
+{
+	if (tcp && tcp->fd != -1) {
+		close(tcp->fd);
+		tcp->fd = -1;
+	}
+}
+
 static NTSTATUS tcp_raw_recv(struct dcerpc_pipe *p, 
 			     TALLOC_CTX *mem_ctx,
 			     DATA_BLOB *blob)
@@ -45,7 +57,8 @@ static NTSTATUS tcp_raw_recv(struct dcerpc_pipe *p,
 
 	ret = read_data(tcp->fd, blob1.data, blob1.length);
 	if (ret != blob1.length) {
-		return NT_STATUS_NET_WRITE_FAULT;
+		tcp_sock_dead(tcp);
+		return NT_STATUS_UNEXPECTED_NETWORK_ERROR;
 	}
 
 	/* this could be a ncacn_http endpoint - this doesn't work
@@ -54,7 +67,8 @@ static NTSTATUS tcp_raw_recv(struct dcerpc_pipe *p,
 		memmove(blob1.data, blob1.data+14, 2);
 		ret = read_data(tcp->fd, blob1.data+2, 14);
 		if (ret != 14) {
-			return NT_STATUS_NET_WRITE_FAULT;
+			tcp_sock_dead(tcp);
+			return NT_STATUS_UNEXPECTED_NETWORK_ERROR;
 		}
 	}
 
@@ -74,7 +88,8 @@ static NTSTATUS tcp_raw_recv(struct dcerpc_pipe *p,
 
 	ret = read_data(tcp->fd, blob->data + blob1.length, frag_length - blob1.length);
 	if (ret != frag_length - blob1.length) {
-		return NT_STATUS_NET_WRITE_FAULT;
+		tcp_sock_dead(tcp);
+		return NT_STATUS_UNEXPECTED_NETWORK_ERROR;
 	}
 
 	return NT_STATUS_OK;
@@ -90,7 +105,8 @@ static NTSTATUS tcp_full_request(struct dcerpc_pipe *p,
 
 	ret = write_data(tcp->fd, request_blob->data, request_blob->length);
 	if (ret != request_blob->length) {
-		return NT_STATUS_NET_WRITE_FAULT;
+		tcp_sock_dead(tcp);
+		return NT_STATUS_UNEXPECTED_NETWORK_ERROR;
 	}
 
 	return tcp_raw_recv(p, mem_ctx, reply_blob);
@@ -120,7 +136,8 @@ static NTSTATUS tcp_initial_request(struct dcerpc_pipe *p,
 
 	ret = write_data(tcp->fd, blob->data, blob->length);
 	if (ret != blob->length) {
-		return NT_STATUS_NET_WRITE_FAULT;
+		tcp_sock_dead(tcp);
+		return NT_STATUS_UNEXPECTED_NETWORK_ERROR;
 	}
 
 	return NT_STATUS_OK;
@@ -134,9 +151,7 @@ static NTSTATUS tcp_shutdown_pipe(struct dcerpc_pipe *p)
 {
 	struct tcp_private *tcp = p->transport.private;
 
-	if (tcp) {
+	tcp_sock_dead(tcp);
-		close(tcp->fd);
-	}
 
 	return NT_STATUS_OK;
 }

source/torture/rpc/autoidl.c

@@ -112,6 +112,11 @@ static void try_expand(TALLOC_CTX *mem_ctx, const struct dcerpc_interface_table
 					      insert_ofs, insert_ofs+n, depth+1);
 			}
 			return;
+		} else {
+#if 0
+			print_depth(depth);
+			printf("expand by %d gives fault 0x%x\n", n, p->last_fault_code);
+#endif
 		}
 		if (p->last_fault_code == 5) {
 			reopen(&p, iface);
@@ -222,7 +227,7 @@ static void test_scan_call(TALLOC_CTX *mem_ctx, const struct dcerpc_interface_ta
 
 static void test_auto_scan(TALLOC_CTX *mem_ctx, const struct dcerpc_interface_table *iface)
 {
-	test_scan_call(mem_ctx, iface, 0x41);
+	test_scan_call(mem_ctx, iface, 0x26);
 }
 
 BOOL torture_rpc_autoidl(int dummy)

source/torture/rpc/samr.c

@@ -701,30 +701,30 @@ static BOOL test_AddMemberToAlias(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 				  struct policy_handle *domain_handle,
 				  const struct dom_sid *domain_sid)
 {
-	struct samr_AddAliasMem r;
+	struct samr_AddAliasMember r;
-	struct samr_DelAliasMem d;
+	struct samr_DeleteAliasMember d;
 	NTSTATUS status;
 	BOOL ret = True;
 	struct dom_sid *sid;
 
 	sid = dom_sid_add_rid(mem_ctx, domain_sid, 512);
 
-	printf("testing AddAliasMem\n");
+	printf("testing AddAliasMember\n");
 	r.in.handle = alias_handle;
 	r.in.sid = sid;
 
-	status = dcerpc_samr_AddAliasMem(p, mem_ctx, &r);
+	status = dcerpc_samr_AddAliasMember(p, mem_ctx, &r);
 	if (!NT_STATUS_IS_OK(status)) {
-		printf("AddAliasMem failed - %s\n", nt_errstr(status));
+		printf("AddAliasMember failed - %s\n", nt_errstr(status));
 		ret = False;
 	}
 
 	d.in.handle = alias_handle;
 	d.in.sid = sid;
 
-	status = dcerpc_samr_DelAliasMem(p, mem_ctx, &d);
+	status = dcerpc_samr_DeleteAliasMember(p, mem_ctx, &d);
 	if (!NT_STATUS_IS_OK(status)) {
-		printf("DelAliasMem failed - %s\n", nt_errstr(status));
+		printf("DelAliasMember failed - %s\n", nt_errstr(status));
 		ret = False;
 	}
 
@@ -2177,6 +2177,8 @@ static BOOL test_AddGroupMember(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 	NTSTATUS status;
 	struct samr_AddGroupMember r;
 	struct samr_DeleteGroupMember d;
+	struct samr_QueryGroupMember q;
+	struct samr_SetMemberAttributesOfGroup s;
 	BOOL ret = True;
 	uint32 rid;
 
@@ -2214,6 +2216,25 @@ static BOOL test_AddGroupMember(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 		return False;
 	}
 
+	/* this one is quite strange. I am using random inputs in the
+	   hope of triggering an error that might give us a clue */
+	s.in.handle = group_handle;
+	s.in.unknown1 = random();
+	s.in.unknown2 = random();
+
+	status = dcerpc_samr_SetMemberAttributesOfGroup(p, mem_ctx, &s);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("SetMemberAttributesOfGroup failed - %s\n", nt_errstr(status));
+		return False;
+	}
+
+	q.in.handle = group_handle;
+
+	status = dcerpc_samr_QueryGroupMember(p, mem_ctx, &q);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("QueryGroupMember failed - %s\n", nt_errstr(status));
+		return False;
+	}
 
 	status = dcerpc_samr_DeleteGroupMember(p, mem_ctx, &d);
 	if (!NT_STATUS_IS_OK(status)) {
@@ -2319,6 +2340,8 @@ static BOOL test_OpenDomain(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx,
 
 	ZERO_STRUCT(user_handle);
 	ZERO_STRUCT(alias_handle);
+	ZERO_STRUCT(group_handle);
+	ZERO_STRUCT(domain_handle);
 
 	printf("Testing OpenDomain\n");