[PATCH] DNS TKEY/TSIG handling, bug 11520

Ralph Boehme slow at samba.org
Wed Jun 15 15:31:50 UTC 2016


On Mon, Jun 13, 2016 at 01:52:32PM -0700, Jeremy Allison wrote:
> What do you think Ralph - am I being too paranoid
> here ?

better paranoid then sorry.

Updated patchset attached, buffer bounds check squashed into the
commit "s4/dns_server: include request MAC in TSIG response MAC
calculation".

Removed Garmin's rb from that commit.

Please review and push if ok. Thanks!

Cheerio!
-slow
-------------- next part --------------
From c3e53cec9cc8070bbc839569f65d04d5f65f774c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 17 May 2016 12:30:46 +0200
Subject: [PATCH 01/12] librpc/ndr: add flag LIBNDR_FLAG_NO_COMPRESSION

This flag can be used to change marshalling behaviour with regard to
compression.

Example: DNS packets make use of so called DNS name compression which
means that for identical strings in a DNS packet, the second string is
replaced with a reference (an offset) to the first.

Setting this flag requests to turns off the marshalling compression.

This will be used in the next commit to prevent name compression in DNS
TSIG records.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 librpc/idl/idl_types.h |  1 +
 librpc/ndr/libndr.h    |  3 +++
 librpc/ndr/ndr_dns.c   | 50 +++++++++++++++++++++++++++-----------------------
 3 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/librpc/idl/idl_types.h b/librpc/idl/idl_types.h
index 838c219..72a5d85 100644
--- a/librpc/idl/idl_types.h
+++ b/librpc/idl/idl_types.h
@@ -40,6 +40,7 @@
 #define NDR_ALIGN2        LIBNDR_FLAG_ALIGN2
 #define NDR_ALIGN4        LIBNDR_FLAG_ALIGN4
 #define NDR_ALIGN8        LIBNDR_FLAG_ALIGN8
+#define NDR_NO_COMP       LIBNDR_FLAG_NO_COMPRESSION
 
 /* this flag is used to force a section of IDL as little endian. It is
    needed for the epmapper IDL, which is defined as always being LE */
diff --git a/librpc/ndr/libndr.h b/librpc/ndr/libndr.h
index 1005437..6034229 100644
--- a/librpc/ndr/libndr.h
+++ b/librpc/ndr/libndr.h
@@ -125,6 +125,9 @@ struct ndr_print {
 #define LIBNDR_FLAG_STR_RAW8		(1<<13)
 #define LIBNDR_STRING_FLAGS		(0x7FFC)
 
+/* Disable string token compression  */
+#define LIBNDR_FLAG_NO_COMPRESSION	(1<<15)
+
 /*
  * don't debug NDR_ERR_BUFSIZE failures,
  * as the available buffer might be incomplete.
diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c
index ab0c83a..fcc1315 100644
--- a/librpc/ndr/ndr_dns.c
+++ b/librpc/ndr/ndr_dns.c
@@ -169,28 +169,30 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr,
 		size_t complen;
 		uint32_t offset;
 
-		/* see if we have pushed the remaining string already,
-		 * if so we use a label pointer to this string
-		 */
-		ndr_err = ndr_token_retrieve_cmp_fn(&ndr->dns_string_list, s,
-						    &offset,
-						    (comparison_fn_t)strcmp,
-						    false);
-		if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-			uint8_t b[2];
-
-			if (offset > 0x3FFF) {
-				return ndr_push_error(ndr, NDR_ERR_STRING,
-						      "offset for dns string " \
-						      "label pointer " \
-						      "%u[%08X] > 0x00003FFF",
-						      offset, offset);
+		if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) {
+			/* see if we have pushed the remaining string already,
+			 * if so we use a label pointer to this string
+			 */
+			ndr_err = ndr_token_retrieve_cmp_fn(&ndr->dns_string_list, s,
+							    &offset,
+							    (comparison_fn_t)strcmp,
+							    false);
+			if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+				uint8_t b[2];
+
+				if (offset > 0x3FFF) {
+					return ndr_push_error(ndr, NDR_ERR_STRING,
+							      "offset for dns string " \
+							      "label pointer " \
+							      "%u[%08X] > 0x00003FFF",
+							      offset, offset);
+				}
+
+				b[0] = 0xC0 | (offset>>8);
+				b[1] = (offset & 0xFF);
+
+				return ndr_push_bytes(ndr, b, 2);
 			}
-
-			b[0] = 0xC0 | (offset>>8);
-			b[1] = (offset & 0xFF);
-
-			return ndr_push_bytes(ndr, b, 2);
 		}
 
 		complen = strcspn(s, ".");
@@ -213,8 +215,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr,
 		/* remember the current componemt + the rest of the string
 		 * so it can be reused later
 		 */
-		NDR_CHECK(ndr_token_store(ndr, &ndr->dns_string_list, s,
-					  ndr->offset));
+		if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) {
+			NDR_CHECK(ndr_token_store(ndr, &ndr->dns_string_list, s,
+						  ndr->offset));
+		}
 
 		/* push just this component into the blob */
 		NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname,
-- 
2.5.0


From a7e1bfdc6f99dc14d72fc3b36f80c8fb1388d6a5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 17 May 2016 14:34:52 +0200
Subject: [PATCH 02/12] librpc/dns: don't compress strings in TKEY and TSIG
 responses

Certain DNS clients fail TSIG record MAC validation if the TSIG record
contains compressed strings.

Windows DNS server behaviour seems to be to not send compressed names in
TKEY and TSIG records.

This patch ensures we conform to this behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 librpc/idl/dns.idl   |  6 +++---
 librpc/ndr/ndr_dns.c | 13 +++++++++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/librpc/idl/dns.idl b/librpc/idl/dns.idl
index 5435fcf..13dd53b 100644
--- a/librpc/idl/dns.idl
+++ b/librpc/idl/dns.idl
@@ -179,7 +179,7 @@ interface dns
 		uint8      option_data[option_length];
 	} dns_opt_record;
 
