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

[FR] NON_DATASET_PACKAGE list & Data Source tag for Auditd_manager #3430

Merged
merged 7 commits into from
Feb 19, 2024

Conversation

Aegrah
Copy link
Contributor

@Aegrah Aegrah commented Feb 6, 2024

Summary

While tuning, one of the goals I have is to maximize index pattern coverage for all of our Linux rules, as I noticed many of the Linux rules I pushed don't have endgame support. Additionally, I noticed while doing my auditd_manager research that adding auditd_manager support is also possible for most exec events. Doing this would allow for additional rule coverage. However, when adding this support, I need to add auditd_manager to the integrations list, and once it is in there, unit testing forces queries to have the event.dataset == "auditd_manager.auditd" set.

To tackle this, I propose adding auditd_manager to the NON_DATASET_PACKAGE list, and add a new tag "Data Source: Auditd Manager". Given that we now have quite a few auditd_manager-related rules, and the list will continue to grow, it makes sense for it to have its own tag.

Test

Running python -m detection_rules view-rule rules/linux/defense_evasion_disable_selinux_attempt.toml after adding auditd_manager to the integration, and supplying the correct logs-auditd_manager.auditd-*, results in the dynamic population of the related_integrations:

"related_integrations": [
    {
      "package": "endpoint",
      "version": "^8.2.0"
    },
    {
      "package": "auditd_manager",
      "version": "^1.0.0"
    }
  ]

Additional information

Issue for this request is created in #3429
Issue for tuning: #3428

@Aegrah Aegrah added enhancement New feature or request Area: DED Team: TRADE labels Feb 6, 2024
@Aegrah Aegrah self-assigned this Feb 6, 2024
@botelastic botelastic bot added python Internal python for the repository schema labels Feb 6, 2024
@Mikaayenson
Copy link
Contributor

@Aegrah Is auditd_manager not a real integration? If so we should update the manifest and pull the integration's schema.

@Aegrah
Copy link
Contributor Author

Aegrah commented Feb 8, 2024

@Mikaayenson I discussed this with @terrancedejesus before pushing this PR and we figured we don't have to update the mappings because no new fields are introduced that are not yet added to the mappings files earlier. Once unknown fields are added to the query, we should update.

If I don't fully understand the workflow, and it should be done now, LMK why exactly and I will update the schemas/manifests.

Copy link
Contributor

@Mikaayenson Mikaayenson left a comment

Choose a reason for hiding this comment

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

This makes sense. If new fields are not found that belong to the integration, then we'll want to revert the definitions change and update the schemas.

@terrancedejesus
Copy link
Contributor

I'd like the opportunity to triage this a bit and make sure we are not breaking anything potentially with these changes. There are some nuances with related integrations build time logic and backporting I need to check. Will post an update in this comment when finished.

@terrancedejesus
Copy link
Contributor

Couple of notes so far while digging into this:

  • What is the difference between auditd and auditd_manager integrations as they are separate?
  • If we add auditd_manager to NON_DATASET_PACKAGES we should remove event.dataset == "auditd_manager.auditd" from the queries. If not we will have duplicate related_integrations entries as shown below. This is because of this logic where we automatically add the package, versus relying on event.dataset from the query as done here. If both exist, then both will be added.
  "related_integrations": [
    {
      "package": "auditd_manager",
      "version": "^1.0.0"
    },
    {
      "integration": "auditd",
      "package": "auditd_manager",
      "version": "^1.0.0"
    }
  • I double checked the package schema identification for auditd_manager since the schema naming is auditd.file for example, which typically requires us to specify the policy template, hence event.dataset == auditd_manager.auditd, however we do not need to do this for this integration. We also shouldn't have any issues if for instance auditd is used as the names are unique to each when finding the package schemas and either way the fields will be validated correctly.
  • Backporting...I assume it will be fine, but we often run into issues when backporting and build time fields diverge in content, often requiring us to up the min-stack. However, since this is change is deterministic based on changes in our repo and not upstream, we should be fine. When we lock versions, we may have forked versions have a version bump which is fine because of the buffers. My concern would be back-and-fourth states between branches...but I believe we avoid that here.

Copy link
Contributor

@terrancedejesus terrancedejesus left a comment

Choose a reason for hiding this comment

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

As discussed with DED, we are good to merge this. However, we should immediately reconcile the rule queries in a separate PR to remove event.dataset.

@Aegrah Aegrah merged commit a637bce into main Feb 19, 2024
13 checks passed
@Aegrah Aegrah deleted the FR-auditd_manager-tag_and_exception branch February 19, 2024 08:37
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
protectionsmachine pushed a commit that referenced this pull request Feb 19, 2024
…3430)

* [FR] Add Auditd_Manager to NON_DATASET_PACKAGE

* Changed alphabetical order

---------

Co-authored-by: Mika Ayenson <Mikaayenson@users.noreply.github.com>
Co-authored-by: Terrance DeJesus <99630311+terrancedejesus@users.noreply.github.com>

(cherry picked from commit a637bce)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport: auto enhancement New feature or request python Internal python for the repository schema Team: TRADE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] event.dataset required for Auditd Manager integration rules
3 participants