Properly constrain use of nonces with AEAD crypto

Stefan (metze) Metzmacher metze at samba.org
Thu May 28 11:58:11 MDT 2015


Hi Simo,

> The current AEAD crypto primitives we have access to and use (AES-128-CCM and
> AES-128-GCM) have a very annoying failure mode if a nonce is ever reused with
> the same key.
> The attached patch adds checks to avoid ever wrapping and resusing a nonce.
> 
> It passes a custom autobuild.

What about the attached patchset it also fixes the client side and a few
minor bugs.

Do you think we need to backport similar fixes to 4.1 and 4.2?
If so please create a bug report and add a link to the commit messages
before possibly pushing to master.

Thanks!
metze
-------------- next part --------------
From af2bb2c4796c645d9c273cfeb131663572ac4f4d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 28 May 2015 15:20:54 +0200
Subject: [PATCH 1/2] libcli/smb: In CCM and GCM mode we can't reuse nonces

Reuse of nonces with AES-CCM and AES-GCM leads to catastrophic failure,
so make sure the server drops the connection if that ever happens.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 libcli/smb/smb2_constants.h |  5 ++++
 libcli/smb/smbXcli_base.c   | 71 ++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/libcli/smb/smb2_constants.h b/libcli/smb/smb2_constants.h
index 2bda4e9..f6edf6b 100644
--- a/libcli/smb/smb2_constants.h
+++ b/libcli/smb/smb2_constants.h
@@ -138,6 +138,11 @@
 /* Values for the SMB2_ENCRYPTION_CAPABILITIES Context (>= 0x310) */
 #define SMB2_ENCRYPTION_AES128_CCM         0x0001 /* only in dialect >= 0x224 */
 #define SMB2_ENCRYPTION_AES128_GCM         0x0002 /* only in dialect >= 0x310 */
+#define SMB2_NONCE_HIGH_MAX(nonce_len_bytes) ((uint64_t)(\
+	((nonce_len_bytes) >= 16) ? UINT64_MAX : \
+	((nonce_len_bytes) <= 8) ? 0 : \
+	(((uint64_t)1 << (((nonce_len_bytes) - 8)*8)) - 1) \
+	))
 
 /* SMB2 session (request) flags */
 #define SMB2_SESSION_FLAG_BINDING       0x01
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index 0754203..0281afb 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -34,6 +34,9 @@
 #include "librpc/ndr/libndr.h"
 #include "libcli/smb/smb2_negotiate_context.h"
 #include "lib/crypto/sha512.h"
+#include "lib/crypto/aes.h"
+#include "lib/crypto/aes_ccm_128.h"
+#include "lib/crypto/aes_gcm_128.h"
 
 struct smbXcli_conn;
 struct smbXcli_req;
@@ -147,6 +150,8 @@ struct smb2cli_session {
 	bool should_encrypt;
 	DATA_BLOB encryption_key;
 	DATA_BLOB decryption_key;
+	uint64_t nonce_high_random;
+	uint64_t nonce_high_max;
 	uint64_t nonce_high;
 	uint64_t nonce_low;
 	uint16_t channel_sequence;
@@ -2831,6 +2836,8 @@ NTSTATUS smb2cli_req_compound_submit(struct tevent_req **reqs,
 	int tf_iov = -1;
 	const DATA_BLOB *encryption_key = NULL;
 	uint64_t encryption_session_id = 0;
+	uint64_t nonce_high = UINT64_MAX;
+	uint64_t nonce_low = UINT64_MAX;
 
 	/*
 	 * 1 for the nbt length, optional TRANSFORM
@@ -2881,6 +2888,31 @@ NTSTATUS smb2cli_req_compound_submit(struct tevent_req **reqs,
 
 		encryption_session_id = state->session->smb2->session_id;
 
+		state->session->smb2->nonce_low += 1;
+		if (state->session->smb2->nonce_low == 0) {
+			state->session->smb2->nonce_high += 1;
+			state->session->smb2->nonce_low += 1;
+		}
+
+		/*
+		 * CCM and GCM algorithms must never have their
+		 * nonce wrap, or the security of the whole
+		 * communication and the keys is destroyed.
+		 * We must drop the connection once we have
+		 * transfered too much data.
+		 *
+		 * NOTE: We assume nonces greater than 8 bytes.
+		 */
+		if (state->session->smb2->nonce_high >=
+		    state->session->smb2->nonce_high_max)
+		{
+			return NT_STATUS_ENCRYPTION_FAILED;
+		}
+
+		nonce_high = state->session->smb2->nonce_high_random;
+		nonce_high += state->session->smb2->nonce_high;
+		nonce_low = state->session->smb2->nonce_low;
+
 		tf_iov = num_iov;
 		iov[num_iov].iov_base = state->smb2.transform;
 		iov[num_iov].iov_len  = sizeof(state->smb2.transform);
@@ -2888,18 +2920,12 @@ NTSTATUS smb2cli_req_compound_submit(struct tevent_req **reqs,
 
 		SBVAL(state->smb2.transform, SMB2_TF_PROTOCOL_ID, SMB2_TF_MAGIC);
 		SBVAL(state->smb2.transform, SMB2_TF_NONCE,
-		      state->session->smb2->nonce_low);
+		      nonce_low);
 		SBVAL(state->smb2.transform, SMB2_TF_NONCE+8,
-		      state->session->smb2->nonce_high);
+		      nonce_high);
 		SBVAL(state->smb2.transform, SMB2_TF_SESSION_ID,
 		      encryption_session_id);
 
-		state->session->smb2->nonce_low += 1;
-		if (state->session->smb2->nonce_low == 0) {
-			state->session->smb2->nonce_high += 1;
-			state->session->smb2->nonce_low += 1;
-		}
-
 		nbt_len += SMB2_TF_HDR_SIZE;
 		break;
 	}
