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

Add PKC JWKSet reloading to JWT authentication doc #88692

Merged
merged 16 commits into from
Aug 2, 2022

Conversation

justincr-elastic
Copy link
Contributor

@justincr-elastic justincr-elastic commented Jul 21, 2022

Doc changes for #88023

  • JWT realm now supports reloading PKC JWKs at runtime.
  • Use JWKS and UTF-8 keywords consistently.
  • Add tip in OIDC JWT section with link to Run-As for dealing with a certain non-OIDC use case.

Doc previews are available for a short time after a build

@justincr-elastic justincr-elastic added >docs General docs changes :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team v8.4.0 labels Jul 21, 2022
@elasticsearchmachine elasticsearchmachine added the Team:Docs Meta label for docs team label Jul 21, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Contributor

@lockewritesdocs lockewritesdocs left a comment

Choose a reason for hiding this comment

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

Left some suggested changes, a few of which I took liberty with based on some information that I had trouble parsing. Let me know if I misstated anything. Happy to iterate further!

x-pack/docs/en/security/authentication/jwt-realm.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/security/authentication/jwt-realm.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/security/authentication/jwt-realm.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/security/authentication/jwt-realm.asciidoc Outdated Show resolved Hide resolved
Comment on lines 461 to 473
IMPORTANT: All other JWT realm validations are checked before a signature failure can trigger a
PKC JWKS reload. Client authentication is strongly recommended. Narrow scoping of allowed algorithms,
allowed issuer, allowed audiences, and clock skew are all strongly recommended too. That way only trusted
client applications and realm-specific user JWT will be able to trigger PKC reload attempts.

If multiple JWT authentication signature failures happen at the same time within an {es} node, reloads are
combined to reduce the reloads sent externally.

IMPORTANT: If JWT signature failures trigger PKC JWKS reloads in different {es} nodes, reload
requests cannot be combined.

IMPORTANT: If JWT signature failures trigger PKC JWKS reloads in the same {es} node, but at different
times, separate reload requests will not be combined.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we can reorder this information and split it into two parts to make it flow a bit better:

First part

All other JWT realm validations are checked before a signature failure can
trigger a PKC JWKS reload. If multiple JWT authentication signature failures
occur simultaneously with a single {es} node, reloads are combined to reduce
the reloads that are sent externally.

Separate reload requests cannot be combined in these instances:

  • If JWT signature failures trigger PKC JWKS reloads in different {es} nodes
  • If JWT signature failures trigger PKC JWKS reloads in the same {es} node, but
    at different times

Second part

This statement is confusing me a bit; I'm not entirely sure what it's referring to:

That way only trusted client applications and realm-specific user JWT will be able to trigger PKC reload attempts.

I'm going to take an educated guess and suggest:

[IMPORTANT]

Enabling client authentication is strongly recommended. Only trusted client applications and realm-specific JWT users can trigger PKC reload attempts. Additionally, configuring the following <<ref-jwt-settings,JWT security settings>> is recommended:

  • allowed_issuer
  • allowed_audiences
  • allowed_clock_skew
  • allowed_signature_algorithms
    ====

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM.

If it helps, a more complete view of the order of how settings are used in the code is currently something like this below. Adding complete details would make things too complicated, so it is a balancing act to decide how much detail to put in. The 4 settings you listed above fall into 3-6, and in slightly different order. I don't know if that matters for that you want to achieve, so I will leave it up to you.

  1. client_authentication.type (with client_authentication.shared_secret, if type is shared_secret)
  2. jwt.cache.size (with jwt.cache.ttl and allowed_clock_skew, if cache is enabled)
  3. allowed_issuer
  4. allowed_audiences
  5. allowed_signature_algorithms
  6. allowed_clock_skew
  7. validate signature (with allowed_signature_algorithms, and one of pkc_jwkset_path, hmac_jwkset, or hmac_key)
  8. if-and-only-if JWT alg is RSA or EC, try reload (or wait if reload is in progress)
  9. if-and-only-if reload succeeds and new contents are different, revalidate signature

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @justincr-elastic -- I applied these changes in b5a9961.

@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:04
@mark-vieira mark-vieira added v8.5.0 and removed v8.4.0 labels Jul 27, 2022
@justincr-elastic
Copy link
Contributor Author

@elasticmachine update branch

@justincr-elastic
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/part-1-fips

@justincr-elastic
Copy link
Contributor Author

@elasticmachine update branch

@lockewritesdocs
Copy link
Contributor

@justincr-elastic, it seems that your changes in 8fe00f2 overwrote my changes in b5a9961. I'm unsure if that was intentional or if there was a GitHub collision 💥 Do you want me to reapply that commit?

Copy link
Contributor

@lockewritesdocs lockewritesdocs left a comment

Choose a reason for hiding this comment

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

LGTM 🦖 Thanks for iterating @justincr-elastic

@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Docs Meta label for docs team Team:Security Meta label for security team v8.4.0 v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants