[SMB] Massive Fixes, Features and Refactoring

[covertivy]

Hello!

I did some research regarding some annoying STATUS_SHARING_VIOLATION and STATUS_ACCESS_DENIED errors.
I know for a fact that some files that cannot be read with smbclient can be copied aside with a command / the file explorer.
This means that theoretically it should be possible to do so over SMB!

I opened up Wireshark and played around a bit - it seems this is caused by over-restrictive share access permissions on impacket's side.
I then dug deeper and saw some mismatching flag usage in the SMBv1 implementation of the protocol so I fixed those too.

To sum up, I added the ability to READ FILES WITH OPEN HANDLES WITH (ALMOST) NO RESTRICTION!!!
The only restriction is of course for some system files (eg. SAM, SECURITY, SYSTEM and basically all files that require a ShadowCopy to allow reading them).
This means that files with "weak handles" can be read remotely WITH ABSOLUTELY NO LIMITATION!

The list contains:

• Event Logs - *.evt / *.evtx
• Files open by Local Users
• Browser Files - Chrome Logins & History
• And much much more!

Glad to suffer for all y'alls pleasure!

[yuri2o1o]

Really important fix that makes SMB more intuitive! Please merge!

[laxa]

Hello, interesting PR, however, it does not seem to be working on my side, am I missing something?

$ smbclient.py 'User:Password'@192.168.122.111
# use C$
# cd users/user/appdata/local/microsoft/edge/user data/default/network
# get cookies
[-] SMB SessionError: code: 0xc0000043 - STATUS_SHARING_VIOLATION - A file cannot be opened because the share access flags are incompatible.

[covertivy]

The problem is with your test case. On chromium based browsers the Cookies file is locked with a "strong handle" which restricts read access to other processes, therefore in this case the file cannot be read by us. However, the other sensitive chrome files such as Local State, Login Data, History and others can be read with no issue!

I am happy to help with any other questions you may have 😊

[covertivy]

Hi again, I am changing the title because this is slowly growing into something much bigger.

I will soon add a better description and list all of the current changes I have made to the SMB libraries but the work is far from over 😅

Changes done so far:

• Add better constant definitions for shareAccessMode, createDisposition and desiredAccess in smb.py
• Fix bad shareAccessMode given in all readFile/retr_file implementations.
• Add set_file_info method in SMBv1.
• Add SMB_DATE and SMB_TIME class definitions.
• Replace poorly implemented getSMBDate and getSMBTime methods from smbserver.py to now use the newly implemented class definitions.
• Moved & Renamed FileTime conversion methods from smbserver.py to smb.py for ease of use and access.
• Fix broken SMBSetFileBasicInfo struct.
• Add setInfo method to the SMBConnection wrapper class.
• Add type hinting all over the SMBConnection class in order to allow easier programming for the user.
• Add example script attrib.py to showcase newly implemented setInfo method.

I believe that is all for now, I will make sure to keep you posted!

Have a good one 😉

[anadrianmanrique]

@covertivy thanks for this great new features you are currently working on. In order to better organize the work I suggest to: As a general rule, keep the size of PRs the smallest possible ( you can open/create new PRs as much as necessary ) and scoped as much as possible too. This will help to have them reviewed / tested / merged faster. For this particular case I would split this into changes related to smbclient and new example for attributes handling. Also I'll wait for you to let me know when code is ready to be reviewed/tested Thanks!

[covertivy]

I see, The main issue is that all of my commits are dependent on each-other. If I create a new pull request all of my dependencies will fail and the tests will not pass... What do you think is the best action to take in this stage? as all of my code is based on the changes I previously made.

In any case I will continue to expand this codebase and let you know when I am ready for the changes to be made 😄 Thank you for your help!

[covertivy]

@anadrianmanrique I believe this is my final addition to this PR. I added two example scripts showing off:

• Querying / Modifying remote file attributes.
• Querying / Modifying remote file timestamps.

