-
Notifications
You must be signed in to change notification settings - Fork 381
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
Create get_gpos module, implement get-folder command, and add some module helper functions #320
base: main
Are you sure you want to change the base?
Conversation
Would it be possible to detect a folder on a remote system? Than we could just integrate it into the existing get commands |
Well the current get-file command references a file, so what would we change it to? I think having them separate makes more sense. |
Hmm yeah i was thinking about moving |
just using the verbs "get" and "put" doesn't make it clear what you are getting or putting, so it's sorta confusing, and honestly, changing all that code to just make it a single call sounds like a huge pain in the ass that I'm not interested in doing lol |
Signed-off-by: Marshall Hallenbeck <[email protected]>
I mean we are on SMB, imo its pretty clear what you are trying to achieve. But if it cant be changed by a fairly simple check or try&except call its fine i guess. I just try to limit the amount of options the smb protocol has, there are already a lot (too much imo) |
Well I disagree with that... we have numerous options to get things such as users, groups, disks, computers, etc all under the SMB protocol. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if self.gpo_name: | ||
if k == "displayname": | ||
context.log.highlight(f"Display Name: {v}") | ||
elif k == "name": | ||
context.log.highlight(f"GUID: {v}") | ||
else: | ||
context.log.highlight(f"{k}: {v}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any specific reason you only "pretty print" the key&value pair if self.gpo_name
is set and not always?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll double check but I believe because there's random (default?) GPOs?
self.logger.debug(f"Sharing violation on {remote_path}: {e}") | ||
return False | ||
except Exception as e: | ||
self.logger.debug(f"Other error when attempting to download file {remote_path}: {e}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo we should include the error message in the fail output not in the debug log
def get_file_single(self, remote_path, download_path, silent=False): | ||
share_name = self.args.share | ||
self.logger.display(f'Copying "{remote_path}" to "{download_path}"') | ||
if not silent: | ||
self.logger.display(f"Copying '{remote_path}' to '{download_path}'") | ||
if self.args.append_host: | ||
download_path = f"{self.hostname}-{remote_path}" | ||
with open(download_path, "wb+") as file: | ||
try: | ||
self.conn.getFile(share_name, remote_path, file.write) | ||
self.logger.success(f'File "{remote_path}" was downloaded to "{download_path}"') | ||
except Exception as e: | ||
self.logger.fail(f'Error writing file "{remote_path}" from share "{share_name}": {e}') | ||
if os.path.getsize(download_path) == 0: | ||
os.remove(download_path) | ||
|
||
if self.download_file(share_name, remote_path, file.write): | ||
if not silent: | ||
self.logger.success(f"File '{remote_path}' was downloaded to '{download_path}'") | ||
else: | ||
self.logger.debug("Opening with READ alone failed, trying to open file with READ/WRITE access") | ||
if self.download_file(share_name, remote_path, file.write, FILE_READ_DATA | FILE_WRITE_DATA): | ||
if not silent: | ||
self.logger.success(f"File '{remote_path}' was downloaded to '{download_path}'") | ||
else: | ||
if not silent: | ||
self.logger.fail(f"Error downloading file '{remote_path}' from share '{share_name}'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am kinda not a huge fan of having 3 different "get file" functions now. Maybe we should merge this function with download_file
and make the access_mode
and log message conditional? That way we can also get the error message in the fail message.
That is true but it is still listed in the |
why the heck did i/it close it lol |
I was initially going to have the get_gpos module have the ability to download all of them, hence the helper functions, but LDAP can only query certain GPO information, so it'd have to be another SMB module to download them all; however, the user can just take the GUID output from the module and pass it to
--get-folder
get_gpos
module:--get-folder
feature: