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

Smb log configurable 7620 v1.1 #12795

Closed

Conversation

catenacyber
Copy link
Contributor

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

Describe changes:

  • smb: add options to configure logging and making it less verbose

SV_BRANCH=OISF/suricata-verify#2352

Will allow rust to list the values of an array such as
dns.types for outputs
suricata.yaml output section for smb now parses a types list
and will restrict logging of transactions to these types.

By default, everything still gets logged

Remove unused rs_smb_log_json_request on the way

Ticket: 7620
@@ -101,6 +105,42 @@ impl ConfNode {
return Self { conf }
}

pub fn get_child_node(&self, key: &str) -> Option<ConfNode> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jasonish thoughts about this commit ? (wrt to bindgen)

It looks to me that we do not want to have C struct ConfNode exported to rust, but instead just have an opaque pointer and some rust-style functions to handle it, so I went this way

@@ -1143,6 +1143,21 @@ SMB Fields
* "response.native_os" (string): SMB1 native OS string
* "response.native_lm" (string): SMB1 native Lan Manager string

One can restrict which transactions are logged by using the "types" field in the
suricata.yaml file. If this field is not specified, all transactions types are logged.
9 values can be specified with this field as shown below:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nit: ten

if let Some(f) = get_smb_log_type_from_str(nodeu.value()) {
r |= f;
} else {
SCLogWarning!("unknown type for smb logging: {}", nodeu.value());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, this should be done for DNS (warn when an unknown type is configured)

Copy link

codecov bot commented Mar 20, 2025

Codecov Report

Attention: Patch coverage is 81.25000% with 24 lines in your changes missing coverage. Please review.

Project coverage is 80.86%. Comparing base (749ffbd) to head (f39195e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12795      +/-   ##
==========================================
+ Coverage   80.85%   80.86%   +0.01%     
==========================================
  Files         936      936              
  Lines      259870   259982     +112     
==========================================
+ Hits       210120   210240     +120     
+ Misses      49750    49742       -8     
Flag Coverage Δ
fuzzcorpus 57.25% <38.40%> (-0.02%) ⬇️
livemode 19.43% <27.20%> (+<0.01%) ⬆️
pcap 44.10% <41.60%> (-0.02%) ⬇️
suricata-verify 63.64% <81.88%> (+0.04%) ⬆️
unittests 58.10% <0.80%> (-0.02%) ⬇️

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.

@suricata-qa
Copy link

ERROR:
ERROR: ASAN TEST FAIL in ASAN_TLPR1_suri QA test

ERROR: QA failed on ASAN_TLPR1_suri.

Pipeline 25268

@catenacyber
Copy link
Contributor Author

Leak fixed in #12797

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.

2 participants