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

Fix permission error in JSON alert #14019

Merged
merged 4 commits into from
Sep 7, 2022

Conversation

FrancoRivero
Copy link
Contributor

@FrancoRivero FrancoRivero commented Jun 24, 2022

Issue
Closes #12073

Description

When FIM is run on Windows logs with a malformed permission, the analysisd module generates a log message and discards this permission. This PR is to add changes to this behavior, right now the module analysisd reads permissions and discard only malformed permissions.

Logs/Alerts example

Syscheck stress test result,

Tests

  • Compilation without warnings in every supported platform
    • Linux
    • Windows
    • MAC OS X
  • Source installation
  • Package installation
  • Source upgrade
  • Package upgrade
  • Review logs syntax and correct language
  • Run stress test (result)
  • Add unit tests to new code

@FrancoRivero FrancoRivero force-pushed the 12073-permission-error-in-json-alert branch from c45c273 to 950a5c8 Compare June 24, 2022 15:56
@FrancoRivero FrancoRivero marked this pull request as ready for review June 27, 2022 10:27
@FrancoRivero FrancoRivero self-assigned this Jun 27, 2022
antoniomanuelfr
antoniomanuelfr previously approved these changes Jul 1, 2022
jotacarma90
jotacarma90 previously approved these changes Jul 7, 2022
Copy link
Member

@jotacarma90 jotacarma90 left a comment

Choose a reason for hiding this comment

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

LGTM

MarcelKemp
MarcelKemp previously approved these changes Jul 7, 2022
src/analysisd/format/to_json.c Outdated Show resolved Hide resolved
src/shared/syscheck_op.c Outdated Show resolved Hide resolved
src/shared/syscheck_op.c Outdated Show resolved Hide resolved
src/unit_tests/shared/test_syscheck_op.c Outdated Show resolved Hide resolved
chemamartinez
chemamartinez previously approved these changes Aug 2, 2022
Copy link
Contributor

@chemamartinez chemamartinez left a comment

Choose a reason for hiding this comment

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

LGTM

@FrancoRivero FrancoRivero force-pushed the 12073-permission-error-in-json-alert branch from c837fd5 to 850faee Compare August 2, 2022 13:16
@FrancoRivero FrancoRivero changed the base branch from 4.4 to master August 2, 2022 13:16
@FrancoRivero FrancoRivero dismissed stale reviews from chemamartinez, MarcelKemp, jotacarma90, and antoniomanuelfr August 2, 2022 13:16

The base branch was changed.

chemamartinez
chemamartinez previously approved these changes Aug 3, 2022
@chemamartinez
Copy link
Contributor

Test: AR IT - Tier 0 - windows_agent is failing with the following error:

============================== warnings summary ===============================
..\..\..\..\Python37\lib\site-packages\win32\lib\pywintypes.py:2
  C:\Python37\lib\site-packages\win32\lib\pywintypes.py:2: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
    import imp, sys, os

This is a dependency error in the test itself that is not related to this pull request. It is already addressed at wazuh/wazuh-qa#2769, so please ignore it.

@chemamartinez chemamartinez added module/fim File Integrity Monitoring module/analysis Issues related to the Analysis daemon labels Aug 3, 2022
antoniomanuelfr
antoniomanuelfr previously approved these changes Aug 8, 2022
chemamartinez
chemamartinez previously approved these changes Aug 16, 2022
@jmv74211
Copy link
Contributor

jmv74211 commented Sep 7, 2022

QA review

  • Type: Manual testing.
  • Status: Approved 🟢
  • Testing issue: Manual testing - Fix permission error in JSON alert wazuh-qa#3133
  • Comments: The QA team has not been able to reproduce the bug as such. The development team has created unit tests that test similar cases and it is tested in this way. QA has manually tested that the bug does not occur, and FIM regression tests have been launched.

@chemamartinez chemamartinez merged commit e1ee9da into master Sep 7, 2022
@chemamartinez chemamartinez deleted the 12073-permission-error-in-json-alert branch September 7, 2022 08:34
@rauldpm rauldpm restored the 12073-permission-error-in-json-alert branch September 25, 2023 16:40
@rauldpm rauldpm deleted the 12073-permission-error-in-json-alert branch September 25, 2023 16:40
@dami-nicastro
Copy link
Member

Hello @chemamartinez and team:
Could you tell me which Wazuh version will have the fix for this issue? Please, also let me know the approximated release date for it.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/analysis Issues related to the Analysis daemon module/fim File Integrity Monitoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error adding permissions to json alert file in syscheck stress test 4.3.0-rc3
7 participants