Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Impacket logoff session id check #291

Merged
merged 1 commit into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
# 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()

Check warning on line 1366 in src/smbprotocol/connection.py

View check run for this annotation

Codecov / codecov/patch

src/smbprotocol/connection.py#L1366

Added line #L1366 was not covered by tests
session_id = header["session_id"].get_value()
if session_id == 0xFFFFFFFFFFFFFFFF:
if session_id == 0xFFFFFFFFFFFFFFFF or (session_id == 0 and command == Commands.SMB2_LOGOFF):

Check warning on line 1368 in src/smbprotocol/connection.py

View check run for this annotation

Codecov / codecov/patch

src/smbprotocol/connection.py#L1368

Added line #L1368 was not covered by tests
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 @@
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