[PATCH] Some refactorings around source4/nbt_server

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Feb 26 09:14:37 UTC 2018


On Mon, Feb 26, 2018 at 10:14:06AM +0100, Volker Lendecke wrote:
> Hi!
> 
> Why? A future commit shall add the "unexpected" pipe to nbt_server,
> the same way that nmbd has it. This is supposed to make that easier,
> and hopefully the code a bit prettier.
> 
> Review appreciated!

... and now with patch.

Volker

-- 
Besuchen Sie die verinice.XP 2018 in Berlin,
Anwenderkonferenz für Informationssicherheit
vom 21.-23.03.2018 im Sofitel Kurfürstendamm
Info & Anmeldung hier: http://veriniceXP.org

SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From ffcb6902589e6100a760a67e100cb6e4e81a4966 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 15 Feb 2018 16:43:59 +0100
Subject: [PATCH 01/17] libsocket: Add "mem_ctx" to socket_create()

Every caller did a talloc_steal() after socket_create(). Just pass in the
correct memory context.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 libcli/nbt/nbtsocket.c                    |  5 ++---
 source4/auth/kerberos/krb5_init_context.c | 10 ++++++----
 source4/lib/socket/connect_multi.c        |  7 +++----
 source4/lib/socket/socket.c               |  5 +++--
 source4/lib/socket/socket.h               |  3 ++-
 source4/lib/socket/testsuite.c            | 12 ++++--------
 source4/libcli/dgram/dgramsocket.c        |  5 ++---
 source4/libcli/ldap/ldap_client.c         |  5 +++--
 source4/librpc/rpc/dcerpc_sock.c          |  5 ++---
 source4/smbd/service_stream.c             | 10 ++++++----
 10 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/libcli/nbt/nbtsocket.c b/libcli/nbt/nbtsocket.c
