Skip to content

Commit 08e5ed2

Browse files
committed
fix: address root cause of empty reference IDs instead of suppressing them
This commit addresses the feedback from @setrofim in PR #338 by fixing the root cause where empty reference value IDs were being generated rather than suppressing them in the consuming code. Changes made: - Fixed multiple scheme handlers (psa-iot, tpm-enacttrust, parsec-tpm, arm-cca, parsec-cca) to return nil instead of []string{""} when encountering errors - Fixed handler/store_rpc.go to return nil instead of []string{""} on RPC call failures - Fixed vts/trustedservices/trustedservices_grpc.go to return nil instead of []string{""} on trust anchor retrieval errors - Removed the workaround that was skipping empty reference IDs in GetAttestation method since the root cause is now fixed The original URL-safe base64 nonce functionality from the PR is preserved and all tests continue to pass. Fixes: Issue identified by @setrofim - empty refvalID should never occur and indicates a bug that should be fixed at the source. Signed-off-by: Kallal Mukherjee <[email protected]>
1 parent 876d5e7 commit 08e5ed2

File tree

7 files changed

+14
-20
lines changed

7 files changed

+14
-20
lines changed

handler/store_rpc.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,13 +241,13 @@ func (s *StoreRPCClient) GetTrustAnchorIDs(token *proto.AttestationToken) ([]str
241241

242242
data, err = json.Marshal(token)
243243
if err != nil {
244-
return []string{""}, fmt.Errorf("marshaling token: %w", err)
244+
return nil, fmt.Errorf("marshaling token: %w", err)
245245
}
246246

247247
err = s.client.Call("Plugin.GetTrustAnchorIDs", data, &resp)
248248
if err != nil {
249249
err = ParseError(err)
250-
return []string{""}, fmt.Errorf("Plugin.GetTrustAnchorIDs RPC call failed: %w", err) // nolint
250+
return nil, fmt.Errorf("Plugin.GetTrustAnchorIDs RPC call failed: %w", err) // nolint
251251
}
252252

253253
return resp, nil

scheme/arm-cca/store_handler.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,16 @@ func (s StoreHandler) SynthKeysFromTrustAnchor(tenantID string, ta *handler.Endo
4848
func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string, error) {
4949
evidence, err := ccatoken.DecodeAndValidateEvidenceFromCBOR(token.Data)
5050
if err != nil {
51-
return []string{""}, handler.BadEvidence(err)
51+
return nil, handler.BadEvidence(err)
5252
}
5353

5454
claims := evidence.PlatformClaims
5555
if err != nil {
56-
return []string{""}, err
56+
return nil, err
5757
}
5858
taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims)
5959
if err != nil {
60-
return []string{""}, err
60+
return nil, err
6161
}
6262

6363
return []string{taID}, nil

scheme/parsec-cca/store_handler.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string
4343

4444
err := evidence.FromCBOR(token.Data)
4545
if err != nil {
46-
return []string{""}, handler.BadEvidence(err)
46+
return nil, handler.BadEvidence(err)
4747
}
4848
claims := evidence.Pat.PlatformClaims
4949

5050
taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims)
5151
if err != nil {
52-
return []string{""}, err
52+
return nil, err
5353
}
5454

5555
return []string{taID}, nil

scheme/parsec-tpm/store_handler.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string
4242
var ev tpm.Evidence
4343
err := ev.FromCBOR(token.Data)
4444
if err != nil {
45-
return []string{""}, handler.BadEvidence(err)
45+
return nil, handler.BadEvidence(err)
4646
}
4747

4848
kat := ev.Kat
4949
if kat == nil {
50-
return []string{""}, errors.New("no key attestation token to fetch Key ID")
50+
return nil, errors.New("no key attestation token to fetch Key ID")
5151
}
5252
kid := *kat.KID
5353
instance_id := base64.StdEncoding.EncodeToString(kid)

scheme/psa-iot/store_handler.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ func (s StoreHandler) SynthKeysFromTrustAnchor(tenantID string, ta *handler.Endo
3838
func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string, error) {
3939
psaToken, err := psatoken.DecodeAndValidateEvidenceFromCOSE(token.Data)
4040
if err != nil {
41-
return []string{""}, handler.BadEvidence(err)
41+
return nil, handler.BadEvidence(err)
4242
}
4343

4444
claims := psaToken.Claims
4545

4646
taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims)
4747
if err != nil {
48-
return []string{""}, err
48+
return nil, err
4949
}
5050

5151
return []string{taID}, nil

scheme/tpm-enacttrust/store_handler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string
4343
strings.Join(EvidenceMediaTypes, ", "),
4444
token.MediaType,
4545
)
46-
return []string{""}, err
46+
return nil, err
4747
}
4848

4949
var decoded Token

vts/trustedservices/trustedservices_grpc.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -442,12 +442,6 @@ func (o *GRPC) GetAttestation(
442442

443443
var multEndorsements []string
444444
for _, refvalID := range appraisal.EvidenceContext.ReferenceIds {
445-
// Skip empty reference IDs (can occur when no software components are provisioned)
446-
if refvalID == "" {
447-
o.logger.Debugw("skipping empty reference ID", "refvalID", refvalID)
448-
continue
449-
}
450-
451445
endorsements, err := o.EnStore.Get(refvalID)
452446
if err != nil && !errors.Is(err, kvstore.ErrKeyNotFound) {
453447
return o.finalize(appraisal, err)
@@ -512,12 +506,12 @@ func (c *GRPC) getTrustAnchors(id []string) ([]string, error) {
512506
for _, taID := range id {
513507
values, err := c.TaStore.Get(taID)
514508
if err != nil {
515-
return []string{""}, err
509+
return nil, err
516510
}
517511

518512
// For now, Veraison schemes only support one trust anchor per trustAnchorID
519513
if len(values) != 1 {
520-
return []string{""}, fmt.Errorf("found %d trust anchors, want 1", len(values))
514+
return nil, fmt.Errorf("found %d trust anchors, want 1", len(values))
521515
}
522516
taValues = append(taValues, values[0])
523517
}

0 commit comments

Comments
 (0)