[PATCH] Reuse of nonces patch breaks SMB3.0 connections without encryption.

Jeremy Allison jra at samba.org
Tue Jun 16 16:58:34 MDT 2015


The commit :

-------------------------------------------------------
commit 461c69bd7c52c8b980cf56be2abf9ce7accb6048
Author: Simo Sorce <idra at samba.org>
Date:   Wed May 20 14:01:44 2015 +0200

    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.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=11300
    
    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>
-------------------------------------------------------

that went into master breaks non-encrypted SMB3.0
connections. The problem is the hunk added here:

@@ -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;

The previous code generated the nonce and did not
look at xconn->smb2.server.cipher, meaning in the
non-encrypted case where xconn->smb2.server.cipher == 0
the nonce was ignored.

Now if a client negotiates >= PROTOCOL_SMB2_24
but doesn't negotiate encryption (i.e. xconn->smb2.server.cipher == 0)
we run smack into the default case and return
NT_STATUS_INVALID_PARAMETER and the client
sessionsetup fails.

The only place the nonces are actually used is
in smb2_get_new_nonce(), which is only used in
two places:

1). smbd_smb2_request_reply() when req->do_encryption == true.
2). smbd_smb2_send_break() when session->global->encryption_required == true
or tcon->global->encryption_required == true.

Now req->do_encryption is set to true if
a header of size SMB2_TF_HDR_SIZE is sent, so
this might get set at any time by a client
request, even if the client didn't negotiate
encryption.

So I think the correct fix to protect all
code paths is in the case where xconn->smb2.server.cipher
is unknown (the default: case) is to set x->nonce_high_max = 0,
and then add a check inside smb2_get_new_nonce() to
return NT_STATUS_ENCRYPTION_FAILED if
x->nonce_high_max == 0.

Patch to do this attached. Please check.

FYI. This bug was found by Codenomicon at
the plugfest.

Cheers,

	Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-smbd-Fix-clients-connecting-unencrypted-with-PROTOCO.patch
Type: text/x-diff
Size: 1754 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150616/9dbe1ddc/attachment.patch>


More information about the samba-technical mailing list