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

feat in net.py: add functionality to enable and disable user accounts #1801

Merged
merged 4 commits into from
Dec 20, 2024

Conversation

marcobarlottini
Copy link
Contributor

Hi,
this PR adds functionality to net.py to enable or disable user accounts
image

image

image

image

@anadrianmanrique anadrianmanrique added feature New feature or request in review This issue or pull request is being analyzed labels Oct 3, 2024
@gabrielg5 gabrielg5 self-assigned this Oct 3, 2024
@gabrielg5
Copy link
Collaborator

gabrielg5 commented Dec 12, 2024

Hi @marcobarlottini, thanks for your PR!

Been checking it and think there's an issue with the flags being set (both in the _hEnableAccount and _hDisableAccount functions)

I think we should be gathering current UserAccountControl information and modifying corresponding flag, otherwise we may be overwriting other flags.

Also in the _hDisableAccount function we are setting the samr.USER_NORMAL_ACCOUNT - making the user not a computer object - (ie: if you run net.py user -disable <computer-account> it is disabled and transformed into a user account)
Which also makes me think that perhaps we should be adding these 2 commands (enable and disable) for computer accounts, for the sake of completeness...

Another detail, not strictly related with you PR, is the constant we are using to set the flag in the _hEnableAccount function

buffer['Control']['UserAccountControl'] = samr.USER_ALL_ADMINCOMMENT
We should be using samr.USER_NORMAL_ACCOUNT instead (values are the same, but the other const is the correct one)

Edit
Rechecking this, found another issue related to the last paragraph.
Computer inherits from User, so when creating a new computer account it is being set as a user account (!).

@gabrielg5 gabrielg5 added waiting for response Further information is needed from people who opened the issue or pull request and removed in review This issue or pull request is being analyzed labels Dec 12, 2024
@marcobarlottini
Copy link
Contributor Author

Hi @gabrielg5,
thanks for pointing what could have ended as bugs in the code base; i have implemented changes to preserve the flags when enabling and disabling accounts: we query and pass the flags before setting the field buffer['Control']['UserAccountControl']

Regarding the last note you added, computer inherits from User but there are fields _create_account_type _enum_account_type

self._create_account_type = samr.USER_WORKSTATION_TRUST_ACCOUNT
that allow for specialization of objects created with the Computer class inherited method create, so they are not created as User objects

I also added command line options to the argparse to have the enable and disable options also for subparser computer for the sake of completeness

Let me know if something needs to be changed

@gabrielg5
Copy link
Collaborator

Hey @marcobarlottini,

yeah, saw those other properties Computer class is setting. I was referring to the value being set when calling to _hEnableAccount in the Create function. That value (0x00000010) is setting that the user is not a computer object.

With your latest changes, however, I'm getting an error when calling with the -create flag

[-] User._hEnableAccount() missing 1 required positional argument: 'user_account_control'

Perhaps we can move the call to Query inside those functions (enable/disable) or directly pass the missing param in this call.

Thanks!

@marcobarlottini
Copy link
Contributor Author

hey @gabrielg5 ,
thanks for your feedback again; i tried before to place the call to query inside the functions_hEnableAccount()and _hDisableAccount() but i get a DCERPC exception , i think because both SetUserAccountControl() and Query() call _open_domain(), so i called Query before

i don't know if you like it but this is how i solved it, by passing in the call to _hEnableAccount() inside Create() the flags _create_account_type ORed with samr.USER_ACCOUNT_DISABLED, it should work as intended for both User and Computer objects

@gabrielg5
Copy link
Collaborator

mmm perhaps by calling inside both functions? (it's the same function that is called in Query)

user_account_control = samr.hSamrQueryInformationUser2(self._dce, user_handle, samr.USER_INFORMATION_CLASS.UserAllInformation)['Buffer']['All']['UserAccountControl']

That way the uac gathering is kept in same context, and no extra parameters are needed neither in enable onr in disable

Note: Tried querying less information, but couldn't find a way yet of gathering the uac only

@marcobarlottini
Copy link
Contributor Author

your solutions seems somewhat cleaner, let me know if you have other improvements!

@gabrielg5
Copy link
Collaborator

Awesome! merging now 🚀
Thank you!!

@gabrielg5 gabrielg5 merged commit c1a53aa into fortra:master Dec 20, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request waiting for response Further information is needed from people who opened the issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants