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

[Security Solution][Endpoint] Add support to Action Responder script for get-file response action #142663

Conversation

paul-tavares
Copy link
Contributor

Summary

  • Adds support to Action Responder script (x-pack/plugins/security_solution/scripts/endpoint/endpoint_action_responder.js) for get-file response action (coming in 8.6). Will write a generated file to ES when action is submitted
  • Alters the .generateResponse() method of EndpointActionGenerator to also generate parameters and output for get-file action
  • Adds type for UploadedFile and const's for the indexes that will store the files.

NOTE: There is no way to test this just yet - unless one creates an action manually via ES and then runs the endpoint action responder script. A subsequent PR will introduce the command into the console.

@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.6.0 labels Oct 4, 2022
@paul-tavares paul-tavares requested a review from a team as a code owner October 4, 2022 19:35
@paul-tavares paul-tavares self-assigned this Oct 4, 2022
@paul-tavares paul-tavares requested a review from a team as a code owner October 4, 2022 19:35
@elasticmachine
Copy link
Contributor

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

@paul-tavares paul-tavares requested a review from pzl October 4, 2022 19:35
},
// randomly before a few hours/minutes/seconds later
started_at: new Date(startedAtTimes[this.randomN(startedAtTimes.length)]).toISOString(),
output: undefined,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

output seemed to be in the wrong place here. It should have been under data

