libcli/smb/smbXcli: maintain smb2 channel_signing_key separate from the signing_key

The signing_key is fix across all channels and is used for session setups
on a channel binding.

Note:
 - the last session setup response is signed with the new channel signing key.
 - the reauth session setups are signed with the channel signing key.

It's also not needed to remember the main session key.

metze

diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index 3478e884e431574561bace41a54bac058dcb4ca8..8f871c3448fea8bb6ab691a4eeed70df911b62a6 100644 (file)
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -134,9 +134,8 @@ struct smbXcli_session {
                uint64_t session_id;
                uint16_t session_flags;
                DATA_BLOB signing_key;
-               DATA_BLOB session_key;
                bool should_sign;
-               bool channel_setup;
+               DATA_BLOB channel_signing_key;
        } smb2;
 };
 
@@ -2495,7 +2494,7 @@ NTSTATUS smb2cli_req_compound_submit(struct tevent_req **reqs,
                uint16_t charge;
                uint16_t credits;
                uint64_t mid;
-               bool should_sign = false;
+               const DATA_BLOB *signing_key = NULL;
 
                if (!tevent_req_is_in_progress(reqs[i])) {
                        return NT_STATUS_INTERNAL_ERROR;
@@ -2587,16 +2586,42 @@ skip_credits:
                nbt_len += reqlen;
 
                if (state->session) {
-                       should_sign = state->session->smb2.should_sign;
-                       if (state->session->smb2.channel_setup) {
+                       bool should_sign = state->session->smb2.should_sign;
+
+                       if (opcode == SMB2_OP_SESSSETUP &&
+                           state->session->smb2.signing_key.length != 0) {
                                should_sign = true;
                        }
+
+                       /*
+                        * We prefer the channel signing key if it is
+                        * already there.
+                        */
+                       if (should_sign) {
+                               signing_key = &state->session->smb2.channel_signing_key;
+                       }
+
+                       /*
+                        * If it is a channel binding, we already have the main
+                        * signing key and try that one.
+                        */
+                       if (signing_key && signing_key->length == 0) {
+                               signing_key = &state->session->smb2.signing_key;
+                       }
+
+                       /*
+                        * If we do not have any session key yet, we skip the
+                        * signing of SMB2_OP_SESSSETUP requests.
+                        */
+                       if (signing_key && signing_key->length == 0) {
+                               signing_key = NULL;
+                       }
                }
 
-               if (should_sign) {
+               if (signing_key) {
                        NTSTATUS status;
 
-                       status = smb2_signing_sign_pdu(state->session->smb2.signing_key,
+                       status = smb2_signing_sign_pdu(*signing_key,
                                                       &iov[hdr_iov], num_iov - hdr_iov);
                        if (!NT_STATUS_IS_OK(status)) {
                                return status;
@@ -2900,7 +2925,8 @@ static NTSTATUS smb2cli_conn_dispatch_incoming(struct smbXcli_conn *conn,
 
                if (session) {
                        should_sign = session->smb2.should_sign;
-                       if (session->smb2.channel_setup) {
+                       if (opcode == SMB2_OP_SESSSETUP &&
+                           session->smb2.signing_key.length != 0) {
                                should_sign = true;
                        }
                }
@@ -2933,17 +2959,39 @@ static NTSTATUS smb2cli_conn_dispatch_incoming(struct smbXcli_conn *conn,
                        }
 
                        last_session = session;
-                       signing_key = &session->smb2.signing_key;
+                       signing_key = &session->smb2.channel_signing_key;
                }
 
-               if ((opcode == SMB2_OP_SESSSETUP) &&
-                    NT_STATUS_IS_OK(status)) {
+               if (opcode == SMB2_OP_SESSSETUP) {
                        /*
-                        * the caller has to check the signing
-                        * as only the caller knows the correct
-                        * session key
+                        * We prefer the channel signing key, if it is
+                        * already there.
+                        *
+                        * If we do not have a channel signing key yet,
+                        * we try the main signing key, if it is not
+                        * the final response.
                         */
-                       signing_key = NULL;
+                       if (signing_key && signing_key->length == 0 &&
+                           !NT_STATUS_IS_OK(status)) {
+                               signing_key = &session->smb2.signing_key;
+                       }
+
+                       if (signing_key && signing_key->length == 0) {
+                               /*
+                                * If we do not have a session key to
+                                * verify the signature, we defer the
+                                * signing check to the caller.
+                                *
+                                * The caller gets NT_STATUS_OK, it
+                                * has to call
+                                * smb2cli_session_set_session_key()
+                                * or
+                                * smb2cli_session_set_channel_key()
+                                * which will check the signature
+                                * with the channel signing key.
+                                */
+                               signing_key = NULL;
+                       }
                }
 
                if (NT_STATUS_EQUAL(status, NT_STATUS_USER_SESSION_DELETED)) {
@@ -4033,19 +4081,23 @@ void smb2cli_session_set_id_and_flags(struct smbXcli_session *session,
        session->smb2.session_flags = session_flags;
 }
 
-NTSTATUS smb2cli_session_update_session_key(struct smbXcli_session *session,
-                                           const DATA_BLOB session_key,
-                                           const struct iovec *recv_iov)
+NTSTATUS smb2cli_session_set_session_key(struct smbXcli_session *session,
+                                        const DATA_BLOB _session_key,
+                                        const struct iovec *recv_iov)
 {
        struct smbXcli_conn *conn = session->conn;
        uint16_t no_sign_flags;
-       DATA_BLOB signing_key;
+       uint8_t session_key[16];
        NTSTATUS status;
 
        if (conn == NULL) {
                return NT_STATUS_INVALID_PARAMETER_MIX;
        }
 
+       if (session->smb2.signing_key.length != 0) {
+               return NT_STATUS_INVALID_PARAMETER_MIX;
+       }
+
        no_sign_flags = SMB2_SESSION_FLAG_IS_GUEST | SMB2_SESSION_FLAG_IS_NULL;
 
        if (session->smb2.session_flags & no_sign_flags) {
@@ -4053,40 +4105,28 @@ NTSTATUS smb2cli_session_update_session_key(struct smbXcli_session *session,
                return NT_STATUS_OK;
        }
 
-       if (session->smb2.signing_key.length > 0) {
-               signing_key = session->smb2.signing_key;
-       } else {
-               signing_key = session_key;
-       }
-       if (session->smb2.channel_setup) {
-               signing_key = session_key;
-       }
-
-       status = smb2_signing_check_pdu(signing_key, recv_iov, 3);
-       if (!NT_STATUS_IS_OK(status)) {
-               return status;
-       }
-
-       if (!session->smb2.channel_setup) {
-               session->smb2.session_key = data_blob_dup_talloc(session,
-                                                                session_key);
-               if (session->smb2.session_key.data == NULL) {
-                       return NT_STATUS_NO_MEMORY;
-               }
-       }
+       ZERO_STRUCT(session_key);
+       memcpy(session_key, _session_key.data,
+              MIN(_session_key.length, sizeof(session_key)));
 
-       if (session->smb2.channel_setup) {
-               data_blob_free(&session->smb2.signing_key);
-               session->smb2.channel_setup = false;
+       session->smb2.signing_key = data_blob_talloc(session,
+                                                    session_key,
+                                                    sizeof(session_key));
+       ZERO_STRUCT(session_key);
+       if (session->smb2.signing_key.data == NULL) {
+               return NT_STATUS_NO_MEMORY;
        }
 
-       if (session->smb2.signing_key.length > 0) {
-               return NT_STATUS_OK;
+       session->smb2.channel_signing_key = data_blob_dup_talloc(session,
+                                               session->smb2.signing_key);
+       if (session->smb2.channel_signing_key.data == NULL) {
+               return NT_STATUS_NO_MEMORY;
        }
 
-       session->smb2.signing_key = data_blob_dup_talloc(session, signing_key);
-       if (session->smb2.signing_key.data == NULL) {
-               return NT_STATUS_NO_MEMORY;
+       status = smb2_signing_check_pdu(session->smb2.channel_signing_key,
+                                       recv_iov, 3);
+       if (!NT_STATUS_IS_OK(status)) {
+               return status;
        }
 
        session->smb2.should_sign = false;
@@ -4124,12 +4164,6 @@ NTSTATUS smb2cli_session_create_channel(TALLOC_CTX *mem_ctx,
        session2->smb2.session_id = session1->smb2.session_id;
        session2->smb2.session_flags = session1->smb2.session_flags;
 
-       session2->smb2.session_key = data_blob_dup_talloc(session2,
-                                               session1->smb2.session_key);
-       if (session2->smb2.session_key.data == NULL) {
-               return NT_STATUS_NO_MEMORY;
-       }
-
        session2->smb2.signing_key = data_blob_dup_talloc(session2,
                                                session1->smb2.signing_key);
        if (session2->smb2.signing_key.data == NULL) {
@@ -4137,7 +4171,6 @@ NTSTATUS smb2cli_session_create_channel(TALLOC_CTX *mem_ctx,
        }
 
        session2->smb2.should_sign = session1->smb2.should_sign;
-       session2->smb2.channel_setup = true;
 
        talloc_set_destructor(session2, smbXcli_session_destructor);
        DLIST_ADD_END(conn->sessions, session2, struct smbXcli_session *);
@@ -4146,3 +4179,40 @@ NTSTATUS smb2cli_session_create_channel(TALLOC_CTX *mem_ctx,
        *_session2 = session2;
        return NT_STATUS_OK;
 }
+
+NTSTATUS smb2cli_session_set_channel_key(struct smbXcli_session *session,
+                                        const DATA_BLOB _channel_key,
+                                        const struct iovec *recv_iov)
+{
+       struct smbXcli_conn *conn = session->conn;
+       uint8_t channel_key[16];
+       NTSTATUS status;
+
+       if (conn == NULL) {
+               return NT_STATUS_INVALID_PARAMETER_MIX;
+       }
+
+       if (session->smb2.channel_signing_key.length != 0) {
+               return NT_STATUS_INVALID_PARAMETER_MIX;
+       }
+
+       ZERO_STRUCT(channel_key);
+       memcpy(channel_key, _channel_key.data,
+              MIN(_channel_key.length, sizeof(channel_key)));
+
+       session->smb2.channel_signing_key = data_blob_talloc(session,
+                                               channel_key,
+                                               sizeof(channel_key));
+       ZERO_STRUCT(channel_key);
+       if (session->smb2.channel_signing_key.data == NULL) {
+               return NT_STATUS_NO_MEMORY;
+       }
+
+       status = smb2_signing_check_pdu(session->smb2.channel_signing_key,
+                                       recv_iov, 3);
+       if (!NT_STATUS_IS_OK(status)) {
+               return status;
+       }
+
+       return NT_STATUS_OK;
+}

diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
index a2b64b1e6f9f1f4bd1017b677b365d70b7785eed..c919f88b1281adaa069983b6e461e99ec0fd6f56 100644 (file)
--- a/libcli/smb/smbXcli_base.h
+++ b/libcli/smb/smbXcli_base.h
@@ -249,12 +249,15 @@ uint64_t smb2cli_session_current_id(struct smbXcli_session *session);
 void smb2cli_session_set_id_and_flags(struct smbXcli_session *session,
                                      uint64_t session_id,
                                      uint16_t session_flags);
-NTSTATUS smb2cli_session_update_session_key(struct smbXcli_session *session,
-                                           const DATA_BLOB session_key,
-                                           const struct iovec *recv_iov);
+NTSTATUS smb2cli_session_set_session_key(struct smbXcli_session *session,
+                                        const DATA_BLOB session_key,
+                                        const struct iovec *recv_iov);
 NTSTATUS smb2cli_session_create_channel(TALLOC_CTX *mem_ctx,
                                        struct smbXcli_session *session1,
                                        struct smbXcli_conn *conn,
                                        struct smbXcli_session **_session2);
+NTSTATUS smb2cli_session_set_channel_key(struct smbXcli_session *session,
+                                        const DATA_BLOB channel_key,
+                                        const struct iovec *recv_iov);
 
 #endif /* _SMBXCLI_BASE_H_ */

diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index 146fc3d3b47d0726dd97868d78445663009f5e63..79b9496f8bf756511f4c52b31c454c4fd2cbcf35 100644 (file)
--- a/source3/libsmb/cliconnect.c
+++ b/source3/libsmb/cliconnect.c
@@ -1519,7 +1519,7 @@ static void cli_session_setup_kerberos_done(struct tevent_req *subreq)
 
        if (cli_state_protocol(state->cli) >= PROTOCOL_SMB2_02) {
                struct smbXcli_session *session = state->cli->smb2.session;
-               status = smb2cli_session_update_session_key(session,
+               status = smb2cli_session_set_session_key(session,
                                                state->session_key_krb5,
                                                recv_iov);
                if (tevent_req_nterror(req, status)) {
@@ -1710,7 +1710,7 @@ static void cli_session_setup_ntlmssp_done(struct tevent_req *subreq)
 
                if (cli_state_protocol(state->cli) >= PROTOCOL_SMB2_02) {
                        struct smbXcli_session *session = state->cli->smb2.session;
-                       status = smb2cli_session_update_session_key(session,
+                       status = smb2cli_session_set_session_key(session,
                                                state->ntlmssp_state->session_key,
                                                recv_iov);
                        if (tevent_req_nterror(req, status)) {

diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c
index 3cd90bfe811161434a50f5de1cd15e6d636d794a..d75ab32d23751c5d6479514eea338f0000735177 100644 (file)
--- a/source3/torture/test_smb2.c
+++ b/source3/torture/test_smb2.c
@@ -548,11 +548,11 @@ bool run_smb2_session_reconnect(int dummy)
 
        /* now grab the session key and try with signing */
 
-       status = smb2cli_session_update_session_key(cli2->smb2.session,
-                                                   session_key,
-                                                   recv_iov);
+       status = smb2cli_session_set_session_key(cli2->smb2.session,
+                                                session_key,
+                                                recv_iov);
        if (!NT_STATUS_IS_OK(status)) {
-               printf("smb2cli_session_update_session_key %s\n", nt_errstr(status));
+               printf("smb2cli_session_set_session_key %s\n", nt_errstr(status));
                return false;
        }
 
@@ -772,7 +772,7 @@ bool run_smb2_multi_channel(int dummy)
        struct tevent_req *subreq;
        DATA_BLOB in_blob = data_blob_null;
        DATA_BLOB out_blob;
-       DATA_BLOB session_key;
+       DATA_BLOB channel_session_key;
        struct auth_generic_state *auth_generic_state;
        struct iovec *recv_iov;
        const char *hello = "Hello, world\n";
@@ -937,18 +937,18 @@ bool run_smb2_multi_channel(int dummy)
        }
 
        status = gensec_session_key(auth_generic_state->gensec_security, talloc_tos(),
-                                   &session_key);
+                                   &channel_session_key);
        if (!NT_STATUS_IS_OK(status)) {
                printf("gensec_session_key returned %s\n",
                        nt_errstr(status));
                return false;
        }
 
-       status = smb2cli_session_update_session_key(cli2->smb2.session,
-                                                   session_key,
-                                                   recv_iov);
+       status = smb2cli_session_set_channel_key(cli2->smb2.session,
+                                                channel_session_key,
+                                                recv_iov);
        if (!NT_STATUS_IS_OK(status)) {
-               printf("smb2cli_session_update_session_key %s\n", nt_errstr(status));
+               printf("smb2cli_session_set_channel_key %s\n", nt_errstr(status));
                return false;
        }
 

diff --git a/source4/libcli/smb2/session.c b/source4/libcli/smb2/session.c
index e35a4734de0d0e9d1e9b60ec02c1dc70d2006ab8..976880857dac18705092a8fdf84df59d90430d78 100644 (file)
--- a/source4/libcli/smb2/session.c
+++ b/source4/libcli/smb2/session.c
@@ -219,9 +219,9 @@ static void smb2_session_setup_spnego_done(struct tevent_req *subreq)
                        return;
                }
 
-               status = smb2cli_session_update_session_key(session->smbXcli,
-                                                           session->session_key,
-                                                           recv_iov);
+               status = smb2cli_session_set_session_key(session->smbXcli,
+                                                        session->session_key,
+                                                        recv_iov);
                if (tevent_req_nterror(req, status)) {
                        return;
                }