Skip to content

Commit

Permalink
[Fleet] Add experimental package verification (server side) (#135570)
Browse files Browse the repository at this point in the history
* add verifyPackageArchiveSignature fn

* add feature flag

* throw error if package verification fails

* add verification info to saved object

* update installation object with verify result

* move to verification_status

* store verification status in cache

* send 422 if verification fails

* only get cached verification result if verify = true

* correctly set verififcation status keys

* unset verification key ID when restarting installation

* self review

* style: neaten types

* pr: Return 400 on verification error

* fix tests

* more test fixes

* PR feedback

* fix types
  • Loading branch information
hop-dev authored Jul 7, 2022
1 parent a59c24b commit dbc251e
Show file tree
Hide file tree
Showing 26 changed files with 338 additions and 90 deletions.
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();
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';
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

0 comments on commit dbc251e

Please sign in to comment.