From 1b46a10c1636c293c4f934b65131b1c513d41fc0 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Fri, 7 Jun 2019 19:00:25 +0200 Subject: [PATCH] libcli/smb: only fallback to the global smb2 signing key if we should sign MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Andreas Schneider Signed-off-by: Stefan Metzmacher Reviewed-by: Günther Deschner --- libcli/smb/smbXcli_base.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c index 0296d5b8752..79e6658182e 100644 --- a/libcli/smb/smbXcli_base.c +++ b/libcli/smb/smbXcli_base.c @@ -3282,7 +3282,8 @@ skip_credits: * If it is a channel binding, we already have the main * signing key and try that one. */ - if (!smb2_signing_key_valid(signing_key)) { + if (signing_key != NULL && + !smb2_signing_key_valid(signing_key)) { signing_key = state->session->smb2->signing_key; } @@ -3290,7 +3291,8 @@ skip_credits: * If we do not have any session key yet, we skip the * signing of SMB2_OP_SESSSETUP requests. */ - if (!smb2_signing_key_valid(signing_key)) { + if (signing_key != NULL && + !smb2_signing_key_valid(signing_key)) { signing_key = NULL; } } @@ -3789,12 +3791,14 @@ static NTSTATUS smb2cli_conn_dispatch_incoming(struct smbXcli_conn *conn, * we try the main signing key, if it is not * the final response. */ - if (!smb2_signing_key_valid(signing_key) && + if (signing_key != NULL && + !smb2_signing_key_valid(signing_key) && !NT_STATUS_IS_OK(status)) { signing_key = session->smb2->signing_key; } - if (!smb2_signing_key_valid(signing_key)) { + if (signing_key != NULL && + !smb2_signing_key_valid(signing_key)) { /* * If we do not have a session key to * verify the signature, we defer the -- 2.47.3