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 Alerts Summary table truncated results #5131

Merged
merged 5 commits into from Jan 25, 2023

Conversation

asteriscos
Copy link
Member

@asteriscos asteriscos commented Jan 16, 2023

Description

Team,

this PR fixes the query result of the PDF report Alerts Summary Table to avoid truncating the number of rows. The parsing algorithm wasn't iterating all the bucket combinations, therefore it returned only the first combination of buckets. Now it iterates al possible combinations of the aggregation buckets and returns all possible results.

Note: We also found server-side functions truncate and sort indexer queries. The issue is described in the issue #5134

Issues Resolved

Closes #5130

Evidence

image

Test

  • Generate a report of each module with and without a pinned agent.
    • The rendered Alerts Summary table should be consistent with the rest of the visualizations and the available alerts in the reported Module Dashboard
  • Setup the Allow agents restriction filter and verify it applies to the report table.

Check allowed Agents

Module Allowed agents filtered
Security events

Security Information Management

Module With pinned agent Without agent
Security events
Integrity monitoring
Office 365
Amazon AWS
Google Cloud Platform
GitHub

Auditing and Policy Monitoring

Module With pinned agent Without agent
Policy monitoring
Security configuration assessment
System auditing
OpenSCAP
CIS-CAT

Threat Detection and Response

Module With pinned agent Without agent
Vulnerabilities
MITRE ATT&CK
VirusTotal
Osquery
Docker listener

Regulatory Compliance

Module With pinned agent Without agent
PCI DSS
NIST 800-53
GDPR
HIPAA
TSC

Check List

  • All tests pass
    • yarn test:jest
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@asteriscos asteriscos changed the base branch from master to 4.4-2.4-wzd January 16, 2023 14:57
@asteriscos asteriscos self-assigned this Jan 16, 2023
@asteriscos asteriscos linked an issue Jan 16, 2023 that may be closed by this pull request
@asteriscos asteriscos marked this pull request as ready for review January 16, 2023 15:01
@asteriscos asteriscos requested a review from a team as a code owner January 16, 2023 15:01
yenienserrano
yenienserrano previously approved these changes Jan 17, 2023
Copy link
Member

@yenienserrano yenienserrano left a comment

Choose a reason for hiding this comment

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

Security Information Management

Module With pinned agent Without agent
Security events 🟢 🟢
Integrity monitoring 🟢 🟢
Office 365 🟢
Amazon AWS 🟢 🟢
Google Cloud Platform 🟢 🟢
GitHub 🟢 🟢

Auditing and Policy Monitoring

Module With pinned agent Without agent
Policy monitoring 🟢 🟢
Security configuration assessment
System auditing 🟢 🟢
OpenSCAP 🟢 🟢
CIS-CAT 🟢 🟢

Threat Detection and Response

Module With pinned agent Without agent
Vulnerabilities
MITRE ATT&CK
VirusTotal
Osquery
Docker listener

Regulatory Compliance

Module With pinned agent Without agent
PCI DSS
NIST 800-53
GDPR
HIPAA
TSC

@Tostti
Copy link
Member

Tostti commented Jan 17, 2023

Security Information Management

Module With pinned agent Without agent
Security events 🟢 🟢
Integrity monitoring 🟢 🟢
Office 365 🟢
Amazon AWS 🟢 🟢
Google Cloud Platform 🟢 🟢
GitHub 🟢 🟢

Auditing and Policy Monitoring

Module With pinned agent Without agent
Policy monitoring 🟢 🟢
Security configuration assessment
System auditing 🟢 🟢
OpenSCAP 🟢 🟢
CIS-CAT 🟢 🟢

Threat Detection and Response

Module With pinned agent Without agent
Vulnerabilities
MITRE ATT&CK 🟢 🟢
VirusTotal 🟢 🟢
Osquery 🟢 🟢
Docker listener 🟢 🟢

Regulatory Compliance

Module With pinned agent Without agent
PCI DSS 🟢 🟢
NIST 800-53 🟢 🟢
GDPR 🟢 🟢
HIPAA 🟢 🟢
TSC 🟢 🟢

Tostti
Tostti previously approved these changes Jan 17, 2023
Copy link
Member

@Tostti Tostti left a comment

Choose a reason for hiding this comment

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

TEST:✔️
CR:✔️

LGTM

@asteriscos
Copy link
Member Author

Important

I found out that the filters applied to the PDF report tables are not including the allowed_agents filtered when an agents RBAC policy is applied to the user.

Allowed Agents applied to the visualizations

Screenshot from 2023-01-18 11-59-30

Segregation of the Allowed Agents filter in the server-side controller

Screenshot from 2023-01-18 12-38-49

Screenshot from 2023-01-18 12-39-27

Final query filters

Screenshot from 2023-01-18 12-40-14

This mus be fixed in this PR

@asteriscos asteriscos dismissed stale reviews from Tostti and yenienserrano via 8d967b7 January 18, 2023 17:35
@asteriscos
Copy link
Member Author

Added allowed_agents filter to the following server-side queries

