From 98d895b5289e0f953b8fb4ec2ca0f7e8c38dc3be Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:02:40 -0700 Subject: [PATCH 01/10] refactor: use dustin/go-humanize for http.cache.size parsing Replace hand-rolled parseByteSize with humanize.ParseBytes, adding support for SI units (KB, MB, GB), fractional values, and flexible whitespace. Co-Authored-By: Claude Opus 4.6 (1M context) --- go.mod | 4 ++-- go.sum | 2 -- internal/config.go | 43 ++++++----------------------------- internal/endorsements_test.go | 16 +++++++++++++ 4 files changed, 25 insertions(+), 40 deletions(-) diff --git a/go.mod b/go.mod index 50d224b..77447c5 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.26.1 require ( github.com/dgraph-io/ristretto/v2 v2.4.0 + github.com/dustin/go-humanize v1.0.1 github.com/fsnotify/fsnotify v1.9.0 github.com/fxamacker/cbor/v2 v2.7.0 github.com/goccy/go-json v0.10.6 @@ -19,6 +20,7 @@ require ( github.com/spf13/viper v1.21.0 golang.org/x/crypto/x509roots/fallback v0.0.0-20260323153451-8400f4a93807 golang.org/x/sync v0.20.0 + golang.org/x/time v0.15.0 ) require ( @@ -30,7 +32,6 @@ require ( github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467 // indirect github.com/digitorus/pkcs7 v0.0.0-20230818184609-3a137a874352 // indirect github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 // indirect - github.com/dustin/go-humanize v1.0.1 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/analysis v0.24.1 // indirect @@ -105,7 +106,6 @@ require ( golang.org/x/sys v0.39.0 // indirect golang.org/x/term v0.38.0 // indirect golang.org/x/text v0.32.0 // indirect - golang.org/x/time v0.15.0 // indirect golang.org/x/tools v0.40.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250929231259-57b25ae835d4 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20251103181224-f26f9409b101 // indirect diff --git a/go.sum b/go.sum index 335c541..441a9af 100644 --- a/go.sum +++ b/go.sum @@ -443,8 +443,6 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.32.0 h1:ZD01bjUt1FQ9WJ0ClOL5vxgxOI/sVCNgX1YtKwcY0mU= golang.org/x/text v0.32.0/go.mod h1:o/rUWzghvpD5TXrTIBuJU77MTaN0ljMWE47kxGJQ7jY= -golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= -golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= diff --git a/internal/config.go b/internal/config.go index c238770..3f5b942 100644 --- a/internal/config.go +++ b/internal/config.go @@ -6,10 +6,10 @@ import ( "net/url" "path/filepath" "regexp" - "strconv" "strings" "time" + "github.com/dustin/go-humanize" "github.com/google/go-tpm/tpm2" "github.com/spf13/viper" ) @@ -308,49 +308,20 @@ func parseLogLevel(s string) slog.Level { } } -// parseByteSize parses a human-readable byte size string like "100MiB" or -// "1GiB" into a byte count. Supported suffixes: B, KiB, MiB, GiB, TiB -// (case-insensitive). A bare number without suffix is treated as bytes. +// parseByteSize parses a human-readable byte size string into a byte count +// using github.com/dustin/go-humanize. Supports both SI (KB, MB, GB, TB) and +// IEC (KiB, MiB, GiB, TiB) suffixes, fractional values, and flexible +// whitespace. func parseByteSize(s string) (int64, error) { s = strings.TrimSpace(s) if s == "" { return 0, fmt.Errorf("empty byte size") } - - suffixes := []struct { - suffix string - multiplier int64 - }{ - {"TiB", 1 << 40}, - {"GiB", 1 << 30}, - {"MiB", 1 << 20}, - {"KiB", 1 << 10}, - {"B", 1}, - } - - lower := strings.ToLower(s) - for _, sf := range suffixes { - if strings.HasSuffix(lower, strings.ToLower(sf.suffix)) { - numStr := strings.TrimSpace(s[:len(s)-len(sf.suffix)]) - n, err := strconv.ParseInt(numStr, 10, 64) - if err != nil { - return 0, fmt.Errorf("invalid byte size %q: %w", s, err) - } - if n < 0 { - return 0, fmt.Errorf("negative byte size %q", s) - } - return n * sf.multiplier, nil - } - } - - n, err := strconv.ParseInt(s, 10, 64) + n, err := humanize.ParseBytes(s) if err != nil { return 0, fmt.Errorf("invalid byte size %q: %w", s, err) } - if n < 0 { - return 0, fmt.Errorf("negative byte size %q", s) - } - return n, nil + return int64(n), nil } // domainNameRe matches valid DNS domain names (no ports, no paths). diff --git a/internal/endorsements_test.go b/internal/endorsements_test.go index 9745d4a..7ee7075 100644 --- a/internal/endorsements_test.go +++ b/internal/endorsements_test.go @@ -443,13 +443,26 @@ func TestParseByteSize(t *testing.T) { want int64 wantError bool }{ + // IEC (binary) suffixes {"100MiB", 100 << 20, false}, {"1GiB", 1 << 30, false}, {"512KiB", 512 << 10, false}, {"1024B", 1024, false}, {"2TiB", 2 << 40, false}, + // SI (decimal) suffixes + {"100MB", 100_000_000, false}, + {"1GB", 1_000_000_000, false}, + {"512KB", 512_000, false}, + {"2TB", 2_000_000_000_000, false}, + // Bare number (bytes) {"42", 42, false}, {"0", 0, false}, + // Whitespace + {" 10 MiB ", 10 << 20, false}, + // Case insensitive + {"100mib", 100 << 20, false}, + {"1gib", 1 << 30, false}, + // Errors {"", 0, true}, {"abc", 0, true}, {"-1MiB", 0, true}, @@ -1583,6 +1596,9 @@ func FuzzParseCacheTTL(f *testing.F) { func FuzzParseByteSize(f *testing.F) { f.Add("100MiB") f.Add("1GiB") + f.Add("100MB") + f.Add("1.5GiB") + f.Add(" 10 MiB ") f.Add("") f.Add("0") f.Add("-1") From 4ead0452af1f77fba3956b09615ed0aa48557d20 Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:04:16 -0700 Subject: [PATCH 02/10] fix: default bind address to 127.0.0.1 instead of 0.0.0.0 The server runs behind an Envoy reverse proxy, so there is no need to bind on all interfaces by default. Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 4 ++-- cmd/root.go | 2 +- config/config.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index de37410..6b9a200 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,7 +61,7 @@ format = "json" level = "info" [server] -host = "0.0.0.0" +host = "127.0.0.1" port = 8187 [paths] @@ -150,7 +150,7 @@ All settings can be configured via environment variables prefixed with `ATTESTAT | `ATTESTATION_SERVER_CONFIG_FILE` | — | — | Path to TOML config file | | `ATTESTATION_SERVER_LOG_FORMAT` | `log.format` | `json` | Log format: `json`/`text` | | `ATTESTATION_SERVER_LOG_LEVEL` | `log.level` | `info` | Log level: `debug`/`info`/`warn`/`error` | -| `ATTESTATION_SERVER_SERVER_HOST` | `server.host` | `0.0.0.0` | HTTP bind host | +| `ATTESTATION_SERVER_SERVER_HOST` | `server.host` | `127.0.0.1` | HTTP bind host | | `ATTESTATION_SERVER_SERVER_PORT` | `server.port` | `8187` | HTTP bind port | | `ATTESTATION_SERVER_PATHS_BUILD_INFO` | `paths.build_info` | `/etc/build-info.json` | Path to build information file | | `ATTESTATION_SERVER_PATHS_ENDORSEMENTS` | `paths.endorsements` | `/etc/endorsements.json` | Path to endorsements URL list file | diff --git a/cmd/root.go b/cmd/root.go index 32bfc21..aaa0c1e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -51,7 +51,7 @@ func initConfig() { // Defaults viper.SetDefault("log.format", "json") viper.SetDefault("log.level", "info") - viper.SetDefault("server.host", "0.0.0.0") + viper.SetDefault("server.host", "127.0.0.1") viper.SetDefault("server.port", 8187) viper.SetDefault("paths.build_info", "/etc/build-info.json") viper.SetDefault("paths.endorsements", "/etc/endorsements.json") diff --git a/config/config.toml b/config/config.toml index 8d6fc24..b47609f 100644 --- a/config/config.toml +++ b/config/config.toml @@ -5,7 +5,7 @@ format = "json" # json, text level = "info" # debug, info, warn, error [server] -host = "0.0.0.0" +host = "127.0.0.1" port = 8187 [paths] From 6e466913e1e5e51dba7528b0d116fa5747ac6b6f Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:14:14 -0700 Subject: [PATCH 03/10] feat: make HTTP cache default TTL configurable (http.cache.default_ttl) Add http.cache.default_ttl config option (env: ATTESTATION_SERVER_HTTP_CACHE_DEFAULT_TTL) to control the fallback TTL when responses have no Cache-Control header. Default bumped from 30m to 1h. Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 8 +++++--- cmd/root.go | 2 ++ config/config.toml | 3 ++- internal/config.go | 6 ++++++ internal/cosign.go | 4 ++-- internal/endorsements.go | 4 ++-- internal/endorsements_test.go | 8 ++++---- internal/fetch.go | 23 ++++++++++++----------- internal/server.go | 7 ++++--- 9 files changed, 39 insertions(+), 26 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6b9a200..37f1fba 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -115,7 +115,8 @@ uri_regex = "" allow_proxy = false [http.cache] -size = "100MiB" +size = "100MiB" +default_ttl = "1h" [dependencies] endpoints = [] @@ -186,6 +187,7 @@ All settings can be configured via environment variables prefixed with `ATTESTAT | `ATTESTATION_SERVER_ENDORSEMENTS_COSIGN_BUILD_SIGNER_URI_REGEX` | `endorsements.cosign.build_signer.uri_regex` | — | Regex match override for BuildSignerURI Fulcio OID (ignored if `uri` is set) | | `ATTESTATION_SERVER_HTTP_ALLOW_PROXY` | `http.allow_proxy` | `false` | Honour `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` env vars for the server's outbound HTTP clients (endorsement/cosign fetches, SEV-SNP CRL fetches, dependency requests). Off by default; required in environments like AWS Nitro Enclaves where a vsock-proxy is the only egress path. TDX collateral fetching (go-tdx-guest) always honours proxy env vars via `http.DefaultTransport` regardless of this setting | | `ATTESTATION_SERVER_HTTP_CACHE_SIZE` | `http.cache.size` | `100MiB` | Maximum memory for the shared HTTP fetch cache (endorsements + cosign signatures, ristretto) | +| `ATTESTATION_SERVER_HTTP_CACHE_DEFAULT_TTL` | `http.cache.default_ttl` | `1h` | Default cache TTL when response has no Cache-Control header (capped at 24h) | List-typed environment variables (`ATTESTATION_SERVER_REPORT_USER_DATA_ENV`, `ATTESTATION_SERVER_DEPENDENCIES_ENDPOINTS`) support comma-separated values: `VAR=a,b,c`. Spaces around commas are trimmed. @@ -297,7 +299,7 @@ Over-limit requests are **stalled** (blocked in a FIFO queue) up to `ratelimit.s When `revocation.enabled` is true (the default), the server checks TEE endorsement key certificates against Certificate Revocation Lists. CRL fetching is conditional on configuration: - **SEV-SNP**: A background goroutine fetches AMD KDS CRLs for all supported product lines (Milan, Genoa, Turin) at `revocation.refresh_interval` (default 12h). Both VCEK and VLEK CRLs are fetched. CRLs are initialized when local SEV-SNP evidence is enabled **or** when dependency endpoints are configured (dependencies may include SEV-SNP evidence requiring revocation checks). The `crlCache` stores parsed `x509.RevocationList` entries and checks endorsement key serial numbers during verification. Design is **fail-open**: if no CRL data is available yet (first fetch still pending or failed), certificates are accepted. CRL fetches use the server's `fetchHTTPClient()` and honour `http.allow_proxy`. -- **TDX**: Revocation checking is delegated to go-tdx-guest's built-in Intel PCS collateral fetching (`CheckRevocations: true, GetCollateral: true`). The server provides a `cachedHTTPSGetter` (via `VerifyOpt.Getter`) that caches Intel PCS responses (TCB info, QE identity, PCK CRL, Root CA CRL) in the shared ristretto cache. On cache hit, no network calls are made. TTL is derived from response `Cache-Control` headers; Intel PCS currently returns no cache headers, so the default 30-minute TTL applies. The go-tdx-guest library still validates `NextUpdate` expiry on all collateral, so stale cached data is rejected. The cached getter uses the server's `fetchHTTPClient()` and honours `http.allow_proxy`. +- **TDX**: Revocation checking is delegated to go-tdx-guest's built-in Intel PCS collateral fetching (`CheckRevocations: true, GetCollateral: true`). The server provides a `cachedHTTPSGetter` (via `VerifyOpt.Getter`) that caches Intel PCS responses (TCB info, QE identity, PCK CRL, Root CA CRL) in the shared ristretto cache. On cache hit, no network calls are made. TTL is derived from response `Cache-Control` headers; Intel PCS currently returns no cache headers, so `http.cache.default_ttl` applies. The go-tdx-guest library still validates `NextUpdate` expiry on all collateral, so stale cached data is rejected. The cached getter uses the server's `fetchHTTPClient()` and honours `http.allow_proxy`. - **Nitro**: No CRL mechanism exists (ephemeral certificate chains per attestation; revocation is handled by AWS at the hypervisor level). When disabled, a startup warning is logged: "certificate revocation checking is disabled, revoked TEE endorsement keys will be accepted". @@ -347,7 +349,7 @@ Uses system/Mozilla root CAs (via `golang.org/x/crypto/x509roots/fallback` blank ### Endorsement cache -Uses `dgraph-io/ristretto/v2` with URL-string keys in a shared `fetcherCache` (stores both `*EndorsementDocument` and `*cosignResult` values — endorsement URLs and signature URLs don't collide). When multiple URLs resolve to the same document (verified byte-for-byte), the same pointer is stored under all URL keys (cost charged once). TTL is derived from Cache-Control `max-age` (capped at 24h, default 30m). +Uses `dgraph-io/ristretto/v2` with URL-string keys in a shared `fetcherCache` (stores both `*EndorsementDocument` and `*cosignResult` values — endorsement URLs and signature URLs don't collide). When multiple URLs resolve to the same document (verified byte-for-byte), the same pointer is stored under all URL keys (cost charged once). TTL is derived from Cache-Control `max-age` (capped at 24h, default `http.cache.default_ttl`). Endorsement URLs are tied to CI commit hashes with immutable content. Extended caching (up to 24h) is by design — measurement changes require new commits and new URLs. The TTL cap and per-request revalidation on cache miss provide eventual consistency. diff --git a/cmd/root.go b/cmd/root.go index aaa0c1e..257fc7b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -76,6 +76,7 @@ func initConfig() { viper.SetDefault("endorsements.client.timeout", "10s") viper.SetDefault("http.allow_proxy", false) viper.SetDefault("http.cache.size", "100MiB") + viper.SetDefault("http.cache.default_ttl", "1h") viper.SetDefault("tls.public.skip_verify", false) viper.SetDefault("endorsements.cosign.verify", true) viper.SetDefault("endorsements.cosign.url_suffix", ".sig") @@ -117,6 +118,7 @@ func initConfig() { _ = viper.BindEnv("endorsements.client.timeout", "ATTESTATION_SERVER_ENDORSEMENTS_CLIENT_TIMEOUT") _ = viper.BindEnv("http.allow_proxy", "ATTESTATION_SERVER_HTTP_ALLOW_PROXY") _ = viper.BindEnv("http.cache.size", "ATTESTATION_SERVER_HTTP_CACHE_SIZE") + _ = viper.BindEnv("http.cache.default_ttl", "ATTESTATION_SERVER_HTTP_CACHE_DEFAULT_TTL") _ = viper.BindEnv("endorsements.cosign.verify", "ATTESTATION_SERVER_ENDORSEMENTS_COSIGN_VERIFY") _ = viper.BindEnv("endorsements.cosign.url_suffix", "ATTESTATION_SERVER_ENDORSEMENTS_COSIGN_URL_SUFFIX") _ = viper.BindEnv("endorsements.cosign.tuf_cache_path", "ATTESTATION_SERVER_ENDORSEMENTS_COSIGN_TUF_CACHE_PATH") diff --git a/config/config.toml b/config/config.toml index b47609f..f567d45 100644 --- a/config/config.toml +++ b/config/config.toml @@ -59,7 +59,8 @@ uri_regex = "" # regex match override for BuildSignerURI OID (ignored if uri is allow_proxy = false # honour HTTP_PROXY/HTTPS_PROXY/NO_PROXY env vars (off by default; needed in Nitro Enclaves with vsock-proxy) [http.cache] -size = "100MiB" # max memory for shared HTTP fetch cache (endorsements + cosign signatures) +size = "100MiB" # max memory for shared HTTP fetch cache (endorsements + cosign signatures) +default_ttl = "1h" # default TTL when response has no Cache-Control header (capped at 24h) [dependencies] # URLs of dependency attestation servers whose reports are fetched, diff --git a/internal/config.go b/internal/config.go index 3f5b942..7899335 100644 --- a/internal/config.go +++ b/internal/config.go @@ -47,6 +47,7 @@ type Config struct { EndorsementClientTimeout time.Duration HTTPAllowProxy bool HTTPCacheSize int64 + HTTPCacheDefaultTTL time.Duration RevocationEnabled bool RevocationRefreshInterval time.Duration RateLimitEnabled bool @@ -121,6 +122,10 @@ func LoadConfig() (*Config, error) { if err != nil { httpCacheSize = 100 << 20 } + httpCacheDefaultTTL, err := time.ParseDuration(viper.GetString("http.cache.default_ttl")) + if err != nil { + httpCacheDefaultTTL = time.Hour + } revocationRefreshInterval, err := time.ParseDuration(viper.GetString("revocation.refresh_interval")) if err != nil { @@ -172,6 +177,7 @@ func LoadConfig() (*Config, error) { EndorsementClientTimeout: endorsementTimeout, HTTPAllowProxy: viper.GetBool("http.allow_proxy"), HTTPCacheSize: httpCacheSize, + HTTPCacheDefaultTTL: httpCacheDefaultTTL, CosignVerify: viper.GetBool("endorsements.cosign.verify"), CosignURLSuffix: viper.GetString("endorsements.cosign.url_suffix"), CosignTUFCachePath: viper.GetString("endorsements.cosign.tuf_cache_path"), diff --git a/internal/cosign.go b/internal/cosign.go index c7ff7e1..8f7b4d6 100644 --- a/internal/cosign.go +++ b/internal/cosign.go @@ -126,13 +126,13 @@ func (s *Server) fetchCosignSignatures(ctx context.Context, urls []*url.URL, cli // Use the most conservative (shortest) TTL across all responses ttl := fetchMaxTTL for _, r := range results { - t := parseCacheTTL(r.header) + t := parseCacheTTL(r.header, s.cfg.HTTPCacheDefaultTTL) if t < ttl { ttl = t } } if ttl <= 0 { - ttl = fetchDefaultTTL + ttl = s.cfg.HTTPCacheDefaultTTL } return results[0].body, len(results[0].body), ttl, nil diff --git a/internal/endorsements.go b/internal/endorsements.go index 3055f3b..1f64b2b 100644 --- a/internal/endorsements.go +++ b/internal/endorsements.go @@ -96,13 +96,13 @@ func (s *Server) fetchEndorsementDocumentsWithClient(ctx context.Context, urls [ // Use the most conservative (shortest) TTL across all responses ttl := fetchMaxTTL for _, r := range results { - t := parseCacheTTL(r.header) + t := parseCacheTTL(r.header, s.cfg.HTTPCacheDefaultTTL) if t < ttl { ttl = t } } if ttl <= 0 { - ttl = fetchDefaultTTL + ttl = s.cfg.HTTPCacheDefaultTTL } return &doc, results[0].body, len(results[0].body), ttl, nil diff --git a/internal/endorsements_test.go b/internal/endorsements_test.go index 7ee7075..d7f5429 100644 --- a/internal/endorsements_test.go +++ b/internal/endorsements_test.go @@ -381,9 +381,9 @@ func TestParseCacheTTL(t *testing.T) { want: 0, }, { - name: "no headers returns default 30m", + name: "no headers returns default", headers: map[string]string{}, - want: 30 * time.Minute, + want: time.Hour, }, { name: "max-age with other directives", @@ -422,7 +422,7 @@ func TestParseCacheTTL(t *testing.T) { for k, v := range tt.headers { h.Set(k, v) } - got := parseCacheTTL(h) + got := parseCacheTTL(h, time.Hour) // Allow 2s tolerance for time-based tests diff := got - tt.want if diff < 0 { @@ -1588,7 +1588,7 @@ func FuzzParseCacheTTL(f *testing.F) { f.Fuzz(func(t *testing.T, cc string) { h := http.Header{} h.Set("Cache-Control", cc) - parseCacheTTL(h) + parseCacheTTL(h, time.Hour) }) } diff --git a/internal/fetch.go b/internal/fetch.go index 3d41e42..4d3a74a 100644 --- a/internal/fetch.go +++ b/internal/fetch.go @@ -36,8 +36,7 @@ const ( fetchRetryInitial = 500 * time.Millisecond fetchRetryMax = 5 * time.Second - fetchDefaultTTL = 30 * time.Minute - fetchMaxTTL = 24 * time.Hour + fetchMaxTTL = 24 * time.Hour ) // fetcherCache is a generic ristretto cache keyed by URL strings. It stores @@ -121,8 +120,8 @@ func (s *Server) fetchHTTPClient() *http.Client { // parseCacheTTL extracts a TTL from HTTP response headers. It checks // Cache-Control for max-age and no-cache/no-store, falls back to the -// Expires header, and defaults to 30 minutes. TTL is capped at 24 hours. -func parseCacheTTL(header http.Header) time.Duration { +// Expires header, and defaults to defaultTTL. TTL is capped at 24 hours. +func parseCacheTTL(header http.Header, defaultTTL time.Duration) time.Duration { if cc := header.Get("Cache-Control"); cc != "" { lower := strings.ToLower(cc) if strings.Contains(lower, "no-cache") || strings.Contains(lower, "no-store") { @@ -161,7 +160,7 @@ func parseCacheTTL(header http.Header) time.Duration { } } - return fetchDefaultTTL + return defaultTTL } // fetchResult holds the result of fetching a single URL. @@ -247,12 +246,14 @@ type cachedHTTPSGetterEntry struct { // cachedHTTPSGetter implements trust.HTTPSGetter with URL-keyed caching // backed by the shared ristretto fetcherCache. On cache hit the network is // skipped entirely. On miss the inner getter is called and the response is -// cached with a TTL derived from the response headers (defaulting to 30 min -// when no Cache-Control is present, which is the case for Intel PCS). +// cached with a TTL derived from the response headers (defaulting to +// http.cache.default_ttl when no Cache-Control is present, which is the case +// for Intel PCS). type cachedHTTPSGetter struct { - inner *simpleHTTPSGetter - cache *fetcherCache - logger *slog.Logger + inner *simpleHTTPSGetter + cache *fetcherCache + logger *slog.Logger + defaultTTL time.Duration } // simpleHTTPSGetter is a proxy-aware replacement for trust.SimpleHTTPSGetter. @@ -298,7 +299,7 @@ func (g *cachedHTTPSGetter) Get(rawURL string) (map[string][]string, []byte, err return nil, nil, err } - ttl := parseCacheTTL(http.Header(header)) + ttl := parseCacheTTL(http.Header(header), g.defaultTTL) entry := &cachedHTTPSGetterEntry{header: header, body: body} g.cache.setGroup([]string{rawURL}, entry, len(body), ttl) diff --git a/internal/server.go b/internal/server.go index 6dfd3cf..675415d 100644 --- a/internal/server.go +++ b/internal/server.go @@ -291,9 +291,10 @@ func NewServer(cfg *Config, logger *slog.Logger) (*Server, error) { s.httpCache = cache } s.tdxGetter = &cachedHTTPSGetter{ - inner: &simpleHTTPSGetter{client: s.fetchHTTPClient()}, - cache: s.httpCache, - logger: logger, + inner: &simpleHTTPSGetter{client: s.fetchHTTPClient()}, + cache: s.httpCache, + defaultTTL: cfg.HTTPCacheDefaultTTL, + logger: logger, } logger.Info("tdx collateral caching enabled") } From ad7ddbf93aa503f6b78b29b492df3661ad98e175 Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:17:59 -0700 Subject: [PATCH 04/10] fix: fail on invalid duration and byte-size config values Previously, parse errors for endorsements.client.timeout, http.cache.size, http.cache.default_ttl, revocation.refresh_interval, and ratelimit.stall_timeout were silently replaced with defaults. This masked misconfigurations in a security-critical service. Now the server exits with a clear error on any unparseable value. Viper defaults still apply when no override is provided. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config.go | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/internal/config.go b/internal/config.go index 7899335..471330d 100644 --- a/internal/config.go +++ b/internal/config.go @@ -114,27 +114,25 @@ func LoadConfig() (*Config, error) { return nil, err } - endorsementTimeout, err := time.ParseDuration(viper.GetString("endorsements.client.timeout")) + endorsementTimeout, err := parseDuration("endorsements.client.timeout") if err != nil { - endorsementTimeout = 10 * time.Second + return nil, err } httpCacheSize, err := parseByteSize(viper.GetString("http.cache.size")) if err != nil { - httpCacheSize = 100 << 20 + return nil, fmt.Errorf("http.cache.size: %w", err) } - httpCacheDefaultTTL, err := time.ParseDuration(viper.GetString("http.cache.default_ttl")) + httpCacheDefaultTTL, err := parseDuration("http.cache.default_ttl") if err != nil { - httpCacheDefaultTTL = time.Hour + return nil, err } - - revocationRefreshInterval, err := time.ParseDuration(viper.GetString("revocation.refresh_interval")) + revocationRefreshInterval, err := parseDuration("revocation.refresh_interval") if err != nil { - revocationRefreshInterval = 12 * time.Hour + return nil, err } - - rateLimitStallTimeout, err := time.ParseDuration(viper.GetString("ratelimit.stall_timeout")) + rateLimitStallTimeout, err := parseDuration("ratelimit.stall_timeout") if err != nil { - rateLimitStallTimeout = 10 * time.Second + return nil, err } cosignBuildSigner := CosignBuildSignerConfig{ @@ -314,6 +312,17 @@ func parseLogLevel(s string) slog.Level { } } +// parseDuration reads a viper string key and parses it as a time.Duration. +// Returns an error if the value is non-empty and unparseable. +func parseDuration(key string) (time.Duration, error) { + s := viper.GetString(key) + d, err := time.ParseDuration(s) + if err != nil { + return 0, fmt.Errorf("%s: invalid duration %q: %w", key, s, err) + } + return d, nil +} + // parseByteSize parses a human-readable byte size string into a byte count // using github.com/dustin/go-humanize. Supports both SI (KB, MB, GB, TB) and // IEC (KiB, MiB, GiB, TiB) suffixes, fractional values, and flexible From 50b3025628bafcd3cfed3201292830327c961645 Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:35:23 -0700 Subject: [PATCH 05/10] fix: clamp defaultTTL to 24h cap in parseCacheTTL A configured http.cache.default_ttl >24h would bypass the documented 24h cap because the default return path did not clamp the value. Now parseCacheTTL clamps defaultTTL at entry so all return paths (max-age, Expires, and default) respect the cap. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/endorsements_test.go | 7 +++++++ internal/fetch.go | 3 +++ 2 files changed, 10 insertions(+) diff --git a/internal/endorsements_test.go b/internal/endorsements_test.go index d7f5429..f5f7ed0 100644 --- a/internal/endorsements_test.go +++ b/internal/endorsements_test.go @@ -433,6 +433,13 @@ func TestParseCacheTTL(t *testing.T) { } }) } + + t.Run("defaultTTL capped at 24h", func(t *testing.T) { + got := parseCacheTTL(http.Header{}, 48*time.Hour) + if got != 24*time.Hour { + t.Errorf("parseCacheTTL() with 48h default = %v, want %v", got, 24*time.Hour) + } + }) } // --- parseByteSize --- diff --git a/internal/fetch.go b/internal/fetch.go index 4d3a74a..c025de4 100644 --- a/internal/fetch.go +++ b/internal/fetch.go @@ -122,6 +122,9 @@ func (s *Server) fetchHTTPClient() *http.Client { // Cache-Control for max-age and no-cache/no-store, falls back to the // Expires header, and defaults to defaultTTL. TTL is capped at 24 hours. func parseCacheTTL(header http.Header, defaultTTL time.Duration) time.Duration { + if defaultTTL > fetchMaxTTL { + defaultTTL = fetchMaxTTL + } if cc := header.Get("Cache-Control"); cc != "" { lower := strings.ToLower(cc) if strings.Contains(lower, "no-cache") || strings.Contains(lower, "no-store") { From 97ef6232221afce5eb9ebb780e79dfcfe4ac1ed4 Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:36:19 -0700 Subject: [PATCH 06/10] fix: guard parseByteSize against uint64-to-int64 overflow humanize.ParseBytes returns uint64; values above math.MaxInt64 (e.g. "9EiB") would silently wrap to negative on the int64 cast. Now returns a clear error instead. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config.go | 4 ++++ internal/endorsements_test.go | 1 + 2 files changed, 5 insertions(+) diff --git a/internal/config.go b/internal/config.go index 471330d..b79da3d 100644 --- a/internal/config.go +++ b/internal/config.go @@ -3,6 +3,7 @@ package app import ( "fmt" "log/slog" + "math" "net/url" "path/filepath" "regexp" @@ -336,6 +337,9 @@ func parseByteSize(s string) (int64, error) { if err != nil { return 0, fmt.Errorf("invalid byte size %q: %w", s, err) } + if n > uint64(math.MaxInt64) { + return 0, fmt.Errorf("byte size %q overflows int64", s) + } return int64(n), nil } diff --git a/internal/endorsements_test.go b/internal/endorsements_test.go index f5f7ed0..a9dbd31 100644 --- a/internal/endorsements_test.go +++ b/internal/endorsements_test.go @@ -474,6 +474,7 @@ func TestParseByteSize(t *testing.T) { {"abc", 0, true}, {"-1MiB", 0, true}, {"-5", 0, true}, + {"9EiB", 0, true}, // overflows int64 } for _, tt := range tests { From 8b20cae9341c29af159536b0f9863a7e4435fcc8 Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:37:15 -0700 Subject: [PATCH 07/10] fix: respect no-cache/no-store TTL=0 from parseCacheTTL parseCacheTTL returns 0 to signal "do not cache" (no-cache, no-store, or expired Expires header). The ttl <= 0 guard in endorsements.go and cosign.go was overriding this with the default TTL, causing responses that should not be cached to be cached. Changed to ttl < 0 so only truly invalid values get the fallback. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/cosign.go | 2 +- internal/endorsements.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/cosign.go b/internal/cosign.go index 8f7b4d6..bfd1298 100644 --- a/internal/cosign.go +++ b/internal/cosign.go @@ -131,7 +131,7 @@ func (s *Server) fetchCosignSignatures(ctx context.Context, urls []*url.URL, cli ttl = t } } - if ttl <= 0 { + if ttl < 0 { ttl = s.cfg.HTTPCacheDefaultTTL } diff --git a/internal/endorsements.go b/internal/endorsements.go index 1f64b2b..acac702 100644 --- a/internal/endorsements.go +++ b/internal/endorsements.go @@ -101,7 +101,7 @@ func (s *Server) fetchEndorsementDocumentsWithClient(ctx context.Context, urls [ ttl = t } } - if ttl <= 0 { + if ttl < 0 { ttl = s.cfg.HTTPCacheDefaultTTL } From c601b56c87cc9f455617131cc835e5519d0a43cc Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:43:12 -0700 Subject: [PATCH 08/10] docs: fix parseDuration comment to reflect actual behavior The comment said "non-empty and unparseable" implying empty strings were accepted, but time.ParseDuration("") returns an error. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config.go b/internal/config.go index b79da3d..5ff6633 100644 --- a/internal/config.go +++ b/internal/config.go @@ -314,7 +314,7 @@ func parseLogLevel(s string) slog.Level { } // parseDuration reads a viper string key and parses it as a time.Duration. -// Returns an error if the value is non-empty and unparseable. +// Returns an error if the value is empty or unparseable. func parseDuration(key string) (time.Duration, error) { s := viper.GetString(key) d, err := time.ParseDuration(s) From ae5e0121d8322c6eb96945fe694a275cd0c6e893 Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 18:53:53 -0700 Subject: [PATCH 09/10] fix: reject negative durations in parseDuration time.ParseDuration accepts negative values like "-5s" which are nonsensical for timeouts and TTLs. Now returns a clear error with the config key name. Added tests for parseDuration (empty, unparseable, negative) and parseByteSize (empty, overflow) error paths. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config.go | 5 +++- internal/config_test.go | 63 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/internal/config.go b/internal/config.go index 5ff6633..07c3f9f 100644 --- a/internal/config.go +++ b/internal/config.go @@ -314,13 +314,16 @@ func parseLogLevel(s string) slog.Level { } // parseDuration reads a viper string key and parses it as a time.Duration. -// Returns an error if the value is empty or unparseable. +// Returns an error if the value is empty, unparseable, or negative. func parseDuration(key string) (time.Duration, error) { s := viper.GetString(key) d, err := time.ParseDuration(s) if err != nil { return 0, fmt.Errorf("%s: invalid duration %q: %w", key, s, err) } + if d < 0 { + return 0, fmt.Errorf("%s: negative duration %q", key, s) + } return d, nil } diff --git a/internal/config_test.go b/internal/config_test.go index 8486589..a123521 100644 --- a/internal/config_test.go +++ b/internal/config_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/google/go-tpm/tpm2" + "github.com/spf13/viper" ) func TestValidateEvidence(t *testing.T) { @@ -745,6 +746,68 @@ func TestCheckEndorsementDomain(t *testing.T) { } } +func TestParseDuration(t *testing.T) { + tests := []struct { + name string + value string + wantErr string + }{ + {name: "valid duration", value: "10s"}, + {name: "zero duration", value: "0s"}, + {name: "empty string", value: "", wantErr: "invalid duration"}, + {name: "unparseable", value: "5xs", wantErr: "invalid duration"}, + {name: "negative duration", value: "-5s", wantErr: "negative duration"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key := "test.parse_duration." + tt.name + viper.Set(key, tt.value) + defer viper.Set(key, nil) + + d, err := parseDuration(key) + if tt.wantErr != "" { + if err == nil { + t.Fatal("expected error, got nil") + } + if !contains(err.Error(), tt.wantErr) { + t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErr) + } + if !contains(err.Error(), key) { + t.Fatalf("error %q does not contain key %q", err.Error(), key) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if d < 0 { + t.Fatalf("parseDuration(%q) = %v, want non-negative", tt.value, d) + } + }) + } +} + +func TestParseByteSizeOverflow(t *testing.T) { + _, err := parseByteSize("9EiB") + if err == nil { + t.Fatal("expected error for int64 overflow, got nil") + } + if !contains(err.Error(), "overflows") { + t.Fatalf("error %q does not contain %q", err.Error(), "overflows") + } +} + +func TestParseByteSizeEmpty(t *testing.T) { + _, err := parseByteSize("") + if err == nil { + t.Fatal("expected error for empty input, got nil") + } + if !contains(err.Error(), "empty") { + t.Fatalf("error %q does not contain %q", err.Error(), "empty") + } +} + func searchString(s, substr string) bool { for i := 0; i <= len(s)-len(substr); i++ { if s[i:i+len(substr)] == substr { From 11b3304acf75867818f4bb768846738000ab97df Mon Sep 17 00:00:00 2001 From: Yaroslav Zhavoronkov Date: Wed, 25 Mar 2026 20:18:27 -0700 Subject: [PATCH 10/10] fix: reject zero durations for timeout and interval config values endorsements.client.timeout (http.Client.Timeout=0 disables timeout), revocation.refresh_interval (time.NewTicker(0) panics), and ratelimit.stall_timeout (context.WithTimeout(ctx,0) expires instantly) all require positive durations. http.cache.default_ttl is intentionally left allowing zero (meaning "don't cache"). Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/config.go b/internal/config.go index 07c3f9f..1be672e 100644 --- a/internal/config.go +++ b/internal/config.go @@ -119,6 +119,9 @@ func LoadConfig() (*Config, error) { if err != nil { return nil, err } + if endorsementTimeout == 0 { + return nil, fmt.Errorf("endorsements.client.timeout: must be positive") + } httpCacheSize, err := parseByteSize(viper.GetString("http.cache.size")) if err != nil { return nil, fmt.Errorf("http.cache.size: %w", err) @@ -131,10 +134,16 @@ func LoadConfig() (*Config, error) { if err != nil { return nil, err } + if revocationRefreshInterval == 0 { + return nil, fmt.Errorf("revocation.refresh_interval: must be positive") + } rateLimitStallTimeout, err := parseDuration("ratelimit.stall_timeout") if err != nil { return nil, err } + if rateLimitStallTimeout == 0 { + return nil, fmt.Errorf("ratelimit.stall_timeout: must be positive") + } cosignBuildSigner := CosignBuildSignerConfig{ URI: viper.GetString("endorsements.cosign.build_signer.uri"),