Skip to content

Commit 69bfd7f

Browse files
committed
alpine: hash only the bytes read when computing the control.tar.gz digest
sha1Reader.Read hashed the whole buffer p instead of p[:n], so when the underlying reader returns a short read the stale tail of p is folded into the control.tar.gz SHA1 that the package signature is verified against, making a validly signed package fail verification. Added TestAlpinePackagePartialReads, which unmarshals the existing signed fixture through a reader that returns one byte per call and checks the digest matches the whole-file read and the signature still verifies. Signed-off-by: sueun-dev <sueun.dev@gmail.com>
1 parent a8f4a17 commit 69bfd7f

2 files changed

Lines changed: 54 additions & 1 deletion

File tree

pkg/types/alpine/apk.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func newSHA1Reader(b *bufio.Reader) *sha1Reader {
6262
func (s *sha1Reader) Read(p []byte) (int, error) {
6363
n, err := s.r.Read(p)
6464
if err == nil && n > 0 && s.addToHash {
65-
s.hasher.Write(p)
65+
s.hasher.Write(p[:n])
6666
}
6767
return n, err
6868
}

pkg/types/alpine/apk_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
package alpine
1717

1818
import (
19+
"bytes"
1920
"fmt"
21+
"io"
2022
"os"
2123
"strings"
2224
"testing"
@@ -136,3 +138,54 @@ func TestAlpineMetadataSizeBoundary(t *testing.T) {
136138
t.Fatalf("expected error containing %q, got %q", expectedErr, err.Error())
137139
}
138140
}
141+
142+
// oneByteReader returns at most one byte per Read, forcing the short reads
143+
// that a network stream (e.g. an http.Response.Body) can legitimately return.
144+
type oneByteReader struct {
145+
r io.Reader
146+
}
147+
148+
func (o oneByteReader) Read(p []byte) (int, error) {
149+
if len(p) > 1 {
150+
p = p[:1]
151+
}
152+
return o.r.Read(p)
153+
}
154+
155+
// TestAlpinePackagePartialReads guards against the control.tar.gz SHA1 digest
156+
// depending on how the input reader chunks its data. The digest that the
157+
// package signature is checked against must be a function of the bytes only,
158+
// not of the read sizes the underlying reader happens to return.
159+
func TestAlpinePackagePartialReads(t *testing.T) {
160+
apk, err := os.ReadFile("tests/test_alpine.apk")
161+
if err != nil {
162+
t.Fatalf("could not read archive: %v", err)
163+
}
164+
pubKey, err := os.Open("tests/test_alpine.pub")
165+
if err != nil {
166+
t.Fatalf("could not open public key: %v", err)
167+
}
168+
defer pubKey.Close()
169+
pub, err := x509.NewPublicKey(pubKey)
170+
if err != nil {
171+
t.Fatalf("failed to parse public key: %v", err)
172+
}
173+
174+
full := Package{}
175+
if err := full.Unmarshal(bytes.NewReader(apk)); err != nil {
176+
t.Fatalf("unmarshal (full reads): %v", err)
177+
}
178+
179+
chunked := Package{}
180+
if err := chunked.Unmarshal(oneByteReader{r: bytes.NewReader(apk)}); err != nil {
181+
t.Fatalf("unmarshal (partial reads): %v", err)
182+
}
183+
184+
if !bytes.Equal(full.controlSHA1Digest, chunked.controlSHA1Digest) {
185+
t.Fatalf("control.tar.gz SHA1 depends on read chunking:\n full = %x\n chunked = %x",
186+
full.controlSHA1Digest, chunked.controlSHA1Digest)
187+
}
188+
if err := chunked.VerifySignature(pub.CryptoPubKey()); err != nil {
189+
t.Fatalf("signature verification failed for validly signed package read in small chunks: %v", err)
190+
}
191+
}

0 commit comments

Comments
 (0)