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

Fixed LAPS attributes #167

Merged
merged 7 commits into from
Oct 18, 2024
Merged

Fixed LAPS attributes #167

merged 7 commits into from
Oct 18, 2024

Conversation

spyr0-sec
Copy link
Contributor

@spyr0-sec spyr0-sec commented Oct 2, 2024

Description

As discussed with @rvazarkar attributes for "new" LAPS were not being captured due to typos

Motivation and Context

ReadLAPSPassword edges were not being created as the password attributes were not captured in the GUID map

How Has This Been Tested?

This has not been tested, just sanity checked by matching up the names in the AD schema

Screenshots (if appropriate):

image

Example of the ms-LAPS-EncryptedPassword GUID

image

Types of changes

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Documentation updates are needed, and have been made accordingly.
  • I have added and/or updated tests to cover my changes.
  • All new and existing tests passed.
  • My changes include a database migration.

Updated LAPS password attributes
Updated logic to create ReadLAPSPassword edges based on updated LAPS password attributes
Updated logic to pull GUIDs for new LAPS password attributes
Corrected new LAPS password expiry attribute
@JonasBK
Copy link
Collaborator

JonasBK commented Oct 7, 2024

Looks like there is another issue with building the GUID cache. I'm getting a handful of these errors in my lab when running SharpHound with v -1:
Error while building GUID cache for EXTERNAL.LOCAL: Query - Caught unrecoverable exception: The size limit was exceeded

That effects legacy LAPS too. Will get that fixed first and then test your PR.

@spyr0-sec
Copy link
Contributor Author

Think I have worked out the confusion. The HasLAPS() logic seems to be not returning the value for the name attribute
(directoryObject.TryGetLongProperty(LDAPProperties.LAPSExpirationTime, out var lapsExpiration)

Whereas, the BuildGuidCache is returning the SchemaIDGuid and the name values for the LAPS extended rights. So I have fixed the attributes in "new" LAPS. @JonasBK / @rvazarkar can I just get you confirm the name values are correct for the legacy LAPS.

LegacyLAPSExpirationTime = value of adminDisplayName / cn / lDAPDisplayName (assumings it different value to name) for ms-mcs-admpwdexpirationtime attribute

LegacyLAPSPassword = value of name for ms-mcs-admpwd attribute.

image

@rvazarkar
Copy link
Contributor

Will test legacy LAPS later and if all good, will merge in. Thank you for the excellent work!

@rvazarkar rvazarkar merged commit 1d9e9c0 into BloodHoundAD:v4 Oct 18, 2024
2 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants