Make issuer validation work with Kubernetes 1.21+ by default

[liggitt]

Issuer validation does not work by default with Kubernetes 1.21+, where bound service account tokens are enabled and the default issuer is no longer kubernetes.io/serviceaccount

It also is not possible to configure issuer validation to work during a 1.20 → 1.21 upgrade where tokens with both the legacy issuer and new issuer can be encountered.

Since a token review is performed, which ensures the token issuer is valid according to the kube-apiserver, the local issuer validation is an optimization. It seems like defaulting issuer validation off would be more compatible with Kubernetes 1.21+

[benashz]

@liggitt Thanks for opening this issue.

It also is not possible to configure issuer validation to work during a 1.20 → 1.21 upgrade where tokens with both the legacy issuer and new issuer can be encountered.

Very good to know!

Since a token review is performed, which ensures the token issuer is valid according to the kube-apiserver, the local issuer validation is an optimization. It seems like defaulting issuer validation off would be more compatible with Kubernetes 1.21+

Can you foresee any possible security implications for the proposal above?

[liggitt]

Can you foresee any possible security implications for the proposal above?

The tokenreview is authoritative for whether the issuer in the token is correct or not. That means that skipping the local issuer check would never allow a login when it should have rejected it, since in all cases, after local validation, a tokenreview is performed:

https://github.com/hashicorp/vault-plugin-auth-kubernetes/blob/e55a97be1ec88df6d6915fa555155536b5655e41/path_login.go#L95-L106

https://github.com/hashicorp/vault-plugin-auth-kubernetes/blob/e55a97be1ec88df6d6915fa555155536b5655e41/path_login.go#L404-L407

The other consideration is whether this permits DOS of the tokenreview client. There are two scenarios I see, depending on whether the JWT signature is verified locally:

https://github.com/hashicorp/vault-plugin-auth-kubernetes/blob/21abc8d40de83a6a62584e606aa5dc5a5ae0ed5a/path_login.go#L275-L278

1. The token signature is not checked locally before calling the token review
   • There is negligible increased DOS risk from malicious requests, since malicious requests could already attempt login with a crafted JWT with the correct issuer and an invalid signature
   • There is some exposure to DOS from accidental requests presenting tokens with an issuer that doesn't match
2. The token signature is checked locally before calling the token review
   • There is negligible increased DOS risk, since a token with a valid signature from the Kubernetes signer should be expected to have a valid issuer as well

[NLRemco]

As much as I agree with disabling local issuer validation would be a valid workaround for the issue. Why make this a temporary solution?

If the TokenReview is authoritative in deciding whether or not the issuer is valid (alongside some of the other claims): why not remove the audience and local issuer validation completely? Local validation would in my opinion be a way better fit to verify the integrity of the contained token, rather than doing the validation that Kubernetes is going to do either way.

[benashz]

Can you foresee any possible security implications for the proposal above?

The tokenreview is authoritative for whether the issuer in the token is correct or not. That means that skipping the local issuer check would never allow a login when it should have rejected it, since in all cases, after local validation, a tokenreview is performed:

True, so one more reason for the local issuer check's obsolescence.

The other consideration is whether this permits DOS of the tokenreview client. There are two scenarios I see, depending on whether the JWT signature is verified locally:

1. The token signature is not checked locally before calling the token review

   * There is negligible increased DOS risk from malicious requests, since malicious requests could already attempt login with a crafted JWT with the correct issuer and an invalid signature
   * There is some exposure to DOS from accidental requests presenting tokens with an issuer that doesn't match

2. The token signature is checked locally before calling the token review

   * There is negligible increased DOS risk, since a token with a valid signature from the Kubernetes signer should be expected to have a valid issuer as well

There are other identifiers that would have to be known in order to get the the token review API as well, so I agree that the DOS risk is negligible for both cases stated above.

Thank you for the detailed assessment @liggitt !

[benashz]

@liggitt @RemcoBuddelmeijer All good feedback. We will be looking at this issue internally.

[tomhjp]

As of Vault 1.9, issuer validation is now disabled by default, so I think we can close this one out as it should work by default, and users have a clean upgrade path if they disable issuer validation prior to upgrading kubernetes. Thanks for raising this @liggitt!