From 4c7bfdefeed88d95ef34dcdf4feeddc2773852f1 Mon Sep 17 00:00:00 2001
From: Jordan Borean <jborean93@gmail.com>
Date: Mon, 7 Nov 2022 06:04:42 +1000
Subject: [PATCH] Fix negotiate request context padding

When using a negotiate request with a hostname that is a multiple of 8
bytes the code will add 8 extra padding bytes. While this works on some
servers others are stricter and can fail the request. This commit fixes
up the padding to ensure no padding is used if the length of a context
request aligns with the 8 byte boundary it is in.
---
 CHANGELOG.md                  |   1 +
 src/smbprotocol/connection.py |   6 +-
 tests/test_connection.py      | 161 ++++++++++++++++++++++++++++++++++
 3 files changed, 164 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5b6fc046..a24e44a7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@
 * Migrated project layout to a `src` based structure rather than including it in the project root
 * Allow `smbclient.copyfile` to copy files on the same server and not just the same share
 * Improve DFS opens by setting the DFS operation header flag when opening a file on a share marked as a DFS share
+* Fix negotiate request padding when using a hostname with a length of 8 characters
 
 ## 1.9.0 - 2022-02-01
 
diff --git a/src/smbprotocol/connection.py b/src/smbprotocol/connection.py
index 3565d4ac..ed9212a9 100644
--- a/src/smbprotocol/connection.py
+++ b/src/smbprotocol/connection.py
@@ -307,9 +307,7 @@ def _parse_negotiate_context_entry(self, data):
         data_length = struct.unpack("<H", data[2:4])[0]
         negotiate_context = SMB2NegotiateContextRequest()
         negotiate_context.unpack(data[: data_length + 8])
-        padded_size = data_length % 8
-        if padded_size != 0:
-            padded_size = 8 - padded_size
+        padded_size = 8 - (data_length % 8 or 8)
 
         return negotiate_context, data[8 + data_length + padded_size :]
 
@@ -376,7 +374,7 @@ def _data_structure_type(self, structure):
 
     def _padding_size(self, structure):
         data_size = len(structure["data"])
-        return 8 - data_size if data_size <= 8 else 8 - (data_size % 8)
+        return 8 - (data_size % 8 or 8)
 
 
 class SMB2PreauthIntegrityCapabilities(Structure):
diff --git a/tests/test_connection.py b/tests/test_connection.py
index 64943116..38eaf98d 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -250,6 +250,167 @@ def test_parse_message(self):
         assert isinstance(net_name["data"].get_value(), SMB2NetnameNegotiateContextId)
         assert net_name["data"]["net_name"].get_value() == "café"
 
