[PATCH] Small refactoring in nbt_server/

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Apr 16 17:29:58 UTC 2018


Hi!

Review appreciated!

Thanks, Volker

-- 
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 56cb808e737577cba20b30702e353b973fde12c3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Feb 2018 13:30:44 +0100
Subject: [PATCH 1/3] nbt_server: Factor out nbtd_name_query_reply_packet

Separate packet creation from sending out the packet. This way packet
creation can be used elsewhere in the future.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/packet.c | 112 ++++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 34 deletions(-)

diff --git a/source4/nbt_server/packet.c b/source4/nbt_server/packet.c
index 2857f1aa9b4..13954962375 100644
--- a/source4/nbt_server/packet.c
+++ b/source4/nbt_server/packet.c
@@ -94,32 +94,31 @@ bool nbtd_self_packet(struct nbt_name_socket *nbtsock,
 	return false;
 }
 
-
-/*
-  send a name query reply
-*/
-void nbtd_name_query_reply(struct nbt_name_socket *nbtsock, 
-			   struct nbt_name_packet *request_packet, 
-			   struct socket_address *src,
-			   struct nbt_name *name, uint32_t ttl,
-			   uint16_t nb_flags, const char **addresses)
+struct nbt_name_packet *nbtd_name_query_reply_packet(
+	TALLOC_CTX *mem_ctx,
+	uint16_t trn_id,
+	uint32_t ttl,
+	uint16_t nb_flags,
+	const struct nbt_name *name,
+	const char **addresses,
+	size_t num_addresses)
 {
 	struct nbt_name_packet *packet;
-	size_t num_addresses = str_list_length(addresses);
-	struct nbtd_interface *iface = talloc_get_type(nbtsock->incoming.private_data,
-						       struct nbtd_interface);
-	struct nbtd_server *nbtsrv = iface->nbtsrv;
-	int i;
+	size_t i;
+	struct nbt_res_rec *answer;
+	struct nbt_rdata_netbios *rdata;
+	NTSTATUS status;
 
 	if (num_addresses == 0) {
-		DEBUG(3,("No addresses in name query reply - failing\n"));
-		return;
+		return NULL;
 	}
 
-	packet = talloc_zero(nbtsock, struct nbt_name_packet);
-	if (packet == NULL) return;
+	packet = talloc_zero(mem_ctx, struct nbt_name_packet);
+	if (packet == NULL) {
+		return NULL;
+	}
 
-	packet->name_trn_id = request_packet->name_trn_id;
+	packet->name_trn_id = trn_id;
 	packet->ancount = 1;
 	packet->operation =
 		NBT_FLAG_REPLY |
@@ -129,23 +128,69 @@ void nbtd_name_query_reply(struct nbt_name_socket *nbtsock,
 		NBT_FLAG_RECURSION_AVAIL;
 
 	packet->answers = talloc_array(packet, struct nbt_res_rec, 1);
-	if (packet->answers == NULL) goto failed;
+	if (packet->answers == NULL) {
+		goto failed;
+	}
+	answer = packet->answers;
 
-	packet->answers[0].name     = *name;
-	packet->answers[0].rr_type  = NBT_QTYPE_NETBIOS;
-	packet->answers[0].rr_class = NBT_QCLASS_IP;
-	packet->answers[0].ttl      = ttl;
-	packet->answers[0].rdata.netbios.length = num_addresses*6;
-	packet->answers[0].rdata.netbios.addresses = 
-		talloc_array(packet->answers, struct nbt_rdata_address, num_addresses);
-	if (packet->answers[0].rdata.netbios.addresses == NULL) goto failed;
-
-	for (i=0;i<num_addresses;i++) {
-		struct nbt_rdata_address *addr = 
-			&packet->answers[0].rdata.netbios.addresses[i];
+	status = nbt_name_dup(packet->answers, name, &answer->name);
+	if (!NT_STATUS_IS_OK(status)) {
+		goto failed;
+	}
+	answer->rr_type  = NBT_QTYPE_NETBIOS;
+	answer->rr_class = NBT_QCLASS_IP;
+	answer->ttl      = ttl;
+
+	rdata = &answer->rdata.netbios;
+	rdata->length = num_addresses*6;
+	rdata->addresses = talloc_array(
+		packet->answers,
+		struct nbt_rdata_address,
+		num_addresses);
+	if (rdata->addresses == NULL) {
+		goto failed;
+	}
+
+	for (i=0; i<num_addresses; i++) {
+		struct nbt_rdata_address *addr = &rdata->addresses[i];
 		addr->nb_flags = nb_flags;
 		addr->ipaddr = talloc_strdup(packet->answers, addresses[i]);
-		if (addr->ipaddr == NULL) goto failed;
+		if (addr->ipaddr == NULL) {
+			goto failed;
+		}
+	}
+
+	return packet;
+
+failed:
+	TALLOC_FREE(packet);
+	return NULL;
+}
+
+/*
+  send a name query reply
+*/
+void nbtd_name_query_reply(struct nbt_name_socket *nbtsock,
+			   struct nbt_name_packet *request_packet,
+			   struct socket_address *src,
+			   struct nbt_name *name, uint32_t ttl,
+			   uint16_t nb_flags, const char **addresses)
+{
+	struct nbt_name_packet *packet;
+	struct nbtd_interface *iface = talloc_get_type(nbtsock->incoming.private_data,
+						       struct nbtd_interface);
+	struct nbtd_server *nbtsrv = iface->nbtsrv;
+
+	packet = nbtd_name_query_reply_packet(
+		nbtsock,
+		request_packet->name_trn_id,
+		ttl,
+		nb_flags,
+		name,
+		addresses,
+		str_list_length(addresses));
+	if (packet == NULL) {
+		return;
 	}
 
 	DEBUG(7,("Sending name query reply for %s at %s to %s:%d\n", 
@@ -154,7 +199,6 @@ void nbtd_name_query_reply(struct nbt_name_socket *nbtsock,
 	nbtsrv->stats.total_sent++;
 	nbt_name_reply_send(nbtsock, src, packet);
 
-failed:
 	talloc_free(packet);
 }
 
-- 
2.11.0


From 26710f04364e74fef384078f1f028e7cf76ba7b3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 2 Feb 2018 15:03:16 +0100
Subject: [PATCH 2/3] nbt_server: Factor out nbtd_node_status_reply_packet

Separate packet creation from sending out the packet. This way packet
creation can be used elsewhere in the future.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/nodestatus.c | 146 +++++++++++++++++++++++++++-------------
 1 file changed, 101 insertions(+), 45 deletions(-)

diff --git a/source4/nbt_server/nodestatus.c b/source4/nbt_server/nodestatus.c
index f71746ab300..e26cda7c5dd 100644
--- a/source4/nbt_server/nodestatus.c
+++ b/source4/nbt_server/nodestatus.c
@@ -26,6 +26,100 @@
 #include "lib/socket/socket.h"
 #include "librpc/gen_ndr/ndr_nbt.h"
 
+struct nbt_name_packet *nbtd_node_status_reply_packet(
+	TALLOC_CTX *mem_ctx,
+	uint16_t trn_id,
+	const struct nbt_name *name,
+	struct nbtd_interface *iface)
+{
+	struct nbtd_iface_name *iname;
+	struct nbt_name_packet *packet;
+	struct nbt_res_rec *answer;
+	struct nbt_rdata_status *stat;
+	uint32_t num_names;
+	NTSTATUS status;
+
+	num_names = 0;
+	for (iname = iface->names; iname != NULL; iname = iname->next) {
+		if ((iname->nb_flags & NBT_NM_ACTIVE) == 0) {
+			continue;
+		}
+		if (strcmp(iname->name.name, "*") == 0) {
+			continue;
+		}
+		num_names += 1;
+	}
+
+	packet = talloc_zero(mem_ctx, struct nbt_name_packet);
+	if (packet == NULL) {
+		return NULL;
+	}
+
+	packet->name_trn_id = trn_id;
+	packet->ancount = 1;
+	packet->operation =
+		NBT_OPCODE_QUERY |
+		NBT_FLAG_REPLY |
+		NBT_FLAG_AUTHORITATIVE;
+
+	packet->answers = talloc_array(packet, struct nbt_res_rec, 1);
+	if (packet->answers == NULL) {
+		goto failed;
+	}
+
+	answer = &packet->answers[0];
+
+	status = nbt_name_dup(packet->answers, name, &answer->name);
+	if (!NT_STATUS_IS_OK(status)) {
+		goto failed;
+	}
+
+	answer->rr_type  = NBT_QTYPE_STATUS;
+	answer->rr_class = NBT_QCLASS_IP;
+	answer->ttl      = 0;
+
+	stat = &packet->answers[0].rdata.status;
+
+	stat->num_names = num_names;
+	stat->names = talloc_zero_array(
+		packet->answers,
+		struct nbt_status_name,
+		num_names);
+	if (stat->names == NULL) {
+		goto failed;
+	}
+
+	num_names = 0;
+	for (iname = iface->names; iname != NULL; iname = iname->next) {
+		struct nbt_status_name *n = &stat->names[num_names];
+
+		if ((iname->nb_flags & NBT_NM_ACTIVE) == 0) {
+			continue;
+		}
+		if (strcmp(iname->name.name, "*") == 0) {
+			continue;
+		}
+
+		n->name = talloc_asprintf(
+			stat->names,
+			"%-15s",
+			iname->name.name);
+		if (n->name == NULL) {
+			goto failed;
+		}
+		n->type = iname->name.type;
+		n->nb_flags = iname->nb_flags;
+
+		num_names += 1;
+	}
+
+	return packet;
+
+failed:
+	TALLOC_FREE(packet);
+	return NULL;
+}
+
 /*
   send a name status reply
 */
@@ -36,53 +130,16 @@ static void nbtd_node_status_reply(struct nbt_name_socket *nbtsock,
 				   struct nbtd_interface *iface)
 {
 	struct nbt_name_packet *packet;
-	uint32_t name_count;
-	struct nbtd_iface_name *iname;
 	struct nbtd_server *nbtsrv = iface->nbtsrv;
-	
-	/* work out how many names to send */
-	name_count = 0;
-	for (iname=iface->names;iname;iname=iname->next) {
-		if ((iname->nb_flags & NBT_NM_ACTIVE) && 
-		    strcmp(iname->name.name, "*") != 0) {
-			name_count++;
-		}
-	}
-
-	packet = talloc_zero(nbtsock, struct nbt_name_packet);
-	if (packet == NULL) return;
 
-	packet->name_trn_id = request_packet->name_trn_id;
-	packet->ancount = 1;
-	packet->operation = NBT_OPCODE_QUERY | NBT_FLAG_REPLY | NBT_FLAG_AUTHORITATIVE;
-
-	packet->answers = talloc_array(packet, struct nbt_res_rec, 1);
-	if (packet->answers == NULL) goto failed;
-
-	packet->answers[0].name     = *name;
-	packet->answers[0].rr_type  = NBT_QTYPE_STATUS;
-	packet->answers[0].rr_class = NBT_QCLASS_IP;
-	packet->answers[0].ttl      = 0;
-	packet->answers[0].rdata.status.num_names = name_count;
-	packet->answers[0].rdata.status.names = talloc_array(packet->answers,
-							     struct nbt_status_name, name_count);
-	if (packet->answers[0].rdata.status.names == NULL) goto failed;
-
-	name_count = 0;
-	for (iname=iface->names;iname;iname=iname->next) {
-		if ((iname->nb_flags & NBT_NM_ACTIVE) && 
-		    strcmp(iname->name.name, "*") != 0) {
-			struct nbt_status_name *n = &packet->answers[0].rdata.status.names[name_count];
-			n->name = talloc_asprintf(packet->answers, "%-15s", iname->name.name);
-			if (n->name == NULL) goto failed;
-			n->type     = iname->name.type;
-			n->nb_flags = iname->nb_flags;
-			name_count++;
-		}
+	packet = nbtd_node_status_reply_packet(
+		nbtsock,
+		request_packet->name_trn_id,
+		name,
+		iface);
+	if (packet == NULL) {
+		return;
 	}
-	/* we deliberately don't fill in the statistics structure as
-	   it could lead to giving attackers too much information */
-	ZERO_STRUCT(packet->answers[0].rdata.status.statistics);
 
 	DEBUG(7,("Sending node status reply for %s to %s:%d\n", 
 		 nbt_name_string(packet, name), src->addr, src->port));
@@ -90,7 +147,6 @@ static void nbtd_node_status_reply(struct nbt_name_socket *nbtsock,
 	nbtsrv->stats.total_sent++;
 	nbt_name_reply_send(nbtsock, src, packet);
 
-failed:
 	talloc_free(packet);
 }
 
-- 
2.11.0


From 0e8fc8230ccf7ae4627f75431c5ec1e057cd6b34 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 16 Apr 2018 16:02:42 +0200
Subject: [PATCH 3/3] nbt_server: Align integer types

sizeof returns size_t

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

diff --git a/source4/nbt_server/dgram/request.c b/source4/nbt_server/dgram/request.c
index 0e9a0459458..908e8d0f35d 100644
--- a/source4/nbt_server/dgram/request.c
+++ b/source4/nbt_server/dgram/request.c
@@ -67,7 +67,7 @@ NTSTATUS nbtd_dgram_setup(struct nbtd_interface *iface, const char *bind_address
 	NTSTATUS status;
 	TALLOC_CTX *tmp_ctx = talloc_new(iface);
 	/* the list of mailslots that we are interested in */
-	int i;
+	size_t i;
 
 	if (!tmp_ctx) {
 		return NT_STATUS_NO_MEMORY;
-- 
2.11.0



More information about the samba-technical mailing list