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

LDAP analyzer #56

Merged
merged 21 commits into from May 19, 2021
Merged

LDAP analyzer #56

merged 21 commits into from May 19, 2021

Conversation

mmguero
Copy link
Contributor

@mmguero mmguero commented May 11, 2021

The LDAP analyzer (and its underlying ASN.1 module) is to the point where it has the things I need from it. I've tested it on as much live traffic and PCAP traffic as I could and it seems to be working well.

Here's what it has:

  • ASN.1 structure decoding: this is probably generally useful for more than just the LDAP parser, so it may be of interest for this to be included somehow as part of spicy's standard modules or whatever
    • everything is working except for the "constructed" forms of ASN1BitString and ASN1OctetString
  • LDAP: the LDAP parsing is basically "done once" through a single call to ASN1Message (which parses itself recursively) and then the application-level data is also parsed via &parse-from a byte array belonging to the outer ASN.1 sequence. This second level of parsing is also done using the ASN.1 data types.
    • events
      • ldap::message - called for each LDAP message
      • ldap::bindreq - when a bind request is made
      • ldap::searchreq - basic search request information
      • ldap::searchres - called each time a search result is returned
    • enums
      • ProtocolOpcode
      • ResultCode
      • BindAuthType
      • SearchScope
      • SearchDerefAlias
      • FilterType
    • Zeek log files
      • ldap.log - contains information about all LDAP messages except those that are search-related. Log lines are grouped by connection ID + message ID
        • ts (time)
        • uid (connection UID)
        • id (connection ID 4-tuple)
        • proto (transport protocol)
        • message_id (LDAP message ID)
        • version (LDAP version for bind requests)
        • opcode (set of 1..n operations from this uid+message_id)
        • result (set of 1..n results from this uid+message_id)
        • diagnostic_message (vector of 0..n diagnostic message strings)
        • object (vector of 0..n "objects," the meaning of which depends on the operation)
        • argument (vector of 0..n "argument," the meaning of which depends on the operation)
      • ldap_search.log - contains information about LDAP searches. Log lines are grouped by connection ID + message ID
        • ts (time)
        • uid (connection UID)
        • id (connection ID 4-tuple)
        • proto (transport protocol)
        • message_id (LDAP message ID)
        • scope (set of 1..n search scopes defined in this uid+message_id)
        • deref (set of 1..n search deref alias options defined in this uid+message_id)
        • base_object (vector of 0..n search base objects specified)
        • result_count (number of result entries returned)
        • result (set of 1..n results from this uid+message_id)
        • diagnostic_message (vector of 0..n diagnostic message strings)
    • test
      • basic tests for detecting plugin presence and simple bind and search result/requests

Here's what it doesn't have, which could be added by future parties interested in expanding it:

  • although LDAP can use UDP as transport, currently the analyzer only looks for 389/tcp, 3268/tcp; this may be easy to change, but I don't have any examples of traffic to test it with so I haven't bothered
  • LDAP referrals are not parsed out of the results
  • SASL credentials in bind requests are not being parsed beyond the mechanism string
  • SASL information in bind responses are not being parsed; for that matter, SASL-based LDAP stuff hasn't been tested much and may have issues
  • Search filters and attributes: while basic parsing is done in the AttributeSelection, AttributeValueAssertion, SubstringFilter and SearchFilter units, I'm not really pulling stuff out from the search filter tree (as the protocols allows arbitrarily complex combinations of AND, OR, substring, etc. that I don't think would be easily representable in a log file)
  • the details of SearchResultReference are not being parsed
  • the only detail of ModifyRequest being parsed is the object name
  • the details of AddRequest are not being parsed
  • the details of ModDNRequest are not being parsed
  • the details of CompareRequest are not being parsed
  • the details of AbandonRequest are not being parsed
  • the details of ExtendedRequest are not being parsed
  • the details of ExtendedResponse are not being parsed
  • the details of IntermediateResponse are not being parsed

mmguero and others added 10 commits May 10, 2021 11:01
* added LDAP stubbed out files

* stubbing PDU types

* work in progress (found asn1.spicy module)

* more asn1 work in progress

* more asn1 work in progress

* more asn1 work in progress

* more asn1 work in progress

* more asn1 work in progress; compiling but some stuff has been commented out. need to examine one by one

* more asn1 work in progress; compiling but some stuff has been commented out. need to examine one by one

* asn1 work in progress

* asn1 work in progress

* stub out debug output

* work in progress

* added debug back in

* more work on bind request

* more work in progress on bind request

* more work on ldap bindRequest

* more work in progress, figururing out application ASN.1 BER class. see https://ldap.com/ldapv3-wire-protocol-reference-asn1-ber/ for a big help

* more work in progress, figururing out application ASN.1

* more work in progress, figururing out application ASN.1

* working on bindrequest

* more work on ldap

* wip on ldap/spicy