Audit requests:

  • getTop3AgentsSudoNonSuccessful
  • getTop3AgentsFailedSyscalls
  • getTopFailedSyscalls

GDPR requests:

  • topGDPRRequirements
  • getRulesByRequirement

Overview requests:

  • topLevel15

PCI requests:

  • topPCIRequirements
  • getRulesByRequirement

Rootcheck requests:

  • top5RootkitsDetected
  • agentsWithHiddenPids
  • agentsWithHiddenPorts

Alerts Summary tables:

  • alerts summary tables

Syscheck requests:

  • top3agents
  • top3Rules
  • lastTenDeletedFiles
  • lastTenModifiedFiles

TSC requests:

  • topTSCRequirements
  • getRulesByRequirement

Vulnerability requests:

  • topAgentCount
  • topCVECount
  • uniqueSeverityCount
  • topPackages
  • topPackagesWithCVE

These requests need to be included in the tests, with the allowed_agents policy and without it.
Each module PDF report data consistency must be checked.

@Desvelao
Copy link
Member

Desvelao commented Jan 23, 2023

Tests

Module With pinned agent Without agent Allowed agents with pinned agent Allowed agent without pinned agent
Security events 🟢 🟢 🟢 🟢
Integrity monitoring 🟢 🟢 🟢 🟢
Office 365 🟢 🟢
Amazon AWS 🟢 🟢 🟢 🟢
Google Cloud Platform 🟢 🟢 🟢 🟢
GitHub 🟢 🟢 🟢 🟢
Policy monitoring 🟢 🟢 🟢 🟢
Security configuration assessment
System auditing 🟢 🟢 🟢 🟢
OpenSCAP 🟢 🟢 🟢 🟢
CIS-CAT 🟢 🟢 🟢 🟢
Vulnerabilities
MITRE ATT&CK 🟢 🟢 🟢 🟢
VirusTotal 🟢 🟢 🟢 🟢
Osquery 🟢 🟢 🟢 🟢
Docker listener 🟢 🟢 🟢 🟢
PCI DSS 🟢 🟢 🟢 🟢
NIST 800-53 🟢 🟢 🟢 🟢
GDPR 🟢 🟢 🟢 🟢
HIPAA 🟢 🟢 🟢 🟢
TSC 🟢 🟢 🟢 🟢

@Desvelao
Copy link
Member

Desvelao commented Jan 23, 2023

I was researching the problem with the tests. There are some tests failing that are related to the checking the md5 of PDF reports.

I think the problem is the note for the allowed agents is added when is not the use case:
image

@asteriscos, could you review it?

EDIT: I think there is a problem with this conditional: https://github.com/wazuh/wazuh-kibana-app/pull/5131/files#diff-e344f646f1844581c2ba5159d750a8f075c476e56350e3bcc6018f87136dd973R344-R346. It is always truthy and the allowed note is added to the PDF module report.

@Tostti
Copy link
Member

Tostti commented Jan 23, 2023

Tests

Module With pinned agent Without agent Allowed agents with pinned agent Allowed agent without pinned agent
Security events 🟢 🟢 🟢 🟢
Integrity monitoring 🟢 🟢 🟢 🟢
Office 365 🟢 🟢
Amazon AWS 🟢 🟢 🟢 🟢
Google Cloud Platform 🟢 🟢 🟢 🟢
GitHub 🟢 🟢 🟢 🟢
Policy monitoring 🟢 🟢 🟢 🟢
Security configuration assessment
System auditing 🟢 🟢 🟢 🟢
OpenSCAP 🟢 🟢 🟢 🟢
CIS-CAT 🟢 🟢 🟢 🟢
Vulnerabilities
MITRE ATT&CK 🟢 🟢 🟢 🟢
VirusTotal 🟢 🟢 🟢 🟢
Osquery 🟢 🟢 🟢 🟢
Docker listener 🟢 🟢 🟢 🟢
PCI DSS 🟢 🟢 🟢 🟢
NIST 800-53 🟢 🟢 🟢 🟢
GDPR 🟢 🟢 🟢 🟢
HIPAA 🟢 🟢 🟢 🟢
TSC 🟢 🔴 🟢 🔴

🔴 TSC

When an agent is pinned, it works as expected and in Alerts summary only shows the alerts for that agent. However, if no agent is pinnned, it only shows alerts for one agent.
In my tests I was able to pin each agent individually and see the corresponding alerts, but without any agent pinned, I always see only the alerts for one of the agents.

Evidence

Agents

imagen

Without pinned agents, 9 pages and only showing agent 002 (4.4-2)

imagen
imagen

With agent 001 pinned, 9 pages and only showing agent 001 (4.4-3)

imagen
imagen

With agent 002 pinned, 9 pages and only showing agent 002 (4.4-2)

imagen
imagen

With agent 003 pinned , 9 pages and only showing agent 003 (4.4)

imagen
imagen

@Desvelao
Copy link
Member

Tests