+    def test_create_message_with_context_no_padding(self):
+        message = SMB3NegotiateRequest()
+        message["security_mode"] = SecurityMode.SMB2_NEGOTIATE_SIGNING_REQUIRED
+        message["capabilities"] = 69
+        message["client_guid"] = uuid.UUID(bytes=b"\x33" * 16)
+        message["dialects"] = [Dialects.SMB_3_1_1]
+
+        preauth_req = SMB2NegotiateContextRequest()
+        preauth_req["context_type"] = NegotiateContextType.SMB2_PREAUTH_INTEGRITY_CAPABILITIES
+        preauth = SMB2PreauthIntegrityCapabilities()
+        preauth["hash_algorithms"] = [HashAlgorithms.SHA_512]
+        preauth["salt"] = b"\x11" * 32
+        preauth_req["data"] = preauth
+
+        enc_cap_req = SMB2NegotiateContextRequest()
+        enc_cap_req["context_type"] = NegotiateContextType.SMB2_ENCRYPTION_CAPABILITIES
+        enc_cap = SMB2EncryptionCapabilities()
+        enc_cap["ciphers"] = [Ciphers.AES_128_GCM, Ciphers.AES_128_CCM]
+        enc_cap_req["data"] = enc_cap
+
+        netname_req = SMB2NegotiateContextRequest()
+        netname_req["context_type"] = NegotiateContextType.SMB2_NETNAME_NEGOTIATE_CONTEXT_ID
+        netname = SMB2NetnameNegotiateContextId()
+        netname["net_name"] = "testing1"  # Length is specific to test no padding needed
+        netname_req["data"] = netname
+
+        signing_req = SMB2NegotiateContextRequest()
+        signing_req["context_type"] = NegotiateContextType.SMB2_SIGNING_CAPABILITIES
+        signing = SMB2SigningCapabilities()
+        signing["signing_algorithms"] = [
+            SigningAlgorithms.AES_GMAC,
+            SigningAlgorithms.AES_CMAC,
+            SigningAlgorithms.HMAC_SHA256,
+        ]
+        signing_req["data"] = signing
+
+        message["negotiate_context_list"] = [preauth_req, enc_cap_req, netname_req, signing_req]
+
+        expected = (
+            b"\x24\x00"
+            b"\x01\x00"
+            b"\x02\x00"
+            b"\x00\x00"
+            b"\x45\x00\x00\x00"
+            b"\x33\x33\x33\x33\x33\x33\x33\x33"
+            b"\x33\x33\x33\x33\x33\x33\x33\x33"
+            b"\x68\x00\x00\x00"
+            b"\x04\x00"
+            b"\x00\x00"
+            b"\x11\x03"
+            b"\x00\x00"
+            b"\x01\x00\x26\x00\x00\x00\x00\x00"
+            b"\x01\x00\x20\x00\x01\x00\x11\x11"
+            b"\x11\x11\x11\x11\x11\x11\x11\x11"
+            b"\x11\x11\x11\x11\x11\x11\x11\x11"
+            b"\x11\x11\x11\x11\x11\x11\x11\x11"
+            b"\x11\x11\x11\x11\x11\x11\x00\x00"
+            b"\x02\x00\x06\x00\x00\x00\x00\x00"
+            b"\x02\x00\x02\x00\x01\x00\x00\x00"
+            b"\x05\x00\x10\x00\x00\x00\x00\x00"
+            b"\x74\x00\x65\x00\x73\x00\x74\x00"
+            b"\x69\x00\x6e\x00\x67\x00\x31\x00"
+            b"\x08\x00\x08\x00\x00\x00\x00\x00"
+            b"\x03\x00\x02\x00\x01\x00\x00\x00"
+        )
+        actual = message.pack()
+        assert len(message) == 144
+        assert actual == expected
+
+    def test_parse_message_with_context_no_padding(self):
+        actual = SMB3NegotiateRequest()
+        data = (
+            b"\x24\x00"
+            b"\x01\x00"
+            b"\x02\x00"
+            b"\x00\x00"
+            b"\x45\x00\x00\x00"
+            b"\x33\x33\x33\x33\x33\x33\x33\x33"
+            b"\x33\x33\x33\x33\x33\x33\x33\x33"
+            b"\x68\x00\x00\x00"
+            b"\x04\x00"
+            b"\x00\x00"
+            b"\x11\x03"
+            b"\x00\x00"
+            b"\x01\x00\x26\x00\x00\x00\x00\x00"
+            b"\x01\x00\x20\x00\x01\x00\x11\x11"
+            b"\x11\x11\x11\x11\x11\x11\x11\x11"
+            b"\x11\x11\x11\x11\x11\x11\x11\x11"
+            b"\x11\x11\x11\x11\x11\x11\x11\x11"
+            b"\x11\x11\x11\x11\x11\x11\x00\x00"
+            b"\x02\x00\x06\x00\x00\x00\x00\x00"
+            b"\x02\x00\x02\x00\x01\x00\x00\x00"
+            b"\x05\x00\x10\x00\x00\x00\x00\x00"
+            b"\x74\x00\x65\x00\x73\x00\x74\x00"
+            b"\x69\x00\x6e\x00\x67\x00\x31\x00"
+            b"\x08\x00\x08\x00\x00\x00\x00\x00"
+            b"\x03\x00\x02\x00\x01\x00\x00\x00"
+        )
+        actual.unpack(data)
+        assert len(actual) == 140
+        assert actual["structure_size"].get_value() == 36
+        assert actual["dialect_count"].get_value() == 1
+        assert actual["security_mode"].get_value() == SecurityMode.SMB2_NEGOTIATE_SIGNING_REQUIRED
+        assert actual["reserved"].get_value() == 0
+        assert actual["capabilities"].get_value() == 69
+        assert actual["client_guid"].get_value() == uuid.UUID(bytes=b"\x33" * 16)
+        assert actual["negotiate_context_offset"].get_value() == 104
+        assert actual["negotiate_context_count"].get_value() == 4
+        assert actual["reserved2"].get_value() == 0
+        assert actual["dialects"].get_value() == [
+            Dialects.SMB_3_1_1,
+        ]
+        assert actual["padding"].get_value() == b"\x00\x00"
+
+        assert len(actual["negotiate_context_list"].get_value()) == 4
+
+        preauth = actual["negotiate_context_list"][0]
+        assert isinstance(preauth, SMB2NegotiateContextRequest)
+        assert len(preauth) == 46
+        assert preauth["context_type"].get_value() == NegotiateContextType.SMB2_PREAUTH_INTEGRITY_CAPABILITIES
+        assert preauth["data_length"].get_value() == 38
+        assert preauth["reserved"].get_value() == 0
+        assert isinstance(preauth["data"].get_value(), SMB2PreauthIntegrityCapabilities)
+        assert preauth["data"]["hash_algorithm_count"].get_value() == 1
+        assert preauth["data"]["salt_length"].get_value() == 32
+        assert preauth["data"]["hash_algorithms"].get_value() == [HashAlgorithms.SHA_512]
+        assert preauth["data"]["salt"].get_value() == b"\x11" * 32
+
+        enc_cap = actual["negotiate_context_list"][1]
+        assert isinstance(enc_cap, SMB2NegotiateContextRequest)
+        assert len(enc_cap) == 14
+        assert enc_cap["context_type"].get_value() == NegotiateContextType.SMB2_ENCRYPTION_CAPABILITIES
+        assert enc_cap["data_length"].get_value() == 6
+        assert enc_cap["reserved"].get_value() == 0
+        assert isinstance(enc_cap["data"].get_value(), SMB2EncryptionCapabilities)
+        assert enc_cap["data"]["cipher_count"].get_value() == 2
+        assert enc_cap["data"]["ciphers"].get_value() == [Ciphers.AES_128_GCM, Ciphers.AES_128_CCM]
+
+        netname = actual["negotiate_context_list"][2]
+        assert isinstance(netname, SMB2NegotiateContextRequest)
+        assert len(netname) == 24
+        assert netname["context_type"].get_value() == NegotiateContextType.SMB2_NETNAME_NEGOTIATE_CONTEXT_ID
+        assert netname["data_length"].get_value() == 16
+        assert netname["reserved"].get_value() == 0
+        assert isinstance(netname["data"].get_value(), SMB2NetnameNegotiateContextId)
+        assert netname["data"]["net_name"].get_value() == "testing1"
+
+        signing = actual["negotiate_context_list"][3]
+        assert isinstance(signing, SMB2NegotiateContextRequest)
+        assert len(signing) == 16
+        assert signing["context_type"].get_value() == NegotiateContextType.SMB2_SIGNING_CAPABILITIES
+        assert signing["data_length"].get_value() == 8
+        assert signing["reserved"].get_value() == 0
+        assert isinstance(signing["data"].get_value(), SMB2SigningCapabilities)
+        assert signing["data"]["signing_algorithm_count"].get_value() == 3
+        assert signing["data"]["signing_algorithms"].get_value() == [
+            SigningAlgorithms.AES_GMAC,
+            SigningAlgorithms.AES_CMAC,
+            SigningAlgorithms.HMAC_SHA256,
+        ]
+
 
 class TestSMB2NegotiateContextRequest(object):
     def test_create_message(self):