[Fleet] Support preconfigured output secrets #170259

hop-dev · 2023-10-31T17:10:26Z

Summary

This PR adds support for outputs with secrets preconfigured in the kibana.yml config file.

As Kibana needs to compare the value of the secret to manage updates, a hash of the value is stored in the output's saved object. The implementation follows option 2 in Infosec's recommendations with the Argon2id algorithm.

See here for information about the argon2 Node package and here for the config options. Here, argon2 was configured with the recommended m=19456 (19 MiB), t=2, p=1 (for some reason, timeCost cannot be set to less than 2).

Closes #166360

Testing

Ensure the outputSecretsStorage experimental feature is enabled.

Add the following to your kibana config:

xpack.fleet.outputs:
  - id: my-logstash-output-with-a-secret
    name: preconfigured logstash output with a secret
    type:  logstash
    hosts: ["localhost:9999"]
    ssl:
      certificate: xxxxxxxxxx
    secrets:
      ssl:
        key: thisissecret

Verify the secret has been correctly created, e.g. by issuing a GET .fleet-secrets/_search request in Dev Tools: the secret should be listed there.
Change the preconfigured value and wait for kibana to restart: the secret should be updated with the new value.

apmmachine · 2023-10-31T17:10:44Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

…te --fix'

… src/core/server/integration_tests/ci_checks'

elasticmachine · 2023-11-16T14:56:46Z

Pinging @elastic/fleet (Team:Fleet)

jillguyonnet · 2023-11-16T15:16:08Z

@elasticmachine merge upstream

juliaElastic · 2023-11-20T10:49:39Z

x-pack/plugins/fleet/server/services/preconfiguration/outputs.test.ts

+      expect.anything()
+    );
+    expect(mockedOutputService.update).not.toBeCalled();
+    // expect(spyAgentPolicyServicBumpAllAgentPoliciesForOutput).not.toBeCalled();


nit: should this line be removed?

juliaElastic · 2023-11-20T10:57:16Z

x-pack/plugins/fleet/server/services/preconfiguration/outputs.ts