index d7abb1bf30b..711e39cbdc5 100644
--- a/libcli/nbt/nbtsocket.c
+++ b/libcli/nbt/nbtsocket.c
@@ -339,13 +339,12 @@ _PUBLIC_ struct nbt_name_socket *nbt_name_socket_init(TALLOC_CTX *mem_ctx,
 	nbtsock->event_ctx = event_ctx;
 	if (nbtsock->event_ctx == NULL) goto failed;
 
-	status = socket_create("ip", SOCKET_TYPE_DGRAM, &nbtsock->sock, 0);
+	status = socket_create(nbtsock, "ip", SOCKET_TYPE_DGRAM,
+			       &nbtsock->sock, 0);
 	if (!NT_STATUS_IS_OK(status)) goto failed;
 
 	socket_set_option(nbtsock->sock, "SO_BROADCAST", "1");
 
-	talloc_steal(nbtsock, nbtsock->sock);
-
 	nbtsock->idr = idr_init(nbtsock);
 	if (nbtsock->idr == NULL) goto failed;
 
diff --git a/source4/auth/kerberos/krb5_init_context.c b/source4/auth/kerberos/krb5_init_context.c
index e2c837abdce..5e771a87cc5 100644
--- a/source4/auth/kerberos/krb5_init_context.c
+++ b/source4/auth/kerberos/krb5_init_context.c
@@ -261,10 +261,14 @@ static krb5_error_code smb_krb5_send_and_recv_func_int(krb5_context context,
 		status = NT_STATUS_INVALID_PARAMETER;
 		switch (hi->proto) {
 		case KRB5_KRBHST_UDP:
-			status = socket_create(name, SOCKET_TYPE_DGRAM, &smb_krb5->sock, 0);
+			status = socket_create(smb_krb5, name,
+					       SOCKET_TYPE_DGRAM,
+					       &smb_krb5->sock, 0);
 			break;
 		case KRB5_KRBHST_TCP:
-			status = socket_create(name, SOCKET_TYPE_STREAM, &smb_krb5->sock, 0);
+			status = socket_create(smb_krb5, name,
+					       SOCKET_TYPE_STREAM,
+					       &smb_krb5->sock, 0);
 			break;
 		case KRB5_KRBHST_HTTP:
 			TALLOC_FREE(frame);
@@ -275,8 +279,6 @@ static krb5_error_code smb_krb5_send_and_recv_func_int(krb5_context context,
 			continue;
 		}
 
-		talloc_steal(smb_krb5, smb_krb5->sock);
-
 		remote_addr = socket_address_from_sockaddr(smb_krb5, a->ai_addr, a->ai_addrlen);
 		if (!remote_addr) {
 			talloc_free(smb_krb5);
diff --git a/source4/lib/socket/connect_multi.c b/source4/lib/socket/connect_multi.c
index c8231b3cff3..b29fffb33b4 100644
--- a/source4/lib/socket/connect_multi.c
+++ b/source4/lib/socket/connect_multi.c
@@ -152,8 +152,9 @@ static void connect_multi_next_socket(struct composite_context *result)
 	if (composite_nomem(state, result)) return;
 
 	state->result = result;
-	result->status = socket_create(multi->server_address[multi->current_address]->family,
-					SOCKET_TYPE_STREAM, &state->sock, 0);
+	result->status = socket_create(
+		state, multi->server_address[multi->current_address]->family,
+		SOCKET_TYPE_STREAM, &state->sock, 0);
 	if (!composite_is_ok(result)) return;
 
 	state->addr = socket_address_copy(state, multi->server_address[multi->current_address]);
@@ -161,8 +162,6 @@ static void connect_multi_next_socket(struct composite_context *result)
 
 	socket_address_set_port(state->addr, multi->ports[multi->current_port]);
 
-	talloc_steal(state, state->sock);
-
 	creq = socket_connect_send(state->sock, NULL, 
 				   state->addr, 0,
 				   result->event_ctx);
diff --git a/source4/lib/socket/socket.c b/source4/lib/socket/socket.c
index 42eb53abcef..98f796e3fed 100644
--- a/source4/lib/socket/socket.c
+++ b/source4/lib/socket/socket.c
@@ -96,7 +96,8 @@ _PUBLIC_ NTSTATUS socket_create_with_ops(TALLOC_CTX *mem_ctx, const struct socke
 	return NT_STATUS_OK;
 }
 
-_PUBLIC_ NTSTATUS socket_create(const char *name, enum socket_type type, 
+_PUBLIC_ NTSTATUS socket_create(TALLOC_CTX *mem_ctx,
+				const char *name, enum socket_type type,
 			        struct socket_context **new_sock, uint32_t flags)
 {
 	const struct socket_ops *ops;
@@ -106,7 +107,7 @@ _PUBLIC_ NTSTATUS socket_create(const char *name, enum socket_type type,
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 
-	return socket_create_with_ops(NULL, ops, new_sock, type, flags);
+	return socket_create_with_ops(mem_ctx, ops, new_sock, type, flags);
 }
 
 _PUBLIC_ NTSTATUS socket_connect(struct socket_context *sock,
diff --git a/source4/lib/socket/socket.h b/source4/lib/socket/socket.h
index 50a20d90911..a492bc10367 100644
--- a/source4/lib/socket/socket.h
+++ b/source4/lib/socket/socket.h
@@ -133,7 +133,8 @@ struct tsocket_address;
 NTSTATUS socket_create_with_ops(TALLOC_CTX *mem_ctx, const struct socket_ops *ops,
 				struct socket_context **new_sock, 
 				enum socket_type type, uint32_t flags);
-NTSTATUS socket_create(const char *name, enum socket_type type, 
+NTSTATUS socket_create(TALLOC_CTX *mem_ctx,
+		       const char *name, enum socket_type type,
 		       struct socket_context **new_sock, uint32_t flags);
 NTSTATUS socket_connect(struct socket_context *sock,
 			const struct socket_address *my_address, 
diff --git a/source4/lib/socket/testsuite.c b/source4/lib/socket/testsuite.c
index 6ebbd1227e8..1df96e3ccf7 100644
--- a/source4/lib/socket/testsuite.c
+++ b/source4/lib/socket/testsuite.c
@@ -45,13 +45,11 @@ static bool test_udp(struct torture_context *tctx)
 
 	load_interface_list(tctx, tctx->lp_ctx, &ifaces);
 
-	status = socket_create("ip", SOCKET_TYPE_DGRAM, &sock1, 0);
+	status = socket_create(mem_ctx, "ip", SOCKET_TYPE_DGRAM, &sock1, 0);
 	torture_assert_ntstatus_ok(tctx, status, "creating DGRAM IP socket 1");
-	talloc_steal(mem_ctx, sock1);
 
-	status = socket_create("ip", SOCKET_TYPE_DGRAM, &sock2, 0);
+	status = socket_create(mem_ctx, "ip", SOCKET_TYPE_DGRAM, &sock2, 0);
 	torture_assert_ntstatus_ok(tctx, status, "creating DGRAM IP socket 1");
-	talloc_steal(mem_ctx, sock2);
 
 	localhost = socket_address_from_strings(sock1, sock1->backend_name, 
 						iface_list_best_ip(ifaces, "127.0.0.1"), 0);
@@ -128,13 +126,11 @@ static bool test_tcp(struct torture_context *tctx)
 	struct tevent_context *ev = tctx->ev;
 	struct interface *ifaces;
 
-	status = socket_create("ip", SOCKET_TYPE_STREAM, &sock1, 0);
+	status = socket_create(mem_ctx, "ip", SOCKET_TYPE_STREAM, &sock1, 0);
 	torture_assert_ntstatus_ok(tctx, status, "creating IP stream socket 1");
-	talloc_steal(mem_ctx, sock1);
 
-	status = socket_create("ip", SOCKET_TYPE_STREAM, &sock2, 0);
+	status = socket_create(mem_ctx, "ip", SOCKET_TYPE_STREAM, &sock2, 0);
 	torture_assert_ntstatus_ok(tctx, status, "creating IP stream socket 1");
-	talloc_steal(mem_ctx, sock2);
 
 	load_interface_list(tctx, tctx->lp_ctx, &ifaces);
 	localhost = socket_address_from_strings(sock1, sock1->backend_name, 
diff --git a/source4/libcli/dgram/dgramsocket.c b/source4/libcli/dgram/dgramsocket.c
index b6e7dd12c7c..154a6670948 100644
--- a/source4/libcli/dgram/dgramsocket.c
+++ b/source4/libcli/dgram/dgramsocket.c
@@ -168,13 +168,12 @@ struct nbt_dgram_socket *nbt_dgram_socket_init(TALLOC_CTX *mem_ctx,
 	dgmsock->event_ctx = event_ctx;
 	if (dgmsock->event_ctx == NULL) goto failed;
 
-	status = socket_create("ip", SOCKET_TYPE_DGRAM, &dgmsock->sock, 0);
+	status = socket_create(dgmsock, "ip", SOCKET_TYPE_DGRAM,
+			       &dgmsock->sock, 0);
 	if (!NT_STATUS_IS_OK(status)) goto failed;
 
 	socket_set_option(dgmsock->sock, "SO_BROADCAST", "1");
 
-	talloc_steal(dgmsock, dgmsock->sock);
-
 	dgmsock->fde = tevent_add_fd(dgmsock->event_ctx, dgmsock,
 				    socket_get_fd(dgmsock->sock), 0,
 				    dgm_socket_handler, dgmsock);
diff --git a/source4/libcli/ldap/ldap_client.c b/source4/libcli/ldap/ldap_client.c
index b5f5da6fa00..1cbcd0d42d5 100644
--- a/source4/libcli/ldap/ldap_client.c
+++ b/source4/libcli/ldap/ldap_client.c
@@ -413,11 +413,12 @@ _PUBLIC_ struct composite_context *ldap_connect_send(struct ldap_connection *con
 		struct socket_address *unix_addr;
 		char path[1025];
 		char *end = NULL;
-		NTSTATUS status = socket_create("unix", SOCKET_TYPE_STREAM, &state->sock, 0);
+		NTSTATUS status = socket_create(state, "unix",
+						SOCKET_TYPE_STREAM,
+						&state->sock, 0);
 		if (!NT_STATUS_IS_OK(status)) {
 			return NULL;
 		}
-		talloc_steal(state, state->sock);
 		SMB_ASSERT(sizeof(protocol)>10);
 		SMB_ASSERT(sizeof(path)>1024);
 	
diff --git a/source4/librpc/rpc/dcerpc_sock.c b/source4/librpc/rpc/dcerpc_sock.c
index 6401534806e..e7ecca73e3c 100644
--- a/source4/librpc/rpc/dcerpc_sock.c
+++ b/source4/librpc/rpc/dcerpc_sock.c
@@ -151,11 +151,10 @@ static struct composite_context *dcerpc_pipe_open_socket_send(TALLOC_CTX *mem_ct
 		if (composite_nomem(s->target_hostname, c)) return c;
 	}
 
-	c->status = socket_create(server->family, SOCKET_TYPE_STREAM, &s->socket_ctx, 0);
+	c->status = socket_create(s, server->family, SOCKET_TYPE_STREAM,
+				  &s->socket_ctx, 0);
 	if (!composite_is_ok(c)) return c;
 
-	talloc_steal(s, s->socket_ctx);
-
 	conn_req = socket_connect_send(s->socket_ctx, s->localaddr, s->server, 0,
 				       c->event_ctx);
 	composite_continue(c, conn_req, continue_socket_connect, c);
diff --git a/source4/smbd/service_stream.c b/source4/smbd/service_stream.c
index 545cd4315b3..fc996d942e6 100644
--- a/source4/smbd/service_stream.c
+++ b/source4/smbd/service_stream.c
@@ -315,10 +315,14 @@ NTSTATUS stream_setup_socket(TALLOC_CTX *mem_ctx,
 			return NT_STATUS_NO_MEMORY;
 		}
 
-		status = socket_create(socket_address->family, SOCKET_TYPE_STREAM, &stream_socket->sock, 0);
+		status = socket_create(stream_socket, socket_address->family,
+				       SOCKET_TYPE_STREAM,
+				       &stream_socket->sock, 0);
 		NT_STATUS_NOT_OK_RETURN(status);
 	} else {
-		status = socket_create(family, SOCKET_TYPE_STREAM, &stream_socket->sock, 0);
+		status = socket_create(stream_socket, family,
+				       SOCKET_TYPE_STREAM,
+				       &stream_socket->sock, 0);
 		NT_STATUS_NOT_OK_RETURN(status);
 
 		/* this is for non-IP sockets, eg. unix domain sockets */
@@ -329,8 +333,6 @@ NTSTATUS stream_setup_socket(TALLOC_CTX *mem_ctx,
 	}
 
 
-	talloc_steal(stream_socket, stream_socket->sock);
-
 	stream_socket->lp_ctx = talloc_reference(stream_socket, lp_ctx);
 
 	/* ready to listen */
-- 
2.11.0


From 7bff7d2724b88576d84a4824c8cf6a3c8effa3a8 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 20:42:19 +0100
Subject: [PATCH 02/17] libdgram: Remove an unused parameter

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/libcli/dgram/libdgram.h     | 3 +--
 source4/libcli/dgram/netlogon.c     | 3 +--
 source4/nbt_server/dgram/netlogon.c | 3 ++-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/source4/libcli/dgram/libdgram.h b/source4/libcli/dgram/libdgram.h
index 280c27d41d1..64a91272058 100644
--- a/source4/libcli/dgram/libdgram.h
+++ b/source4/libcli/dgram/libdgram.h
@@ -127,8 +127,7 @@ NTSTATUS dgram_mailslot_netlogon_reply(struct nbt_dgram_socket *dgmsock,
 				       const char *my_netbios_name,
 				       const char *mailslot_name,
 				       struct nbt_netlogon_response *reply);
-NTSTATUS dgram_mailslot_netlogon_parse_request(struct dgram_mailslot_handler *dgmslot,
-					       TALLOC_CTX *mem_ctx,
+NTSTATUS dgram_mailslot_netlogon_parse_request(TALLOC_CTX *mem_ctx,
 					       struct nbt_dgram_packet *dgram,
 					       struct nbt_netlogon_packet *netlogon);
 
diff --git a/source4/libcli/dgram/netlogon.c b/source4/libcli/dgram/netlogon.c
index a16a3b96624..f224afda79d 100644
--- a/source4/libcli/dgram/netlogon.c
+++ b/source4/libcli/dgram/netlogon.c
@@ -99,8 +99,7 @@ NTSTATUS dgram_mailslot_netlogon_reply(struct nbt_dgram_socket *dgmsock,
 /*
   parse a netlogon request. The packet must be a valid mailslot packet
 */
-NTSTATUS dgram_mailslot_netlogon_parse_request(struct dgram_mailslot_handler *dgmslot,
-					       TALLOC_CTX *mem_ctx,
+NTSTATUS dgram_mailslot_netlogon_parse_request(TALLOC_CTX *mem_ctx,
 					       struct nbt_dgram_packet *dgram,
 					       struct nbt_netlogon_packet *netlogon)
 {
diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index c88ffb51f2c..1a0b9eeb491 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -171,7 +171,8 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 
 	DEBUG(5,("netlogon request to %s from %s:%d\n",
 		 nbt_name_string(netlogon, name), src->addr, src->port));
-	status = dgram_mailslot_netlogon_parse_request(dgmslot, netlogon, packet, netlogon);
+	status = dgram_mailslot_netlogon_parse_request(netlogon, packet,
+						       netlogon);
 	if (!NT_STATUS_IS_OK(status)) goto failed;
 
 	switch (netlogon->command) {
-- 
2.11.0


From dd435158295b842acac75ed296bca1d25136cf85 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 20:45:22 +0100
Subject: [PATCH 03/17] libdgram: Remove an unused parameter

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/libcli/dgram/libdgram.h | 3 +--
 source4/libcli/dgram/netlogon.c | 7 +++----
 source4/nbt_server/irpc.c       | 2 +-
 source4/torture/nbt/dgram.c     | 3 ++-
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/source4/libcli/dgram/libdgram.h b/source4/libcli/dgram/libdgram.h
index 64a91272058..0f313a67cf5 100644
--- a/source4/libcli/dgram/libdgram.h
+++ b/source4/libcli/dgram/libdgram.h
@@ -131,8 +131,7 @@ NTSTATUS dgram_mailslot_netlogon_parse_request(TALLOC_CTX *mem_ctx,
 					       struct nbt_dgram_packet *dgram,
 					       struct nbt_netlogon_packet *netlogon);
 
-NTSTATUS dgram_mailslot_netlogon_parse_response(struct dgram_mailslot_handler *dgmslot,
-						TALLOC_CTX *mem_ctx,
+NTSTATUS dgram_mailslot_netlogon_parse_response(TALLOC_CTX *mem_ctx,
 						struct nbt_dgram_packet *dgram,
 						struct nbt_netlogon_response *netlogon);
 
diff --git a/source4/libcli/dgram/netlogon.c b/source4/libcli/dgram/netlogon.c
index f224afda79d..25612acc6d0 100644
--- a/source4/libcli/dgram/netlogon.c
+++ b/source4/libcli/dgram/netlogon.c
@@ -123,10 +123,9 @@ NTSTATUS dgram_mailslot_netlogon_parse_request(TALLOC_CTX *mem_ctx,
 /*
   parse a netlogon response. The packet must be a valid mailslot packet
 */
-NTSTATUS dgram_mailslot_netlogon_parse_response(struct dgram_mailslot_handler *dgmslot,
-				       TALLOC_CTX *mem_ctx,
-				       struct nbt_dgram_packet *dgram,
-				       struct nbt_netlogon_response *netlogon)
+NTSTATUS dgram_mailslot_netlogon_parse_response(TALLOC_CTX *mem_ctx,
+						struct nbt_dgram_packet *dgram,
+						struct nbt_netlogon_response *netlogon)
 {
 	NTSTATUS status;
 	DATA_BLOB data = dgram_mailslot_data(dgram);
diff --git a/source4/nbt_server/irpc.c b/source4/nbt_server/irpc.c
index 261e066b16b..cfe617b53b0 100644
--- a/source4/nbt_server/irpc.c
+++ b/source4/nbt_server/irpc.c
@@ -70,7 +70,7 @@ static void getdc_recv_netlogon_reply(struct dgram_mailslot_handler *dgmslot,
 	struct nbt_netlogon_response netlogon;
 	NTSTATUS status;
 
-	status = dgram_mailslot_netlogon_parse_response(dgmslot, packet, packet,
+	status = dgram_mailslot_netlogon_parse_response(packet, packet,
 							&netlogon);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(5, ("dgram_mailslot_ntlogon_parse failed: %s\n",
diff --git a/source4/torture/nbt/dgram.c b/source4/torture/nbt/dgram.c
index 549ecad1f94..2f7ea19443f 100644
--- a/source4/torture/nbt/dgram.c
+++ b/source4/torture/nbt/dgram.c
@@ -51,7 +51,8 @@ static void netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 	printf("netlogon reply from %s:%d\n", src->addr, src->port);
 
 	/* Fills in the netlogon pointer */
-	status = dgram_mailslot_netlogon_parse_response(dgmslot, netlogon, packet, netlogon);
+	status = dgram_mailslot_netlogon_parse_response(netlogon, packet,
+							netlogon);
 	if (!NT_STATUS_IS_OK(status)) {
 		printf("Failed to parse netlogon packet from %s:%d\n",
 		       src->addr, src->port);
-- 
2.11.0


From bf3aa3be5bba690cf5208bc93b22ba03b1a925bd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 16 Feb 2018 20:21:17 +0100
Subject: [PATCH 04/17] nbt_server: Remove some unused parameters

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 1a0b9eeb491..879e21dc638 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -36,8 +36,7 @@
 /*
   reply to a GETDC request
  */
-static void nbtd_netlogon_getdc(struct dgram_mailslot_handler *dgmslot, 
-				struct nbtd_interface *iface,
+static void nbtd_netlogon_getdc(struct nbtd_interface *iface,
 				struct nbt_dgram_packet *packet, 
 				const struct socket_address *src,
 				struct nbt_netlogon_packet *netlogon)
@@ -90,8 +89,7 @@ static void nbtd_netlogon_getdc(struct dgram_mailslot_handler *dgmslot,
 /*
   reply to a ADS style GETDC request
  */
-static void nbtd_netlogon_samlogon(struct dgram_mailslot_handler *dgmslot,
-				   struct nbtd_interface *iface,
+static void nbtd_netlogon_samlogon(struct nbtd_interface *iface,
 				   struct nbt_dgram_packet *packet, 
 				   const struct socket_address *src,
 				   struct nbt_netlogon_packet *netlogon)
@@ -177,12 +175,10 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 
 	switch (netlogon->command) {
 	case LOGON_PRIMARY_QUERY:
-		nbtd_netlogon_getdc(dgmslot, iface, packet, 
-				    src, netlogon);
+		nbtd_netlogon_getdc(iface, packet, src, netlogon);
 		break;
 	case LOGON_SAM_LOGON_REQUEST:
-		nbtd_netlogon_samlogon(dgmslot, iface, packet, 
-				       src, netlogon);
+		nbtd_netlogon_samlogon(iface, packet, src, netlogon);
 		break;
 	default:
 		DEBUG(2,("unknown netlogon op %d from %s:%d\n", 
-- 
2.11.0


From 542d4aa7d7efcb99a0ca3319dafd316188612410 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 16:49:00 +0100
Subject: [PATCH 05/17] nbt_server: Remove a pointless assignment

I don't see how data.msg.dest_name.type is accessed further down

dgram_mailslot_netlogon_reply only accesses packet->src_addr, packet->src_port
and packet->data.msg.source_name, *not* data.msg.dest_name. Also, "packet" is
thrown away after this call.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 879e21dc638..0954716c663 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -131,8 +131,6 @@ static void nbtd_netlogon_samlogon(struct nbtd_interface *iface,
 
 	netlogon_response.response_type = NETLOGON_SAMLOGON;
 
-	packet->data.msg.dest_name.type = 0;
-
 	dgram_mailslot_netlogon_reply(reply_iface->dgmsock, 
 				      packet, 
 				      lpcfg_netbios_name(iface->nbtsrv->task->lp_ctx),
-- 
2.11.0


From 7a26aa017d20fef6d551d99edfef21b4d3560f8d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 17:05:09 +0100
Subject: [PATCH 06/17] nbt_server: Factor out dgram sending from reply
 construction

Separation of concerns. Only one call to dgram_mailslot_netlogon_reply, which
does the UDP send.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 118 +++++++++++++++++++++++-------------
 1 file changed, 76 insertions(+), 42 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 0954716c663..24265d9a4dd 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -36,20 +36,21 @@
 /*
   reply to a GETDC request
  */
-static void nbtd_netlogon_getdc(struct nbtd_interface *iface,
-				struct nbt_dgram_packet *packet, 
-				const struct socket_address *src,
-				struct nbt_netlogon_packet *netlogon)
+static NTSTATUS nbtd_netlogon_getdc(struct nbtd_interface *iface,
+				    struct nbt_dgram_packet *packet,
+				    const struct socket_address *src,
+				    struct nbt_netlogon_packet *netlogon,
+				    TALLOC_CTX *mem_ctx,
+				    struct nbt_netlogon_response **presponse)
 {
 	struct nbt_name *name = &packet->data.msg.dest_name;
-	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(iface, src->addr, false);
 	struct nbt_netlogon_response_from_pdc *pdc;
 	struct ldb_context *samctx;
-	struct nbt_netlogon_response netlogon_response;
+	struct nbt_netlogon_response *response;
 
 	/* only answer getdc requests on the PDC or LOGON names */
 	if (name->type != NBT_NAME_PDC && name->type != NBT_NAME_LOGON) {
-		return;
+		return NT_STATUS_NOT_SUPPORTED;
 	}
 
 	samctx = iface->nbtsrv->sam_ctx;
@@ -57,59 +58,75 @@ static void nbtd_netlogon_getdc(struct nbtd_interface *iface,
 	if (lpcfg_server_role(iface->nbtsrv->task->lp_ctx) != ROLE_ACTIVE_DIRECTORY_DC
 	    || !samdb_is_pdc(samctx)) {
 		DEBUG(2, ("Not a PDC, so not processing LOGON_PRIMARY_QUERY\n"));
-		return;		
+		return NT_STATUS_NOT_SUPPORTED;
 	}
 
 	if (strcasecmp_m(name->name, lpcfg_workgroup(iface->nbtsrv->task->lp_ctx)) != 0) {
 		DEBUG(5,("GetDC requested for a domian %s that we don't host\n", name->name));
-		return;
+		return NT_STATUS_NOT_SUPPORTED;
 	}
 
 	/* setup a GETDC reply */
-	ZERO_STRUCT(netlogon_response);
-	netlogon_response.response_type = NETLOGON_GET_PDC;
-	pdc = &netlogon_response.data.get_pdc;
+	response = talloc_zero(mem_ctx, struct nbt_netlogon_response);
+	if (response == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	response->response_type = NETLOGON_GET_PDC;
+	pdc = &response->data.get_pdc;
 
 	pdc->command = NETLOGON_RESPONSE_FROM_PDC;
-	pdc->pdc_name         = lpcfg_netbios_name(iface->nbtsrv->task->lp_ctx);
+
+	pdc->pdc_name = talloc_strdup(
+		response, lpcfg_netbios_name(iface->nbtsrv->task->lp_ctx));
+	if (pdc->pdc_name == NULL) {
+		TALLOC_FREE(response);
+		return NT_STATUS_NO_MEMORY;
+	}
+
 	pdc->unicode_pdc_name = pdc->pdc_name;
-	pdc->domain_name      = lpcfg_workgroup(iface->nbtsrv->task->lp_ctx);
+
+	pdc->domain_name = talloc_strdup(
+		response, lpcfg_workgroup(iface->nbtsrv->task->lp_ctx));
+	if (pdc->domain_name == NULL) {
+		TALLOC_FREE(response);
+		return NT_STATUS_NO_MEMORY;
+	}
+
 	pdc->nt_version       = 1;
 	pdc->lmnt_token       = 0xFFFF;
 	pdc->lm20_token       = 0xFFFF;
 
-	dgram_mailslot_netlogon_reply(reply_iface->dgmsock, 
-				      packet, 
-				      lpcfg_netbios_name(iface->nbtsrv->task->lp_ctx),
-				      netlogon->req.pdc.mailslot_name,
-				      &netlogon_response);
+	*presponse = response;
+	return NT_STATUS_OK;
 }
 
-
 /*
   reply to a ADS style GETDC request
  */
-static void nbtd_netlogon_samlogon(struct nbtd_interface *iface,
-				   struct nbt_dgram_packet *packet, 
-				   const struct socket_address *src,
-				   struct nbt_netlogon_packet *netlogon)
+static NTSTATUS nbtd_netlogon_samlogon(
+	struct nbtd_interface *iface,
+	struct nbt_dgram_packet *packet,
+	const struct socket_address *src,
+	struct nbt_netlogon_packet *netlogon,
+	TALLOC_CTX *mem_ctx,
+	struct nbt_netlogon_response **presponse)
 {
 	struct nbt_name *name = &packet->data.msg.dest_name;
-	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(iface, src->addr, false);
 	struct ldb_context *samctx;
+	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(iface, src->addr, false);
 	const char *my_ip = reply_iface->ip_address; 
 	struct dom_sid *sid;
-	struct nbt_netlogon_response netlogon_response;
+	struct nbt_netlogon_response *response;
 	NTSTATUS status;
 
 	if (!my_ip) {
 		DEBUG(0, ("Could not obtain own IP address for datagram socket\n"));
-		return;
+		return NT_STATUS_NOT_SUPPORTED;
 	}
 
 	/* only answer getdc requests on the PDC or LOGON names */
 	if (name->type != NBT_NAME_PDC && name->type != NBT_NAME_LOGON) {
-		return;
+		return NT_STATUS_NOT_SUPPORTED;
 	}
 
 	samctx = iface->nbtsrv->sam_ctx;
@@ -120,25 +137,29 @@ static void nbtd_netlogon_samlogon(struct nbtd_interface *iface,
 		sid = NULL;
 	}
 
-	status = fill_netlogon_samlogon_response(samctx, packet, NULL, name->name, sid, NULL, 
-						 netlogon->req.logon.user_name, netlogon->req.logon.acct_control, src->addr, 
-						 netlogon->req.logon.nt_version, iface->nbtsrv->task->lp_ctx, &netlogon_response.data.samlogon, false);
+	response = talloc_zero(mem_ctx, struct nbt_netlogon_response);
+	if (response == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	response->response_type = NETLOGON_SAMLOGON;
+
+	status = fill_netlogon_samlogon_response(
+		samctx, response, NULL, name->name, sid, NULL,
+		netlogon->req.logon.user_name,
+		netlogon->req.logon.acct_control, src->addr,
+		netlogon->req.logon.nt_version, iface->nbtsrv->task->lp_ctx,
+		&response->data.samlogon, false);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(2,("NBT netlogon query failed domain=%s sid=%s version=%d - %s\n",
 			 name->name, dom_sid_string(packet, sid), netlogon->req.logon.nt_version, nt_errstr(status)));
-		return;
+		TALLOC_FREE(response);
+		return status;
 	}
 
-	netlogon_response.response_type = NETLOGON_SAMLOGON;
-
-	dgram_mailslot_netlogon_reply(reply_iface->dgmsock, 
-				      packet, 
-				      lpcfg_netbios_name(iface->nbtsrv->task->lp_ctx),
-				      netlogon->req.logon.mailslot_name,
-				      &netlogon_response);
+	*presponse = response;
+	return NT_STATUS_OK;
 }
 
-
 /*
   handle incoming netlogon mailslot requests
 */
@@ -151,8 +172,11 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 		talloc_get_type(dgmslot->private_data, struct nbtd_interface);
 	struct nbt_netlogon_packet *netlogon = 
 		talloc(dgmslot, struct nbt_netlogon_packet);
+	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(
+		iface, src->addr, false);
 	struct nbtd_iface_name *iname;
 	struct nbt_name *name = &packet->data.msg.dest_name;
+	struct nbt_netlogon_response *response;
 
 	if (netlogon == NULL) goto failed;
 
@@ -173,18 +197,28 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 
 	switch (netlogon->command) {
 	case LOGON_PRIMARY_QUERY:
-		nbtd_netlogon_getdc(iface, packet, src, netlogon);
+		status = nbtd_netlogon_getdc(iface, packet, src, netlogon,
+					     netlogon, &response);
 		break;
 	case LOGON_SAM_LOGON_REQUEST:
-		nbtd_netlogon_samlogon(iface, packet, src, netlogon);
+		status = nbtd_netlogon_samlogon(iface, packet, src, netlogon,
+						netlogon, &response);
 		break;
 	default:
 		DEBUG(2,("unknown netlogon op %d from %s:%d\n", 
 			 netlogon->command, src->addr, src->port));
 		NDR_PRINT_DEBUG(nbt_netlogon_packet, netlogon);
+		status = NT_STATUS_NOT_SUPPORTED;
 		break;
 	}
 
+	if (NT_STATUS_IS_OK(status)) {
+		dgram_mailslot_netlogon_reply(
+			reply_iface->dgmsock, packet,
+			lpcfg_netbios_name(iface->nbtsrv->task->lp_ctx),
+			netlogon->req.logon.mailslot_name, response);
+	}
+
 	talloc_free(netlogon);
 	return;
 
-- 
2.11.0


From c3519b3e4f90c8d027d84b49e05d95d913e70379 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 17:09:31 +0100
Subject: [PATCH 07/17] nbt_server: Make nbtd_mailslot_netlogon_handler a bit
 more idiomatic

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 24265d9a4dd..5ed3223ddf2 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -170,15 +170,17 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 	NTSTATUS status = NT_STATUS_NO_MEMORY;
 	struct nbtd_interface *iface = 
 		talloc_get_type(dgmslot->private_data, struct nbtd_interface);
-	struct nbt_netlogon_packet *netlogon = 
-		talloc(dgmslot, struct nbt_netlogon_packet);
+	struct nbt_netlogon_packet *netlogon;
 	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(
 		iface, src->addr, false);
 	struct nbtd_iface_name *iname;
 	struct nbt_name *name = &packet->data.msg.dest_name;
 	struct nbt_netlogon_response *response;
 
-	if (netlogon == NULL) goto failed;
+	netlogon = talloc(dgmslot, struct nbt_netlogon_packet);
+	if (netlogon == NULL) {
+		goto failed;
+	}
 
 	/*
 	  see if the we are listening on the destination netbios name
-- 
2.11.0


From e9daa3a62cf7f4ab7aa26583250e3441fcec6d76 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 17:11:41 +0100
Subject: [PATCH 08/17] nbt_server: Centralize a consistency check

This is a "should NEVER happen" and applies to both
LOGON_PRIMARY_QUERY and LOGON_SAM_LOGON_REQUEST

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 5ed3223ddf2..ede3e77c965 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -113,17 +113,10 @@ static NTSTATUS nbtd_netlogon_samlogon(
 {
 	struct nbt_name *name = &packet->data.msg.dest_name;
 	struct ldb_context *samctx;
-	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(iface, src->addr, false);
-	const char *my_ip = reply_iface->ip_address; 
 	struct dom_sid *sid;
 	struct nbt_netlogon_response *response;
 	NTSTATUS status;
 
-	if (!my_ip) {
-		DEBUG(0, ("Could not obtain own IP address for datagram socket\n"));
-		return NT_STATUS_NOT_SUPPORTED;
-	}
-
 	/* only answer getdc requests on the PDC or LOGON names */
 	if (name->type != NBT_NAME_PDC && name->type != NBT_NAME_LOGON) {
 		return NT_STATUS_NOT_SUPPORTED;
@@ -170,13 +163,19 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 	NTSTATUS status = NT_STATUS_NO_MEMORY;
 	struct nbtd_interface *iface = 
 		talloc_get_type(dgmslot->private_data, struct nbtd_interface);
-	struct nbt_netlogon_packet *netlogon;
+	struct nbt_netlogon_packet *netlogon = NULL;
 	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(
 		iface, src->addr, false);
 	struct nbtd_iface_name *iname;
 	struct nbt_name *name = &packet->data.msg.dest_name;
 	struct nbt_netlogon_response *response;
 
+	if (reply_iface->ip_address == NULL) {
+		DBG_WARNING("Could not obtain own IP address for datagram "
+			    "socket\n");
+		goto failed;
+	}
+
 	netlogon = talloc(dgmslot, struct nbt_netlogon_packet);
 	if (netlogon == NULL) {
 		goto failed;
-- 
2.11.0


From 9cce14e60aca072b9bfce406f01216684121d4b5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 17:16:07 +0100
Subject: [PATCH 09/17] nbt_server: nbtd_netlogon_getdc needs the nbtsrv, not
 the interface

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index ede3e77c965..610e41eb951 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -36,7 +36,7 @@
 /*
   reply to a GETDC request
  */
-static NTSTATUS nbtd_netlogon_getdc(struct nbtd_interface *iface,
+static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 				    struct nbt_dgram_packet *packet,
 				    const struct socket_address *src,
 				    struct nbt_netlogon_packet *netlogon,
@@ -53,15 +53,15 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_interface *iface,
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
-	samctx = iface->nbtsrv->sam_ctx;
+	samctx = nbtsrv->sam_ctx;
 
-	if (lpcfg_server_role(iface->nbtsrv->task->lp_ctx) != ROLE_ACTIVE_DIRECTORY_DC
+	if (lpcfg_server_role(nbtsrv->task->lp_ctx) != ROLE_ACTIVE_DIRECTORY_DC
 	    || !samdb_is_pdc(samctx)) {
 		DEBUG(2, ("Not a PDC, so not processing LOGON_PRIMARY_QUERY\n"));
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
-	if (strcasecmp_m(name->name, lpcfg_workgroup(iface->nbtsrv->task->lp_ctx)) != 0) {
+	if (strcasecmp_m(name->name, lpcfg_workgroup(nbtsrv->task->lp_ctx)) != 0) {
 		DEBUG(5,("GetDC requested for a domian %s that we don't host\n", name->name));
 		return NT_STATUS_NOT_SUPPORTED;
 	}
@@ -77,7 +77,7 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_interface *iface,
 	pdc->command = NETLOGON_RESPONSE_FROM_PDC;
 
 	pdc->pdc_name = talloc_strdup(
-		response, lpcfg_netbios_name(iface->nbtsrv->task->lp_ctx));
+		response, lpcfg_netbios_name(nbtsrv->task->lp_ctx));
 	if (pdc->pdc_name == NULL) {
 		TALLOC_FREE(response);
 		return NT_STATUS_NO_MEMORY;
@@ -86,7 +86,7 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_interface *iface,
 	pdc->unicode_pdc_name = pdc->pdc_name;
 
 	pdc->domain_name = talloc_strdup(
-		response, lpcfg_workgroup(iface->nbtsrv->task->lp_ctx));
+		response, lpcfg_workgroup(nbtsrv->task->lp_ctx));
 	if (pdc->domain_name == NULL) {
 		TALLOC_FREE(response);
 		return NT_STATUS_NO_MEMORY;
@@ -198,8 +198,8 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 
 	switch (netlogon->command) {
 	case LOGON_PRIMARY_QUERY:
-		status = nbtd_netlogon_getdc(iface, packet, src, netlogon,
-					     netlogon, &response);
+		status = nbtd_netlogon_getdc(iface->nbtsrv, packet, src,
+					     netlogon, netlogon, &response);
 		break;
 	case LOGON_SAM_LOGON_REQUEST:
 		status = nbtd_netlogon_samlogon(iface, packet, src, netlogon,
-- 
2.11.0


From 7a6fe80ff6d3baa1af576f9dc2c4bf4f149708c6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 17:18:29 +0100
Subject: [PATCH 10/17] nbt_server: nbtd_netlogon_samlogon needs the nbtsrv,
 not the inteface

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 610e41eb951..9c9458cbda8 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -104,7 +104,7 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
   reply to a ADS style GETDC request
  */
 static NTSTATUS nbtd_netlogon_samlogon(
-	struct nbtd_interface *iface,
+	struct nbtd_server *nbtsrv,
 	struct nbt_dgram_packet *packet,
 	const struct socket_address *src,
 	struct nbt_netlogon_packet *netlogon,
@@ -122,7 +122,7 @@ static NTSTATUS nbtd_netlogon_samlogon(
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
-	samctx = iface->nbtsrv->sam_ctx;
+	samctx = nbtsrv->sam_ctx;
 
 	if (netlogon->req.logon.sid_size) {
 		sid = &netlogon->req.logon.sid;
@@ -140,7 +140,7 @@ static NTSTATUS nbtd_netlogon_samlogon(
 		samctx, response, NULL, name->name, sid, NULL,
 		netlogon->req.logon.user_name,
 		netlogon->req.logon.acct_control, src->addr,
-		netlogon->req.logon.nt_version, iface->nbtsrv->task->lp_ctx,
+		netlogon->req.logon.nt_version, nbtsrv->task->lp_ctx,
 		&response->data.samlogon, false);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(2,("NBT netlogon query failed domain=%s sid=%s version=%d - %s\n",
@@ -202,8 +202,8 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 					     netlogon, netlogon, &response);
 		break;
 	case LOGON_SAM_LOGON_REQUEST:
-		status = nbtd_netlogon_samlogon(iface, packet, src, netlogon,
-						netlogon, &response);
+		status = nbtd_netlogon_samlogon(iface->nbtsrv, packet, src,
+						netlogon, netlogon, &response);
 		break;
 	default:
 		DEBUG(2,("unknown netlogon op %d from %s:%d\n", 
-- 
2.11.0


From 16494a41b54fa5bb3ad389b9fa39cd90cc7af3ab Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 17:22:11 +0100
Subject: [PATCH 11/17] nbt_server: Fix a typo ("domian->domain")

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 9c9458cbda8..f9fbf381cf4 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -62,7 +62,8 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 	}
 
 	if (strcasecmp_m(name->name, lpcfg_workgroup(nbtsrv->task->lp_ctx)) != 0) {
-		DEBUG(5,("GetDC requested for a domian %s that we don't host\n", name->name));
+		DBG_INFO("GetDC requested for a domain %s that we don't "
+			 "host\n", name->name);
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
-- 
2.11.0


From 4efe057ed4ac90d7af302eae1812979ca7b28c47 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 17:25:03 +0100
Subject: [PATCH 12/17] nbt_server: nbtd_netlogon_getdc needs just the dst_name

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index f9fbf381cf4..36a214bdcf8 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -37,19 +37,19 @@
   reply to a GETDC request
  */
 static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
-				    struct nbt_dgram_packet *packet,
+				    struct nbt_name *dst_name,
 				    const struct socket_address *src,
 				    struct nbt_netlogon_packet *netlogon,
 				    TALLOC_CTX *mem_ctx,
 				    struct nbt_netlogon_response **presponse)
 {
-	struct nbt_name *name = &packet->data.msg.dest_name;
 	struct nbt_netlogon_response_from_pdc *pdc;
 	struct ldb_context *samctx;
 	struct nbt_netlogon_response *response;
 
 	/* only answer getdc requests on the PDC or LOGON names */
-	if (name->type != NBT_NAME_PDC && name->type != NBT_NAME_LOGON) {
+	if ((dst_name->type != NBT_NAME_PDC) &&
+	    (dst_name->type != NBT_NAME_LOGON)) {
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
@@ -61,9 +61,10 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
-	if (strcasecmp_m(name->name, lpcfg_workgroup(nbtsrv->task->lp_ctx)) != 0) {
+	if (strcasecmp_m(dst_name->name,
+			 lpcfg_workgroup(nbtsrv->task->lp_ctx)) != 0) {
 		DBG_INFO("GetDC requested for a domain %s that we don't "
-			 "host\n", name->name);
+			 "host\n", dst_name->name);
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
@@ -199,7 +200,8 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 
 	switch (netlogon->command) {
 	case LOGON_PRIMARY_QUERY:
-		status = nbtd_netlogon_getdc(iface->nbtsrv, packet, src,
+		status = nbtd_netlogon_getdc(iface->nbtsrv,
+					     &packet->data.msg.dest_name, src,
 					     netlogon, netlogon, &response);
 		break;
 	case LOGON_SAM_LOGON_REQUEST:
-- 
2.11.0


From c86eec7534ee77aa276500df548b6cce1c02a974 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 17:33:32 +0100
Subject: [PATCH 13/17] nbt_server: nbtd_netlogon_samlogon needs the dst_name,
 not the packet

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 36a214bdcf8..d37f914fa68 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -107,20 +107,20 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
  */
 static NTSTATUS nbtd_netlogon_samlogon(
 	struct nbtd_server *nbtsrv,
-	struct nbt_dgram_packet *packet,
+	struct nbt_name *dst_name,
 	const struct socket_address *src,
 	struct nbt_netlogon_packet *netlogon,
 	TALLOC_CTX *mem_ctx,
 	struct nbt_netlogon_response **presponse)
 {
-	struct nbt_name *name = &packet->data.msg.dest_name;
 	struct ldb_context *samctx;
 	struct dom_sid *sid;
 	struct nbt_netlogon_response *response;
 	NTSTATUS status;
 
 	/* only answer getdc requests on the PDC or LOGON names */
-	if (name->type != NBT_NAME_PDC && name->type != NBT_NAME_LOGON) {
+	if ((dst_name->type != NBT_NAME_PDC) &&
+	    (dst_name->type != NBT_NAME_LOGON)) {
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
@@ -139,14 +139,16 @@ static NTSTATUS nbtd_netlogon_samlogon(
 	response->response_type = NETLOGON_SAMLOGON;
 
 	status = fill_netlogon_samlogon_response(
-		samctx, response, NULL, name->name, sid, NULL,
+		samctx, response, NULL, dst_name->name, sid, NULL,
 		netlogon->req.logon.user_name,
 		netlogon->req.logon.acct_control, src->addr,
 		netlogon->req.logon.nt_version, nbtsrv->task->lp_ctx,
 		&response->data.samlogon, false);
 	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(2,("NBT netlogon query failed domain=%s sid=%s version=%d - %s\n",
-			 name->name, dom_sid_string(packet, sid), netlogon->req.logon.nt_version, nt_errstr(status)));
+		DBG_NOTICE("NBT netlogon query failed domain=%s sid=%s "
+			   "version=%d - %s\n", dst_name->name,
+			   dom_sid_string(response, sid),
+			   netlogon->req.logon.nt_version, nt_errstr(status));
 		TALLOC_FREE(response);
 		return status;
 	}
@@ -205,8 +207,9 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 					     netlogon, netlogon, &response);
 		break;
 	case LOGON_SAM_LOGON_REQUEST:
-		status = nbtd_netlogon_samlogon(iface->nbtsrv, packet, src,
-						netlogon, netlogon, &response);
+		status = nbtd_netlogon_samlogon(
+			iface->nbtsrv, &packet->data.msg.dest_name, src,
+			netlogon, netlogon, &response);
 		break;
 	default:
 		DEBUG(2,("unknown netlogon op %d from %s:%d\n", 
-- 
2.11.0


From 84a0b3f7ac6a358358988f62da076bf9a1851a54 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 20:04:23 +0100
Subject: [PATCH 14/17] nbt_server: Avoid an "else" branch

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index d37f914fa68..4753ca88a3d 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -114,7 +114,7 @@ static NTSTATUS nbtd_netlogon_samlogon(
 	struct nbt_netlogon_response **presponse)
 {
 	struct ldb_context *samctx;
-	struct dom_sid *sid;
+	struct dom_sid *sid = NULL;
 	struct nbt_netlogon_response *response;
 	NTSTATUS status;
 
@@ -126,10 +126,8 @@ static NTSTATUS nbtd_netlogon_samlogon(
 
 	samctx = nbtsrv->sam_ctx;
 
-	if (netlogon->req.logon.sid_size) {
+	if (netlogon->req.logon.sid_size != 0) {
 		sid = &netlogon->req.logon.sid;
-	} else {
-		sid = NULL;
 	}
 
 	response = talloc_zero(mem_ctx, struct nbt_netlogon_response);
-- 
2.11.0


From 66fafb271050e25d9a99f4b859653ddadc9a633d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 20:06:36 +0100
Subject: [PATCH 15/17] nbt_server: Avoid a talloc call

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 4753ca88a3d..3a7b1d0e4ad 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -143,9 +143,11 @@ static NTSTATUS nbtd_netlogon_samlogon(
 		netlogon->req.logon.nt_version, nbtsrv->task->lp_ctx,
 		&response->data.samlogon, false);
 	if (!NT_STATUS_IS_OK(status)) {
+		char buf[DOM_SID_STR_BUFLEN];
+		dom_sid_string_buf(sid, buf, sizeof(buf));
+
 		DBG_NOTICE("NBT netlogon query failed domain=%s sid=%s "
-			   "version=%d - %s\n", dst_name->name,
-			   dom_sid_string(response, sid),
+			   "version=%d - %s\n", dst_name->name, buf,
 			   netlogon->req.logon.nt_version, nt_errstr(status));
 		TALLOC_FREE(response);
 		return status;
-- 
2.11.0


From dd01e62c641dcb378dd34cdfe9042595d4507cf2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 20:09:03 +0100
Subject: [PATCH 16/17] nbt_server: nbtd_netlogon_getdc doesn't need "src"

---
 source4/nbt_server/dgram/netlogon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 3a7b1d0e4ad..7698acafb3c 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -38,7 +38,6 @@
  */
 static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 				    struct nbt_name *dst_name,
-				    const struct socket_address *src,
 				    struct nbt_netlogon_packet *netlogon,
 				    TALLOC_CTX *mem_ctx,
 				    struct nbt_netlogon_response **presponse)
@@ -203,7 +202,7 @@ void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
 	switch (netlogon->command) {
 	case LOGON_PRIMARY_QUERY:
 		status = nbtd_netlogon_getdc(iface->nbtsrv,
-					     &packet->data.msg.dest_name, src,
+					     &packet->data.msg.dest_name,
 					     netlogon, netlogon, &response);
 		break;
 	case LOGON_SAM_LOGON_REQUEST:
-- 
2.11.0


From 953a209960bb23e2b546edacb5f403714633548e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 17 Feb 2018 21:02:41 +0100
Subject: [PATCH 17/17] nbt_server: Factor out packet generation for netlogon
 reply

This also fixes an inaccuracy (or even a bug?): The previous code pulled
the reply mailslot always through req.logon.mailslot_name, which is the
union for LOGON_SAM_LOGON_REQUESTs. The LOGON_PRIMARY_QUERY must be
referenced by req.pdc.mailslot_name. It might have worked by chance, but
this should be more correct.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 156 +++++++++++++++++++++++-------------
 1 file changed, 102 insertions(+), 54 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 7698acafb3c..1fee1d8cc1e 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -40,11 +40,13 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 				    struct nbt_name *dst_name,
 				    struct nbt_netlogon_packet *netlogon,
 				    TALLOC_CTX *mem_ctx,
-				    struct nbt_netlogon_response **presponse)
+				    struct nbt_netlogon_response **presponse,
+				    char **preply_mailslot)
 {
 	struct nbt_netlogon_response_from_pdc *pdc;
 	struct ldb_context *samctx;
-	struct nbt_netlogon_response *response;
+	struct nbt_netlogon_response *response = NULL;
+	char *reply_mailslot = NULL;
 
 	/* only answer getdc requests on the PDC or LOGON names */
 	if ((dst_name->type != NBT_NAME_PDC) &&
@@ -67,10 +69,16 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 		return NT_STATUS_NOT_SUPPORTED;
 	}
 
+	reply_mailslot = talloc_strdup(
+		mem_ctx, netlogon->req.pdc.mailslot_name);
+	if (reply_mailslot == NULL) {
+		goto nomem;
+	}
+
 	/* setup a GETDC reply */
 	response = talloc_zero(mem_ctx, struct nbt_netlogon_response);
 	if (response == NULL) {
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 	response->response_type = NETLOGON_GET_PDC;
 	pdc = &response->data.get_pdc;
@@ -80,8 +88,7 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 	pdc->pdc_name = talloc_strdup(
 		response, lpcfg_netbios_name(nbtsrv->task->lp_ctx));
 	if (pdc->pdc_name == NULL) {
-		TALLOC_FREE(response);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	pdc->unicode_pdc_name = pdc->pdc_name;
@@ -89,8 +96,7 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 	pdc->domain_name = talloc_strdup(
 		response, lpcfg_workgroup(nbtsrv->task->lp_ctx));
 	if (pdc->domain_name == NULL) {
-		TALLOC_FREE(response);
-		return NT_STATUS_NO_MEMORY;
+		goto nomem;
 	}
 
 	pdc->nt_version       = 1;
@@ -98,7 +104,13 @@ static NTSTATUS nbtd_netlogon_getdc(struct nbtd_server *nbtsrv,
 	pdc->lm20_token       = 0xFFFF;
 
 	*presponse = response;
+	*preply_mailslot = reply_mailslot;
 	return NT_STATUS_OK;
+
+nomem:
+	TALLOC_FREE(response);
+	TALLOC_FREE(reply_mailslot);
+	return NT_STATUS_NO_MEMORY;
 }
 
 /*
@@ -110,11 +122,13 @@ static NTSTATUS nbtd_netlogon_samlogon(
 	const struct socket_address *src,
 	struct nbt_netlogon_packet *netlogon,
 	TALLOC_CTX *mem_ctx,
-	struct nbt_netlogon_response **presponse)
+	struct nbt_netlogon_response **presponse,
+	char **preply_mailslot)
 {
 	struct ldb_context *samctx;
 	struct dom_sid *sid = NULL;
-	struct nbt_netlogon_response *response;
+	struct nbt_netlogon_response *response = NULL;
+	char *reply_mailslot = NULL;
 	NTSTATUS status;
 
 	/* only answer getdc requests on the PDC or LOGON names */
@@ -129,8 +143,15 @@ static NTSTATUS nbtd_netlogon_samlogon(
 		sid = &netlogon->req.logon.sid;
 	}
 
+	reply_mailslot = talloc_strdup(
+		mem_ctx, netlogon->req.logon.mailslot_name);
+	if (reply_mailslot == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
 	response = talloc_zero(mem_ctx, struct nbt_netlogon_response);
 	if (response == NULL) {
+		TALLOC_FREE(reply_mailslot);
 		return NT_STATUS_NO_MEMORY;
 	}
 	response->response_type = NETLOGON_SAMLOGON;
@@ -148,89 +169,116 @@ static NTSTATUS nbtd_netlogon_samlogon(
 		DBG_NOTICE("NBT netlogon query failed domain=%s sid=%s "
 			   "version=%d - %s\n", dst_name->name, buf,
 			   netlogon->req.logon.nt_version, nt_errstr(status));
+		TALLOC_FREE(reply_mailslot);
 		TALLOC_FREE(response);
 		return status;
 	}
 
 	*presponse = response;
+	*preply_mailslot = reply_mailslot;
 	return NT_STATUS_OK;
 }
 
-/*
-  handle incoming netlogon mailslot requests
-*/
-void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot, 
-				    struct nbt_dgram_packet *packet, 
-				    struct socket_address *src)
+static NTSTATUS nbtd_mailslot_netlogon_reply(
+	struct nbtd_interface *iface,
+	struct nbt_dgram_packet *packet,
+	struct socket_address *src,
+	TALLOC_CTX *mem_ctx,
+	struct nbt_netlogon_response **presponse,
+	char **preply_mailslot)
 {
-	NTSTATUS status = NT_STATUS_NO_MEMORY;
-	struct nbtd_interface *iface = 
-		talloc_get_type(dgmslot->private_data, struct nbtd_interface);
-	struct nbt_netlogon_packet *netlogon = NULL;
-	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(
-		iface, src->addr, false);
+	struct nbt_netlogon_packet *netlogon;
+	struct nbt_name *dst_name = &packet->data.msg.dest_name;
+	struct nbt_netlogon_response *response = NULL;
 	struct nbtd_iface_name *iname;
-	struct nbt_name *name = &packet->data.msg.dest_name;
-	struct nbt_netlogon_response *response;
-
-	if (reply_iface->ip_address == NULL) {
-		DBG_WARNING("Could not obtain own IP address for datagram "
-			    "socket\n");
-		goto failed;
-	}
-
-	netlogon = talloc(dgmslot, struct nbt_netlogon_packet);
-	if (netlogon == NULL) {
-		goto failed;
-	}
+	char *reply_mailslot = NULL;
+	NTSTATUS status;
 
 	/*
 	  see if the we are listening on the destination netbios name
 	*/
-	iname = nbtd_find_iname(iface, name, 0);
+	iname = nbtd_find_iname(iface, dst_name, 0);
 	if (iname == NULL) {
-		status = NT_STATUS_BAD_NETWORK_NAME;
-		goto failed;
+		return NT_STATUS_BAD_NETWORK_NAME;
+	}
+
+	netlogon = talloc(mem_ctx, struct nbt_netlogon_packet);
+	if (netlogon == NULL) {
+		return NT_STATUS_NO_MEMORY;
 	}
 
-	DEBUG(5,("netlogon request to %s from %s:%d\n",
-		 nbt_name_string(netlogon, name), src->addr, src->port));
 	status = dgram_mailslot_netlogon_parse_request(netlogon, packet,
 						       netlogon);
-	if (!NT_STATUS_IS_OK(status)) goto failed;
+	if (!NT_STATUS_IS_OK(status)) {
+		goto failed;
+	}
 
 	switch (netlogon->command) {
 	case LOGON_PRIMARY_QUERY:
-		status = nbtd_netlogon_getdc(iface->nbtsrv,
-					     &packet->data.msg.dest_name,
-					     netlogon, netlogon, &response);
+		status = nbtd_netlogon_getdc(
+			iface->nbtsrv, &packet->data.msg.dest_name,
+			netlogon, mem_ctx, &response, &reply_mailslot);
 		break;
 	case LOGON_SAM_LOGON_REQUEST:
 		status = nbtd_netlogon_samlogon(
 			iface->nbtsrv, &packet->data.msg.dest_name, src,
-			netlogon, netlogon, &response);
+			netlogon, mem_ctx, &response, &reply_mailslot);
 		break;
 	default:
-		DEBUG(2,("unknown netlogon op %d from %s:%d\n", 
+		DEBUG(2,("unknown netlogon op %d from %s:%d\n",
 			 netlogon->command, src->addr, src->port));
 		NDR_PRINT_DEBUG(nbt_netlogon_packet, netlogon);
 		status = NT_STATUS_NOT_SUPPORTED;
 		break;
 	}
 
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_DEBUG("Calculating reply failed: %s\n",
+			  nt_errstr(status));
+		goto failed;
+	}
+
+	*presponse = response;
+	*preply_mailslot = reply_mailslot;
+	return NT_STATUS_OK;
+
+failed:
+	TALLOC_FREE(reply_mailslot);
+	TALLOC_FREE(netlogon);
+	return status;
+}
+
+/*
+  handle incoming netlogon mailslot requests
+*/
+void nbtd_mailslot_netlogon_handler(struct dgram_mailslot_handler *dgmslot,
+				    struct nbt_dgram_packet *packet,
+				    struct socket_address *src)
+{
+	NTSTATUS status = NT_STATUS_NO_MEMORY;
+	struct nbtd_interface *iface =
+		talloc_get_type(dgmslot->private_data, struct nbtd_interface);
+	struct nbtd_interface *reply_iface = nbtd_find_reply_iface(
+		iface, src->addr, false);
+	struct nbt_netlogon_response *response = NULL;
+	char *reply_mailslot = NULL;
+
+	if (reply_iface->ip_address == NULL) {
+		DBG_WARNING("Could not obtain own IP address for datagram "
+			    "socket\n");
+		return;
+	}
+
+	status = nbtd_mailslot_netlogon_reply(
+		iface, packet, src, dgmslot, &response, &reply_mailslot);
+
 	if (NT_STATUS_IS_OK(status)) {
 		dgram_mailslot_netlogon_reply(
 			reply_iface->dgmsock, packet,
 			lpcfg_netbios_name(iface->nbtsrv->task->lp_ctx),
-			netlogon->req.logon.mailslot_name, response);
+			reply_mailslot, response);
 	}
 
-	talloc_free(netlogon);
-	return;
-
-failed:
-	DEBUG(2,("nbtd netlogon handler failed from %s:%d to %s - %s\n",
-		 src->addr, src->port, nbt_name_string(netlogon, name),
-		 nt_errstr(status)));
-	talloc_free(netlogon);
+	TALLOC_FREE(response);
+	TALLOC_FREE(reply_mailslot);
 }
-- 
2.11.0



More information about the samba-technical mailing list