JetBrains / teamcity-hashicorp-vault-plugin

TeamCity plugin to support HashiCorp Vault
Apache License 2.0
28 stars 18 forks source link

TeamCity server logs show unable to revoke token #34

Closed semiria closed 4 years ago

semiria commented 4 years ago

What happened: TeamCity server logs show an error stating it is unable to revoke a token after running a build that fetched secrets from Vault successfully.

What you expected to happen: TeamCity server logs show no error upon revoking token.

How to reproduce it (as minimally and precisely as possible):

  1. Do a minimal build configuration that gets secrets from Vault
  2. Run build configuration
  3. Go to TeamCity server logs
  4. Notice the error message in the logs

More information: TeamCity server logs:

[2020-07-28 17:19:43,162]   WARN [@2e0f68e1"; Normal executor 13] - ty.vault.server.VaultConnector - Failed to revoke token: org.springframework.web.client.HttpClientErrorException: 400 Bad Request

Plugin configuration:

Vault policy for approle:

path "secret/teamcity/build-configurations/*" {
  capabilities = ["read", "list"]
}

path "auth/token/lookup-self" {
    capabilities = ["read"]
}

path "auth/token/renew-self" {
    capabilities = ["update"]
}

path "auth/token/revoke-self" {
    capabilities = ["update"]
}

path "auth/token/revoke-accessor" {
    capabilities = ["update"]
}

Build configuration and parameters: image image

Related to comment: https://github.com/JetBrains/teamcity-hashicorp-vault-plugin/issues/19#issuecomment-458938660

semiria commented 4 years ago

It seems there is an attempt to revoke the token twice, once on the agent side after the build completes: https://github.com/JetBrains/teamcity-hashicorp-vault-plugin/blob/6791ca4a0b71a11f28f7e174e0c757253a00b958/common/src/main/java/org/jetbrains/teamcity/vault/support/LifecycleAwareSessionManager.java#L73-L80

And once on the server side after the build completes: https://github.com/JetBrains/teamcity-hashicorp-vault-plugin/blob/6791ca4a0b71a11f28f7e174e0c757253a00b958/server/src/main/kotlin/org/jetbrains/teamcity/vault/server/VaultConnector.kt#L73

So when the plugin tries to revoke the token on server side, the token has already been revoked and throws an error. This results in the server token never being revoked.

VladRassokhin commented 4 years ago

And once on the server side after the build completes:

That's done intentionally since logic on agent may break, agent may break, whatever else may happen and we must revoke token. That's why server tries to revoke token itself once build has finished.

Not sure why 400 is reported by Vault for revoking revoked token, I'd expect that request to be idempotent.

So solution is to ignore that warning in logs.

This results in the server token never being revoked.

There's no 'server token', server only hold accessor to agent's token

semiria commented 4 years ago

That's done intentionally since logic on agent may break, agent may break, whatever else may happen and we must revoke token. That's why server tries to revoke token itself once build has finished.

Makes perfect sense

Not sure why 400 is reported by Vault for revoking revoked token, I'd expect that request to be idempotent.

Revoking a non existing (or in this case already revoked) accessor will result in a 400 (for reference I'm using Vault v1.4.2):

curl -i --header "X-Vault-Token: xxx" --request POST --data '{"accessor": "yyy"}' https://vault.url.com/v1/auth/token/revoke-accessor
HTTP/2 400
cache-control: no-store
content-type: application/json
content-length: 59
date: Tue, 28 Jul 2020 20:00:24 GMT

In the plugin it results in an exception thrown and the revoke function on server side cannot complete: https://github.com/JetBrains/teamcity-hashicorp-vault-plugin/blob/6791ca4a0b71a11f28f7e174e0c757253a00b958/server/src/main/kotlin/org/jetbrains/teamcity/vault/server/VaultConnector.kt#L64-L81

The exception is thrown at line 73, the rest of the try block is not executed.

A new token is created on server side at line 71, but that new token is never revoked because of the exception at line 73.

So solution is to ignore that warning in logs.

The side effect of ignoring the warning in the logs is that the new token created on server side is never revoked.

VladRassokhin commented 4 years ago

Hm, seems I've already forgotten the code I've added to server side :/ Will take a look on PR

VladRassokhin commented 4 years ago

Meanwhile I've opened https://github.com/hashicorp/vault/issues/9636 since vault behaviour is inconsistent

VladRassokhin commented 4 years ago

Please try latest build

semiria commented 4 years ago

I don't see the error message in the server log anymore and I don't have extra tokens on each build run. Looks like it's fixed! Thank you

VladRassokhin commented 4 years ago

Good, thanks for feedback