All using pure SMB 😄

I would love to help with any questions regarding it. Really hope to see it merged soon ❤️

[anadrianmanrique]

@covertivy I'll be testing/reviewing this PR. Can you rebase your branch, in order to resolve conflicts with changes made in the context of#1962? thanks

[covertivy]

@anadrianmanrique Done :)

[anadrianmanrique]

@covertivy I'm currently reviewing this PR. Testing has been great so far. Really nice features has been implemented. I'll post some minor changes for the code review. I'm the meanwhile I would like to request, if possible, to revert changes related to annotations and cosmetic fixes as well ( that doesn't include comments/documentation etc ), as we want to keep the code as consistent as possible. Thanks!

[covertivy]

Pity, I really hoped for better type hinting, but oh well 😅 @anadrianmanrique do you believe the addition of annotations be accepted if all of the codebase was annotated as well? I honestly believe it would drastically improve impacket as a whole... Anyways, I will get to work removing all annotations & type hints from the project.

[anadrianmanrique]

@covertivy yes I understand what you mean, and I agree. But at the moment we want to minimize the amount of changes in the code base as we are approaching some stabilization phase prior to the 0.13 release. We want to have this kind of changes controlled in a very well defined scoped task, and also done in a different phase of the release process. Hope you can understand, and apologies for any inconvenience.

[covertivy]

No worries all good, thanks for the amazing work!

[covertivy]

I believe I corrected all of the undesired annotations & design changes.
Let me know if any more work is required @anadrianmanrique.

[anadrianmanrique]

added some request changes. Additionally, changes in test_smb.py are run in the context of this PR, so they can be uncommented

[nullsection]

So it appears that the cookies file still appears locked. History file works in this branch.

# pwd
/Users/user/AppData/local/Google/Chrome/User Data/Default/Network
# get Cookies
[-] SMB SessionError: code: 0xc0000043 - STATUS_SHARING_VIOLATION - A file cannot be opened because the share access flags are incompatible.

[covertivy]

I explained all this in this comment.
@laxa asked the exact same question, read my reply and feel free to ask questions 😉

[nullsection]

@laxa asked the exact same question, read my reply and feel free to ask questions 😉

My bad! Probably should have seen that!

Thanks

[anadrianmanrique]

merging now, thanks for the PR

[covertivy]

Thanks!
It's been a pleasure!

[1058274]

Merging this PR seems to have broken ntlmrelayx indirectly:

(.venv) bob@debian:/tmp/tmp.AF3wd7q2VI/impacket$ python3 ./examples/ntlmrelayx.py -t 0

Traceback (most recent call last):
  File "/tmp/tmp.AF3wd7q2VI/impacket/./examples/ntlmrelayx.py", line 56, in <module>
    from impacket.examples.ntlmrelayx.servers import SMBRelayServer, HTTPRelayServer, WCFRelayServer, RAWRelayServer, RPCRelayServer
  File "/tmp/tmp.AF3wd7q2VI/impacket/.venv/lib/python3.11/site-packages/impacket/examples/ntlmrelayx/servers/__init__.py", line 12, in <module>
    from impacket.examples.ntlmrelayx.servers.smbrelayserver import SMBRelayServer
  File "/tmp/tmp.AF3wd7q2VI/impacket/.venv/lib/python3.11/site-packages/impacket/examples/ntlmrelayx/servers/smbrelayserver.py", line 46, in <module>
    from impacket.smbserver import getFileTime, decodeSMBString, encodeSMBString
ImportError: cannot import name 'getFileTime' from 'impacket.smbserver' (/tmp/tmp.AF3wd7q2VI/impacket/.venv/lib/python3.11/site-packages/impacket/smbserver.py)

[anadrianmanrique]

@1058274 thanks for reporting. fixed in #2019

[covertivy]

Apparently also broke some of impacket's exec examples.
I fixed it in #2038