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

Add NTLMv1 flag on GPO #101

Open
wants to merge 2 commits into
base: v3
Choose a base branch
from
Open

Add NTLMv1 flag on GPO #101

wants to merge 2 commits into from

Conversation

Hackndo
Copy link

@Hackndo Hackndo commented Feb 3, 2024

Similar to BloodHoundAD/SharpHound3#47

If GPO object forces LmCompatibilityLevel to be less than 3, then the computers it will be applied on will use NTLMv1 when authenticating.

This information seems very useful from an attacking perspective as authentication can be coerced and NTLMv1 hash cracked or relayed without MIC

(Also SpecterOps/SharpHound#87 on SharpHound)

image

Copy link

github-actions bot commented Feb 3, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@Hackndo
Copy link
Author

Hackndo commented Feb 3, 2024

I have read the CLA Document and I hereby sign the CLA

@rvazarkar
Copy link
Contributor

The implementation seems fine, but I'm sort of debating if it makes more sense to just merge this in with the existing gpo object processor stuff. @definitelynotagoblin @ddlees do either of you have any feelings on this? It is doing a bit more than our normal object processor does, but generally our processors are loosely linked to a particular collection method, so I think this would probably fit in ObjectProps instead

@JonasBK
Copy link
Collaborator

JonasBK commented Jul 31, 2024

Hi @Hackndo,

Thank you for the two PRs - awesome work! 🙌

We have discussed internally and reached the conclusion that we would like to have two properties for this setting you collect:

  • LM Compatibility Level Raw (lmcompatibilitylevelraw)
  • LM Compatibility Level (lmcompatibilitylevel)

The raw property should hold the int value of the registry setting. The other one should hold a string with corresponding setting value i.e. one of these:

  • Send LM & NTLM responses
  • Send LM & NTLM - use NTLMv2 session security if negotiated
  • Send NTLM responses only
  • Send NTLMv2 responses only
  • Send NTLMv2 responses only. Refuse LM
  • Send NTLMv2 responses only. Refuse LM & NTLM

Are you interested in updating your PR to create those properties? If not, then we will merge your PRs in and we will make the changes on top such that your commits still end up in the git history.

Let me know what you think and thanks again for contributing!

@Hackndo
Copy link
Author

Hackndo commented Aug 1, 2024

Hello,
I'll update my PR in a few days.
Thank you for your feedback
Edit (2nd of August): There were major changes in LDAPUtils in v4. I'll need some time to wrap my head around this. :)

@JonasBK
Copy link
Collaborator

JonasBK commented Sep 26, 2024

Hi @Hackndo,

Just checking if you are still interested in making the update to the PR :)

If not, we can still will merge your PRs in and make the changes on top such that your commits still end up in the git history.

@Hackndo
Copy link
Author

Hackndo commented Sep 26, 2024

Hey there. I still have this in mind but yes, maybe it will be better if you merge and add the necessary changes on top.
Thanks 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants