diff --git a/pkg/teeattestation/hash.go b/pkg/teeattestation/hash.go index fb38dce1e0..830139b952 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,26 @@ 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 { +// 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") + } 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..d5065f364b 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,18 @@ 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) { + 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) +} 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)