@@ -5425,6 +5451,7 @@ NTSTATUS smb2cli_session_set_session_key(struct smbXcli_session *session,
 		struct _derivation decryption;
 		struct _derivation application;
 	} derivation = { };
+	size_t nonce_size = 0;
 
 	if (conn == NULL) {
 		return NT_STATUS_INVALID_PARAMETER_MIX;
@@ -5617,9 +5644,31 @@ NTSTATUS smb2cli_session_set_session_key(struct smbXcli_session *session,
 		session->smb2->should_encrypt = false;
 	}
 
-	generate_random_buffer((uint8_t *)&session->smb2->nonce_high,
-			       sizeof(session->smb2->nonce_high));
-	session->smb2->nonce_low = 1;
+	/*
+	 * CCM and GCM algorithms must never have their
+	 * nonce wrap, or the security of the whole
+	 * communication and the keys is destroyed.
+	 * We must drop the connection once we have
+	 * transfered too much data.
+	 *
+	 * NOTE: We assume nonces greater than 8 bytes.
+	 */
+	generate_random_buffer((uint8_t *)&session->smb2->nonce_high_random,
+			       sizeof(session->smb2->nonce_high_random));
+	switch (conn->smb2.server.cipher) {
+	case SMB2_ENCRYPTION_AES128_CCM:
+		nonce_size = AES_CCM_128_NONCE_SIZE;
+		break;
+	case SMB2_ENCRYPTION_AES128_GCM:
+		nonce_size = AES_GCM_128_IV_SIZE;
+		break;
+	default:
+		nonce_size = 0;
+		break;
+	}
+	session->smb2->nonce_high_max = SMB2_NONCE_HIGH_MAX(nonce_size);
+	session->smb2->nonce_high = 0;
+	session->smb2->nonce_low = 0;
 
 	return NT_STATUS_OK;
 }
-- 
1.9.1


From c1ea2b36207e516264d40806629420e4755f6ec7 Mon Sep 17 00:00:00 2001
From: Simo Sorce <idra at samba.org>
Date: Wed, 20 May 2015 14:01:44 +0200
Subject: [PATCH 2/2] s3:smb2_server: In CCM and GCM mode we can't reuse nonces

Reuse of nonces with AES-CCM and AES-GCM leads to catastrophic failure,
so make sure the server drops the connection if that ever happens.

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

Signed-off-by: Simo Sorce <simo at redhat.com>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/librpc/idl/smbXsrv.idl |  2 ++
 source3/smbd/smb2_server.c     | 76 +++++++++++++++++++++++++++++-------------
 source3/smbd/smb2_sesssetup.c  | 31 +++++++++++++++--
 3 files changed, 83 insertions(+), 26 deletions(-)

diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl
index b3a24a5..4367d72 100644
--- a/source3/librpc/idl/smbXsrv.idl
+++ b/source3/librpc/idl/smbXsrv.idl
@@ -185,6 +185,8 @@ interface smbXsrv
 		[ref] smbXsrv_session_global0		*global;
 		NTSTATUS				status;
 		NTTIME					idle_time;
+		hyper					nonce_high_random;
+		hyper					nonce_high_max;
 		hyper					nonce_high;
 		hyper					nonce_low;
 		[ignore] gensec_security		*gensec;
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 38590a9..a8d54cb 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1458,6 +1458,41 @@ static DATA_BLOB smbd_smb2_signing_key(struct smbXsrv_session *session,
 	return key;
 }
 
