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.

Sign up for GitHub

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

Jump to bottom

[Security Solution] add blocklist list #126390

Merged
merged 1 commit into from
Mar 1, 2022
Merged

[Security Solution] add blocklist list #126390

merged 1 commit into from
Mar 1, 2022

Conversation

joeypoon
Copy link
Member

@joeypoon joeypoon commented Feb 24, 2022 •
edited
Loading

Summary

Add blocklist list
Screen Shot 2022-02-24 at 5 30 22 PM

For maintainers

@joeypoon joeypoon added Team:Defend Workflows “EDR Workflows” sub-team of Security Solution release_note:feature Makes this part of the condensed release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.2.0 labels Feb 24, 2022
@joeypoon joeypoon force-pushed the feature/blocklist-list branch 4 times, most recently from ac9430d to bca3591 Compare February 25, 2022 00:13
@joeypoon joeypoon marked this pull request as ready for review February 28, 2022 15:17
@joeypoon joeypoon requested review from a team as code owners February 28, 2022 15:17
@joeypoon joeypoon requested review from pzl and dasansol92 February 28, 2022 15:17
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

@joeypoon joeypoon force-pushed the feature/blocklist-list branch from 9b4f5ea to 0e629bd Compare February 28, 2022 22:38
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2879 2881 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/securitysolution-list-constants 9 12 +3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lists 140.7KB 140.8KB +69.0B
securitySolution 4.7MB 4.7MB +1.1KB
total +1.2KB
Unknown metric groups

API count

id before after diff
@kbn/securitysolution-list-constants 23 26 +3

History

  • 💚 Build #26429 succeeded 9b4f5eaf9891ddac832d891ad5c7698ccadd69b3
  • 💔 Build #26367 failed bca35917a7a7de5689efbdde6785b162b43043ff
  • 💔 Build #26360 failed ac9430db0c79fa492e254a0a5f424a2fa1ab5c26
  • 💔 Build #26332 failed f892cefba52b52f64a6890b112944ed0777b559d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

paul-tavares
paul-tavares approved these changes Mar 1, 2022
View reviewed changes
Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

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

Left a few comments, but nothing that needs to be done in this PR since we're trying to get this in so others are unblocked. You can address it in a subsequent one

🚢 it

.../plugins/security_solution/common/endpoint/data_generators/exceptions_list_item_generator.ts
return this.generate({
name: `Blocklist ${this.randomString(5)}`,
list_id: ENDPOINT_BLOCKLISTS_LIST_ID,
item_id: `generator_endpoint_blocklist_${this.randomUUID()}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

can you use the .seededUUIDv4() method instead so that we get deterministic output based on the seed used to insatiate the class instance

x-pack/plugins/security_solution/public/management/pages/blocklist/view/blocklist.tsx
@@ -39,7 +39,7 @@ const BLOCKLIST_PAGE_LABELS: ArtifactListPageProps['labels'] = {
defaultMessage: 'Blocklist',
}),
pageAboutInfo: i18n.translate('xpack.securitySolution.blocklist.pageAboutInfo', {
defaultMessage: '(DEV: temporarily using isolation exception api)', // FIXME: need wording from PM
defaultMessage: 'Add a blocklist to block applications or files from running.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add endpoint to the end of this sentence?

Suggested change
defaultMessage: 'Add a blocklist to block applications or files from running.',
defaultMessage: 'Add a blocklist to block applications or files from running on the endpoint.',

x-pack/plugins/security_solution/public/management/pages/blocklist/constants.ts

export const BLOCKLISTS_LIST_TYPE: ExceptionListType = ExceptionListTypeEnum.ENDPOINT_BLOCKLISTS;

export const BLOCKLISTS_LIST_DEFINITION: CreateExceptionListSchema = {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is one (maybe two) more places that need to be updated with the list id. Please add it to: ALL_ENDPOINT_ARTIFACT_LIST_IDS const in x-pack/plugins/security_solution/common/endpoint/service/artifacts/constants.ts:18.

@dasansol92 I think there is a place on the server side too that needs updating for the "delete" policy use case (fleet server extension) ??

Copy link
Contributor

@dasansol92 dasansol92 Mar 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Right, it's in x-pack/plugins/security_solution/server/fleet_integration/handlers/remove_policy_from_artifacts.ts:18 but I think we can import there the one in x-pack/plugins/security_solution/common/endpoint/service/artifacts/constants.ts and remove the duplication

dasansol92
dasansol92 reviewed Mar 1, 2022
View reviewed changes
x-pack/plugins/security_solution/public/management/pages/blocklist/services/index.ts
* 2.0.
*/

export * from './blocklists_api_client';
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm wrong but I think we had some issues with the wildcard export in the past, if it's not needed (I don't think so), can we export just what we want?

@joeypoon joeypoon merged commit 4e3d2f6 into elastic:main Mar 1, 2022
@joeypoon joeypoon deleted the feature/blocklist-list branch March 1, 2022 15:46
@kibanamachine
Copy link
Contributor

💔 Backport failed

The pull request could not be backported due to the following error:
There are no branches to backport to. Aborting.

How to fix

Re-run the backport manually:

node scripts/backport --pr 126390

Questions ?

Please refer to the Backport tool documentation

@joeypoon joeypoon mentioned this pull request Mar 1, 2022
Merged
1 task
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Mar 3, 2022
@ashokaditya
update blocklist generator to add random os entry
dc59195 
refs elastic/pull/126390
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126390 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 3, 2022
ashokaditya added a commit that referenced this pull request Mar 4, 2022
@ashokaditya @kibanamachine
[Security Solution][Endpoint] Generate blocklist artifacts (#126517)
dd93f0c 
* Generate blocklist artifacts

fixes elastic/security-team/issues/2783

* update tests to include event filters, host isolation exceptions and blocklists

fixes elastic/security-team/issues/2783

* todo comment

* fix typo

* Unify artifact kuery method into one

Since the os specific filter and policy filter strings are same for trusted apps, event filters, host isolation exceptions and blocklists, it makes sense to unify these into a single method that accepts a listId param to distinguish each artifact. Morever, since endpoint list does not need specific policy id for a policy filter, this can also be unified into the same method.

fixes elastic/security-team/issues/2783

* update blocklist generator to add random os entry

refs /pull/126390

* Move entries back in `ExceptionsListItemGenerator`

review changes

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126390 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126390 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126390 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126390 or prevent reminders by adding the backport:skip label.

2 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126390 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 126390 or prevent reminders by adding the backport:skip label.

@paul-tavares paul-tavares added the backport:skip This commit does not require backporting label Mar 14, 2022
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 14, 2022
@joepeeples joepeeples mentioned this pull request Apr 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashokaditya ashokaditya ashokaditya left review comments

@dasansol92 dasansol92 dasansol92 left review comments

@pzl pzl pzl approved these changes

@paul-tavares paul-tavares paul-tavares approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed backport:skip This commit does not require backporting release_note:feature Makes this part of the condensed release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@joeypoon @elasticmachine @kibana-ci @kibanamachine @pzl @ashokaditya @dasansol92 @paul-tavares