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

fix(plugins): implement plugin JWTs, ping/prune #339

Merged
merged 19 commits into from
Mar 27, 2024

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Mar 21, 2024

Welcome to Cryostat3! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Fixes: #189
Fixes #10
Based on #337

Description of the change:

This fixes up the current half-baked discovery plugin system and brings it up to parity with 2.4's implementation. The JWT dependency and creation and validation logic was a copy-paste from 2.4 with minor adjustments for dependency injection and some refactoring at callsites or method signatures to work with or fit better into the Quarkus style.

  1. When discovery plugins register, the server produces a JWT (JSON web token) to supply back to the plugin, rather than producing a UUID as a fake token. The JWT embeds information about the plugin's location on the network, the server's identity, the plugin's ID and discovery realm, and the length of time that the token will be valid for. The server then signs and encrypts this token before passing it to the plugin.
  2. When a plugin attempts to perform any discovery API action such as publishing discovery information (overwriting previous data for the specified realm), checking registration, or unregistering, then its token is checked. If a token is not supplied the request is rejected. The server decrypts the token and checks its signature - if the signature doesn't match then the request is rejected (the token may have been tampered with). Then the server checks that the various claims within the token match - the plugin providing the token should match the plugin network location embedded in the token, the token should match the plugin's ID and realm, the token should have been issued by this particular Cryostat instance (already near-guaranteed by the fact that the cryptographic functions of signature validation and decryption succeeded), and in all cases other than deregistration that the token has not yet expired. If all of this checks out then the server services the request.
  3. The tokens are symmetrically encrypted and decrypted by the server. That is, only the server knows the encryption key - the token is an opaque, unintelligible string from the client's (plugin's) perspective.

Also,

  1. When plugins register, part of the information they provide is the callback URL where the Cryostat server should be able to reach the plugin, to verify that it is still up and reachable as well as to inform the plugin that it should refresh its token soon. This PR fixes this check on the server side so that it actually performs the check properly, using the correct credentials that the plugin has already registered into the server's encrypted credential keyring.
  2. Periodically (every 5 minutes starting at plugin registration, by default) the server will ping each registered plugin. The initial request above is a GET, whereas the periodic ping is a POST. The plugins are expected to simply respond to a GET with an OK response, whereas a POST request should trigger an OK response as well as trigger the plugin to follow up shortly with a request to refresh its token. This server-side periodic ping logic is also re-implemented in this PR. The new implementation is actually an enhancement over 2.4's implementation, since in 2.4 the timer is a global timer shared by all plugins regardless of their individual registration times. In this PR, each plugin gets its own scheduled task, so the refresh timing always starts 5 minutes after the plugin's registration, rather than any time within the first 5 minutes and then every 5 minutes following.

How to manually test:

  1. Run CRYOSTAT_IMAGE=quay.io... bash smoketest.bash... -Ot
  2. Verify that sample-app 1, 2, and 3, as well as quarkus-test-agent, are all up and running and discoverable HTTP targets. Let this setup run for some time to ensure that plugins (agents) are able to be pinged by the server, refresh their tokens, etc. The default ping period is 5 minutes which implies that plugins' tokens are valid for 10 minutes, so 10-15 minutes of run time should be sufficient to trigger all behaviour.
  3. Use curl or http and try to make requests to the various /api/v2.2/discovery plugin endpoints. Without a valid JWT you should not be permitted to make any actions now. Previously you would be able to ex. deregister a plugin or publish information on its behalf, even without knowing a valid JWT corresponding to that plugin and its realm.

@github-actions github-actions bot added dependent needs-triage Needs thorough attention from code reviewers labels Mar 21, 2024
@andrewazores andrewazores removed the needs-triage Needs thorough attention from code reviewers label Mar 21, 2024
@andrewazores andrewazores force-pushed the discovery-jwt branch 2 times, most recently from 50b3f4c to e1e8db8 Compare March 22, 2024 13:35
Copy link

This PR/issue depends on:

@andrewazores
Copy link
Member Author

/build_test

@andrewazores andrewazores marked this pull request as ready for review March 22, 2024 13:45
@andrewazores andrewazores requested a review from a team as a code owner March 22, 2024 13:45
Copy link

Workflow started at 3/22/2024, 9:46:09 AM. View Actions Run.

Copy link

CI build and push: All tests pass ✅ (JDK21)
https://github.com/cryostatio/cryostat3/actions/runs/8391294135

Copy link

No OpenAPI schema changes detected.

Copy link

CI build and push: At least one test failed ❌ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8391294135

@andrewazores
Copy link
Member Author

/build_test

@andrewazores andrewazores reopened this Mar 22, 2024
@github-actions github-actions bot added the needs-triage Needs thorough attention from code reviewers label Mar 22, 2024
Copy link

Workflow started at 3/22/2024, 9:57:13 AM. View Actions Run.

@andrewazores andrewazores removed the needs-triage Needs thorough attention from code reviewers label Mar 22, 2024
Copy link

CI build and push: All tests pass ✅ (JDK21)
https://github.com/cryostatio/cryostat3/actions/runs/8391427041

Copy link

No OpenAPI schema changes detected.

Copy link

CI build and push: All tests pass ✅ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8391427041

Copy link
Contributor

@aali309 aali309 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link

CI build and push: All tests pass ✅ (JDK21)
https://github.com/cryostatio/cryostat3/actions/runs/8395910102

Copy link

No OpenAPI schema changes detected.

Copy link

CI build and push: All tests pass ✅ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8395910102

@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 3/27/2024, 1:35:55 PM. View Actions Run.

Copy link

CI build and push: All tests pass ✅ (JDK21)
https://github.com/cryostatio/cryostat3/actions/runs/8456159715

Copy link

No OpenAPI schema changes detected.

Copy link

CI build and push: All tests pass ✅ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8456159715

@andrewazores andrewazores merged commit 1f57e15 into cryostatio:main Mar 27, 2024
8 checks passed
@andrewazores andrewazores deleted the discovery-jwt branch March 27, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] Update discovery plugin registration scheme to match 2.4 [Story] Discovery JWT
2 participants