Skip to content

Conversation

@kaddujames501-ship-it
Copy link

@kaddujames501-ship-it kaddujames501-ship-it commented Nov 10, 2025

Bug #4330
Make sure these boxes are checked accordingly before submitting your Pull Request -- thank you.

Contribution style:

Our Contribution agreements:

Changes (if applicable):

Link to ticket: https://redmine.openinfosecfoundation.org/issues/4330#change-42436

Describe changes:

  • Previously, the force-hash configuration for file logging silently accepted
    unknown or invalid hash types (e.g., 'shanani'), leading to potential
    misconfiguration.

This patch updates FileForceHashParseCfg() in util-file.c to:

  • Allow only 'md5', 'sha1', or 'sha256' values.
  • Return an error for unknown values.
  • Propagate the error to the output module, causing Suricata to abort in
    test mode (-T) if invalid values are detected.

Tested by adding an invalid hash type ('shanani') in suricata.yaml and
confirming that Suricata fails with the correct error message.

Screenshot from 2025-11-05 17-37-44

Provide values to any of the below to override the defaults.

  • To use a Suricata-Verify or Suricata-Update pull request,
    link to the pull request in the respective _BRANCH variable.
  • Leave unused overrides blank or remove.

SV_BRANCH=OISF/suricata-verify#2751

James Kaddu: [email protected]

Previously, the force-hash configuration for file logging silently accepted
unknown or invalid hash types (e.g., 'shanani')
Now, invalid values trigger FatalError.

Bug OISF#4330
@victorjulien
Copy link
Member

"grep suricata.log: no such file or directory" does not seem like it is a correct error message.

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 : good
Commits segmentation : ok
Commit messages : good
Git ID set : looks fine for me
CLA : I think you signed it
Doc update : not needed
Redmine ticket : ok
Rustfmt : no rust
Tests : I pushed a new better version ;-)
Dependencies added: none

@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.17%. Comparing base (c61f1cb) to head (a431d76).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14305      +/-   ##
==========================================
- Coverage   84.21%   84.17%   -0.04%     
==========================================
  Files        1013     1012       -1     
  Lines      262126   261872     -254     
==========================================
- Hits       220752   220438     -314     
- Misses      41374    41434      +60     
Flag Coverage Δ
fuzzcorpus 63.31% <0.00%> (-0.02%) ⬇️
livemode 18.77% <0.00%> (+0.05%) ⬆️
pcap 44.63% <0.00%> (+0.03%) ⬆️
suricata-verify 64.93% <100.00%> (+0.04%) ⬆️
unittests 59.22% <0.00%> (-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

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

If Philippe's SV test is a good match for this PR, I think that this is good, now.

@catenacyber
Copy link
Contributor

If Philippe's SV test is a good match for this PR, I think that this is good, now.

So, would you review/approve my SV PR ?

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.

4 participants