Fix TrustedServices empty reference ID handling and integration test base64 encoding #343
Conversation
kallal79
force-pushed
the
fix/trusted-services-empty-ref-ids
branch
from
0787c2e to
25cc480
Compare
|
|
||
| var multEndorsements []string | ||
| for _, refvalID := range appraisal.EvidenceContext.ReferenceIds { | ||
| // Skip empty reference IDs (can occur when no software components are provisioned) |
There was a problem hiding this comment.
This does not seem right. If there are no software compoments, then surely, appraisal.EvidenceContext.ReferenceIds itself should be empty, rather than contain an empty string?
There was a problem hiding this comment.
Thanks sir @setrofim — you're absolutely right, and the edge case is now handled.
There was a problem hiding this comment.
Updated code still filters out empty string "IDs". My point was that I'm unclear on how they can occur -- when there no s/w components, the ReferenceIds list should just be empty. The fact that empty strings can occur looks like a but, and the correct solution would be to senure they're not inserted in the first place, rather than filtered out here.
There was a problem hiding this comment.
???
Those are the commits I was refering to in my previsou message. You should NOT be checking for empty refID's when accessing appraisal.EvidenceContext.ReferenceIds. Those should not be in the list to begin with. The correct fix is not to skip/filter them out, but to prevent them from being added in the first place.
kallal79
force-pushed
the
fix/trusted-services-empty-ref-ids
branch
from
1dd4529 to
b366f3e
Compare
|
Kindly requesting re-review from @setrofim @yogeshbdeshpande @thomas-fossati for PR #343 |
…e64 for PSA tokens - PSA evidence token generation (evcli psa create) expects standard base64 nonces - Server now returns URL-safe base64 nonces in challenge-response sessions - Added conversion from URL-safe to standard base64 for PSA claims generation - Matches existing conversion logic already used for CCA tokens - Resolves 'illegal base64 data' errors in integration tests Signed-off-by: Kallal Mukherjee <[email protected]>
Fixes veraison#42. When attestation schemes return empty reference value IDs, the GetAttestation method now skips them before calling kvstore.Get() to avoid 'the supplied key is empty' errors. This commonly occurs when no software components are provisioned in trust anchors, causing handlers to return []string{""} for missing software reference IDs. Signed-off-by: GitHub Copilot <[email protected]> Signed-off-by: Kallal Mukherjee <[email protected]>
This addresses the reviewer feedback from @setrofim about treating the root cause rather than symptoms. Empty reference IDs are now filtered immediately after GetRefValueIDs() to ensure EvidenceContext.ReferenceIds never contains empty strings, rather than skipping them later in the loop. This is a cleaner approach that prevents the issue from propagating throughout the system and maintains data integrity at the source. Signed-off-by: Kallal Mukherjee <[email protected]>
kallal79
force-pushed
the
fix/trusted-services-empty-ref-ids
branch
from
b366f3e to
ac343f6
Compare
Previously, various attestation scheme handlers were incorrectly returning
[]string{""} (slice containing an empty string) when encountering errors
in GetTrustAnchorIDs, instead of returning nil or an empty slice. This
caused downstream code in TrustedServices.GetAttestation to receive and
process empty reference IDs.
This fix addresses the root cause by modifying all scheme handlers to:
- Return nil slices instead of slices containing empty strings on errors
- Properly propagate errors without malformed return values
Additionally, fixes base64 encoding in integration tests for PSA scheme
by converting URL-safe base64 nonces to standard base64.
This approach is cleaner than filtering empty strings downstream and
prevents the issue from propagating throughout the system, as suggested
by @setrofim in PR veraison#343.
Fixes veraison#42
|
Kindly requesting re-review from @setrofim @yogeshbdeshpande @thomas-fossati for PR #343 |
| } | ||
|
|
||
| // Filter out empty reference IDs (can occur when no software components are provisioned) | ||
| filteredReferenceIDs := make([]string, 0, len(referenceIDs)) |
There was a problem hiding this comment.
This filtering shoudn't be necessary.
…y strings
- Store handlers now return nil/proper errors instead of []string{""}
- Removed unnecessary filtering logic in trustedservices_grpc.go
- Fixes issue mentioned by setrofim in PR veraison#343 review
The filtering shouldn't be necessary as store handlers should not
return empty reference IDs in the first place.
|
Requesting re-review and approval for PR #343 from sir @setrofim , sir @yogeshbdeshpande , sir @thomas-fossati , sir @cowbon .. |
2 participants
Description:
This PR addresses two related issues:
Issue #42: TrustedServices errors when no softwareIds in evidence context
Problem: When attestation schemes return empty reference value IDs (common when no software components are provisioned),
GetAttestationfails with "the supplied key is empty" error from kvstore.Solution: Added validation in
GetAttestationto skip empty reference IDs before callingkvstore.Get().Impact: Allows attestation to succeed in environments without software components provisioned.
Integration Test Base64 Encoding Issue
Problem: PSA integration tests were failing with "illegal base64 data" when
evcli psa createprocessed URL-safe base64 nonces.Solution: Convert URL-safe base64 nonces to standard base64 before PSA token generation, matching existing CCA implementation.
Impact: Fixes integration test failures for PSA scheme.
Testing
Related Issues
Fixes #42