spring-projects / spring-vault

Provides familiar Spring abstractions for HashiCorp Vault
https://spring.io/projects/spring-vault
Apache License 2.0
283 stars 186 forks source link

LifecycleAwareSessionManager doesn't differentiate between failed token renewals and intermittent network exceptions #646

Closed marcboudreau closed 2 years ago

marcboudreau commented 3 years ago

The LifecycleAwareSessionManager.renewToken() method invokes the LifecycleAwareSessionManager.doRenew(LifecycleAwareSessionManager.TokenWrapper) method inside of a try block and only catches instances of the RuntimeException class. If the execution enters into the catch block, the exception is logged and no further attempts are made to renew the token. In order for this code to be reliable, the Vault server cannot afford any downtime at all, because any network hiccup that causes an exception like the following for example:

Cannot renew token; nested exception is org.springframework.web.client.ResourceAccessException: I/O error on POST request for "https://_my_vault_server_/v1/auth/token/renew-self": Connection reset; nested exception is javax.net.ssl.SSLException: Connection reset

It seems to me that until the Vault server responded with an HTTP 403 error, there's no harm to continue scheduling renewal attempts until the token is known (by the spring-vault code) to expire.

mp911de commented 3 years ago

Since version 2.2, you can configure a LeaseStrategy. dropOnError is the default so you can customize the behavior for your case.

marcboudreau commented 3 years ago

Thank you for the prompt response. I'm new to this code base and hadn't explored what the LeaseStrategy could offer. However, I don't think that using an alternate LeaseStrategy by itself would resolve the issue. It would simply control whether the line https://github.com/spring-projects/spring-vault/blob/712d8ec0b667684c5f4b325c1cd7cae2919cabf4/spring-vault-core/src/main/java/org/springframework/vault/authentication/LifecycleAwareSessionManager.java#L213 gets called or not.

The renewToken method still returns false unconditionally line 224, which then breaks the chain in the scheduleRenewal() method https://github.com/spring-projects/spring-vault/blob/712d8ec0b667684c5f4b325c1cd7cae2919cabf4/spring-vault-core/src/main/java/org/springframework/vault/authentication/LifecycleAwareSessionManager.java#L341. It seems like if the returned value of getLeaseStrategy().shouldDrop(exception) could be used to also control the returned value of the renewToken function, then I could see this working quite well.

marcboudreau commented 3 years ago

It's been nearly 2 months since I've provided more feedback and yet that label is still attached to this issue. Is there some additional information that I can provide?

mp911de commented 3 years ago

I updated the ticket state. I hadn't the time to look into details yet.

turaleck commented 3 years ago

@marcboudreau asking you this question since it looks like you've studied the code. Isn't it true that if LeaseStrategy. dropOnError is true, then the token will be set to empty, causing the next request to initiate a new login request?

@mp911de feel free to shed some light on this as well?

In my case, I'd be OK if clearing the token forces the next vault request to first create a new token.

marcboudreau commented 3 years ago

@turaleck in some cases that would be fine. I'm thinking if using the github or kubernetes authentication method, where there is no explicit limit on the lifetime of the tokens used to complete the authentication. However, with authentication methods such as approle, the secret_id has a TTL, which limits its lifetime. There's also the possibility that a maximum number of uses was configured for the secret_id as well.

In our case, we are creating the Vault tokens as part of our deployment pipeline and setting the VAULT_TOKEN environment variable accordingly and the vault.token property is set to token.

We do have one of our senior developers, exploring alternatives this week, and in the event that we find something, we will share it.

mp911de commented 2 years ago

We now control the renewal by considering the outcome of LeaseStrategy to retain the token and attempt a renewal later on.

navinkaushik commented 8 months ago

Currently, we are using spring vault library and would like to use LeaseStrategy to retain the lease in case of an error. Is there any sample on how to do that ?