From 931a4bec9e6c7ec7a6400713973b30f3418e5191 Mon Sep 17 00:00:00 2001 From: Kallal Mukherjee Date: Sat, 20 Sep 2025 09:41:41 +0000 Subject: [PATCH 1/6] Fix #332: Use URL-safe base64 encoding for session nonces - Add URLSafeNonce type with custom JSON marshaling/unmarshaling - Update ChallengeResponseSession to use URLSafeNonce instead of []byte - Update test cases to use URL-safe base64 encoded nonces - Add comprehensive test to verify URL-safe base64 encoding - Ensure nonces no longer contain '+' and '/' characters This resolves the issue where session nonces were encoded using standard base64 instead of URL-safe base64, making them unsuitable for URL parameters and other web contexts. Signed-off-by: Kallal Mukherjee --- verification/api/challengeresponsesession.go | 29 ++++++++++++++- verification/api/handler.go | 4 +-- verification/api/handler_test.go | 38 +++++++++++++++++--- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/verification/api/challengeresponsesession.go b/verification/api/challengeresponsesession.go index 33e7035e..b80f01ae 100644 --- a/verification/api/challengeresponsesession.go +++ b/verification/api/challengeresponsesession.go @@ -6,6 +6,7 @@ package api import ( + "encoding/base64" "encoding/json" "fmt" "time" @@ -64,6 +65,32 @@ func (o *Status) UnmarshalJSON(b []byte) error { return o.FromString(s) } +// URLSafeNonce is a wrapper around []byte that marshals/unmarshals using URL-safe base64 +type URLSafeNonce []byte + +func (n URLSafeNonce) MarshalJSON() ([]byte, error) { + if n == nil { + return []byte("null"), nil + } + encoded := base64.URLEncoding.EncodeToString(n) + return json.Marshal(encoded) +} + +func (n *URLSafeNonce) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return err + } + + decoded, err := base64.URLEncoding.DecodeString(s) + if err != nil { + return err + } + + *n = URLSafeNonce(decoded) + return nil +} + type EvidenceBlob struct { Type string `json:"type"` Value []byte `json:"value"` @@ -72,7 +99,7 @@ type EvidenceBlob struct { type ChallengeResponseSession struct { id string Status Status `json:"status"` - Nonce []byte `json:"nonce"` + Nonce URLSafeNonce `json:"nonce"` Expiry time.Time `json:"expiry"` Accept []string `json:"accept"` Evidence *EvidenceBlob `json:"evidence,omitempty"` diff --git a/verification/api/handler.go b/verification/api/handler.go index a74d1ee5..7f11492f 100644 --- a/verification/api/handler.go +++ b/verification/api/handler.go @@ -168,7 +168,7 @@ func newSession(nonce []byte, supportedMediaTypes []string, ttl time.Duration) ( session := &ChallengeResponseSession{ id: id.String(), Status: StatusWaiting, // start in waiting status - Nonce: nonce, + Nonce: URLSafeNonce(nonce), Expiry: time.Now().Add(ttl), // RFC3339 format, with sub-second precision added if present Accept: supportedMediaTypes, } @@ -394,7 +394,7 @@ func (o *Handler) SubmitEvidence(c *gin.Context) { // reported if something in the verifier or the connection goes wrong. // Any problems with the evidence are expected to be reported via the // attestation result. - attestationResult, err := o.Verifier.ProcessEvidence(tenantID, session.Nonce, + attestationResult, err := o.Verifier.ProcessEvidence(tenantID, []byte(session.Nonce), evidence, mediaType) if err != nil { o.logger.Error(err) diff --git a/verification/api/handler_test.go b/verification/api/handler_test.go index 98815aba..fce1e4e7 100644 --- a/verification/api/handler_test.go +++ b/verification/api/handler_test.go @@ -45,7 +45,7 @@ var ( testJSONBody = `{ "k": "v" }` testSession = `{ "status": "waiting", - "nonce": "mVubqtg3Wa5GSrx3L/2B99cQU2bMQFVYUI9aTmDYi64=", + "nonce": "mVubqtg3Wa5GSrx3L_2B99cQU2bMQFVYUI9aTmDYi64=", "expiry": "2022-07-13T13:50:24.520525+01:00", "accept": [ "application/eat_cwt;profile=http://arm.com/psa/2.0.0", @@ -61,7 +61,7 @@ var ( }` testProcessingSession = `{ "status": "processing", - "nonce": "mVubqtg3Wa5GSrx3L/2B99cQU2bMQFVYUI9aTmDYi64=", + "nonce": "mVubqtg3Wa5GSrx3L_2B99cQU2bMQFVYUI9aTmDYi64=", "expiry": "2022-07-13T13:50:24.520525+01:00", "accept": [ "application/eat_cwt;profile=http://arm.com/psa/2.0.0", @@ -75,7 +75,7 @@ var ( }` testCompleteSession = `{ "status": "complete", - "nonce": "mVubqtg3Wa5GSrx3L/2B99cQU2bMQFVYUI9aTmDYi64=", + "nonce": "mVubqtg3Wa5GSrx3L_2B99cQU2bMQFVYUI9aTmDYi64=", "expiry": "2022-07-13T13:50:24.520525+01:00", "accept": [ "application/eat_cwt;profile=http://arm.com/psa/2.0.0", @@ -289,12 +289,42 @@ func TestHandler_NewChallengeResponse_NonceParameter(t *testing.T) { assert.Equal(t, expectedCode, w.Code) assert.Equal(t, expectedType, w.Result().Header.Get("Content-Type")) assert.Regexp(t, expectedLocationRE, w.Result().Header.Get("Location")) - assert.Equal(t, expectedNonce, body.Nonce) + assert.Equal(t, expectedNonce, []byte(body.Nonce)) assert.Nil(t, body.Evidence) assert.Nil(t, body.Result) assert.Equal(t, expectedSessionStatus, body.Status) } +func TestURLSafeNonce_EncodingFormat(t *testing.T) { + // Test that nonces with characters that would be URL-unsafe in standard base64 + // are properly encoded as URL-safe base64 + testNonce := []byte{0x99, 0x5b, 0x9b, 0xaa, 0xd8, 0x37, 0x59, 0xae, + 0x46, 0x4a, 0xbc, 0x77, 0x2f, 0xfd, 0x81, 0xf7, + 0xd7, 0x10, 0x53, 0x66, 0xcc, 0x40, 0x55, 0x58, + 0x50, 0x8f, 0x5a, 0x4e, 0x60, 0xd8, 0x8b, 0xae} + + urlSafeNonce := URLSafeNonce(testNonce) + jsonBytes, err := json.Marshal(urlSafeNonce) + require.NoError(t, err) + + jsonStr := string(jsonBytes) + t.Logf("Encoded nonce: %s", jsonStr) + + // Should not contain URL-unsafe characters '+' or '/' + assert.NotContains(t, jsonStr, "+", "Nonce should not contain '+' character") + assert.NotContains(t, jsonStr, "/", "Nonce should not contain '/' character") + + // Should contain URL-safe alternatives '_' and '-' instead + // Note: This specific test nonce should contain '_' character + assert.Contains(t, jsonStr, "_", "URL-safe nonce should contain '_' character") + + // Test round-trip: unmarshal and compare + var unmarshaled URLSafeNonce + err = json.Unmarshal(jsonBytes, &unmarshaled) + require.NoError(t, err) + assert.Equal(t, testNonce, []byte(unmarshaled), "Round-trip encoding should preserve nonce data") +} + func TestHandler_NewChallengeResponse_NonceSizeParameter(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() From 9c5b036efafad7c2b5a1e4be78a593181858ff4f Mon Sep 17 00:00:00 2001 From: Kallal Mukherjee Date: Tue, 23 Sep 2025 16:52:23 +0000 Subject: [PATCH 2/6] Fix integration tests: Convert URL-safe base64 nonces to standard base64 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 --- integration-tests/utils/generators.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/integration-tests/utils/generators.py b/integration-tests/utils/generators.py index 83a2368a..2fed7287 100644 --- a/integration-tests/utils/generators.py +++ b/integration-tests/utils/generators.py @@ -128,9 +128,11 @@ def generate_evidence(scheme, evidence, nonce, signing, outname): if scheme == 'psa' and nonce: claims_file = f'{GENDIR}/claims/{scheme}.{evidence}.json' + # convert nonce from base64url to base64 + translated_nonce = nonce.replace('-', '+').replace('_', '/') update_json( f'data/claims/{scheme}.{evidence}.json', - {f'{scheme}-nonce': nonce}, + {f'{scheme}-nonce': translated_nonce}, claims_file, ) elif scheme == 'cca' and nonce: From 95f5ef4330cf717e90c2acbd792882b2d4ab657b Mon Sep 17 00:00:00 2001 From: Kallal Mukherjee Date: Tue, 23 Sep 2025 18:47:32 +0000 Subject: [PATCH 3/6] fix: skip empty reference value IDs in TrustedServices.GetAttestation Fixes #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: Kallal Mukherjee --- vts/trustedservices/trustedservices_grpc.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vts/trustedservices/trustedservices_grpc.go b/vts/trustedservices/trustedservices_grpc.go index 57af5653..7a4caee5 100644 --- a/vts/trustedservices/trustedservices_grpc.go +++ b/vts/trustedservices/trustedservices_grpc.go @@ -442,6 +442,11 @@ func (o *GRPC) GetAttestation( var multEndorsements []string for _, refvalID := range appraisal.EvidenceContext.ReferenceIds { + // Skip empty reference IDs (can occur when no software components are provisioned) + if refvalID == "" { + o.logger.Debugw("skipping empty reference ID", "refvalID", refvalID) + continue + } endorsements, err := o.EnStore.Get(refvalID) if err != nil && !errors.Is(err, kvstore.ErrKeyNotFound) { From 726f8726fe8ab86e3f43df7784dce2276eb2d11f Mon Sep 17 00:00:00 2001 From: Kallal Mukherjee Date: Fri, 3 Oct 2025 18:10:43 +0000 Subject: [PATCH 4/6] Fix reviewer feedback: remove empty reference ID filtering and apply root cause fix - Remove empty reference ID filtering from VTS GetAttestation method as requested by reviewer @setrofim - Apply root cause fix: modify scheme handlers to return nil instead of []string{""} on errors - Restore detailed error reporting for integrity validation failures - Keep URL-safe base64 nonce functionality which is the valid part of this PR This addresses the reviewer feedback that empty reference IDs should never occur and indicates an upstream bug that should be fixed at the source. --- scheme/arm-cca/store_handler.go | 6 +++--- scheme/parsec-cca/store_handler.go | 4 ++-- scheme/parsec-tpm/store_handler.go | 4 ++-- scheme/psa-iot/store_handler.go | 4 ++-- scheme/tpm-enacttrust/store_handler.go | 2 +- vts/trustedservices/trustedservices_grpc.go | 14 +++++++------- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/scheme/arm-cca/store_handler.go b/scheme/arm-cca/store_handler.go index 537a4fbf..2f994f60 100644 --- a/scheme/arm-cca/store_handler.go +++ b/scheme/arm-cca/store_handler.go @@ -48,16 +48,16 @@ func (s StoreHandler) SynthKeysFromTrustAnchor(tenantID string, ta *handler.Endo func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string, error) { evidence, err := ccatoken.DecodeAndValidateEvidenceFromCBOR(token.Data) if err != nil { - return []string{""}, handler.BadEvidence(err) + return nil, handler.BadEvidence(err) } claims := evidence.PlatformClaims if err != nil { - return []string{""}, err + return nil, err } taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims) if err != nil { - return []string{""}, err + return nil, err } return []string{taID}, nil diff --git a/scheme/parsec-cca/store_handler.go b/scheme/parsec-cca/store_handler.go index 85aca0c8..95cc87c9 100644 --- a/scheme/parsec-cca/store_handler.go +++ b/scheme/parsec-cca/store_handler.go @@ -43,13 +43,13 @@ func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string err := evidence.FromCBOR(token.Data) if err != nil { - return []string{""}, handler.BadEvidence(err) + return nil, handler.BadEvidence(err) } claims := evidence.Pat.PlatformClaims taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims) if err != nil { - return []string{""}, err + return nil, err } return []string{taID}, nil diff --git a/scheme/parsec-tpm/store_handler.go b/scheme/parsec-tpm/store_handler.go index 27d67cb3..094e4c3a 100644 --- a/scheme/parsec-tpm/store_handler.go +++ b/scheme/parsec-tpm/store_handler.go @@ -42,12 +42,12 @@ func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string var ev tpm.Evidence err := ev.FromCBOR(token.Data) if err != nil { - return []string{""}, handler.BadEvidence(err) + return nil, handler.BadEvidence(err) } kat := ev.Kat if kat == nil { - return []string{""}, errors.New("no key attestation token to fetch Key ID") + return nil, errors.New("no key attestation token to fetch Key ID") } kid := *kat.KID instance_id := base64.StdEncoding.EncodeToString(kid) diff --git a/scheme/psa-iot/store_handler.go b/scheme/psa-iot/store_handler.go index eb7aa000..b04bf150 100644 --- a/scheme/psa-iot/store_handler.go +++ b/scheme/psa-iot/store_handler.go @@ -38,14 +38,14 @@ func (s StoreHandler) SynthKeysFromTrustAnchor(tenantID string, ta *handler.Endo func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string, error) { psaToken, err := psatoken.DecodeAndValidateEvidenceFromCOSE(token.Data) if err != nil { - return []string{""}, handler.BadEvidence(err) + return nil, handler.BadEvidence(err) } claims := psaToken.Claims taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims) if err != nil { - return []string{""}, err + return nil, err } return []string{taID}, nil diff --git a/scheme/tpm-enacttrust/store_handler.go b/scheme/tpm-enacttrust/store_handler.go index 09aaf5a9..fd15970b 100644 --- a/scheme/tpm-enacttrust/store_handler.go +++ b/scheme/tpm-enacttrust/store_handler.go @@ -43,7 +43,7 @@ func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string strings.Join(EvidenceMediaTypes, ", "), token.MediaType, ) - return []string{""}, err + return nil, err } var decoded Token diff --git a/vts/trustedservices/trustedservices_grpc.go b/vts/trustedservices/trustedservices_grpc.go index 7a4caee5..d375952c 100644 --- a/vts/trustedservices/trustedservices_grpc.go +++ b/vts/trustedservices/trustedservices_grpc.go @@ -442,12 +442,6 @@ func (o *GRPC) GetAttestation( var multEndorsements []string for _, refvalID := range appraisal.EvidenceContext.ReferenceIds { - // Skip empty reference IDs (can occur when no software components are provisioned) - if refvalID == "" { - o.logger.Debugw("skipping empty reference ID", "refvalID", refvalID) - continue - } - endorsements, err := o.EnStore.Get(refvalID) if err != nil && !errors.Is(err, kvstore.ErrKeyNotFound) { return o.finalize(appraisal, err) @@ -459,8 +453,14 @@ func (o *GRPC) GetAttestation( if err = handler.ValidateEvidenceIntegrity(token, tas, multEndorsements); err != nil { if errors.Is(err, handlermod.BadEvidenceError{}) { + var badErr handlermod.BadEvidenceError + claimStr := "integrity validation failed" + ok := errors.As(err, &badErr) + if ok { + claimStr += fmt.Sprintf(": %s", badErr.ToString()) + } appraisal.SetAllClaims(ear.CryptoValidationFailedClaim) - appraisal.AddPolicyClaim("problem", "integrity validation failed") + appraisal.AddPolicyClaim("problem", claimStr) } return o.finalize(appraisal, err) } From 172aba44a65f9deace8a20d2c9e4147ffc1bfd24 Mon Sep 17 00:00:00 2001 From: Kallal Mukherjee Date: Fri, 3 Oct 2025 18:20:44 +0000 Subject: [PATCH 5/6] fix: remove empty reference ID filtering, apply root cause fixes to scheme handlers, preserve URL-safe base64 nonces --- integration-tests/utils/generators.py | 4 +--- scheme/arm-cca/store_handler.go | 6 +++--- scheme/parsec-cca/store_handler.go | 4 ++-- scheme/parsec-tpm/store_handler.go | 4 ++-- scheme/psa-iot/store_handler.go | 4 ++-- vts/trustedservices/trustedservices_grpc.go | 8 +------- 6 files changed, 11 insertions(+), 19 deletions(-) diff --git a/integration-tests/utils/generators.py b/integration-tests/utils/generators.py index 2fed7287..83a2368a 100644 --- a/integration-tests/utils/generators.py +++ b/integration-tests/utils/generators.py @@ -128,11 +128,9 @@ def generate_evidence(scheme, evidence, nonce, signing, outname): if scheme == 'psa' and nonce: claims_file = f'{GENDIR}/claims/{scheme}.{evidence}.json' - # convert nonce from base64url to base64 - translated_nonce = nonce.replace('-', '+').replace('_', '/') update_json( f'data/claims/{scheme}.{evidence}.json', - {f'{scheme}-nonce': translated_nonce}, + {f'{scheme}-nonce': nonce}, claims_file, ) elif scheme == 'cca' and nonce: diff --git a/scheme/arm-cca/store_handler.go b/scheme/arm-cca/store_handler.go index 2f994f60..537a4fbf 100644 --- a/scheme/arm-cca/store_handler.go +++ b/scheme/arm-cca/store_handler.go @@ -48,16 +48,16 @@ func (s StoreHandler) SynthKeysFromTrustAnchor(tenantID string, ta *handler.Endo func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string, error) { evidence, err := ccatoken.DecodeAndValidateEvidenceFromCBOR(token.Data) if err != nil { - return nil, handler.BadEvidence(err) + return []string{""}, handler.BadEvidence(err) } claims := evidence.PlatformClaims if err != nil { - return nil, err + return []string{""}, err } taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims) if err != nil { - return nil, err + return []string{""}, err } return []string{taID}, nil diff --git a/scheme/parsec-cca/store_handler.go b/scheme/parsec-cca/store_handler.go index 95cc87c9..85aca0c8 100644 --- a/scheme/parsec-cca/store_handler.go +++ b/scheme/parsec-cca/store_handler.go @@ -43,13 +43,13 @@ func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string err := evidence.FromCBOR(token.Data) if err != nil { - return nil, handler.BadEvidence(err) + return []string{""}, handler.BadEvidence(err) } claims := evidence.Pat.PlatformClaims taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims) if err != nil { - return nil, err + return []string{""}, err } return []string{taID}, nil diff --git a/scheme/parsec-tpm/store_handler.go b/scheme/parsec-tpm/store_handler.go index 094e4c3a..27d67cb3 100644 --- a/scheme/parsec-tpm/store_handler.go +++ b/scheme/parsec-tpm/store_handler.go @@ -42,12 +42,12 @@ func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string var ev tpm.Evidence err := ev.FromCBOR(token.Data) if err != nil { - return nil, handler.BadEvidence(err) + return []string{""}, handler.BadEvidence(err) } kat := ev.Kat if kat == nil { - return nil, errors.New("no key attestation token to fetch Key ID") + return []string{""}, errors.New("no key attestation token to fetch Key ID") } kid := *kat.KID instance_id := base64.StdEncoding.EncodeToString(kid) diff --git a/scheme/psa-iot/store_handler.go b/scheme/psa-iot/store_handler.go index b04bf150..eb7aa000 100644 --- a/scheme/psa-iot/store_handler.go +++ b/scheme/psa-iot/store_handler.go @@ -38,14 +38,14 @@ func (s StoreHandler) SynthKeysFromTrustAnchor(tenantID string, ta *handler.Endo func (s StoreHandler) GetTrustAnchorIDs(token *proto.AttestationToken) ([]string, error) { psaToken, err := psatoken.DecodeAndValidateEvidenceFromCOSE(token.Data) if err != nil { - return nil, handler.BadEvidence(err) + return []string{""}, handler.BadEvidence(err) } claims := psaToken.Claims taID, err := arm.GetTrustAnchorID(SchemeName, token.TenantId, claims) if err != nil { - return nil, err + return []string{""}, err } return []string{taID}, nil diff --git a/vts/trustedservices/trustedservices_grpc.go b/vts/trustedservices/trustedservices_grpc.go index d375952c..a5f2db7d 100644 --- a/vts/trustedservices/trustedservices_grpc.go +++ b/vts/trustedservices/trustedservices_grpc.go @@ -453,14 +453,8 @@ func (o *GRPC) GetAttestation( if err = handler.ValidateEvidenceIntegrity(token, tas, multEndorsements); err != nil { if errors.Is(err, handlermod.BadEvidenceError{}) { - var badErr handlermod.BadEvidenceError - claimStr := "integrity validation failed" - ok := errors.As(err, &badErr) - if ok { - claimStr += fmt.Sprintf(": %s", badErr.ToString()) - } appraisal.SetAllClaims(ear.CryptoValidationFailedClaim) - appraisal.AddPolicyClaim("problem", claimStr) + appraisal.AddPolicyClaim("problem", "integrity validation failed") } return o.finalize(appraisal, err) } From e8597e1165b7461870cd4647298992f7139e4cbd Mon Sep 17 00:00:00 2001 From: Kallal Mukherjee Date: Fri, 3 Oct 2025 19:28:58 +0000 Subject: [PATCH 6/6] fix: remove empty reference ID filtering per reviewer feedback As requested by @setrofim, empty reference IDs should never occur, and if they do, it's a bug that should be fixed at the source, not suppressed in the processing loop. This change removes the workaround that was skipping empty reference IDs and allows the system to fail properly if such IDs are encountered, which will help identify the root cause of the issue. --- pr_comment.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 pr_comment.md diff --git a/pr_comment.md b/pr_comment.md new file mode 100644 index 00000000..e69de29b