fix: [Connectivity-ZTIS] Deterministic SVID selection - #1246
Conversation
rpanackal
changed the title
Jonas-Isr
left a comment
Jonas-Isr
left a comment
There was a problem hiding this comment.
Only a minor point and a comment. Looks good to me otherwise :)
CharlesDuboisSAP
left a comment
CharlesDuboisSAP
left a comment
There was a problem hiding this comment.
None of the test fail when you remove the svidPicker
| .filter(svid -> expectedSpiffeId.equals(svid.getSpiffeId())) | ||
| .findFirst() | ||
| .orElseThrow( | ||
| () -> new CloudPlatformException( |
There was a problem hiding this comment.
Why throw? If not found we could use the old behaviour of not setting a svidPicker?
There was a problem hiding this comment.
Falling back to the old behavior would reproduce the bug in a smaller scope — it would only behave correctly when there's exactly one SVID, or when the library happens to pick the right one from an unordered list. That's the non-determinism we're fixing.
There was a problem hiding this comment.
It only throws when it doesn’t find the svid from the list right?
There was a problem hiding this comment.
Also, just improved the tests to address the other concern regarding removing the svidPicker
…k/cloudplatform/connectivity/ZeroTrustIdentityService.java Co-authored-by: Jonas-Isr <jonas.israel@sap.com>
| .builder() | ||
| .spiffeSocketPath(socketPath) | ||
| .initTimeout(DEFAULT_SOCKET_TIMEOUT) | ||
| .svidPicker(svids -> pickSvid(svids, expectedSpiffeId)) |
There was a problem hiding this comment.
If I remove the svidPicker, the tests are still green apart from assertThat(picker).isNotNull();
I don't think any test checks the actual change of this PR, the svidPicker
There was a problem hiding this comment.
Then wdym it's a compromise, if you revert the production code the tests are still green
3 participants
Fixes
ZeroTrustIdentityServicedid not configure ansvidPickerwhen buildingX509SourceOptions, causingjava-spiffe-coreto fall back togetDefaultSvid()which simply returns the first SVID in an unordered list. When the SPIRE agent returns multiple SVIDs to a workload (e.g., after creating a second service key for the same ZTIS instance, or when a co-located workload has overlapping selectors), this resulted in non-deterministic SVID selection that could silently present the wrong identity during mTLS handshakes.This change configures an
svidPickerthat deterministically selects the SVID whose SPIFFE ID matches the expected ID from the ZTIS service binding (credentials.workload.spiffeID), as required by the SPIFFE specification.Feature scope:
svidPickerinZeroTrustIdentityService.initX509Source()to select SVID by SPIFFE IDpickSvidhelper method with proper error handling for no-match scenariosDefinition of Done
Documentation updated