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] User Manifest Cleanup + Artifact Compression #70759

Merged
merged 53 commits into from
Jul 9, 2020
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
133ea27
Stateless exception list translation with improved runtime checks
madirey Jul 2, 2020
9e43633
use flatMap and reduce to simplify logic
madirey Jul 3, 2020
e9f74b4
Update to new manifest format
madirey Jul 4, 2020
2a266b3
Merge branch 'master' of github.com:elastic/kibana into user-allowlis…
madirey Jul 4, 2020
ceca5fa
Fix test fixture SO data type
madirey Jul 4, 2020
49d22d2
Fix another test fixture data type
madirey Jul 4, 2020
9428ca5
Fix sha256 reference in artifact_client
madirey Jul 4, 2020
c376638
Refactor to remove usages of 'then' and tidy up a bit
madirey Jul 5, 2020
617d5f1
sync master
madirey Jul 5, 2020
13d9ec8
Zlib compression
Jul 6, 2020
4718110
prefer byteLength to length
Jul 6, 2020
feb1202
Make ingestManager optional for security-solution startup
madirey Jul 6, 2020
4ce792b
Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…
madirey Jul 6, 2020
aecf718
Fix download functionality
madirey Jul 7, 2020
56886ad
Use eql for deep equality check
madirey Jul 7, 2020
03e75c9
Fix base64 download bug
madirey Jul 7, 2020
8620510
Add test for artifact download
madirey Jul 7, 2020
a8216e1
Merge branch 'master' of github.com:elastic/kibana into fix-download
madirey Jul 7, 2020
1456697
Add more tests to ensure cached versions of artifacts are correct
madirey Jul 7, 2020
eae1b89
Convert to new format
madirey Jul 7, 2020
ec60546
Deflate
Jul 7, 2020
a32f960
Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…
Jul 7, 2020
339c887
missed some refs
madirey Jul 7, 2020
61b2ca8
Merge branch 'master' of github.com:elastic/kibana into user-allowlis…
Jul 7, 2020
aca7b8c
partial fix to wrapper format
madirey Jul 7, 2020
04d5701
update fixtures and integration test
madirey Jul 7, 2020
ed87ad2
Fixing unit tests
Jul 7, 2020
f2c5ba1
Merge branch 'fix-download' of github.com:madirey/kibana into fix-dow…
Jul 7, 2020
70ce85a
Merge branch 'master' of github.com:elastic/kibana into fix-download
madirey Jul 7, 2020
9d331d2
Merge branch 'master' of github.com:elastic/kibana into user-allowlis…
madirey Jul 8, 2020
ead9ae8
sync with fix-download branch
madirey Jul 8, 2020
5095d85
merge master, fix tests
madirey Jul 8, 2020
094c358
small bug fixes
madirey Jul 8, 2020
d6aefee
artifact and manifest versioning changes
madirey Jul 8, 2020
be42e8d
Merge branch 'master' of github.com:elastic/kibana into user-allowlis…
madirey Jul 8, 2020
7e48f52
Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…
Jul 8, 2020
0bd3530
Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…
Jul 8, 2020
206f222
Merge branch 'master' of github.com:elastic/kibana into user-allowlis…
madirey Jul 8, 2020
ae856b5
Remove access tag from download endpoint
madirey Jul 8, 2020
ced63a9
Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…
Jul 8, 2020
69f59fd
Adding decompression to integration test
Jul 8, 2020
4795132
Removing tag from route
Jul 8, 2020
c18948b
add try/catch in ingest callback handler
madirey Jul 8, 2020
8bef643
Fixing
Jul 8, 2020
a2b3a1b
Merging
Jul 8, 2020
a24acc8
Removing last expect from unit test for tag
Jul 8, 2020
04dbbcf
type fixes
madirey Jul 8, 2020
0809b77
Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…
madirey Jul 8, 2020
2bf4acb
Add compression type to manifest
madirey Jul 8, 2020
50b49a5
Merge branch 'master' into user-allowlist-artifacts-pt3
elasticmachine Jul 9, 2020
046f271
Merge branch 'master' into user-allowlist-artifacts-pt3
elasticmachine Jul 9, 2020
a4414f9
Merge branch 'master' into user-allowlist-artifacts-pt3
elasticmachine Jul 9, 2020
899a673
Reverting ingestManager back to being required for now
Jul 9, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"embeddable",
"features",
"home",
"ingestManager",
"taskManager",
"inspector",
"licensing",
Expand All @@ -21,6 +20,7 @@
],
"optionalPlugins": [
"encryptedSavedObjects",
"ingestManager",
"ml",
"newsfeed",
"security",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import { httpServerMock } from '../../../../../src/core/server/mocks';
import { EndpointAppContextService } from './endpoint_app_context_services';

describe('test endpoint app context services', () => {
it('should throw error on getAgentService if start is not called', async () => {
it('should return undefined on getAgentService if dependencies are not enabled', async () => {
const endpointAppContextService = new EndpointAppContextService();
expect(() => endpointAppContextService.getAgentService()).toThrow(Error);
});
it('should return undefined on getManifestManager if start is not called', async () => {
it('should return undefined on getManifestManager if dependencies are not enabled', async () => {
const endpointAppContextService = new EndpointAppContextService();
expect(endpointAppContextService.getManifestManager()).toEqual(undefined);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ import { AgentService, IngestManagerStartContract } from '../../../ingest_manage
import { getPackageConfigCreateCallback } from './ingest_integration';
import { ManifestManager } from './services/artifacts';

export type EndpointAppContextServiceStartContract = Pick<
IngestManagerStartContract,
'agentService'
export type EndpointAppContextServiceStartContract = Partial<
Pick<IngestManagerStartContract, 'agentService'>
> & {
manifestManager?: ManifestManager | undefined;
registerIngestCallback: IngestManagerStartContract['registerExternalCallback'];
manifestManager?: ManifestManager;
registerIngestCallback?: IngestManagerStartContract['registerExternalCallback'];
savedObjectsStart: SavedObjectsServiceStart;
};

Expand All @@ -35,7 +34,7 @@ export class EndpointAppContextService {
this.manifestManager = dependencies.manifestManager;
this.savedObjectsStart = dependencies.savedObjectsStart;

if (this.manifestManager !== undefined) {
if (this.manifestManager && dependencies.registerIngestCallback) {
dependencies.registerIngestCallback(
'packageConfigCreate',
getPackageConfigCreateCallback(this.manifestManager)
Expand All @@ -45,10 +44,7 @@ export class EndpointAppContextService {

public stop() {}

public getAgentService(): AgentService {
if (!this.agentService) {
throw new Error(`must call start on ${EndpointAppContextService.name} to call getter`);
}
public getAgentService(): AgentService | undefined {
return this.agentService;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,19 @@ export const getPackageConfigCreateCallback = (
// follow the types/schema expected
let updatedPackageConfig = newPackageConfig as NewPolicyData;

const wrappedManifest = await manifestManager.refresh({ initialize: true });
if (wrappedManifest !== null) {
// get snapshot based on exception-list-agnostic SOs
// with diffs from last dispatched manifest, if it exists
const snapshot = await manifestManager.getSnapshot({ initialize: true });

if (snapshot === null) {
// TODO: log error... should not be in this state
return updatedPackageConfig;
}

if (snapshot.diffs.length > 0) {
// create new artifacts
await manifestManager.syncArtifacts(snapshot, 'add');

// Until we get the Default Policy Configuration in the Endpoint package,
// we will add it here manually at creation time.
// @ts-ignore
Expand All @@ -42,7 +53,7 @@ export const getPackageConfigCreateCallback = (
streams: [],
config: {
artifact_manifest: {
value: wrappedManifest.manifest.toEndpointFormat(),
value: snapshot.manifest.toEndpointFormat(),
},
policy: {
value: policyConfigFactory(),
Expand All @@ -57,9 +68,18 @@ export const getPackageConfigCreateCallback = (
try {
return updatedPackageConfig;
} finally {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit -> We might think about sending a promise instead of counting on finally. I think that will be more readable too

// TODO: confirm creation of package config
// then commit.
await manifestManager.commit(wrappedManifest);
if (snapshot.diffs.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit -> I think it will be nice to have try/catch here to avoid bubble up error and have a better way to handle it

// const created = await manifestManager.confirmPackageConfigExists(updatedPackageConfig.name);
const created = true;
if (created) {
await manifestManager.commit(snapshot.manifest);

// clean up old artifacts
await manifestManager.syncArtifacts(snapshot, 'delete');
} else {
// TODO: log error
}
}
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const DEFAULT_MAX_SIZE = 10;
* FIFO cache implementation for artifact downloads.
*/
export class ExceptionsCache {
private cache: Map<string, string>;
private cache: Map<string, Buffer>;
private queue: string[];
private maxSize: number;

Expand All @@ -20,7 +20,7 @@ export class ExceptionsCache {
this.maxSize = maxSize || DEFAULT_MAX_SIZE;
}

set(id: string, body: string) {
set(id: string, body: Buffer) {
if (this.queue.length + 1 > this.maxSize) {
const entry = this.queue.shift();
if (entry !== undefined) {
Expand All @@ -31,7 +31,7 @@ export class ExceptionsCache {
this.cache.set(id, body);
}

get(id: string): string | undefined {
get(id: string): Buffer | undefined {
return this.cache.get(id);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { createHash } from 'crypto';
import { deflate } from 'zlib';
import { validate } from '../../../../common/validate';

import { Entry, EntryNested } from '../../../../../lists/common/schemas/types/entries';
Expand Down Expand Up @@ -32,6 +33,7 @@ export async function buildArtifact(
const exceptionsBuffer = Buffer.from(JSON.stringify(exceptions));
const sha256 = createHash('sha256').update(exceptionsBuffer.toString()).digest('hex');

// Keep compression info empty in case its a duplicate. Lazily compress before committing if needed.
return {
identifier: `${ArtifactConstants.GLOBAL_ALLOWLIST_NAME}-${os}-${schemaVersion}`,
compressionAlgorithm: 'none',
Expand Down Expand Up @@ -170,3 +172,15 @@ function translateEntry(
}
}
}

export function compressExceptionList(buffer: Buffer): Promise<Buffer> {
return new Promise((resolve, reject) => {
deflate(buffer, function (err, buf) {
if (err) {
reject(err);
} else {
resolve(buf);
}
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,22 @@ export class ManifestTask {
return;
}

manifestManager
.refresh()
.then((wrappedManifest) => {
if (wrappedManifest) {
return manifestManager.dispatch(wrappedManifest);
}
})
.then((wrappedManifest) => {
if (wrappedManifest) {
return manifestManager.commit(wrappedManifest);
}
})
.catch((err) => {
this.logger.error(err);
});
try {
// get snapshot based on exception-list-agnostic SOs
// with diffs from last dispatched manifest
const snapshot = await manifestManager.getSnapshot();
if (snapshot && snapshot.diffs.length > 0) {
// create new artifacts
await manifestManager.syncArtifacts(snapshot, 'add');
// write to ingest-manager package config
await manifestManager.dispatch(snapshot.manifest);
// commit latest manifest state to user-artifact-manifest SO
await manifestManager.commit(snapshot.manifest);
// clean up old artifacts
await manifestManager.syncArtifacts(snapshot, 'delete');
}
} catch (err) {
this.logger.error(err);
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ describe('test alerts route', () => {
let mockScopedClient: jest.Mocked<ILegacyScopedClusterClient>;
let mockSavedObjectClient: jest.Mocked<SavedObjectsClientContract>;
let mockResponse: jest.Mocked<KibanaResponseFactory>;
// @ts-ignore
let routeConfig: RouteConfig<unknown, unknown, unknown, never>;
let routeHandler: RequestHandler<unknown, unknown, unknown>;
let endpointAppContextService: EndpointAppContextService;
Expand All @@ -98,8 +97,9 @@ describe('test alerts route', () => {
// The authentication with the Fleet Plugin needs a separate scoped SO Client
ingestSavedObjectClient = savedObjectsClientMock.create();
ingestSavedObjectClient.find.mockReturnValue(Promise.resolve(mockIngestSOResponse));
// @ts-ignore
startContract.savedObjectsStart.getScopedClient.mockReturnValue(ingestSavedObjectClient);
(startContract.savedObjectsStart.getScopedClient as jest.Mock).mockReturnValue(
ingestSavedObjectClient
);
endpointAppContextService.start(startContract);

registerDownloadExceptionListRoute(
Expand Down Expand Up @@ -147,6 +147,8 @@ describe('test alerts route', () => {
path.startsWith('/api/endpoint/artifacts/download')
)!;

expect(routeConfig.options).toEqual({});

await routeHandler(
({
core: {
Expand All @@ -160,8 +162,8 @@ describe('test alerts route', () => {
);

const expectedHeaders = {
'content-encoding': 'application/json',
'content-disposition': `attachment; filename=${mockArtifactName}.json`,
'content-encoding': 'identity',
'content-disposition': `attachment; filename=${mockArtifactName}.zz`,
};

expect(mockResponse.ok).toBeCalled();
Expand Down Expand Up @@ -217,7 +219,7 @@ describe('test alerts route', () => {
// Add to the download cache
const mockArtifact = expectedEndpointExceptions;
const cacheKey = `${mockArtifactName}-${mockSha}`;
cache.set(cacheKey, JSON.stringify(mockArtifact));
cache.set(cacheKey, Buffer.from(JSON.stringify(mockArtifact))); // TODO: add compression here

[routeConfig, routeHandler] = routerMock.get.mock.calls.find(([{ path }]) =>
path.startsWith('/api/endpoint/artifacts/download')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ export function registerDownloadExceptionListRoute(
},
options: { tags: [] },
},
// @ts-ignore
async (context, req, res) => {
let scopedSOClient: SavedObjectsClientContract;
const logger = endpointContext.logFactory.get('download_exception_list');
Expand All @@ -62,12 +61,12 @@ export function registerDownloadExceptionListRoute(
}
}

const buildAndValidateResponse = (artName: string, body: string): IKibanaResponse => {
const buildAndValidateResponse = (artName: string, body: Buffer): IKibanaResponse => {
const artifact: HttpResponseOptions = {
body,
headers: {
'content-encoding': 'application/json',
'content-disposition': `attachment; filename=${artName}.json`,
'content-encoding': 'identity',
'content-disposition': `attachment; filename=${artName}.zz`,
},
};

Expand All @@ -90,7 +89,7 @@ export function registerDownloadExceptionListRoute(
return scopedSOClient
.get<InternalArtifactSchema>(ArtifactConstants.SAVED_OBJECT_TYPE, id)
.then((artifact: SavedObject<InternalArtifactSchema>) => {
const body = Buffer.from(artifact.attributes.body, 'base64').toString();
const body = Buffer.from(artifact.attributes.body, 'base64');
cache.set(id, body);
return buildAndValidateResponse(artifact.attributes.identifier, body);
})
Expand Down
Loading