Skip to content

detect: add keywords for LDAPDN - v5#12634

Closed
AkakiAlice wants to merge 5 commits intoOISF:masterfrom
AkakiAlice:detect-ldap-dn-7471-v5
Closed

detect: add keywords for LDAPDN - v5#12634
AkakiAlice wants to merge 5 commits intoOISF:masterfrom
AkakiAlice:detect-ldap-dn-7471-v5

Conversation

@AkakiAlice
Copy link
Contributor

Ticket: #7471

Contribution style:

Our Contribution agreements:

Changes (if applicable):

Link to ticket: https://redmine.openinfosecfoundation.org/issues/7471

Description:

  • Implement keywords ldap.request.dn and ldap.responses.dn

Changes:

  • Change nit: commits to doc:
  • Remove unwanted files

SV_BRANCH=OISF/suricata-verify#2303
Previous PR: #12620

ldap.request.dn matches on LDAPDN from request operations
This keyword maps the following eve fields:
ldap.request.bind_request.name
ldap.request.add_request.entry
ldap.request.search_request.base_object
ldap.request.modify_request.object
ldap.request.del_request.dn
ldap.request.mod_dn_request.entry
ldap.request.compare_request.entry
It is a sticky buffer
Supports prefiltering

Ticket: OISF#7471
ldap.responses.dn matches on LDAPDN from responses operations
This keyword maps the following eve fields:
ldap.responses[].search_result_entry.base_object
ldap.responses[].bind_response.matched_dn
ldap.responses[].search_result_done.matched_dn
ldap.responses[].modify_response.matched_dn
ldap.responses[].add_response.matched_dn
ldap.responses[].del_response.matched_dn
ldap.responses[].mod_dn_response.matched_dn
ldap.responses[].compare_response.matched_dn
ldap.responses[].extended_response.matched_dn
It is a sticky buffer
Supports prefiltering

Ticket: OISF#7471
@codecov
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 85.81560% with 20 lines in your changes missing coverage. Please review.

Project coverage is 80.74%. Comparing base (6fc617c) to head (c32b9dd).
Report is 22 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12634      +/-   ##
==========================================
- Coverage   80.77%   80.74%   -0.03%     
==========================================
  Files         932      932              
  Lines      259286   259411     +125     
==========================================
+ Hits       209437   209472      +35     
- Misses      49849    49939      +90     
Flag Coverage Δ
fuzzcorpus 56.98% <36.87%> (-0.03%) ⬇️
livemode 19.38% <36.87%> (+<0.01%) ⬆️
pcap 44.10% <36.87%> (-0.05%) ⬇️
suricata-verify 63.44% <85.81%> (-0.04%) ⬇️
unittests 58.33% <36.87%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@catenacyber catenacyber 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 the work Alice

CI : ✅
Code : good
Commits segmentation : ok
Commit messages : good
Git ID set : looks fine for me
CLA : you already contributed
Doc update : thanks
Redmine ticket : ok
Rustfmt : good from your side, but rustfmt rust/src/ldap/*.rs gives a small change from Jason :-p
Tests : good
Dependencies added: none

*buffer_len = 0;

let response = &tx.responses[local_id as usize];
// We expect every response in one tx to be the same protocol_op
Copy link
Contributor

Choose a reason for hiding this comment

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

What about IntermediateResponse and SearchResultReference cases ?

Copy link
Contributor

Choose a reason for hiding this comment

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

For Intermediate Response, we should find real pcaps with it to see how it happens

Copy link
Contributor

Choose a reason for hiding this comment

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

And we also need a pcap with SearchResultReference

Copy link
Contributor

Choose a reason for hiding this comment

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

(For me, the PR is good, but I would like to better test/investigate/understand these cases)

@victorjulien victorjulien added this to the 8.0 milestone Feb 21, 2025
@victorjulien
Copy link
Member

Merged in #12653, thanks!

@catenacyber
Copy link
Contributor

Thanks Alice for your first sticky buffer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants