Skip to content

fix ZTIS certificate caching#1135

Open
mpern wants to merge 1 commit intoSAP:mainfrom
mpern:bugfix/ztis-rotation
Open

fix ZTIS certificate caching#1135
mpern wants to merge 1 commit intoSAP:mainfrom
mpern:bugfix/ztis-rotation

Conversation

@mpern
Copy link
Copy Markdown
Contributor

@mpern mpern commented Apr 1, 2026

Context

Fixes the following issues:

Feature scope:

refresh the cached ZTIS keystore if certificate is close to expiration.
otherwise requests fail if the backend is up for more than 7 days.

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Documentation updated
  • Release notes updated

@mpern mpern force-pushed the bugfix/ztis-rotation branch 2 times, most recently from d8ca612 to 657a0fc Compare April 1, 2026 11:40
refresh the cache if certificate is close to expiration
@mpern mpern force-pushed the bugfix/ztis-rotation branch from 657a0fc to a0064f0 Compare April 1, 2026 12:53
@MatKuhr
Copy link
Copy Markdown
Member

MatKuhr commented Apr 1, 2026

Thanks for the suggestion, but I'm not yet convinced this PR changes much / fixes the issue. Currently, the code relies on the object reference comparison, and while that is certainly less robust than actually comparing certificates as we had intended, the logic should still work.

If the underlying SVID changes, the X509Svid POJO should also be re-instantiated, so the object reference should be different and the equals comparison will fail. I don't see how the POJO could remain the same object reference while the underlying SVID changes from looking at the code.

@mpern
Copy link
Copy Markdown
Contributor Author

mpern commented Apr 1, 2026

I removed the object reference comparison from ZeroTrustIdentityService already.
As mentioned in one of the linked issues, the spiffe library removed Lombok from their project which also removed the deep equals the current implementation relies in.

isKeyStoreCached now checks if

  • spiffe ID is the same (sanity check)
  • if the workload / leaf certificate is close to expiration

Since getX509Svid uses the spiffe library to get the actual client cert from the workload API, a cache miss ensures a new, valid and different cert than the one before.

That said, I need to double check the new tests for ZtisClientIdentity if they properly reflect a rotation scenario

@mpern
Copy link
Copy Markdown
Contributor Author

mpern commented Apr 1, 2026

Now that I typed it all out I get your point. How can the pojo be equal if the cert was rotated 🤔

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants