diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e79541b..5d53a3b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,7 +12,11 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go-version: ['1.21', '1.22', '1.23', '1.25.1'] + # Earlier Go versions remain in the matrix as portability smoke + # tests; with go.mod requiring go 1.25.5 they auto-toolchain-upgrade. + # '1.25' resolves to the latest 1.25.x patch so security fixes land + # in CI automatically without manual workflow edits. + go-version: ['1.21', '1.22', '1.23', '1.25'] steps: - name: Checkout code @@ -58,22 +62,30 @@ jobs: - name: Generate coverage HTML report run: go tool cover -html=coverage.out -o coverage.html - if: matrix.go-version == '1.25.1' + if: matrix.go-version == '1.25' - name: Upload coverage artifacts uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: coverage-${{ matrix.go-version }} path: coverage.html - if: matrix.go-version == '1.25.1' + if: matrix.go-version == '1.25' - name: Run benchmarks run: go test -bench=. -benchmem -benchtime=10s ./pkg/cms - name: Run fuzz tests (short) run: | - go test -fuzz=FuzzVerify -fuzztime=10s ./pkg/cms - go test -fuzz=FuzzParseASN1Length -fuzztime=10s ./pkg/cms + # Anchored regexes so e.g. ^FuzzVerify$ doesn't also match + # FuzzVerifyAcceptsOnlyCanonicalForm (go test -fuzz refuses + # when the pattern matches multiple fuzz targets). + go test -fuzz='^FuzzVerify$' -fuzztime=10s ./pkg/cms + go test -fuzz='^FuzzParseASN1Length$' -fuzztime=10s ./pkg/cms + go test -fuzz='^FuzzSignVerifyRoundtrip$' -fuzztime=10s ./pkg/cms + go test -fuzz='^FuzzSignDataWithoutAttributesRoundtrip$' -fuzztime=10s ./pkg/cms + go test -fuzz='^FuzzCertBagSubstitution$' -fuzztime=10s ./pkg/cms + go test -fuzz='^FuzzReplaceOIDBytes$' -fuzztime=10s ./pkg/cms + go test -fuzz='^FuzzVerifyAcceptsOnlyCanonicalForm$' -fuzztime=10s ./pkg/cms security: name: Security Scan @@ -88,12 +100,24 @@ jobs: - name: Set up Go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: - go-version: '1.25.1' + go-version: '1.25' - name: Run gosec run: | go install github.com/securego/gosec/v2/cmd/gosec@v2.22.4 - gosec -exclude=G115 ./... + # G115 (integer overflow on conversion) was previously excluded + # globally as a workaround for ASN.1 DER length-encoding code, but + # a full scan now yields 0 issues — the global exclude is no longer + # needed. If a legitimate G115 site appears later, suppress it + # locally with a `// #nosec G115 -- reason` comment rather than + # re-introducing the global mask. + gosec ./... + + - name: Run govulncheck + run: | + go install golang.org/x/vuln/cmd/govulncheck@latest + # Surface stdlib and dependency CVEs reachable from our call graph. + govulncheck ./... lint: name: Lint @@ -106,7 +130,7 @@ jobs: - name: Set up Go uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: - go-version: '1.25.1' + go-version: '1.25' - name: Run golangci-lint uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8 diff --git a/go.mod b/go.mod index b00700d..265559b 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/agentic-research/go-cms -go 1.25.1 +go 1.25.5 diff --git a/pkg/cms/behavioral_fuzz_test.go b/pkg/cms/behavioral_fuzz_test.go new file mode 100644 index 0000000..0cae41d --- /dev/null +++ b/pkg/cms/behavioral_fuzz_test.go @@ -0,0 +1,233 @@ +package cms + +import ( + "bytes" + "crypto/ed25519" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "testing" + "time" +) + +const fuzzMaxInputSize = 1 << 20 // 1 MiB per input + +// newFuzzSigner builds an ephemeral self-signed Ed25519 certificate and a +// trust pool for the behavioral fuzzers. The cert is built once per fuzz +// function and reused across iterations, isolating the fuzzer to data +// variation only. +func newFuzzSigner(tb testing.TB) (*x509.Certificate, ed25519.PrivateKey, *x509.CertPool) { + tb.Helper() + _, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + tb.Fatalf("ed25519.GenerateKey: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{Organization: []string{"go-cms behavioral fuzz"}}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, priv.Public(), priv) + if err != nil { + tb.Fatalf("x509.CreateCertificate: %v", err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + tb.Fatalf("x509.ParseCertificate: %v", err) + } + pool := x509.NewCertPool() + pool.AddCert(cert) + return cert, priv, pool +} + +// newPool builds a CertPool containing the given certs. Helper for fuzzers +// that need to express different trust topologies. +func newPool(certs ...*x509.Certificate) *x509.CertPool { + pool := x509.NewCertPool() + for _, c := range certs { + pool.AddCert(c) + } + return pool +} + +// FuzzSignVerifyRoundtrip asserts the behavioral contract of the primary +// signing entry point (SignData / Case 1, with signed attributes): +// +// - For any input, signing then verifying must succeed. +// - Verifying with tampered detached data must fail. +// - Tampering any byte of the signature blob must cause verification to fail. +// +// This is the load-bearing behavioral fuzzer: bugs in SignedAttributes +// encoding, digest computation, or signature placement would surface here +// even when unit-test happy paths still pass. +func FuzzSignVerifyRoundtrip(f *testing.F) { + f.Add([]byte(""), uint(0)) + f.Add([]byte("a"), uint(0)) + f.Add([]byte("Hello, CMS!"), uint(7)) + f.Add(bytes.Repeat([]byte{0xff}, 1024), uint(500)) + f.Add([]byte{0x00, 0x01, 0x02, 0x03}, uint(2)) + f.Add([]byte{0x30, 0x82, 0x01, 0x00}, uint(0)) // ASN.1-shaped data + + cert, priv, pool := newFuzzSigner(f) + opts := VerifyOptions{Roots: pool} + + f.Fuzz(func(t *testing.T, data []byte, tamperIdx uint) { + if len(data) > fuzzMaxInputSize { + t.Skip("oversize input") + } + + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData failed for %d-byte input: %v", len(data), err) + } + + if _, err := Verify(sig, data, opts); err != nil { + t.Fatalf("Verify rejected its own roundtrip output (%d-byte input): %v", len(data), err) + } + + // Tampering the detached data must always be rejected (when there's + // data to tamper). + if len(data) > 0 { + td := append([]byte(nil), data...) + td[tamperIdx%uint(len(data))] ^= 0x80 + if !bytes.Equal(td, data) { + if _, err := Verify(sig, td, opts); err == nil { + t.Fatalf("Verify accepted tampered data: original=%x tampered=%x", data, td) + } + } + } + + // Tampering any byte of the signature blob must also be rejected. + ts := append([]byte(nil), sig...) + ts[tamperIdx%uint(len(ts))] ^= 0x80 + if _, err := Verify(ts, data, opts); err == nil { + t.Fatalf("Verify accepted tampered signature blob (flipped byte %d)", tamperIdx%uint(len(ts))) + } + }) +} + +// FuzzSignDataWithoutAttributesRoundtrip exercises the Case 2 path +// (RFC 5652 §5.4 case 2: SignerInfo with no signedAttrs). The signature is +// computed directly over the content rather than over a DER-encoded +// SignedAttributes set — a distinct code path with its own bug surface. +// This fuzzer would have caught the Case 2 verifier bug fixed in PR #9. +func FuzzSignDataWithoutAttributesRoundtrip(f *testing.F) { + f.Add([]byte(""), uint(0)) + f.Add([]byte("a"), uint(0)) + f.Add(bytes.Repeat([]byte{0xff}, 1024), uint(0)) + f.Add([]byte{0x00, 0x01, 0x02, 0x03}, uint(1)) + + cert, priv, pool := newFuzzSigner(f) + opts := VerifyOptions{Roots: pool} + + f.Fuzz(func(t *testing.T, data []byte, tamperIdx uint) { + if len(data) > fuzzMaxInputSize { + t.Skip("oversize input") + } + + sig, err := SignDataWithoutAttributes(data, cert, priv) + if err != nil { + t.Fatalf("SignDataWithoutAttributes failed: %v", err) + } + + if _, err := Verify(sig, data, opts); err != nil { + t.Fatalf("Verify rejected Case 2 roundtrip output: %v", err) + } + + if len(data) > 0 { + td := append([]byte(nil), data...) + td[tamperIdx%uint(len(data))] ^= 0x80 + if !bytes.Equal(td, data) { + if _, err := Verify(sig, td, opts); err == nil { + t.Fatalf("Case 2 Verify accepted tampered data") + } + } + } + + ts := append([]byte(nil), sig...) + ts[tamperIdx%uint(len(ts))] ^= 0x80 + if _, err := Verify(ts, data, opts); err == nil { + t.Fatalf("Case 2 Verify accepted tampered signature blob") + } + }) +} + +// FuzzSignDataWithSignerRoundtrip exercises the crypto.Signer abstraction +// added in PR #6 (SignDataWithSigner). The contract: any bug introduced by +// the abstraction layer that diverges from the direct SignData path would +// surface as either a sign failure, verify failure, or — worst case — +// silent acceptance of mismatched data. ed25519.PrivateKey satisfies +// crypto.Signer natively. +func FuzzSignDataWithSignerRoundtrip(f *testing.F) { + f.Add([]byte(""), uint(0)) + f.Add([]byte("a"), uint(0)) + f.Add(bytes.Repeat([]byte{0xff}, 1024), uint(0)) + + cert, priv, pool := newFuzzSigner(f) + opts := VerifyOptions{Roots: pool} + + f.Fuzz(func(t *testing.T, data []byte, tamperIdx uint) { + if len(data) > fuzzMaxInputSize { + t.Skip("oversize input") + } + + sig, err := SignDataWithSigner(data, cert, priv) + if err != nil { + t.Fatalf("SignDataWithSigner failed: %v", err) + } + + if _, err := Verify(sig, data, opts); err != nil { + t.Fatalf("Verify rejected crypto.Signer roundtrip output: %v", err) + } + + if len(data) > 0 { + td := append([]byte(nil), data...) + td[tamperIdx%uint(len(data))] ^= 0x80 + if !bytes.Equal(td, data) { + if _, err := Verify(sig, td, opts); err == nil { + t.Fatalf("crypto.Signer Verify accepted tampered data") + } + } + } + }) +} + +// FuzzCase2SignDeterminism asserts that for the Case 2 path (no signed +// attributes) the full CMS output is byte-identical across repeated calls +// with the same data + key. Ed25519 is a deterministic signature scheme; +// any non-determinism here would imply RNG leaking into the CMS encoder, +// which is both a malleability concern (multiple distinct valid signatures +// for the same input) and a potential side-channel. +// +// Case 1 (with signed attributes) is intentionally excluded because the +// signing-time attribute changes per call. +func FuzzCase2SignDeterminism(f *testing.F) { + f.Add([]byte("")) + f.Add([]byte("a")) + f.Add([]byte("deterministic ed25519 over CMS")) + f.Add(bytes.Repeat([]byte{0xab}, 256)) + + cert, priv, _ := newFuzzSigner(f) + + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > fuzzMaxInputSize { + t.Skip("oversize input") + } + + sig1, err := SignDataWithoutAttributes(data, cert, priv) + if err != nil { + t.Fatalf("SignDataWithoutAttributes #1 failed: %v", err) + } + sig2, err := SignDataWithoutAttributes(data, cert, priv) + if err != nil { + t.Fatalf("SignDataWithoutAttributes #2 failed: %v", err) + } + if !bytes.Equal(sig1, sig2) { + t.Fatalf("Case 2 non-deterministic for same data+key (len=%d):\n sig1=%x\n sig2=%x", + len(data), sig1, sig2) + } + }) +} diff --git a/pkg/cms/concurrency_test.go b/pkg/cms/concurrency_test.go new file mode 100644 index 0000000..596aa66 --- /dev/null +++ b/pkg/cms/concurrency_test.go @@ -0,0 +1,135 @@ +package cms + +import ( + "bytes" + "sync" + "testing" +) + +// TestVerifyConcurrent asserts that Verify is safe to call concurrently +// against the same CMS blob and trust pool. A bug here — any hidden shared +// state, package-level cache, or mutable type passed by value-of-pointer — +// would surface either as a race (under `go test -race`) or a verification +// inconsistency across goroutines. +// +// Run with: go test -race ./pkg/cms -run TestVerifyConcurrent +func TestVerifyConcurrent(t *testing.T) { + cert, priv, pool := newFuzzSigner(t) + opts := VerifyOptions{Roots: pool} + + data := []byte("concurrent verify invariant") + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + const ( + goroutines = 32 + perGoroutine = 200 + totalAttempts = goroutines * perGoroutine + ) + + var wg sync.WaitGroup + failures := make(chan error, totalAttempts) + + for range goroutines { + wg.Go(func() { + for range perGoroutine { + certs, err := Verify(sig, data, opts) + if err != nil { + failures <- err + return + } + if len(certs) == 0 || !bytes.Equal(certs[0].Raw, cert.Raw) { + failures <- errInconsistentResult + return + } + } + }) + } + wg.Wait() + close(failures) + + for err := range failures { + t.Errorf("concurrent Verify failed: %v", err) + return + } +} + +// TestSignConcurrent asserts that SignData is safe to call concurrently with +// the same key+cert. Ed25519 signing is deterministic, so all outputs of the +// Case 2 path should be byte-identical. Concurrent Case 1 outputs will +// differ in the signing-time attribute but each must independently verify. +func TestSignConcurrent(t *testing.T) { + cert, priv, pool := newFuzzSigner(t) + opts := VerifyOptions{Roots: pool} + + data := []byte("concurrent sign invariant") + + const goroutines = 32 + + t.Run("case2_deterministic", func(t *testing.T) { + baseline, err := SignDataWithoutAttributes(data, cert, priv) + if err != nil { + t.Fatalf("SignDataWithoutAttributes baseline: %v", err) + } + + var wg sync.WaitGroup + mismatches := make(chan int, goroutines) + + for range goroutines { + wg.Go(func() { + sig, err := SignDataWithoutAttributes(data, cert, priv) + if err != nil { + mismatches <- -1 + return + } + if !bytes.Equal(sig, baseline) { + mismatches <- len(sig) + } + }) + } + wg.Wait() + close(mismatches) + + for n := range mismatches { + t.Errorf("concurrent Case 2 sign produced non-deterministic output (len=%d)", n) + } + }) + + t.Run("case1_each_verifies", func(t *testing.T) { + var wg sync.WaitGroup + failures := make(chan error, goroutines) + + for range goroutines { + wg.Go(func() { + sig, err := SignData(data, cert, priv) + if err != nil { + failures <- err + return + } + if _, err := Verify(sig, data, opts); err != nil { + failures <- err + } + }) + } + wg.Wait() + close(failures) + + for err := range failures { + t.Errorf("concurrent Case 1 sign+verify failed: %v", err) + return + } + }) +} + +// errInconsistentResult is a sentinel returned through the failures channel +// when a concurrent Verify returns the wrong cert. Defined as a package-level +// value so the test reporter prints a stable message. +var errInconsistentResult = inconsistentResultErr{} + +type inconsistentResultErr struct{} + +func (inconsistentResultErr) Error() string { + return "Verify returned inconsistent cert across goroutines" +} diff --git a/pkg/cms/der_strictness_test.go b/pkg/cms/der_strictness_test.go new file mode 100644 index 0000000..fac42b3 --- /dev/null +++ b/pkg/cms/der_strictness_test.go @@ -0,0 +1,73 @@ +package cms + +import "testing" + +// TestParseASN1LengthRejectsNonCanonical asserts that parseASN1Length rejects +// non-canonical DER length encodings. DER (RFC 5652 §10.1 mandates DER for +// SignedAttributes) requires: +// +// - Short form (single byte, value < 128) when the length fits. +// - Long form (0x8N followed by N bytes) only for lengths >= 128, using +// the minimum number of bytes (no leading zero bytes). +// +// Without these checks, an attacker can re-encode any length byte in a CMS +// blob from short form to long form, producing structurally distinct but +// cryptographically valid alternate forms of the same message — a +// malleability bypass that breaks content-addressing or duplicate detection +// guarantees built on top of the CMS blob hash. +func TestParseASN1LengthRejectsNonCanonical(t *testing.T) { + // Each case is a length encoding followed by enough payload bytes that + // the bounds check passes — so the only reason for rejection should be + // non-canonical encoding. + cases := []struct { + name string + input []byte + }{ + {"long-form for value 0 (must use short)", []byte{0x81, 0x00}}, + {"long-form for value 5 (must use short)", append([]byte{0x81, 0x05}, make([]byte, 5)...)}, + {"long-form for value 127 (must use short)", append([]byte{0x81, 0x7f}, make([]byte, 127)...)}, + {"long-form for value 128 with leading zero", append([]byte{0x82, 0x00, 0x80}, make([]byte, 128)...)}, + {"long-form for value 256 with leading zero", append([]byte{0x83, 0x00, 0x01, 0x00}, make([]byte, 256)...)}, + {"long-form four-byte with leading zero", append([]byte{0x84, 0x00, 0x00, 0x01, 0x00}, make([]byte, 256)...)}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, _, err := parseASN1Length(tc.input, 0) + if err == nil { + t.Errorf("parseASN1Length accepted non-canonical DER encoding %x; RFC 5652 §10.1 requires DER for SignedAttributes", tc.input) + } + }) + } +} + +// TestParseASN1LengthAcceptsCanonical confirms valid DER length encodings +// are still accepted after the strictness check is added. +func TestParseASN1LengthAcceptsCanonical(t *testing.T) { + cases := []struct { + name string + input []byte + length int + }{ + {"short-form 0", []byte{0x00}, 0}, + {"short-form 5", []byte{0x05, 1, 2, 3, 4, 5}, 5}, + {"short-form 127", append([]byte{0x7f}, make([]byte, 127)...), 127}, + {"long-form 128", append([]byte{0x81, 0x80}, make([]byte, 128)...), 128}, + {"long-form 255", append([]byte{0x81, 0xff}, make([]byte, 255)...), 255}, + {"long-form 256", append([]byte{0x82, 0x01, 0x00}, make([]byte, 256)...), 256}, + {"long-form 65535", append([]byte{0x82, 0xff, 0xff}, make([]byte, 65535)...), 65535}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + length, _, err := parseASN1Length(tc.input, 0) + if err != nil { + t.Errorf("parseASN1Length rejected canonical DER encoding %x: %v", tc.input[:min(len(tc.input), 8)], err) + return + } + if length != tc.length { + t.Errorf("parseASN1Length returned length %d, expected %d", length, tc.length) + } + }) + } +} diff --git a/pkg/cms/tamper_enum_test.go b/pkg/cms/tamper_enum_test.go new file mode 100644 index 0000000..1a511e5 --- /dev/null +++ b/pkg/cms/tamper_enum_test.go @@ -0,0 +1,94 @@ +package cms + +import ( + "sort" + "testing" +) + +// TestEnumerateUnsignedBytesCase1 is an audit-style exhaustive test: for each +// byte of a valid Case 1 (with-signed-attributes) CMS, flip it with 0xff and +// record whether Verify still accepts the result. The set of positions that +// survive is the *exact* attack surface — every byte an adversary can modify +// without invalidating the signature. +// +// The invariant we enforce: every surviving position must fall within a +// documented unsigned region. If the count exceeds the expected envelope, a +// new unsigned bypass has been introduced. +// +// Documented unsigned regions in a strict Case 1 CMS (Ed25519 + SHA-512): +// - SignedData length encoding bytes (BER/DER ambiguity not enforced) +// - ContentInfo length encoding bytes (same) +// - EncContentInfo length encoding bytes (same) +// - bytes inside the embedded x509 certificate that are not load-bearing +// for either the SKI/IssuerAndSerial match or the verifier's chain check +// +// A surge in this number is a regression signal — investigate every new +// position individually. +func TestEnumerateUnsignedBytesCase1(t *testing.T) { + cert, priv, pool := newFuzzSigner(t) + opts := VerifyOptions{Roots: pool} + + data := []byte("byte-by-byte tamper audit") + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + survivors := tamperSurvey(t, sig, data, opts) + + t.Logf("Case 1 CMS length: %d bytes", len(sig)) + t.Logf("Surviving unsigned positions (count=%d): %v", len(survivors), survivors) + + // Empirical envelope: with a freshly issued, RNG-serial cert, the + // unsigned region is dominated by cert-internal bytes (DER length + // encodings, optional fields, ASN.1 padding) that don't feed into either + // the SKI/IssuerAndSerial signer-id lookup or the signature itself. + // Allow up to 8 unsigned positions, alert if exceeded. Tune up only + // after auditing each new survivor. + const allowedCase1 = 8 + if len(survivors) > allowedCase1 { + t.Errorf("unsigned region grew unexpectedly: %d survivors (allowed: %d). Audit each new position before bumping this bound.", len(survivors), allowedCase1) + } +} + +// TestEnumerateUnsignedBytesCase2 does the same for the Case 2 path +// (no signed attributes). Case 2 has a smaller surface because the SignerInfo +// itself is more compact and there are no signed-attributes ordering bytes. +func TestEnumerateUnsignedBytesCase2(t *testing.T) { + cert, priv, pool := newFuzzSigner(t) + opts := VerifyOptions{Roots: pool} + + data := []byte("byte-by-byte tamper audit case 2") + sig, err := SignDataWithoutAttributes(data, cert, priv) + if err != nil { + t.Fatalf("SignDataWithoutAttributes: %v", err) + } + + survivors := tamperSurvey(t, sig, data, opts) + + t.Logf("Case 2 CMS length: %d bytes", len(sig)) + t.Logf("Surviving unsigned positions (count=%d): %v", len(survivors), survivors) + + const allowedCase2 = 8 + if len(survivors) > allowedCase2 { + t.Errorf("Case 2 unsigned region grew unexpectedly: %d survivors (allowed: %d).", len(survivors), allowedCase2) + } +} + +// tamperSurvey flips each byte of sig in turn (XOR 0xff) and returns the +// sorted list of positions where Verify still accepted the result. A robust +// signed structure should yield very few such positions — only those bytes +// genuinely outside the signed region. +func tamperSurvey(t *testing.T, sig, data []byte, opts VerifyOptions) []int { + t.Helper() + var survivors []int + for i := range sig { + tampered := append([]byte(nil), sig...) + tampered[i] ^= 0xff + if _, err := Verify(tampered, data, opts); err == nil { + survivors = append(survivors, i) + } + } + sort.Ints(survivors) + return survivors +} diff --git a/pkg/cms/testdata/fuzz/FuzzSignDataWithoutAttributesRoundtrip/48d0f8f6cbb39a02 b/pkg/cms/testdata/fuzz/FuzzSignDataWithoutAttributesRoundtrip/48d0f8f6cbb39a02 new file mode 100644 index 0000000..349fbb9 --- /dev/null +++ b/pkg/cms/testdata/fuzz/FuzzSignDataWithoutAttributesRoundtrip/48d0f8f6cbb39a02 @@ -0,0 +1,3 @@ +go test fuzz v1 +[]byte("") +uint(50) diff --git a/pkg/cms/testdata/fuzz/FuzzSignVerifyRoundtrip/bbfdccec14dad20a b/pkg/cms/testdata/fuzz/FuzzSignVerifyRoundtrip/bbfdccec14dad20a new file mode 100644 index 0000000..50470f1 --- /dev/null +++ b/pkg/cms/testdata/fuzz/FuzzSignVerifyRoundtrip/bbfdccec14dad20a @@ -0,0 +1,3 @@ +go test fuzz v1 +[]byte("0") +uint(25) diff --git a/pkg/cms/tier2_fuzz_test.go b/pkg/cms/tier2_fuzz_test.go new file mode 100644 index 0000000..1fb8f40 --- /dev/null +++ b/pkg/cms/tier2_fuzz_test.go @@ -0,0 +1,197 @@ +package cms + +import ( + "bytes" + "testing" +) + +// FuzzInsertByte asserts that inserting a single arbitrary byte at any +// position of a valid CMS structure breaks verification. Insertion shifts +// every subsequent byte by one, which should invalidate the ASN.1 length +// chain top-to-bottom; any insertion the verifier still accepts is either a +// length-encoding bypass or a code path that doesn't actually consult the +// inserted region. +func FuzzInsertByte(f *testing.F) { + f.Add([]byte("seed"), uint(0), byte(0x00)) + f.Add([]byte("seed"), uint(10), byte(0xff)) + + cert, priv, pool := newFuzzSigner(f) + opts := VerifyOptions{Roots: pool} + + f.Fuzz(func(t *testing.T, data []byte, insertIdx uint, insertVal byte) { + if len(data) > fuzzMaxInputSize { + t.Skip("oversize input") + } + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + idx := int(insertIdx % uint(len(sig)+1)) + tampered := make([]byte, 0, len(sig)+1) + tampered = append(tampered, sig[:idx]...) + tampered = append(tampered, insertVal) + tampered = append(tampered, sig[idx:]...) + + if _, err := Verify(tampered, data, opts); err == nil { + t.Fatalf("Verify accepted CMS with extra byte inserted at offset %d (val=0x%02x)", idx, insertVal) + } + }) +} + +// FuzzDeleteByte asserts that deleting any single byte breaks verification. +// Deletion shifts the trailing bytes and shortens the structure — every +// length field on the outer chain should fail to match. +func FuzzDeleteByte(f *testing.F) { + f.Add([]byte("seed"), uint(0)) + f.Add([]byte("seed"), uint(50)) + + cert, priv, pool := newFuzzSigner(f) + opts := VerifyOptions{Roots: pool} + + f.Fuzz(func(t *testing.T, data []byte, deleteIdx uint) { + if len(data) > fuzzMaxInputSize { + t.Skip("oversize input") + } + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + idx := int(deleteIdx % uint(len(sig))) + tampered := make([]byte, 0, len(sig)-1) + tampered = append(tampered, sig[:idx]...) + tampered = append(tampered, sig[idx+1:]...) + + if _, err := Verify(tampered, data, opts); err == nil { + t.Fatalf("Verify accepted CMS with byte deleted at offset %d", idx) + } + }) +} + +// FuzzAppendTrailingData asserts that any non-empty data appended after a +// valid CMS structure must be rejected. The outer ContentInfo parse should +// detect trailing data; if it doesn't, that's a parser laxity bypass. +func FuzzAppendTrailingData(f *testing.F) { + f.Add([]byte("seed"), []byte{0x00}) + f.Add([]byte("seed"), []byte{0xff, 0xff, 0xff}) + + cert, priv, pool := newFuzzSigner(f) + opts := VerifyOptions{Roots: pool} + + f.Fuzz(func(t *testing.T, data []byte, trailer []byte) { + if len(data) > fuzzMaxInputSize || len(trailer) > 4096 { + t.Skip("oversize input") + } + if len(trailer) == 0 { + t.Skip("empty trailer is no-op") + } + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData: %v", err) + } + tampered := append([]byte(nil), sig...) + tampered = append(tampered, trailer...) + + if _, err := Verify(tampered, data, opts); err == nil { + t.Fatalf("Verify accepted CMS with %d trailing bytes appended", len(trailer)) + } + }) +} + +// FuzzCertBagSubstitution generates two unrelated signing pairs (A, B), signs +// data with A, then replaces every occurrence of A's cert DER inside the CMS +// blob with B's cert DER (sizes match because both certs are produced with +// the same template). Verification must reject — the signature was produced +// by A's key, so B's public key cannot verify it. +// +// This is the canonical "wrong cert in cert bag" attack. A bug here would be +// catastrophic: a valid signature from any trusted key would attest to any +// chosen cert/identity. +func FuzzCertBagSubstitution(f *testing.F) { + f.Add([]byte("")) + f.Add([]byte("a")) + f.Add([]byte("substitution attack data")) + + certA, keyA, _ := newFuzzSigner(f) + certB, _, _ := newFuzzSigner(f) + + // Build a trust pool containing only A — substitution must still fail + // whether or not B is trusted. + poolA := newPool(certA) + poolAB := newPool(certA, certB) + + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > fuzzMaxInputSize { + t.Skip("oversize input") + } + sig, err := SignData(data, certA, keyA) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + // Substitute A's cert with B's cert in the CMS blob (same length + // because both came from CreateCertificate with the same template; + // if sizes happen to differ, skip this iteration — we cannot do a + // naive in-place swap without recomputing all ASN.1 lengths). + if len(certA.Raw) != len(certB.Raw) { + t.Skip("cert size mismatch — naive substitution requires equal-length DER") + } + + idx := bytes.Index(sig, certA.Raw) + if idx < 0 { + t.Skip("could not locate cert A's DER inside CMS (unusual encoding)") + } + substituted := append([]byte(nil), sig...) + copy(substituted[idx:idx+len(certB.Raw)], certB.Raw) + + // With either trust pool, substitution must fail. + for _, opts := range []VerifyOptions{{Roots: poolA}, {Roots: poolAB}} { + if _, err := Verify(substituted, data, opts); err == nil { + t.Fatalf("Verify accepted CMS after cert substitution A→B (poolHasB=%v)", opts.Roots == poolAB) + } + } + }) +} + +// FuzzVerifyAcceptsOnlyCanonicalForm asserts that, holding the data and +// trust root constant, the verifier yields *only* accept-or-reject outcomes +// — never panics, never crashes — across heavily mutated inputs derived +// from a valid signature. This is a stronger statement than FuzzVerify's +// "doesn't panic on arbitrary input": we start from a valid CMS and confirm +// that random mutations either get rejected cleanly or accepted with the +// correct cert bound to the result. +func FuzzVerifyAcceptsOnlyCanonicalForm(f *testing.F) { + f.Add([]byte("hello"), uint(0), uint(50), byte(0xa5)) + + cert, priv, pool := newFuzzSigner(f) + opts := VerifyOptions{Roots: pool} + data := []byte("canonical-form invariant data") + good, err := SignData(data, cert, priv) + if err != nil { + f.Fatalf("setup: SignData: %v", err) + } + + f.Fuzz(func(t *testing.T, _ []byte, idx1, idx2 uint, val byte) { + // Two-byte tamper at fuzz-controlled positions. The invariant: if + // Verify returns no error, the returned cert chain MUST start with + // the original signer cert. Otherwise we have accepted a forgery. + tampered := append([]byte(nil), good...) + p1 := int(idx1 % uint(len(tampered))) + p2 := int(idx2 % uint(len(tampered))) + tampered[p1] ^= val + tampered[p2] ^= val + + certs, err := Verify(tampered, data, opts) + if err != nil { + return // rejection is acceptable + } + // Accepted — must be byte-identical to the cert we signed with. + if len(certs) == 0 { + t.Fatalf("Verify accepted but returned no certs (positions %d,%d)", p1, p2) + } + if !bytes.Equal(certs[0].Raw, cert.Raw) { + t.Fatalf("Verify accepted with substituted cert (positions %d,%d)", p1, p2) + } + }) +} diff --git a/pkg/cms/tier3_fuzz_test.go b/pkg/cms/tier3_fuzz_test.go new file mode 100644 index 0000000..683e785 --- /dev/null +++ b/pkg/cms/tier3_fuzz_test.go @@ -0,0 +1,251 @@ +package cms + +import ( + "bytes" + "crypto/ed25519" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "testing" + "time" +) + +// FuzzReplaceOIDBytes locates the Ed25519 signature-algorithm OID inside a +// valid CMS blob and replaces it with arbitrary bytes of the same length. +// Verification must reject — the verifier dispatches by algorithm OID, and +// accepting any substitution means an attacker can claim a signature was +// produced by a different algorithm than it actually was. +// +// The Ed25519 OID encoding (1.3.101.112) is "06 03 2b 65 70" — 5 bytes +// including tag and length. We look for this sequence and overwrite the +// three OID-value bytes (2b 65 70) with fuzzer-supplied bytes. +func FuzzReplaceOIDBytes(f *testing.F) { + f.Add([]byte("data"), []byte{0x00, 0x00, 0x00}) + f.Add([]byte("data"), []byte{0x2a, 0x86, 0x48}) // start of RSA OID + f.Add([]byte("data"), []byte{0xff, 0xff, 0xff}) + + cert, priv, pool := newFuzzSigner(f) + opts := VerifyOptions{Roots: pool} + + // Marker for the Ed25519 OID as it appears inside SignerInfo's + // SignatureAlgorithm AlgorithmIdentifier (and elsewhere). + ed25519OID := []byte{0x06, 0x03, 0x2b, 0x65, 0x70} + + f.Fuzz(func(t *testing.T, data []byte, replacement []byte) { + if len(data) > fuzzMaxInputSize { + t.Skip("oversize input") + } + if len(replacement) != 3 { + t.Skip("replacement must be exactly 3 bytes (OID value length)") + } + // Don't replace with the same bytes — that's a no-op. + if bytes.Equal(replacement, ed25519OID[2:]) { + t.Skip("no-op replacement") + } + + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + // Replace every occurrence of the Ed25519 OID value bytes with the + // fuzz-supplied bytes. Multiple occurrences exist (DigestAlgorithms, + // SignerInfo.SignatureAlgorithm, possibly inside the cert itself). + tampered := append([]byte(nil), sig...) + found := false + for i := 0; i+len(ed25519OID) <= len(tampered); i++ { + if bytes.Equal(tampered[i:i+len(ed25519OID)], ed25519OID) { + copy(tampered[i+2:i+5], replacement) + found = true + } + } + if !found { + t.Skip("ed25519 OID marker not found") + } + if bytes.Equal(tampered, sig) { + t.Skip("no bytes actually changed") + } + + if _, err := Verify(tampered, data, opts); err == nil { + t.Fatalf("Verify accepted CMS with Ed25519 OID replaced by %x", replacement) + } + }) +} + +// FuzzDeclaredLengthOverflow asserts the parser refuses to allocate or read +// based on attacker-declared lengths that exceed the actual blob size. This +// is a DoS / OOM defense: a small CMS that *declares* a 4-GB SignedAttrs +// SET must not cause the verifier to allocate that much memory or stall +// the goroutine indefinitely. +// +// The fuzzer constructs short blobs starting with a real outer header but +// with an inflated inner length, and asserts Verify rejects in bounded +// time without OOM. +func FuzzDeclaredLengthOverflow(f *testing.F) { + // Each input has shape: + f.Add(uint32(1 << 24)) // 16 MB declared + f.Add(uint32(1 << 30)) // 1 GB declared + f.Add(uint32(0xffffffff)) + + f.Fuzz(func(t *testing.T, declaredLen uint32) { + // Build: SEQUENCE [long-form 4-byte length = declaredLen] + buf := []byte{ + 0x30, 0x84, // outer SEQUENCE, long-form 4 bytes + byte(declaredLen >> 24), + byte(declaredLen >> 16), + byte(declaredLen >> 8), + byte(declaredLen), + } + + // Must reject without OOM or panic. The bounded `parseASN1Length` + // check (length <= remaining) makes this immediate, but a regression + // that lifts that bound would surface here. + _, _ = Verify(buf, nil, VerifyOptions{}) + // No assertion: we only care that this function returned at all. + // If we hit a hang, the Go test harness will eventually time it out. + }) +} + +// TestSignatureRegionSurgery replaces just the Ed25519 signature bytes of a +// valid CMS with all-zero, all-0xff, and random bytes. Verification must +// reject every variant. Because Ed25519 signatures are 64 bytes and have no +// internal structure visible to the parser, replacing the sig is a tighter +// test than tampering arbitrary bytes — it isolates the cryptographic +// verification step from the structural/encoding checks. +func TestSignatureRegionSurgery(t *testing.T) { + cert, priv, pool := newFuzzSigner(t) + opts := VerifyOptions{Roots: pool} + + data := []byte("signature region surgery target") + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + // Locate the 64-byte signature region. It is OCTET STRING-encoded + // (tag 0x04) at the tail of the SignerInfo. Match by scanning for + // "04 40" (OCTET STRING, length 64) and taking the last occurrence + // (earlier "04 40" patterns can appear inside the cert). + const ( + sigTag = 0x04 + sigLen = 64 + header = 2 // tag + length byte + ) + idx := -1 + for i := 0; i+header+sigLen <= len(sig); i++ { + if sig[i] == sigTag && sig[i+1] == sigLen { + idx = i + } + } + if idx < 0 { + t.Fatalf("could not locate Ed25519 signature region (04 40)") + } + t.Logf("signature region at offset %d..%d", idx+header, idx+header+sigLen) + + cases := map[string][]byte{ + "all-zero": bytes.Repeat([]byte{0x00}, sigLen), + "all-0xff": bytes.Repeat([]byte{0xff}, sigLen), + "flip-last-byte": func() []byte { // original sig with last byte XOR'd + out := make([]byte, sigLen) + copy(out, sig[idx+header:idx+header+sigLen]) + out[sigLen-1] ^= 0xff + return out + }(), + "flip-first-byte": func() []byte { + out := make([]byte, sigLen) + copy(out, sig[idx+header:idx+header+sigLen]) + out[0] ^= 0xff + return out + }(), + } + + // Add a random replacement. + rb := make([]byte, sigLen) + if _, err := rand.Read(rb); err == nil { + cases["random"] = rb + } + + for name, replacement := range cases { + t.Run(name, func(t *testing.T) { + tampered := append([]byte(nil), sig...) + copy(tampered[idx+header:idx+header+sigLen], replacement) + if _, err := Verify(tampered, data, opts); err == nil { + t.Errorf("Verify accepted CMS with %s signature replacement", name) + } + }) + } +} + +// TestVerifyEmptyCertBag asserts the verifier rejects a CMS message that +// declares no signer certificate. Without a cert, the verifier cannot match +// the SID to a public key and cannot perform chain validation. Accepting +// silently here would be a forgery surface. +func TestVerifyEmptyCertBag(t *testing.T) { + cert, priv, pool := newFuzzSigner(t) + opts := VerifyOptions{Roots: pool} + + data := []byte("empty cert bag test") + sig, err := SignData(data, cert, priv) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + // The certs[0] tag is "A0 ". To strip it, find the + // tag-length-prefix that encodes exactly the embedded certificate's + // raw DER, then remove that span and adjust enclosing lengths. + // Doing this surgery cleanly is non-trivial; instead we exercise the + // equivalent code path: Verify against a CMS blob with the cert bag + // zero-filled. The parser will fail to recover the signer cert. + tampered := append([]byte(nil), sig...) + certIdx := bytes.Index(tampered, cert.Raw) + if certIdx < 0 { + t.Skip("cert DER not located in CMS blob") + } + for i := certIdx; i < certIdx+len(cert.Raw); i++ { + tampered[i] = 0 + } + + if _, err := Verify(tampered, data, opts); err == nil { + t.Fatal("Verify accepted CMS with zero-filled cert bag") + } +} + +// TestVerifyWithExtraTrustedCert confirms adding an unrelated trusted cert +// to the cert bag doesn't allow the unrelated cert to attest the signature. +// The verifier must use the cert whose key actually produced the signature, +// not any old trusted cert that happens to be present. +func TestVerifyWithExtraTrustedCert(t *testing.T) { + // Signer A: actually signs the data. + certA, keyA, _ := newFuzzSigner(t) + + // Signer B: independent, trusted, but did NOT sign anything. + _, _, _ = newFuzzSigner(t) // B's key is unused after generation + tmplB := &x509.Certificate{ + SerialNumber: big.NewInt(2), + Subject: pkix.Name{Organization: []string{"unrelated trusted"}}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + } + _, keyB, _ := ed25519.GenerateKey(rand.Reader) + derB, _ := x509.CreateCertificate(rand.Reader, tmplB, tmplB, keyB.Public(), keyB) + certB, _ := x509.ParseCertificate(derB) + + // Trust pool contains both — but only A signed. + pool := newPool(certA, certB) + + data := []byte("extra-cert-in-pool invariant") + sig, err := SignData(data, certA, keyA) + if err != nil { + t.Fatalf("SignData: %v", err) + } + + verified, err := Verify(sig, data, VerifyOptions{Roots: pool}) + if err != nil { + t.Fatalf("Verify rejected legitimate signature when extra trusted cert present: %v", err) + } + if len(verified) == 0 || !bytes.Equal(verified[0].Raw, certA.Raw) { + t.Fatal("Verify did not return signer-A's cert when an extra trusted cert was in the pool") + } +} diff --git a/pkg/cms/verifier.go b/pkg/cms/verifier.go index cac53b0..fb07b44 100644 --- a/pkg/cms/verifier.go +++ b/pkg/cms/verifier.go @@ -49,12 +49,8 @@ import ( // ASN.1 tag constants for better readability const ( - tagSequence = 0x30 // SEQUENCE tag tagSet = 0x31 // SET tag tagContextSpecific0 = 0xA0 // CONTEXT SPECIFIC [0] tag - tagOctetString = 0x04 // OCTET STRING tag - tagInteger = 0x02 // INTEGER tag - tagBitString = 0x03 // BIT STRING tag tagSetTag = 17 // SET tag value (for compound check) asn1ClassContext = 2 // Context-specific class ) @@ -168,12 +164,18 @@ func parseSignedData(ci *contentInfo) (*signedData, error) { return nil, NewValidationError("SignedData", "", "trailing data", nil) } - // Do not enforce a specific SignedData.Version here; values vary with features per RFC 5652. - // Version can vary based on CMS features: - // - Version 1: Basic SignedData - // - Version 3: Contains SignerInfo with subjectKeyIdentifier - // - Version 4: Contains attribute certificates (v2) - // - Version 5: Contains SignerInfo with subjectKeyIdentifier AND attribute certificates + // RFC 5652 §5.1: SignedData.Version must be one of {1, 3, 4, 5} depending + // on which features are present. Any other value (including negatives and + // large positives produced by single-byte tampering of the INTEGER + // payload) must be rejected. + switch sd.Version { + case 1, 3, 4, 5: + // permitted by spec + default: + return nil, NewValidationError("SignedData.Version", + fmt.Sprintf("%d", sd.Version), + "invalid SignedData version (RFC 5652 §5.1: expected 1, 3, 4, or 5)", nil) + } return &sd, nil } @@ -470,7 +472,17 @@ func verifyCertificateChain(signerCert *x509.Certificate, allCerts []*x509.Certi // verifyMessageDigest verifies the message digest in signed attributes if present func verifyMessageDigest(si *signerInfo, detachedData []byte, expectedContentType asn1.ObjectIdentifier) error { if len(si.SignedAttrs.FullBytes) == 0 { - return nil // No signed attributes, nothing to verify + // RFC 5652 §11.1: a signedAttributes contentType attribute MUST be + // present unless the eContentType is id-data. Equivalently, when + // signedAttributes are absent (Case 2), the encapsulated content type + // MUST be id-data. Without this check, an attacker can flip bytes in + // the EncapContentInfo OID without invalidating the signature. + if !expectedContentType.Equal(oidData) { + return NewValidationError("EncapContentInfo.ContentType", + expectedContentType.String(), + "must be id-data when signedAttributes are absent (RFC 5652 §11.1)", nil) + } + return nil } // Parse signed attributes @@ -717,9 +729,21 @@ func Verify(cmsSignature, detachedData []byte, opts VerifyOptions) ([]*x509.Cert // Helper Functions -// parseASN1Length parses ASN.1 DER/BER length encoding from data starting at offset -// Returns the length value and new position after length bytes, or error if invalid -// This function properly validates bounds to prevent panics from malformed input +// parseASN1Length parses an ASN.1 DER length encoding from data starting at +// offset. Returns the length value and the new position after the length +// bytes, or an error if the encoding is invalid or non-canonical. +// +// RFC 5652 §10.1 mandates DER encoding for CMS SignedAttributes, and DER +// requires *canonical* length encoding: +// - Lengths < 128 MUST use the short form (single byte). +// - Lengths >= 128 MUST use the long form with the minimum number of bytes +// (no leading zero bytes in the long-form length value). +// +// Accepting non-canonical forms creates a malleability surface: a single +// CMS message could be re-encoded into structurally distinct but +// cryptographically valid alternates, breaking content-addressing and +// duplicate-detection guarantees built atop the CMS blob hash. We therefore +// enforce DER canonicality at this internal helper boundary. func parseASN1Length(data []byte, offset int) (length int, newPos int, err error) { if offset >= len(data) { return 0, 0, fmt.Errorf("offset %d exceeds data length %d", offset, len(data)) @@ -728,18 +752,21 @@ func parseASN1Length(data []byte, offset int) (length int, newPos int, err error pos := offset firstByte := data[pos] - if firstByte < 0x80 { - // Short form: length is in single byte (0-127) + switch { + case firstByte < 0x80: + // Short form: length is in single byte (0-127). Canonical. length = int(firstByte) newPos = pos + 1 - } else if firstByte == 0x80 { - // Indefinite length - not supported in DER - return 0, 0, fmt.Errorf("indefinite length encoding not supported") - } else { - // Long form: firstByte & 0x7f tells us number of length bytes + + case firstByte == 0x80: + // Indefinite length — BER only, never DER. + return 0, 0, fmt.Errorf("indefinite length encoding not supported (DER required)") + + default: + // Long form: firstByte & 0x7f tells us number of length bytes. numBytes := int(firstByte & 0x7f) if numBytes > 4 { - // We don't support lengths requiring more than 4 bytes (>4GB) + // We don't support lengths requiring more than 4 bytes (>4GB). return 0, 0, fmt.Errorf("length encoding with %d bytes not supported", numBytes) } @@ -749,24 +776,34 @@ func parseASN1Length(data []byte, offset int) (length int, newPos int, err error numBytes, pos+numBytes, len(data)) } - // Parse the length value with overflow protection + // DER canonicality: leading byte of long-form length MUST NOT be 0x00. + // (A leading zero means a shorter long-form encoding would suffice.) + if numBytes > 1 && data[pos] == 0x00 { + return 0, 0, fmt.Errorf("non-canonical DER: long-form length has leading zero byte") + } + + // Parse the length value with overflow protection. length = 0 for i := 0; i < numBytes; i++ { - // Check for integer overflow on 32-bit systems - // On 32-bit systems, int max is 2^31-1 (2147483647) prevLength := length length = (length << 8) | int(data[pos+i]) - // Detect overflow: if length wrapped around to negative or decreased + // Detect overflow: if length wrapped around to negative or decreased. if length < 0 || (prevLength > 0 && length < prevLength) { return 0, 0, fmt.Errorf("integer overflow in length encoding") } } newPos = pos + numBytes + + // DER canonicality: long form must only be used when short form + // would not suffice (i.e., length >= 128). + if length < 0x80 { + return 0, 0, fmt.Errorf("non-canonical DER: long-form length %d must use short form", length) + } } - // Critical: Validate length doesn't exceed remaining data - if newPos+length > len(data) || newPos+length < 0 { // Check for overflow in addition + // Critical: validate length doesn't exceed remaining data. + if newPos+length > len(data) || newPos+length < 0 { // overflow in addition return 0, 0, fmt.Errorf("length %d exceeds remaining data %d", length, len(data)-newPos) }