Skip to content

Conversation

@tmolbergen
Copy link

Description

Add argument option to load ldapusername/ldappassword from environment variable (operator define the env variable name to pull from) or a operator defineable json file. I've specifically left out any configuration options or similar as it probably goes to much in conflict with the enterprise version of Sharphound

Motivation and Context

I am not a big fan of spraying passwords in my windows logs. This should hopefully to some extent mitigate that. Do note that this is not to be looked at as a security feature, but it atleast prevents the host from spraying passwords into logs and collected logs.

How Has This Been Tested?

Compiled code, tested with json file and envoptions (also tested the --ldapusername --ldappassword options).
This was tested on 4 different active directory domains. The changes are not huge and didnt impact any of the operations of Sharphound.

Screenshots (if appropriate):

Types of changes

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • [X ] 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.

Didnt know there were any tests or where to define them. Im not doing anything towards the database, so that shouldnt be needed to define. Not sure if there are any other places than the readme to update

Im by no means of the imagination a developer, but it would be nice to have this or some similar feature included to ensure that passwords are not sprayed in the logs.

@ddlees
Copy link
Member

ddlees commented Jan 8, 2025

recheck

@github-actions
Copy link

github-actions bot commented Jan 8, 2025

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

@tmolbergen
Copy link
Author

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

@tmolbergen
Copy link
Author

recheck

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.

2 participants