GCK KMS Quota exceeded

[yesid-bocanegra]

We are using the vault integration with KMS to encrypt and decrypt text. We are using the recommended approach for it, but we have found that an error is thrown under load. we are running some load tests and vault start throwing the following after a couple of seconds:

{
   "errors": [
        "1 error occurred:\n\t* failed to get underlying crypto key: rpc error: code = ResourceExhausted desc = Quota exceeded for quota metric 'Read requests' and limit 'Read requests per minute' of service '[cloudkms.googleapis.com](http://cloudkms.googleapis.com/)' for consumer 'project_number:310079220581'.\nerror details: name = ErrorInfo reason = RATE_LIMIT_EXCEEDED domain = [googleapis.com](http://googleapis.com/) metadata = map[consumer:projects/310079220581 quota_limit:ReadRequestsPerMinutePerProject quota_limit_value:300 quota_location:global quota_metric:[cloudkms.googleapis.com/read_requests](http://cloudkms.googleapis.com/read_requests) service:[cloudkms.googleapis.com](http://cloudkms.googleapis.com/)]\nerror details: name = Help desc = Request a higher quota limit. url = [[https://cloud.google.com/docs/quota#requesting_higher_quota](https://cloud.google.com/docs/quota#requesting_higher_quota])](https://cloud.google.com/docs/quota#requesting_higher_quota](https://cloud.google.com/docs/quota%23requesting_higher_quota]))"
    ]
}

It looks like there is a Read Requests going under the hood and that is exhausting the quota for that endpoint, we have checked the code and we think this might the culprit: path_decrypt.go:122

Should some form of cache should be used in this scenario to keep under control the amount of calls to the Read Request operation?

Please let me know if more details are necessary to diagnose this error.

Vault versions affected = [1.10.3, 1.13.3]
Java version: 11
Spring vault core version: 2.3.3

[yesid-bocanegra]

@robmonte do you know who might be able to help check this issue?

[robmonte]

Hi @yesid-bocanegra, thanks for the suggestion!

We think this is a great idea, caching seems appropriate as the Key Purpose cannot be changed after it is created. It would be very helpful if you could also provide any further details you might uncover that shows this key lookup is indeed the resource exhausting the read quota, as it would speed up any validations done.

If you or any other community members wish to open a PR for this, it would be very welcome! 😄 I can create an internal ticket for us to prioritize this at a later date, but cannot provide any guesses as to when that may occur. If a PR does get submitted it may help with said prioritization.

[yesid-bocanegra]

Hi @robmonte thanks for the swift reply, we have reviewed the possible causes and we believe that might be the source, looking at the code, the error is caught and the predefined message is prefixed to KMS's error response.

We have checked the GCP KMS api docs and found that kmspb.GetCryptoKeyRequest might be the one doing the read request, this method calls to googleapis/cloud/kms/v1#GetCryptoKeyRequest, which makes a gRPC request to KeyManagementService.GetCryptoKey using this schema kms/apiv1/kmspb#GetCryptoKeyRequest. That request uses GetCryptoKeyRequest and according to the docs that api call should be issuing a cryptoKeys.get.

We are checking if we could send a PR for this. Any suggestion on the cache strategy? we don't know vault's internals and maybe there is a storage option that can hold this information.

[robmonte]

@yesid-bocanegra The caching is likely fine to remain in-memory until we see a need for it to be persisted in the future. For the actual caching strategy, Vault already uses go-cache which provides an expiration mechanism built in.

We shouldn't have any need to expire entries in the cache (or at least it can be very long-lived) as the key purpose doesn't change while active, however we'd need to remove the entry when a key gets deleted. There could potentially be a need for a capacity limit in the case of having many keys but that isn't likely.

Question, can the key purpose change with a new key version? If so, that would also require a remove-old/add-new to the cache. I can't think of anything else to consider at the moment.

[yesid-bocanegra]

@robmonte I have created a PR with the proposed change, let me know if something should be changed. #33

[maxcoulombe]

Closing this issue as the linked PR added the proposed in-memory cache to avoid quota exceeded errors.

Thanks again @yesid-bocanegra for reporting the issue and contributing the solution!