Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to configure the refresh hint of the local bundle #4297

Closed
sorindumitru opened this issue Jun 28, 2023 · 3 comments · Fixed by #4400 or #4847
Closed

Add the ability to configure the refresh hint of the local bundle #4297

sorindumitru opened this issue Jun 28, 2023 · 3 comments · Fixed by #4400 or #4847
Assignees
Labels
priority/backlog Issue is approved and in the backlog
Milestone

Comments

@sorindumitru
Copy link
Collaborator

sorindumitru commented Jun 28, 2023

spire-server refreshes the trust bundle for domains it federates with based on the spiffe_refresh_hint in the trust bundle, or in the absence of it based on the lifetime of the X509 CAs from it.

This is what the specifications says that the refresh hint should be used for, but it
can miss updates if it follows that advice. For example:

  • You increase the number of replicas for spire-servers in your trust domain. Those new instances have no key to use so they create one, add it to the bundle and start using it for signing SVIDs. Trust domains federating with us won't learn about these new keys until the next refresh interval happens which can be hours or even days away.
  • A key gets revoked and removed from the bundle. Trust domains federating with us will still accept SVIDs signed by this key until the next refresh interval.

spire-server should refersh as often as possible to learn about these unexpected changes to the trust bundle of a trust domain. Question is how often. Having this be once every couple of minutes is usually enough to minimise issues, but it also depends a bit on what the endpoint hosting the bundle can handle so maybe it makes sense to have this configurable.

Maybe this should go even further to deprecating the refresh hint in the specification since I think most implementation run into these kinds of issues where unscheduled updates of the trust bundle happen.

  • Version: any
  • Platform: any
  • Subsystem: server, federation
@amartinezfayo amartinezfayo added the triage/in-progress Issue triage is in progress label Jul 3, 2023
@rturner3 rturner3 assigned rturner3 and amartinezfayo and unassigned rturner3 Jul 6, 2023
@rturner3 rturner3 added priority/backlog Issue is approved and in the backlog and removed triage/in-progress Issue triage is in progress labels Jul 6, 2023
@amartinezfayo
Copy link
Member

Thank you @sorindumitru for opening this issue.
This is something that we want to address. Thank you also for contributing with a possible solution (#4296). We will be reviewing it and commenting on the PR.

@amartinezfayo amartinezfayo changed the title spire-server fails to keep federated bundle up to date due to infrequent refresh Add the ability to configure the refresh hint of the local bundle Jul 18, 2023
@amartinezfayo
Copy link
Member

Possible implementation:

  1. Introduce a new setting under the bundle_endpoint setting in the federation section of the SPIRE Server configuration to be able to configure the refresh hint of the local bundle. Probably log a warning if this is set to a high value.
  2. When SPIRE updates foreign bundles, look at the refresh hint of the foreign bundle. If it's set, refresh the bundle at the specified interval (keeping the current logic of making sure that we refresh at a safe interval not shorter than 1 minute). If it's not set, default to 5 minutes.
  3. The ultimate goal is that SPIRE migrates to a 5 minutes default interval for foreign bundle refreshing across the board. To that end, we could start logging a warning when the new configurable is absent in the bundle_endpoint configuration, saying that it will take that default value in a future version of SPIRE. I think that we will need to wait until 1.9 to make the switch to that default.

@amartinezfayo
Copy link
Member

@sorindumitru I'm assigning this to you, since you offered your help to implement this. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment