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

detect: add keywords for LDAP attributes - v1 #12708

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AkakiAlice
Copy link
Contributor

Ticket: #7533

Contribution style:

Our Contribution agreements:

Changes (if applicable):

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

Description:

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

SV_BRANCH=OISF/suricata-verify#2331

ldap.request.attribute_type matches on LDAP attribute type/description
This keyword maps the following eve fields:
ldap.request.search_request.attributes[]
ldap.request.modify_request.changes[].modification.attribute_type
ldap.request.add_request.attributes[].name
ldap.request.compare_request.attribute_value_assertion.description
It is a sticky buffer
Supports prefiltering

Ticket: OISF#7533
ldap.responses.attribute_type matches on LDAP attribute type/description
This keyword maps the eve field ldap.responses[].search_result_entry.attributes[].type
It is a sticky buffer
Supports prefiltering

Ticket: OISF#7533
Copy link

codecov bot commented Mar 3, 2025

Codecov Report

Attention: Patch coverage is 83.70370% with 22 lines in your changes missing coverage. Please review.

Project coverage is 80.72%. Comparing base (93bd193) to head (012d3cc).
Report is 85 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #12708    +/-   ##
========================================
  Coverage   80.71%   80.72%            
========================================
  Files         936      936            
  Lines      259393   259528   +135     
========================================
+ Hits       209368   209502   +134     
- Misses      50025    50026     +1     
Flag Coverage Δ
fuzzcorpus 56.94% <28.14%> (-0.03%) ⬇️
livemode 19.42% <28.14%> (+<0.01%) ⬆️
pcap 44.21% <28.14%> (+0.02%) ⬆️
suricata-verify 63.52% <83.70%> (+<0.01%) ⬆️
unittests 58.20% <28.14%> (-0.01%) ⬇️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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,

CI : ✅
Code : looking now
Commits segmentation : ok
Commit messages : good, we could mention multi-buffer in the commit message as well
Git ID set : looks fine for me
CLA : you already contributed
Doc update : ok, ⚠️ we should mention multi-buffer for ldap.request.attribute_type nd there is also the multi-buffer page to update ;-)
Redmine ticket : ok
Rustfmt : could you add the rustfmt commit ?
Tests : approved SV PR
Dependencies added: none

- ``ldap.request.search_request.attributes[]``
- ``ldap.request.modify_request.changes[].modification.attribute_type``
- ``ldap.request.add_request.attributes[].name``
- ``ldap.request.compare_request.attribute_value_assertion.description``
Copy link
Contributor

Choose a reason for hiding this comment

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

@pc-anssi @glongo what do you think about this it looks like the asn1 spec type AttributeType ::= LDAPString is named differently in eve.json for different kinds of operations ?

@catenacyber
Copy link
Contributor

Code all good :-)

@catenacyber
Copy link
Contributor

Besides the doc point, this is good.

If you want to add the keyword on AttributeType=AttributeValue it can be in the next version of this PR, or in a later follow-up PR

@victorjulien victorjulien added the needs rebase Needs rebase to master label Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Needs rebase to master
Development

Successfully merging this pull request may close these issues.

3 participants