search

scitokens / scitokens-cpp

A C++ implementation of the SciTokens library with a C library interface
Apache License 2.0
5 stars 22 forks source link

Make issuer key cache length configurable #86

Open brianhlin opened 2 years ago

brianhlin commented 2 years ago

We'd like to be able to specify the length of time that issuer keys are cached (currently 4 days)

djw8605 commented 2 years ago

We are thinking of a configuration file, does that work for you?

brianhlin commented 2 years ago

Hrm, I was thinking that this would be something that we could specify in the HTCondor configuration and then HTCondor would pass along its cache lifetime preference when making calls to the SciTokens library. @jaimefrey and @timtheisen may have some opinions on whether or not a SciTokens library config file would be workable for us.

JaimeFrey commented 2 years ago

I don't foresee any issues on the HTCondor side for a SciTokens configuration file. I would presume there'd be a way to specify what filename to use via the API. It'd be easy for us to have that be configured from the HTCondor config file.

bbockelm commented 1 year ago

@djw8605 - should we really have a configuration file here? Or should we expose a configuration API that the application above (for @JaimeFrey, this would be HTCondor) can invoke and populate with their config file?

What's the format and location for the Python library configuration file?

jthiltges commented 2 months ago

Having recently been thinking about cache lifetimes, could this be turned around, and have key cache lifetime driven by the issuer? Use the Cache-Control max-age on the jwks_uri reply (with upper/lower guardrails).

djw8605 commented 2 months ago

At one time, we discussed the minimum of either the cache-control or a configured min lifetime. I wonder if that still makes sense.

jbasney commented 2 months ago

In https://github.com/WLCG-AuthZ-WG/common-jwt-profile/pull/17 we discussed updating the WLCG profile to respect the Cache-Control header, but in the end we just updated the maximum from 1 day to 4 days.

Repeating a few observations I made in that WLCG pull request:

  1. The HTTP caching headers should take precedence, so long as the lifetime is within the minimum and maximum values. Respecting the HTTP caching headers allows providers to adjust caching as needed (e.g., during key rollover periods).
  2. We should clarify cache update times (polling frequency) versus cache eviction times. I see that implementations are polling the JWKS endpoints every 10 minutes (which is too frequent according to the 1 hour minimum in the WLCG profile) but caching keys for longer periods (hours/days) in case the JWKS endpoints are offline. This distinction between the two time periods (polling for updates versus evicting stale items from the cache) needs to be clearly specified in the configuration.
  3. We should cache key sets, not individual keys, so if the issuer removes a key from its key set (e.g., in case that individual key is compromised), that individual key is removed from the cache at the next update interval and does not stay in the cache waiting for the longer eviction period.
jbasney commented 2 months ago

There's a TODO comment for looking at the cache-control header: https://github.com/scitokens/scitokens-cpp/blob/4f821636b81a11707026f53b9384acecfc541440/src/scitokens_internal.cpp#L731

For comparison, the python library already looks at the cache-control header: https://github.com/scitokens/scitokens/blob/8d60a0759620050e12ebbda8eee206de0bf93479/src/scitokens/utils/keycache.py#L303