@@ -221,6 +338,7 @@ function isPreconfiguredOutputDifferentFromCurrent(
    isDifferent(existingOutput.config_yaml, preconfiguredOutput.config_yaml) ||
    isDifferent(existingOutput.proxy_id, preconfiguredOutput.proxy_id) ||
    isDifferent(existingOutput.allow_edit ?? [], preconfiguredOutput.allow_edit ?? []) ||
-    kafkaFieldsAreDifferent()
+    (await kafkaFieldsAreDifferent()) ||
+    (await logstashFieldsAreDifferent())


Could we cover kafka and logstash output changes in unit tests?

Thanks for that @juliaElastic - your suggestion made me realise the tests were not properly testing the comparison of hashes. I pushed a commit that addresses that.

juliaElastic

Looks good, added a comment to improve test coverage.

jloleysens · 2023-11-20T15:29:40Z

packages/kbn-check-mappings-update-cli/current_mappings.json

+              },
+              "hash": {
+                "type": "keyword",
+                "index": false


hash field should be excluded from SO mappings as it is not powering any search.

I might be missing something here, maybe @juliaElastic can help me clarify.

As I understand, this means removing the hash field from x-pack/plugins/fleet/server/saved_objects/index.ts altogether. This makes sense as we're not searching on the hash, but I'm unsure whether we'll be able to retrieve the hash in all scenarios. FWIW I've tested the repro steps locally and it does work.

Solved via Slack - for reference, this tutorial was useful: https://github.com/elastic/kibana/blob/main/dev_docs/tutorials/saved_objects.mdx.

jillguyonnet · 2023-11-21T11:20:38Z

@elasticmachine merge upstream

…te --fix'

jillguyonnet · 2023-11-21T16:20:25Z

@elasticmachine merge upstream

… src/core/server/integration_tests/ci_checks'

jloleysens

thanks for the core-related update! changes LGTM

jillguyonnet · 2023-11-22T10:48:22Z

@elasticmachine merge upstream

jillguyonnet · 2023-11-22T15:07:52Z

@elasticmachine merge upstream

kpollich

The logic + tests all look great to me. I did have one comment about improving some existing log messages as part of this PR, but otherwise LGTM! 🚀

x-pack/plugins/fleet/server/services/preconfiguration/outputs.ts

kibana-ci · 2023-11-22T19:02:01Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 7707de5

Failed CI Steps

Metrics [docs]

✅ unchanged

History

💔 Build #177910 failed ce1b576
💔 Build #177864 failed b2921ce
💔 Build #177517 failed 93441a5
💔 Build #177258 failed aa387ae
💚 Build #176573 succeeded a1718ce

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

cc @jillguyonnet

mistic · 2023-11-23T18:11:44Z

@hop-dev I had the need to reverted this PR at 78f7b80. @pheyos found this commit was breaking tests the QA team have in place on Windows. The reason for this was that this pr added a native module as a production dependency (argon2). Is it possible for you to avoid using a native module dependency and found a replacement for it? We really like to avoid adding new ones other than the re2 we are currently using because of the implications those dependencies have. Otherwise we will have to patch the build process to support this argon2 native module

This reverts commit 1f7a527.

hop-dev · 2023-11-24T08:46:33Z

@jillguyonnet 😓

I have done a search it looks like all node argon2 libraries use the native argon2. It may be worth looking at the other hashes recomended by the security team and see if there is a better lib?

jillguyonnet · 2023-11-27T09:20:54Z

Hey @hop-dev I'm back from PTO, taking a look at this now 👀

jillguyonnet · 2023-11-27T14:59:01Z

It looks like we could use scrypt (second recommendation) as it has a built-in implementation within the crypto module.

@mistic @hop-dev Would you have any objections to that?

## Summary Support for outputs with secrets preconfigured in `kibana.yml`. This was already implemented in #170259, which had to be reverted as the `argon2` package was causing Windows tests to test (cf. [this comment](#170259 (comment))). The present implementation follows [option 2 in Infosec's recommendations](elastic/infosec#14853 (comment)) with the Scrypt algorithm using the following params: N=2^14 (16 MiB), r=8 (1024 bytes), p=5. Note that Scrypt is built-in within the `crypto` module (see [here](https://nodejs.org/api/crypto.html#cryptoscryptpassword-salt-keylen-options-callback) for documentation). Closes #166360 ### Testing 1. Ensure the [`outputSecretsStorage` experimental feature ](https://github.com/elastic/kibana/blob/fd4fdb01bcb011c0968a4ccab7ba739c3195016a/x-pack/plugins/fleet/common/experimental_features.ts#L26)is enabled. 2. Add the following to your kibana config: ``` xpack.fleet.outputs: - id: my-logstash-output-with-a-secret name: preconfigured logstash output with a secret type: logstash hosts: ["localhost:9999"] ssl: certificate: xxxxxxxxxx secrets: ssl: key: thisissecret ``` 3. Verify the secret has been correctly created, e.g. by issuing a `GET .fleet-secrets/_search` request in Dev Tools: the secret should be listed there. 4. Change the preconfigured value and wait for kibana to restart: the secret should be updated with the new value. --------- Co-authored-by: Mark Hopkin <mark.hopkin@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

hop-dev added 4 commits October 25, 2023 15:35

add node-argon2

dac8a85

add hash to saved object mapping

d70b9dd

add hash to output type

cf99bed

use argon2 to diff preconfigured secrets

38d4e4a

hop-dev mentioned this pull request Oct 31, 2023

[Fleet] Support preconfigured output secrets #166360

Closed

kibanamachine and others added 7 commits October 31, 2023 17:35

[CI] Auto-commit changed files from 'node scripts/check_mappings_upda…

f119d43

…te --fix'

[CI] Auto-commit changed files from 'node scripts/jest_integration -u…

9e3d696

… src/core/server/integration_tests/ci_checks'

Merge branch 'main' into HEAD

65afd55

[CI] Auto-commit changed files from 'node scripts/jest_integration -u…

32d3d08

… src/core/server/integration_tests/ci_checks'

Configure argon2 with recommended params

b8b5b7c

Add and fix preconfigured output tests

85aaaf2

Add type to hash param

49fdb03

jillguyonnet self-assigned this Nov 16, 2023

jillguyonnet added Team:Fleet Team label for Observability Data Collection Fleet team v8.12.0 release_note:feature Makes this part of the condensed release notes backport:skip This commit does not require backporting labels Nov 16, 2023

jillguyonnet mentioned this pull request Nov 16, 2023

Support preconfigured output secrets using HMAC #171396

Closed

2 tasks

Fix typo

3429e0f

jillguyonnet marked this pull request as ready for review November 16, 2023 14:56

jillguyonnet requested review from a team as code owners November 16, 2023 14:56

kpollich self-requested a review November 16, 2023 15:12

Merge branch 'main' into 166360-preconfigured-output-secrets

a1718ce

juliaElastic reviewed Nov 20, 2023

View reviewed changes

juliaElastic approved these changes Nov 20, 2023

View reviewed changes

jloleysens requested changes Nov 20, 2023

View reviewed changes

jillguyonnet added 2 commits November 21, 2023 11:19

Fix and add unit tests

279f3f7

Exclude hash from SO mappings

99f10d9

kibanamachine added 2 commits November 21, 2023 06:20

Merge branch 'main' into 166360-preconfigured-output-secrets

aacc26d

[CI] Auto-commit changed files from 'node scripts/check_mappings_upda…

aa387ae

…te --fix'

kibanamachine and others added 4 commits November 21, 2023 11:20

Merge branch 'main' into 166360-preconfigured-output-secrets

93441a5

Add extra unit tests

8509230

Remove SO mappings

38e12f0

[CI] Auto-commit changed files from 'node scripts/jest_integration -u…

b2921ce

… src/core/server/integration_tests/ci_checks'

jloleysens approved these changes Nov 22, 2023

View reviewed changes

Merge branch 'main' into 166360-preconfigured-output-secrets

ce1b576

Merge branch 'main' into 166360-preconfigured-output-secrets

7baa692

kpollich approved these changes Nov 22, 2023

View reviewed changes

x-pack/plugins/fleet/server/services/preconfiguration/outputs.ts Outdated Show resolved Hide resolved

Improve log messages

7707de5

jillguyonnet merged commit 1f7a527 into elastic:main Nov 23, 2023
29 checks passed

mistic added the reverted label Nov 23, 2023

kibanamachine pushed a commit to XavierM/kibana that referenced this pull request Nov 23, 2023

Revert "[Fleet] Support preconfigured output secrets (elastic#170259)"

78f7b80

This reverts commit 1f7a527.

jillguyonnet mentioned this pull request Nov 28, 2023

[Fleet] Support preconfigured output secrets (scrypt edition) #172041

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Support preconfigured output secrets #170259

[Fleet] Support preconfigured output secrets #170259

hop-dev commented Oct 31, 2023 •

edited by jillguyonnet

apmmachine commented Oct 31, 2023

elasticmachine commented Nov 16, 2023

jillguyonnet commented Nov 16, 2023

juliaElastic Nov 20, 2023

juliaElastic Nov 20, 2023

jillguyonnet Nov 21, 2023

juliaElastic left a comment

jloleysens Nov 20, 2023

jillguyonnet Nov 21, 2023

jillguyonnet Nov 22, 2023

jillguyonnet commented Nov 21, 2023

jillguyonnet commented Nov 21, 2023

jloleysens left a comment

jillguyonnet commented Nov 22, 2023

jillguyonnet commented Nov 22, 2023

kpollich left a comment

kibana-ci commented Nov 22, 2023

mistic commented Nov 23, 2023

hop-dev commented Nov 24, 2023 •

edited

jillguyonnet commented Nov 27, 2023

jillguyonnet commented Nov 27, 2023

[Fleet] Support preconfigured output secrets #170259

[Fleet] Support preconfigured output secrets #170259

Conversation

hop-dev commented Oct 31, 2023 • edited by jillguyonnet

Summary

Testing

apmmachine commented Oct 31, 2023

🤖 GitHub comments

elasticmachine commented Nov 16, 2023

jillguyonnet commented Nov 16, 2023

juliaElastic Nov 20, 2023

Choose a reason for hiding this comment

juliaElastic Nov 20, 2023

Choose a reason for hiding this comment

jillguyonnet Nov 21, 2023

Choose a reason for hiding this comment

juliaElastic left a comment

Choose a reason for hiding this comment

jloleysens Nov 20, 2023

Choose a reason for hiding this comment

jillguyonnet Nov 21, 2023

Choose a reason for hiding this comment

jillguyonnet Nov 22, 2023

Choose a reason for hiding this comment

jillguyonnet commented Nov 21, 2023

jillguyonnet commented Nov 21, 2023

jloleysens left a comment

Choose a reason for hiding this comment

jillguyonnet commented Nov 22, 2023

jillguyonnet commented Nov 22, 2023

kpollich left a comment

Choose a reason for hiding this comment

kibana-ci commented Nov 22, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

History

mistic commented Nov 23, 2023

hop-dev commented Nov 24, 2023 • edited

jillguyonnet commented Nov 27, 2023

jillguyonnet commented Nov 27, 2023

hop-dev commented Oct 31, 2023 •

edited by jillguyonnet

hop-dev commented Nov 24, 2023 •

edited