-
Notifications
You must be signed in to change notification settings - Fork 513
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] Add source_updated_at
to Rule Schema as a Build Time Field
#2826
Comments
…158450) Addresses: #154614 #154615 Figma designs: https://www.figma.com/file/gLHm8LpTtSkAUQHrkG3RHU/%5B8.7%5D-%5BRules%5D-Rule-Immutability%2FCustomization?type=design&node-id=2935-577576&t=ziqgnlEJBpowqa7F-0 ## Summary - Removes `prebuiltRulesNewUpgradeAndInstallationWorkflowsEnabled` feature flag. All new prebuilt endpoints now available by default. - Creates the UI for the new **rules installation** and **rules upgrade** workflows. - Creates new **Add Rules** page, which lists rules available for installation. - Creates new **Rule Updates** page, which lists rules which have available updates. - Creates new, separate contexts for the **Add Rules** and the **Rule Updates** page, and the hooks to use them (`useAddPrebuiltRulesTableContext` and `useUpgradePrebuiltRulesTableContext` respectively) - Creates prebuilt rule hooks, which consume new endpoints: - `useFetchPrebuiltRulesStatusQuery` and `usePrebuiltRulesStatus` consume the `/internal/detection_engine/prebuilt_rules/status` endpoint and provide information about number of rules available for installation, number of installed rules, and number of rules with available updates. - `useFetchPrebuiltRulesInstallReviewQuery` and `usePrebuiltRulesInstallReview` consume the `/internal/detection_engine/prebuilt_rules/installation/_review` endpoint and return the rules available for installation which are listed in the **Add Rules** page. - `useFetchPrebuiltRulesUpgradeReviewQuery` and `usePrebuiltRulesUpgradeReview` consume the `/internal/detection_engine/prebuilt_rules/upgrade/_review` endpoint and return the rules which have available updates, and are listed in the **Rule Updates** page. - `usePerformInstallAllRules`, `usePerformInstallSpecificRules`, and its respective mutation hooks `usePerformAllRulesInstallMutation` and `usePerformSpecificRulesInstallMutation` consume the `/internal/detection_engine/prebuilt_rules/upgrade/_perform` endpoint in order to install rules. - `usePerformUpgradeAllRules`, `usePerformUpgradeSpecificRules` and its respective mutation hooks `usePerformAllRulesUpgradeMutation` and `usePerformSpecificRulesUpgradeMutation` consume the `/internal/detection_engine/prebuilt_rules/upgrade/_perform` endpoint in order to upgrade rules. ### Deprecated code **Hooks:** - `useCreatePrebuiltRulesMutation` - `useInstallPrePackagedRules` - `useCreatePrePackagedRules` - `usePrePackagedRulesInstallationStatus` - `usePrePackagedTimelinesInstallationStatus` ### Major points to resolve - **Timeline templates installation**: Since this PR stops using the `/api/detection_engine/rules/prepackaged` endpoint in favour of the new ones, we are not currently installing timeline templates. Serverside, we will need a new endpoint to install them separately from rules? In the UI, how would this still work: would they get installed in the background now? Or maybe have a new button for it somewhere? - **ML Jobs warning**: when updating rules, we currently have a wrapper to add confirmation modal for users who may be running older ML Jobs that would be overridden by updating their rules. This PR removes that code, but we'll need to reintroduce it for the cases of: upgrading single rules, upgrading a selection of rules, upgrading all rules. ### Deviations from design This PR includes a reduced scope to the final workflow shown in the Figma designs. Most notably, in Milestone 2, to be released in 8.9, we did not build the flyout that, in the Add Rules page, shows the rule details when the user clicks on it, so the user can review it before installing. The same is true in the Rule Updates table, which does not allow, for now, reviewing the rules. In both cases, the user can only click in "Install Rule" and "Upgrade Rule". There are other differences in the UI, for technical reasons: - Both for the Add Rules page and the Rule Updates table we decided to use **EUI's InMemoryTable**. Since the endpoint that return the data to populate both of these tables do not allow for sorting, filtering and paging, we decided to use the InMemoryTable for both cases, as all of those functions are handled out-of-the-box by the EUI component. The relatively low number of items that populate these tables means that we won't face significant performance issues. However, this meant a number of deviations from the designs: - Since filtering, sorting and pagination are handled by the table, the contexts for these table do not includes any internal state relating to these functions. This makes it hard to recreate the RuleUtilityBar for each of these components or make the existing one reusable. We therefore decided to leave the Utility Bar for the new two tables out of scope, and deviate from the design by moving the button that the user can click on o install or upgrade the selected rules to beside the "Install all" or "Upgrade all" buttons. This button is shown only when at least one rule of the table is selected. - The **tags filter box** that comes out-of-the-box with the InMemoryTable can only be positioned to the right of the search bar, instead of the left like we have in our main **Installed Rules** table. Also, clicking on the tabs adds the text to the search bar, and the box does not allow for negative selection of tabs (exclusion). - The search bar filters on keystroke rather than on Enter. This behaviour can be changed, but it feels more useful than the other behaviour for these new two tables. - The search bar filters by searching the user's input in any of the string properties of first order within the rule object. This means that the search bar can be used to look up rules according to their name, description, rule_id, etc (but not for example for MITRE techniques, which are an object.) This behaviour, however, is also customisable. - Neither the Add Rules table nor the Rule Updates tables display the _Last updated_ column which is shown in the design. Since the original intent of the designers is to show when the rule asset (`security-rule`) was created or updated, this is information we don't currently have within the SO. After discussion with @ksevasilyeva and @ARWNightingale, we decided, for now, to remove the column. In the meantime, @terrancedejesus [created an issue to include `createdAt` and `updatedAt` fields](elastic/detection-rules#2826) within the rule assets, that we can use to display in the table in later iterations. #### Other remaining work: - Introduce confirmation modals when the user clicks on the "Install all" or "Upgrade all" modal. - Unit testing for new hooks and components. - Other component redesign: Rule Filter, Tag Filter #### How to test rule upgrade 1. Have at least one rule installed 2. Find its `rule_id` from the Network tab. 3. Make a request to `PATCH /api/detection_engine/rules` with the `rule_id` in the payload, and also set the `version` to a number lower than the current version. 4. Reload the page. 5. The `/upgrade/_review` endpoint will now return that rule as available for upgrade. ### Videos #### Rule Installation Workflow https://github.com/elastic/kibana/assets/5354282/5a219625-beb1-48ee-a9fc-ff48b69eeae0 #### Rule Upgrade Workflow https://github.com/elastic/kibana/assets/5354282/b5f3c23b-004a-462c-bbdd-ed04321f5ce7 ### TODO - [x] Align copy, use "update" instead of "upgrade" - [ ] Persist user's choice when they dismiss the upgrade/install rules callouts till the next package release (create a separate task for that) - [ ] Unify table controls (search bar and tags), use the ones we have on the rules management table - [ ] After rule installation, adjust copy, and display that all available rules have been installed. Add a "Go Back" CTA - [ ] Add links from the available rules table to docs - [ ] Rule severity sorting should take semantics into consideration --------- Co-authored-by: Dmitrii <[email protected]> Co-authored-by: Dmitrii Shevchenko <[email protected]> Co-authored-by: Sergi Massaneda <[email protected]>
UpdateWe have some considerations with adding this. These fields are currently in the rule meta since they do not matter for the SHA256 hash calculation. As a result, typically anything we add to the hash calculation should be moved into the rule data itself and have validation done on the values. Options:
Either way we need to add validation to these field value pairs to keep the date values consistent. @Mikaayenson @eric-forte-elastic @brokensound77 - Any additional thoughts? |
I do not see an issue with this approach/solution 👍 Just as a note, we will need to update unit tests to also have def _package_kibana_index_file(self, save_dir):
"""Convert and save index file with package."""
sorted_rules = sorted(self.rules, key=lambda k: (k.contents.metadata.creation_date, os.path.basename(k.path))) None of these should be an issue as the functions/tests have access to the |
@terrancedejesus Can you explain why we want to make the change to move these fields at all (or even add to the build)? Was it requested upstream? |
Requested upstream from @jpdjere for UI regarding rule update workflow. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@jpdjere is this still a request from your team? If so, I'd like to get it correctly scoped for one of our upcoming sprints. Thank you! |
Hi @terrancedejesus , yes this is still a valid request. We won't be working on anything that needs this data for 8.11 but probably 8.12 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Hi @terrancedejesus . Do you have bandwidth for this in any upcoming releases? |
@jpdjere - Thanks for the follow up! I added this to our teams next sprint cycle which starts Nov 27. With recent adjustments to our current sprint cycle, I will attempt to get started with this to determine if it is relatively straight forward and if so will have it in earlier. |
Hello @jpdjere 👋 , Can you provide more context for this request? We are just trying to understand the reasoning and whether this is the best representation of this information. Right now, since we do not push this with the rules, the dates are pulled from kibana:
This is reflective of the rules as they apply to a users stack, which seems accurate and informative. Our dev cycle creates situations where rules may not be released for a few days or weeks after modification, so there is inconsistency that may cause confusion. More so, I think it may be more valuable understanding when a rule was created and modified within a stack vs when it was developed. These fields under our metadata are currently used as a means to inform us on changes from a maintenance perspective. Thoughts? |
Hi @brokensound77 . Thanks for the follow up. The idea behind this request is to give the users an idea of how "recent" the updates to a specific rule are, in order to know how long those specific updates for the rule have been pending. Here's a screenshot of the UI as proposed by our designers: This becomes especially important when the user has neglected addressing updates from one (or many) package releases, and after some considerable amount of time sees in our Rule Update table a list of rule updates corresponding to more than one releases. For example, a user seeing this table in October could see listed 4 rules that have updates coming from a package release made in March (so their (Sorry if this timelines don't make sense, I don't have in my mind right now what is your release cadence). Having said that:
I think that's a valid concern that can cause confusion to users, given the false impression that updates have been pending for a long time, when the rule updates have just been released. A couple questions:
(This second question, I think, is not that important considering that the user might accumulate updates from many subsequent releases, but it's good to know). |
@jpdjere - Apologies for the questions going unanswered.
We release OOB updates bi-weekly. Therefore, updated dated discrepancy could be 1-14 days apart as it would depend on the pull request merge and when the package reaches EPR. This could be expanded more if the release takes longer than expected, but is a rare occurrence.
Any time a rule is updated, the updated date value is updated. Therefore it could be any date between when the last package was available in EPR to the date when the next package is available in EPR. Again, we release bi-weekly so there is an approximate range of ~14 different dates that could apply. |
Update 01/08/2023We are moving forward with this as it is a requirement upstream for customizing prebuilt detection rules, milestone 3. Below are considerations:
@jpdjere or @banderror - Can you provide insight to the following for us. Thank you in advance!
|
Option 1 - In this option, we rely on
Commit Reference: 3bc8df6 |
Option 2 - We only add Commit Reference: 0402dc2 |
Option 3 - We move the date fields from |
Option 4 - Similar to option 2, only instead of using metadata, is there any reason why we can't use the date of the release (almost like a build time field)? From the description, it doesn't seem necessary to have the exact date the rule was modified by our rule authors. |
@Mikaayenson Great alternative with a few caveats. Our source-of-truth is typically the repository since that is where we lock versions. Let's say we lock versions and a rule has changes that cause the SHA256 to change. This "state" of the rule is only noticed during the lock versions, which we also release our commits from. Technically, up until this version lock, our rule could go through several changes and updates, but it is only when we lock versions do we track the current state of the rule. The last We also have to take into consideration release timing. Releases could take 1-2 days, thus the potential for a divergence of dynamic dates based on building the package could occur not only from the version lock, but also between each package. All packages would have to be released GA on the same day for them to accurately reflect the same updated date. |
@terrancedejesus Sorry for the delay in replying.
Yes, we understand this will be the case. We will add
We strongly prefer to have the Would this be fine by you as well? Or would it have side-effects in hashing, etc? Sorry for the confusion in the discussion above, where we talked about "root-level". |
@jpdjere - Thanks for the reply!
No problem with us, easy to adjust and thank you for clarification. Here would be an updated rule asset, does this work? Example
|
Great, thanks a lot! 👍 Yes, that looks good. Just a nit - wanted to make sure that the date format is ISO 8601 with the UTC format; the example above is missing the miliseconds and the Z at the end: |
@jpdjere - Thanks for responding.
The milliseconds and Since we are adding From your image, it looks like the dates are separate from the actual security rule, therefore we are simply providing a way for you to retrieve this date when assets are shipped? @brokensound77 - Am I missing the point here or questions we discussed? |
That's OK, the information about year, month and day is enough for us. So dates that look like
We will be adding the
Yes, it will, as part of the
Those dates you see in the image above not the That's why the |
@jpdjere - Thank you for providing additional insights.
With this being said, if it is part of the rule schema, required or not, it is a breaking change for us because of backporting. We will need to change our approach and add this to our rule schema, rather than dynamically populate and push into the rule asset. @brokensound77 - With this field being optional, I think it would be best to be a build time field, determined from rule metadata that we can only build for the compatible semantic version of the stack the feature is being added to. Regarding backporting, this will cause ALL of our rules to receive version bumps, for each release package. We have done this before, so I can get started on our strategy to implement this. Before I do, any additional thoughts? |
Hey @jpdjere @terrancedejesus @Mikaayenson 👋 So there were lots of comments in this thread, and I'd like to double-check that after all these comments we're on the same page. Let me try to reiterate on our agreements and please correct me or add anything. We're going to add a new optional field The latest 111 version of this rule looks like this in the package: {
"type": "security-rule",
"id": "52376a86-ee86-4967-97ae-1a05f55816f0",
"attributes": {
"rule_id": "52376a86-ee86-4967-97ae-1a05f55816f0",
"name": "Linux Restricted Shell Breakout via Linux Binary(s)",
"description": "Identifies the abuse of a Linux binary to break out of a restricted shell or environment by spawning an interactive system shell. The activity of spawning a shell from a binary is not common behavior for a user or system administrator, and may indicate an attempt to evade detection, increase capabilities or enhance the stability of an adversary.",
"type": "eql",
"language": "eql",
"index": ["logs-endpoint.events.*"],
// other rule fields...
"version": 111
}
} The next 112 version should look like that: {
"type": "security-rule",
"id": "52376a86-ee86-4967-97ae-1a05f55816f0",
"attributes": {
"rule_id": "52376a86-ee86-4967-97ae-1a05f55816f0",
"name": "Linux Restricted Shell Breakout via Linux Binary(s)",
"description": "Identifies the abuse of a Linux binary to break out of a restricted shell or environment by spawning an interactive system shell. The activity of spawning a shell from a binary is not common behavior for a user or system administrator, and may indicate an attempt to evade detection, increase capabilities or enhance the stability of an adversary.",
"type": "eql",
"language": "eql",
"index": ["logs-endpoint.events.*"],
// other rule fields...
"version": 112,
"elastic_update_date": "2024-01-29T00:00:00.000Z"
}
} This field will be optional in our rule asset schema in Kibana. The field should be specified for all latest versions of all rules in the next version of the package for Kibana 8.13. The field can be omitted for all existing historical (previous) versions of rules as of today, but should be specified for all historical rule versions created after today in the future. For example, for the The field's value must be formatted in the standard ISO format. Time of the day is not required and can be set to The new field must not be backported to any packages compatible with Kibana |
creation_date
and updated_date
to rule objects in release packageselastic_last_update
to Rule Schema as a Build Time Field
Hey @terrancedejesus @Mikaayenson, last ask from our side: let's please change the name of the field to After chatting with @jpdjere we figured we want the name to be resilient to hypothetical future capabilities in Kibana, such as user- or community-created packages with |
Alright so I did a bit of digging.
This gets me to the real problem and that is how we backport and version lock. As I attempt to showcase in the image below whenever we have a new field that is applied to all rules, optional or not, our versioning strategy does not do a good job of supporting this because the version is checked per backport branch where the SHA256 hashes are calculated. If these are different, then the version bumps +1. The important part to understand is that, in this example, in 8.11 a rule will not have The only option at this time would be to min-stack ALL rules to 8.12 so any updates, tunings, new rules would only go back to 8.12 stacks which is out of sync with our current supported stacks current-3, therefore this is a breaking change as @brokensound77 has stated. While we have introduced breaking changes before regarding this, it seems like a lot of breaking for a timestamp we can supply in metadata when shipping the rule asset to avoid breaking our backporting and versioning. |
@Mikaayenson DED has an epic or meta somewhere for refactoring Detection Rules. May be worth exploring the schema for version lock file(s) in Detection Rules. I believe there is some resilience that can be added with a couple of options:
Remember that when we build a package per stack version, we build it from that branch specifically so we could align that with its own state of the rules for that branch somehow.
|
@terrancedejesus I'm thinking about how to simplify this for you. Can we do this:
|
elastic_last_update
to Rule Schema as a Build Time Fieldsource_update_at
to Rule Schema as a Build Time Field
source_update_at
to Rule Schema as a Build Time Fieldsource_updated_at
to Rule Schema as a Build Time Field
After discussion with @Mikaayenson...there were a couple options we wanted to explore to hopefully get this in on our end to not be a blocker for @banderror 's team. The final proposal, as shown in the pull request, is to do the following and address each concern: We need to align our schemas with upstream
We do not want rule authors duplicating the update date that already exists in rule metadata
We need to consider versioning as this is likely a breaking change requiring min-stack updates to 8.12 across all rules
We need to remain consistent across our code and not introduce anything new that has to be managed
NOTE I want to emphasize that we should not always revert to adding new build time fields here. For instance, We need to ensure unit tests exist or are adjusted to accommodate our changes
|
@terrancedejesus @Mikaayenson Copying this from slack: We haven’t worked on adding support for I guess it's not a big difference so this shouldn't block you from working on some implementation of this field, but please hold off merging anything until we approve the proposal on our side and get an approval from your side. |
We need to coordinate this addition with the TRaDE team (elastic/detection-rules#2826).
@banderror Feel free to ping when you're ready to pick this back up and we'll try to resource/prioritize it. |
Is your feature request related to a problem? Please describe.
No.
Describe the solution you'd like
Add
creation_date
andupdated_date
to rule objects when a release package is created.Additional context
When we build a rules release package, all rule objects should have a
creation_date
andupdated_date
field in them. This will be used by Kibana for the updates review workflow.@jpdjere @approksiu
Dev branch: https://github.com/elastic/detection-rules/tree/fr-add-dates-to-rule-data
The text was updated successfully, but these errors were encountered: