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

[EDR Workflows][Fleet] Improve uninstall token validation in Fleet setup #175679

Conversation

gergoabraham
Copy link
Contributor

@gergoabraham gergoabraham commented Jan 26, 2024

Summary

This PR is a proposal for improving Uninstall Token validation inside Fleet. Any feedback from @elastic/fleet team is very welcome 🙌

What it does:

  • moves Uninstall Token generation and validation from Fleet Setup to Fleet plugin start in order to not perform these steps every time POST api/fleet/setup is called
  • adds a summary to issues with uninstall tokens to Kibana logs
    e.g.
[2024-01-30T12:53:31.803+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,fb652173-7e07-47c1-8f42-e469d789d7ca": Unsupported state or unable to authenticate data
[2024-01-30T12:53:31.885+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,2c34c587-f81c-4534-80e3-f45fb0d0c3f9": Unsupported state or unable to authenticate data
[2024-01-30T12:53:31.886+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,e4d8cf22-0d8d-43c6-b21a-e11e7aea9932": Unsupported state or unable to authenticate data
[2024-01-30T12:53:32.522+01:00][WARN ][plugins.fleet] Failed to decrypt 3 of 1130 Uninstall Token(s)

Checklist

Delete any items that are not applicable to this PR.

@gergoabraham gergoabraham added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Defend Workflows “EDR Workflows” sub-team of Security Solution labels Jan 26, 2024
@gergoabraham gergoabraham self-assigned this Jan 26, 2024
@gergoabraham gergoabraham force-pushed the improve-uninstall-token-validation-in-fleet-setup branch 2 times, most recently from fd983c2 to 472cf04 Compare January 26, 2024 13:59
@gergoabraham
Copy link
Contributor Author

/ci

@gergoabraham gergoabraham force-pushed the improve-uninstall-token-validation-in-fleet-setup branch from f6c1354 to 5a18b3c Compare January 30, 2024 11:41
@gergoabraham gergoabraham force-pushed the improve-uninstall-token-validation-in-fleet-setup branch from 5a18b3c to a846f8d Compare January 30, 2024 11:57
@gergoabraham gergoabraham marked this pull request as ready for review January 30, 2024 13:42
@gergoabraham gergoabraham requested a review from a team as a code owner January 30, 2024 13:42
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

@@ -583,6 +583,9 @@ export class FleetPlugin
}
);

// initialize (generate/encrypt/validate) Uninstall Tokens asynchronously
this.initializeUninstallTokens();
Copy link
Contributor

Choose a reason for hiding this comment

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

The change looks good to me, my only concern is what happens if there is an unexpected error coming from this function, it would stop the Fleet plugin from starting. We should probably make sure to catch any possible errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that makes sense, thanks for the suggestion!
added it here, and unified the error handling of the two functions for generation and validation: a170752

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jan 30, 2024
Copy link
Member

@joeypoon joeypoon left a comment

Choose a reason for hiding this comment

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

🙆‍♂️


if (appContextService.getEncryptedSavedObjectsSetup()?.canEncrypt) {
logger.debug('Checking for and encrypting plain text uninstall tokens');
await appContextService.getUninstallTokenService()?.encryptTokens();
Copy link
Member

Choose a reason for hiding this comment

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

not a big speed gain but we can probably leave this async since nothing depends on it being encrypted.

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 left the await here, so every task that possibly modify tokens (generation and encryption) finishes first, before validation starts on the same dataset, so we validate the final results

the Fleet plugin start does not wait for any of these sequential steps, though

Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

LGTM

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #69 / security app Management "after all" hook in "Management"
  • [job] [logs] FTR Configs #69 / security app Management "before all" hook in "Management"

Metrics [docs]

✅ unchanged

History

  • 💛 Build #189978 was flaky 67ef25f571e60400ffec8ad437fd5f9bd1480f5b

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

cc @gergoabraham

@gergoabraham gergoabraham merged commit 17239f2 into elastic:main Feb 1, 2024
20 checks passed
@gergoabraham gergoabraham deleted the improve-uninstall-token-validation-in-fleet-setup branch February 6, 2024 08:28
fkanout pushed a commit to fkanout/kibana that referenced this pull request Feb 7, 2024
…tup (elastic#175679)

## Summary

This PR is a proposal for improving Uninstall Token validation inside
Fleet. Any feedback from @elastic/fleet team is very welcome 🙌

What it does:
- moves Uninstall Token generation and validation from Fleet Setup to
Fleet plugin start in order to not perform these steps every time `POST
api/fleet/setup` is called
- adds a summary to issues with uninstall tokens to Kibana logs
e.g.
```
[2024-01-30T12:53:31.803+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,fb652173-7e07-47c1-8f42-e469d789d7ca": Unsupported state or unable to authenticate data
[2024-01-30T12:53:31.885+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,2c34c587-f81c-4534-80e3-f45fb0d0c3f9": Unsupported state or unable to authenticate data
[2024-01-30T12:53:31.886+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,e4d8cf22-0d8d-43c6-b21a-e11e7aea9932": Unsupported state or unable to authenticate data
[2024-01-30T12:53:32.522+01:00][WARN ][plugins.fleet] Failed to decrypt 3 of 1130 Uninstall Token(s)
```

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…tup (elastic#175679)

## Summary

This PR is a proposal for improving Uninstall Token validation inside
Fleet. Any feedback from @elastic/fleet team is very welcome 🙌

What it does:
- moves Uninstall Token generation and validation from Fleet Setup to
Fleet plugin start in order to not perform these steps every time `POST
api/fleet/setup` is called
- adds a summary to issues with uninstall tokens to Kibana logs
e.g.
```
[2024-01-30T12:53:31.803+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,fb652173-7e07-47c1-8f42-e469d789d7ca": Unsupported state or unable to authenticate data
[2024-01-30T12:53:31.885+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,2c34c587-f81c-4534-80e3-f45fb0d0c3f9": Unsupported state or unable to authenticate data
[2024-01-30T12:53:31.886+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,e4d8cf22-0d8d-43c6-b21a-e11e7aea9932": Unsupported state or unable to authenticate data
[2024-01-30T12:53:32.522+01:00][WARN ][plugins.fleet] Failed to decrypt 3 of 1130 Uninstall Token(s)
```

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
…tup (elastic#175679)

## Summary

This PR is a proposal for improving Uninstall Token validation inside
Fleet. Any feedback from @elastic/fleet team is very welcome 🙌

What it does:
- moves Uninstall Token generation and validation from Fleet Setup to
Fleet plugin start in order to not perform these steps every time `POST
api/fleet/setup` is called
- adds a summary to issues with uninstall tokens to Kibana logs
e.g.
```
[2024-01-30T12:53:31.803+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,fb652173-7e07-47c1-8f42-e469d789d7ca": Unsupported state or unable to authenticate data
[2024-01-30T12:53:31.885+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,2c34c587-f81c-4534-80e3-f45fb0d0c3f9": Unsupported state or unable to authenticate data
[2024-01-30T12:53:31.886+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,e4d8cf22-0d8d-43c6-b21a-e11e7aea9932": Unsupported state or unable to authenticate data
[2024-01-30T12:53:32.522+01:00][WARN ][plugins.fleet] Failed to decrypt 3 of 1130 Uninstall Token(s)
```

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
@gergoabraham
Copy link
Contributor Author

tested on 8.13 BC4:

  • with 1500 policies,
  • deleting them all -> they are only regenerated on Kibana restart
  • messing up some of them -> no issues during fleet setup
    all good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Fleet Team label for Observability Data Collection Fleet team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants