Author SHA1 Message Date
sneak edab668a20 Never report index.mf at the top of the base as unlisted in check (closes #229)
check / check (push) Waiting to run
gen and fetch write the manifest as index.mf at the top of the tree.
check left out only the manifest file it was given, so given a URL, or a
manifest stored elsewhere, it reported that index.mf as not in the
manifest, and --no-extra-files failed a tree fetch had just verified.
check now drops that one report for index.mf at the top of the base
directory. A manifest that lists index.mf still has it checked, and
index.mf in a subdirectory or fetch's leftover .index.mf.tmp are still
reported. The change is in package cli, which owns the default manifest
name; the Checker in package mfer is unchanged.

Model: opus-5-5
2026-10-15 21:27:11 +00:00
clawbot 06127e291f Describe releases as SemVer git tags in the README (closes #226)
check / check (push) Waiting to run
The first section of the README said the project had no versioned
release. It now says releases are git tags named by SemVer and names
the first one. That sentence opens the paragraph on where `mfer
version` gets its number, in place of its old opening sentence, "A
version is a git tag", which the new one covers. The rest of that
paragraph is unchanged and still matches `script/build`,
`script/docker` and the `Dockerfile`.

Model: opus-5-5
2026-10-15 22:42:35 +02:00
clawbot 31e22a9337 Refuse a manifest entry whose mode is above 0777 on load (closes #225)
check / check (push) Waiting to run
Loading accepted an entry recording a mode above 0777, such as 04755.
check then reported a file with exactly the recorded permission bits
as a mode mismatch, list and export printed the out-of-range mode, and
freshen silently rewrote it as the permission bits alone. Loading now
refuses such an entry, naming the path and the mode, so every command
that reads a manifest refuses it with one message. fetch's own check
for this could no longer be reached and is gone; its test moves to the
loading tests in package mfer. docs/FORMAT.md says readers reject a
mode above 0777.

Model: opus-5-5
2026-10-15 22:02:54 +02:00
clawbot 95a37d8009 Load the manifest check downloads from memory, not a temp file (closes #223)
check / check (push) Waiting to run
check given a URL wrote the manifest it downloaded to a file in the temp
directory before loading it, so it failed wherever that directory is
missing or cannot be written, such as the scratch image script/cibuild
builds. It now keeps the manifest in memory, as fetch, list and export
do. NewChecker reads a manifest handed to it as a reader through the new
CheckerOptions.ManifestReader field, in place of ManifestPath, because it
otherwise reads the manifest from the filesystem holding the files it
checks. Only check uses that field. verifyRequiredSigner takes the
signer's fingerprint instead of a Checker, so fetch passes the signer of
the manifest it has already loaded and builds no Checker.

Model: opus-5-5
2026-10-15 21:59:12 +02:00
clawbot 7d601a5902 Require a SHA-256 hash in every manifest entry (closes #224)
check / check (push) Waiting to run
Loading accepted an entry whose hashes held no SHA-256 multihash, and
check and fetch, which compute only SHA-256, then reported the intact
file as a hash mismatch. Loading now refuses, naming the path, an entry
that does not list exactly one SHA-256 multihash; other hashes beside
it are allowed. AddFileWithHash accepts only a SHA-256 multihash, and
freshen carries an unchanged entry's SHA-256 hash over instead of its
first hash, found with SHA256Multihash, new in package mfer.
docs/FORMAT.md states the rule for writers and readers. Two loading
tests whose entries held an empty hash now give each entry a SHA-256
hash; the decoding-size test's path lengths change with it.

Model: opus-5-5
2026-10-15 21:25:27 +02:00
clawbot 6a3f532c8d Return the context's error when it ends during a fetch request (closes #221)
check / check (push) Canceled after 0s
retryingClient.get checked its context right after each request and
returned that request's error, so a deadline that passed while a 503 was
on its way came back as the 503. TestGetStopsWaitingWhenCanceled failed
on that now and then on a loaded host. When the context ends before a
retry, during the request or during the wait, get now returns an error
wrapping both the context's error and the last try's error.

A new test covers the context ending during the request, and
TestGetStopsWaitingWhenCanceled now covers it ending during the wait.
Both answer through a transport rather than a server, so neither depends
on how fast the 503 arrives.

Model: opus-5-5
2026-10-14 01:55:41 +02:00
15 changed files with 658 additions and 364 deletions
+8 -8
View File
@@ -13,14 +13,11 @@ The structure of these files can be found
schema `mfer/mf.proto` for exact field numbers and types. Both are included in
the [project repository](https://git.eeqj.de/sneak/mfer).
The current version is pre-1.0 and while the repo was published in 2022, there
has not yet been any versioned release. [SemVer](https://semver.org) will be
used for releases.
A version is a git tag, and no file in the tree records a version number.
`mfer version` prints the revision the build stamped into the binary, which for
`make build` and the Docker image is the output of `git describe` on the
checkout they build. A binary built without that stamp, as
Releases are git tags named by [SemVer](https://semver.org), starting with
`v1.0.0`. No file in the tree records a version number. `mfer version` prints
the revision the build stamped into the binary, which for `make build` and the
Docker image is the output of `git describe` on the checkout they build. A
binary built without that stamp, as
`go install sneak.berlin/go/mfer/cmd/mfer@<tag>` builds it, prints the module
version Go records in it instead. A program that imports the `mfer` package
finds the version it was built against in its own build information
@@ -304,6 +301,9 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
`--no-extra-files` each one is a failure instead. A named pipe, socket or
device, a symlink to a directory and a broken symlink are never reported,
since gen cannot list them
- never reports `index.mf` at the top of the base directory as a file the
manifest does not list, even when given another manifest or a URL, since
gen and fetch write the manifest there
- `mfer freshen` / `mfer freshen .`
- rewrites `index.mf` to list the files now under the directory holding it,
or under the one `--base` names, hashing only the files that are new or
+29 -21
View File
@@ -80,29 +80,28 @@ After decompressing `innerMessage`, the result is a serialized `MFFile`
Each file entry contains:
| Field | Number | Type | Description |
| ---------- | ------ | ------------------------- | ------------------------------------------------------------- |
| `path` | 1 | string | Relative file path (see Path Rules) |
| `size` | 2 | int64 | File size in bytes |
| `hashes` | 3 | repeated `MFFileChecksum` | At least one hash required; readers reject an entry with none |
| `mimeType` | 301 | string (optional) | MIME type |
| `mtime` | 302 | Timestamp (optional) | Modification time |
| `ctime` | 303 | Timestamp (optional) | Change time (inode metadata change) |
| `mode` | 304 | uint32 | Permission bits (see File Mode) |
| Field | Number | Type | Description |
| ---------- | ------ | ------------------------- | ------------------------------------------------------------ |
| `path` | 1 | string | Relative file path (see Path Rules) |
| `size` | 2 | int64 | File size in bytes |
| `hashes` | 3 | repeated `MFFileChecksum` | Must include exactly one SHA-256 multihash (see Hash Format) |
| `mimeType` | 301 | string (optional) | MIME type |
| `mtime` | 302 | Timestamp (optional) | Modification time |
| `ctime` | 303 | Timestamp (optional) | Change time (inode metadata change) |
| `mode` | 304 | uint32 | Permission bits (see File Mode) |
## File Mode
`mode` holds a file's Unix permission bits, the nine `rwx` bits, so it is never
above `0777` (octal); the setuid, setgid and sticky bits are never recorded.
Writers record `0000` unless whoever creates the manifest asks for permissions.
`0000`, the proto3 default, means no mode was recorded: readers never check or
apply it.
above `0777` (octal); the setuid, setgid and sticky bits are never recorded. A
reader must reject a manifest with an entry whose mode is above `0777`. Writers
record `0000` unless whoever creates the manifest asks for permissions. `0000`,
the proto3 default, means no mode was recorded: readers never check or apply it.
The reference implementation records modes when `gen` or `freshen` is given
`--include-permissions`. `check` fails a file whose permission bits differ from
a recorded mode other than `0000`. `fetch` sets a recorded mode other than
`0000` on each file it writes, and refuses a manifest that records a mode above
`0777` before it requests any file.
`0000` on each file it writes.
## Path Rules
@@ -133,8 +132,16 @@ Each checksum is a single `bytes multiHash` field containing a
self-describing: the encoded bytes include a varint algorithm identifier
followed by a varint digest length followed by the digest itself.
The 1.0 implementation writes SHA-256 multihashes (`0x12` algorithm code).
Implementations must be able to verify SHA-256 multihashes at minimum.
A writer must give every file entry exactly one SHA-256 multihash of the file's
content: the algorithm code `0x12`, the digest length `0x20` and the 32 bytes of
the digest. It may list multihashes of other algorithms beside it. A reader must
refuse a manifest with an entry that has no SHA-256 multihash or more than one,
whatever other hashes the entry lists. A verifier then needs only SHA-256,
checking each file against its entry's SHA-256 multihash; it may ignore the
other hashes.
The reference implementation writes one hash for each entry, the SHA-256
multihash, and its `check` and `fetch` compute only SHA-256.
## Signature Scheme
@@ -212,10 +219,11 @@ compresses the same inner message to the same bytes:
- Each entry holds only `path`, `size`, `hashes` with exactly one hash, `mtime`
and `mode`. The hash is the SHA-256 multihash of the file's content, except
that `freshen` reads only the files that are new or whose size, `mtime` or
`mode` changed, and for every other file carries the first hash of its
existing entry over unchanged. `mtime` is the file's modification time as the
file system gives it, to the nanosecond, and `mode` is `0000` unless `gen` or
`freshen` is given `--include-permissions`.
`mode` changed, and for every other file carries the SHA-256 multihash of its
existing entry over unchanged, leaving out any other hash the entry lists.
`mtime` is the file's modification time as the file system gives it, to the
nanosecond, and `mode` is `0000` unless `gen` or `freshen` is given
`--include-permissions`.
- `createdAt` is left out unless `gen` or `freshen` is given
`--include-timestamps`. Then it holds the time the run started building the
manifest, to the nanosecond, so it differs on every run.
+41 -52
View File
@@ -2,6 +2,7 @@
package cli
import (
"bytes"
"context"
"encoding/hex"
"errors"
@@ -94,47 +95,36 @@ func findManifest(fs afero.Fs, dir string) (string, error) {
return path, nil
}
// fetchManifestToTemp downloads a manifest URL to a temporary file and
// returns the temp file path. The caller is responsible for removing it.
func (mfa *CLIApp) fetchManifestToTemp(
ctx context.Context, url string,
) (string, error) {
rc, fetchErr := mfa.openManifestReader(ctx, url)
if fetchErr != nil {
return "", fetchErr
// downloadManifest downloads the manifest at url into memory. It refuses a
// manifest larger than mfa.maxManifestSize.
func (mfa *CLIApp) downloadManifest(ctx context.Context, url string) ([]byte, error) {
rc, err := mfa.openManifestReader(ctx, url)
if err != nil {
return nil, err
}
tmpFile, tmpErr := afero.TempFile(mfa.Fs, "", "mfer-manifest-*.mf")
if tmpErr != nil {
_ = rc.Close()
defer func() { _ = rc.Close() }()
return "", tmpErr
}
tmpPath := tmpFile.Name()
// Copying stops one byte past mfa.maxManifestSize, which is enough to
// Reading stops one byte past mfa.maxManifestSize, which is enough to
// tell that the manifest is too large.
written, cpErr := io.Copy(tmpFile, io.LimitReader(rc, mfa.maxManifestSize+1))
_ = rc.Close()
_ = tmpFile.Close()
if cpErr == nil && written > mfa.maxManifestSize {
cpErr = fmt.Errorf("%w of %d bytes", errManifestTooLarge, mfa.maxManifestSize)
manifestData, err := io.ReadAll(io.LimitReader(rc, mfa.maxManifestSize+1))
if err != nil {
return nil, fmt.Errorf("download manifest: %w", err)
}
if cpErr != nil {
_ = mfa.Fs.Remove(tmpPath)
return "", fmt.Errorf("download manifest: %w", cpErr)
if int64(len(manifestData)) > mfa.maxManifestSize {
return nil, fmt.Errorf("download manifest: %w of %d bytes",
errManifestTooLarge, mfa.maxManifestSize)
}
return tmpPath, nil
return manifestData, nil
}
// verifyRequiredSigner enforces the --require-signature fingerprint
// against the key that made the manifest's signature.
func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
// against signer, the fingerprint of the key that made the manifest's
// signature, which is empty when the manifest is not signed. Loading the
// manifest checked that signer is that key's fingerprint.
func verifyRequiredSigner(signer, requiredSigner string) error {
// Validate fingerprint format: must be exactly 40 hex characters
if len(requiredSigner) != fingerprintHexLen {
return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner))
@@ -145,15 +135,11 @@ func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error {
return fmt.Errorf("invalid fingerprint: %w", err)
}
if !chk.IsSigned() {
if signer == "" {
return fmt.Errorf("%w, but signature from %s is required",
errManifestNotSigned, requiredSigner)
}
// Loading the manifest checked that the signer is the fingerprint of
// the key that made the signature.
signer := string(chk.Signer())
// Compare fingerprints - must be exact match (case-insensitive)
if !strings.EqualFold(signer, requiredSigner) {
return fmt.Errorf("embedded signing key fingerprint %s %w %s",
@@ -209,7 +195,10 @@ func countCheckFailures(
// findExtraFiles reports files present on disk but absent from the
// manifest, and anything the search cannot read: each is a failure, logged
// at error level, under --no-extra-files, otherwise a warning.
// at error level, under --no-extra-files, otherwise a warning. index.mf at
// the top of the base directory is never reported as absent from the
// manifest, even when check is given another manifest or a URL, since gen
// and fetch write the manifest there.
func findExtraFiles(
ctx context.Context, cmd *cli.Command, chk *mfer.Checker, failures *int64,
) error {
@@ -218,6 +207,10 @@ func findExtraFiles(
go func() {
for result := range extraResults {
if result.Status == mfer.StatusExtra && result.Path == defaultManifestName {
continue
}
if cmd.Bool("no-extra-files") {
*failures++
@@ -299,32 +292,28 @@ func (mfa *CLIApp) checkManifestOperation(
return err
}
// Done before a URL is swapped for the temp file it is downloaded to,
// whose directory is not the base.
basePath := resolveBasePath(cmd, manifestPath)
// URL manifests need to be downloaded to a temp file for the checker
opts := &mfer.CheckerOptions{BasePath: basePath, Fs: mfa.Fs}
// A manifest given by URL is downloaded into memory and read from
// there.
if isHTTPURL(manifestPath) {
tmpPath, tmpErr := mfa.fetchManifestToTemp(ctx, manifestPath)
if tmpErr != nil {
return tmpErr
manifestData, downloadErr := mfa.downloadManifest(ctx, manifestPath)
if downloadErr != nil {
return downloadErr
}
defer func() { _ = mfa.Fs.Remove(tmpPath) }()
manifestPath = tmpPath
opts.ManifestReader = bytes.NewReader(manifestData)
} else {
opts.ManifestPath = manifestPath
}
showProgress := cmd.Bool("progress")
log.Infof("checking manifest %s with base %s", manifestPath, basePath)
// Create checker
chk, err := mfer.NewChecker(&mfer.CheckerOptions{
ManifestPath: manifestPath,
BasePath: basePath,
Fs: mfa.Fs,
})
chk, err := mfer.NewChecker(opts)
if err != nil {
return fmt.Errorf("load manifest: %w", err)
}
@@ -332,7 +321,7 @@ func (mfa *CLIApp) checkManifestOperation(
// Check signature requirement. An empty fingerprint is checked too,
// and refused as one of the wrong length.
if cmd.IsSet(flagRequireSignature) {
err = verifyRequiredSigner(chk, cmd.String(flagRequireSignature))
err = verifyRequiredSigner(string(chk.Signer()), cmd.String(flagRequireSignature))
if err != nil {
return err
}
+143
View File
@@ -1167,6 +1167,122 @@ func TestCheckURLManifestResolvesEntriesAgainstCurrentDirectory(t *testing.T) {
assert.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
}
// TestCheckURLManifestWithoutTempDirectory checks the tree under --base
// against a manifest given by URL while TMPDIR names a directory that does
// not exist. check keeps the manifest it downloads in memory, so it passes.
// Setting TMPDIR keeps it from running in parallel with other tests.
func TestCheckURLManifestWithoutTempDirectory(t *testing.T) {
files := map[string][]byte{testFileTxt: []byte("hello")}
server := httptest.NewServer(fetchTestHandler(manifestOf(t, files), files))
defer server.Close()
base := t.TempDir()
require.NoError(t,
os.WriteFile(filepath.Join(base, testFileTxt), files[testFileTxt], 0o600))
t.Setenv("TMPDIR", filepath.Join(t.TempDir(), "missing"))
opts := testOpts([]string{
testApp, cmdCheck, testFlagNoExtra, testFlagBase, base,
server.URL + "/" + defaultManifestName,
}, afero.NewOsFs())
assert.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
}
// TestCheckNeverReportsIndexMFAtTopOfBase checks a tree holding index.mf at
// its top, where gen and fetch write the manifest, against a manifest that
// does not list it, given by URL and by a path outside the tree: check never
// reports it, and passes with and without --no-extra-files. index.mf in a
// subdirectory, and the temp file fetch saves index.mf through, are still
// reported.
func TestCheckNeverReportsIndexMFAtTopOfBase(t *testing.T) {
t.Parallel()
files := map[string][]byte{testFileTxt: []byte("hello")}
manifest := manifestOf(t, files)
server := httptest.NewServer(fetchTestHandler(manifest, files))
t.Cleanup(server.Close)
manifestOutsideTree := filepath.Join(t.TempDir(), "other.mf")
require.NoError(t, os.WriteFile(manifestOutsideTree, manifest, 0o600))
for name, manifestArg := range map[string]string{
"manifest by URL": server.URL + "/" + defaultManifestName,
"manifest by path": manifestOutsideTree,
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
base := t.TempDir()
require.NoError(t, os.WriteFile(
filepath.Join(base, testFileTxt), files[testFileTxt], 0o600))
require.NoError(t, os.WriteFile(
filepath.Join(base, defaultManifestName), manifest, 0o600))
check := func(flags ...string) (int, string) {
opts := testOpts(slices.Concat(
[]string{testApp, cmdCheck}, flags,
[]string{testFlagBase, base, manifestArg},
), afero.NewOsFs())
return runCLI(opts), testStderr(t, opts)
}
exitCode, stderr := check()
assert.Equal(t, 0, exitCode, "stderr: %s", stderr)
assert.NotContains(t, stderr, "EXTRA: "+defaultManifestName)
exitCode, stderr = check(testFlagNoExtra)
assert.Equal(t, 0, exitCode, "stderr: %s", stderr)
assert.NotContains(t, stderr, "EXTRA: "+defaultManifestName)
reported := []string{
testSubdirManifest,
tempPathFor(defaultManifestName),
}
require.NoError(t, os.Mkdir(filepath.Join(base, testSubdir), 0o750))
for _, path := range reported {
require.NoError(t,
os.WriteFile(filepath.Join(base, path), manifest, 0o600))
}
exitCode, stderr = check(testFlagNoExtra)
assert.Equal(t, 1, exitCode, "stderr: %s", stderr)
assert.NotContains(t, stderr, "EXTRA: "+defaultManifestName)
for _, path := range reported {
assert.Contains(t, stderr, "EXTRA: "+path)
}
})
}
}
// TestCheckChecksListedIndexMF checks a tree against a manifest that lists
// index.mf at the top of the tree, where that file has changed since: check
// reports it like any other changed file.
func TestCheckChecksListedIndexMF(t *testing.T) {
t.Parallel()
manifestPath := filepath.Join(t.TempDir(), "other.mf")
require.NoError(t, os.WriteFile(manifestPath, manifestOf(t, map[string][]byte{
defaultManifestName: []byte("listed"),
}), 0o600))
base := t.TempDir()
require.NoError(t, os.WriteFile(
filepath.Join(base, defaultManifestName), []byte("edited"), 0o600))
opts := testOpts([]string{
testApp, cmdCheck, testFlagBase, base, manifestPath,
}, afero.NewOsFs())
assert.Equal(t, 1, runCLI(opts), "stderr: %s", testStderr(t, opts))
assert.Contains(t, testStderr(t, opts), "HASH_MISMATCH: "+defaultManifestName)
}
// unlistableDirFs is a filesystem on which one directory cannot be listed.
type unlistableDirFs struct {
afero.Fs
@@ -1215,6 +1331,33 @@ func TestCheckWithUnlistableDirectory(t *testing.T) {
assert.Contains(t, testStderr(t, opts), "unlisted.txt")
}
// TestCheckReportsUnreadableIndexMFAtTopOfBase has a directory named
// index.mf at the top of the base that cannot be listed. check leaves out
// only the report that index.mf is not in the manifest, so it reports the
// error and fails under --no-extra-files.
func TestCheckReportsUnreadableIndexMFAtTopOfBase(t *testing.T) {
t.Parallel()
unreadable := filepath.Join(testDir, defaultManifestName)
mem := afero.NewMemMapFs()
require.NoError(t, mem.MkdirAll(unreadable, 0o755))
writeTestFile(t, mem, testFile1, "hello")
opts := testOpts([]string{
testApp, cmdGenerate, "-q", "-o", testManifest, testDir,
}, mem)
require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
fs := unlistableDirFs{Fs: mem, dir: unreadable}
opts = testOpts([]string{
testApp, cmdCheck, testFlagNoExtra, testFlagBase, testDir, testManifest,
}, fs)
assert.Equal(t, 1, runCLI(opts), "stderr: %s", testStderr(t, opts))
assert.Contains(t, testStderr(t, opts), "ERROR: "+defaultManifestName)
}
func TestGenerateAtomicWriteNoTempFileOnSuccess(t *testing.T) {
t.Parallel()
+6 -38
View File
@@ -50,38 +50,6 @@ func runLocked(fn func() error) error {
return fn()
}
// unsignedChecker builds a Checker over a freshly scanned, unsigned manifest.
func unsignedChecker(t *testing.T) *mfer.Checker {
t.Helper()
// The scan and the manifest load log to the process-global logger, so
// serialize with the CLI runs.
runMu.Lock()
defer runMu.Unlock()
fs := afero.NewMemMapFs()
require.NoError(t, fs.MkdirAll("/d", 0o755))
require.NoError(t, afero.WriteFile(fs, "/d/f.txt", []byte("hi"), 0o644))
s := mfer.NewScannerWithOptions(&mfer.ScannerOptions{Fs: fs})
require.NoError(t, s.EnumeratePath("/d", nil))
var buf bytes.Buffer
require.NoError(t, s.ToManifest(context.Background(), &buf, nil))
require.NoError(t, afero.WriteFile(fs, "/d/index.mf", buf.Bytes(), 0o644))
chk, err := mfer.NewChecker(&mfer.CheckerOptions{
ManifestPath: "/d/index.mf",
BasePath: "/d",
Fs: fs,
})
require.NoError(t, err)
require.False(t, chk.IsSigned())
return chk
}
func TestNoManifestFoundMessage(t *testing.T) {
t.Parallel()
@@ -97,7 +65,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
t.Run("invalid fingerprint length", func(t *testing.T) {
t.Parallel()
err := verifyRequiredSigner(unsignedChecker(t), "12345678")
err := verifyRequiredSigner("", "12345678")
require.ErrorIs(t, err, errInvalidFingerprint)
assert.EqualError(t, err,
"invalid fingerprint: must be exactly 40 hex characters, got 8")
@@ -106,7 +74,7 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
t.Run("manifest not signed", func(t *testing.T) {
t.Parallel()
err := verifyRequiredSigner(unsignedChecker(t), msgFpA)
err := verifyRequiredSigner("", msgFpA)
require.ErrorIs(t, err, errManifestNotSigned)
assert.EqualError(t, err,
"manifest is not signed, but signature from "+msgFpA+" is required")
@@ -121,13 +89,13 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
func TestSignerMismatchMessage(t *testing.T) {
t.Parallel()
chk := signedChecker(t,
signedManifest(t, map[string][]byte{"f.txt": []byte("signed file")}))
signer := string(signedChecker(t,
signedManifest(t, map[string][]byte{"f.txt": []byte("signed file")})).Signer())
err := verifyRequiredSigner(chk, msgFpB)
err := verifyRequiredSigner(signer, msgFpB)
require.ErrorIs(t, err, errSignerMismatch)
assert.EqualError(t, err,
"embedded signing key fingerprint "+string(chk.Signer())+
"embedded signing key fingerprint "+signer+
" does not match required "+msgFpB)
}
+13 -47
View File
@@ -19,7 +19,6 @@ import (
"github.com/dustin/go-humanize"
"github.com/multiformats/go-multihash"
"github.com/spf13/afero"
"github.com/urfave/cli/v3"
"sneak.berlin/go/mfer/internal/log"
"sneak.berlin/go/mfer/mfer"
@@ -96,9 +95,6 @@ var (
// writes another file.
errNameClash = errors.New(
"manifest lists a file where fetch writes another file")
// errModeOutOfRange indicates a manifest that lists a mode with more
// than the permission bits, such as setuid.
errModeOutOfRange = errors.New("manifest lists a mode outside 0777")
)
// DownloadProgress reports the progress of a single file download.
@@ -126,8 +122,9 @@ type retryingClient struct {
// to a limit that doubles each time, unless the server's Retry-After
// header says how long to wait. Any other failure, an error from use
// included, is returned at once and unwrapped; a non-OK status is
// returned as errHTTPStatus. use must start over each time it is called,
// since a retry calls it again with a new response.
// returned as errHTTPStatus. If ctx ends before a retry, the error wraps
// both ctx's error and the last try's error. use must start over each
// time it is called, since a retry calls it again with a new response.
func (c retryingClient) get(
ctx context.Context, rawURL string, use func(*http.Response) error,
) error {
@@ -169,15 +166,19 @@ func (c retryingClient) get(
}
}
if !retry || attempt == fetchAttempts || ctx.Err() != nil {
if !retry || attempt == fetchAttempts {
return err
}
if ctx.Err() != nil {
return fmt.Errorf("%w: %w", ctx.Err(), err)
}
log.Warnf("%s: %s, retrying in %s", rawURL, err, wait.Round(time.Millisecond))
select {
case <-ctx.Done():
return ctx.Err()
return fmt.Errorf("%w: %w", ctx.Err(), err)
case <-time.After(wait):
}
@@ -424,9 +425,9 @@ func (mfa *CLIApp) fetchManifestOperation(
// fetchManifest downloads the manifest at manifestURL and parses it,
// enforcing --require-signature if it is given and refusing a manifest
// that lists a file where fetch writes another or a mode outside 0777. It
// returns the manifest as downloaded, to be saved once the files are in
// place, and the files it lists.
// that lists a file where fetch writes another. It returns the manifest as
// downloaded, to be saved once the files are in place, and the files it
// lists.
func (mfa *CLIApp) fetchManifest(
ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string,
) ([]byte, []*mfer.MFFilePath, error) {
@@ -470,7 +471,7 @@ func (mfa *CLIApp) fetchManifest(
}
if requiredSigner != "" {
err = verifyFetchedSigner(manifestData, requiredSigner)
err = verifyRequiredSigner(string(manifest.Signer()), requiredSigner)
if err != nil {
return nil, nil, err
}
@@ -483,16 +484,6 @@ func (mfa *CLIApp) fetchManifest(
return nil, nil, err
}
// fetch sets each recorded mode on the file it writes, so a mode above
// 0777, which could carry setuid, setgid or sticky bits, is refused
// before any file is requested.
for _, f := range files {
if f.GetMode() > uint32(os.ModePerm) {
return nil, nil, fmt.Errorf("%w: %s (%#o)",
errModeOutOfRange, f.GetPath(), f.GetMode())
}
}
log.Infof("manifest contains %d files", len(files))
return manifestData, files, nil
@@ -548,31 +539,6 @@ func checkNoNameClash(files []*mfer.MFFilePath) error {
return nil
}
// verifyFetchedSigner enforces --require-signature on the fetched manifest
// exactly as check does. verifyRequiredSigner takes a Checker, which loads
// its manifest from a file, so the manifest is handed to it as a file in
// memory.
func verifyFetchedSigner(manifestData []byte, requiredSigner string) error {
memFs := afero.NewMemMapFs()
manifestPath := "/" + defaultManifestName
err := afero.WriteFile(memFs, manifestPath, manifestData, filePerms)
if err != nil {
return err
}
chk, err := mfer.NewChecker(&mfer.CheckerOptions{
ManifestPath: manifestPath,
BasePath: "/",
Fs: memFs,
})
if err != nil {
return fmt.Errorf("load manifest: %w", err)
}
return verifyRequiredSigner(chk, requiredSigner)
}
// saveManifest writes the fetched manifest into dest under the default
// manifest name, the way fetch writes every file: to a new temp file that
// is then renamed into place.
+51 -86
View File
@@ -19,15 +19,12 @@ import (
"sync/atomic"
"testing"
"time"
"uuid"
"github.com/klauspost/compress/zstd"
"github.com/multiformats/go-multihash"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
urfcli "github.com/urfave/cli/v3"
"google.golang.org/protobuf/proto"
"sneak.berlin/go/mfer/mfer"
)
@@ -489,18 +486,17 @@ func TestFetchStopsReadingFilePastListedSize(t *testing.T) {
// TestManifestDownloadStopsAtLimit serves a manifest that never ends to
// fetch and to check, with the most they download of a manifest lowered
// to 64 KiB. Each must stop reading at that limit, fail with an error
// naming it and leave no temp file.
// to 64 KiB. Each must stop reading at that limit and fail with an error
// naming it.
func TestManifestDownloadStopsAtLimit(t *testing.T) {
t.Parallel()
server := httptest.NewServer(
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = io.Copy(w, zeros{})
}))
defer server.Close()
tmpDir := t.TempDir()
t.Setenv("TMPDIR", tmpDir)
mfa := &CLIApp{Fs: afero.NewOsFs(), maxManifestSize: 64 << 10}
fetch := mfa.fetchCommand()
@@ -520,10 +516,6 @@ func TestManifestDownloadStopsAtLimit(t *testing.T) {
"download manifest: file exceeds maximum allowed size of 65536 bytes",
cmd.Name)
}
leftover, err := os.ReadDir(tmpDir)
require.NoError(t, err)
assert.Empty(t, leftover)
}
//nolint:paralleltest // changes the process-global working directory
@@ -897,23 +889,59 @@ func TestGetHonorsRetryAfter(t *testing.T) {
}
}
// TestGetStopsWaitingWhenCanceled gives get a server that always answers
// 503 and an hour's wait before each retry, then lets the context expire
// during that wait. get returning at all shows it stopped waiting.
// instant503Transport answers every request with a 503 at once, so the
// 503 is in hand before any deadline can pass.
type instant503Transport struct{}
func (instant503Transport) RoundTrip(*http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: http.StatusServiceUnavailable,
Body: http.NoBody,
}, nil
}
// TestGetStopsWaitingWhenCanceled answers every request with a 503 and
// sets an hour's wait before each retry, then lets the context expire
// during that wait. get returning at all shows it stopped waiting; it must
// return the deadline error and keep the 503 with it.
func TestGetStopsWaitingWhenCanceled(t *testing.T) {
t.Parallel()
server := httptest.NewServer(
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusServiceUnavailable)
}))
defer server.Close()
client := testClient()
client.client.Transport = instant503Transport{}
client.firstDelay = time.Hour
err := getNothing(client, server.URL, 50*time.Millisecond)
err := getNothing(client, "http://example.invalid/", 50*time.Millisecond)
require.ErrorIs(t, err, context.DeadlineExceeded)
require.ErrorIs(t, err, errHTTPStatus)
}
// late503Transport answers every request with a 503, but only once the
// request's context has ended. A real server cannot stand in for it: the
// client drops a response still on its way when the context ends.
type late503Transport struct{}
func (late503Transport) RoundTrip(req *http.Request) (*http.Response, error) {
<-req.Context().Done()
return &http.Response{
StatusCode: http.StatusServiceUnavailable,
Body: http.NoBody,
}, nil
}
// TestGetStopsWhenContextEndsDuringRequest lets the context expire while
// the first request is still waiting for its 503. get must return the
// deadline error and keep the 503 with it.
func TestGetStopsWhenContextEndsDuringRequest(t *testing.T) {
t.Parallel()
client := testClient()
client.client.Transport = late503Transport{}
err := getNothing(client, "http://example.invalid/", 50*time.Millisecond)
require.ErrorIs(t, err, context.DeadlineExceeded)
require.ErrorIs(t, err, errHTTPStatus)
}
// TestDownloadFileRetriesToSuccess serves a file that fails twice before
@@ -1504,27 +1532,9 @@ func TestFetchSetsRecordedMode(t *testing.T) {
[]string{"/" + defaultManifestName, "/" + testFileTxt}, requested)
}
// TestFetchRefusesModeOutsidePermissionBits fetches manifests that record
// a mode with the setuid bit, once as Unix writes it (04755) and once as
// Go keeps it in os.FileMode. fetch must refuse both before it creates the
// destination or requests any file.
func TestFetchRefusesModeOutsidePermissionBits(t *testing.T) {
t.Parallel()
files := map[string][]byte{testFileTxt: []byte("a file")}
assertFetchRefused(t, manifestWithMode(t, testFileTxt, files[testFileTxt], 0o4755),
files, "manifest lists a mode outside 0777: file.txt (04755)")
assertFetchRefused(t,
manifestWithMode(t, testFileTxt, files[testFileTxt],
uint32(os.ModeSetuid|0o755)),
files, "manifest lists a mode outside 0777: file.txt (040000755)")
}
// TestDownloadFileSetsOnlyPermissionBits downloads a file whose entry
// records mode 0755 together with setuid, setgid or sticky, as Go keeps
// them in os.FileMode. fetchManifest would refuse such an entry; called
// them in os.FileMode. Loading a manifest refuses such an entry; called
// directly, downloadFile must still set only the permission bits.
func TestDownloadFileSetsOnlyPermissionBits(t *testing.T) {
t.Parallel()
@@ -1561,51 +1571,6 @@ func TestDownloadFileSetsOnlyPermissionBits(t *testing.T) {
}
}
// manifestWithMode returns a manifest listing one file, path, with content
// and the given recorded mode. It is assembled by hand, since the builder
// never records a mode outside 0777.
func manifestWithMode(t *testing.T, path string, content []byte, mode uint32) []byte {
t.Helper()
digest := sha256.Sum256(content)
hash, err := multihash.Encode(digest[:], multihash.SHA2_256)
require.NoError(t, err)
id := uuid.NewV4()
inner, err := proto.Marshal(&mfer.MFFile{
Version: mfer.MFFile_VERSION_ONE,
Files: []*mfer.MFFilePath{{
Path: path,
Size: int64(len(content)),
Hashes: []*mfer.MFFileChecksum{{MultiHash: hash}},
Mode: mode,
}},
Uuid: id[:],
})
require.NoError(t, err)
encoder, err := zstd.NewWriter(nil)
require.NoError(t, err)
compressed := encoder.EncodeAll(inner, nil)
require.NoError(t, encoder.Close())
sum := sha256.Sum256(compressed)
outer, err := proto.Marshal(&mfer.MFFileOuter{
Version: mfer.MFFileOuter_VERSION_ONE,
CompressionType: mfer.MFFileOuter_COMPRESSION_ZSTD,
Size: int64(len(inner)),
Sha256: sum[:],
Uuid: id[:],
InnerMessage: compressed,
})
require.NoError(t, err)
return append([]byte(mfer.MAGIC), outer...)
}
// builtManifest returns a manifest of files, built directly rather than
// scanned, since a scan lists no hidden files.
func builtManifest(t *testing.T, files map[string][]byte) []byte {
+11 -11
View File
@@ -33,9 +33,9 @@ var (
// modification time where one is required to carry it forward unchanged.
errEntryMissingMtime = errors.New("manifest entry has no mtime")
// errEntryMissingHash indicates a manifest entry that carries no hash
// to carry forward.
errEntryMissingHash = errors.New("manifest entry has no hash")
// errEntryMissingHash indicates a manifest entry that carries no
// SHA-256 hash to carry forward.
errEntryMissingHash = errors.New("manifest entry has no SHA-256 hash")
)
// errSignKeyNeeded indicates a signed manifest given to freshen with no
@@ -699,26 +699,26 @@ func addFileToBuilder(
// Entries reach this path only when recordEntry classified them as
// unchanged, which requires a recorded mtime, so an absent mtime here is
// an error rather than something to paper over with the Unix epoch.
// Loading refuses an entry with no hash; one that got here anyway is an
// error too, not a file to leave out of the manifest.
// Only the entry's SHA-256 hash is carried over, wherever the entry lists
// it, since check and fetch compare with no other. Loading refuses an
// entry with no SHA-256 hash; one that got here anyway is an error too,
// not a file to leave out of the manifest.
func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
mtime, ok := entryMtime(entry)
if !ok {
return fmt.Errorf("%w: %s", errEntryMissingMtime, entry.GetPath())
}
if len(entry.GetHashes()) == 0 {
hash := mfer.SHA256Multihash(entry)
if hash == nil {
return fmt.Errorf("%w: %s", errEntryMissingHash, entry.GetPath())
}
err := b.AddFileWithHash(mfer.RelFilePath(entry.GetPath()),
mfer.FileSize(entry.GetSize()), mfer.ModTime(mtime),
fs.FileMode(entry.GetMode()), entry.GetHashes()[0].GetMultiHash())
fs.FileMode(entry.GetMode()), hash)
if err != nil {
return fmt.Errorf(
"manifest entry %s: %w (regenerate the manifest with mfer generate)",
entry.GetPath(), err,
)
return fmt.Errorf("manifest entry %s: %w", entry.GetPath(), err)
}
return nil
+60 -29
View File
@@ -2,7 +2,10 @@
package cli
import (
"bytes"
"context"
"crypto/sha256"
"crypto/sha512"
"maps"
"os"
"path/filepath"
@@ -584,48 +587,76 @@ func TestFreshenAddExistingRejectsMissingMtime(t *testing.T) {
assert.Contains(t, err.Error(), "file1.txt")
}
// TestFreshenAddExistingRejectsMissingHash pins that an entry with no hash
// is an error rather than a file left out of the rebuilt manifest.
// TestFreshenAddExistingRejectsMissingHash pins that an entry with no
// SHA-256 hash, whether it lists no hash or only a SHA-1 one, is an error
// rather than a file left out of the rebuilt manifest.
func TestFreshenAddExistingRejectsMissingHash(t *testing.T) {
t.Parallel()
b := mfer.NewBuilder()
entry := &mfer.MFFilePath{
Path: "data.bin",
Size: 8,
Mtime: &mfer.Timestamp{Seconds: 1_700_000_000},
}
err := addExistingToBuilder(b, entry)
require.ErrorIs(t, err, errEntryMissingHash)
assert.Contains(t, err.Error(), "data.bin")
assert.Zero(t, b.FileCount())
}
// TestFreshenAddExistingRejectsShortHash pins that an existing manifest
// entry whose hash the builder refuses is reported as a problem with the
// manifest, with the command that fixes it.
func TestFreshenAddExistingRejectsShortHash(t *testing.T) {
t.Parallel()
sha1Hash, err := multihash.Encode(make([]byte, 20), multihash.SHA1)
require.NoError(t, err)
tests := []struct {
name string
hashes []*mfer.MFFileChecksum
}{
{"no hash", nil},
{"SHA-1 hash only", []*mfer.MFFileChecksum{{MultiHash: sha1Hash}}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
b := mfer.NewBuilder()
entry := &mfer.MFFilePath{
Path: "data.bin",
Size: 8,
Mtime: &mfer.Timestamp{Seconds: 1_700_000_000},
Hashes: tt.hashes,
}
err := addExistingToBuilder(b, entry)
require.ErrorIs(t, err, errEntryMissingHash)
assert.Contains(t, err.Error(), "data.bin")
assert.Zero(t, b.FileCount())
})
}
}
// TestFreshenAddExistingKeepsSHA256Hash pins that an unchanged entry that
// lists another hash before its SHA-256 hash is carried over with the
// SHA-256 hash, the one check and fetch compare with, and nothing else.
func TestFreshenAddExistingKeepsSHA256Hash(t *testing.T) {
t.Parallel()
sha512Hash, err := multihash.Encode(make([]byte, sha512.Size), multihash.SHA2_512)
require.NoError(t, err)
sha256Hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
require.NoError(t, err)
b := mfer.NewBuilder()
entry := &mfer.MFFilePath{
Path: "old.txt",
Size: 8,
Path: "kept.txt",
Size: 6,
Mtime: &mfer.Timestamp{Seconds: 1_700_000_000},
Hashes: []*mfer.MFFileChecksum{
{MultiHash: sha1Hash},
{MultiHash: sha512Hash},
{MultiHash: sha256Hash},
},
}
err = addExistingToBuilder(b, entry)
require.Error(t, err)
assert.Contains(t, err.Error(), "manifest entry old.txt")
assert.Contains(t, err.Error(), "mfer generate")
assert.Zero(t, b.FileCount())
require.NoError(t, addExistingToBuilder(b, entry))
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
m, err := mfer.NewManifestFromReader(&buf)
require.NoError(t, err)
require.Len(t, m.Files(), 1)
require.Len(t, m.Files()[0].GetHashes(), 1)
assert.Equal(t, sha256Hash, m.Files()[0].GetHashes()[0].GetMultiHash())
}
// TestEntryMtime pins the presence semantics the callers depend on.
+26 -17
View File
@@ -37,8 +37,7 @@ var (
errPathDot = errors.New("contains '.' segment")
errSizeMismatch = errors.New("size mismatch")
errNegativeSize = errors.New("size cannot be negative")
errHashNotMultihash = errors.New("hash is not a valid multihash")
errHashTooShort = errors.New("hash digest is too short")
errHashNotSHA256 = errors.New("hash is not a SHA-256 multihash")
errDuplicatePath = errors.New("duplicate path")
)
@@ -113,6 +112,28 @@ func (m ModTime) Timestamp() *Timestamp {
// Multihash represents a multihash-encoded file hash (typically SHA2-256).
type Multihash []byte
// SHA256Multihash returns the first of entry's hashes that is a SHA-256
// multihash, or nil if none is. Loading a manifest refuses an entry that
// does not list exactly one, so for a loaded entry it is the only one.
func SHA256Multihash(entry *MFFilePath) Multihash {
for _, h := range entry.GetHashes() {
if isSHA256Multihash(h.GetMultiHash()) {
return h.GetMultiHash()
}
}
return nil
}
// isSHA256Multihash reports whether hash is a SHA-256 multihash: the
// multihash code for SHA-256, then a 32-byte digest.
func isSHA256Multihash(hash []byte) bool {
decoded, err := multihash.Decode(hash)
return err == nil && decoded.Code == multihash.SHA2_256 &&
len(decoded.Digest) == sha256.Size
}
// FileHashProgress reports progress during file hashing.
type FileHashProgress struct {
BytesRead FileSize // Total bytes read so far for the current file
@@ -241,8 +262,7 @@ func (b *Builder) FileCount() int {
// This is useful when the hash is already known (e.g., from an existing manifest).
// Only mode's permission bits (mode.Perm()) are recorded; 0 records none.
// Returns an error if path is invalid or already added, size is negative,
// or hash is not a multihash with a digest of at least 32 bytes, as long
// as SHA-256's.
// or hash is not a SHA-256 multihash, the hash every entry must carry.
func (b *Builder) AddFileWithHash(
path RelFilePath,
size FileSize,
@@ -259,19 +279,8 @@ func (b *Builder) AddFileWithHash(
return errNegativeSize
}
decoded, err := multihash.Decode(hash)
if err != nil {
return fmt.Errorf("%w: %w", errHashNotMultihash, err)
}
// The reader's limit on decoding cost (maxDecodedGrowth) assumes every
// hash is at least as long as a SHA-256 multihash, so a manifest of
// shorter ones could fail to load.
if len(decoded.Digest) < sha256.Size {
return fmt.Errorf(
"%w: %d bytes, at least %d needed",
errHashTooShort, len(decoded.Digest), sha256.Size,
)
if !isSHA256Multihash(hash) {
return errHashNotSHA256
}
entry := &MFFilePath{
+13 -10
View File
@@ -5,6 +5,7 @@ import (
"bytes"
"context"
"crypto/sha256"
"crypto/sha512"
"fmt"
"path/filepath"
"strings"
@@ -94,23 +95,25 @@ func TestBuilderAddFileWithHashRejectsBadHashes(t *testing.T) {
sha1Hash, err := multihash.Encode(make([]byte, 20), multihash.SHA1)
require.NoError(t, err)
sha512Hash, err := multihash.Encode(make([]byte, sha512.Size), multihash.SHA2_512)
require.NoError(t, err)
truncatedHash, err := multihash.Encode(make([]byte, sha256.Size-1), multihash.SHA2_256)
require.NoError(t, err)
tests := []struct {
name string
hash Multihash
want error
}{
{"nil hash", nil, errHashNotMultihash},
{"empty hash", []byte{}, errHashNotMultihash},
{"one-byte hash", []byte{0x12}, errHashNotMultihash},
{"nil hash", nil},
{"empty hash", []byte{}},
{"one-byte hash", []byte{0x12}},
// A SHA-256 code and 32-byte length, then only two bytes of digest.
{"malformed multihash", []byte{0x12, 0x20, 0x01, 0x02}, errHashNotMultihash},
// A valid multihash, but its 20-byte SHA-1 digest is too short.
{"SHA-1 multihash", sha1Hash, errHashTooShort},
// A valid multihash whose 31-byte digest is one byte short.
{"31-byte digest", truncatedHash, errHashTooShort},
{"malformed multihash", []byte{0x12, 0x20, 0x01, 0x02}},
{"SHA-1 multihash", sha1Hash},
{"SHA-512 multihash", sha512Hash},
// A SHA-256 code with a 31-byte digest.
{"31-byte digest", truncatedHash},
}
for _, tt := range tests {
@@ -119,7 +122,7 @@ func TestBuilderAddFileWithHashRejectsBadHashes(t *testing.T) {
b := NewBuilder()
err := b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), 0, tt.hash)
require.ErrorIs(t, err, tt.want)
require.ErrorIs(t, err, errHashNotSHA256)
assert.Equal(t, 0, b.FileCount())
})
}
+35 -15
View File
@@ -80,7 +80,8 @@ type Checker struct {
// manifestPaths is a set of paths in the manifest for quick lookup
manifestPaths map[RelFilePath]struct{}
// manifestInfo is the manifest file, which FindExtraFiles leaves out,
// matched with os.SameFile.
// matched with os.SameFile. It is nil for a manifest read from
// CheckerOptions.ManifestReader.
manifestInfo os.FileInfo
// signature info from the manifest
signature []byte
@@ -90,8 +91,13 @@ type Checker struct {
// CheckerOptions configures a Checker.
type CheckerOptions struct {
// ManifestPath is the manifest file to check against (required).
// ManifestPath is the manifest file to check against. It is required
// unless ManifestReader is given.
ManifestPath string
// ManifestReader, if not nil, is read for the manifest to check against
// in place of ManifestPath, such as a manifest downloaded into memory.
// FindExtraFiles then has no manifest file to leave out.
ManifestReader io.Reader
// BasePath is the directory relative to which manifest paths are
// resolved (required).
BasePath string
@@ -100,9 +106,10 @@ type CheckerOptions struct {
}
// NewChecker creates a new Checker with the given options. It returns an
// error if opts is nil or either path is empty.
// error if opts is nil, the base path is empty, or neither a manifest path
// nor a manifest reader is given.
func NewChecker(opts *CheckerOptions) (*Checker, error) {
if opts == nil || opts.ManifestPath == "" {
if opts == nil || (opts.ManifestPath == "" && opts.ManifestReader == nil) {
return nil, errManifestPathEmpty
}
@@ -115,12 +122,30 @@ func NewChecker(opts *CheckerOptions) (*Checker, error) {
fs = afero.NewOsFs()
}
m, err := NewManifestFromFile(&ManifestFromFileOptions{
Path: opts.ManifestPath,
Fs: fs,
})
if err != nil {
return nil, err
var (
m *manifest
manifestInfo os.FileInfo
err error
)
if opts.ManifestReader != nil {
m, err = NewManifestFromReader(opts.ManifestReader)
if err != nil {
return nil, err
}
} else {
m, err = NewManifestFromFile(&ManifestFromFileOptions{
Path: opts.ManifestPath,
Fs: fs,
})
if err != nil {
return nil, err
}
manifestInfo, err = fs.Stat(opts.ManifestPath)
if err != nil {
return nil, err
}
}
abs, err := filepath.Abs(opts.BasePath)
@@ -135,11 +160,6 @@ func NewChecker(opts *CheckerOptions) (*Checker, error) {
manifestPaths[RelFilePath(f.GetPath())] = struct{}{}
}
manifestInfo, err := fs.Stat(opts.ManifestPath)
if err != nil {
return nil, err
}
return &Checker{
basePath: AbsFilePath(abs),
files: files,
+32
View File
@@ -189,6 +189,38 @@ func TestNewCheckerNilFsUsesOsFs(t *testing.T) {
assert.Equal(t, StatusOK, (<-results).Status)
}
// TestNewCheckerReadsManifestFromReader gives NewChecker the manifest as a
// reader and no manifest path, on a filesystem that holds only the files it
// lists: it checks those files and finds nothing unlisted.
func TestNewCheckerReadsManifestFromReader(t *testing.T) {
t.Parallel()
files := map[string][]byte{testFile1: []byte("hello")}
manifestFs := afero.NewMemMapFs()
createTestManifest(t, manifestFs, testManifestPath, files)
manifestData, err := afero.ReadFile(manifestFs, testManifestPath)
require.NoError(t, err)
fs := afero.NewMemMapFs()
createFilesOnDisk(t, fs, files)
chk, err := NewChecker(&CheckerOptions{
ManifestReader: bytes.NewReader(manifestData),
BasePath: testDataDir,
Fs: fs,
})
require.NoError(t, err)
results := make(chan Result, 1)
require.NoError(t, chk.Check(context.Background(), results, nil))
assert.Equal(t, Result{Path: testFile1, Status: StatusOK}, <-results)
extras := make(chan Result, len(files))
require.NoError(t, chk.FindExtraFiles(context.Background(), extras))
assert.Empty(t, extras)
}
func TestCheckerFileCountAndTotalBytes(t *testing.T) {
t.Parallel()
+39 -5
View File
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"io"
"os"
"strings"
"github.com/klauspost/compress/zstd"
@@ -27,7 +28,9 @@ var (
errUUIDMismatch = errors.New("outer and inner UUID mismatch")
errInvalidFileFormat = errors.New("invalid file format")
errInvalidManifestPath = errors.New("invalid file entry")
errEntryMissingHash = errors.New("file entry has no hash")
errEntryMissingHash = errors.New("file entry has no SHA-256 hash")
errEntryExtraSHA256Hash = errors.New("file entry has more than one SHA-256 hash")
errModeOutOfRange = errors.New("file entry has a mode above 0777")
errDecodedTooLarge = errors.New("too much memory needed")
errSignerNotSigningKey = errors.New(
"signer is not the fingerprint of the key that made the signature")
@@ -236,6 +239,30 @@ func forEachBytesField(
return nil
}
// validateEntryHashes refuses, naming its path, an entry that does not list
// exactly one SHA-256 multihash; hashes of other algorithms beside it are
// allowed. check and fetch compute only SHA-256, so with none they would
// have nothing to check the file against, and with two check would pass a
// file that matches either.
func validateEntryHashes(entry *MFFilePath) error {
sha256Hashes := 0
for _, h := range entry.GetHashes() {
if isSHA256Multihash(h.GetMultiHash()) {
sha256Hashes++
}
}
switch {
case sha256Hashes == 0:
return fmt.Errorf("%w: %q", errEntryMissingHash, entry.GetPath())
case sha256Hashes > 1:
return fmt.Errorf("%w: %q", errEntryExtraSHA256Hash, entry.GetPath())
default:
return nil
}
}
func (m *manifest) deserializeInner() error {
err := m.validateOuterHeader()
if err != nil {
@@ -294,8 +321,10 @@ func (m *manifest) deserializeInner() error {
// untrusted .mf. Reject loudly on the first offender rather than
// dropping entries, which would let a hostile manifest hide files from a
// check. A path listed twice is refused too: check would check the one
// file against both entries. So is an entry with no hash: there is
// nothing to check the file against.
// file against both entries. So is an entry that does not list exactly
// one SHA-256 hash, and one that records a mode above 0777, such as one
// with setuid: a mode holds the permission bits alone, which are all
// check compares and fetch sets.
seen := make(map[string]bool, len(m.pbInner.GetFiles()))
for _, f := range m.pbInner.GetFiles() {
@@ -304,8 +333,13 @@ func (m *manifest) deserializeInner() error {
return fmt.Errorf("%w: %w", errInvalidManifestPath, err)
}
if len(f.GetHashes()) == 0 {
return fmt.Errorf("%w: %q", errEntryMissingHash, f.GetPath())
err = validateEntryHashes(f)
if err != nil {
return err
}
if f.GetMode() > uint32(os.ModePerm) {
return fmt.Errorf("%w: %q (%#o)", errModeOutOfRange, f.GetPath(), f.GetMode())
}
if seen[f.GetPath()] {
+151 -25
View File
@@ -5,8 +5,11 @@ import (
"bytes"
"context"
"crypto/sha256"
"crypto/sha512"
"fmt"
"os"
"strconv"
"strings"
"testing"
"time"
"uuid"
@@ -167,40 +170,154 @@ func TestDeserializeRefusesPathListedTwice(t *testing.T) {
}
}
// A manifest holding a file entry with no hash is refused as it is loaded,
// naming the path: there is nothing to check the file against.
func TestDeserializeRefusesEntryWithNoHash(t *testing.T) {
// A file entry must list exactly one SHA-256 hash, and may list hashes of
// other algorithms beside it. An entry with none, even one that lists a
// SHA-512 hash, is refused as it is loaded, and so is an entry with two;
// each refusal names the path. An entry listing a SHA-512 hash and then a
// SHA-256 one loads with both.
func TestDeserializeRequiresExactlyOneSHA256Hash(t *testing.T) {
t.Parallel()
id := uuid.NewV4()
inner, err := proto.Marshal(&MFFile{
Version: MFFile_VERSION_ONE,
Files: []*MFFilePath{{Path: "x", Size: 1}},
Uuid: id[:],
})
sha256Hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
require.NoError(t, err)
_, err = NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
require.ErrorIs(t, err, errEntryMissingHash)
require.EqualError(t, err, `file entry has no hash: "x"`)
otherSHA256Hash, err := multihash.Encode(
bytes.Repeat([]byte{1}, sha256.Size), multihash.SHA2_256)
require.NoError(t, err)
sha512Hash, err := multihash.Encode(make([]byte, sha512.Size), multihash.SHA2_512)
require.NoError(t, err)
tests := []struct {
name string
hashes [][]byte
want error // nil for an entry that loads
}{
{"no hash", nil, errEntryMissingHash},
{"SHA-512 hash only", [][]byte{sha512Hash}, errEntryMissingHash},
{
"two SHA-256 hashes",
[][]byte{sha256Hash, otherSHA256Hash},
errEntryExtraSHA256Hash,
},
{"SHA-512 hash then SHA-256 hash", [][]byte{sha512Hash, sha256Hash}, nil},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
entry := &MFFilePath{Path: "x", Size: 1}
for _, h := range tt.hashes {
entry.Hashes = append(entry.Hashes, &MFFileChecksum{MultiHash: h})
}
id := uuid.NewV4()
inner, err := proto.Marshal(&MFFile{
Version: MFFile_VERSION_ONE,
Files: []*MFFilePath{entry},
Uuid: id[:],
})
require.NoError(t, err)
m, err := NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
if tt.want == nil {
require.NoError(t, err)
require.Len(t, m.Files(), 1)
assert.Len(t, m.Files()[0].GetHashes(), len(tt.hashes))
return
}
require.ErrorIs(t, err, tt.want)
assert.Contains(t, err.Error(), `"x"`)
})
}
}
// Entries of a path, an empty hash, an empty MIME type and empty modification
// and change times are counted at 432 bytes each (176 + 112 + 16 + 64 + 64)
// and take 16 bytes plus the path to encode. A 37-character path makes that
// 53 bytes, about 8.2 times: refused, and leaving any one of the five
// uncounted, even the MIME type, brings it under 8. A 39-character path makes
// it 55 bytes, about 7.9 times: loaded. Each entry's path is its number,
// padded with zeros to that length, since a manifest lists a path only once.
// A file entry that records a mode with more than the permission bits is
// refused as it is loaded, naming the path and the mode: once with setuid as
// Unix writes it (04755) and once as Go keeps it in os.FileMode. An entry
// recording 0777 loads.
func TestDeserializeRefusesModeOutsidePermissionBits(t *testing.T) {
t.Parallel()
hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
require.NoError(t, err)
tests := []struct {
name string
mode uint32
want string // "" for an entry that loads
}{
{
"setuid as Unix writes it",
0o4755,
`file entry has a mode above 0777: "x" (04755)`,
},
{
"setuid as Go keeps it",
uint32(os.ModeSetuid | 0o755),
`file entry has a mode above 0777: "x" (040000755)`,
},
{"permission bits only", 0o777, ""},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
id := uuid.NewV4()
inner, err := proto.Marshal(&MFFile{
Version: MFFile_VERSION_ONE,
Files: []*MFFilePath{{
Path: "x",
Size: 1,
Hashes: []*MFFileChecksum{{MultiHash: hash}},
Mode: tt.mode,
}},
Uuid: id[:],
})
require.NoError(t, err)
m, err := NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
if tt.want == "" {
require.NoError(t, err)
require.Len(t, m.Files(), 1)
assert.Equal(t, tt.mode, m.Files()[0].GetMode())
return
}
require.ErrorIs(t, err, errModeOutOfRange)
require.EqualError(t, err, tt.want)
})
}
}
// Entries of a path, a SHA-256 hash, an empty MIME type and empty
// modification and change times are counted at 432 bytes each (176 + 112 +
// 16 + 64 + 64) and take 52 bytes plus the path to encode. A one-character
// path makes that 53 bytes, about 8.2 times: refused, and leaving any one of
// the five uncounted, even the MIME type, brings it under 8. A two-character
// path makes it 54 bytes, 8 times exactly: loaded. There is one entry for
// each base-36 digit, and each entry's path is its number in base 36, padded
// with zeros to that length, since a manifest lists a path only once.
func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
t.Parallel()
hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
require.NoError(t, err)
checksum, err := proto.Marshal(&MFFileChecksum{MultiHash: hash})
require.NoError(t, err)
tests := []struct {
pathLen int
refused bool
}{
{37, true},
{39, false},
{1, true},
{2, false},
}
for _, tt := range tests {
@@ -213,11 +330,14 @@ func TestDeserializeRefusesEntriesThatDecodeTooLarge(t *testing.T) {
inner = protowire.AppendTag(inner, 102, protowire.BytesType) // MFFile.uuid
inner = protowire.AppendBytes(inner, id[:])
for i := range 1000 {
for i := range 36 {
path := strconv.FormatInt(int64(i), 36)
path = strings.Repeat("0", tt.pathLen-len(path)) + path
entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
entry = protowire.AppendString(entry, fmt.Sprintf("%0*d", tt.pathLen, i))
entry = protowire.AppendString(entry, path)
entry = protowire.AppendTag(entry, 3, protowire.BytesType) // MFFilePath.hashes
entry = protowire.AppendBytes(entry, nil)
entry = protowire.AppendBytes(entry, checksum)
entry = protowire.AppendTag(entry, 301, protowire.BytesType) // MFFilePath.mimeType
entry = protowire.AppendBytes(entry, nil)
entry = protowire.AppendTag(entry, 302, protowire.BytesType) // MFFilePath.mtime
@@ -247,10 +367,16 @@ func TestDeserializeDropsUnknownFields(t *testing.T) {
unknown := protowire.AppendTag(nil, 99, protowire.BytesType) // in no message
unknown = protowire.AppendBytes(unknown, []byte("not known"))
hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
require.NoError(t, err)
checksum, err := proto.Marshal(&MFFileChecksum{MultiHash: hash})
require.NoError(t, err)
entry := protowire.AppendTag(nil, 1, protowire.BytesType) // MFFilePath.path
entry = protowire.AppendString(entry, "a")
entry = protowire.AppendTag(entry, 3, protowire.BytesType) // MFFilePath.hashes
entry = protowire.AppendBytes(entry, nil)
entry = protowire.AppendBytes(entry, checksum)
entry = append(entry, unknown...)
id := uuid.NewV4()