From 2bed7bc87a684251427e4bbba4f0e1d3d5e00665 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Wed, 9 Oct 2024 07:21:08 +1000 Subject: [PATCH] Fix Impacket logoff session id check (#291) Fixes the session lookup logic when the logoff response contains a session id of 0. This occurs when using an Impacket server and will have the client just lookup the session id from the request. Also adjusts the connection cache clearing behaviour to remove the connection from the cache regardless of the result of the disconnect behaviour. --- CHANGELOG.md | 5 +++++ pyproject.toml | 2 +- src/smbclient/_pool.py | 4 ++-- src/smbprotocol/connection.py | 7 +++++-- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f47d52..f9e205a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 1.14.1 - TBD + +* Add workaround for Impackets logoff response not setting the session id for message verification +* Remove connection from global connection cache even if failing to close it + ## 1.14.0 - 2024-08-26 * Dropped support for Python 3.7 diff --git a/pyproject.toml b/pyproject.toml index cc77d32..530c1d8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "setuptools.build_meta" [project] name = "smbprotocol" -version = "1.14.0" +version = "1.14.1" description = "Interact with a server using the SMB 2/3 Protocol" readme = "README.md" requires-python = ">=3.8" diff --git a/src/smbclient/_pool.py b/src/smbclient/_pool.py index 3a7dc50..06f06f1 100644 --- a/src/smbclient/_pool.py +++ b/src/smbclient/_pool.py @@ -440,10 +440,10 @@ def reset_connection_cache(fail_on_error=True, connection_cache=None): if connection_cache is None: connection_cache = _SMB_CONNECTIONS - for name, connection in list(connection_cache.items()): + for name in list(connection_cache.keys()): + connection = connection_cache.pop(name) try: connection.disconnect() - del connection_cache[name] except Exception as e: if fail_on_error: raise diff --git a/src/smbprotocol/connection.py b/src/smbprotocol/connection.py index da41081..953c434 100644 --- a/src/smbprotocol/connection.py +++ b/src/smbprotocol/connection.py @@ -1360,8 +1360,12 @@ def _process_message_thread(self): # unreliable for async responses. Instead get the Session Id from the original request object if # the Session Id is 0xFFFFFFFFFFFFFFFF. # https://social.msdn.microsoft.com/Forums/en-US/a580f7bc-6746-4876-83db-6ac209b202c4/mssmb2-change-notify-response-sessionid?forum=os_fileservices + # Impacket also sets session id to 0 on the logoff response + # so fallback to the request for that one + # https://github.com/jborean93/smbprotocol/issues/289#issuecomment-2396040117. + command = header["command"].get_value() session_id = header["session_id"].get_value() - if session_id == 0xFFFFFFFFFFFFFFFF: + if session_id == 0xFFFFFFFFFFFFFFFF or (session_id == 0 and command == Commands.SMB2_LOGOFF): session_id = request.session_id # No need to waste CPU cycles to verify the signature if we already decrypted the header. @@ -1378,7 +1382,6 @@ def _process_message_thread(self): with self.sequence_lock: self.sequence_window["high"] += credit_response - command = header["command"].get_value() status = header["status"].get_value() if command == Commands.SMB2_NEGOTIATE: self.preauth_integrity_hash_value.append(b_header)