Closed edermi closed 3 years ago
First of all, thanks so much for the detailed and awesome PR! I'm happy you were able to figure out enough of my spaghetti code to make it all work!
I'm thinking of making/testing some changes that should hopefully make this a bit more straightforward and remove some of the logic from the module code and into the LDAP session code. I hadn't considered the need to make multiple searchRequests and write those results to the channels and while your approach totally works, I might hack up something quickly and share with you to see if it makes life a bit easier. I'll push some new commit if you don't mind and we can discuss there?
Thanks again!!
Thanks for taking a look, IMO the code is easy to understand and follow (at least the parts I had to go through). Working on this on a new commit is fine for me, just @ me in the comments so I'm getting notified
@edermi - sorry for the delay. I may have got carried away, but I kept reworking/refactoring to try and make this easy as possible for future modules. Check out my latest commits to your branch. The biggest changes I made to to the base code is:
func (w *LDAPSession) ExecuteBulkSearchRequest(searchRequests []*ldap.SearchRequest) error
Module
interface: func (d DnsNamesModule) DisplayFilter(entry *adschema.ADEntry) bool
--ignore-display-filters
optionI think after we get these modules merged I'll tackle parsing the binary DNS Record entry separately
Please test and let me know what you think. This is really, really cool stuff - I appreciate the idea and the PR!
Oh, forgot to mention - do you mind adding these new sweet modules to https://github.com/ropnop/go-windapsearch/blob/master/pkg/modules/README.md ?
Hi @ropnop, first of all thanks for taking a look and refactoring my shortcut hacks into something reusable. All your suggestions sound good to me, I'm fine with them. I will take a look, test and provide feedback, but it may take some time as I'm really busy for the next few weeks. I'll also add the documentation.
Hey! Have you had a chance to test in your lab? Finally got some free time this weekend so I’m looking to merge this and clean up some code :)
Nope, but its on my list for this weekend :) Sorry for the delay, end of the year is stuffed with work...
Ha no worries man. Same here. Really appreciate the work, I’m pumped on this new feature!
Hi, I'm done with my review. It looks really nice, I'm glad that you took some time to bring my hack into shape. Nothing more to say apart from my comment on deferring the CloseChannels()
call here
I've added a description of the modules to the readme, maybe take another look at it before merging.
Nice catch! Everything looks good, merging in 👍 I've got some new work no I can merge on top of this that actually unmarshals the binary DNS records too :)
This PR adds enumeration of DNS zones, see https://dirkjanm.io/getting-in-the-zone-dumping-active-directory-dns-with-adidnsdump/
Unfortunately, this topic is a bit difficult...:
ManualWriteSearchResultsToChan
that allows more than one*ldap.SearchResult
as argument__kerberos
) by DNs containing a substring, without success. Therefore, the filtering is done manually.Unfortunately, this pull request also contains my other pull request #9 because I needed it for developing/testing and apparently messed up my local checkout, I'm really sorry for that. I can try to fix this, but my git experience is that in 90% of cases, things just get worse. If this is a blocker for merging, I will try cleaning this up.