Module With pinned agent Without agent Allowed agents with pinned agent Allowed agent without pinned agent
Security events
Integrity monitoring
Office 365
Amazon AWS
Google Cloud Platform
GitHub
Policy monitoring
Security configuration assessment
System auditing
OpenSCAP
CIS-CAT
Vulnerabilities
MITRE ATT&CK
VirusTotal
Osquery
Docker listener
PCI DSS
NIST 800-53
GDPR
HIPAA
TSC

TSC

When an agent is pinned, it works as expected and in Alerts summary only shows the alerts for that agent. However, if no agent is pinnned, it only shows alerts for one agent. In my tests I was able to pin each agent individually and see the corresponding alerts, but without any agent pinned, I always see only the alerts for one of the agents.

Evidence

Agents

imagen

Without pinned agents, 9 pages and only showing agent 002 (4.4-2)

imagen imagen

With agent 001 pinned, 9 pages and only showing agent 001 (4.4-3)

imagen imagen

With agent 002 pinned, 9 pages and only showing agent 002 (4.4-2)

imagen imagen

With agent 003 pinned , 9 pages and only showing agent 003 (4.4)

imagen imagen

I think that problem is related to the following issue: #5134

@github-actions
Copy link
Contributor

Code coverage (Jest) % values
Statements 8.76% ( 3237 / 36912 )
Branches 4.5% ( 1291 / 28673 )
Functions 7.63% ( 698 / 9143 )
Lines 8.83% ( 3122 / 35346 )

Copy link
Member

@Desvelao Desvelao left a comment

Choose a reason for hiding this comment

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

review:

code 🟢
test 🟢

I checked when the allowed agents should appear and it is working as expected. I verified the next cases:

case result
With pinned agent 🟢
Without pinned agent 🟢
With pinned agent and allowed agents 🟢
Without pinned agent and allowed agents 🟢

Copy link
Member

@Tostti Tostti left a comment

Choose a reason for hiding this comment

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

Checked when the allowed agents should appear and it is working as expected. Verified the next cases:

case result
With pinned agent 🟢
Without pinned agent 🟢
With pinned agent and allowed agents 🟢
Without pinned agent and allowed agents 🟢

TEST:✔️
CR:✔️

LGTM

@Desvelao Desvelao merged commit c540580 into 4.4-2.4-wzd Jan 25, 2023
@Desvelao Desvelao deleted the fix/alerts-summary-table-truncated-5130 branch January 25, 2023 08:49
@github-actions
Copy link
Contributor

The backport to 4.4-7.16 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.4-7.16 4.4-7.16
# Navigate to the new working tree
cd .worktrees/backport-4.4-7.16
# Create a new branch
git switch --create backport-5131-to-4.4-7.16
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c54058037361a724126d6bf8b7a6f6254e0a6670
# Push it to GitHub
git push --set-upstream origin backport-5131-to-4.4-7.16
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.4-7.16

Then, create a pull request where the base branch is 4.4-7.16 and the compare/head branch is backport-5131-to-4.4-7.16.

@github-actions
Copy link
Contributor

The backport to 4.4-7.10 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.4-7.10 4.4-7.10
# Navigate to the new working tree
cd .worktrees/backport-4.4-7.10
# Create a new branch
git switch --create backport-5131-to-4.4-7.10
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c54058037361a724126d6bf8b7a6f6254e0a6670
# Push it to GitHub
git push --set-upstream origin backport-5131-to-4.4-7.10
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.4-7.10

Then, create a pull request where the base branch is 4.4-7.10 and the compare/head branch is backport-5131-to-4.4-7.10.

Desvelao pushed a commit that referenced this pull request Jan 25, 2023
* Fix query result parsing algorithm

* Added changelog

* Add allowed_agents filter to the report queries

* Fix agentsFilter conditional

Co-authored-by: Antonio <34042064+Desvelao@users.noreply.github.com>
(cherry picked from commit c540580)
Desvelao pushed a commit that referenced this pull request Jan 25, 2023
* Fix query result parsing algorithm

* Added changelog

* Add allowed_agents filter to the report queries

* Fix agentsFilter conditional

Co-authored-by: Antonio <34042064+Desvelao@users.noreply.github.com>
(cherry picked from commit c540580)
asteriscos added a commit that referenced this pull request Jan 26, 2023
Fix Alerts Summary table truncated results (#5131)

* Fix query result parsing algorithm

* Added changelog

* Add allowed_agents filter to the report queries

* Fix agentsFilter conditional

Co-authored-by: Antonio <34042064+Desvelao@users.noreply.github.com>
(cherry picked from commit c540580)

Co-authored-by: Federico Rodriguez <federico.rodriguez@wazuh.com>
Desvelao added a commit that referenced this pull request Jan 27, 2023
Fix Alerts Summary table truncated results (#5131)

* Fix query result parsing algorithm

* Added changelog

* Add allowed_agents filter to the report queries

* Fix agentsFilter conditional

Co-authored-by: Antonio <34042064+Desvelao@users.noreply.github.com>
(cherry picked from commit c540580)

Co-authored-by: Federico Rodriguez <federico.rodriguez@wazuh.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alerts Summary table results truncated
4 participants