Skip to content

Commit

Permalink
Fix Impacket logoff session id check (#291)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jborean93 authored Oct 8, 2024
1 parent a951779 commit 2bed7bc
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions src/smbclient/_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions src/smbprotocol/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down

0 comments on commit 2bed7bc

Please sign in to comment.