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