+static NTSTATUS smb2_get_new_nonce(struct smbXsrv_session *session,
+				   uint64_t *new_nonce_high,
+				   uint64_t *new_nonce_low)
+{
+	uint64_t nonce_high;
+	uint64_t nonce_low;
+
+	session->nonce_low += 1;
+	if (session->nonce_low == 0) {
+		session->nonce_low += 1;
+		session->nonce_high += 1;
+	}
+
+	/*
+	 * CCM and GCM algorithms must never have their
+	 * nonce wrap, or the security of the whole
+	 * communication and the keys is destroyed.
+	 * We must drop the connection once we have
+	 * transfered too much data.
+	 *
+	 * NOTE: We assume nonces greater than 8 bytes.
+	 */
+	if (session->nonce_high >= session->nonce_high_max) {
+		return NT_STATUS_ENCRYPTION_FAILED;
+	}
+
+	nonce_high = session->nonce_high_random;
+	nonce_high += session->nonce_high;
+	nonce_low = session->nonce_low;
+
+	*new_nonce_high = nonce_high;
+	*new_nonce_low = nonce_low;
+	return NT_STATUS_OK;
+}
+
 static void smbd_smb2_request_pending_timer(struct tevent_context *ev,
 					    struct tevent_timer *te,
 					    struct timeval current_time,
@@ -1524,15 +1559,13 @@ static void smbd_smb2_request_pending_timer(struct tevent_context *ev,
 	dyn = body + 8;
 
 	if (req->do_encryption) {
-		struct smbXsrv_session *x = req->session;
-
-		nonce_high = x->nonce_high;
-		nonce_low = x->nonce_low;
-
-		x->nonce_low += 1;
-		if (x->nonce_low == 0) {
-			x->nonce_low += 1;
-			x->nonce_high += 1;
+		status = smb2_get_new_nonce(req->session,
+					    &nonce_high,
+					    &nonce_low);
+		if (!NT_STATUS_IS_OK(status)) {
+			smbd_server_connection_terminate(xconn,
+							 nt_errstr(status));
+			return;
 		}
 	}
 
@@ -2374,17 +2407,14 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req)
 		DATA_BLOB encryption_key = req->session->global->encryption_key;
 		uint8_t *tf;
 		uint64_t session_id = req->session->global->session_wire_id;
-		struct smbXsrv_session *x = req->session;
 		uint64_t nonce_high;
 		uint64_t nonce_low;
 
-		nonce_high = x->nonce_high;
-		nonce_low = x->nonce_low;
-
-		x->nonce_low += 1;
-		if (x->nonce_low == 0) {
-			x->nonce_low += 1;
-			x->nonce_high += 1;
+		status = smb2_get_new_nonce(req->session,
+					    &nonce_high,
+					    &nonce_low);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 
 		/*
@@ -2829,13 +2859,11 @@ static NTSTATUS smbd_smb2_send_break(struct smbXsrv_connection *xconn,
 	talloc_set_name_const(state, "struct smbd_smb2_send_break_state");
 
 	if (do_encryption) {
-		nonce_high = session->nonce_high;
-		nonce_low = session->nonce_low;
-
-		session->nonce_low += 1;
-		if (session->nonce_low == 0) {
-			session->nonce_low += 1;
-			session->nonce_high += 1;
+		status = smb2_get_new_nonce(session,
+					    &nonce_high,
+					    &nonce_low);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
 		}
 	}
 
diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index 28707ff..3e80da8 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -29,6 +29,9 @@
 #include "../libcli/security/security.h"
 #include "../lib/util/tevent_ntstatus.h"
 #include "lib/crypto/sha512.h"
+#include "lib/crypto/aes.h"
+#include "lib/crypto/aes_ccm_128.h"
+#include "lib/crypto/aes_gcm_128.h"
 
 static struct tevent_req *smbd_smb2_session_setup_wrap_send(TALLOC_CTX *mem_ctx,
 					struct tevent_context *ev,
@@ -335,6 +338,7 @@ static NTSTATUS smbd_smb2_auth_generic_return(struct smbXsrv_session *session,
 
 	if (xconn->protocol >= PROTOCOL_SMB2_24) {
 		struct _derivation *d = &derivation.encryption;
+		size_t nonce_size;
 
 		x->global->encryption_key = data_blob_talloc(x->global,
 							     session_key,
@@ -349,8 +353,31 @@ static NTSTATUS smbd_smb2_auth_generic_return(struct smbXsrv_session *session,
 				    d->context.data, d->context.length,
 				    x->global->encryption_key.data);
 
-		generate_random_buffer((uint8_t *)&x->nonce_high, sizeof(x->nonce_high));
-		x->nonce_low = 1;
+		/*
+		 * CCM and GCM algorithms must never have their
+		 * nonce wrap, or the security of the whole
+		 * communication and the keys is destroyed.
+		 * We must drop the connection once we have
+		 * transfered too much data.
+		 *
+		 * NOTE: We assume nonces greater than 8 bytes.
+		 */
+		generate_random_buffer((uint8_t *)&x->nonce_high_random,
+				       sizeof(x->nonce_high_random));
+		switch (xconn->smb2.server.cipher) {
+		case SMB2_ENCRYPTION_AES128_CCM:
+			nonce_size = AES_CCM_128_NONCE_SIZE;
+			break;
+		case SMB2_ENCRYPTION_AES128_GCM:
+			nonce_size = AES_GCM_128_IV_SIZE;
+			break;
+		default:
+			ZERO_STRUCT(session_key);
+			return NT_STATUS_INVALID_PARAMETER;
+		}
+		x->nonce_high_max = SMB2_NONCE_HIGH_MAX(nonce_size);
+		x->nonce_high = 0;
+		x->nonce_low = 0;
 	}
 
 	x->global->application_key = data_blob_dup_talloc(x->global,
-- 
1.9.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150528/58c247b6/attachment.pgp>


More information about the samba-technical mailing list