Use path-based lease revocation

[robinvandenberg]

[Problem]
I'm using the SecretLeaseContainer to manage a secret that rotates. On shutdown of the application SecretLeaseContainer .destroy() is called. Destroying it will revoke all leases.
I'm using LeaseEndpoints.SysLeases.
I think there is a bug in the following code in LeaseEndpoints.SysLeases

operations.exchange("sys/leases/revoke", HttpMethod.PUT, LeaseEndpoints.getLeaseRevocationBody(lease), Map.class, lease.getLeaseId());

Eventhough the leaseId is passed as uriVariables, it is not expanded as such, because that would require "sys/leases/revoke/{leaseId}"

The problem with calling the "sys/lease/revoke" endpoint, is that (my assumption/lack of vault knowledge) my NPA should not have the rights to have update permissions on this path.
This would make my NPA be able to revoke all leases that are handed out for different purposes to different NPAs.

[Expectation]
Revoking a lease on a new endpoint will do a PUT to:
"sys/leases/revoke/{leaseId}", as this complete path will be a path that can be modified by the NPA that also requested/read the lease.

[mp911de]

There is probably an issue with dangling lease.getLeaseId() that is not consumed by path expansion. The revocation PUT sends the lease_id through the body, hence the LeaseEndpoints.getLeaseRevocationBody(lease) call.

[mp911de]

See also: https://www.vaultproject.io/api/system/leases.html#revoke-lease

Did you experience an issue or did you go over the code? Care to explain what NPA is?

[robinvandenberg]

Indeed, the leaseId is not put into the path, as it will be if you reserve something in the path for it. (i.e. "/../{leaseId}")

It is indeed put in the body, but I have no idea why vault would need that. (maybe for legacy endpoints) Because as long as it's posting it on a very generic path (/sys/leases/revoke), i'm getting a 403 unless I give my service admin/god-mode.

To clarify with an example
I have an IAM role on an aws backend:'my-iam-role'
I have a service account, (through k8s) with a policy that gives access on /aws/creds/my-iam-role.
I get a lease with id "aws/creds/my-iam-role/somerandomstring"
On application close, the following HTTP request is done by LeaseEndpoints.SysLeases.doRevoke:

PUT /v1/sys/leases/revoke
{"lease_id":"aws/creds/my-iam-role/somerandomstring"}

It returns a 403 permission denied.
I don't want to give my serviceaccount a policy with [update] rights on the vault path "sys/leases/revoke", (although that fixed it).
This makes my service basically admin. With this right it's able to revoke any lease on this path. That includes leases from other processes using the vault.

A proper vault policy would be to give it [update] rights on the path:
"sys/leases/revoke/aws/creds/my-iam-role.

Therefore, I believe that the SecretLeaseContainer + LeaseEndpoints should do a PUT on
any endpoint that starts with "sys/leases/revoke/aws/creds/my-iam-role".
This would also be the behaviour if this:

		public void revoke(Lease lease, RestOperations operations) {

			operations.exchange("sys/leases/revoke", HttpMethod.PUT,
					LeaseEndpoints.getLeaseRevocationBody(lease), Map.class,
					lease.getLeaseId());
		}

would be changed to this:

		public void revoke(Lease lease, RestOperations operations) {

			operations.exchange("sys/leases/revoke/{leaseId}", HttpMethod.PUT,
					LeaseEndpoints.getLeaseRevocationBody(lease), Map.class,
					lease.getLeaseId());
		}

[mp911de]

The documentation lists only the body-based API as official API and we had several cases where we relied on undocumented API that broke functionality.

Also, constraining your policy to sys/leases/revoke/aws/creds/ does not really help because your application could remove any AWS lease, not only your own. So while this might seem an improvement, at first sight, it's not a real solution.

Is there a reason why there's no policy setting to allow revocation of own leases?
It makes sense to fix that issue in Vault. We should remove the dangling lease.getLeaseId() parameter on our side.

[robinvandenberg]

Oops, alll the <something-here> got replaced. I removed the '<>' in my previous comment.
The policy should specify sys/leases/revoke/aws/creds/my-iam-role, or that is how we want to set it up.

I agree that it isn't completely clear from vault API documentation, but this was what we've discovered by testing against vault.
If the leaseId is in the path, this policy is sufficient. If it is absent I need to have a policy allowing me to revoke any lease.

[mp911de]

Before we attempt addressing that issue in Spring Vault, please file an issue in the Vault project issue tracker to discuss it with the Vault folks.

[robinvandenberg]

hashicorp/vault#7536

[mp911de]

We're not going to implement this enhancement as per comment in the linked Vault ticket.

[ThomasKasene]

Sorry for bringing this back from the dead but is there a reason we can't use the /sys/leases/revoke-prefix/:prefix-endpoint?
Documentation: https://www.vaultproject.io/api/system/leases.html#revoke-prefix