Author SHA1 Message Date
sneak a10c588152 Refuse to freshen a signed manifest without --sign-key (closes #191)
check / check (push) Waiting to run
freshen built the new manifest from its flags alone, so a signed
manifest freshened without --sign-key came out unsigned while the log
said its signature verified. freshen now stops after loading such a
manifest, before scanning, with an error naming the signer's
fingerprint and --sign-key, and leaves the manifest as it was. With
--sign-key it rewrites and signs as before.

The library gains Signer() on the loaded manifest: the signing key's
fingerprint, or nil when the manifest is unsigned, as Checker.Signer
gives it.

Model: opus-5-5
2026-10-13 15:15:13 +00:00
10 changed files with 118 additions and 253 deletions
+3
View File
@@ -300,6 +300,9 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
- leaves out hidden files unless given `--include-dotfiles`, and symlinks
unless given `--follow-symlinks`, which lists each symlink to a regular
file under its own name with the contents of the file it points to
- refuses a signed manifest, leaving it as it was and naming the fingerprint
of the key that signed it, unless `--sign-key` or `MFER_SIGN_KEY` names a
key to sign the new one with
- `mfer gen --sign-key key.asc` / `mfer freshen --sign-key key.asc`
- signs the manifest with the OpenPGP secret key in `key.asc`, armored or
binary, as `gpg --export-secret-keys` writes it; `MFER_SIGN_KEY` names the
+9 -20
View File
@@ -75,15 +75,15 @@ 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` | At least one hash required |
| `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
@@ -174,17 +174,6 @@ also refuses a manifest whose `signingPubKey` holds a DSA key or subkey, or any
secret key or subkey, since checking the self-signatures of a DSA key or the
numbers of a secret key can take hours when those numbers are very large.
A signature is good only if the key that made it was valid at the signature's
creation time. The primary key is judged by the newest self-signature on its
primary user ID, and a subkey by its newest binding signature. By the
signature's creation time the primary key and that self-signature must have been
made and neither may have expired, and when a subkey made the signature the same
holds of the subkey and its binding signature. A signature made before that
self-signature or binding signature is not good, even if an older one made the
key valid at that time. A key that has expired since does not make the signature
bad, because a manifest outlives its key. A signature past its own expiry time
is not good, whether or not the key has expired too.
## Deterministic Serialization
By default, manifests are generated deterministically:
+15 -14
View File
@@ -28,15 +28,13 @@ const (
scanProgressInterval = 100
)
var (
// errEntryMissingMtime indicates a manifest entry that carries no
// modification time where one is required to carry it forward unchanged.
errEntryMissingMtime = errors.New("manifest entry has no mtime")
// errEntryMissingMtime indicates a manifest entry that carries no
// modification time where one is required to carry it forward unchanged.
var 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")
)
// errSignKeyNeeded indicates a signed manifest given to freshen with no
// key to sign the rewritten one.
var errSignKeyNeeded = errors.New("give --sign-key to keep it signed")
// FreshenStatus contains progress information for the freshen operation.
type FreshenStatus struct {
@@ -481,9 +479,10 @@ func runFreshenHash(
}
// loadExistingEntries loads the manifest and indexes its file entries
// by path.
// by path. It refuses a signed manifest when no --sign-key is given, as
// the rewritten manifest would be unsigned.
func (mfa *CLIApp) loadExistingEntries(
manifestPath string,
cmd *cli.Command, manifestPath string,
) (map[string]*mfer.MFFilePath, error) {
log.Infof("loading manifest from %s", manifestPath)
@@ -496,6 +495,10 @@ func (mfa *CLIApp) loadExistingEntries(
return nil, fmt.Errorf("load manifest: %w", err)
}
if signer := manifest.Signer(); signer != nil && cmd.String("sign-key") == "" {
return nil, fmt.Errorf("manifest is signed by %s: %w", signer, errSignKeyNeeded)
}
existingFiles := manifest.Files()
log.Infof("manifest contains %d files", len(existingFiles))
@@ -526,7 +529,7 @@ func (mfa *CLIApp) freshenManifestOperation(
return err
}
existingByPath, err := mfa.loadExistingEntries(manifestPath)
existingByPath, err := mfa.loadExistingEntries(cmd, manifestPath)
if err != nil {
return err
}
@@ -650,8 +653,6 @@ 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.
func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
mtime, ok := entryMtime(entry)
if !ok {
@@ -659,7 +660,7 @@ func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
}
if len(entry.GetHashes()) == 0 {
return fmt.Errorf("%w: %s", errEntryMissingHash, entry.GetPath())
return nil
}
err := b.AddFileWithHash(mfer.RelFilePath(entry.GetPath()),
-18
View File
@@ -471,24 +471,6 @@ 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.
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.
+50
View File
@@ -73,6 +73,56 @@ func TestGenAndFreshenSignWithKeyFile(t *testing.T) {
}
}
// TestFreshenSignedManifestNeedsSignKey runs freshen on a signed manifest
// after a file is added. Without --sign-key it must fail, naming the
// signer and --sign-key, and leave the manifest as it was; with --sign-key
// it must rewrite the manifest, signed.
func TestFreshenSignedManifestNeedsSignKey(t *testing.T) {
t.Setenv(envSignKeyPassphrase, "")
secretKey, fingerprint := testSecretKey(t, nil, nil)
fs := afero.NewOsFs()
keyFile := filepath.Join(t.TempDir(), "key.asc")
root := t.TempDir()
manifestPath := filepath.Join(root, defaultManifestName)
require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600))
writeTestFile(t, fs, filepath.Join(root, testFileTxt), "hello")
opts := testOpts([]string{
testApp, cmdGenerate, "-q", testFlagSignKey, keyFile,
"-o", manifestPath, root,
}, fs)
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
before, err := afero.ReadFile(fs, manifestPath)
require.NoError(t, err)
writeTestFile(t, fs, filepath.Join(root, "added.txt"), "added")
opts = testOpts([]string{testApp, cmdFreshen, "-q", manifestPath}, fs)
assert.Equal(t, 1, runCLI(opts))
assert.Contains(t, testStderr(t, opts), fingerprint)
assert.Contains(t, testStderr(t, opts), testFlagSignKey)
after, err := afero.ReadFile(fs, manifestPath)
require.NoError(t, err)
assert.Equal(t, before, after)
opts = testOpts([]string{
testApp, cmdFreshen, "-q", testFlagSignKey, keyFile, manifestPath,
}, fs)
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
opts = testOpts([]string{
testApp, cmdCheck, "-q",
"--" + flagRequireSignature, fingerprint, manifestPath,
}, fs)
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
assert.Len(t, manifestFiles(t, fs, manifestPath), 2)
}
// TestSignWithProtectedKeyNeedsPassphrase runs gen with a protected key,
// with MFER_SIGN_KEY_PASSPHRASE empty and no terminal to ask on. gen must
// fail, naming the variable, and write no manifest.
+1 -7
View File
@@ -27,7 +27,6 @@ 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")
errDecodedTooLarge = errors.New("too much memory needed")
errSignerNotSigningKey = errors.New(
"signer is not the fingerprint of the key that made the signature")
@@ -294,8 +293,7 @@ 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.
seen := make(map[string]bool, len(m.pbInner.GetFiles()))
for _, f := range m.pbInner.GetFiles() {
@@ -304,10 +302,6 @@ 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())
}
if seen[f.GetPath()] {
return fmt.Errorf("%w %q", errDuplicatePath, f.GetPath())
}
-20
View File
@@ -165,24 +165,6 @@ 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) {
t.Parallel()
id := uuid.NewV4()
inner, err := proto.Marshal(&MFFile{
Version: MFFile_VERSION_ONE,
Files: []*MFFilePath{{Path: "x", Size: 1}},
Uuid: id[:],
})
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"`)
}
// 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
@@ -247,8 +229,6 @@ func TestDeserializeDropsUnknownFields(t *testing.T) {
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 = append(entry, unknown...)
id := uuid.NewV4()
+11
View File
@@ -47,6 +47,17 @@ func (m *manifest) Files() []*MFFilePath {
return m.pbInner.GetFiles()
}
// Signer returns the fingerprint of the key that made a loaded manifest's
// signature, which loading the manifest checked, or nil if the manifest is
// not signed.
func (m *manifest) Signer() []byte {
if len(m.pbOuter.GetSignature()) == 0 {
return nil
}
return m.pbOuter.GetSigner()
}
// signatureString generates the canonical string used for signing/verification.
// Format: MAGIC-UUID-MULTIHASH where UUID and multihash are hex-encoded.
// Requires pbOuter to be set with Uuid and Sha256 fields.
+7 -36
View File
@@ -8,7 +8,6 @@ import (
"io"
"slices"
"strings"
"time"
"github.com/ProtonMail/go-crypto/openpgp"
"github.com/ProtonMail/go-crypto/openpgp/armor"
@@ -57,9 +56,7 @@ var (
"signature must hold exactly one signature")
errNotOneArmoredBlock = errors.New(
"must be exactly one armored block and nothing else")
errMalformedArmor = errors.New("armor is malformed")
errKeyNotValidWhenSigned = errors.New(
"signature was made when its key was not valid")
errMalformedArmor = errors.New("armor is malformed")
)
// SigningOptions holds the key a manifest is signed with.
@@ -245,11 +242,10 @@ func fingerprint(key *openpgp.Entity) string {
return strings.ToUpper(hex.EncodeToString(key.PrimaryKey.Fingerprint))
}
// verifySignature checks that signature is one good, unexpired OpenPGP
// signature over data, made by the one primary key in pubKey or one of its
// subkeys while that key was valid, and returns that primary key's
// fingerprint. The key may have expired since. signature and pubKey may
// each be armored or binary.
// verifySignature checks that signature is one good OpenPGP signature
// over data, made by the one primary key in pubKey or one of its subkeys,
// and returns that primary key's fingerprint. signature and pubKey may each
// be armored or binary.
func verifySignature(data, signature, pubKey []byte) (string, error) {
key, err := readOneKey(pubKey, "embedded public key block", true)
if err != nil {
@@ -270,39 +266,14 @@ func verifySignature(data, signature, pubKey []byte) (string, error) {
return "", fmt.Errorf("%w, found %d", errNotOneSignature, sigs)
}
sig, _, err := openpgp.VerifyDetachedSignature(openpgp.EntityList{key},
_, err = openpgp.CheckDetachedSignature(openpgp.EntityList{key},
bytes.NewReader(data), bytes.NewReader(sigData), nil)
// A manifest outlives its signing key, so a signature by a key that
// has expired since is still good, unless the signature has expired
// itself. The library does not check that once it finds the key
// expired.
if errors.Is(err, pgperrors.ErrKeyExpired) && sig.SigExpired(time.Now()) {
err = pgperrors.ErrSignatureExpired
}
// has expired since is still good.
if err != nil && !errors.Is(err, pgperrors.ErrKeyExpired) {
return "", fmt.Errorf("verify signature: %w", err)
}
// The library checks whether the key is valid at the time its config
// gives, which above is now. Verifying again as of the signature's
// creation time refuses a signature made before the key or subkey that
// made it was made, after it expired, or after its self-signature or
// binding signature expired. The library judges the primary key by the
// newest self-signature on its primary user ID and a subkey by its
// newest binding signature, so this also refuses a signature made
// before either of those, even if an older one was valid then.
signedAt := &packet.Config{
Time: func() time.Time { return sig.CreationTime },
}
_, _, err = openpgp.VerifyDetachedSignature(openpgp.EntityList{key},
bytes.NewReader(data), bytes.NewReader(sigData), signedAt)
if err != nil {
return "", fmt.Errorf("verify signature: %w: %w",
errKeyNotValidWhenSigned, err)
}
return fingerprint(key), nil
}
+22 -138
View File
@@ -4,7 +4,6 @@ package mfer
import (
"bytes"
"context"
"crypto"
"crypto/dsa" //nolint:staticcheck // SA1019: tests need a DSA key to refuse
"io"
"math/big"
@@ -19,7 +18,6 @@ import (
"github.com/ProtonMail/go-crypto/openpgp"
"github.com/ProtonMail/go-crypto/openpgp/armor"
"github.com/ProtonMail/go-crypto/openpgp/elgamal"
pgperrors "github.com/ProtonMail/go-crypto/openpgp/errors"
"github.com/ProtonMail/go-crypto/openpgp/packet"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
@@ -177,6 +175,25 @@ func TestBuilderWithSigning(t *testing.T) {
"-----BEGIN PGP PUBLIC KEY BLOCK-----")
}
// TestManifestSigner checks that Signer gives the signing key's
// fingerprint for a signed manifest, and nil once the signature is taken
// out, although the signer field is still there.
func TestManifestSigner(t *testing.T) {
t.Parallel()
key := newTestKey(t, nil)
data := signedTestManifest(t, &SigningOptions{SecretKey: armoredSecretKeys(t, key)})
signed, err := NewManifestFromReader(bytes.NewReader(data))
require.NoError(t, err)
assert.Equal(t, fingerprint(key), string(signed.Signer()))
unsigned, err := NewManifestFromReader(bytes.NewReader(
rewriteOuter(t, data, func(outer *MFFileOuter) { outer.Signature = nil })))
require.NoError(t, err)
assert.Nil(t, unsigned.Signer())
}
func TestScannerWithSigning(t *testing.T) {
t.Parallel()
@@ -418,11 +435,8 @@ func TestVerifySignature(t *testing.T) {
assert.Error(t, err)
}
// TestVerifySignatureKeyExpiredSince verifies two signatures made in 2020
// by a key that expired a day after it was made. A signature with no
// expiry time is still good, as is one made then by a signing subkey that
// expired a day after it was made. One that expired an hour after it was
// made is refused, although the key has expired too.
// TestVerifySignatureKeyExpiredSince verifies a signature made in 2020 by
// a key that expired a day after it was made. The signature is still good.
func TestVerifySignatureKeyExpiredSince(t *testing.T) {
t.Parallel()
@@ -434,14 +448,9 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
key := newTestKey(t, config)
data := []byte("signed in 2020")
var sig, expiringSig bytes.Buffer
var sig bytes.Buffer
require.NoError(t, openpgp.ArmoredDetachSign(&sig, key, bytes.NewReader(data), config))
require.NoError(t, openpgp.ArmoredDetachSign(&expiringSig, key,
bytes.NewReader(data), &packet.Config{
Time: config.Time,
SigLifetimeSecs: uint32(time.Hour.Seconds()),
}))
pubKey, err := armoredPublicKey(key)
require.NoError(t, err)
@@ -449,131 +458,6 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
signer, err := verifySignature(data, sig.Bytes(), pubKey)
require.NoError(t, err)
assert.Equal(t, fingerprint(key), signer)
withSubkey := newTestKey(t, &packet.Config{Time: config.Time})
require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
Algorithm: packet.PubKeyAlgoEdDSA, Time: config.Time,
KeyLifetimeSecs: config.KeyLifetimeSecs,
}))
var subkeySig bytes.Buffer
require.NoError(t, openpgp.ArmoredDetachSign(&subkeySig, withSubkey,
bytes.NewReader(data), config))
subkeyPubKey, err := armoredPublicKey(withSubkey)
require.NoError(t, err)
_, err = verifySignature(data, subkeySig.Bytes(), subkeyPubKey)
require.NoError(t, err)
_, err = verifySignature(data, expiringSig.Bytes(), pubKey)
require.ErrorIs(t, err, pgperrors.ErrSignatureExpired)
}
// signDated returns a detached signature over data by signer, dated when.
// openpgp.DetachSign signs only with a key that is valid at the time its
// config gives, so this makes the signature packet without that check.
func signDated(t *testing.T, signer *packet.PrivateKey, data []byte,
when time.Time,
) []byte {
t.Helper()
sig := &packet.Signature{
SigType: packet.SigTypeBinary,
PubKeyAlgo: signer.PubKeyAlgo,
Hash: crypto.SHA256,
CreationTime: when,
IssuerKeyId: &signer.KeyId,
}
h, err := sig.PrepareSign(nil)
require.NoError(t, err)
_, err = h.Write(data)
require.NoError(t, err)
require.NoError(t, sig.Sign(h, signer, nil))
var buf bytes.Buffer
require.NoError(t, sig.Serialize(&buf))
return buf.Bytes()
}
// TestVerifySignatureRefusesKeyNotValidWhenSigned verifies signatures
// dated when the key or subkey that made them was not valid: after the key
// expired; after the key's self-signature, or the signing subkey's binding
// signature, expired, the key or subkey having expired since as well; and
// before the key was made, also by a key made in 2100. Each must be
// refused.
func TestVerifySignatureRefusesKeyNotValidWhenSigned(t *testing.T) {
t.Parallel()
made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
twoDaysLater := made.Add(48 * time.Hour)
day := uint32((24 * time.Hour).Seconds())
clock := func(when time.Time) func() time.Time {
return func() time.Time { return when }
}
expiredAfterADay := newTestKey(t, &packet.Config{
Time: clock(made), KeyLifetimeSecs: day,
})
selfSignatureExpiredAfterADay := newTestKey(t, &packet.Config{
Time: clock(made), KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
})
withSubkey := newTestKey(t, &packet.Config{Time: clock(made)})
require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
Algorithm: packet.PubKeyAlgoEdDSA, Time: clock(made),
KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
}))
subkey := withSubkey.Subkeys[len(withSubkey.Subkeys)-1].PrivateKey
madeADayLater := newTestKey(t, &packet.Config{
Time: clock(made.Add(24 * time.Hour)),
})
madeIn2100 := newTestKey(t, &packet.Config{
Time: clock(time.Date(2100, 1, 1, 0, 0, 0, 0, time.UTC)),
})
for name, test := range map[string]struct {
key *openpgp.Entity
signer *packet.PrivateKey
when time.Time
}{
"after the key expired": {
expiredAfterADay, expiredAfterADay.PrivateKey, twoDaysLater,
},
"after the self-signature expired": {
selfSignatureExpiredAfterADay,
selfSignatureExpiredAfterADay.PrivateKey, twoDaysLater,
},
"after the binding signature expired": {
withSubkey, subkey, twoDaysLater,
},
"before the key was made": {
madeADayLater, madeADayLater.PrivateKey, made,
},
"before a key made in 2100": {
madeIn2100, madeIn2100.PrivateKey, made,
},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
data := []byte("signed in 2020")
pubKey, err := armoredPublicKey(test.key)
require.NoError(t, err)
_, err = verifySignature(data,
signDated(t, test.signer, data, test.when), pubKey)
require.ErrorIs(t, err, errKeyNotValidWhenSigned)
})
}
}
func TestManifestSignatureVerification(t *testing.T) {