-	typedef [public] struct {
+	typedef [flag(NDR_NO_COMP),public] struct {
 		dns_string     algorithm;
 		uint32         inception;
 		uint32         expiration;
@@ -191,7 +191,7 @@ interface dns
 		uint8          other_data[other_size];
 	} dns_tkey_record;
 
-	typedef [public] struct {
+	typedef [flag(NDR_NO_COMP),public] struct {
 		dns_string algorithm_name;
 		uint16     time_prefix; /* 0 until February 2106*/
 		uint32     time;
@@ -204,7 +204,7 @@ interface dns
 		uint8      other_data[other_size];
 	} dns_tsig_record;
 
-	typedef [flag(NDR_NOALIGN|NDR_BIG_ENDIAN|NDR_PAHEX),public] struct {
+	typedef [flag(NDR_NO_COMP|NDR_NOALIGN|NDR_BIG_ENDIAN|NDR_PAHEX),public] struct {
 		dns_string	name;
 		dns_qclass 	rr_class;
 		uint32     	ttl;
diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c
index fcc1315..7e6286a 100644
--- a/librpc/ndr/ndr_dns.c
+++ b/librpc/ndr/ndr_dns.c
@@ -268,8 +268,21 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_res_rec(struct ndr_push *ndr,
 	ndr_set_flags(&ndr->flags, LIBNDR_PRINT_ARRAY_HEX |
 				   LIBNDR_FLAG_NOALIGN);
 	if (ndr_flags & NDR_SCALARS) {
+		uint32_t _flags_save_name = ndr->flags;
+
 		NDR_CHECK(ndr_push_align(ndr, 4));
+
+		switch (r->rr_type) {
+		case DNS_QTYPE_TKEY:
+		case DNS_QTYPE_TSIG:
+			ndr_set_flags(&ndr->flags, LIBNDR_FLAG_NO_COMPRESSION);
+			break;
+		default:
+			break;
+		}
 		NDR_CHECK(ndr_push_dns_string(ndr, NDR_SCALARS, r->name));
+		ndr->flags = _flags_save_name;
+
 		NDR_CHECK(ndr_push_dns_qtype(ndr, NDR_SCALARS, r->rr_type));
 		NDR_CHECK(ndr_push_dns_qclass(ndr, NDR_SCALARS, r->rr_class));
 		NDR_CHECK(ndr_push_uint32(ndr, NDR_SCALARS, r->ttl));
-- 
2.5.0


From 5ec1227022fff949709ed531b9336b0f4194896f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 14 May 2016 19:08:51 +0200
Subject: [PATCH 03/12] librpc/dns: remove original_id from dns_fake_tsig_rec

Cf RFC2845, 3.4.2. "TSIG Variables", the request id (original_id) is not
used in the MAC calculation. This also explains the mysterious 2 bytes
padding.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 librpc/idl/dns.idl              | 1 -
 source4/dns_server/dns_crypto.c | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/librpc/idl/dns.idl b/librpc/idl/dns.idl
index 13dd53b..aebb106 100644
--- a/librpc/idl/dns.idl
+++ b/librpc/idl/dns.idl
@@ -212,7 +212,6 @@ interface dns
 		uint16     	time_prefix; /* 0 until February 2106*/
 		uint32     	time;
 		uint16     	fudge;
-		uint16     	original_id;
 		uint16     	error;
 		uint16     	other_size;
 		uint8      	other_data[other_size];
diff --git a/source4/dns_server/dns_crypto.c b/source4/dns_server/dns_crypto.c
index 3f199de..502887f 100644
--- a/source4/dns_server/dns_crypto.c
+++ b/source4/dns_server/dns_crypto.c
@@ -207,9 +207,6 @@ WERROR dns_verify_tsig(struct dns_server *dns,
 		return WERR_NOMEM;
 	}
 
-	/*FIXME: Why is there too much padding? */
-	buffer_len -= 2;
-
 	/* Now we also need to count down the additional record counter */
 	arcount = RSVAL(buffer, 10);
 	RSSVAL(buffer, 10, arcount-1);
-- 
2.5.0


From c18ee49bfc0875a57096032e713bcc86f1a12e78 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 23 May 2016 19:09:05 +0200
Subject: [PATCH 04/12] s4/dns_server: include request MAC in TSIG response MAC
 calculation

According to RFC 2845 "4.2 TSIG on Answers", when the request is signed,
the request MAC must be included in the response MAC calculation.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source4/dns_server/dns_crypto.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/source4/dns_server/dns_crypto.c b/source4/dns_server/dns_crypto.c
index 502887f..a8d76a4 100644
--- a/source4/dns_server/dns_crypto.c
+++ b/source4/dns_server/dns_crypto.c
@@ -243,12 +243,14 @@ WERROR dns_sign_tsig(struct dns_server *dns,
 	time_t current_time = time(NULL);
 	DATA_BLOB packet_blob, tsig_blob, sig;
 	uint8_t *buffer = NULL;
+	uint8_t *p = NULL;
 	size_t buffer_len = 0;
 	struct dns_server_tkey * tkey = NULL;
 	struct dns_res_rec *tsig = talloc_zero(mem_ctx, struct dns_res_rec);
 
 	struct dns_fake_tsig_rec *check_rec = talloc_zero(mem_ctx,
 			struct dns_fake_tsig_rec);
+	size_t mac_size = 0;
 
 	if (tsig == NULL) {
 		return WERR_NOMEM;
@@ -298,15 +300,44 @@ WERROR dns_sign_tsig(struct dns_server *dns,
 		return DNS_ERR(SERVER_FAILURE);
 	}
 
-	buffer_len = packet_blob.length + tsig_blob.length;
+	if (state->tsig != NULL) {
+		mac_size = state->tsig->rdata.tsig_record.mac_size;
+	}
+
+	buffer_len = mac_size;
+
+	buffer_len += packet_blob.length;
+	if (buffer_len < packet_blob.length) {
+		return WERR_INVALID_PARAM;
+	}
+	buffer_len += tsig_blob.length;
+	if (buffer_len < tsig_blob.length) {
+		return WERR_INVALID_PARAM;
+	}
+
 	buffer = talloc_zero_array(mem_ctx, uint8_t, buffer_len);
 	if (buffer == NULL) {
 		return WERR_NOMEM;
 	}
 
-	memcpy(buffer, packet_blob.data, packet_blob.length);
-	memcpy(buffer+packet_blob.length, tsig_blob.data, tsig_blob.length);
+	p = buffer;
+
+	/*
+	 * RFC 2845 "4.2 TSIG on Answers", how to lay out the buffer
+	 * that we're going to sign:
+	 * 1. MAC of request (if present)
+	 * 2. Outgoing packet
+	 * 3. TSIG record
+	 */
+	if (mac_size > 0) {
+		memcpy(p, state->tsig->rdata.tsig_record.mac, mac_size);
+		p += mac_size;
+	}
+
+	memcpy(p, packet_blob.data, packet_blob.length);
+	p += packet_blob.length;
 
+	memcpy(p, tsig_blob.data, tsig_blob.length);
 
 	status = gensec_sign_packet(tkey->gensec, mem_ctx, buffer, buffer_len,
 				    buffer, buffer_len, &sig);
-- 
2.5.0


From b814fc572aec256638111ad3700fde6eec822dda Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 30 May 2016 16:03:33 +0200
Subject: [PATCH 05/12] s4/dns_server: split out function that does the MAC
 computation

Split out function that does the MAC computation from the TSIG record
creating function. This will later simplify the code when creating error
responsed to TSIG requests with bad MACs where we have to add the TSIG
record with an empty MAC.

No functional behaviour change besides hard coding "gss-tsig" algorithm
name: later when sending a TSIG error response for a TKEY request with a
bad keyname, we won't have a tkey to fetch the algorithm name from.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/dns_server/dns_crypto.c | 75 ++++++++++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 27 deletions(-)

diff --git a/source4/dns_server/dns_crypto.c b/source4/dns_server/dns_crypto.c
index a8d76a4..a49b23c 100644
--- a/source4/dns_server/dns_crypto.c
+++ b/source4/dns_server/dns_crypto.c
@@ -231,41 +231,27 @@ WERROR dns_verify_tsig(struct dns_server *dns,
 	return WERR_OK;
 }
 
-WERROR dns_sign_tsig(struct dns_server *dns,
-		     TALLOC_CTX *mem_ctx,
-		     struct dns_request_state *state,
-		     struct dns_name_packet *packet,
-		     uint16_t error)
+static WERROR dns_tsig_compute_mac(TALLOC_CTX *mem_ctx,
+				   struct dns_request_state *state,
+				   struct dns_name_packet *packet,
+				   struct dns_server_tkey *tkey,
+				   time_t current_time,
+				   DATA_BLOB *_psig)
 {
-	WERROR werror;
 	NTSTATUS status;
 	enum ndr_err_code ndr_err;
-	time_t current_time = time(NULL);
 	DATA_BLOB packet_blob, tsig_blob, sig;
 	uint8_t *buffer = NULL;
 	uint8_t *p = NULL;
 	size_t buffer_len = 0;
-	struct dns_server_tkey * tkey = NULL;
-	struct dns_res_rec *tsig = talloc_zero(mem_ctx, struct dns_res_rec);
-
 	struct dns_fake_tsig_rec *check_rec = talloc_zero(mem_ctx,
 			struct dns_fake_tsig_rec);
 	size_t mac_size = 0;
 
-	if (tsig == NULL) {
-		return WERR_NOMEM;
-	}
-
 	if (check_rec == NULL) {
 		return WERR_NOMEM;
 	}
 
-	tkey = dns_find_tkey(dns->tkeys, state->key_name);
-	if (tkey == NULL) {
-		/* FIXME: read up on what to do when we can't find a key */
-		return WERR_OK;
-	}
-
 	/* first build and verify check packet */
 	check_rec->name = talloc_strdup(check_rec, tkey->name);
 	if (check_rec->name == NULL) {
@@ -345,19 +331,54 @@ WERROR dns_sign_tsig(struct dns_server *dns,
 		return ntstatus_to_werror(status);
 	}
 
-	tsig->name = talloc_strdup(tsig, check_rec->name);
+	*_psig = sig;
+	return WERR_OK;
+}
+
+WERROR dns_sign_tsig(struct dns_server *dns,
+		     TALLOC_CTX *mem_ctx,
+		     struct dns_request_state *state,
+		     struct dns_name_packet *packet,
+		     uint16_t error)
+{
+	WERROR werror;
+	time_t current_time = time(NULL);
+	struct dns_server_tkey *tkey = NULL;
+	struct dns_res_rec *tsig = NULL;
+	DATA_BLOB sig = (DATA_BLOB) {
+		.data = NULL,
+		.length = 0
+	};
+
+	tsig = talloc_zero(mem_ctx, struct dns_res_rec);
+	if (tsig == NULL) {
+		return WERR_NOMEM;
+	}
+
+	tkey = dns_find_tkey(dns->tkeys, state->key_name);
+	if (tkey == NULL) {
+		/* FIXME: read up on what to do when we can't find a key */
+		return WERR_OK;
+	}
+
+	werror = dns_tsig_compute_mac(mem_ctx, state, packet,
+				      tkey, current_time, &sig);
+	if (!W_ERROR_IS_OK(werror)) {
+		return werror;
+	}
+
+	tsig->name = talloc_strdup(tsig, state->key_name);
 	if (tsig->name == NULL) {
 		return WERR_NOMEM;
 	}
-	tsig->rr_class = check_rec->rr_class;
+	tsig->rr_class = DNS_QCLASS_ANY;
 	tsig->rr_type = DNS_QTYPE_TSIG;
 	tsig->ttl = 0;
 	tsig->length = UINT16_MAX;
-	tsig->rdata.tsig_record.algorithm_name = talloc_strdup(tsig,
-			check_rec->algorithm_name);
-	tsig->rdata.tsig_record.time_prefix = check_rec->time_prefix;
-	tsig->rdata.tsig_record.time = check_rec->time;
-	tsig->rdata.tsig_record.fudge = check_rec->fudge;
+	tsig->rdata.tsig_record.algorithm_name = talloc_strdup(tsig, "gss-tsig");
+	tsig->rdata.tsig_record.time_prefix = 0;
+	tsig->rdata.tsig_record.time = current_time;
+	tsig->rdata.tsig_record.fudge = 300;
 	tsig->rdata.tsig_record.error = state->tsig_error;
 	tsig->rdata.tsig_record.original_id = packet->id;
 	tsig->rdata.tsig_record.other_size = 0;
-- 
2.5.0


From ef1b334dfefe8a6beff8b96b977c183619447c25 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 30 May 2016 16:56:21 +0200
Subject: [PATCH 06/12] s4/dns_server: not finding the key here is a fatal
 error

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/dns_server/dns_crypto.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/source4/dns_server/dns_crypto.c b/source4/dns_server/dns_crypto.c
index a49b23c..1590814 100644
--- a/source4/dns_server/dns_crypto.c
+++ b/source4/dns_server/dns_crypto.c
@@ -357,8 +357,7 @@ WERROR dns_sign_tsig(struct dns_server *dns,
 
 	tkey = dns_find_tkey(dns->tkeys, state->key_name);
 	if (tkey == NULL) {
-		/* FIXME: read up on what to do when we can't find a key */
-		return WERR_OK;
+		return DNS_ERR(SERVER_FAILURE);
 	}
 
 	werror = dns_tsig_compute_mac(mem_ctx, state, packet,
-- 
2.5.0


From 6dacffd537d8531f27558f985ee3ee877db7d4b1 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 30 May 2016 16:37:32 +0200
Subject: [PATCH 07/12] s4/dns_server: ensure we store the key name in error
 code paths

We need the TKEY name when adding TSIG records to error responses.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/dns_server/dns_crypto.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/source4/dns_server/dns_crypto.c b/source4/dns_server/dns_crypto.c
index 1590814..47b75fc 100644
--- a/source4/dns_server/dns_crypto.c
+++ b/source4/dns_server/dns_crypto.c
@@ -146,10 +146,27 @@ WERROR dns_verify_tsig(struct dns_server *dns,
 
 	tkey = dns_find_tkey(dns->tkeys, state->tsig->name);
 	if (tkey == NULL) {
+		/*
+		 * We must save the name for use in the TSIG error
+		 * response and have no choice here but to save the
+		 * keyname from the TSIG request.
+		 */
+		state->key_name = talloc_strdup(state->mem_ctx,
+						state->tsig->name);
 		state->tsig_error = DNS_RCODE_BADKEY;
 		return DNS_ERR(NOTAUTH);
 	}
 
+	/*
+	 * Remember the keyname that found an existing tkey, used
+	 * later to fetch the key with dns_find_tkey() when signing
+	 * and adding a TSIG record with MAC.
+	 */
+	state->key_name = talloc_strdup(state->mem_ctx, tkey->name);
+	if (state->key_name == NULL) {
+		return WERR_NOMEM;
+	}
+
 	/* FIXME: check TSIG here */
 	if (check_rec == NULL) {
 		return WERR_NOMEM;
@@ -223,10 +240,6 @@ WERROR dns_verify_tsig(struct dns_server *dns,
 	}
 
 	state->authenticated = true;
-	state->key_name = talloc_strdup(state->mem_ctx, tkey->name);
-	if (state->key_name == NULL) {
-		return WERR_NOMEM;
-	}
 
 	return WERR_OK;
 }
-- 
2.5.0


From 2fdcf7930bb52a9afe4210719fcafba75958f35b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 30 May 2016 16:40:45 +0200
Subject: [PATCH 08/12] s4/dns_server: error codes for failing MAC verification
 in TSIG requests

According to RFC 2845 "4.5.3. MAC check and error handling" we must
return NOTAUTH and DNS_RCODE_BADSIG when MAC verification fails.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/dns_server/dns_crypto.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source4/dns_server/dns_crypto.c b/source4/dns_server/dns_crypto.c
index 47b75fc..21d30d7 100644
--- a/source4/dns_server/dns_crypto.c
+++ b/source4/dns_server/dns_crypto.c
@@ -231,7 +231,8 @@ WERROR dns_verify_tsig(struct dns_server *dns,
 	status = gensec_check_packet(tkey->gensec, buffer, buffer_len,
 				    buffer, buffer_len, &sig);
 	if (NT_STATUS_EQUAL(NT_STATUS_ACCESS_DENIED, status)) {
-		return DNS_ERR(BADKEY);
+		state->tsig_error = DNS_RCODE_BADSIG;
+		return DNS_ERR(NOTAUTH);
 	}
 
 	if (!NT_STATUS_IS_OK(status)) {
-- 
2.5.0


From 455ee0e614d3637563b99174dd9790d8a3d793f3 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 30 May 2016 17:25:56 +0200
Subject: [PATCH 09/12] s4/dns_server: don't compute TSIG MAC in TSIG error
 records

See RFC 2845 "4.3. TSIG on TSIG Error returns".

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/dns_server/dns_crypto.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/source4/dns_server/dns_crypto.c b/source4/dns_server/dns_crypto.c
index 21d30d7..6079357 100644
--- a/source4/dns_server/dns_crypto.c
+++ b/source4/dns_server/dns_crypto.c
@@ -357,7 +357,6 @@ WERROR dns_sign_tsig(struct dns_server *dns,
 {
 	WERROR werror;
 	time_t current_time = time(NULL);
-	struct dns_server_tkey *tkey = NULL;
 	struct dns_res_rec *tsig = NULL;
 	DATA_BLOB sig = (DATA_BLOB) {
 		.data = NULL,
@@ -369,15 +368,18 @@ WERROR dns_sign_tsig(struct dns_server *dns,
 		return WERR_NOMEM;
 	}
 
-	tkey = dns_find_tkey(dns->tkeys, state->key_name);
-	if (tkey == NULL) {
-		return DNS_ERR(SERVER_FAILURE);
-	}
+	if (state->tsig_error == DNS_RCODE_OK) {
+		struct dns_server_tkey *tkey = dns_find_tkey(
+			dns->tkeys, state->key_name);
+		if (tkey == NULL) {
+			return DNS_ERR(SERVER_FAILURE);
+		}
 
-	werror = dns_tsig_compute_mac(mem_ctx, state, packet,
-				      tkey, current_time, &sig);
-	if (!W_ERROR_IS_OK(werror)) {
-		return werror;
+		werror = dns_tsig_compute_mac(mem_ctx, state, packet,
+					      tkey, current_time, &sig);
+		if (!W_ERROR_IS_OK(werror)) {
+			return werror;
+		}
 	}
 
 	tsig->name = talloc_strdup(tsig, state->key_name);
@@ -396,9 +398,10 @@ WERROR dns_sign_tsig(struct dns_server *dns,
 	tsig->rdata.tsig_record.original_id = packet->id;
 	tsig->rdata.tsig_record.other_size = 0;
 	tsig->rdata.tsig_record.other_data = NULL;
-	tsig->rdata.tsig_record.mac_size = sig.length;
-	tsig->rdata.tsig_record.mac = talloc_memdup(tsig, sig.data, sig.length);
-
+	if (sig.length > 0) {
+		tsig->rdata.tsig_record.mac_size = sig.length;
+		tsig->rdata.tsig_record.mac = talloc_memdup(tsig, sig.data, sig.length);
+	}
 
 	if (packet->arcount == 0) {
 		packet->additional = talloc_zero(mem_ctx, struct dns_res_rec);
-- 
2.5.0


From cd1de82be2c95161c6792aacb63ac971a1a37c76 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 30 May 2016 16:42:14 +0200
Subject: [PATCH 10/12] s4/dns_server: prepare sending correct error responses
 for dns_verify_tsig() errors

Call dns_verify_tsig() after updating state.flags and assign and use
out_packet for dns_verify_tsig().

We will need the updated flags when sending TSIG error responses when
TSIG request MAC verification fails and dns_verify_tsig() uses the
passed in packet as response, so we have to make sure we copy in_packet
to out_packet before calling out and pass out_packet.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/dns_server/dns_server.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/source4/dns_server/dns_server.c b/source4/dns_server/dns_server.c
index ae7ec7a..c943b7e 100644
--- a/source4/dns_server/dns_server.c
+++ b/source4/dns_server/dns_server.c
@@ -152,14 +152,6 @@ static struct tevent_req *dns_process_send(TALLOC_CTX *mem_ctx,
 		NDR_PRINT_DEBUGC(DBGC_DNS, dns_name_packet, &state->in_packet);
 	}
 
-	ret = dns_verify_tsig(dns, state, &state->state, &state->in_packet, in);
-	if (!W_ERROR_IS_OK(ret)) {
-		DEBUG(1, ("Failed to verify TSIG!\n"));
-		state->dns_err = werr_to_dns_err(ret);
-		tevent_req_done(req);
-		return tevent_req_post(req, ev);
-	}
-
 	if (state->in_packet.operation & DNS_FLAG_REPLY) {
 		DEBUG(1, ("Won't reply to replies.\n"));
 		tevent_req_werror(req, WERR_INVALID_PARAM);
@@ -176,6 +168,13 @@ static struct tevent_req *dns_process_send(TALLOC_CTX *mem_ctx,
 
 	state->out_packet = state->in_packet;
 
+	ret = dns_verify_tsig(dns, state, &state->state, &state->out_packet, in);
+	if (!W_ERROR_IS_OK(ret)) {
+		state->dns_err = werr_to_dns_err(ret);
+		tevent_req_done(req);
+		return tevent_req_post(req, ev);
+	}
+
 	switch (state->in_packet.operation & DNS_OPCODE) {
 	case DNS_OPCODE_QUERY:
 		subreq = dns_server_process_query_send(
-- 
2.5.0


From 303532e40fae763cfbbe02b06d66846febb3b8e9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 30 May 2016 16:44:00 +0200
Subject: [PATCH 11/12] s4/dns_server: enable sending of TSIG error records

This final patch enables sending TSIG error records by adding
DNS_RCODE_NOTAUTH to the set of error conditions that are allowed to
trigger sending a full generated response.

See RFC 2845 "4.5.1. KEY check and error handling" and "4.5.3. MAC check
and error handling".

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Garming Sam <garming at catalyst.net.nz>
---
 source4/dns_server/dns_server.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source4/dns_server/dns_server.c b/source4/dns_server/dns_server.c
index c943b7e..347cdb7 100644
--- a/source4/dns_server/dns_server.c
+++ b/source4/dns_server/dns_server.c
@@ -235,7 +235,9 @@ static WERROR dns_process_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
 		return ret;
 	}
 	if ((state->dns_err != DNS_RCODE_OK) &&
-	    (state->dns_err != DNS_RCODE_NXDOMAIN)) {
+	    (state->dns_err != DNS_RCODE_NXDOMAIN) &&
+	    (state->dns_err != DNS_RCODE_NOTAUTH))
+	{
 		goto drop;
 	}
 	if (state->dns_err != DNS_RCODE_OK) {
-- 
2.5.0


From ebdfa32fbb668b739ff56803ebd323d7ef2827cb Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 4 May 2016 14:26:16 +0200
Subject: [PATCH 12/12] selftest: add test for DNS updates with TKEY/TSIG

Bug: https://bugzilla.samba.org/show_bug.cgi?id=11520

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 python/samba/tests/dns_tkey.py | 487 +++++++++++++++++++++++++++++++++++++++++
 source4/selftest/tests.py      |   2 +
 2 files changed, 489 insertions(+)
 create mode 100644 python/samba/tests/dns_tkey.py

diff --git a/python/samba/tests/dns_tkey.py b/python/samba/tests/dns_tkey.py
new file mode 100644
index 0000000..e47a647
--- /dev/null
+++ b/python/samba/tests/dns_tkey.py
@@ -0,0 +1,487 @@
+# Unix SMB/CIFS implementation.
+# Copyright (C) Kai Blin  <kai at samba.org> 2011
+# Copyright (C) Ralph Boehme  <slow at samba.org> 2016
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import sys
+import struct
+import random
+import socket
+import optparse
+import uuid
+import time
+import samba.ndr as ndr
+import samba.getopt as options
+from samba import credentials
+from samba.dcerpc import dns, dnsp
+from samba.tests.subunitrun import SubunitOptions, TestProgram
+from samba import gensec, tests
+
+parser = optparse.OptionParser("dns.py <server name> <server ip> [options]")
+sambaopts = options.SambaOptions(parser)
+parser.add_option_group(sambaopts)
+
+# This timeout only has relevance when testing against Windows
+# Format errors tend to return patchy responses, so a timeout is needed.
+parser.add_option("--timeout", type="int", dest="timeout",
+                  help="Specify timeout for DNS requests")
+
+# use command line creds if available
+credopts = options.CredentialsOptions(parser)
+parser.add_option_group(credopts)
+subunitopts = SubunitOptions(parser)
+parser.add_option_group(subunitopts)
+
+opts, args = parser.parse_args()
+timeout = opts.timeout
+
+if len(args) < 2:
+    parser.print_usage()
+    sys.exit(1)
+
+server_name = args[0]
+server_ip = args[1]
+
+
+class DNSTest(tests.TestCase):
+    def setUp(self):
+        super(DNSTest, self).setUp()
+        self.server = server_name
+        self.server_ip = server_ip
+        self.settings = {}
+        self.settings["lp_ctx"] = self.lp_ctx = tests.env_loadparm()
+        self.settings["target_hostname"] = self.server
+
+        self.creds = credentials.Credentials()
+        self.creds.guess(self.lp_ctx)
+        self.creds.set_username(tests.env_get_var_value('USERNAME'))
+        self.creds.set_password(tests.env_get_var_value('PASSWORD'))
+        self.creds.set_kerberos_state(credentials.MUST_USE_KERBEROS)
+        self.newrecname = "tkeytsig.%s" % self.get_dns_domain()
+
+    def errstr(self, errcode):
+        "Return a readable error code"
+        string_codes = [
+            "OK",
+            "FORMERR",
+            "SERVFAIL",
+            "NXDOMAIN",
+            "NOTIMP",
+            "REFUSED",
+            "YXDOMAIN",
+            "YXRRSET",
+            "NXRRSET",
+            "NOTAUTH",
+            "NOTZONE",
+            "0x0B",
+            "0x0C",
+            "0x0D",
+            "0x0E",
+            "0x0F",
+            "BADSIG",
+            "BADKEY"
+        ]
+
+        return string_codes[errcode]
+
+    def assert_rcode_equals(self, rcode, expected):
+        "Helper function to check return code"
+        self.assertEquals(rcode, expected, "Expected RCODE %s, got %s" %
+                          (self.errstr(expected), self.errstr(rcode)))
+
+    def assert_dns_rcode_equals(self, packet, rcode):
+        "Helper function to check return code"
+        p_errcode = packet.operation & 0x000F
+        self.assertEquals(p_errcode, rcode, "Expected RCODE %s, got %s" %
+                          (self.errstr(rcode), self.errstr(p_errcode)))
+
+    def assert_dns_opcode_equals(self, packet, opcode):
+        "Helper function to check opcode"
+        p_opcode = packet.operation & 0x7800
+        self.assertEquals(p_opcode, opcode, "Expected OPCODE %s, got %s" %
+                          (opcode, p_opcode))
+
+    def make_name_packet(self, opcode, qid=None):
+        "Helper creating a dns.name_packet"
+        p = dns.name_packet()
+        if qid is None:
+            p.id = random.randint(0x0, 0xff00)
+        p.operation = opcode
+        p.questions = []
+        p.additional = []
+        return p
+
+    def finish_name_packet(self, packet, questions):
+        "Helper to finalize a dns.name_packet"
+        packet.qdcount = len(questions)
+        packet.questions = questions
+
+    def make_name_question(self, name, qtype, qclass):
+        "Helper creating a dns.name_question"
+        q = dns.name_question()
+        q.name = name
+        q.question_type = qtype
+        q.question_class = qclass
+        return q
+
+    def make_txt_record(self, records):
+        rdata_txt = dns.txt_record()
+        s_list = dnsp.string_list()
+        s_list.count = len(records)
+        s_list.str = records
+        rdata_txt.txt = s_list
+        return rdata_txt
+
+    def get_dns_domain(self):
+        "Helper to get dns domain"
+        return self.creds.get_realm().lower()
+
+    def dns_transaction_udp(self, packet, host,
+                            dump=False, timeout=timeout):
+        "send a DNS query and read the reply"
+        s = None
+        try:
+            send_packet = ndr.ndr_pack(packet)
+            if dump:
+                print self.hexdump(send_packet)
+            s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
+            s.settimeout(timeout)
+            s.connect((host, 53))
+            s.sendall(send_packet, 0)
+            recv_packet = s.recv(2048, 0)
+            if dump:
+                print self.hexdump(recv_packet)
+            response = ndr.ndr_unpack(dns.name_packet, recv_packet)
+            return (response, recv_packet)
+        finally:
+            if s is not None:
+                s.close()
+
+    def dns_transaction_tcp(self, packet, host,
+                            dump=False, timeout=timeout):
+        "send a DNS query and read the reply, also return the raw packet"
+        s = None
+        try:
+            send_packet = ndr.ndr_pack(packet)
+            if dump:
+                print self.hexdump(send_packet)
+            s = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0)
+            s.settimeout(timeout)
+            s.connect((host, 53))
+            tcp_packet = struct.pack('!H', len(send_packet))
+            tcp_packet += send_packet
+            s.sendall(tcp_packet)
+
+            recv_packet = s.recv(0xffff + 2, 0)
+            if dump:
+                print self.hexdump(recv_packet)
+            response = ndr.ndr_unpack(dns.name_packet, recv_packet[2:])
+
+        finally:
+            if s is not None:
+                s.close()
+
+        # unpacking and packing again should produce same bytestream
+        my_packet = ndr.ndr_pack(response)
+        self.assertEquals(my_packet, recv_packet[2:])
+
+        return (response, recv_packet[2:])
+
+    def tkey_trans(self):
+        "Do a TKEY transaction and establish a gensec context"
+
+        self.key_name = "%s.%s" % (uuid.uuid4(), self.get_dns_domain())
+
+        p = self.make_name_packet(dns.DNS_OPCODE_QUERY)
+        q = self.make_name_question(self.key_name,
+                                    dns.DNS_QTYPE_TKEY,
+                                    dns.DNS_QCLASS_IN)
+        questions = []
+        questions.append(q)
+        self.finish_name_packet(p, questions)
+
+        r = dns.res_rec()
+        r.name = self.key_name
+        r.rr_type = dns.DNS_QTYPE_TKEY
+        r.rr_class = dns.DNS_QCLASS_IN
+        r.ttl = 0
+        r.length = 0xffff
+        rdata = dns.tkey_record()
+        rdata.algorithm = "gss-tsig"
+        rdata.inception = int(time.time())
+        rdata.expiration = int(time.time()) + 60*60
+        rdata.mode = dns.DNS_TKEY_MODE_GSSAPI
+        rdata.error = 0
+        rdata.other_size = 0
+
+        self.g = gensec.Security.start_client(self.settings)
+        self.g.set_credentials(self.creds)
+        self.g.set_target_service("dns")
+        self.g.set_target_hostname(self.server)
+        self.g.want_feature(gensec.FEATURE_SIGN)
+        self.g.start_mech_by_name("spnego")
+
+        finished = False
+        client_to_server = ""
+
+        (finished, server_to_client) = self.g.update(client_to_server)
+        self.assertFalse(finished)
+
+        data = [ord(x) for x in list(server_to_client)]
+        rdata.key_data = data
+        rdata.key_size = len(data)
+        r.rdata = rdata
+
+        additional = [r]
+        p.arcount = 1
+        p.additional = additional
+
+        (response, response_packet) = self.dns_transaction_tcp(p, self.server_ip)
+        self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK)
+
+        tkey_record = response.answers[0].rdata
+        data = [chr(x) for x in tkey_record.key_data]
+        server_to_client = ''.join(data)
+        (finished, client_to_server) = self.g.update(server_to_client)
+        self.assertTrue(finished)
+
+        self.verify_packet(response, response_packet)
+
+    def verify_packet(self, response, response_packet, request_mac=""):
+        self.assertEqual(response.additional[0].rr_type, dns.DNS_QTYPE_TSIG)
+
+        tsig_record = response.additional[0].rdata
+        mac = ''.join([chr(x) for x in tsig_record.mac])
+
+        # Cut off tsig record from dns response packet for MAC verification
+        # and reset additional record count.
+        key_name_len = len(self.key_name) + 2
+        tsig_record_len = len(ndr.ndr_pack(tsig_record)) + key_name_len + 10
+
+        response_packet_list = list(response_packet)
+        del response_packet_list[-tsig_record_len:]
+        response_packet_list[11] = chr(0)
+        response_packet_wo_tsig = ''.join(response_packet_list)
+
+        fake_tsig = dns.fake_tsig_rec()
+        fake_tsig.name = self.key_name
+        fake_tsig.rr_class = dns.DNS_QCLASS_ANY
+        fake_tsig.ttl = 0
+        fake_tsig.time_prefix = tsig_record.time_prefix
+        fake_tsig.time = tsig_record.time
+        fake_tsig.algorithm_name = tsig_record.algorithm_name
+        fake_tsig.fudge = tsig_record.fudge
+        fake_tsig.error = 0
+        fake_tsig.other_size = 0
+        fake_tsig_packet = ndr.ndr_pack(fake_tsig)
+
+        data = request_mac + response_packet_wo_tsig + fake_tsig_packet
+        self.g.check_packet(data, data, mac)
+
+    def sign_packet(self, packet, key_name):
+        "Sign a packet, calculate a MAC and add TSIG record"
+        packet_data = ndr.ndr_pack(packet)
+
+        fake_tsig = dns.fake_tsig_rec()
+        fake_tsig.name = key_name
+        fake_tsig.rr_class = dns.DNS_QCLASS_ANY
+        fake_tsig.ttl = 0
+        fake_tsig.time_prefix = 0
+        fake_tsig.time = int(time.time())
+        fake_tsig.algorithm_name = "gss-tsig"
+        fake_tsig.fudge = 300
+        fake_tsig.error = 0
+        fake_tsig.other_size = 0
+        fake_tsig_packet = ndr.ndr_pack(fake_tsig)
+
+        data = packet_data + fake_tsig_packet
+        mac = self.g.sign_packet(data, data)
+        mac_list = [ord(x) for x in list(mac)]
+
+        rdata = dns.tsig_record()
+        rdata.algorithm_name = "gss-tsig"
+        rdata.time_prefix = 0
+        rdata.time = fake_tsig.time
+        rdata.fudge = 300
+        rdata.original_id = packet.id
+        rdata.error = 0
+        rdata.other_size = 0
+        rdata.mac = mac_list
+        rdata.mac_size = len(mac_list)
+
+        r = dns.res_rec()
+        r.name = key_name
+        r.rr_type = dns.DNS_QTYPE_TSIG
+        r.rr_class = dns.DNS_QCLASS_ANY
+        r.ttl = 0
+        r.length = 0xffff
+        r.rdata = rdata
+
+        additional = [r]
+        packet.additional = additional
+        packet.arcount = 1
+
+        return mac
+
+    def bad_sign_packet(self, packet, key_name):
+        '''Add bad signature for a packet by bitflipping
+        the final byte in the MAC'''
+
+        mac_list = [ord(x) for x in list("badmac")]
+
+        rdata = dns.tsig_record()
+        rdata.algorithm_name = "gss-tsig"
+        rdata.time_prefix = 0
+        rdata.time = int(time.time())
+        rdata.fudge = 300
+        rdata.original_id = packet.id
+        rdata.error = 0
+        rdata.other_size = 0
+        rdata.mac = mac_list
+        rdata.mac_size = len(mac_list)
+
+        r = dns.res_rec()
+        r.name = key_name
+        r.rr_type = dns.DNS_QTYPE_TSIG
+        r.rr_class = dns.DNS_QCLASS_ANY
+        r.ttl = 0
+        r.length = 0xffff
+        r.rdata = rdata
+
+        additional = [r]
+        packet.additional = additional
+        packet.arcount = 1
+
+    def search_record(self, name):
+        p = self.make_name_packet(dns.DNS_OPCODE_QUERY)
+        questions = []
+
+        q = self.make_name_question(name, dns.DNS_QTYPE_TXT, dns.DNS_QCLASS_IN)
+        questions.append(q)
+
+        self.finish_name_packet(p, questions)
+        (response, response_packet) = self.dns_transaction_udp(p, self.server_ip)
+        return response.operation & 0x000F
+
+    def make_update_request(self, delete=False):
+        "Create a DNS update request"
+
+        rr_class = dns.DNS_QCLASS_IN
+        ttl = 900
+
+        if delete:
+            rr_class = dns.DNS_QCLASS_NONE
+            ttl = 0
+
+        p = self.make_name_packet(dns.DNS_OPCODE_UPDATE)
+        q = self.make_name_question(self.get_dns_domain(),
+                                    dns.DNS_QTYPE_SOA,
+                                    dns.DNS_QCLASS_IN)
+        questions = []
+        questions.append(q)
+        self.finish_name_packet(p, questions)
+
+        updates = []
+        r = dns.res_rec()
+        r.name = self.newrecname
+        r.rr_type = dns.DNS_QTYPE_TXT
+        r.rr_class = rr_class
+        r.ttl = ttl
+        r.length = 0xffff
+        rdata = self.make_txt_record(['"This is a test"'])
+        r.rdata = rdata
+        updates.append(r)
+        p.nscount = len(updates)
+        p.nsrecs = updates
+
+        return p
+
+
+class TestDNSUpdates(DNSTest):
+    def test_tkey(self):
+        "test DNS TKEY handshake"
+
+        self.tkey_trans()
+
+    def test_update_wo_tsig(self):
+        "test DNS update without TSIG record"
+
+        p = self.make_update_request()
+        (response, response_p) = self.dns_transaction_udp(p, self.server_ip)
+        self.assert_dns_rcode_equals(response, dns.DNS_RCODE_REFUSED)
+
+        rcode = self.search_record(self.newrecname)
+        self.assert_rcode_equals(rcode, dns.DNS_RCODE_NXDOMAIN)
+
+    def test_update_tsig_bad_keyname(self):
+        "test DNS update with a TSIG record with a bad keyname"
+
+        self.tkey_trans()
+
+        p = self.make_update_request()
+        self.sign_packet(p, "badkey")
+        (response, response_p) = self.dns_transaction_udp(p, self.server_ip)
+        self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NOTAUTH)
+        tsig_record = response.additional[0].rdata
+        self.assertEquals(tsig_record.error, dns.DNS_RCODE_BADKEY)
+        self.assertEquals(tsig_record.mac_size, 0)
+
+        rcode = self.search_record(self.newrecname)
+        self.assert_rcode_equals(rcode, dns.DNS_RCODE_NXDOMAIN)
+
+    def test_update_tsig_bad_mac(self):
+        "test DNS update with a TSIG record with a bad MAC"
+
+        self.tkey_trans()
+
+        p = self.make_update_request()
+        self.bad_sign_packet(p, self.key_name)
+        (response, response_p) = self.dns_transaction_udp(p, self.server_ip)
+        self.assert_dns_rcode_equals(response, dns.DNS_RCODE_NOTAUTH)
+        tsig_record = response.additional[0].rdata
+        self.assertEquals(tsig_record.error, dns.DNS_RCODE_BADSIG)
+        self.assertEquals(tsig_record.mac_size, 0)
+
+        rcode = self.search_record(self.newrecname)
+        self.assert_rcode_equals(rcode, dns.DNS_RCODE_NXDOMAIN)
+
+    def test_update_tsig(self):
+        "test DNS update with correct TSIG record"
+
+        self.tkey_trans()
+
+        p = self.make_update_request()
+        mac = self.sign_packet(p, self.key_name)
+        (response, response_p) = self.dns_transaction_udp(p, self.server_ip)
+        self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK)
+        self.verify_packet(response, response_p, mac)
+
+        # Check the record is around
+        rcode = self.search_record(self.newrecname)
+        self.assert_rcode_equals(rcode, dns.DNS_RCODE_OK)
+
+        # Now delete the record
+        p = self.make_update_request(delete=True)
+        mac = self.sign_packet(p, self.key_name)
+        (response, response_p) = self.dns_transaction_udp(p, self.server_ip)
+        self.assert_dns_rcode_equals(response, dns.DNS_RCODE_OK)
+        self.verify_packet(response, response_p, mac)
+
+        # check it's gone
+        rcode = self.search_record(self.newrecname)
+        self.assert_rcode_equals(rcode, dns.DNS_RCODE_NXDOMAIN)
+
+TestProgram(module=__name__, opts=subunitopts)
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 5be840d..8e92da3 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -360,6 +360,8 @@ plantestsuite_loadlist("samba.tests.dns", "fl2003dc:local", [python, os.path.joi
 
 plantestsuite_loadlist("samba.tests.dns_forwarder", "fl2003dc:local", [python, os.path.join(srcdir(), "python/samba/tests/dns_forwarder.py"), '$SERVER', '$SERVER_IP', '$DNS_FORWARDER1', '$DNS_FORWARDER2', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
 
+plantestsuite_loadlist("samba.tests.dns_tkey", "fl2008r2dc", [python, os.path.join(srcdir(), "python/samba/tests/dns_tkey.py"), '$SERVER', '$SERVER_IP', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT'])
+
 for t in smbtorture4_testsuites("dns_internal."):
     plansmbtorture4testsuite(t, "ad_dc_ntvfs:local", '//$SERVER/whavever')
 
-- 
2.5.0



More information about the samba-technical mailing list