Added --impersonate flag on mssql protocol#1067
Added --impersonate flag on mssql protocol#1067T1erno wants to merge 1 commit intoPennyw0rth:mainfrom
Conversation
|
ok for me ✅ |
Sounds good, from my side as well. However the argument/module calling is done in the connection.py and should not be reimplemented here. Thanks for the PR! |
|
@T1erno please add this new flag to the e2e tests |
NeffIsBack
left a comment
There was a problem hiding this comment.
Really cool technique :)
One thought: Do we want to continue execution even if impersonation fails? My intuition would be no (and I think that is also the way it is currently implemented?), but let's discuss it. If we don't, we should just exit().
| def call_cmd_args(self): | ||
| if getattr(self.args, "impersonate", None) and not self.impersonation_applied and not self.impersonate(): | ||
| return | ||
| for attr, value in vars(self.args).items(): | ||
| if attr == "impersonate": | ||
| continue | ||
| if hasattr(self, attr) and callable(getattr(self, attr)) and value is not False and value is not None: | ||
| self.logger.debug(f"Calling {attr}()") | ||
| getattr(self, attr)() | ||
|
|
||
| def call_modules(self): | ||
| if getattr(self.args, "impersonate", None) and not self.impersonation_applied and not self.impersonate(): | ||
| return | ||
| super().call_modules() | ||
|
|
There was a problem hiding this comment.
Please modify the logic in connection.py, so that args are called regardless of modules being specified and before these are executed.
| if not target: | ||
| self.logger.error("No impersonation target specified") | ||
| return False |
There was a problem hiding this comment.
That can be enforced via argparse with nargs=1
| raw_target = str(target) | ||
| lowered = raw_target.lower() |
There was a problem hiding this comment.
Argparse will always return strings (type=str), so no need to manually cast it. Besides, please do all the string manipulation in one line, e.g. target =args.impersonate.lower()
| if self.args.debug: | ||
| res = self.conn.sql_query("SELECT SYSTEM_USER, SUSER_SNAME();") | ||
| self.logger.debug(f"Impersonated context: {res}") |
There was a problem hiding this comment.
Just always execute this logic. Debug logs will be filtered anyway if the arg is not specified
Description
The --impersonate flag has been introduced in the MSSQL protocol, adding a first-level protocol-level way to set EXECUTE AS before executing any command arguments or modules
The mssql_priv module is not included in the impersonation flow; this recovers direct impersonation so that users can execute queries/modules with another login/user without additional module modifications
A module-only approach would be limited to module execution, and arbitrary arguments cannot be combined with modules. A protocol-level flag is applied universally and consistently across all CLI actions and modules, without the need to change each module or overload its options
This might be better than modifying the actual modules, which would only affect the internal logic of that module and still not change the session context for other actions. --impersonate sets the session context once, so all subsequent operations are executed under the intended login/user, in a more predictable and reusable way
As you can see in the image, the modules and queries are executed in the context of the impersonated user
Type of change
Insert an "x" inside the brackets for relevant items (do not delete options)
Screenshots (if appropriate):
Checklist:
Insert an "x" inside the brackets for completed and relevant items (do not delete options)
poetry run python -m ruff check . --preview, use--fixto automatically fix what it can)tests/e2e_commands.txtfile if necessary (new modules or features are required to be added to the e2e tests)