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] Allow access to Endpoint Metadata for users that might only have READONLY access #106328

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Jul 20, 2021

Summary

  • Endpoint metadata API was modified to use an internal SavedObjects client when using Fleet service to retrieve data (Agent Policies, Agent). This will enable users with ReadOnly permissions to the indexes supporting Endpoint to access endpoint metadata api
  • Fleet: Changes Agent retrieval service methods so that the agent status is calculated here rather than in the API Route handler
  • Refactor: A new EndpointMetadataService was created and methods crated to support the GET metadata/{id} api

Testing:

For API testing:

  1. The following indexes should be added to a role as read only:

    • metrics-endpoint.metadata_current_*
    • .fleet-agents*
    • .fleet-actions*
  2. Use credentials of a user whose role has only readonly permission to the above indexes and query the /api/endpoint/metadata/{id} api

For security solution UI testing:

  1. Run the following scripts:
x-pack/plugins/security_solution/server/lib/detection_engine/scripts/roles_users/t1_analyst/post_detections_role.sh
x-pack/plugins/security_solution/server/lib/detection_engine/scripts/roles_users/t1_analyst/post_detections_user.sh

one will create a role (t1_analyst) and the other an associated user by the same name.

  1. Change/reset the password on the t1_analyst
  2. Login to the Kibana wiht the t1_analyst user and load some data (ex. run endpoint data loader)
  3. Endpoint metadata information should be available in the UI under Alert Details flyout, Host Details and Cases

olm-1504-allow-readonly-access-to-metadata

Checklist

@paul-tavares paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution auto-backport Deprecated - use backport:version if exact versions are needed v7.15.0 labels Jul 20, 2021
@paul-tavares paul-tavares self-assigned this Jul 20, 2021
@paul-tavares paul-tavares marked this pull request as ready for review July 21, 2021 20:54
@paul-tavares paul-tavares requested review from a team as code owners July 21, 2021 20:54
@paul-tavares paul-tavares requested review from pzl and joeypoon July 21, 2021 20:54
@elasticmachine
Copy link
Contributor

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

@@ -58,6 +62,33 @@ export const getLogger = (endpointAppContext: EndpointAppContext): Logger => {
return endpointAppContext.logFactory.get('metadata');
};

const errorHandler = <E extends Error>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generic error handler for all metadata routes. Currently only being used in the GET single endpoint metadata api

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jul 21, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)


type SavedObjectsClientContractKeys = keyof SavedObjectsClientContract;

const RESTRICTED_METHODS: readonly SavedObjectsClientContractKeys[] = [
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 initially wanted to do "allow" list rather than "deny" list, but found that the SO client attempts to access other internal properties on the client that are not actually exposed via the SavedObjectsClientContract. Some of the methods below I don't even know what they do (openPointINTimeForType??), but since they sounded like to me that they might possibly be mutating type of methods, I block them.

) {}

/**
* An INTERNAL Saved Object client that is effectively the system user and has all privileges and permissions and
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should i remove most of the "warning" around this comment now that we use a restricted so client?

Copy link
Member

Choose a reason for hiding this comment

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

nah keep it, so new people (or forgetful us in the future) don't accidentally make changes that strip back your read-only-ness. So it's not accidentally grabbed and used elsewhere. Security holes R no joke

@paul-tavares paul-tavares requested a review from pzl July 27, 2021 16:30
methodName: SavedObjectsClientContractKeys,
receiver: unknown
): unknown {
if (RESTRICTED_METHODS.includes(methodName)) {
Copy link
Member

Choose a reason for hiding this comment

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

cool. I think the remaining danger here you already realize is if new writable methods are added to SO client, we'll miss them here. Will just have to keep an eye on changes. Maybe another reason to keep the warning around in full

@paul-tavares
Copy link
Contributor Author

@elasticmachine merge upstream

@paul-tavares
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @paul-tavares

@paul-tavares paul-tavares merged commit b6e9d8d into elastic:master Jul 28, 2021
@paul-tavares paul-tavares deleted the task/olm-1504-allow-metadata-api-with-readonly-user branch July 28, 2021 12:42
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 106328

paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Jul 28, 2021
…sers that might only have READONLY access (elastic#106328)

* Remove `getScopedSavedObjectsClient()` from server `EndpointAppContextServices` (not used)
* new EndpointMetadataService service (handles GET of a single endpoint for now)
* ensure `getFleetAgent()` sets the current status for the agent
* Replace the Route handler for Get of single endpoint with new call to service
* Fleet: change agent service to also calculate and return the agent status via calls from the agent service
* Fleet agent generator changed to set a random status
* generic `createInternalReadonlySoClient()` with runtime validation for non-readonly methods
# Conflicts:
#	x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts
jbudz added a commit that referenced this pull request Jul 28, 2021
…ta for users that might only have READONLY access (#106328)"

This reverts commit b6e9d8d.
@jbudz
Copy link
Member

jbudz commented Jul 28, 2021

This was reverted with 3d33710. I suspect there was a timing conflict with #106110.

https://kibana-ci.elastic.co/job/elastic+kibana+master/15858/execution/node/398/log/

paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Jul 28, 2021
…sers that might only have READONLY access (elastic#106328)

* Remove `getScopedSavedObjectsClient()` from server `EndpointAppContextServices` (not used)
* new EndpointMetadataService service (handles GET of a single endpoint for now)
* ensure `getFleetAgent()` sets the current status for the agent
* Replace the Route handler for Get of single endpoint with new call to service
* Fleet: change agent service to also calculate and return the agent status via calls from the agent service
* Fleet agent generator changed to set a random status
* generic `createInternalReadonlySoClient()` with runtime validation for non-readonly methods

(cherry picked from commit b6e9d8d)
streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021
…sers that might only have READONLY access (elastic#106328)

* Remove `getScopedSavedObjectsClient()` from server `EndpointAppContextServices` (not used)
* new EndpointMetadataService service (handles GET of a single endpoint for now)
* ensure `getFleetAgent()` sets the current status for the agent
* Replace the Route handler for Get of single endpoint with new call to service
* Fleet: change agent service to also calculate and return the agent status via calls from the agent service
* Fleet agent generator changed to set a random status
* generic `createInternalReadonlySoClient()` with runtime validation for non-readonly methods
streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021
…ta for users that might only have READONLY access (elastic#106328)"

This reverts commit b6e9d8d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes reverted Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Fleet Team label for Observability Data Collection Fleet team v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants