Adds additional error handling in the SMB download function
Adds error message when no hosts have ports 139 and 445 open
Original Scenario: Attempting to download a file named backup_credentials.txt from SMB share named backup
The path manipulations in the download_file function will replace any instance of the share name with empty space
path = path.replace(share, '')
In this scenario, the replacement will lead the path variable to evaluate to \\_credentials.txt rather than the expected \\backup_credentials.txt
A quick fix is to limit the replace to only one occurrence
path = path.replace(share, '', 1)
While debugging this issue, I noticed the handle for the output file path was not always closed if exceptions were raised. A good way to correct this issue is by creating the file handle using a with statement so the handle will definitely be closed.
No Valid Hosts Error Message
When there are no hosts that have ports 139 and 445 open, the program will close without a message. There needs to be more transparency into where the failure lies so we can troubleshoot issues faster. I think a lot of areas of the code could use more messaging to assist with troubleshooting. This may warrant the introduction of a logger so output can be filtered to specific logs levels and users can decide what level of verbosity they would like to receive.
Comments
@ShawnDEvans Thank you for putting this tool together! This is my first time contributing to the repo and I would be happy to continue if that would be supported. Some of the fixes in this PR also apply to may other areas of the code (pathing and file handles) and I would be happy to release follow-ups to address those. I wanted to start small, feel out the appetite for changes here.
Summary
Original Scenario: Attempting to download a file named
backup_credentials.txt
from SMB share namedbackup
The path manipulations in the download_file function will replace any instance of the share name with empty space
path = path.replace(share, '')
In this scenario, the replacement will lead the path variable to evaluate to
\\_credentials.txt
rather than the expected\\backup_credentials.txt
A quick fix is to limit the replace to only one occurrence
path = path.replace(share, '', 1)
While debugging this issue, I noticed the handle for the output file path was not always closed if exceptions were raised. A good way to correct this issue is by creating the file handle using a with statement so the handle will definitely be closed.
No Valid Hosts Error Message
When there are no hosts that have ports 139 and 445 open, the program will close without a message. There needs to be more transparency into where the failure lies so we can troubleshoot issues faster. I think a lot of areas of the code could use more messaging to assist with troubleshooting. This may warrant the introduction of a logger so output can be filtered to specific logs levels and users can decide what level of verbosity they would like to receive.
Comments
@ShawnDEvans Thank you for putting this tool together! This is my first time contributing to the repo and I would be happy to continue if that would be supported. Some of the fixes in this PR also apply to may other areas of the code (pathing and file handles) and I would be happy to release follow-ups to address those. I wanted to start small, feel out the appetite for changes here.