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@samba.org>

Signed-off-by: Andreas Schneider <asn@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Günther Deschner <gd@samba.org>

1 files 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