hashicorp / vault-plugin-auth-kubernetes

Vault authentication plugin for Kubernetes Service Accounts
https://www.vaultproject.io/docs/auth/kubernetes.html
Mozilla Public License 2.0
206 stars 62 forks source link

The audience claim is not checked, the parameter `audience` in the Kubernetes auth role is ineffective #175

Closed maelvls closed 1 year ago

maelvls commented 1 year ago

⚠️ WARNING: ⚠️ the issue I reported is wrong. I mixed up the paramater audiences= with the correct audience=. Since Vault doesn't validate parameters (at least not in the Kubernetes plugin), I didn't realize that audiences= was ignored entirely. Thanks to Tom Proctor for finding this out.

Vault 1.12 (and all previous versions) has a bug that prevents checking the JWT audience. If you configure a Kubernetes auth role with the parameter audience set to vault and expect Vault to refuse JWTs that don't have this audience, Vault will still accept any audience:

vault write auth/kubernetes/role/cert-manager \
    bound_service_account_names=vault \
    bound_service_account_namespaces=cert-manager \
    audiences=vault   # <-- ignored 🔥

The Vault documentation indicates that the audience claim will be verified:

audience (string: "") – Optional Audience claim to verify in the JWT.

The code in path_login.go goes something like this:

parsedClaims := serviceAccount{}
_ = mapstructure.Decode(jwtClaims, &parsedClaims)
r, _ := tr.Review(ctx, client, jwtStr, parsedClaims.Audience)

Vault is checking that the audience in the JWT matches the audience in the JWT, instead of checking that the audience in the JWT matches the role's audience.

Context: as part of The Vault issuer can now be given a serviceAccountRef (PR 5502), we wanted to use the audience claim verification to prevent someone hijacking a service account by creating a cert-manager Issuer.

Reproducing

First, setup Kubernetes 1.25 with Vault 1.12.1:

kind create cluster --image kindest/node:v1.25.3
helm upgrade --install vault hashicorp/vault --version 0.23.0 --set server.dev.enabled=true --set server.logLevel=debug --set global.tlsDisable=true --wait
kubectl exec vault-0 -i -- vault auth enable kubernetes
kubectl exec vault-0 -i -- sh -c 'vault write auth/kubernetes/config \
   token_reviewer_jwt="$(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" \
   kubernetes_host="https://$KUBERNETES_PORT_443_TCP_ADDR:443" \
   kubernetes_ca_cert=@/var/run/secrets/kubernetes.io/serviceaccount/ca.crt \
   issuer=https://kubernetes.default.svc.cluster.local'

Let us create a role that expected the audience with-audience:

kubectl exec vault-0 -i -- vault write auth/kubernetes/role/with-audience \
   bound_service_account_names=default \
   bound_service_account_namespaces=default \
   audiences=vault

Now, let us login using a token that has the audience foobar. It should fail since it doesn't match vault:

$ kubectl exec vault-0 -i -- vault write auth/kubernetes/login role=something \
  jwt="$(kubectl create token -n default default --audience=foobar)"
Key                                       Value
---                                       -----
token                                     hvs.CAESIHdhpK5bTrOil

The login call succeeds: Vault ignores the fact that the audience doesn't match.

Using mitmproxy (I documented the procedure in https://hackmd.io/@maelvls/vault-audience-kubernetes-auth), I captured the TokenReview call that is made to the Kubernetes API server:

POST /apis/authentication.k8s.io/v1/tokenreviews HTTP/1.1
Authorization: Bearer abc
Content-Type: application/json

{
  "metadata": { "creationTimestamp": null },
  "spec": {
    "token": "eyJhbGciOiJSUzI1NiIsImtpZCI6Im9SYUp6YTM0LV82dUtBa0ZzYVI5TGIxOHNiRGdSYUhLUG5OSXpPN1gwY1EifQ.eyJhdWQiOlsiZm9vYmFyIl0sImV4cCI6MTY3MDk2Nzk3NiwiaWF0IjoxNjcwOTY0Mzc2LCJpc3MiOiJodHRwczovL2t1YmVybmV0ZXMuZGVmYXVsdC5zdmMuY2x1c3Rlci5sb2NhbCIsImt1YmVybmV0ZXMuaW8iOnsibmFtZXNwYWNlIjoiZGVmYXVsdCIsInNlcnZpY2VhY2NvdW50Ijp7Im5hbWUiOiJkZWZhdWx0IiwidWlkIjoiMWI1YTllMjEtMTE0Yy00YWY1LTg0NTMtYmE4ZWZjODNmY2JmIn19LCJuYmYiOjE2NzA5NjQzNzYsInN1YiI6InN5c3RlbTpzZXJ2aWNlYWNjb3VudDpkZWZhdWx0OmRlZmF1bHQifQ.RMx_1qireu3Ujn3GoSFktqPMBAUIFEXaCJLl6E9fO0CA8qCXYJoDLsDGg09NZ8Lpf01fETqI6arX6xiT5ElaAv4d0kdla1hx1MZcRGGFsjC1nUldyDaGr43NqZfaKNfva8a9edhdomc7IjGa8efiORcy1jCS6RDeGA3Bxkf81smc-IudjHCUvcco7iNXuIUj7Kugjv6ZVj5kKTJd3VUqyR846XzqHAuXV4L6cFKqbct03O3h4eSjA7yPpUmpAYrDXPjAsVqkbPnrczAylrG7MI7S7iPD5AXv8NZAmKLgBFw10xRErUU1ihvMVcSxPQrlMSoQPqIO7w8FcQlp7LE9Xg",
    "audiences": ["foobar"]
  },
  "status": { "user": {} }
}

The response is positive:

HTTP/1.1 201 Created

As you can see, Vault does not pass the audience vault to the TokenReview request. Instead, it copies whatever aud is in the JWT into the TokenReview request.

f4z3r commented 1 year ago

I had a look at this an I have to say I am a bit surprised. I checked the code and my understanding is that it should fail even before the TokenReview is sent. While it is true that the service account is populated with the audience of the JWT token before the review, the audience should be validated already here: https://github.com/hashicorp/vault-plugin-auth-kubernetes/blob/eabe60240605c4cc3c2d73038931c2fbf47ff6aa/path_login.go#L307

I wrote a simple unit test to check this and ran through with it in the debugger, and it seems to do exactly what you would expect ... I linked this issue in the commit.

tomhjp commented 1 year ago

Thanks for the detailed report, and also the follow-up investigation. I haven't gone too deep into this yet, but one thing I've noticed is a typo in the repro:

kubectl exec vault-0 -i -- vault write auth/kubernetes/role/with-audience \
  bound_service_account_names=default \
  bound_service_account_namespaces=default \
  audiences=vault

The audiences parameter there should be audience. Unfortunately, providing the wrong parameters doesn't provide an error, it just gets silently ignored, so it's acting as though the audience hasn't been set at all. When I try the repro with audience instead, I get an error on authenticating. I need to refresh my memory on the specifics of audience handling though, as I'm still not confident there isn't a bug here.

maelvls commented 1 year ago

Hey Jakob and Tom,

Tom, you are correct: I incorrectly used audiences= and Vault silently ignored it. I ran my tests with the correct audience=, and I am now getting the expected error message:

* invalid audience (aud) claim: audience claim does not match any expected audience

Jakob, thank you for the unit test, it definitely proves that this is a false alarm. My apologies!

tomhjp commented 1 year ago

Hey @maelvls, I just wanted to say thanks for highlighting this - after taking a closer look, I think I identified some related improvements we could make. If you're interested, I'd welcome extra eyes: #179