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

feat(kuma-cp) consolidate tokens logic to support expiration, rotation, revocation and RSA256 #3376

Merged
merged 4 commits into from
Nov 30, 2021

Conversation

jakubdyszkiewicz
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz commented Nov 29, 2021

Summary

This PR consolidates the logic of JWT tokens in the project, to have one standard for all the tokens.

I refactored the logic to core/tokens to serve the functionality as a "library" for token implementations (dataplane token, zone ingress token, user token).

All the tokens now have

  • Expiration. Not required for now for backwards compatibility (aside for user token, because it was required when we introduced it). If not specified then it's valid for 10 years.
  • Revocation. In case that token is compromised, we can now put the ID of the token in revocation list in Secret/GlobalSecret.
  • Signing Key rotation. In case that signing key is compromised, you can now rotate it without downtime.
  • RSA256 signing method. All the tokens are now signed with RSA256. This is future proof if we want to build generating token offline (kumactl or other system), but only validate it in Kuma with public RSA key.
    All new tokens are generated using RSA256, but we still support HMAC256 for backwards compatibility.
  • PEM-encoded signing key. Signing key (private RSA key) was encoded in GlobalSecret/Secert using ASN.1. It is now encoded using PEM which suppose to be easier to manage.
    We still support ASN.1 signing keys for backwards compatibility.

All those features were available for User Token already but not for other tokens.

Issues resolved

Fix #2955
Fix #3197

Documentation

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

  • No need for changes in UPGRADE.md
  • The change is backwards compatible.

…on, revocation

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team as a code owner November 29, 2021 11:23
@jpeach jpeach changed the title feat(kuma-cp) consolildate tokens logic to support expiration, rotation, revocation and RSA256 feat(kuma-cp) consolidate tokens logic to support expiration, rotation, revocation and RSA256 Nov 30, 2021
Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

This is a great improvement. Just some little nits and suggestions.

pkg/core/tokens/meshed_signing_key_accessor.go Outdated Show resolved Hide resolved
pkg/core/tokens/meshed_signing_key_manager.go Show resolved Hide resolved
pkg/core/tokens/meshed_signing_key_manager.go Outdated Show resolved Hide resolved
pkg/core/tokens/meshed_signing_key_manager.go Outdated Show resolved Hide resolved
pkg/core/tokens/revocations.go Show resolved Hide resolved
pkg/tokens/builtin/issuer/validator.go Show resolved Hide resolved
pkg/defaults/mesh/mesh.go Outdated Show resolved Hide resolved
pkg/core/tokens/validator.go Show resolved Hide resolved
pkg/core/tokens/signing_key.go Outdated Show resolved Hide resolved
pkg/core/tokens/signing_key_manager.go Outdated Show resolved Hide resolved
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #3376 (03fa106) into master (16f0848) will increase coverage by 0.13%.
The diff coverage is 78.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3376      +/-   ##
==========================================
+ Coverage   52.13%   52.27%   +0.13%     
==========================================
  Files         932      939       +7     
  Lines       54547    54618      +71     
==========================================
+ Hits        28439    28552     +113     
+ Misses      23882    23846      -36     
+ Partials     2226     2220       -6     
Impacted Files Coverage Δ
pkg/core/rest/errors/error_handler.go 70.53% <0.00%> (ø)
pkg/kds/context/context.go 52.63% <0.00%> (ø)
pkg/kds/zone/components.go 16.92% <0.00%> (ø)
pkg/tokens/builtin/zoneingress/token.go 0.00% <0.00%> (ø)
pkg/tokens/builtin/zoneingress/validator.go 30.00% <30.00%> (ø)
pkg/defaults/components.go 87.75% <33.33%> (-0.49%) ⬇️
pkg/defaults/mesh/signing_key.go 60.00% <33.33%> (ø)
pkg/tokens/builtin/zoneingress/issuer.go 42.85% <42.85%> (+37.30%) ⬆️
pkg/core/tokens/default_signing_key.go 68.42% <57.14%> (ø)
pkg/tokens/builtin/server/webservice.go 60.91% <61.53%> (-0.02%) ⬇️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16f0848...03fa106. Read the comment docs.

pkg/core/tokens/default_signing_key.go Show resolved Hide resolved
pkg/core/tokens/issuer_test.go Outdated Show resolved Hide resolved
pkg/core/tokens/signing_key_manager.go Outdated Show resolved Hide resolved
pkg/core/tokens/revocations.go Outdated Show resolved Hide resolved
pkg/core/tokens/signing_key_manager.go Outdated Show resolved Hide resolved
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@jakubdyszkiewicz jakubdyszkiewicz merged commit 8708885 into master Nov 30, 2021
@jakubdyszkiewicz jakubdyszkiewicz deleted the chore/refactor-tokens branch November 30, 2021 14:52
mergify bot pushed a commit that referenced this pull request Nov 30, 2021
…n, revocation and RSA256 (#3376)

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
(cherry picked from commit 8708885)

# Conflicts:
#	pkg/defaults/mesh/signing_key.go
#	pkg/tokens/builtin/issuer/signing_key.go
jakubdyszkiewicz added a commit that referenced this pull request Dec 2, 2021
…n, revocation and RSA256 (#3376)

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
jakubdyszkiewicz pushed a commit that referenced this pull request Dec 2, 2021
…n, revocation and RSA256 (#3376)

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possibility to expire dataplane tokens Consolidate JWT issuance and key management
4 participants