From 27e2120d989bae20786cd7c374628eae6ccb210a Mon Sep 17 00:00:00 2001 From: Tejaswi Nadahalli Date: Thu, 18 Jun 2026 14:07:12 +0200 Subject: [PATCH 1/2] fix(teeattestation/nitro): enforce max age on attestation timestamp [CL112-10] verifyAttestationDocument only checked the attestation timestamp was non-zero; currentTime was used solely for cert validity (a 1-2 year window). A valid attestation therefore stayed valid until the leaf cert expired. Reject attestations older than MaxAttestationAge (5m), since the protocol produces them immediately before consumption. Add a test that a fresh attestation verifies and a stale one is rejected. --- pkg/teeattestation/nitro/validate_test.go | 20 ++++++++++++++++++++ pkg/teeattestation/nitro/verify.go | 10 ++++++++++ 2 files changed, 30 insertions(+) diff --git a/pkg/teeattestation/nitro/validate_test.go b/pkg/teeattestation/nitro/validate_test.go index de38339402..9e325e4c2f 100644 --- a/pkg/teeattestation/nitro/validate_test.go +++ b/pkg/teeattestation/nitro/validate_test.go @@ -2,6 +2,7 @@ package nitro import ( "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -22,6 +23,25 @@ func TestValidateAttestation_Attestor(t *testing.T) { require.NoError(t, err) } +// TestVerifyAttestationDocument_MaxAge covers PRIV-438 / CL112-10: a fresh +// attestation verifies, but one older than MaxAttestationAge is rejected even +// though the leaf cert is still valid. +func TestVerifyAttestationDocument_MaxAge(t *testing.T) { + fa, err := nitrofake.NewAttestor() + require.NoError(t, err) + doc, err := fa.CreateAttestation([]byte("user-data")) + require.NoError(t, err) + pool := fa.CARoots() + + // Fresh: accepted. + _, err = verifyAttestationDocument(doc, pool, time.Now()) + require.NoError(t, err) + + // Older than the window: rejected (cert is still valid, only age fails). + _, err = verifyAttestationDocument(doc, pool, time.Now().Add(MaxAttestationAge+time.Minute)) + require.ErrorIs(t, err, errStaleAttestation) +} + func TestValidateAttestation_WrongUserData(t *testing.T) { fa, err := nitrofake.NewAttestor() require.NoError(t, err) diff --git a/pkg/teeattestation/nitro/verify.go b/pkg/teeattestation/nitro/verify.go index ac141a60a8..3ac8df9f0e 100644 --- a/pkg/teeattestation/nitro/verify.go +++ b/pkg/teeattestation/nitro/verify.go @@ -23,6 +23,7 @@ var ( errMandatoryFieldsMissing = errors.New("attestation document is missing mandatory fields") errBadDigest = errors.New("attestation digest is not SHA384") errBadTimestamp = errors.New("attestation timestamp is 0") + errStaleAttestation = errors.New("attestation is older than the max allowed age") errBadPCRs = errors.New("attestation pcrs is less than 1 or more than 32") errBadPCRIndex = errors.New("attestation pcr index is not in [0, 32)") errBadPCRValue = errors.New("attestation pcr value length is invalid") @@ -36,6 +37,9 @@ var ( errBadSignature = errors.New("attestation signature does not match certificate") ) +// MaxAttestationAge rejects attestations not consumed promptly after issuance. [CL112-10] +const MaxAttestationAge = 5 * time.Minute + type verifyResult struct { document *attestationDocument signatureOK bool @@ -114,6 +118,12 @@ func verifyAttestationDocument(data []byte, roots *x509.CertPool, currentTime ti if currentTime.IsZero() { currentTime = time.Now() } + + // doc.Timestamp is epoch milliseconds (already validated non-zero). + issued := time.UnixMilli(int64(doc.Timestamp)) //nolint:gosec // timestamp is positive and within int64 + if currentTime.Sub(issued) > MaxAttestationAge { + return nil, errStaleAttestation + } if _, err := leafCert.Verify(x509.VerifyOptions{ Intermediates: intermediates, Roots: roots, From 8f6bb6ae3c3ff207304a19c7d12a4361e64b2721 Mon Sep 17 00:00:00 2001 From: Tejaswi Nadahalli Date: Thu, 18 Jun 2026 16:53:01 +0200 Subject: [PATCH 2/2] address Copilot review on PR #2170 - Unexport maxAttestationAge (in-package use only). - Bound doc.Timestamp to int64 before time.UnixMilli; broaden errBadTimestamp. - Reject far-future timestamps too via absolute skew; reword errStaleAttestation. - Add a future-timestamp test case. --- pkg/teeattestation/nitro/validate_test.go | 14 +++++++++----- pkg/teeattestation/nitro/verify.go | 19 ++++++++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/pkg/teeattestation/nitro/validate_test.go b/pkg/teeattestation/nitro/validate_test.go index 9e325e4c2f..a3b93caca6 100644 --- a/pkg/teeattestation/nitro/validate_test.go +++ b/pkg/teeattestation/nitro/validate_test.go @@ -23,9 +23,9 @@ func TestValidateAttestation_Attestor(t *testing.T) { require.NoError(t, err) } -// TestVerifyAttestationDocument_MaxAge covers PRIV-438 / CL112-10: a fresh -// attestation verifies, but one older than MaxAttestationAge is rejected even -// though the leaf cert is still valid. +// TestVerifyAttestationDocument_MaxAge covers PRIV-438 / CL112-10: fresh +// attestations verify; stale or far-future ones are rejected even while the +// leaf cert is still valid. func TestVerifyAttestationDocument_MaxAge(t *testing.T) { fa, err := nitrofake.NewAttestor() require.NoError(t, err) @@ -37,8 +37,12 @@ func TestVerifyAttestationDocument_MaxAge(t *testing.T) { _, err = verifyAttestationDocument(doc, pool, time.Now()) require.NoError(t, err) - // Older than the window: rejected (cert is still valid, only age fails). - _, err = verifyAttestationDocument(doc, pool, time.Now().Add(MaxAttestationAge+time.Minute)) + // Too old: rejected (cert is still valid, only age fails). + _, err = verifyAttestationDocument(doc, pool, time.Now().Add(maxAttestationAge+time.Minute)) + require.ErrorIs(t, err, errStaleAttestation) + + // Too far in the future: also rejected. + _, err = verifyAttestationDocument(doc, pool, time.Now().Add(-(maxAttestationAge + time.Minute))) require.ErrorIs(t, err, errStaleAttestation) } diff --git a/pkg/teeattestation/nitro/verify.go b/pkg/teeattestation/nitro/verify.go index 3ac8df9f0e..d8a4fbc4b7 100644 --- a/pkg/teeattestation/nitro/verify.go +++ b/pkg/teeattestation/nitro/verify.go @@ -7,6 +7,7 @@ import ( "crypto/x509" "errors" "fmt" + "math" "math/big" "time" @@ -22,8 +23,8 @@ var ( errBadAttestationDocument = errors.New("bad attestation document") errMandatoryFieldsMissing = errors.New("attestation document is missing mandatory fields") errBadDigest = errors.New("attestation digest is not SHA384") - errBadTimestamp = errors.New("attestation timestamp is 0") - errStaleAttestation = errors.New("attestation is older than the max allowed age") + errBadTimestamp = errors.New("attestation timestamp is zero or out of range") + errStaleAttestation = errors.New("attestation timestamp is outside the allowed window") errBadPCRs = errors.New("attestation pcrs is less than 1 or more than 32") errBadPCRIndex = errors.New("attestation pcr index is not in [0, 32)") errBadPCRValue = errors.New("attestation pcr value length is invalid") @@ -37,8 +38,8 @@ var ( errBadSignature = errors.New("attestation signature does not match certificate") ) -// MaxAttestationAge rejects attestations not consumed promptly after issuance. [CL112-10] -const MaxAttestationAge = 5 * time.Minute +// maxAttestationAge rejects attestations not consumed promptly after issuance. [CL112-10] +const maxAttestationAge = 5 * time.Minute type verifyResult struct { document *attestationDocument @@ -119,9 +120,13 @@ func verifyAttestationDocument(data []byte, roots *x509.CertPool, currentTime ti currentTime = time.Now() } - // doc.Timestamp is epoch milliseconds (already validated non-zero). - issued := time.UnixMilli(int64(doc.Timestamp)) //nolint:gosec // timestamp is positive and within int64 - if currentTime.Sub(issued) > MaxAttestationAge { + // doc.Timestamp is epoch milliseconds; reject if it overflows int64. + if doc.Timestamp > math.MaxInt64 { + return nil, errBadTimestamp + } + issued := time.UnixMilli(int64(doc.Timestamp)) + // Reject both stale and far-future timestamps (absolute skew). + if skew := currentTime.Sub(issued); skew > maxAttestationAge || skew < -maxAttestationAge { return nil, errStaleAttestation } if _, err := leafCert.Verify(x509.VerifyOptions{