From 6f8a6d4de496e1243d623d879810da09d5d67371 Mon Sep 17 00:00:00 2001 From: vreff <104409744+vreff@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:55:56 -0700 Subject: [PATCH 1/3] Restrict tag for DomainHash to be alphanumeric --- pkg/teeattestation/hash.go | 18 ++++++++++++++-- pkg/teeattestation/hash_test.go | 26 +++++++++++++---------- pkg/teeattestation/nitro/validate_test.go | 17 +++++++++------ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/pkg/teeattestation/hash.go b/pkg/teeattestation/hash.go index fb38dce1e0..0673ea9284 100644 --- a/pkg/teeattestation/hash.go +++ b/pkg/teeattestation/hash.go @@ -6,6 +6,7 @@ package teeattestation import ( "crypto/sha256" "encoding/binary" + "fmt" "hash" ) @@ -14,12 +15,25 @@ const DomainSeparator = "CONFIDENTIAL_COMPUTE_PAYLOAD" // DomainHash computes SHA-256 over the DomainSeparator and the length-prefixed // tag and data, so distinct (tag, data) pairs can never share a pre-image. [CL112-14] -func DomainHash(tag string, data []byte) []byte { +func DomainHash(tag string, data []byte) ([]byte, error) { + if tag == "" || !isAlphanumeric(tag) { + return nil, fmt.Errorf("invalid tag: must be non-empty and contain only alphanumeric characters") + } h := sha256.New() h.Write([]byte(DomainSeparator)) writeWithLength(h, []byte(tag)) writeWithLength(h, data) - return h.Sum(nil) + return h.Sum(nil), nil +} + +// isAlphanumeric reports whether s consists only of ASCII letters and digits. +func isAlphanumeric(s string) bool { + for _, r := range s { + if !(r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9') { + return false + } + } + return true } // writeWithLength writes an 8-byte big-endian length prefix followed by data. diff --git a/pkg/teeattestation/hash_test.go b/pkg/teeattestation/hash_test.go index 43ffe8c76b..358a10651f 100644 --- a/pkg/teeattestation/hash_test.go +++ b/pkg/teeattestation/hash_test.go @@ -1,16 +1,18 @@ package teeattestation import ( - "bytes" "crypto/sha256" "testing" + + "github.com/stretchr/testify/require" ) func TestDomainHash(t *testing.T) { tag := "TestTag" data := []byte(`{"key":"value"}`) - got := DomainHash(tag, data) + got, err := DomainHash(tag, data) + require.NoError(t, err) h := sha256.New() h.Write([]byte(DomainSeparator)) @@ -30,8 +32,10 @@ func TestDomainHash(t *testing.T) { func TestDomainHash_DifferentTags(t *testing.T) { data := []byte("same-data") - h1 := DomainHash("Tag1", data) - h2 := DomainHash("Tag2", data) + h1, err := DomainHash("Tag1", data) + require.NoError(t, err) + h2, err := DomainHash("Tag2", data) + require.NoError(t, err) for i := range h1 { if h1[i] != h2[i] { @@ -43,8 +47,10 @@ func TestDomainHash_DifferentTags(t *testing.T) { func TestDomainHash_DifferentData(t *testing.T) { tag := "SameTag" - h1 := DomainHash(tag, []byte("data-a")) - h2 := DomainHash(tag, []byte("data-b")) + h1, err := DomainHash(tag, []byte("data-a")) + require.NoError(t, err) + h2, err := DomainHash(tag, []byte("data-b")) + require.NoError(t, err) for i := range h1 { if h1[i] != h2[i] { @@ -54,9 +60,7 @@ func TestDomainHash_DifferentData(t *testing.T) { t.Fatal("different data should produce different hashes") } -// CL112-14 regression: the old newline scheme let ("A\nB","C") and ("A","B\nC") collide. -func TestDomainHash_NoBoundaryCollision(t *testing.T) { - if bytes.Equal(DomainHash("A\nB", []byte("C")), DomainHash("A", []byte("B\nC"))) { - t.Fatal("tag/data boundary collision: distinct pairs must not share a hash") - } +func TestDomainHash_InvalidTag(t *testing.T) { + _, err := DomainHash("A\nB", []byte("C")) + require.Equal(t, err.Error(), "invalid tag: must be non-empty and contain only alphanumeric characters") } diff --git a/pkg/teeattestation/nitro/validate_test.go b/pkg/teeattestation/nitro/validate_test.go index a3b93caca6..0cbde2cc76 100644 --- a/pkg/teeattestation/nitro/validate_test.go +++ b/pkg/teeattestation/nitro/validate_test.go @@ -15,7 +15,8 @@ func TestValidateAttestation_Attestor(t *testing.T) { fa, err := nitrofake.NewAttestor() require.NoError(t, err) - userData := teeattestation.DomainHash("test-tag", []byte(`{"key":"value"}`)) + userData, err := teeattestation.DomainHash("testtag", []byte(`{"key":"value"}`)) + require.NoError(t, err) doc, err := fa.CreateAttestation(userData) require.NoError(t, err) @@ -50,11 +51,13 @@ func TestValidateAttestation_WrongUserData(t *testing.T) { fa, err := nitrofake.NewAttestor() require.NoError(t, err) - userData := teeattestation.DomainHash("test-tag", []byte(`{"key":"value"}`)) + userData, err := teeattestation.DomainHash("testtag", []byte(`{"key":"value"}`)) + require.NoError(t, err) doc, err := fa.CreateAttestation(userData) require.NoError(t, err) - wrongData := teeattestation.DomainHash("wrong-tag", []byte(`{"key":"value"}`)) + wrongData, err := teeattestation.DomainHash("wrongtag", []byte(`{"key":"value"}`)) + require.NoError(t, err) err = ValidateAttestationWithRoots(doc, wrongData, fa.TrustedPCRsJSON(), fa.CARootsPEM()) require.Error(t, err) require.Contains(t, err.Error(), "expected user data") @@ -64,7 +67,7 @@ func TestValidateAttestation_WrongPCRs(t *testing.T) { fa, err := nitrofake.NewAttestor() require.NoError(t, err) - userData := []byte("test-data") + userData := []byte("testdata") doc, err := fa.CreateAttestation(userData) require.NoError(t, err) @@ -78,7 +81,8 @@ func TestValidateAndParse_SurfacesIdentityFields(t *testing.T) { fa, err := nitrofake.NewAttestor() require.NoError(t, err) - userData := teeattestation.DomainHash("test-tag", []byte(`{"key":"value"}`)) + userData, err := teeattestation.DomainHash("testtag", []byte(`{"key":"value"}`)) + require.NoError(t, err) nonce := []byte("request-id-as-nonce") identityKey := []byte("enclave-long-lived-identity-pubkey") @@ -109,7 +113,8 @@ func TestValidateAndParse_FailsOnWrongUserData(t *testing.T) { fa, err := nitrofake.NewAttestor() require.NoError(t, err) - userData := teeattestation.DomainHash("test-tag", []byte(`{"key":"value"}`)) + userData, err := teeattestation.DomainHash("testtag", []byte(`{"key":"value"}`)) + require.NoError(t, err) doc, err := fa.CreateAttestation(userData, nitrofake.WithNonce([]byte("n"))) require.NoError(t, err) From 286f2cb63ba16c154ee6266a6f447c0291a1755c Mon Sep 17 00:00:00 2001 From: vreff <104409744+vreff@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:02:53 -0700 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- pkg/teeattestation/hash.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/teeattestation/hash.go b/pkg/teeattestation/hash.go index 0673ea9284..830139b952 100644 --- a/pkg/teeattestation/hash.go +++ b/pkg/teeattestation/hash.go @@ -15,6 +15,7 @@ const DomainSeparator = "CONFIDENTIAL_COMPUTE_PAYLOAD" // DomainHash computes SHA-256 over the DomainSeparator and the length-prefixed // tag and data, so distinct (tag, data) pairs can never share a pre-image. [CL112-14] +// It returns an error if tag is empty or contains non-ASCII alphanumeric characters. func DomainHash(tag string, data []byte) ([]byte, error) { if tag == "" || !isAlphanumeric(tag) { return nil, fmt.Errorf("invalid tag: must be non-empty and contain only alphanumeric characters") From 643fcd3b554e8ba26390118b2ee0193b22426f23 Mon Sep 17 00:00:00 2001 From: vreff <104409744+vreff@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:03:16 -0700 Subject: [PATCH 3/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- pkg/teeattestation/hash_test.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/teeattestation/hash_test.go b/pkg/teeattestation/hash_test.go index 358a10651f..d5065f364b 100644 --- a/pkg/teeattestation/hash_test.go +++ b/pkg/teeattestation/hash_test.go @@ -61,6 +61,17 @@ func TestDomainHash_DifferentData(t *testing.T) { } func TestDomainHash_InvalidTag(t *testing.T) { - _, err := DomainHash("A\nB", []byte("C")) - require.Equal(t, err.Error(), "invalid tag: must be non-empty and contain only alphanumeric characters") + for _, tag := range []string{"", "A\nB", "tag-1"} { + _, err := DomainHash(tag, []byte("C")) + require.EqualError(t, err, "invalid tag: must be non-empty and contain only alphanumeric characters") + } +} + +// CL112-14 regression: without length-prefixing, ("AB","C") and ("A","BC") could collide. +func TestDomainHash_NoBoundaryCollision(t *testing.T) { + h1, err := DomainHash("AB", []byte("C")) + require.NoError(t, err) + h2, err := DomainHash("A", []byte("BC")) + require.NoError(t, err) + require.NotEqual(t, h1, h2) }