Skip to content
This repository has been archived by the owner on Dec 6, 2023. It is now read-only.

LDAP protocol improvements and scan-network module bugfix #642

Merged
merged 8 commits into from Oct 5, 2022
Merged

LDAP protocol improvements and scan-network module bugfix #642

merged 8 commits into from Oct 5, 2022

Conversation

nurfed1
Copy link
Contributor

@nurfed1 nurfed1 commented Sep 19, 2022

In this pull request I tried fixing some LDAP related issues.

Changes

cme/protocols/ldap.py:

  • When the --no-smb is provided, automatic domain lookup is disabled and --domain should be provided.
  • The LDAP module uses the kdchost or domain parameter to resolve the target ldap server(s?!), instead of designated host parameter. I restored the use of the host parameter as the ldap server
  • The baseDN is generated using the kdchost or domain parameter, depending on the login function (kerberos_login/ plaintext_login/hash_login), which does not make sense. For example when the kdchost points to an actual DC (e.g. dc01.test.com), this results in the invalid baseDN dc=test,dc=test,dc=com. Additionally, using the domain parameter result would result in an incorrect baseDN during cross domain/forest authentication where user domain != target domain. I added twp additional methods to resolve the baseDN: 1. the baseDN parameter and 2. anonymous LDAP lookup to resolve the baseDN.
  • During kerberos login the self.username variable is never defined, this can cause issues in other places (e.g. the new whoami module). I implemented the ldap whoami request to retrieve the username after succesfull kerberos authentication.
  • Fix various inconsistencies between ldap login functions including: missing admin checks, missing add_user_bh calls and improved success logging messages

cme/modules/scan-network.py:

  • The ldap2domain function does not check if the prodived DN is lowercase when searching for "dc=".

Other notes that are not (yet?) addressed:

  • Should the ldap module really implement the --local-auth parameter?
  • The ports parameter is not used

@mpgn
Copy link
Contributor

mpgn commented Sep 19, 2022

Hello thanks for the PR. This is a lot of change while you are solving different problem in one PR.

Can you post some screenshot for the cross domain fix before and after ? Actual code was working pretty well for this kind of situation last time I use it so I'm kind of surprise.

cme ldap forest1.lab -u xxx -p xxx -d forest1.lab --kdcHost dc1.forest2.lab

@nurfed1
Copy link
Contributor Author

nurfed1 commented Sep 20, 2022

Hi,

Sorry if there are to many changes in this PR. While reading and fixing the code. I came across quite a few inconsistencies and with the limited time I have I just tried to fix as much as possible.

My current lab environment looks like this:
forest 1: nurfed.lab (dc01.nurfed.lab)
forest 2: test.lab (dc02.test.lab)

I deduced the following command for my lab environment (using your sample command):
cme ldap nurfed.lab -u nurfedlab.user1 -p 'McsvgVw3to7cBRy8' -d nurfed.lab --kdcHost dc02.test.lab --users

First of all, I have to note that the actual command seems odd because, it will perform NTLM authentication and therefore the kdcHost parameter should not be used? Where I would expect the command to connect to nurfed.lab, it actually connects to the kdchost dc02.test.lab (as shown below).

As seen in the screenshot below, the command fails because the incorrect basedn "dc=dc02,dc=test,dc=lab" is generated based on the kdchost.
image

After tweaking the kdcHost parameter, we get the following command:
cme ldap nurfed.lab -u nurfedlab.user1 -p 'McsvgVw3to7cBRy8' -d nurfed.lab --kdcHost test.lab --users

The results in the following output, the command reports that it connected to DC01, but in fact it actually connected to DC02.TEST.LAB and returned it's users.
image

With my code changes, the following command would ignore the kdchost parameter due to ntlm auth and connect to nurfed.lab:
cme ldap nurfed.lab -u nurfedlab.user1 -p 'McsvgVw3to7cBRy8' -d nurfed.lab --kdcHost test.lab --users

The adjusted command to connect to test.lab would look like the following :
cme ldap test.lab -u nurfedlab.user1 -p 'McsvgVw3to7cBRy8' -d nurfed.lab --users
image

@mpgn
Copy link
Contributor

mpgn commented Sep 20, 2022

I see, interesting ! I will review it :)

@mpgn mpgn added the in review label Sep 20, 2022
@nurfed1
Copy link
Contributor Author

nurfed1 commented Sep 20, 2022

I tried to make a similar comparison for kerberos authentication and I came across additional issues. It seems impacket ldap implementation fails to properly request kerberos tickets for cross forest authentication and It only succeeds when there already are kerberos tickets in the ccache.

During previous testing of my code there were proper kerberos tickets in my ccache file and therefore I didn't encounter errors during cross forest authentication in the past. Thereforce I conclude the my code also doesn't fix cross forest auth but it does fix the following odd behavior.

Tthe following two commands result in unexpected behavior, Where I would expect the command to connect to test.lab, it actually connects to the kdchost dc01.nurfed.lab. The screenshot also incorrectly shows DC02 instead of DC01.

cme ldap test.lab -k -d nurfed.lab --kdcHost dc01.nurfed.lab --users
cme ldap dc02.test.lab -k -d nurfed.lab --kdcHost dc01.nurfed.lab --users

image

I'm not sure if I'm capable enough to fix impacket ldap or add a workaround in cme myself, but I'll have a look.

@nurfed1
Copy link
Contributor Author

nurfed1 commented Sep 21, 2022

Hi mpgn,

Please ignore my comment about impacket ldap cross domain kerberos authentication being broken. I think something in my lab broke instead, (ended up rebuilding everything last night)...

My code however did still have some issues with cross domain kerberos authentication and the kerberoasting/asreproasting commands. This should all be fixed in the lastest commit.

All commands are now properly tested (kerberoasting, asreproast, trusted-for-delegation, -password-not-required, admin-count, ...)

@mpgn
Copy link
Contributor

mpgn commented Sep 21, 2022

Ok I will check it in my lab :)

@mpgn mpgn added all good tested in my lab and removed in review labels Sep 23, 2022
@mpgn mpgn merged commit fc57723 into byt3bl33d3r:master Oct 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
all good tested in my lab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants