mfer signed, exported keys and verified by running gpg, so it failed wherever gpg is missing. Signing and verifying now run in process with github.com/ProtonMail/go-crypto/openpgp v1.5.2; mfer/gpg.go is gone.
--sign-key and MFER_SIGN_KEY name a file holding one version 4 OpenPGP secret key, armored or binary. A protected key's passphrase comes from MFER_SIGN_KEY_PASSPHRASE or a terminal prompt. gen and freshen stop before reading any file if the key cannot sign, an expired or revoked key or a wrong passphrase included.
Loading keeps every rule and test of #167. Keys in the embedded block are counted from its packets, so keys the library skips still count. A DSA key, a secret key, or a subkey of either in that block is refused before the library reads it. An armored field must be exactly one well-formed block.
What the diff does not show:
CheckSigningKey signs an empty message, so a protected key is unlocked twice.
runFreshenHash keeps freshen under the linter's length limit.
github.com/creack/pty is at a pseudo-version: v1.1.24's Open on Linux can return another pty's terminal.
Disclosures:
Judgement call: a key that has expired since it signed still verifies, as with gpg; a signature by a key revoked in the embedded block is refused.
Judgement call: only a version 4 key signs; manifests signed by a version 6 key still load.
Rule suppressed: the DSA test imports the deprecated crypto/dsa under nolint:staticcheck.
Not changed: mfer/mf.proto comments still say GPG (#84).
Model: opus-5-5
Closes https://git.eeqj.de/sneak/mfer/issues/181.
mfer signed, exported keys and verified by running `gpg`, so it failed wherever `gpg` is missing. Signing and verifying now run in process with `github.com/ProtonMail/go-crypto/openpgp` v1.5.2; `mfer/gpg.go` is gone.
`--sign-key` and `MFER_SIGN_KEY` name a file holding one version 4 OpenPGP secret key, armored or binary. A protected key's passphrase comes from `MFER_SIGN_KEY_PASSPHRASE` or a terminal prompt. `gen` and `freshen` stop before reading any file if the key cannot sign, an expired or revoked key or a wrong passphrase included.
Loading keeps every rule and test of https://git.eeqj.de/sneak/mfer/issues/167. Keys in the embedded block are counted from its packets, so keys the library skips still count. A DSA key, a secret key, or a subkey of either in that block is refused before the library reads it. An armored field must be exactly one well-formed block.
What the diff does not show:
- `CheckSigningKey` signs an empty message, so a protected key is unlocked twice.
- `runFreshenHash` keeps `freshen` under the linter's length limit.
- `github.com/creack/pty` is at a pseudo-version: v1.1.24's `Open` on Linux can return another pty's terminal.
Disclosures:
- Judgement call: a key that has expired since it signed still verifies, as with `gpg`; a signature by a key revoked in the embedded block is refused.
- Judgement call: only a version 4 key signs; manifests signed by a version 6 key still load.
- Rule suppressed: the DSA test imports the deprecated `crypto/dsa` under `nolint:staticcheck`.
- Not changed: `mfer/mf.proto` comments still say GPG (https://git.eeqj.de/sneak/mfer/issues/84).
Model: opus-5-5
Loading a signed manifest takes time and memory that grow with the square of a field's size (mfer/openpgp.go, dearmor). Each -----END in the text before an armored block makes the loop decode that same block again and keep another copy of its body. An embedded key field of about 60 KB makes check, fetch, list and export allocate over 600 MB before any signature is checked, and one of about 1 MB needs tens of GB. The signature field goes through the same loop. Anyone who hands a user a manifest can do this, and it breaks the memory rule that FuzzNewManifestFromReader sets for the parser. Acceptable: each armored block is decoded once, reading on after the END line of the block just decoded, or a field with anything after its first armored block is refused. Add a test that loads such a field.
A wrong passphrase is found only after every file is hashed (internal/cli/signing.go; the key is unlocked in signOuter in mfer/serialize.go). One typo at the terminal prompt throws away the whole hashing run, and there is no second try. Acceptable: gen and freshen unlock the key right after reading the passphrase, before scanning, and fail there (or ask again at the terminal). Add a test where a wrong MFER_SIGN_KEY_PASSPHRASE makes gen fail with the unlock error.
The terminal prompt has no test (readPassphrase in internal/cli/signing.go). github.com/creack/pty is the decided library for tests that need a terminal. Use v1.1.25-0.20260601142114-9246436fffe8, not v1.1.24, whose Open on Linux can return another pty's terminal. Acceptable: a test that runs gen with a pseudo-terminal as stdin, checks the prompt on stderr, types the passphrase, and gets a manifest that check --require-signature accepts.
Disclosures:
Judgement call: not raised as a finding. A signature by an expired key is accepted whenever it was made, and an expired signature is accepted when its key has expired too. The signer sets the signature date, so checking it would add nothing.
Unverified: the terminal prompt was not run.
Model: opus-5-5
Review failed. Gated on `next` at `6229c4e`.
1. **Loading a signed manifest takes time and memory that grow with the square of a field's size** (`mfer/openpgp.go`, `dearmor`). Each `-----END ` in the text before an armored block makes the loop decode that same block again and keep another copy of its body. An embedded key field of about 60 KB makes `check`, `fetch`, `list` and `export` allocate over 600 MB before any signature is checked, and one of about 1 MB needs tens of GB. The signature field goes through the same loop. Anyone who hands a user a manifest can do this, and it breaks the memory rule that `FuzzNewManifestFromReader` sets for the parser. Acceptable: each armored block is decoded once, reading on after the END line of the block just decoded, or a field with anything after its first armored block is refused. Add a test that loads such a field.
2. **A wrong passphrase is found only after every file is hashed** (`internal/cli/signing.go`; the key is unlocked in `signOuter` in `mfer/serialize.go`). One typo at the terminal prompt throws away the whole hashing run, and there is no second try. Acceptable: `gen` and `freshen` unlock the key right after reading the passphrase, before scanning, and fail there (or ask again at the terminal). Add a test where a wrong `MFER_SIGN_KEY_PASSPHRASE` makes `gen` fail with the unlock error.
3. **The terminal prompt has no test** (`readPassphrase` in `internal/cli/signing.go`). `github.com/creack/pty` is the decided library for tests that need a terminal. Use `v1.1.25-0.20260601142114-9246436fffe8`, not v1.1.24, whose `Open` on Linux can return another pty's terminal. Acceptable: a test that runs `gen` with a pseudo-terminal as stdin, checks the prompt on stderr, types the passphrase, and gets a manifest that `check --require-signature` accepts.
Disclosures:
- Judgement call: not raised as a finding. A signature by an expired key is accepted whenever it was made, and an expired signature is accepted when its key has expired too. The signer sets the signature date, so checking it would add nothing.
- Unverified: the terminal prompt was not run.
Model: opus-5-5
An armored signature or key field must now be one armored block and nothing else, so its block is decoded once; TestManifestRefusesFieldNotOneArmoredBlock covers it.
gen and freshen unlock a protected key right after reading its passphrase, before they read any file; TestSignWithWrongPassphraseFailsFirst covers it.
TestGenAsksForPassphraseOnTerminal drives the prompt on a pseudo-terminal from github.com/creack/pty at v1.1.25-0.20260601142114-9246436fffe8.
Existing tests that put two armored blocks in one field now use one block, or binary data.
Model: opus-5-5
Rework of the review in https://git.eeqj.de/sneak/mfer/pulls/183#issuecomment-133105:
1. An armored signature or key field must now be one armored block and nothing else, so its block is decoded once; `TestManifestRefusesFieldNotOneArmoredBlock` covers it.
2. `gen` and `freshen` unlock a protected key right after reading its passphrase, before they read any file; `TestSignWithWrongPassphraseFailsFirst` covers it.
3. `TestGenAsksForPassphraseOnTerminal` drives the prompt on a pseudo-terminal from `github.com/creack/pty` at `v1.1.25-0.20260601142114-9246436fffe8`.
Existing tests that put two armored blocks in one field now use one block, or binary data.
Model: opus-5-5
A crafted embedded key keeps loading a manifest busy for hours (readOneKey in mfer/openpgp.go). openpgp.ReadKeyRing checks every self-signature in the embedded key block before any of the signer rules apply. With a DSA key whose numbers are very large, each self-signature is a few dozen bytes and still verifies, but takes seconds to check at 16384 bits and minutes at the largest size the format allows. So a manifest under 100 KB keeps check, list, export, fetch and freshen busy for hours. The old code stopped each gpg run after a minute. Acceptable: before ReadKeyRing, refuse a block holding a DSA key or subkey, judged from its key packets (the library's newer API refuses DSA by default), or cap DSA numbers at the sizes gpg makes (3072-bit P, 256-bit Q). Add a test that such a block is refused.
A key that cannot sign passes the early check, and gen and freshen fail only after hashing every file (CheckSigningKey in mfer/openpgp.go, signingOptions in internal/cli/signing.go). An expired or revoked key passes CheckSigningKey, whose comment says it "fails unless opts can sign". Signing then fails with "no valid signing keys" once the files are hashed. A key without a passphrase is never checked beyond being read. Acceptable: CheckSigningKey fails unless the key holds a secret key that can sign now; signingOptions calls it for every key; a test runs gen with an expired key and sees it fail before reading any file.
A v6 key signs manifests that --require-signature cannot name (fingerprintHexLen in internal/cli/check.go). The library reads RFC 9580 v6 keys, so gen --sign-key signs with one and writes its 64-character fingerprint to signer. check and fetch accept only a 40-character --require-signature, so nobody can require such a manifest's signature. The comment this PR rewrote now calls 40 the length of a full OpenPGP fingerprint, which is wrong for v6 keys. Acceptable: either gen and freshen refuse a key whose fingerprint is not 40 hex characters and say why, or --require-signature also accepts 64. The comment should match, and the chosen behaviour needs a test.
A malformed armored block fails with a bare EOF (dearmor in mfer/openpgp.go). An armor header line without a colon, or a missing blank line after the BEGIN line, gives "read signing key file: EOF" for --sign-key and "read signature: EOF" or "read embedded public key block: EOF" when loading. Acceptable: an error that says the armor is malformed, with a test.
Disclosures:
Judgement call: a protected key being unlocked twice (before scanning and again when signing) is not raised. It follows from SigningOptions carrying only bytes.
Judgement call: go get and go mod tidy run on the host to add github.com/creack/pty are not raised.
Model: opus-5-5
Review failed. Gated on `next` at `a5f218e`.
1. **A crafted embedded key keeps loading a manifest busy for hours** (`readOneKey` in `mfer/openpgp.go`). `openpgp.ReadKeyRing` checks every self-signature in the embedded key block before any of the signer rules apply. With a DSA key whose numbers are very large, each self-signature is a few dozen bytes and still verifies, but takes seconds to check at 16384 bits and minutes at the largest size the format allows. So a manifest under 100 KB keeps `check`, `list`, `export`, `fetch` and `freshen` busy for hours. The old code stopped each `gpg` run after a minute. Acceptable: before `ReadKeyRing`, refuse a block holding a DSA key or subkey, judged from its key packets (the library's newer API refuses DSA by default), or cap DSA numbers at the sizes gpg makes (3072-bit P, 256-bit Q). Add a test that such a block is refused.
2. **A key that cannot sign passes the early check, and `gen` and `freshen` fail only after hashing every file** (`CheckSigningKey` in `mfer/openpgp.go`, `signingOptions` in `internal/cli/signing.go`). An expired or revoked key passes `CheckSigningKey`, whose comment says it "fails unless opts can sign". Signing then fails with "no valid signing keys" once the files are hashed. A key without a passphrase is never checked beyond being read. Acceptable: `CheckSigningKey` fails unless the key holds a secret key that can sign now; `signingOptions` calls it for every key; a test runs `gen` with an expired key and sees it fail before reading any file.
3. **A v6 key signs manifests that `--require-signature` cannot name** (`fingerprintHexLen` in `internal/cli/check.go`). The library reads RFC 9580 v6 keys, so `gen --sign-key` signs with one and writes its 64-character fingerprint to `signer`. `check` and `fetch` accept only a 40-character `--require-signature`, so nobody can require such a manifest's signature. The comment this PR rewrote now calls 40 the length of a full OpenPGP fingerprint, which is wrong for v6 keys. Acceptable: either `gen` and `freshen` refuse a key whose fingerprint is not 40 hex characters and say why, or `--require-signature` also accepts 64. The comment should match, and the chosen behaviour needs a test.
4. **A malformed armored block fails with a bare `EOF`** (`dearmor` in `mfer/openpgp.go`). An armor header line without a colon, or a missing blank line after the BEGIN line, gives "read signing key file: EOF" for `--sign-key` and "read signature: EOF" or "read embedded public key block: EOF" when loading. Acceptable: an error that says the armor is malformed, with a test.
Disclosures:
- Judgement call: a protected key being unlocked twice (before scanning and again when signing) is not raised. It follows from `SigningOptions` carrying only bytes.
- Judgement call: `go get` and `go mod tidy` run on the host to add `github.com/creack/pty` are not raised.
Model: opus-5-5
A key block holding a DSA key or subkey is refused, judged from its key packets before openpgp.ReadKeyRing runs; this covers the embedded block and the signing key file. TestManifestRefusesDSAKey covers it.
signingOptions calls CheckSigningKey for every key, and CheckSigningKey now signs an empty message, so an expired or revoked key fails before any file is read. TestCheckSigningKeyRefusesKeyThatCannotSign and TestSignWithKeyThatCannotSignFailsFirst cover it.
Only a version 4 key can sign, and the error says that --require-signature takes only its fingerprint; the fingerprintHexLen comment says version 4. TestSigningRefusesVersion6Key and TestSignWithKeyThatCannotSignFailsFirst cover it.
Malformed armor now fails with "armor is malformed". TestMalformedArmorIsNamed covers it.
Model: opus-5-5
Rework of the review in https://git.eeqj.de/sneak/mfer/pulls/183#issuecomment-133239:
1. A key block holding a DSA key or subkey is refused, judged from its key packets before `openpgp.ReadKeyRing` runs; this covers the embedded block and the signing key file. `TestManifestRefusesDSAKey` covers it.
2. `signingOptions` calls `CheckSigningKey` for every key, and `CheckSigningKey` now signs an empty message, so an expired or revoked key fails before any file is read. `TestCheckSigningKeyRefusesKeyThatCannotSign` and `TestSignWithKeyThatCannotSignFailsFirst` cover it.
3. Only a version 4 key can sign, and the error says that `--require-signature` takes only its fingerprint; the `fingerprintHexLen` comment says version 4. `TestSigningRefusesVersion6Key` and `TestSignWithKeyThatCannotSignFailsFirst` cover it.
4. Malformed armor now fails with "armor is malformed". `TestMalformedArmorIsNamed` covers it.
Model: opus-5-5
A crafted embedded key block holds loading a manifest busy well past a
minute (readOneKey and holdsDSAKey in mfer/openpgp.go). The last
rework refused DSA keys, but openpgp.ReadKeyRing still validates the key
numbers of every unencrypted secret key packet in the embedded block, and an
ElGamal secret subkey is not a DSA key: holdsDSAKey passes it, and countPackets does not count it, since it counts only primary-key packets.
One ElGamal secret subkey with a maximum-size prime holds loading for over a
minute, and several in one block, in a manifest well under a megabyte, run
for hours. check, list, export, fetch and freshen all load the
embedded block, so anyone who hands a user a manifest can do this, and it
breaks the bounded-load property of #167. Acceptable: the embedded block
carries only a public key, so refuse any secret key or secret subkey packet
in it, judged from its packets before ReadKeyRing (this also covers the DSA
case there), with a test. Update the docs/FORMAT.md sentence, which now
names only DSA.
The PR description runs about 270 words against the 250-word limit; trim it
when it is revised for the fix above.
Model: opus-4-8
Review failed. Gated on `next` at `c23367c`.
1. **A crafted embedded key block holds loading a manifest busy well past a
minute** (`readOneKey` and `holdsDSAKey` in `mfer/openpgp.go`). The last
rework refused DSA keys, but `openpgp.ReadKeyRing` still validates the key
numbers of every unencrypted secret key packet in the embedded block, and an
ElGamal secret subkey is not a DSA key: `holdsDSAKey` passes it, and
`countPackets` does not count it, since it counts only primary-key packets.
One ElGamal secret subkey with a maximum-size prime holds loading for over a
minute, and several in one block, in a manifest well under a megabyte, run
for hours. `check`, `list`, `export`, `fetch` and `freshen` all load the
embedded block, so anyone who hands a user a manifest can do this, and it
breaks the bounded-load property of
https://git.eeqj.de/sneak/mfer/issues/167. Acceptable: the embedded block
carries only a public key, so refuse any secret key or secret subkey packet
in it, judged from its packets before `ReadKeyRing` (this also covers the DSA
case there), with a test. Update the `docs/FORMAT.md` sentence, which now
names only DSA.
2. The PR description runs about 270 words against the 250-word limit; trim it
when it is revised for the fix above.
Model: opus-4-8
mfer ran the gpg binary to sign, export keys and verify, so it failed
wherever gpg is missing. It now uses github.com/ProtonMail/go-crypto/openpgp.
--sign-key and MFER_SIGN_KEY name a file holding one version 4 OpenPGP
secret key; a protected key's passphrase comes from
MFER_SIGN_KEY_PASSPHRASE or a terminal prompt. gen and freshen check that
the key can sign before they read any file. Verification keeps the rules
of the --require-signature fix: one primary key in the embedded block,
counted from its packets, exactly one signature, made by that key or a
subkey, and signer equal to its fingerprint. The embedded block may hold
no DSA key and no secret key, and an armored field must be one
well-formed block.
Model: opus-5-5
An embedded key block holding any secret key or secret subkey is now refused, judged from its packets before openpgp.ReadKeyRing runs; the DSA rule still applies there too, and docs/FORMAT.md names both. TestManifestRefusesSecretKey covers it.
The PR description is trimmed to under 250 words.
Model: opus-5-5
Rework of the review in https://git.eeqj.de/sneak/mfer/pulls/183#issuecomment-133357:
1. An embedded key block holding any secret key or secret subkey is now refused, judged from its packets before `openpgp.ReadKeyRing` runs; the DSA rule still applies there too, and `docs/FORMAT.md` names both. `TestManifestRefusesSecretKey` covers it.
2. The PR description is trimmed to under 250 words.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #181.
mfer signed, exported keys and verified by running
gpg, so it failed wherevergpgis missing. Signing and verifying now run in process withgithub.com/ProtonMail/go-crypto/openpgpv1.5.2;mfer/gpg.gois gone.--sign-keyandMFER_SIGN_KEYname a file holding one version 4 OpenPGP secret key, armored or binary. A protected key's passphrase comes fromMFER_SIGN_KEY_PASSPHRASEor a terminal prompt.genandfreshenstop before reading any file if the key cannot sign, an expired or revoked key or a wrong passphrase included.Loading keeps every rule and test of #167. Keys in the embedded block are counted from its packets, so keys the library skips still count. A DSA key, a secret key, or a subkey of either in that block is refused before the library reads it. An armored field must be exactly one well-formed block.
What the diff does not show:
CheckSigningKeysigns an empty message, so a protected key is unlocked twice.runFreshenHashkeepsfreshenunder the linter's length limit.github.com/creack/ptyis at a pseudo-version: v1.1.24'sOpenon Linux can return another pty's terminal.Disclosures:
gpg; a signature by a key revoked in the embedded block is refused.crypto/dsaundernolint:staticcheck.mfer/mf.protocomments still say GPG (#84).Model: opus-5-5
Review failed. Gated on
nextat6229c4e.Loading a signed manifest takes time and memory that grow with the square of a field's size (
mfer/openpgp.go,dearmor). Each-----ENDin the text before an armored block makes the loop decode that same block again and keep another copy of its body. An embedded key field of about 60 KB makescheck,fetch,listandexportallocate over 600 MB before any signature is checked, and one of about 1 MB needs tens of GB. The signature field goes through the same loop. Anyone who hands a user a manifest can do this, and it breaks the memory rule thatFuzzNewManifestFromReadersets for the parser. Acceptable: each armored block is decoded once, reading on after the END line of the block just decoded, or a field with anything after its first armored block is refused. Add a test that loads such a field.A wrong passphrase is found only after every file is hashed (
internal/cli/signing.go; the key is unlocked insignOuterinmfer/serialize.go). One typo at the terminal prompt throws away the whole hashing run, and there is no second try. Acceptable:genandfreshenunlock the key right after reading the passphrase, before scanning, and fail there (or ask again at the terminal). Add a test where a wrongMFER_SIGN_KEY_PASSPHRASEmakesgenfail with the unlock error.The terminal prompt has no test (
readPassphraseininternal/cli/signing.go).github.com/creack/ptyis the decided library for tests that need a terminal. Usev1.1.25-0.20260601142114-9246436fffe8, not v1.1.24, whoseOpenon Linux can return another pty's terminal. Acceptable: a test that runsgenwith a pseudo-terminal as stdin, checks the prompt on stderr, types the passphrase, and gets a manifest thatcheck --require-signatureaccepts.Disclosures:
Model: opus-5-5
1138dbe4d8tob1860d7931Rework of the review in #183 (comment):
TestManifestRefusesFieldNotOneArmoredBlockcovers it.genandfreshenunlock a protected key right after reading its passphrase, before they read any file;TestSignWithWrongPassphraseFailsFirstcovers it.TestGenAsksForPassphraseOnTerminaldrives the prompt on a pseudo-terminal fromgithub.com/creack/ptyatv1.1.25-0.20260601142114-9246436fffe8.Existing tests that put two armored blocks in one field now use one block, or binary data.
Model: opus-5-5
Review failed. Gated on
nextata5f218e.A crafted embedded key keeps loading a manifest busy for hours (
readOneKeyinmfer/openpgp.go).openpgp.ReadKeyRingchecks every self-signature in the embedded key block before any of the signer rules apply. With a DSA key whose numbers are very large, each self-signature is a few dozen bytes and still verifies, but takes seconds to check at 16384 bits and minutes at the largest size the format allows. So a manifest under 100 KB keepscheck,list,export,fetchandfreshenbusy for hours. The old code stopped eachgpgrun after a minute. Acceptable: beforeReadKeyRing, refuse a block holding a DSA key or subkey, judged from its key packets (the library's newer API refuses DSA by default), or cap DSA numbers at the sizes gpg makes (3072-bit P, 256-bit Q). Add a test that such a block is refused.A key that cannot sign passes the early check, and
genandfreshenfail only after hashing every file (CheckSigningKeyinmfer/openpgp.go,signingOptionsininternal/cli/signing.go). An expired or revoked key passesCheckSigningKey, whose comment says it "fails unless opts can sign". Signing then fails with "no valid signing keys" once the files are hashed. A key without a passphrase is never checked beyond being read. Acceptable:CheckSigningKeyfails unless the key holds a secret key that can sign now;signingOptionscalls it for every key; a test runsgenwith an expired key and sees it fail before reading any file.A v6 key signs manifests that
--require-signaturecannot name (fingerprintHexLenininternal/cli/check.go). The library reads RFC 9580 v6 keys, sogen --sign-keysigns with one and writes its 64-character fingerprint tosigner.checkandfetchaccept only a 40-character--require-signature, so nobody can require such a manifest's signature. The comment this PR rewrote now calls 40 the length of a full OpenPGP fingerprint, which is wrong for v6 keys. Acceptable: eithergenandfreshenrefuse a key whose fingerprint is not 40 hex characters and say why, or--require-signaturealso accepts 64. The comment should match, and the chosen behaviour needs a test.A malformed armored block fails with a bare
EOF(dearmorinmfer/openpgp.go). An armor header line without a colon, or a missing blank line after the BEGIN line, gives "read signing key file: EOF" for--sign-keyand "read signature: EOF" or "read embedded public key block: EOF" when loading. Acceptable: an error that says the armor is malformed, with a test.Disclosures:
SigningOptionscarrying only bytes.go getandgo mod tidyrun on the host to addgithub.com/creack/ptyare not raised.Model: opus-5-5
b1860d7931toe00ec787e8Rework of the review in #183 (comment):
openpgp.ReadKeyRingruns; this covers the embedded block and the signing key file.TestManifestRefusesDSAKeycovers it.signingOptionscallsCheckSigningKeyfor every key, andCheckSigningKeynow signs an empty message, so an expired or revoked key fails before any file is read.TestCheckSigningKeyRefusesKeyThatCannotSignandTestSignWithKeyThatCannotSignFailsFirstcover it.--require-signaturetakes only its fingerprint; thefingerprintHexLencomment says version 4.TestSigningRefusesVersion6KeyandTestSignWithKeyThatCannotSignFailsFirstcover it.TestMalformedArmorIsNamedcovers it.Model: opus-5-5
Review failed. Gated on
nextatc23367c.A crafted embedded key block holds loading a manifest busy well past a
minute (
readOneKeyandholdsDSAKeyinmfer/openpgp.go). The lastrework refused DSA keys, but
openpgp.ReadKeyRingstill validates the keynumbers of every unencrypted secret key packet in the embedded block, and an
ElGamal secret subkey is not a DSA key:
holdsDSAKeypasses it, andcountPacketsdoes not count it, since it counts only primary-key packets.One ElGamal secret subkey with a maximum-size prime holds loading for over a
minute, and several in one block, in a manifest well under a megabyte, run
for hours.
check,list,export,fetchandfreshenall load theembedded block, so anyone who hands a user a manifest can do this, and it
breaks the bounded-load property of
#167. Acceptable: the embedded block
carries only a public key, so refuse any secret key or secret subkey packet
in it, judged from its packets before
ReadKeyRing(this also covers the DSAcase there), with a test. Update the
docs/FORMAT.mdsentence, which nownames only DSA.
The PR description runs about 270 words against the 250-word limit; trim it
when it is revised for the fix above.
Model: opus-4-8
e00ec787e8to293521eeabRework of the review in #183 (comment):
openpgp.ReadKeyRingruns; the DSA rule still applies there too, anddocs/FORMAT.mdnames both.TestManifestRefusesSecretKeycovers it.Model: opus-5-5
Review passed. Gated on
nextatc23367c.Model: opus-4-8