* The Metadata information about a file that was uploaded by Endpoint
* as a result of a `get-file` response action
*/
export interface UploadedFile {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI - having had some more time looking at the file upload and how we will use it in the UI, I'm not sure that we will need this Type. Or we might needed it, but not under common/ - maybe on the server side only. We might not actually need to interact with the file stored because the Endpoint action response will likely have everything we need.

* as a result of a `get-file` response action
*/
export interface UploadedFile {
file: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only defined a subset of the ECS file attributes. Let me now if I should define others and/or the full set.

// Add the file's metadata
const fileMeta = await esClient.index<UploadedFile>({
index: FILE_STORAGE_METADATA_INDEX,
id: `${action.id}.${action.hosts[0]}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pzl - I think this is right for now, correct?
If anything changes, and you remember, let us know and we'll update.

Copy link
Member

Choose a reason for hiding this comment

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

correct for now. This is a completely opaque value that we get to determine. So whatever order or value is easiest for us to look up again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

either approach works IMO. both pieces of data is on the Action request/response


await esClient.index({
index: FILE_STORAGE_DATA_INDEX,
id: `${fileMeta._id}.0`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pzl - I think this is right. I did not have a file with multiple chunks, but looking at the data you sent me, I assumed that each chunk's id would be comprised of the File's meta document _id + .<chunk_number>.

Copy link
Member

Choose a reason for hiding this comment

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

precisely correct

id: `${fileMeta._id}.0`,
body: {
bid: fileMeta._id,
last: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this correct for file stored all within one chunk?

Copy link
Member

Choose a reason for hiding this comment

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

no, what I sent you actually had a bug in it. Should still be true for 1-chunk-docs

Comment on lines +35 to +36
ssdeep?: string;
tlsh?: string;
Copy link
Member

Choose a reason for hiding this comment

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

I think it's highly unlikely we will ever use these hashes. After determining the capabilities of agent/endpoint to generate hashes, we will support a specific subset, since fleet server will also need to perform that same hashing, it will be a known subset we support. But is yet to be determined until they start work

md5, sha256 is probably a safe assumption. unless we need to add something simpler like crc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will adjust once we know that.

* of the contents and metadata happens. Deleted files should be treated as if they don’t
* exist. Only files in READY state can transition into DELETED state.
*/
Status: 'AWAITING_UPLOAD' | 'UPLOADING' | 'READY' | 'UPLOAD_ERROR' | 'DELETED';
Copy link
Member

Choose a reason for hiding this comment

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

🔥

body: {
bid: fileMeta._id,
last: false,
data: 'UEsDBBQACAAIAFVeRFUAAAAAAAAAABMAAAAMACAAYmFkX2ZpbGUudHh0VVQNAAdTVjxjU1Y8Y1NWPGN1eAsAAQT1AQAABBQAAAArycgsVgCiRIWkxBSFtMycVC4AUEsHCKkCwMsTAAAAEwAAAFBLAQIUAxQACAAIAFVeRFWpAsDLEwAAABMAAAAMACAAAAAAAAAAAACkgQAAAABiYWRfZmlsZS50eHRVVA0AB1NWPGNTVjxjU1Y8Y3V4CwABBPUBAAAEFAAAAFBLBQYAAAAAAQABAFoAAABtAAAAAAA=',
Copy link
Member

Choose a reason for hiding this comment

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

for 💩 and 🤣 I actually checked what this is.

It's a zip of a single file, bad_file.txt with contents: this is a bad file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LOL - yep 😎

(and thanks for testing that out)

@paul-tavares paul-tavares requested a review from pzl October 4, 2022 20:29
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

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
securitySolution 6.6MB 6.6MB +11.0B

History

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

cc @paul-tavares

Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

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

Also please add the get-file command to x-pack/plugins/security_solution/public/management/components/endpoint_response_actions_list/components/use_action_history_url_params.test.ts tests

ssdeep?: string;
tlsh?: string;
};
mime_type?: string;
Copy link
Member

Choose a reason for hiding this comment

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

Should we restrict this to the allowed mime types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I think we could since the file the endpoint will upload will always be zip. I'll wait on that until we actually get this working with the Endpoint and get agreement on the expectation.

Copy link
Member

Choose a reason for hiding this comment

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

I see. 👍

Comment on lines +560 to +563
expect(filterList.querySelectorAll('ul>li').length).toEqual(RESPONSE_ACTION_COMMANDS.length);
expect(
Array.from(filterList.querySelectorAll('ul>li')).map((option) => option.textContent)
).toEqual(['isolate', 'release', 'kill-process', 'suspend-process', 'processes']);
).toEqual(['isolate', 'release', 'kill-process', 'suspend-process', 'processes', 'get-file']);
Copy link
Member

Choose a reason for hiding this comment

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

🎉

@paul-tavares
Copy link
Contributor Author

@ashokaditya

Re: adding to test "...components/endpoint_response_actions_list/components/use_action_history_url_params.test.ts"

I will do this in the next PR. I would like to do some refactoring and move some of the const associated with Response actions to common/service/constants.ts so that they are all in one place. I would like to rename them too in order to clarify what they are.

Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

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

🚀 🚢

@paul-tavares paul-tavares merged commit 3fa8a87 into elastic:main Oct 5, 2022
@paul-tavares paul-tavares deleted the task/olm-add-generator-get-file-support branch October 5, 2022 14:01
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 5, 2022
@nicpenning
Copy link

Awesome, I am excited for this feature. Since I can't quite follow the complexities of the code, I would like to ask:

Does the end goal .zip file contain a password and if so is it configurable?

WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
…for `get-file` response action (elastic#142663)

* Add `const`'s for file storage indexes

* Add file storage TS type

* Add support to Action Responder script for `get-file` response action

* Add support to action generator to output `get-file` responses

* correct `last` property of the upload chunk

* Fix type

* fix UI jest test failures

* Fix server jest tests and Generator bug
@paul-tavares
Copy link
Contributor Author

@nicpenning - yes, the zip file that is uploaded by the Endpoint with the requested file will have a password set on it and currently we are not planning on making that configurable. We are planning on using elastic as the password.

@nicpenning
Copy link

Awesome, thanks! That should be great.

WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
…for `get-file` response action (elastic#142663)

* Add `const`'s for file storage indexes

* Add file storage TS type

* Add support to Action Responder script for `get-file` response action

* Add support to action generator to output `get-file` responses

* correct `last` property of the upload chunk

* Fix type

* fix UI jest test failures

* Fix server jest tests and Generator bug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants