[SCM] Samba Shared Repository - branch master updated
Stefan Metzmacher
metze at samba.org
Wed Jun 12 13:57:02 UTC 2019
The branch, master has been updated
via b336d09b7b1 libcli/smb: harden smbXcli_session_shallow_copy against nonce reusage
via 317054f6eb7 libcli/smb: s/smbXcli_session_copy/smbXcli_session_shallow_copy
via 4d81e48a171 s4:torture: force signing in the smb2.session.bind1 test
via 1b46a10c163 libcli/smb: only fallback to the global smb2 signing key if we should sign
via 7b1eab10937 libcli/smb: make sure the session->{smb2->,smb2_channel.}signing_key is never NULL!
via 2ad02acf386 Revert "libcli:smb: Fix signing with multichannel"
via 824db296727 Revert "libcli/smb: add missing struct smb2_signing_key allocation in smb2cli_session_set_channel_key()"
from 08750166542 libcli/smb: add missing struct smb2_signing_key allocation in smb2cli_session_set_channel_key()
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit b336d09b7b18370098ee73e63cf794a161e1ecb3
Author: Stefan Metzmacher <metze at samba.org>
Date: Tue Jun 11 17:44:04 2019 +0200
libcli/smb: harden smbXcli_session_shallow_copy against nonce reusage
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
Autobuild-Date(master): Wed Jun 12 13:56:19 UTC 2019 on sn-devel-184
commit 317054f6eb7c485d8a5476df6df7dbc05a51c4a4
Author: Stefan Metzmacher <metze at samba.org>
Date: Tue Jun 11 17:42:38 2019 +0200
libcli/smb: s/smbXcli_session_copy/smbXcli_session_shallow_copy
We should make clear that this is a function for testing only,
with possible strange side effects.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
commit 4d81e48a17138485b71b1e108600f168a0110b96
Author: Stefan Metzmacher <metze at samba.org>
Date: Fri Jun 7 19:38:26 2019 +0200
s4:torture: force signing in the smb2.session.bind1 test
This test is supposed to test which signing keys are used on
each of the channels, so it's important to require signing.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Günther Deschner <gd at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
commit 1b46a10c1636c293c4f934b65131b1c513d41fc0
Author: Andreas Schneider <asn at samba.org>
Date: Fri Jun 7 19:00:25 2019 +0200
libcli/smb: only fallback to the global smb2 signing key if we should sign
We should only sign if we're asked for it. The signing keys are
always generated, so we were always using global signing key
and signed with it when signing was not asked for.
By luck this was the correct signing key for the 1st channel.
But multi channel connections where broken is the server nor the client
require/desire signing. It seems the tests only ever run against
Windows domain controllers, which always require signing.
Note that the following code in smb2cli_req_create() makes
sure that we always sign session binds:
if (cmd == SMB2_OP_SESSSETUP &&
!smb2_signing_key_valid(session->smb2_channel.signing_key) &&
smb2_signing_key_valid(session->smb2->signing_key))
{
/*
* a session bind needs to be signed
*/
state->smb2.should_sign = true;
}
This removed a logic changed introduced in commit
17e22e020fcb84fb9ddda350915369dc9ea28ef1. As
if (!smb2_signing_key_valid(signing_key)) {
is not the same as:
if (signing_key && signing_key->length == 0) {
it's the same as:
if (signing_key == NULL || signing_key->length == 0) {
so we need:
if (signing_key != NULL && !smb2_signing_key_valid(signing_key)) {
Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Günther Deschner <gd at samba.org>
commit 7b1eab10937c0922fe686319a732991240e5c6a4
Author: Stefan Metzmacher <metze at samba.org>
Date: Fri Jun 7 18:58:43 2019 +0200
libcli/smb: make sure the session->{smb2->,smb2_channel.}signing_key is never NULL!
Before commit 17e22e020fcb84fb9ddda350915369dc9ea28ef1 they we not a
pointer and always be present.
We used the local pointer variable 'signing_key = NULL' and logic like
this:
if (state->smb2.should_sign) {
signing_key = state->session->smb2_channel.signing_key;
}
if (signing_key != NULL ...
In order to keep this we need to nake sure
state->session->smb2_channel.signing_key is never NULL!
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
commit 2ad02acf386757b50a957e35bb29a7d34c921e53
Author: Stefan Metzmacher <metze at samba.org>
Date: Tue Jun 11 17:47:33 2019 +0200
Revert "libcli:smb: Fix signing with multichannel"
This reverts commit 1817db965dc0caf55e4308fa4d9203ab4381dc90.
This was pushed to fast, the corrected commit follows.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
commit 824db2967279dd01e7cfd69abf9b848975b5d451
Author: Stefan Metzmacher <metze at samba.org>
Date: Tue Jun 11 17:47:24 2019 +0200
Revert "libcli/smb: add missing struct smb2_signing_key allocation in smb2cli_session_set_channel_key()"
This reverts commit 08750166542f46644038d1ff9d839b270436addf.
This was pushed to fast, the corrected commit follows.
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
-----------------------------------------------------------------------
Summary of changes:
libcli/smb/smbXcli_base.c | 79 +++++++++++++++++++++++++++--------------
libcli/smb/smbXcli_base.h | 2 +-
source4/torture/smb2/compound.c | 6 ++--
source4/torture/smb2/session.c | 10 +++++-
4 files changed, 65 insertions(+), 32 deletions(-)
Changeset truncated at 500 lines:
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index 133b961528b..1af550d9cdd 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -5530,9 +5530,27 @@ struct smbXcli_session *smbXcli_session_create(TALLOC_CTX *mem_ctx,
}
talloc_set_destructor(session, smbXcli_session_destructor);
+ session->smb2->signing_key = talloc_zero(session,
+ struct smb2_signing_key);
+ if (session->smb2->signing_key == NULL) {
+ talloc_free(session);
+ return NULL;
+ }
+ talloc_set_destructor(session->smb2->signing_key,
+ smb2_signing_key_destructor);
+
DLIST_ADD_END(conn->sessions, session);
session->conn = conn;
+ session->smb2_channel.signing_key =
+ talloc_zero(session, struct smb2_signing_key);
+ if (session->smb2_channel.signing_key == NULL) {
+ talloc_free(session);
+ return NULL;
+ }
+ talloc_set_destructor(session->smb2_channel.signing_key,
+ smb2_signing_key_destructor);
+
memcpy(session->smb2_channel.preauth_sha512,
conn->smb2.preauth_sha512,
sizeof(session->smb2_channel.preauth_sha512));
@@ -5540,10 +5558,12 @@ struct smbXcli_session *smbXcli_session_create(TALLOC_CTX *mem_ctx,
return session;
}
-struct smbXcli_session *smbXcli_session_copy(TALLOC_CTX *mem_ctx,
+struct smbXcli_session *smbXcli_session_shallow_copy(TALLOC_CTX *mem_ctx,
struct smbXcli_session *src)
{
struct smbXcli_session *session;
+ struct timespec ts;
+ NTTIME nt;
session = talloc_zero(mem_ctx, struct smbXcli_session);
if (session == NULL) {
@@ -5555,11 +5575,33 @@ struct smbXcli_session *smbXcli_session_copy(TALLOC_CTX *mem_ctx,
return NULL;
}
+ /*
+ * Note we keep a pointer to the session keys of the
+ * main session and rely on the caller to free the
+ * shallow copy first!
+ */
session->conn = src->conn;
*session->smb2 = *src->smb2;
session->smb2_channel = src->smb2_channel;
session->disconnect_expired = src->disconnect_expired;
+ /*
+ * This is only supposed to be called in test code
+ * but we should not reuse nonces!
+ *
+ * Add the current timestamp as NTTIME to nonce_high
+ * and set nonce_low to a value we can recognize in captures.
+ */
+ clock_gettime_mono(&ts);
+ nt = unix_timespec_to_nt_time(ts);
+ nt &= session->smb2->nonce_high_max;
+ if (nt == session->smb2->nonce_high_max || nt < UINT8_MAX) {
+ talloc_free(session);
+ return NULL;
+ }
+ session->smb2->nonce_high += nt;
+ session->smb2->nonce_low = UINT32_MAX;
+
DLIST_ADD_END(src->conn->sessions, session);
talloc_set_destructor(session, smbXcli_session_destructor);
@@ -6042,15 +6084,6 @@ NTSTATUS smb2cli_session_set_session_key(struct smbXcli_session *session,
memcpy(session_key, _session_key.data,
MIN(_session_key.length, sizeof(session_key)));
- session->smb2->signing_key = talloc_zero(session,
- struct smb2_signing_key);
- if (session->smb2->signing_key == NULL) {
- ZERO_STRUCT(session_key);
- return NT_STATUS_NO_MEMORY;
- }
- talloc_set_destructor(session->smb2->signing_key,
- smb2_signing_key_destructor);
-
session->smb2->signing_key->blob =
data_blob_talloc(session->smb2->signing_key,
session_key,
@@ -6121,14 +6154,6 @@ NTSTATUS smb2cli_session_set_session_key(struct smbXcli_session *session,
}
ZERO_STRUCT(session_key);
- session->smb2_channel.signing_key =
- talloc_zero(session, struct smb2_signing_key);
- if (session->smb2_channel.signing_key == NULL) {
- return NT_STATUS_NO_MEMORY;
- }
- talloc_set_destructor(session->smb2_channel.signing_key,
- smb2_signing_key_destructor);
-
session->smb2_channel.signing_key->blob =
data_blob_dup_talloc(session->smb2_channel.signing_key,
session->smb2->signing_key->blob);
@@ -6247,6 +6272,15 @@ NTSTATUS smb2cli_session_create_channel(TALLOC_CTX *mem_ctx,
DLIST_ADD_END(conn->sessions, session2);
session2->conn = conn;
+ session2->smb2_channel.signing_key =
+ talloc_zero(session2, struct smb2_signing_key);
+ if (session2->smb2_channel.signing_key == NULL) {
+ talloc_free(session2);
+ return NT_STATUS_NO_MEMORY;
+ }
+ talloc_set_destructor(session2->smb2_channel.signing_key,
+ smb2_signing_key_destructor);
+
memcpy(session2->smb2_channel.preauth_sha512,
conn->smb2.preauth_sha512,
sizeof(session2->smb2_channel.preauth_sha512));
@@ -6302,15 +6336,6 @@ NTSTATUS smb2cli_session_set_channel_key(struct smbXcli_session *session,
memcpy(channel_key, _channel_key.data,
MIN(_channel_key.length, sizeof(channel_key)));
- session->smb2_channel.signing_key = talloc_zero(session,
- struct smb2_signing_key);
- if (session->smb2_channel.signing_key == NULL) {
- ZERO_STRUCT(channel_key);
- return NT_STATUS_NO_MEMORY;
- }
- talloc_set_destructor(session->smb2_channel.signing_key,
- smb2_signing_key_destructor);
-
session->smb2_channel.signing_key->blob =
data_blob_talloc(session->smb2_channel.signing_key,
channel_key,
diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
index a132fbe95af..2afc7165cd9 100644
--- a/libcli/smb/smbXcli_base.h
+++ b/libcli/smb/smbXcli_base.h
@@ -466,7 +466,7 @@ NTSTATUS smb2cli_validate_negotiate_info_recv(struct tevent_req *req);
struct smbXcli_session *smbXcli_session_create(TALLOC_CTX *mem_ctx,
struct smbXcli_conn *conn);
-struct smbXcli_session *smbXcli_session_copy(TALLOC_CTX *mem_ctx,
+struct smbXcli_session *smbXcli_session_shallow_copy(TALLOC_CTX *mem_ctx,
struct smbXcli_session *src);
bool smbXcli_session_is_guest(struct smbXcli_session *session);
bool smbXcli_session_is_authenticated(struct smbXcli_session *session);
diff --git a/source4/torture/smb2/compound.c b/source4/torture/smb2/compound.c
index a84efc20186..fd657a4a16e 100644
--- a/source4/torture/smb2/compound.c
+++ b/source4/torture/smb2/compound.c
@@ -262,7 +262,7 @@ static bool test_compound_related1(struct torture_context *tctx,
0, /* capabilities */
0 /* maximal_access */);
- tree->session->smbXcli = smbXcli_session_copy(tree->session,
+ tree->session->smbXcli = smbXcli_session_shallow_copy(tree->session,
tree->session->smbXcli);
smb2cli_session_set_id_and_flags(tree->session->smbXcli, UINT64_MAX, 0);
@@ -341,7 +341,7 @@ static bool test_compound_related2(struct torture_context *tctx,
0, /* capabilities */
0 /* maximal_access */);
- tree->session->smbXcli = smbXcli_session_copy(tree->session,
+ tree->session->smbXcli = smbXcli_session_shallow_copy(tree->session,
tree->session->smbXcli);
smb2cli_session_set_id_and_flags(tree->session->smbXcli, UINT64_MAX, 0);
@@ -930,7 +930,7 @@ static bool test_compound_invalid2(struct torture_context *tctx,
0, /* capabilities */
0 /* maximal_access */);
- tree->session->smbXcli = smbXcli_session_copy(tree->session,
+ tree->session->smbXcli = smbXcli_session_shallow_copy(tree->session,
tree->session->smbXcli);
smb2cli_session_set_id_and_flags(tree->session->smbXcli, UINT64_MAX, 0);
diff --git a/source4/torture/smb2/session.c b/source4/torture/smb2/session.c
index d3044e79fb8..07c6faebb15 100644
--- a/source4/torture/smb2/session.c
+++ b/source4/torture/smb2/session.c
@@ -1717,6 +1717,7 @@ bool test_session_bind1(struct torture_context *tctx, struct smb2_tree *tree1)
bool ret = false;
struct smb2_tree *tree2 = NULL;
struct smb2_transport *transport1 = tree1->session->transport;
+ struct smbcli_options options2;
struct smb2_transport *transport2 = NULL;
struct smb2_session *session1_1 = tree1->session;
struct smb2_session *session1_2 = NULL;
@@ -1729,6 +1730,13 @@ bool test_session_bind1(struct torture_context *tctx, struct smb2_tree *tree1)
torture_skip(tctx, "server doesn't support SMB2_CAP_MULTI_CHANNEL\n");
}
+ /*
+ * We always want signing for this test!
+ */
+ smb2cli_tcon_should_sign(tree1->smbXcli, true);
+ options2 = transport1->options;
+ options2.signing = SMB_SIGNING_REQUIRED;
+
/* Add some random component to the file name. */
snprintf(fname, sizeof(fname), "session_bind1_%s.dat",
generate_random_str(tctx, 8));
@@ -1757,7 +1765,7 @@ bool test_session_bind1(struct torture_context *tctx, struct smb2_tree *tree1)
credentials,
&tree2,
tctx->ev,
- &transport1->options,
+ &options2,
lpcfg_socket_options(tctx->lp_ctx),
lpcfg_gensec_settings(tctx, tctx->lp_ctx)
);
--
Samba Shared Repository
More information about the samba-cvs
mailing list