diff --git a/go.mod b/go.mod index 8aa4df4f3..5758650c0 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module chainguard.dev/apko go 1.26.0 require ( - chainguard.dev/sdk v0.1.184 + chainguard.dev/sdk v0.1.191 github.com/chainguard-dev/clog v1.8.1 github.com/charmbracelet/log v1.0.0 github.com/go-git/go-git/v5 v5.19.2 @@ -17,6 +17,7 @@ require ( github.com/klauspost/pgzip v1.2.6 github.com/package-url/packageurl-go v0.1.6 github.com/pavlo-v-chernykh/keystore-go/v4 v4.5.0 + github.com/pjbgf/sha1cd v0.6.0 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 @@ -104,7 +105,6 @@ require ( github.com/opencontainers/image-spec v1.1.1 // indirect github.com/pb33f/ordered-map/v2 v2.3.1 // indirect github.com/pierrec/lz4/v4 v4.1.27 // indirect - github.com/pjbgf/sha1cd v0.6.0 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.24.1 // indirect diff --git a/go.sum b/go.sum index 45e7e66d0..d5a104c8c 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ chainguard.dev/go-grpc-kit v0.18.0 h1:kr451ml4eBgNcc0ytqcSSPZ3KGtyfHRrrBcYxW/oZwI= chainguard.dev/go-grpc-kit v0.18.0/go.mod h1:N1ZZiV3KkNHFlIS6RPhLT9ppfObC7aa1hqWMPmNiTA0= -chainguard.dev/sdk v0.1.184 h1:L9SLup6giGc+qGUDZ7iuMKL5wC0rg6US23L2k5tbg5k= -chainguard.dev/sdk v0.1.184/go.mod h1:EdDABW102LAdDIbUXoTokZlRlyTa6D2ml0tC9Agnpbs= +chainguard.dev/sdk v0.1.191 h1:9HEpIHedW8CAzZyTpLP8hdeVv1XLVGKw3C+SuCE6Ueg= +chainguard.dev/sdk v0.1.191/go.mod h1:EdDABW102LAdDIbUXoTokZlRlyTa6D2ml0tC9Agnpbs= cloud.google.com/go/auth v0.22.0 h1:Xp9wAKkLoeaYb5pYZZoQGz4E9sdPxIbzS3gywZE3ciQ= cloud.google.com/go/auth v0.22.0/go.mod h1:M9o2Oz+YI2jAfxewJgb1vyI3vceHF+eohmxyzmrl+9s= cloud.google.com/go/auth/oauth2adapt v0.2.8 h1:keo8NaayQZ6wimpNSmW5OPc283g65QNIiLpZnkHRbnc= diff --git a/internal/sha1cd/sha1cd.go b/internal/sha1cd/sha1cd.go new file mode 100644 index 000000000..d40eaff3d --- /dev/null +++ b/internal/sha1cd/sha1cd.go @@ -0,0 +1,83 @@ +// Copyright 2026 Chainguard, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package sha1cd computes SHA-1 digests with collision detection. +// +// apk identifies control sections, signatures and individual files by SHA-1, +// and signs legacy indexes over SHA-1, so we cannot stop computing it. What we +// can do is refuse to trust a digest computed over input that carries the +// cryptanalytic signature of a SHA-1 collision attack. Callers finalise a hash +// with [Sum], which runs that check and reports [ErrCollision] instead of +// returning a digest that must not be used. +// +// For input with no collision signature the digests are identical to +// crypto/sha1, so this is a drop-in replacement. For input that does collide +// the digest differs (the underlying implementation rehashes to a safe value), +// which is a second reason never to use a digest without checking first. +package sha1cd + +import ( + "errors" + "hash" + + upstream "github.com/pjbgf/sha1cd" +) + +// Size is the length in bytes of a SHA-1 digest. +const Size = upstream.Size + +// ErrCollision reports that the hashed input exhibits the characteristics of a +// SHA-1 collision attack, so its digest cannot be trusted to identify content. +var ErrCollision = errors.New("sha1 collision attack detected in hashed input") + +// New returns a [hash.Hash] computing SHA-1 with collision detection. +// +// Finalise it with [Sum] rather than its own Sum method: collision detection is +// only conclusive once the digest is finalised, and Sum is what checks it. +func New() hash.Hash { + return upstream.New() +} + +// Sum finalises h and returns its digest. +// +// When h came from [New], the digest is checked before it is returned and +// ErrCollision is reported instead if the input collides. +// +// A hash that cannot detect collisions, such as sha256, is simply finalised. +// That passthrough is for callers holding a hash.Hash whose algorithm is only +// known at runtime, so that whichever hash they end up with is checked if it is +// SHA-1. Where the algorithm is known at the call site, call [SumBytes] or that +// digest's own package instead, so the code does not read as though SHA-1 were +// involved when it is not. +func Sum(h hash.Hash) ([]byte, error) { + crh, ok := h.(upstream.CollisionResistantHash) + if !ok { + return h.Sum(nil), nil + } + sum, collision := crh.CollisionResistantSum(nil) + if collision { + return nil, ErrCollision + } + return sum, nil +} + +// SumBytes returns the SHA-1 of data, or ErrCollision if data exhibits the +// characteristics of a SHA-1 collision attack. +func SumBytes(data []byte) ([]byte, error) { + sum, collision := upstream.Sum(data) + if collision { + return nil, ErrCollision + } + return sum[:], nil +} diff --git a/internal/sha1cd/sha1cd_test.go b/internal/sha1cd/sha1cd_test.go new file mode 100644 index 000000000..afd5d9f38 --- /dev/null +++ b/internal/sha1cd/sha1cd_test.go @@ -0,0 +1,115 @@ +// Copyright 2026 Chainguard, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sha1cd + +import ( + "crypto/sha256" + "encoding/hex" + "errors" + "testing" + + "chainguard.dev/apko/internal/sha1cd/sha1cdtest" +) + +// Digests of input with no collision signature must match crypto/sha1, so that +// swapping the implementation does not change any checksum apk cares about. +var golden = map[string]string{ + "": "da39a3ee5e6b4b0d3255bfef95601890afd80709", + "abc": "a9993e364706816aba3e25717850c26c9cd0d89d", + "hello world": "2aae6c35c94fcfb415dbe95f408b9ce91ee846ed", +} + +func TestSumBytesGolden(t *testing.T) { + for in, want := range golden { + sum, err := SumBytes([]byte(in)) + if err != nil { + t.Errorf("SumBytes(%q): %v", in, err) + continue + } + if got := hex.EncodeToString(sum); got != want { + t.Errorf("SumBytes(%q) = %s, want %s", in, got, want) + } + } +} + +func TestSumGolden(t *testing.T) { + for in, want := range golden { + h := New() + if _, err := h.Write([]byte(in)); err != nil { + t.Errorf("Write(%q): %v", in, err) + continue + } + sum, err := Sum(h) + if err != nil { + t.Errorf("Sum(%q): %v", in, err) + continue + } + if got := hex.EncodeToString(sum); got != want { + t.Errorf("Sum(%q) = %s, want %s", in, got, want) + } + if len(sum) != Size { + t.Errorf("Sum(%q) is %d bytes, want %d", in, len(sum), Size) + } + } +} + +func TestSumBytesCollision(t *testing.T) { + sum, err := SumBytes(sha1cdtest.Shattered(t)) + if !errors.Is(err, ErrCollision) { + t.Errorf("SumBytes(shattered) = %x, %v; want ErrCollision", sum, err) + } + if sum != nil { + t.Errorf("SumBytes(shattered) returned a digest %x alongside the error", sum) + } +} + +// The collision is only detectable once the digest is finalised, so check that +// it is caught however the input was fed in. +func TestSumCollision(t *testing.T) { + data := sha1cdtest.Shattered(t) + + for _, chunk := range []int{1, 7, 64, 320} { + h := New() + for i := 0; i < len(data); i += chunk { + if _, err := h.Write(data[i:min(i+chunk, len(data))]); err != nil { + t.Fatalf("chunk %d: write: %v", chunk, err) + } + } + sum, err := Sum(h) + if !errors.Is(err, ErrCollision) { + t.Errorf("chunk %d: Sum = %x, %v; want ErrCollision", chunk, sum, err) + } + if sum != nil { + t.Errorf("chunk %d: Sum returned a digest %x alongside the error", chunk, sum) + } + } +} + +// Sum also finalises hashes that cannot detect collisions, so callers choosing a +// digest algorithm at runtime have a single finalisation path. +func TestSumPassesThroughOtherHashes(t *testing.T) { + h := sha256.New() + if _, err := h.Write([]byte("hello world")); err != nil { + t.Fatal(err) + } + sum, err := Sum(h) + if err != nil { + t.Fatalf("Sum(sha256): %v", err) + } + const want = "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" + if got := hex.EncodeToString(sum); got != want { + t.Errorf("Sum(sha256) = %s, want %s", got, want) + } +} diff --git a/internal/sha1cd/sha1cdtest/shattered.go b/internal/sha1cd/sha1cdtest/shattered.go new file mode 100644 index 000000000..cc2de9766 --- /dev/null +++ b/internal/sha1cd/sha1cdtest/shattered.go @@ -0,0 +1,60 @@ +// Copyright 2026 Chainguard, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package sha1cdtest provides a real SHA-1 collision, shared by the tests that +// check collision detection actually fires. +// +// This cannot be a shattered_test.go: Go refuses to import a package built only +// from _test.go files ("no non-test Go files"), so a shared fixture has to live +// in an ordinary file. Three things keep it out of production code anyway. It is +// under internal/, so nothing outside apko can reach it. [Shattered] takes a +// [testing.TB], which only a test can supply. And no non-test file imports this +// package, so it is never linked into any apko binary — `go list -deps` over the +// commands does not mention it. +package sha1cdtest + +import ( + "encoding/hex" + "testing" +) + +// shattered is the first 320 bytes of shattered-1.pdf, the identical-prefix +// SHA-1 collision published as SHAttered (https://shattered.io). Both halves of +// that collision share these bytes, and they are what the collision detection +// recognises, so hashing them must be refused. +const shattered = "255044462d312e330a25e2e3cfd30a0a0a312030206f626a0a3c3c2f57696474" + + "682032203020522f4865696768742033203020522f547970652034203020522f" + + "537562747970652035203020522f46696c7465722036203020522f436f6c6f72" + + "53706163652037203020522f4c656e6774682038203020522f42697473506572" + + "436f6d706f6e656e7420383e3e0a73747265616d0affd8fffe00245348412d31" + + "20697320646561642121212121852fec092339759c39b1a1c63c4c97e1fffe01" + + "7346dc9166b67e118f029ab621b2560ff9ca67cca8c7f85ba84c79030c2b3de2" + + "18f86db3a90901d5df45c14f26fedfb3dc38e96ac22fe7bd728f0e45bce046d2" + + "3c570feb141398bb552ef5a0a82be331fea48037b8b5d71f0e332edf93ac3500" + + "eb4ddc0decc1a864790c782c76215660dd309791d06bd0af3f98cda4bc4629b1" + +// Shattered returns a fresh copy of the colliding bytes. Hashing them with +// chainguard.dev/apko/internal/sha1cd reports sha1cd.ErrCollision. +func Shattered(tb testing.TB) []byte { + tb.Helper() + + b, err := hex.DecodeString(shattered) + if err != nil { + tb.Fatalf("corrupt shattered vector: %v", err) + } + if len(b) != 320 { + tb.Fatalf("shattered vector is %d bytes, want 320", len(b)) + } + return b +} diff --git a/pkg/apk/apk/index.go b/pkg/apk/apk/index.go index ffc225a0b..56dfe065c 100644 --- a/pkg/apk/apk/index.go +++ b/pkg/apk/apk/index.go @@ -19,6 +19,7 @@ import ( "bytes" "context" "crypto" + "crypto/sha256" "errors" "fmt" "io" @@ -38,6 +39,7 @@ import ( "go.opentelemetry.io/otel" "golang.org/x/sync/errgroup" + "chainguard.dev/apko/internal/sha1cd" "chainguard.dev/apko/pkg/apk/auth" sign "chainguard.dev/apko/pkg/apk/signature" ) @@ -446,11 +448,11 @@ func parseRepositoryIndex(ctx context.Context, u string, keys map[string][]byte, for _, sig := range sigs { // compute the digest if not already done if _, hasDigest := indexDigest[sig.DigestAlgorithm]; !hasDigest { - h := sig.DigestAlgorithm.New() - if n, err := h.Write(indexData); err != nil || n != len(indexData) { - return nil, fmt.Errorf("unable to hash data: %w", err) + digest, err := hashIndex(sig.DigestAlgorithm, indexData) + if err != nil { + return nil, err } - indexDigest[sig.DigestAlgorithm] = h.Sum(nil) + indexDigest[sig.DigestAlgorithm] = digest } if err := sign.RSAVerifyDigest(indexDigest[sig.DigestAlgorithm], sig.DigestAlgorithm, sig.Signature, keys[sig.KeyID]); err == nil { verified = true @@ -483,6 +485,27 @@ func parseRepositoryIndex(ctx context.Context, u string, keys map[string][]byte, return index, err } +// hashIndex digests the index data under the algorithm a signature was made +// with. +func hashIndex(algorithm crypto.Hash, indexData []byte) ([]byte, error) { + switch algorithm { + case crypto.SHA1: + // Legacy apk index signatures are RSA over SHA-1, so this digest carries + // collision detection: an index crafted around a SHA-1 collision must be + // rejected, not verified against someone else's signature. + digest, err := sha1cd.SumBytes(indexData) + if err != nil { + return nil, fmt.Errorf("hashing repository index: %w", err) + } + return digest, nil + case crypto.SHA256: + digest := sha256.Sum256(indexData) + return digest[:], nil + default: + return nil, fmt.Errorf("unsupported index digest algorithm: %s", algorithm) + } +} + type indexOpts struct { ignoreSignatures bool noSignatureIndexes []string diff --git a/pkg/apk/apk/index_signature_test.go b/pkg/apk/apk/index_signature_test.go new file mode 100644 index 000000000..c97099159 --- /dev/null +++ b/pkg/apk/apk/index_signature_test.go @@ -0,0 +1,107 @@ +// Copyright 2026 Chainguard, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package apk + +import ( + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/hex" + "encoding/pem" + "errors" + "testing" + + "chainguard.dev/apko/internal/sha1cd" + "chainguard.dev/apko/internal/sha1cd/sha1cdtest" + sign "chainguard.dev/apko/pkg/apk/signature" +) + +func TestHashIndex(t *testing.T) { + // Digests must be unchanged from crypto/sha1 and crypto/sha256, or every + // existing signature over an index would stop verifying. + for _, tc := range []struct { + algorithm crypto.Hash + want string + }{ + {crypto.SHA1, "2aae6c35c94fcfb415dbe95f408b9ce91ee846ed"}, + {crypto.SHA256, "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9"}, + } { + digest, err := hashIndex(tc.algorithm, []byte("hello world")) + if err != nil { + t.Errorf("hashIndex(%s): %v", tc.algorithm, err) + continue + } + if got := hex.EncodeToString(digest); got != tc.want { + t.Errorf("hashIndex(%s) = %s, want %s", tc.algorithm, got, tc.want) + } + } +} + +// An index built around a SHA-1 collision must be refused before its digest can +// be handed to signature verification. +func TestHashIndexRejectsCollision(t *testing.T) { + digest, err := hashIndex(crypto.SHA1, sha1cdtest.Shattered(t)) + if !errors.Is(err, sha1cd.ErrCollision) { + t.Errorf("hashIndex = %x, %v; want ErrCollision", digest, err) + } + if digest != nil { + t.Errorf("hashIndex returned a digest %x alongside the error", digest) + } +} + +func TestHashIndexUnsupportedAlgorithm(t *testing.T) { + if _, err := hashIndex(crypto.SHA512, []byte("hello world")); err == nil { + t.Error("hashIndex(SHA512) = nil error, want an unsupported algorithm error") + } +} + +// Verifying legacy RSA/SHA-1 index signatures must keep working now that apko no +// longer registers crypto.SHA1 in the crypto hash registry. +func TestVerifyLegacySHA1IndexSignature(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generating key: %v", err) + } + pubDER, err := x509.MarshalPKIXPublicKey(&key.PublicKey) + if err != nil { + t.Fatalf("marshalling public key: %v", err) + } + pubPEM := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: pubDER}) + + indexData := []byte("C:Q1eVchkwzRw6t2f8kNKDIcm/6DrE0=\nP:apko\nV:1.0.0\n") + digest, err := hashIndex(crypto.SHA1, indexData) + if err != nil { + t.Fatalf("hashing index: %v", err) + } + + signature, err := rsa.SignPKCS1v15(rand.Reader, key, crypto.SHA1, digest) + if err != nil { + t.Fatalf("signing index: %v", err) + } + + if err := sign.RSAVerifyDigest(digest, crypto.SHA1, signature, pubPEM); err != nil { + t.Errorf("RSAVerifyDigest(SHA1) = %v, want nil", err) + } + + // And a signature over different data must still be rejected. + otherDigest, err := hashIndex(crypto.SHA1, append(indexData, '\n')) + if err != nil { + t.Fatalf("hashing index: %v", err) + } + if err := sign.RSAVerifyDigest(otherDigest, crypto.SHA1, signature, pubPEM); err == nil { + t.Error("RSAVerifyDigest accepted a signature over different index data") + } +} diff --git a/pkg/apk/apk/install.go b/pkg/apk/apk/install.go index f08bd615a..e2bd8de69 100644 --- a/pkg/apk/apk/install.go +++ b/pkg/apk/apk/install.go @@ -18,7 +18,6 @@ import ( "archive/tar" "bytes" "context" - "crypto/sha1" //nolint:gosec // this is what apk tools is using "encoding/base64" "encoding/hex" "errors" @@ -30,6 +29,7 @@ import ( "go.opentelemetry.io/otel" + "chainguard.dev/apko/internal/sha1cd" "chainguard.dev/apko/pkg/apk/expandapk/tarfs" ) @@ -39,7 +39,7 @@ func (a *APK) writeOneFile(header *tar.Header, r io.Reader, allowOverwrite bool) if _, err := a.fs.Stat(header.Name); err == nil { if !allowOverwrite { // get the sum of the file, so we can compare it to the new file - w := sha1.New() //nolint:gosec // this is what apk tools is using + w := sha1cd.New() f, err := a.fs.Open(header.Name) if err != nil { return fmt.Errorf("unable to open existing file to calculate sum %s: %w", header.Name, err) @@ -48,7 +48,11 @@ func (a *APK) writeOneFile(header *tar.Header, r io.Reader, allowOverwrite bool) if _, err := io.Copy(w, f); err != nil { return fmt.Errorf("unable to calculate sum of existing file %s: %w", header.Name, err) } - return FileExistsError{Path: header.Name, Sha1: w.Sum(nil)} + sum, err := sha1cd.Sum(w) + if err != nil { + return fmt.Errorf("unable to calculate sum of existing file %s: %w", header.Name, err) + } + return FileExistsError{Path: header.Name, Sha1: sum} } // allowOverwrite, so remove the file if err := a.fs.Remove(header.Name); err != nil { @@ -85,7 +89,7 @@ func (a *APK) installRegularFile(header *tar.Header, tr *tar.Reader, tmpDir stri if checksum == nil { // There was no checksum header, which is unexpected, but we can just recalculate it. - w := sha1.New() //nolint:gosec // this is what apk tools is using + w := sha1cd.New() tee := io.TeeReader(tr, w) // we need to calculate the checksum of the file, and then pass it to the writeOneFile, @@ -105,7 +109,10 @@ func (a *APK) installRegularFile(header *tar.Header, tr *tar.Reader, tmpDir stri if offset != 0 { return false, fmt.Errorf("error seeking to start of temp file for %s: offset is %d", header.Name, offset) } - checksum = w.Sum(nil) + checksum, err = sha1cd.Sum(w) + if err != nil { + return false, fmt.Errorf("hashing %s: %w", header.Name, err) + } r = f } diff --git a/pkg/apk/apk/install_test.go b/pkg/apk/apk/install_test.go index 4a4f053d4..c72bb90ed 100644 --- a/pkg/apk/apk/install_test.go +++ b/pkg/apk/apk/install_test.go @@ -19,7 +19,6 @@ import ( "bytes" "compress/gzip" "context" - "crypto/sha1" //nolint:gosec // this is what apk tools is using "crypto/sha256" "encoding/base64" "encoding/hex" @@ -31,6 +30,8 @@ import ( "text/template" "github.com/stretchr/testify/require" + + "chainguard.dev/apko/internal/sha1cd" ) type testDirEntry struct { @@ -374,7 +375,7 @@ func fakePackage(t *testing.T, pkg *Package, entries []testDirEntry, dataHashOve f, err := os.CreateTemp(t.TempDir(), pkg.Name+"*.apk") require.NoError(t, err) - h := sha1.New() //nolint:gosec + h := sha1cd.New() ctlZw := gzip.NewWriter(io.MultiWriter(f, h)) ctlTw := tar.NewWriter(ctlZw) @@ -393,10 +394,13 @@ func fakePackage(t *testing.T, pkg *Package, entries []testDirEntry, dataHashOve require.NoError(t, err) require.NoError(t, f.Close()) + sum, err := sha1cd.Sum(h) + require.NoError(t, err) + return &testPackage{ file: f.Name(), pkg: pkg, - checksum: "Q1" + base64.StdEncoding.EncodeToString(h.Sum(nil)), + checksum: "Q1" + base64.StdEncoding.EncodeToString(sum), } } diff --git a/pkg/apk/apk/package.go b/pkg/apk/apk/package.go index 0f55e33cf..194fadb92 100644 --- a/pkg/apk/apk/package.go +++ b/pkg/apk/apk/package.go @@ -19,13 +19,12 @@ import ( "bytes" "compress/gzip" "context" - "crypto/sha1" //nolint:gosec // this is what apk tools is using "fmt" - "hash" "io" "strings" "time" + "chainguard.dev/apko/internal/sha1cd" "chainguard.dev/apko/pkg/apk/expandapk" "chainguard.dev/apko/pkg/apk/types" ) @@ -108,7 +107,7 @@ type Package = types.Package // ParsePackage parses a .apk file and returns a Package struct func ParsePackage(ctx context.Context, apkPackage io.Reader, size uint64) (*Package, error) { - pkginfo, h, err := ParsePackageInfo(apkPackage) + pkginfo, checksum, err := ParsePackageInfo(apkPackage) if err != nil { return nil, err } @@ -122,7 +121,7 @@ func ParsePackage(ctx context.Context, apkPackage io.Reader, size uint64) (*Pack Origin: pkginfo.Origin, Maintainer: pkginfo.Maintainer, URL: pkginfo.URL, - Checksum: h.Sum(nil), + Checksum: checksum, Dependencies: pkginfo.Dependencies, Provides: pkginfo.Provides, InstallIf: pkginfo.InstallIf, @@ -137,8 +136,11 @@ func ParsePackage(ctx context.Context, apkPackage io.Reader, size uint64) (*Pack }, nil } -// ParsePackageInfo returns a parsed .PKGINFO from an APK reader and the control section hash. -func ParsePackageInfo(apkPackage io.Reader) (*PackageInfo, hash.Hash, error) { +// ParsePackageInfo returns a parsed .PKGINFO from an APK reader and the SHA-1 of +// the control section. The control section hash is returned finalised, rather +// than as an unfinalised hash.Hash for the caller to sum, so that it cannot be +// used without its SHA-1 collision check having run. +func ParsePackageInfo(apkPackage io.Reader) (*PackageInfo, []byte, error) { split, err := expandapk.Split(apkPackage) if err != nil { return nil, nil, fmt.Errorf("splitting apk: %w", err) @@ -154,9 +156,9 @@ func ParsePackageInfo(apkPackage io.Reader) (*PackageInfo, hash.Hash, error) { return nil, nil, err } - h := sha1.New() //nolint:gosec - if _, err = h.Write(b); err != nil { - return nil, nil, err + checksum, err := sha1cd.SumBytes(b) + if err != nil { + return nil, nil, fmt.Errorf("hashing control section: %w", err) } zr, err := gzip.NewReader(bytes.NewReader(b)) @@ -177,7 +179,7 @@ func ParsePackageInfo(apkPackage io.Reader) (*PackageInfo, hash.Hash, error) { return nil, nil, fmt.Errorf("parsing .PKGINFO: %w", err) } - return pkg, h, nil + return pkg, checksum, nil } } } diff --git a/pkg/apk/apk/package_getter.go b/pkg/apk/apk/package_getter.go index a397c8977..3e4e653db 100644 --- a/pkg/apk/apk/package_getter.go +++ b/pkg/apk/apk/package_getter.go @@ -17,7 +17,6 @@ package apk import ( "bytes" "context" - "crypto/sha1" //nolint:gosec // this is what apk tools is using "encoding/base64" "encoding/hex" "errors" @@ -35,6 +34,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "chainguard.dev/apko/internal/sha1cd" "chainguard.dev/apko/pkg/apk/auth" "chainguard.dev/apko/pkg/apk/expandapk" "chainguard.dev/apko/pkg/apk/expandapk/tarfs" @@ -310,11 +310,11 @@ func sha1File(path string) ([]byte, error) { return nil, err } defer f.Close() - h := sha1.New() //nolint:gosec // this is what apk tools is using + h := sha1cd.New() if _, err := io.Copy(h, f); err != nil { return nil, err } - return h.Sum(nil), nil + return sha1cd.Sum(h) } // fetchPackage fetches a package from the network or local filesystem. @@ -498,8 +498,11 @@ func (d *defaultPackageGetter) cachedPackage(ctx context.Context, pkg Installabl if err != nil { return nil, err } - signatureHash := sha1.Sum(signatureData) //nolint:gosec // this is what apk tools is using - exp.SignatureHash = signatureHash[:] + signatureHash, err := sha1cd.SumBytes(signatureData) + if err != nil { + return nil, fmt.Errorf("hashing cached signature %q: %w", sig, err) + } + exp.SignatureHash = signatureHash } pkgInfo, err := exp.PkgInfo() diff --git a/pkg/apk/apk/resolveapk.go b/pkg/apk/apk/resolveapk.go index ad825fb93..0f8dde060 100644 --- a/pkg/apk/apk/resolveapk.go +++ b/pkg/apk/apk/resolveapk.go @@ -4,12 +4,11 @@ package apk import ( "bytes" "context" - "crypto/sha1" "crypto/sha256" "fmt" - "hash" "io" + "chainguard.dev/apko/internal/sha1cd" "chainguard.dev/apko/pkg/apk/expandapk" "go.opentelemetry.io/otel" @@ -47,13 +46,15 @@ func ResolveApk(ctx context.Context, source io.Reader) (*APKResolved, error) { // When it's signed the control section is the second stream control, data = split[1], split[2] - var h hash.Hash = sha1.New() //nolint:gosec + h := sha1cd.New() size, err := io.Copy(h, split[0]) if err != nil { return nil, fmt.Errorf("hashing signature: %w", err) } resolved.SignatureSize = int(size) - resolved.SignatureHash = h.Sum(nil) + if resolved.SignatureHash, err = sha1cd.Sum(h); err != nil { + return nil, fmt.Errorf("hashing signature: %w", err) + } } buf := bytes.NewBuffer(nil) @@ -61,8 +62,11 @@ func ResolveApk(ctx context.Context, source io.Reader) (*APKResolved, error) { return nil, fmt.Errorf("hashing control: %w", err) } resolved.ControlSize = buf.Len() - ctrlHash := sha1.Sum(buf.Bytes()) - resolved.ControlHash = ctrlHash[:] + ctrlHash, err := sha1cd.SumBytes(buf.Bytes()) + if err != nil { + return nil, fmt.Errorf("hashing control: %w", err) + } + resolved.ControlHash = ctrlHash dataHash := sha256.New() size, err := io.Copy(dataHash, data) diff --git a/pkg/apk/expandapk/checksums_test.go b/pkg/apk/expandapk/checksums_test.go new file mode 100644 index 000000000..2ae6c75f9 --- /dev/null +++ b/pkg/apk/expandapk/checksums_test.go @@ -0,0 +1,102 @@ +// Copyright 2026 Chainguard, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package expandapk + +import ( + "archive/tar" + "bytes" + "context" + "encoding/hex" + "errors" + "testing" + + "chainguard.dev/apko/internal/sha1cd" + "chainguard.dev/apko/internal/sha1cd/sha1cdtest" +) + +// tarWithFile builds a one-file tar whose entry carries the given SHA-1 in the +// PAX record apk uses for per-file checksums. +func tarWithFile(t *testing.T, content, checksum []byte) []byte { + t.Helper() + + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + if err := tw.WriteHeader(&tar.Header{ + Name: "usr/share/collide", + Typeflag: tar.TypeReg, + Mode: 0o644, + Size: int64(len(content)), + Format: tar.FormatPAX, + PAXRecords: map[string]string{paxRecordsChecksumKey: hex.EncodeToString(checksum)}, + }); err != nil { + t.Fatalf("writing header: %v", err) + } + if _, err := tw.Write(content); err != nil { + t.Fatalf("writing content: %v", err) + } + if err := tw.Close(); err != nil { + t.Fatalf("closing tar: %v", err) + } + return buf.Bytes() +} + +// A file whose content is a known SHA-1 collision must be rejected, even when +// the checksum in its header is the one collision detection produces: the +// digest is untrustworthy, so it cannot be used to accept the file. +func TestCheckSumsRejectsCollision(t *testing.T) { + collide := sha1cdtest.Shattered(t) + + // The digest sha1cd computes for colliding input, which an attacker would + // have to put in the header for the comparison to otherwise succeed. + h := sha1cd.New() + if _, err := h.Write(collide); err != nil { + t.Fatal(err) + } + mitigated := h.Sum(nil) + + err := checkSums(context.Background(), bytes.NewReader(tarWithFile(t, collide, mitigated))) + if !errors.Is(err, sha1cd.ErrCollision) { + t.Errorf("checkSums = %v, want ErrCollision", err) + } +} + +// The same path must still accept an ordinary file, so the check is not simply +// failing everything. +func TestCheckSumsAcceptsMatchingChecksum(t *testing.T) { + content := []byte("hello world") + sum, err := sha1cd.SumBytes(content) + if err != nil { + t.Fatal(err) + } + + if err := checkSums(context.Background(), bytes.NewReader(tarWithFile(t, content, sum))); err != nil { + t.Errorf("checkSums = %v, want nil", err) + } +} + +func TestCheckSumsRejectsMismatchedChecksum(t *testing.T) { + wrong, err := sha1cd.SumBytes([]byte("not the content")) + if err != nil { + t.Fatal(err) + } + + err = checkSums(context.Background(), bytes.NewReader(tarWithFile(t, []byte("hello world"), wrong))) + if err == nil { + t.Error("checkSums = nil, want a checksum mismatch error") + } + if errors.Is(err, sha1cd.ErrCollision) { + t.Errorf("checkSums = %v, want a checksum mismatch rather than a collision", err) + } +} diff --git a/pkg/apk/expandapk/expandapk.go b/pkg/apk/expandapk/expandapk.go index 7bcb4363e..c76a0ffac 100644 --- a/pkg/apk/expandapk/expandapk.go +++ b/pkg/apk/expandapk/expandapk.go @@ -11,7 +11,6 @@ import ( "bufio" "bytes" "context" - "crypto/sha1" "crypto/sha256" "errors" "fmt" @@ -22,6 +21,7 @@ import ( "strings" "sync" + "chainguard.dev/apko/internal/sha1cd" "chainguard.dev/apko/pkg/apk/expandapk/tarfs" "chainguard.dev/apko/pkg/apk/types" "chainguard.dev/apko/pkg/limitio" @@ -450,8 +450,10 @@ func ExpandApkWithOptions(ctx context.Context, source io.Reader, cacheDir string hashes := [][]byte{} maxStreamsReached := false for { - // Control section uses sha1. - var h hash.Hash = sha1.New() //nolint:gosec // this is what apk tools is using + // Control section uses sha1, data section uses sha256 (below), so h is + // either. Both are finalised with sha1cd.Sum, which checks for a SHA-1 + // collision when h is the sha1 one and plainly finalises the sha256 one. + var h hash.Hash = sha1cd.New() if err := sw.Next(); err != nil { if err == errExpandApkWriterMaxStreams { @@ -486,7 +488,11 @@ func ExpandApkWithOptions(ctx context.Context, source io.Reader, cacheDir string return nil, fmt.Errorf("expandApk error 3: %w", err) } - hashes = append(hashes, h.Sum(nil)) + sum, err := sha1cd.Sum(h) + if err != nil { + return nil, fmt.Errorf("hashing %s: %w", sw.CurrentName(), err) + } + hashes = append(hashes, sum) gzipStreams = append(gzipStreams, sw.CurrentName()) } else { // While we verify checksums, also tee the tar to a separate file. @@ -517,7 +523,11 @@ func ExpandApkWithOptions(ctx context.Context, source io.Reader, cacheDir string return nil, fmt.Errorf("closing tarfile: %w", err) } gzipStreams = append(gzipStreams, sw.CurrentName()) - hashes = append(hashes, h.Sum(nil)) + sum, err := sha1cd.Sum(h) + if err != nil { + return nil, fmt.Errorf("hashing %s: %w", sw.CurrentName(), err) + } + hashes = append(hashes, sum) break } } @@ -640,13 +650,18 @@ func checkSums(ctx context.Context, r io.Reader) error { continue } - w := sha1.New() //nolint:gosec // this is what apk tools is using + w := sha1cd.New() if _, err := io.Copy(w, tr); err != nil { return fmt.Errorf("hashing %s: %w", header.Name, err) } - if want, got := checksum, w.Sum(nil); !bytes.Equal(want, got) { + sum, err := sha1cd.Sum(w) + if err != nil { + return fmt.Errorf("hashing %s: %w", header.Name, err) + } + + if want, got := checksum, sum; !bytes.Equal(want, got) { return fmt.Errorf("checksum mismatch: %s header was %x, computed %x", header.Name, want, got) } } diff --git a/pkg/apk/signature/rsa.go b/pkg/apk/signature/rsa.go index cf88aac18..3bf11badd 100644 --- a/pkg/apk/signature/rsa.go +++ b/pkg/apk/signature/rsa.go @@ -7,7 +7,6 @@ import ( "crypto" "crypto/rand" "crypto/rsa" - _ "crypto/sha1" //nolint:gosec _ "crypto/sha256" "crypto/x509" "encoding/pem" @@ -76,6 +75,14 @@ func RSASignDigest(digest []byte, digestType crypto.Hash, keyFile, passphrase st // RSAVerifyDigest is exported for use in tests and verifies a // signature over the provided hash of a message. The key file must be // in the PEM format. +// +// digestType may be crypto.SHA1, for the legacy apk index signatures that are +// still in the wild. Note that crypto/sha1 is deliberately not registered as +// crypto.SHA1 anywhere in apko: every SHA-1 digest apko computes goes through +// chainguard.dev/apko/internal/sha1cd, which refuses digests of input bearing +// the signature of a collision attack, and a registered crypto.SHA1 would offer +// a collision-blind way to compute one. Verification does not need the hash +// registered, only its digest size. func RSAVerifyDigest(digest []byte, digestType crypto.Hash, signature []byte, publicKey []byte) error { if len(digest) != digestType.Size() { return errDigestLength diff --git a/pkg/tarfs/fs.go b/pkg/tarfs/fs.go index c37756b54..d45b883c3 100644 --- a/pkg/tarfs/fs.go +++ b/pkg/tarfs/fs.go @@ -17,7 +17,6 @@ package tarfs import ( "archive/tar" "bytes" - "crypto/sha1" //nolint:gosec // this is what apk tools is using "encoding/base64" "encoding/hex" "errors" @@ -35,6 +34,7 @@ import ( "golang.org/x/sys/unix" + "chainguard.dev/apko/internal/sha1cd" "chainguard.dev/apko/pkg/apk/apk" apkfs "chainguard.dev/apko/pkg/apk/fs" ) @@ -477,9 +477,10 @@ func (m *memFS) writeHeader(name string, te tarEntry) (bool, error) { // This can happen when go-apk's InitKeyring conflicts with alpine-keys. // Since those files will be in memory, quickly compute the checksum and // ignore this file if they match. - h := sha1.New() //nolint:gosec // this is what apk tools is using - h.Write(existing.data) - checksum := h.Sum(nil) + checksum, err := sha1cd.SumBytes(existing.data) + if err != nil { + return false, fmt.Errorf("hashing conflicting file for %q: %w", name, err) + } if bytes.Equal(want.checksum, checksum) { return false, nil diff --git a/pkg/tarfs/fs_test.go b/pkg/tarfs/fs_test.go index 5e3403676..810027333 100644 --- a/pkg/tarfs/fs_test.go +++ b/pkg/tarfs/fs_test.go @@ -17,7 +17,6 @@ package tarfs_test import ( "archive/tar" "context" - "crypto/sha1" "encoding/hex" "io/fs" "path/filepath" @@ -25,6 +24,7 @@ import ( "github.com/stretchr/testify/require" + "chainguard.dev/apko/internal/sha1cd" "chainguard.dev/apko/pkg/apk/apk" "chainguard.dev/apko/pkg/build" @@ -124,8 +124,9 @@ func TestTarFS(t *testing.T) { Typeflag: tar.TypeSymlink, Linkname: "etc/os-release-symlink", } - originalDigest := sha1.Sum([]byte(original.Linkname)) //nolint:gosec - originalChecksum := hex.EncodeToString(originalDigest[:]) + originalDigest, err := sha1cd.SumBytes([]byte(original.Linkname)) + require.NoError(t, err) + originalChecksum := hex.EncodeToString(originalDigest) original.PAXRecords = map[string]string{ "APK-TOOLS.checksum.SHA1": originalChecksum, } @@ -139,8 +140,9 @@ func TestTarFS(t *testing.T) { Typeflag: tar.TypeSymlink, Linkname: "etc/somewhere-else", } - linkDigest := sha1.Sum([]byte(link.Linkname)) //nolint:gosec - linkChecksum := hex.EncodeToString(linkDigest[:]) + linkDigest, err := sha1cd.SumBytes([]byte(link.Linkname)) + require.NoError(t, err) + linkChecksum := hex.EncodeToString(linkDigest) link.PAXRecords = map[string]string{ "APK-TOOLS.checksum.SHA1": linkChecksum, }