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

hop-dev · 2022-06-30T15:40:17Z

Summary

Part of #133822.

Add package verification on package install. Verify a package signature if:

the feature flag is enabled packageVerification
the package has a .sig file in the registry (none will yet)
a local public key is configured at xpack.fleet.packageVerification.gpgKeyPath

Otherwise package verification status is always set to "unknown".

Summary of changes

Add new feature flag packageVerification, disabled by default
Add new fields to installation saved object (saved object migration to follow in separate PR)
- verification_status - unknown | verified | unverified
- verification_key_id - present if status is not unknown, they id of the key used to do verification.
Add new cache for storing archive verification status
When downloading archive, perform verification and cache result
Verify bundled packages when downloading them (but don't currently store the .sig file for future verification, so will still have unknown verification status)
Return 400 response code if package verification fails
If a package is installed with force: true then install will proceed even if verification fails

Testing steps

1. Run the version of the EPR with package signatures locally:

docker run -p 8080:8080 docker.elastic.co/observability-ci/package-registry/distribution:PR-463

1. set xpack.fleet.registryUrl: http://localhost:8080
1. download local public key curl https://artifacts.elastic.co/GPG-KEY-elasticsearch -o /tmp/GPG-KEY-elasticsearch
1. set xpack.fleet.packageVerification.gpgKeyPath: /tmp/GPG-KEY-elasticsearch
1. Enable the feature by setting:

xpack.fleet.enableExperimental: 
    - packageVerification

1. Optional - to test if verification fails, here is a GPG public key which isnt the elastic one:
  NOT_THE_ELASTIC_KEY.txt

Test scenarios:

if verification feature flag is enabled and:
- verification is successful: Install should succeed,verification_key_id should be d27d666cd88e42b4 and verification_status should be verified
- verification is not successful : (e.g by using a different public key supplied above) Install should not succeed, 400 response code returned,verification_key_id should be d27d666cd88e42b4 and verification_status should be unverified
- no public key configured locally: (e.g by unsetting gpgKey config)Install should succeed,verification_key_id should not be set and verification_status should be unknown
- missing .sig file: (e.g by connecting to the normal EPR) Install should succeed,verification_key_id should not be set and verification_status should be unknown
verification feature flag not enabled: Install should succeed,verification_key_id should not be set and verification_status should be unknown

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

hop-dev · 2022-06-30T15:42:30Z

x-pack/plugins/fleet/server/errors/handlers.ts

@@ -60,6 +61,9 @@ const getHTTPResponseCode = (error: IngestManagerError): number => {
  if (error instanceof PackageUnsupportedMediaTypeError) {
    return 415; // Unsupported Media Type
  }
+  if (error instanceof PackageFailedVerificationError) {
+    return 422; // Unprocessable Entity


Not sure about this response code for when a package fails verification, I was also tempted by Forbidden or Bad Request

400 Bad Request seems most appropriate here. 422 is usually more applicable when creating/updating some resource and the request body fails some validation check in my experience.

hop-dev · 2022-06-30T16:04:54Z

x-pack/plugins/fleet/server/services/epm/archive/index.ts

@@ -5,7 +5,7 @@
 * 2.0.
 */

-import type { AssetParts, InstallSource } from '../../../../common/types';


installSource was unused

hop-dev · 2022-06-30T16:06:34Z

x-pack/plugins/fleet/server/services/epm/packages/install.ts

+  if (verificationResult) {
+    savedObjectUpdate = {
+      ...savedObjectUpdate,
+      verification_key_id: null, // unset any previous verification key id


Not sure if this is a bit messy, if for example I install a packge and it is verified, then I reinstall it and it is "unknown", we need to unset the verification key ID, I've done this by nulling it.

This makes sense to me

hop-dev · 2022-06-30T16:11:14Z

x-pack/plugins/fleet/server/services/epm/registry/index.ts

@@ -256,32 +282,65 @@ function ensureContentType(archivePath: string) {
  return contentType;
 }

-export async function ensureCachedArchiveInfo(


this function was unused

elasticmachine · 2022-06-30T16:17:14Z

Pinging @elastic/fleet (Team:Fleet)

pzl

security_solution bits are 👌

nchaulet · 2022-07-04T12:52:56Z

x-pack/plugins/fleet/server/services/epm/archive/cache.ts

 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();


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

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

kpollich

Couple naming suggestions and one clarification around log level. Requested changes are nonblocking though, so marking as approved.

kpollich · 2022-07-06T20:28:56Z

x-pack/plugins/fleet/server/services/epm/packages/install.ts

+  if (verificationResult) {
+    savedObjectUpdate = {
+      ...savedObjectUpdate,
+      verification_key_id: null, // unset any previous verification key id


This makes sense to me

x-pack/plugins/fleet/server/services/epm/packages/package_verification.ts

x-pack/plugins/fleet/server/services/epm/registry/index.ts

kibana-ci · 2022-07-07T12:22:16Z

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`fleet`	1376	1379	+3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	5.2MB	5.2MB	+30.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`fleet`	111.3KB	111.3KB	+23.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id	before	after	diff
`epm-packages`	20	22	+2

Unknown metric groups

API count

id	before	after	diff
`fleet`	1507	1510	+3

History

💔 Build #55854 failed 36a17de
💔 Build #55740 failed 4236cbb5aec7bb18c7057a8bff9d2af5f43b6384
💚 Build #55070 succeeded 58ea0e8e2680f29137b45c62b909b7f73ba56a19
💚 Build #54959 succeeded e61cebda40f2e36587bdec9279f60508d8139cab
💔 Build #54718 failed eb62b01ffb2e2d6541f89a30cc55ce1a8986755d

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

cc @hop-dev

parkiino

onboarding-lifecycle-management team files look good to me

hop-dev added release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.4.0 labels Jun 30, 2022

hop-dev self-assigned this Jun 30, 2022

hop-dev commented Jun 30, 2022

View reviewed changes

hop-dev requested a review from kpollich June 30, 2022 16:16

hop-dev marked this pull request as ready for review June 30, 2022 16:17

hop-dev requested review from a team as code owners June 30, 2022 16:17

hop-dev requested review from pzl and parkiino June 30, 2022 16:17

pzl approved these changes Jun 30, 2022

View reviewed changes

hop-dev force-pushed the 133822-package-signature-verification branch from eb62b01 to e61cebd Compare July 4, 2022 10:56

nchaulet reviewed Jul 4, 2022

View reviewed changes

hop-dev force-pushed the 133822-package-signature-verification branch 2 times, most recently from 58ea0e8 to 4236cbb Compare July 6, 2022 19:41

kpollich approved these changes Jul 6, 2022

View reviewed changes

hop-dev added 9 commits July 7, 2022 11:19

add verifyPackageArchiveSignature fn

87e97aa

add feature flag

a604e6f

throw error if package verification fails

0f856b6

add verification info to saved object

03bd31e

update installation object with verify result

f60ca91

move to verification_status

065a078

store verification status in cache

512b2b1

send 422 if verification fails

d6b38a2

only get cached verification result if verify = true

f855e2c

hop-dev added 8 commits July 7, 2022 11:19

correctly set verififcation status keys

f92e02b

unset verification key ID when restarting installation

31550b6

self review

17008e7

style: neaten types

d755572

pr: Return 400 on verification error

a0711bf

fix tests

3dd6d3b

more test fixes

f5e4d72

PR feedback

36a17de

hop-dev force-pushed the 133822-package-signature-verification branch from 4236cbb to 36a17de Compare July 7, 2022 10:19

fix types

a87848d

parkiino approved these changes Jul 7, 2022

View reviewed changes

hop-dev merged commit dbc251e into elastic:main Jul 7, 2022

kibanamachine added the backport:skip This commit does not require backporting label Jul 7, 2022

hop-dev deleted the 133822-package-signature-verification branch July 7, 2022 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hop-dev commented Jun 30, 2022 •

edited

Loading

hop-dev Jun 30, 2022 •

edited

Loading

kpollich Jun 30, 2022

hop-dev Jun 30, 2022

hop-dev Jun 30, 2022

kpollich Jul 6, 2022

hop-dev Jun 30, 2022

elasticmachine commented Jun 30, 2022

pzl left a comment

nchaulet Jul 4, 2022

hop-dev Jul 6, 2022

kpollich left a comment

kpollich Jul 6, 2022

kibana-ci commented Jul 7, 2022

API count

parkiino left a comment

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

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

Conversation

hop-dev commented Jun 30, 2022 • edited Loading

Summary

Summary of changes

Testing steps

Checklist

hop-dev Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Jun 30, 2022

pzl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpollich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Jul 7, 2022

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

Saved Objects .kibana field count

API count

History

parkiino left a comment

Choose a reason for hiding this comment

hop-dev commented Jun 30, 2022 •

edited

Loading

hop-dev Jun 30, 2022 •

edited

Loading