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

[Fleet] Add experimental package verification (server side) #135570

Merged
merged 18 commits into from
Jul 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions x-pack/plugins/fleet/.storybook/context/fixtures/packages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export const items: GetPackagesResponse['items'] = [
install_started_at: '2021-08-25T19:44:37.078Z',
install_source: 'registry',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
references: [],
coreMigrationVersion: '7.14.0',
Expand Down Expand Up @@ -100,6 +101,7 @@ export const items: GetPackagesResponse['items'] = [
install_started_at: '2021-08-25T19:44:37.078Z',
install_source: 'registry',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
references: [],
coreMigrationVersion: '7.14.0',
Expand Down Expand Up @@ -162,6 +164,7 @@ export const items: GetPackagesResponse['items'] = [
install_started_at: '2021-08-25T19:44:37.078Z',
install_source: 'registry',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
references: [],
coreMigrationVersion: '7.14.0',
Expand Down Expand Up @@ -199,6 +202,7 @@ export const items: GetPackagesResponse['items'] = [
install_started_at: '2021-08-25T19:44:37.078Z',
install_source: 'registry',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
references: [],
coreMigrationVersion: '7.14.0',
Expand Down Expand Up @@ -270,6 +274,7 @@ export const items: GetPackagesResponse['items'] = [
install_started_at: '2021-08-25T19:44:37.078Z',
install_source: 'registry',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
references: [],
coreMigrationVersion: '7.14.0',
Expand Down Expand Up @@ -307,6 +312,7 @@ export const items: GetPackagesResponse['items'] = [
install_started_at: '2021-08-25T19:44:37.078Z',
install_source: 'registry',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
references: [],
coreMigrationVersion: '7.14.0',
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/common/experimental_features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type ExperimentalFeatures = typeof allowedExperimentalValues;
*/
export const allowedExperimentalValues = Object.freeze({
createPackagePolicyMultiPageLayout: true,
packageVerification: false,
});

type ExperimentalConfigKeys = Array<keyof ExperimentalFeatures>;
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/fleet/common/types/models/epm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ type RegistryOverridesToOptional = Pick<PackageSpecManifest, 'title' | 'release'
interface RegistryAdditionalProperties {
assets?: string[];
download: string;
signature_path?: string;
path: string;
readme?: string;
internal?: boolean; // Registry addition[0] and EPM uses it[1] [0]: https://github.com/elastic/package-registry/blob/dd7b021893aa8d66a5a5fde963d8ff2792a9b8fa/util/package.go#L63 [1]
Expand Down Expand Up @@ -400,6 +401,7 @@ export interface IntegrationCardItem {
fromIntegrations?: string;
}

export type PackageVerificationStatus = 'verified' | 'unverified' | 'unknown';
export type PackagesGroupedByStatus = Record<ValueOf<InstallationStatus>, PackageList>;
export type PackageInfo =
| Installable<Merge<RegistryPackage, EpmPackageAdditions>>
Expand All @@ -419,6 +421,8 @@ export interface Installation extends SavedObjectAttributes {
installed_kibana_space_id?: string;
keep_policies_up_to_date?: boolean;
install_format_schema_version?: string;
verification_status: PackageVerificationStatus;
verification_key_id?: string | null;
}

export interface PackageUsageStats {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const Installed = ({ width, ...props }: Args) => {
install_source: 'registry',
install_started_at: '2020-01-01T00:00:00.000Z',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
references: [],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ import { createIntegrationsTestRendererMock } from '../../../../../../mock';

import { ExperimentalFeaturesService } from '../../../../services';

ExperimentalFeaturesService.init({ createPackagePolicyMultiPageLayout: false });
// @ts-ignore this saves us having to define all experimental features
ExperimentalFeaturesService.init({});
import { Detail } from '.';

describe('when on integration detail', () => {
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/fleet/server/errors/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
RegistryConnectionError,
RegistryError,
RegistryResponseError,
PackageFailedVerificationError,
} from '.';

type IngestErrorHandler = (
Expand Down Expand Up @@ -60,6 +61,9 @@ const getHTTPResponseCode = (error: IngestManagerError): number => {
if (error instanceof PackageUnsupportedMediaTypeError) {
return 415; // Unsupported Media Type
}
if (error instanceof PackageFailedVerificationError) {
return 400; // Bad Request
}
if (error instanceof ConcurrentInstallOperationError) {
return 409; // Conflict
}
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/fleet/server/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ export class RegistryResponseError extends RegistryError {
export class PackageNotFoundError extends IngestManagerError {}
export class PackageKeyInvalidError extends IngestManagerError {}
export class PackageOutdatedError extends IngestManagerError {}
export class PackageFailedVerificationError extends IngestManagerError {
constructor(pkgKey: string) {
super(`${pkgKey} failed signature verification.`);
}
}
export class AgentPolicyError extends IngestManagerError {}
export class AgentPolicyNotFoundError extends IngestManagerError {}
export class AgentNotFoundError extends IngestManagerError {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ describe('validate bundled packages', () => {
bundledPackage.version
);

const packageArchive = await Registry.fetchArchiveBuffer(
bundledPackage.name,
bundledPackage.version
);
const packageArchive = await Registry.fetchArchiveBuffer({
pkgName: bundledPackage.name,
pkgVersion: bundledPackage.version,
shouldVerify: false,
});

return { registryPackage, packageArchive };
})
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/fleet/server/saved_objects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ const getSavedObjectTypes = (
enabled: false,
type: 'object',
},
verification_status: { type: 'keyword' },
verification_key_id: { type: 'keyword' },
installed_es: {
type: 'nested',
properties: {
Expand Down
19 changes: 16 additions & 3 deletions x-pack/plugins/fleet/server/services/epm/archive/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,37 @@
* 2.0.
*/

import type { ArchivePackage, RegistryPackage } from '../../../../common';
import type { ArchivePackage, RegistryPackage, PackageVerificationResult } from '../../../types';
import { appContextService } from '../..';

import type { ArchiveEntry } from '.';

type SharedKeyString = string;

const sharedKey = ({ name, version }: SharedKey): SharedKeyString => `${name}-${version}`;

const archiveEntryCache: Map<ArchiveEntry['path'], ArchiveEntry['buffer']> = new Map();
export const getArchiveEntry = (key: string) => archiveEntryCache.get(key);
export const setArchiveEntry = (key: string, value: Buffer) => archiveEntryCache.set(key, value);
export const hasArchiveEntry = (key: string) => archiveEntryCache.has(key);
export const clearArchiveEntries = () => archiveEntryCache.clear();
export const deleteArchiveEntry = (key: string) => archiveEntryCache.delete(key);

const verificationResultCache: Map<SharedKeyString, PackageVerificationResult> = new Map();
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if we should avoid this cache pattern at some point it's not going to scale (it's probably more problematic for archiveEntryCache) do we really need to cache things how often are computing the verification?

If we really need to cache it it's probably better to use an LRU cache with a max size

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 looked into removing caching, unfortunately as we do not cache the full archive only the sub-files this wasn't possible. I agree that the way that we cache isn't great, I think changing the way we cache should be done as a seperate ticket which I have raised here: #135790

export const getVerificationResult = (key: SharedKey) =>
verificationResultCache.get(sharedKey(key));
export const setVerificationResult = (key: SharedKey, value: PackageVerificationResult) =>
verificationResultCache.set(sharedKey(key), value);
export const hasVerificationResult = (key: SharedKey) =>
verificationResultCache.has(sharedKey(key));
export const clearVerificationResults = () => verificationResultCache.clear();
export const deleteVerificationResult = (key: SharedKey) =>
verificationResultCache.delete(sharedKey(key));

export interface SharedKey {
name: string;
version: string;
}
type SharedKeyString = string;

const archiveFilelistCache: Map<SharedKeyString, string[]> = new Map();
export const getArchiveFilelist = (keyArgs: SharedKey) =>
Expand All @@ -38,7 +52,6 @@ export const deleteArchiveFilelist = (keyArgs: SharedKey) =>
archiveFilelistCache.delete(sharedKey(keyArgs));

const packageInfoCache: Map<SharedKeyString, ArchivePackage | RegistryPackage> = new Map();
const sharedKey = ({ name, version }: SharedKey) => `${name}-${version}`;

export const getPackageInfo = (args: SharedKey) => {
return packageInfoCache.get(sharedKey(args));
Expand Down
4 changes: 1 addition & 3 deletions x-pack/plugins/fleet/server/services/epm/archive/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import type { AssetParts, InstallSource } from '../../../../common/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.

installSource was unused

import type { AssetParts } from '../../../../common/types';
import { PackageInvalidArchiveError, PackageUnsupportedMediaTypeError } from '../../../errors';

import {
Expand Down Expand Up @@ -35,13 +35,11 @@ export async function unpackBufferToCache({
version,
contentType,
archiveBuffer,
installSource,
}: {
name: string;
version: string;
contentType: string;
archiveBuffer: Buffer;
installSource: InstallSource;
}): Promise<string[]> {
// Make sure any buffers from previous installations from registry or upload are deleted first
clearPackageFileCache({ name, version });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ function getTest(
method: mocks.packageClient.getRegistryPackage.bind(mocks.packageClient),
args: ['package name', '8.0.0'],
spy: jest.spyOn(epmRegistry, 'getRegistryPackage'),
spyArgs: ['package name', '8.0.0'],
spyArgs: ['package name', '8.0.0', undefined],
spyResponse: {
packageInfo: { name: 'getRegistryPackage test' },
paths: ['/some/test/path'],
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/package_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,13 @@ class PackageClientImpl implements PackageClient {
return fetchFindLatestPackageOrThrow(packageName);
}

public async getRegistryPackage(packageName: string, packageVersion: string) {
public async getRegistryPackage(
packageName: string,
packageVersion: string,
options?: Parameters<typeof getRegistryPackage>['2']
) {
await this.#runPreflight();
return getRegistryPackage(packageName, packageVersion);
return getRegistryPackage(packageName, packageVersion, options);
}

public async reinstallEsAssets(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,16 @@ import {
PACKAGE_POLICY_SAVED_OBJECT_TYPE,
SO_SEARCH_LIMIT,
} from '../../../../common';
import type { InstallablePackage, InstallSource, PackageAssetReference } from '../../../../common';
import { PACKAGES_SAVED_OBJECT_TYPE, FLEET_INSTALL_FORMAT_VERSION } from '../../../constants';
import type { AssetReference, Installation, InstallType } from '../../../types';
import type {
AssetReference,
Installation,
InstallType,
InstallablePackage,
InstallSource,
PackageAssetReference,
PackageVerificationResult,
} from '../../../types';
import { prepareToInstallTemplates } from '../elasticsearch/template/install';
import { removeLegacyTemplates } from '../elasticsearch/template/remove_legacy';
import {
Expand All @@ -39,7 +46,7 @@ import { saveArchiveEntries } from '../archive/storage';
import { ConcurrentInstallOperationError } from '../../../errors';
import { packagePolicyService } from '../..';

import { createInstallation, updateEsAssetReferences } from './install';
import { createInstallation, updateEsAssetReferences, restartInstallation } from './install';
import { withPackageSpan } from './utils';

// this is only exported for testing
Expand All @@ -56,6 +63,7 @@ export async function _installPackage({
installType,
installSource,
spaceId,
verificationResult,
}: {
savedObjectsClient: SavedObjectsClientContract;
savedObjectsImporter: Pick<SavedObjectsImporter, 'import' | 'resolveImportErrors'>;
Expand All @@ -67,6 +75,7 @@ export async function _installPackage({
installType: InstallType;
installSource: InstallSource;
spaceId: string;
verificationResult?: PackageVerificationResult;
}): Promise<AssetReference[]> {
const { name: pkgName, version: pkgVersion } = packageInfo;

Expand All @@ -88,11 +97,12 @@ export async function _installPackage({
} else {
// if no installation is running, or the installation has been running longer than MAX_TIME_COMPLETE_INSTALL
// (it might be stuck) update the saved object and proceed
await savedObjectsClient.update<Installation>(PACKAGES_SAVED_OBJECT_TYPE, pkgName, {
install_version: pkgVersion,
install_status: 'installing',
install_started_at: new Date().toISOString(),
install_source: installSource,
await restartInstallation({
savedObjectsClient,
pkgName,
pkgVersion,
installSource,
verificationResult,
});
}
} else {
Expand All @@ -101,6 +111,7 @@ export async function _installPackage({
packageInfo,
installSource,
spaceId,
verificationResult,
});
}

Expand Down
10 changes: 8 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/packages/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ import type {
GetCategoriesRequest,
} from '../../../../common/types';
import type { Installation, PackageInfo } from '../../../types';
import { IngestManagerError, PackageNotFoundError } from '../../../errors';
import {
IngestManagerError,
PackageFailedVerificationError,
PackageNotFoundError,
} from '../../../errors';
import { appContextService } from '../..';
import * as Registry from '../registry';
import { getEsPackage } from '../archive/storage';
Expand Down Expand Up @@ -268,8 +272,10 @@ export async function getPackageFromSource(options: {
try {
res = await Registry.getRegistryPackage(pkgName, pkgVersion);
logger.debug(`retrieved installed package ${pkgName}-${pkgVersion} from registry`);
// TODO: add to cache and storage here?
} catch (error) {
if (error instanceof PackageFailedVerificationError) {
throw error;
}
// treating this is a 404 as no status code returned
// in the unlikely event its missing from cache, storage, and never installed from registry
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const mockInstallation: SavedObject<Installation> = {
install_started_at: new Date().toISOString(),
install_source: 'registry',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
};
const mockInstallationUpdateFail: SavedObject<Installation> = {
Expand All @@ -50,6 +51,7 @@ const mockInstallationUpdateFail: SavedObject<Installation> = {
install_started_at: new Date().toISOString(),
install_source: 'registry',
keep_policies_up_to_date: false,
verification_status: 'unknown',
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ jest.mock('../archive', () => {
),
unpackBufferToCache: jest.fn(),
setPackageInfo: jest.fn(),
deleteVerificationResult: jest.fn(),
};
});

Expand Down Expand Up @@ -205,6 +206,7 @@ describe('install', () => {
});

it('should install from bundled package if one exists', async () => {
(install._installPackage as jest.Mock).mockResolvedValue({});
mockGetBundledPackages.mockResolvedValue([
{
name: 'test_package',
Expand All @@ -213,14 +215,16 @@ describe('install', () => {
},
]);

await installPackage({
const response = await installPackage({
spaceId: DEFAULT_SPACE_ID,
installSource: 'registry',
pkgkey: 'test_package-1.0.0',
savedObjectsClient: savedObjectsClientMock.create(),
esClient: {} as ElasticsearchClient,
});

expect(response.error).toBeUndefined();

expect(install._installPackage).toHaveBeenCalledWith(
expect.objectContaining({ installSource: 'upload' })
);
Expand Down
Loading