github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 17:28:26 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Security Solution][Endpoint] Use "valid" agents for creating action requests and updating cases (#187145)

Browse source 
## Summary

This PR corrects an API we were able to create multi-agent action for
duplicate/non-existing agent ids. The changes in the PR fix that
1. by not including invalid agent ids (that do not have Elastic Defend
installed) and
2. adds a warning info in the action request comment so that the user is
aware of discarded agents



This action when errored shows multi agent action Host/Error grouping.

**Before** (notice the single `Hostname` above outputs)
You could do this:
```json5
POST /api/endpoint/action/suspend_process
{
    "endpoint_ids": [
        "1d793b46-7b6a-4dd4-a6b7-4c75718e04be",
        "1d793b46-7b6a-4dd4-a6b7-4c75718e04be"
    ],
    "parameters": {
        "pid": "42424242"
    }
}
``` 
and get a response that looked like (notice the duplicate agent ids in
`agents`)
```json5
{
    "data": {
        "id": "ff66dc36-5be9-4b12-a94f-09421baed8da",
        "agentType": "endpoint",
        "agents": [
            "1d793b46-7b6a-4dd4-a6b7-4c75718e04be",
             1d793b46-7b6a-4dd4-a6b7-4c75718e04be
        ],
        "hosts": {
            "1d793b46-7b6a-4dd4-a6b7-4c75718e04be": {
                "name": "ashokaditya-dev-7131"
            }
        },
        "command": "suspend-process",
        "startedAt": "2024-06-28T13:56:40.524Z",
        "isCompleted": false,
        "wasSuccessful": false,
        "isExpired": false,
        "status": "pending",
        "outputs": {},
        "agentState": {
            "1d793b46-7b6a-4dd4-a6b7-4c75718e04be": {
                "isCompleted": false,
                "wasSuccessful": false
            }
        },
        "createdBy": "elastic",
        "parameters": {
            "pid": 4242424242
        }
    }
}
```

**after**
With this change when you try and send a request such as
```json5
POST /api/endpoint/action/suspend_process
{
    "endpoint_ids": [
        "1d793b46-7b6a-4dd4-a6b7-4c75718e04be",
        "1d793b46-7b6a-4dd4-a6b7-4c75718e04be",
        "no-dice"
    ]
    ,
    "parameters": {
        "pid": 4242424242
    }
}
```

you get a response that looks like (notice neither the duplicate agent
id nor the invalid agent id in `agents`, also additional WARNING in the
`comment` field)
```json5
{
    "data": {
        "id": "9bf67816-4d58-4843-bc7a-776f6017b06b",
        "agentType": "endpoint",
        "agents": [
            "1d793b46-7b6a-4dd4-a6b7-4c75718e04be"
        ],
        "hosts": {
            "1d793b46-7b6a-4dd4-a6b7-4c75718e04be": {
                "name": "ashokaditya-dev-7131"
            }
        },
        "command": "suspend-process",
        "startedAt": "2024-06-28T15:13:20.138Z",
        "isCompleted": false,
        "wasSuccessful": false,
        "isExpired": false,
        "status": "pending",
        "outputs": {},
        "agentState": {
            "1d793b46-7b6a-4dd4-a6b7-4c75718e04be": {
                "isCompleted": false,
                "wasSuccessful": false
            }
        },
        "createdBy": "elastic",
        "comment": "undefined; WARNING: The following agent ids are not valid: [\"no-dice\"] and would not be included in action request.",
        "parameters": {
            "pid": 4242424242
        }
    }
}
```
follow up of elastic/kibana/pull/186284

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.

When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| Multiple Spaces—unexpected behavior in non-default Kibana Space.
| Low | High | Integration tests will verify that all features are still
supported in non-default Kibana Space and when user switches between
spaces. |
| Multiple nodes—Elasticsearch polling might have race conditions
when multiple Kibana nodes are polling for the same tasks. | High | Low
| Tasks are idempotent, so executing them multiple times will not result
in logical error, but will degrade performance. To test for this case we
add plenty of unit tests around this logic and document manual testing
procedure. |
| Code should gracefully handle cases when feature X or plugin Y are
disabled. | Medium | High | Unit tests will verify that any feature flag
or plugin combination still results in our service operational. |
| [See more potential risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) |


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
This commit is contained in:
Ash 2024-07-01 12:00:54 +02:00 • committed by GitHub
parent 79f71b7e06
commit cbe6b7b9b3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 144 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

126
x-pack/plugins/security_solution/server/endpoint/services/actions/clients/endpoint/endpoint_actions_client.test.ts
View file

@ -27,7 +27,7 @@ describe('EndpointActionsClient', () => {
'endpoint_ids' | 'case_ids'
> => {
return {
endpoint_ids: ['1-2-3', 'invalid-id'],
endpoint_ids: ['1-2-3', 'invalid-id', '1-2-3'],
case_ids: ['case-a'],
};
};
@ -42,8 +42,8 @@ describe('EndpointActionsClient', () => {
responseActionsClientMock.createIsolateOptions(getCommonResponseActionOptions())
);
expect(classConstructorOptions.endpointService.createLogger().debug).toHaveBeenCalledWith(
'The following agent ids are not valid: ["invalid-id"]'
expect(classConstructorOptions.endpointService.createLogger().warn).toHaveBeenCalledWith(
'The following agent ids are not valid: ["invalid-id"] and will not be included in action request'
);
});
@ -85,11 +85,87 @@ describe('EndpointActionsClient', () => {
});
});
it('should write action request document', async () => {
it('should write action request document to endpoint action request index with given set of valid/invalid agent ids', async () => {
await endpointActionsClient.isolate(
responseActionsClientMock.createIsolateOptions(getCommonResponseActionOptions())
);
expect(classConstructorOptions.esClient.index).toHaveBeenCalledWith(
{
index: ENDPOINT_ACTIONS_INDEX,
document: {
'@timestamp': expect.any(String),
EndpointActions: {
action_id: expect.any(String),
data: {
command: 'isolate',
comment:
'test comment. (WARNING: The following agent ids are not valid: ["invalid-id"] and will not be included in action request)',
parameters: undefined,
},
expiration: expect.any(String),
input_type: 'endpoint',
type: 'INPUT_ACTION',
},
agent: {
id: ['1-2-3'],
},
user: {
id: 'foo',
},
},
refresh: 'wait_for',
},
expect.anything()
);
});
it('should write correct comment when invalid agent ids', async () => {
await endpointActionsClient.isolate(
responseActionsClientMock.createIsolateOptions({
...getCommonResponseActionOptions(),
comment: '',
})
);
expect(classConstructorOptions.esClient.index).toHaveBeenCalledWith(
{
index: ENDPOINT_ACTIONS_INDEX,
document: {
'@timestamp': expect.any(String),
EndpointActions: {
action_id: expect.any(String),
data: {
command: 'isolate',
comment:
'(WARNING: The following agent ids are not valid: ["invalid-id"] and will not be included in action request)',
parameters: undefined,
},
expiration: expect.any(String),
input_type: 'endpoint',
type: 'INPUT_ACTION',
},
agent: {
id: ['1-2-3'],
},
user: {
id: 'foo',
},
},
refresh: 'wait_for',
},
expect.anything()
);
});
it('should write action request document to endpoint action request index with given valid agent ids', async () => {
await endpointActionsClient.isolate(
responseActionsClientMock.createIsolateOptions({
endpoint_ids: ['1-2-3'],
case_ids: ['case-a'],
})
);
expect(classConstructorOptions.esClient.index).toHaveBeenCalledWith(
{
index: ENDPOINT_ACTIONS_INDEX,
@ -107,7 +183,7 @@ describe('EndpointActionsClient', () => {
type: 'INPUT_ACTION',
},
agent: {
id: ['1-2-3', 'invalid-id'],
id: ['1-2-3'],
},
user: {
id: 'foo',
@ -119,9 +195,12 @@ describe('EndpointActionsClient', () => {
);
});
it('should update cases', async () => {
it('should update cases for valid agent ids', async () => {
await endpointActionsClient.isolate(
responseActionsClientMock.createIsolateOptions(getCommonResponseActionOptions())
responseActionsClientMock.createIsolateOptions({
endpoint_ids: ['1-2-3'],
case_ids: ['case-a'],
})
);
expect(classConstructorOptions.casesClient?.attachments.bulkCreate).toHaveBeenCalledWith({
@ -138,10 +217,38 @@ describe('EndpointActionsClient', () => {
endpointId: '1-2-3',
hostname: 'Host-ku5jy6j0pw',
},
],
},
externalReferenceStorage: {
type: 'elasticSearchDoc',
},
owner: 'securitySolution',
type: 'externalReference',
},
],
caseId: 'case-a',
});
});
it('should update cases for valid/invalid agent ids', async () => {
await endpointActionsClient.isolate(
responseActionsClientMock.createIsolateOptions(getCommonResponseActionOptions())
);
expect(classConstructorOptions.casesClient?.attachments.bulkCreate).toHaveBeenCalledWith({
attachments: [
{
externalReferenceAttachmentTypeId: 'endpoint',
externalReferenceId: expect.any(String),
externalReferenceMetadata: {
command: 'isolate',
comment:
'test comment. (WARNING: The following agent ids are not valid: ["invalid-id"] and will not be included in action request)',
targets: [
{
agentType: 'endpoint',
endpointId: 'invalid-id',
hostname: '',
endpointId: '1-2-3',
hostname: 'Host-ku5jy6j0pw',
},
],
},
@ -175,6 +282,7 @@ describe('EndpointActionsClient', () => {
{ meta: true }
);
});
it('should create an action with error when agents are invalid', async () => {
// @ts-expect-error mocking this for testing purposes
endpointActionsClient.checkAgentIds = jest.fn().mockResolvedValueOnce({

35
x-pack/plugins/security_solution/server/endpoint/services/actions/clients/endpoint/endpoint_actions_client.ts
View file

@ -52,6 +52,13 @@ import type {
} from '../lib/types';
import { DEFAULT_EXECUTE_ACTION_TIMEOUT } from '../../../../../../common/endpoint/service/response_actions/constants';
const getInvalidAgentsWarning = (invalidAgents: string[]) =>
invalidAgents.length
? `The following agent ids are not valid: ${JSON.stringify(
invalidAgents
)} and will not be included in action request`
: '';
export class EndpointActionsClient extends ResponseActionsClientImpl {
protected readonly agentType: ResponseActionAgentType = 'endpoint';
@ -61,14 +68,15 @@ export class EndpointActionsClient extends ResponseActionsClientImpl {
allValid: boolean;
hosts: HostMetadata[];
}> {
const uniqueIds = [...new Set(ids)];
const foundEndpointHosts = await this.options.endpointService
.getEndpointMetadataService()
.getMetadataForEndpoints(this.options.esClient, [...new Set(ids)]);
.getMetadataForEndpoints(this.options.esClient, uniqueIds);
const validIds = foundEndpointHosts.map((endpoint: HostMetadata) => endpoint.elastic.agent.id);
const invalidIds = ids.filter((id) => !validIds.includes(id));
if (invalidIds.length) {
this.log.debug(`The following agent ids are not valid: ${JSON.stringify(invalidIds)}`);
this.log.warn(getInvalidAgentsWarning(invalidIds));
}
return {
@ -88,12 +96,12 @@ export class EndpointActionsClient extends ResponseActionsClientImpl {
actionReq: TOptions,
options?: TMethodOptions
): Promise<TResponse> {
const agentIds = await this.checkAgentIds(actionReq.endpoint_ids);
const validatedAgents = await this.checkAgentIds(actionReq.endpoint_ids);
const actionId = uuidv4();
const { error: validationError } = await this.validateRequest({
...actionReq,
command,
endpoint_ids: agentIds.valid || [],
endpoint_ids: validatedAgents.valid || [],
});
const { hosts, ruleName, ruleId, error } = this.getMethodOptions<TMethodOptions>(options);
@ -104,7 +112,7 @@ export class EndpointActionsClient extends ResponseActionsClientImpl {
try {
await this.dispatchActionViaFleet({
actionId,
agents: agentIds.valid,
agents: validatedAgents.valid,
data: {
command,
comment: actionReq.comment,
@ -122,29 +130,40 @@ export class EndpointActionsClient extends ResponseActionsClientImpl {
}
}
// Append warning message to comment if there are invalid agents
const commentMessage = actionReq.comment ? actionReq.comment : '';
const warningMessage = `(WARNING: ${getInvalidAgentsWarning(validatedAgents.invalid)})`;
const comment = validatedAgents.invalid.length
? commentMessage
? `${commentMessage}. ${warningMessage}`
: warningMessage
: actionReq.comment;
// Write action to endpoint index
await this.writeActionRequestToEndpointIndex({
...actionReq,
endpoint_ids: validatedAgents.valid,
error: actionError,
ruleId,
ruleName,
hosts,
actionId,
command,
comment,
});
// Update cases
await this.updateCases({
command,
actionId,
comment: actionReq.comment,
comment,
caseIds: actionReq.case_ids,
alertIds: actionReq.alert_ids,
hosts: actionReq.endpoint_ids.map((hostId) => {
hosts: validatedAgents.valid.map((hostId) => {
return {
hostId,
hostname:
agentIds.hosts.find((host) => host.agent.id === hostId)?.host.hostname ??
validatedAgents.hosts.find((host) => host.agent.id === hostId)?.host.hostname ??
hosts?.[hostId].name ??
'',
};