* comment out specifying vector length

* more work in progress on ldap

* LDAP work in progress

* Fix indents and remove wrapper.

* Spaces to tabs.

* Switch to spaces.

* Update source for trace file.

* Fix various vector parsing issues.  Also remove typing from the_type since we don't know all cases yet.

* Added Cisco vendor IDs.

* Update baselines.

* Add another vendor id.

* work in progress with zeek integration plumbing:

* plumbing in place for logging

* more logging work in progress

* more logging work in progress

* comment out some stuff

* redue verbosity

* print out numbers of unparsed bytes

* debugging ldap

* specify message length so we don't parse more than we should per-message

* ldap work in progress

* push 'catch-all' bytes &eod array to the sub-messages

* debug print out the list of unparsed data

* need to parse ldap messages in an array

* Adding result

* don't explicitly set a bool for hasResult

* explicitly set a bool for hasResult

* add column

* use unset value instead of a separate boolean

* progress on ldap.log

* added more results

* more work on ldap log

* make op and result set of enum instead of vector of enum

* add comments

* need EOL

* formatting and work on ldap processor

* more work on ldap

* working on putting search into its own separate log file

* working on putting search into its own separate log file

* more work on search filtering

* work in progress on the ldap processor; asn1 can now be recursive, although I'm not using it yet because it's a whole mindshift from what i've been doing

* Added more debug printing

* Added more debug printing

* for now store application types in a big 'bytes' array

* Added more debug printing

* recursive parsing for ldap via asn1

* great progress on ldap

* great progress on ldap

* great progress on ldap

* Allow success with empty entries

* formatting, and use &convert to decomplicate member access

* use strings instead of enums for log output

Co-authored-by: Keith Jones <keith@keithjjones.com>
Co-authored-by: Robin Sommer <robin@corelight.com>
…able to fix CI integration error.

Thanks to @bbannier in #56 (comment): "This combination of stringification, tuples, and|...| triggers the CI error you are seeing."

This could be removed once zeek/spicy#919 is in.
Copy link
Contributor

@bbannier bbannier left a comment

Choose a reason for hiding this comment

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

Thanks for this PR. I made a first round through the ASN.1 parser. Will look at the other parts now.

analyzer/protocol/ldap/asn1.spicy Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/asn1.spicy Show resolved Hide resolved
analyzer/protocol/ldap/ldap.evt Show resolved Hide resolved
analyzer/protocol/ldap/__load__.zeek Show resolved Hide resolved
analyzer/protocol/ldap/CMakeLists.txt Show resolved Hide resolved
analyzer/protocol/ldap/ldap.spicy Show resolved Hide resolved
analyzer/protocol/ldap/ldap.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/ldap.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/ldap.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/ldap.spicy Outdated Show resolved Hide resolved
analyzer/protocol/ldap/ldap.spicy Outdated Show resolved Hide resolved
@mmguero mmguero changed the title LDAP analyzer LDAP analyzer [WIP] May 12, 2021
@mmguero
Copy link
Contributor Author

mmguero commented May 12, 2021

Finished @bbannier 's review and making changes from it. I'm going to do some more testing and will reply here once I feel things are all as stable as they were pre-review. Thanks a lot

@mmguero mmguero changed the title LDAP analyzer [WIP] LDAP analyzer May 12, 2021
@mmguero
Copy link
Contributor Author

mmguero commented May 12, 2021

Made the changes suggested by @bbannier in his review, minus the few of them that I gave reasons for in the replies to his comments. I'm still getting all the same .log output and passing my tests, so that's a good thing ;).

mmguero added a commit to cisagov/Malcolm that referenced this pull request May 13, 2021
* [Network analyzers](https://github.com/cisagov/malcolm#Protocols)
    - Added support for [EtherCAT](https://en.wikipedia.org/wiki/EtherCAT) ([ICS protocol](https://github.com/cisagov/icsnpp-ethercat))
    - Fixed and improved Spicy-based [LDAP analyzer](zeek/spicy-analyzers#56)
    - Detect VPN [protocols](https://github.com/zeek/spicy-analyzers/tree/main/analyzer/protocol) IPsec, OpenVPN and WireGuard

* New or improved
    - Updated many Kibana dashboards and added dashbaords for newly-supported network protocols
    - Improved output of debug logs from docker images
    - Many minor improvements to underlying system for ISO installations
    - **Massively** cut build time for Hedgehog ISO and Zeek Docker container by using .deb packages from released versions rather than building from source
    - During build, [install all Zeek plugins](https://github.com/cisagov/Malcolm/blob/master/shared/bin/zeek_install_plugins.sh) via zkg

* Version updates
    - **[Zeek](https://github.com/zeek/zeek/releases) v4.0.1**
    - [Spicy](https://github.com/zeek/spicy) v1.0.0
    - [Open Distro For Elasticsearch](https://opendistro.github.io/for-elasticsearch-docs/version-history/) v1.13.2
    - [Yara](https://github.com/VirusTotal/yara/releases) v4.1.0
    - [Capa](https://github.com/fireeye/capa/releases) v1.6.3
    - switch from centos:7 to [amazonlinux:2](https://hub.docker.com/_/amazonlinux) for base Docker image to build Kibana plugins
    - [stunnel](https://www.stunnel.org/NEWS.html) v5.59
    - [NGINX](https://nginx.org/) v1.20.0
    - [LLVM/clang](https://releases.llvm.org/11.0.1/docs/ReleaseNotes.html) toolchain v11
    - Flask-Cors v3.0.9 for Hedgehog kiosk interface (dependabot-flagged [security alert](https://nvd.nist.gov/vuln/detail/CVE-2020-25032))
    - latest updates of various Zeek plugins, system and python packages, etc.
    - all Python scripts updated to Python 3

* Bugs fixed
    - When LDAP authentication is used instead of BASIC authentication, show a landing page rather than a server error when attempting to browse to the local authentication management interface
    - Fixed a [regression bug](idaholab#42) where Malcolm fails to start correctly if not using UID/GID 1000:1000
    - [Don't automatically expose](idaholab#38) elasticsearch (and logstash) ports unless explicitly configured to do so
    - freshclam should update the clamav database [during docker image build](idaholab#39)
bbannier
bbannier previously approved these changes May 18, 2021
Copy link
Contributor

@bbannier bbannier left a comment

Choose a reason for hiding this comment

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

Thanks for spending the time to clean this up @mmguero.

Overall this looks good to me. Like I mentioned offline we could still make this easier to consume by minimizing data exposed by units (you often use members for parsing, and then set some dependent unit var; ideally we'd wouldn't expose the member used for parsing after that). If you do not have time for that cleanup, I believe we take that on after merging.

Let's also see whether @rsmmr has any additional suggestions.

@bbannier bbannier requested a review from rsmmr May 18, 2021 13:50
bbannier
bbannier previously approved these changes May 18, 2021
@mmguero mmguero changed the title LDAP analyzer LDAP analyzer [WIP] May 18, 2021
@mmguero
Copy link
Contributor Author

mmguero commented May 18, 2021

Something got a little bungled in merging from upstream, I'll fix whatever it is and comment here again when I'm good-to-go.

@bbannier bbannier self-requested a review May 18, 2021 15:03
Squashed commit of the following:

commit 4cd5e79
Author: SG <13872653+mmguero@users.noreply.github.com>
Date:   Tue May 18 12:32:26 2021 -0600

    something in 51ec8fd broke something, this branch is debugging it

commit bbf65a7
Author: SG <13872653+mmguero@users.noreply.github.com>
Date:   Tue May 18 12:29:39 2021 -0600

    something in 51ec8fd broke something, this branch is debugging it

commit 3636f30
Author: SG <13872653+mmguero@users.noreply.github.com>
Date:   Tue May 18 12:27:18 2021 -0600

    Formatting

commit 7957242
Author: SG <13872653+mmguero@users.noreply.github.com>
Date:   Tue May 18 12:24:38 2021 -0600

    something in 51ec8fd broke something, this branch is debugging it

commit 8bed45b
Merge: d86336c 6696428
Author: SG <13872653+mmguero@users.noreply.github.com>
Date:   Tue May 18 12:21:58 2021 -0600

    Merge remote-tracking branch 'mmguero-dev/main' into topic/ldapdebug

commit d86336c
Author: SG <13872653+mmguero@users.noreply.github.com>
Date:   Tue May 18 12:06:38 2021 -0600

    something in 51ec8fd broke something, this branch is debugging it

commit 292186c
Author: SG <13872653+mmguero@users.noreply.github.com>
Date:   Tue May 18 11:59:45 2021 -0600

    something in 51ec8fd broke something, this branch is debugging it
@mmguero
Copy link
Contributor Author

mmguero commented May 18, 2021

Okay, I got it sorted. I think I'm good whenever @bbannier and/or @rsmmr are.

@mmguero mmguero changed the title LDAP analyzer [WIP] LDAP analyzer May 18, 2021
@rsmmr rsmmr self-assigned this May 19, 2021
@rsmmr rsmmr merged commit 0687331 into zeek:main May 19, 2021
@rsmmr
Copy link
Member

rsmmr commented May 19, 2021

This has come together nicely, good job!

I went ahead and merge it, and created a ticket tracking some TODOs: #58

Also, I turned the PR's description into a README. @mmguero Feel free to file another PR if you want clean that up a bit more.

mmguero added a commit to mmguero-dev/Malcolm that referenced this pull request May 19, 2021
…tching my plugin install sript to use zeek/spicy-analyzers instead of mmguero-dev/spicy-analyzers
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.

None yet

4 participants