[Security Solution][Detections] Allows rules to be updated from EPR

[rw-access]

Summary

Allow the rules to be updated from EPR, or from the file system if EPR can't be reached.

The general workflow for the best-effort rules update mechanism:

• When the status API is checked for the first time, the function checkAndStageUpdate() is run in the background, non-blocking, to check if EPR is reachable and there are any updates available. It will return the rules compiled in from the filesystem (rawRules)
• If EPR is reachable and there is an update available, it is downloaded and the version (checkAndStageUpdate) and decoded rules (latestRulesDownload) are stored in memory
• Upon subsequent checks to the status API, whenever that it is, it'll see if there's an update already downloaded. If there is, then that will be used. If not, then it will use the rules in the file system
• Every check to the _status API will re-trigger the checkAndStageUpdate() in the background. We could add a simple TTL in a global var to throttle the call to fleet's fetchFindLatestPackage. Or we could encourage fleet to do the same
• When returning the status of prepackaged rules via _status, a new rules_package_version field is added which returns the package version to the front-end, if it is being used for reference. This is to make sure that if a user consents to one package update (you have X rules to update or install), we don't install a newer package when they click the button.
• Downgrades still never happen, so we don't have to worry about Kibana going offline and trying to downgrade from EPR to the FS

Not done yet:

• Timelines are currently out of scope. I'm thinking that they should be shipped with the package too?
• No extra testing for the EPR component. I'm not totally sure the best way to do this
• Lint errors, and I'm using internal fleet APIs, so there's a little bit of plumbing there
• Deprecation of rules

I'm using elastic/package-storage#843 to test out EPR.

Checklist

Delete any items that are not applicable to this PR.

• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[FrankHassanabad]

I would do a return process.env.ENABLE_REGISTRY_RULES || false here and then we will have it by default turned off. In your local .bashrc/.zshrc we can flip it to true.

[FrankHassanabad]

Oh wait, never mind, I see what you're doing here. You're using the config globally and then flipping it here from the plugin. You can do this for now, I'm fine with it. Usually we push the config object with the feature flags in the REST routes. I will see if I can give a pointer there for you, but this is fine right now as written. It will work out.

[rw-access]

so entirely remove it from the config then?
followed by a change from L31-L39, removing this function and doing this instead?

const usePackageRegistryRules = process.env.ENABLE_REGISTRY_RULES || false;

edit -- oh i was delayed. okay I'll leave it as is unless you have an update or stronger suggestion

[FrankHassanabad]

I think you can just do this:

  const rulePromises = rulePaths.map<AddPrepackagedRulesSchema>((path) => {
    return JSON.parse(getAsset(path).toString('utf8'));
  });

  return validateAllPrepackagedRules(rulePromises);

Not seeing anything with Promises here going on.

[rw-access]

4311ef4

[FrankHassanabad]

usually constants are capital letters screaming such as DETECTION_RULES_PACKAGE_NAME = 'detection_rules' as a convention or they're just lowercase like detectionRulesPackageName.

We have a global constants file that is used front and backend here:

x-pack/plugins/security_solution/common/constants.ts

But some people prefer to put their constants in a file called constants.ts that is local and next to their file if they don't think it will be shared outside of their code. I'm fine with constants local to a file like you have it or any of the above. Putting it in the common section will add more bytes to the plugin downloaded to the front end so it might be better to keep it local to the file or just in another constants.ts next to this one.

Context fwiw.

[rw-access]

doh! yeah, I had a moment from Go, which yells at you if you have a global constant that doesn't follow the same naming convention as as a variable (it's dumb).

I added a constants.ts next to this file and moved this there
b301d4a

[FrankHassanabad]

I would remove this commented out line and put in its place:

TODO: Push down the logger to give a warning that this should warn about errors seen during an attempted download

[FrankHassanabad]

You'll probably have to get someone from the epm team to also give you a LGTM, but looking below I think they might want it like so (from their patterns):

export type AssetType =
  | KibanaAssetType
  | ElasticsearchAssetType
  | ValueOf<AgentAssetType>
  | RuleAssetType;

/*
 Enum mapping of a rules saved object asset type
*/
export enum RuleAssetType {
  rules = 'rules',
}

(caveat, I haven't tested that, just coming off the top of my head)

[FrankHassanabad]

You'll have to mess with these types and figure out how to make the body optional. I am pretty sure the way to do that is to make the type basically a union with null so that when you have request.body the body value can be a null value.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 11a0e05
• Storybooks Preview

Metrics [docs]

Async chunks

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

id	before	after	diff
fleet	754.6KB	754.6KB	+24.0B
securitySolution	7.7MB	7.7MB	+462.0B
total			+486.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
securitySolution	234.8KB	235.0KB	+211.0B

History

• 💔 Build #108852 failed b4c2267

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

[elasticmachine]

Pinging @elastic/security-detections-response (Team:Detections and Resp)

[FrankHassanabad]

We did a joint review and this is experimental and behind a feature flag to get the code in quick enough.

There will be more than likely a few followups from detections team members and/or others for additions such as:

• FTR e2e structural tests to test the new version that gets pushes down in the REST routes so that it cannot be broken by others doing refactoring like the other REST endpoints have those tests already.
• jest unit tests for the globals and possibly slight tweaks to the functions to make unit testing those parts simple.

Longer term tests might involve fleet and EPM within the e2e FTR's at a later time. Last note is that when we remove the feature flag to prefer/always use online rules we might have to tweak/change the existing tests as they are written within the pre-packaged rules and the cypress.

None of this feels like big issues. Just minor stuff.

[elasticmachine]

Pinging @elastic/fleet (Team:Fleet)

[ruflin]

@rw-access Could we convert this PR to a draft to make sure it does not get merged before all the changes are made in the package-spec? Spec should always go in first to make sure all parts in the stack are aligned.

[nchaulet]

I have some concerns on how the installation is working here, here the security plugin is going to use directly the package from the registry and installing the package will not really install anything, I am wondering if we should have a mechanism where the security plugin do not has to deal with the package.

If the security rules are a saved object for example, the fleet plugin could create the rules during install and the security plugin will consume that saved object.

@jfsiii I would love to have your thought on that

[ruflin]

A more generic question here: How will this work if lets say I add 2 rules to the nginx package. Will this all still work?