Author SHA1 Message Date
sneak 7900c878e1 Include gpg's stderr in the error when gpg fails (closes #146)
check / check (push) Waiting to run
When gpg failed while decrypting, encrypting or looking up a key
fingerprint, the error gave only gpg's exit status, so the user could
not tell why. cmd.Output keeps gpg's stderr in the *exec.ExitError it
returns; the three errors now end with that stderr, trimmed. When gpg
wrote nothing to stderr, the error is left as it was.

gpg's stderr can run to more than one line, and the error then does too.

The regression test gives gpg data that is not an OpenPGP message
instead of one encrypted to a missing key: gpg reports both the same
way, and this case starts no gpg-agent the test would have to stop.

Model: opus-5-5
2026-10-15 02:13:19 +00:00
clawbot 438c36e7a9 A secret too large for the locked-memory limit fails with an error, not a panic (closes #145)
check / check (push) Waiting to run
secret add, import and generate secret crashed with memguard's panic once a
value outgrew the locked-memory limit, and because the value was held twice,
half the usual 8 MiB limit was enough. secret now raises the limit to the hard
limit at startup. Each of the three commands refuses, before its first locked
allocation, a limit that leaves fewer than 8 pages for its keys and buffers.
The value is read a page at a time into a memguard.Stream, which keeps it
encrypted, and copied once into locked memory after its size is checked
against 100 MiB and the limit. It leaves the stream through Next, not Read:
Read re-encrypts a leftover piece, and memguard hangs if that runs out of
locked memory.

Model: opus-5-5
2026-10-15 04:02:14 +02:00
9 changed files with 369 additions and 285 deletions
+11 -3
View File
@@ -129,6 +129,17 @@ one.
### Secret Management
#### Secret Size
While `secret` works on a secret's value it keeps the value in locked memory, so
the value counts against the locked-memory limit (`ulimit -l`). `secret` raises
that limit to the hard limit when it starts. `secret add`, `secret import` and
`secret generate secret` refuse a value that does not fit in the limit with 8
memory pages to spare (32 KiB with 4 KiB pages), with an error that names the
limit. Linux's usual limit of 8 MiB allows a value of nearly 8 MiB; a larger one
needs a higher hard limit, such as `memlock` in `/etc/security/limits.conf`. No
value may be over 100 MiB, whatever the limit.
#### `secret add <secret-name> [--force]`
Adds a secret to the current vault. Reads the secret value from stdin.
@@ -296,9 +307,6 @@ generates a new Age key.
Decrypts data using an Age key stored as a secret.
With `--output`, both hold the result in memory and write it to `--output` only
once they have succeeded, so a failed command leaves `--output` untouched.
## Storage Architecture
### Directory Structure
+17 -39
View File
@@ -1,7 +1,6 @@
package cli
import (
"bytes"
"errors"
"fmt"
"io"
@@ -189,15 +188,17 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
input = file
}
// Set up output writer. With --output the result is held in memory
// and written only once encryption has succeeded, so a failure leaves
// --output untouched and --input can be the same file.
// Set up output writer
output := cli.cmd.OutOrStdout()
var result bytes.Buffer
if outputFile != "" {
output = &result
file, err := cli.fs.Create(outputFile)
if err != nil {
return fmt.Errorf("failed to create output file: %w", err)
}
defer func() { _ = file.Close() }()
output = file
}
// Encrypt the data
@@ -216,11 +217,7 @@ func (cli *Instance) Encrypt(secretName, inputFile, outputFile string) error {
return fmt.Errorf("failed to finalize encryption: %w", err)
}
if outputFile == "" {
return nil
}
return cli.writeOutputFile(outputFile, result.Bytes())
return nil
}
// Decrypt decrypts data using an age secret key stored in a secret
@@ -281,15 +278,17 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
input = file
}
// Set up output writer. With --output the result is held in memory
// and written only once decryption has succeeded, so a failure leaves
// --output untouched and --input can be the same file.
// Set up output writer
output := cli.cmd.OutOrStdout()
var result bytes.Buffer
if outputFile != "" {
output = &result
file, err := cli.fs.Create(outputFile)
if err != nil {
return fmt.Errorf("failed to create output file: %w", err)
}
defer func() { _ = file.Close() }()
output = file
}
// Decrypt the data
@@ -303,27 +302,6 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
return fmt.Errorf("failed to decrypt data: %w", err)
}
if outputFile == "" {
return nil
}
return cli.writeOutputFile(outputFile, result.Bytes())
}
// writeOutputFile creates outputFile, or empties it if it exists, and
// writes result into it.
func (cli *Instance) writeOutputFile(outputFile string, result []byte) error {
file, err := cli.fs.Create(outputFile)
if err != nil {
return fmt.Errorf("failed to create output file: %w", err)
}
defer func() { _ = file.Close() }()
_, err = file.Write(result)
if err != nil {
return fmt.Errorf("failed to write output file: %w", err)
}
return nil
}
-134
View File
@@ -1,134 +0,0 @@
//nolint:testpackage // sets the unexported fields of Instance
package cli
import (
"fmt"
"io"
"os"
"path/filepath"
"testing"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"github.com/stretchr/testify/require"
"sneak.berlin/go/secret/internal/vault"
)
// plainText is the content of plain.txt in the directory newCryptoTest makes.
const plainText = "document"
// newCryptoTest returns an Instance whose current vault, in a new state
// directory on the real filesystem, holds the age key secret "key", and a
// new directory holding only plain.txt and plain.txt.age, its encryption
// with that key.
func newCryptoTest(t *testing.T) (*Instance, string) {
t.Helper()
fs := afero.NewOsFs()
stateDir := t.TempDir()
_, err := vault.CreateVault(fs, stateDir, "default", testMnemonicBuffer(t), nil)
require.NoError(t, err)
cli := NewCLIInstanceWithStateDir(fs, stateDir)
cli.Mnemonic = testMnemonicBuffer(t)
cli.cmd = &cobra.Command{}
dir := t.TempDir()
plain := filepath.Join(dir, "plain.txt")
require.NoError(t, afero.WriteFile(fs, plain, []byte(plainText), 0o600))
require.NoError(t, cli.Encrypt("key", plain, plain+".age"))
return cli, dir
}
// TestFailedCryptoLeavesOutputUnchanged is a regression test for
// https://git.eeqj.de/sneak/secret/issues/143, where secret encrypt and
// secret decrypt emptied the file named by --output before they failed.
// A failed command must leave an existing file as it was and create none,
// and give the same error as without --output.
func TestFailedCryptoLeavesOutputUnchanged(t *testing.T) {
t.Parallel()
tests := []struct {
name string
run func(cli *Instance, dir, output string) error
}{
// Reading a directory fails once encryption has begun
{"encrypt", func(cli *Instance, dir, output string) error {
return cli.Encrypt("key", dir, output)
}},
// plain.txt is not an age file
{"decrypt", func(cli *Instance, dir, output string) error {
return cli.Decrypt("key", filepath.Join(dir, "plain.txt"), output)
}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
cli, dir := newCryptoTest(t)
existing := filepath.Join(dir, "existing.txt")
require.NoError(t, afero.WriteFile(cli.fs, existing, []byte("precious"), 0o600))
cli.cmd.SetOut(io.Discard)
toStdout := tt.run(cli, dir, "")
require.Error(t, toStdout)
require.EqualError(t, tt.run(cli, dir, existing), toStdout.Error())
require.EqualError(t, tt.run(cli, dir, filepath.Join(dir, "new.txt")),
toStdout.Error())
content, err := afero.ReadFile(cli.fs, existing)
require.NoError(t, err)
require.Equal(t, "precious", string(content))
exists, err := afero.Exists(cli.fs, filepath.Join(dir, "new.txt"))
require.NoError(t, err)
require.False(t, exists, "a failed %s created --output", tt.name)
})
}
}
// TestCryptoInPlace checks that secret encrypt and secret decrypt given
// one file as both --input and --output replace it with the right result.
// Before https://git.eeqj.de/sneak/secret/issues/143 they emptied the file
// before reading it.
func TestCryptoInPlace(t *testing.T) {
t.Parallel()
cli, dir := newCryptoTest(t)
file := filepath.Join(dir, "plain.txt")
require.NoError(t, cli.Encrypt("key", file, file))
require.NoError(t, cli.Decrypt("key", file, file))
content, err := afero.ReadFile(cli.fs, file)
require.NoError(t, err)
require.Equal(t, plainText, string(content))
}
// TestDecryptToDescriptorOfDeletedFile checks that secret decrypt given
// /dev/fd/N as --output writes through that descriptor. A program may give
// the command a temporary file it has already deleted as stdout, pass
// /dev/stdout as --output and read the result back from that file.
func TestDecryptToDescriptorOfDeletedFile(t *testing.T) {
t.Parallel()
cli, dir := newCryptoTest(t)
file, err := os.CreateTemp(t.TempDir(), "out")
require.NoError(t, err)
defer func() { _ = file.Close() }()
require.NoError(t, os.Remove(file.Name()))
output := fmt.Sprintf("/dev/fd/%d", file.Fd())
require.NoError(t, cli.Decrypt("key", filepath.Join(dir, "plain.txt.age"), output))
content, err := io.ReadAll(file)
require.NoError(t, err)
require.Equal(t, plainText, string(content))
}
+16 -4
View File
@@ -72,6 +72,12 @@ func newGenerateSecretCmd() *cobra.Command {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
// Before readSecretEnv, which makes the first locked allocation
err = checkLockedMemoryLimit(0)
if err != nil {
return err
}
destroySecrets := cli.readSecretEnv()
defer destroySecrets()
@@ -135,10 +141,16 @@ func (cli *Instance) GenerateSecret(
return errLengthTooSmall
}
var (
secretValue string
err error
)
if length > maxSecretSize {
return errSecretTooLarge
}
err := checkLockedMemoryLimit(length)
if err != nil {
return err
}
var secretValue string
switch secretType {
case "base58":
+15
View File
@@ -14,6 +14,8 @@ import (
// every memguard buffer before it returns, so the caller must do nothing
// but exit with the code.
func Entry() int {
raiseLockedMemoryLimit()
// On SIGINT or SIGTERM memguard runs this function, wipes every buffer
// and exits with status 1. The passphrase prompt turns echo off on the
// controlling terminal until the read finishes, so a signal there would
@@ -51,6 +53,19 @@ func Entry() int {
return 0
}
// raiseLockedMemoryLimit raises this process's limit on locked memory, which
// holds every secret value, to the hard limit, the most a process may set
// without privileges. When that fails the limit stays as it was, and
// checkLockedMemoryLimit names it in its error.
func raiseLockedMemoryLimit() {
var limit unix.Rlimit
if unix.Getrlimit(unix.RLIMIT_MEMLOCK, &limit) == nil {
limit.Cur = limit.Max
_ = unix.Setrlimit(unix.RLIMIT_MEMLOCK, &limit)
}
}
func newRootCmd() *cobra.Command {
secret.Debug("newRootCmd starting")
+111 -95
View File
@@ -14,6 +14,7 @@ import (
"github.com/awnumar/memguard"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"golang.org/x/sys/unix"
"sneak.berlin/go/secret/internal/secret"
"sneak.berlin/go/secret/internal/vault"
)
@@ -24,26 +25,30 @@ const (
// vaultSecretParts is the number of parts when splitting vault:secret
vaultSecretParts = 2
// initialBufferSize is the starting size for secret read buffers (4KB)
initialBufferSize = 4 * 1024
// maxSecretSize is the maximum allowed size of a secret (100MB)
maxSecretSize = 100 * 1024 * 1024
// lockedMemoryReservePages is the locked memory, in pages, that `secret
// add`, `secret import` and `secret generate secret` may need besides
// the value they store, for keys and the read buffer: memguard locks
// whole pages, at least one for each buffer. `secret add` was measured
// to need 7 with both SB_SECRET_MNEMONIC and SB_UNLOCK_PASSPHRASE set,
// and 6 with one; the eighth is to spare.
lockedMemoryReservePages = 8
// kibibyte is the unit of the locked-memory limit in `ulimit -l`.
kibibyte = 1024
)
// Sentinel errors for secret operations
var (
errSecretTooLarge = errors.New("secret too large: exceeds 100MB limit")
errSecretTooLarge = errors.New("secret too large: exceeds 100MB limit")
errLockedMemoryLimit = errors.New(
"secret too large for the locked-memory limit")
errCrossVaultSourceUnqualified = errors.New(
"source must specify vault (e.g., vault:secret) for cross-vault move")
errMoveOntoItself = errors.New("cannot be moved onto itself")
)
// bufferInfo tracks a protected buffer and the number of bytes used in it
type bufferInfo struct {
buffer *memguard.LockedBuffer
used int
}
// ParseVaultSecretRef parses a "vault:secret" or just "secret" reference
// Returns (vaultName, secretName, isQualified)
// If no vault is specified, returns empty vaultName and isQualified=false
@@ -75,6 +80,12 @@ func newAddCmd() *cobra.Command {
cli.cmd = cmd // Set the command for stdin access
// Before readSecretEnv, which makes the first locked allocation
err = checkLockedMemoryLimit(0)
if err != nil {
return err
}
destroySecrets := cli.readSecretEnv()
defer destroySecrets()
@@ -174,6 +185,12 @@ func newImportCmd() *cobra.Command {
return fmt.Errorf("failed to initialize CLI: %w", err)
}
// Before readSecretEnv, which makes the first locked allocation
err = checkLockedMemoryLimit(0)
if err != nil {
return err
}
destroySecrets := cli.readSecretEnv()
defer destroySecrets()
@@ -270,88 +287,105 @@ The source secret is deleted after successful copy.`,
return cmd
}
// updateBufferSize updates the buffer size based on usage pattern
func updateBufferSize(currentSize int, sameSize *int) int {
const (
doubleAfterBuffers = 2
growthFactor = 2
)
// checkLockedMemoryLimit returns an error naming the locked-memory limit
// (`ulimit -l`) when a value of size bytes does not fit in it with
// lockedMemoryReservePages. memguard, which holds secret values in locked
// memory, crashes or hangs the process when it cannot lock more, so a
// command calls this with size 0 before its first locked allocation, and
// again with the value's size before a buffer for the value is allocated.
func checkLockedMemoryLimit(size int) error {
var limit unix.Rlimit
*sameSize++
if *sameSize >= doubleAfterBuffers {
*sameSize = 0
return currentSize * growthFactor
err := unix.Getrlimit(unix.RLIMIT_MEMLOCK, &limit)
if err != nil {
return fmt.Errorf("failed to read the locked-memory limit: %w", err)
}
return currentSize
}
// memguard locks whole pages, so the value is counted in whole pages.
pageSize := os.Getpagesize()
valuePages := (size + pageSize - 1) / pageSize
// destroyBuffers destroys every buffer in the list
func destroyBuffers(buffers []bufferInfo) {
for _, b := range buffers {
b.buffer.Destroy()
//nolint:gosec // G115: a page count and a page size are never negative
needed := uint64((valuePages + lockedMemoryReservePages) * pageSize)
if needed > limit.Cur {
return fmt.Errorf("%w of %d KiB (ulimit -l)",
errLockedMemoryLimit, limit.Cur/kibibyte)
}
return nil
}
// readSecretFromReader reads all data from reader into protected buffers,
// enforcing the maximum secret size. On failure the accumulated buffers
// are destroyed; on success the caller must destroy them.
func readSecretFromReader(reader io.Reader) ([]bufferInfo, int, error) {
var buffers []bufferInfo
// readSecretFromReader reads all of reader into a new locked buffer, which
// the caller must destroy, leaving out one trailing newline when trimNewline
// is set. It reads a page at a time into a memguard.Stream, which keeps what
// it is given encrypted, so that the value is in locked memory only once, in
// the buffer returned, and both size checks come before that buffer is
// allocated. It takes the value back out with Stream.Next, not Stream.Read:
// Read puts back the part of a piece that does not fit, and memguard hangs
// when it runs out of locked memory doing that.
func readSecretFromReader(
reader io.Reader, trimNewline bool,
) (*memguard.LockedBuffer, error) {
stream := memguard.NewStream()
totalSize := 0
currentBufferSize := initialBufferSize
sameSize := 0
chunk := memguard.NewBuffer(os.Getpagesize())
defer chunk.Destroy()
size := 0
endsWithNewline := false
for {
// Create a new buffer
buffer := memguard.NewBuffer(currentBufferSize)
n, err := io.ReadFull(reader, buffer.Bytes())
if n == 0 {
// No data read, destroy the unused buffer
buffer.Destroy()
} else {
buffers = append(buffers, bufferInfo{buffer: buffer, used: n})
totalSize += n
if totalSize > maxSecretSize {
destroyBuffers(buffers)
return nil, 0, errSecretTooLarge
n, err := io.ReadFull(reader, chunk.Bytes())
if n > 0 {
size += n
if size > maxSecretSize {
return nil, errSecretTooLarge
}
// If we filled the buffer, consider growing for next iteration
if n == currentBufferSize {
currentBufferSize = updateBufferSize(currentBufferSize, &sameSize)
}
endsWithNewline = chunk.Bytes()[n-1] == '\n'
// Write encrypts the bytes and wipes them from chunk.
_, _ = stream.Write(chunk.Bytes()[:n])
}
if err == io.EOF || errors.Is(err, io.ErrUnexpectedEOF) {
if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
break
} else if err != nil {
destroyBuffers(buffers)
}
return nil, 0, err
if err != nil {
return nil, err
}
}
return buffers, totalSize, nil
}
// Freed before the value's buffer is allocated, so that the two are
// never locked at once.
chunk.Destroy()
// combineBuffers copies the used portions of buffers into a single
// protected buffer of totalSize bytes
func combineBuffers(buffers []bufferInfo, totalSize int) *memguard.LockedBuffer {
valueBuffer := memguard.NewBuffer(totalSize)
offset := 0
for _, b := range buffers {
copy(valueBuffer.Bytes()[offset:], b.buffer.Bytes()[:b.used])
offset += b.used
if trimNewline && endsWithNewline {
size--
}
return valueBuffer
err := checkLockedMemoryLimit(size)
if err != nil {
return nil, err
}
value := memguard.NewBuffer(size)
// copy stops at the value's size, so a trimmed newline is not copied.
for offset := 0; offset < size; {
decrypted, err := stream.Next()
if err != nil {
value.Destroy()
return nil, fmt.Errorf("failed to decrypt the value read: %w", err)
}
offset += copy(value.Bytes()[offset:], decrypted.Bytes())
decrypted.Destroy()
}
return value, nil
}
// AddSecret adds a secret to the current vault
@@ -370,35 +404,21 @@ func (cli *Instance) AddSecret(secretName string, force bool) error {
secret.Debug("Got current vault", "vault_name", vlt.GetName())
// Read secret value directly into protected buffers
secret.Debug("Reading secret value from stdin into protected buffers")
// Read the secret value, less a trailing newline, into locked memory
secret.Debug("Reading secret value from stdin into a protected buffer")
buffers, totalSize, err := readSecretFromReader(cli.cmd.InOrStdin())
valueBuffer, err := readSecretFromReader(cli.cmd.InOrStdin(), true)
if err != nil {
if errors.Is(err, errSecretTooLarge) {
if errors.Is(err, errSecretTooLarge) || errors.Is(err, errLockedMemoryLimit) {
return err
}
return fmt.Errorf("failed to read secret value: %w", err)
}
defer destroyBuffers(buffers)
// Check for trailing newline in the last buffer
if len(buffers) > 0 && totalSize > 0 {
lastBuffer := &buffers[len(buffers)-1]
if lastBuffer.buffer.Bytes()[lastBuffer.used-1] == '\n' {
lastBuffer.used--
totalSize--
}
}
secret.Debug("Read secret value from stdin",
"value_length", totalSize, "buffers", len(buffers))
// Combine all buffers into a single protected buffer
valueBuffer := combineBuffers(buffers, totalSize)
defer valueBuffer.Destroy()
secret.Debug("Read secret value from stdin", "value_length", valueBuffer.Size())
// Locked only now that stdin has been read: in `secret encrypt key |
// secret add name`, holding the lock while reading would leave each
// command waiting for the other.
@@ -667,7 +687,7 @@ func (cli *Instance) ImportSecret(
vlt.Mnemonic, vlt.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
// Read secret value from the source file into protected buffers
// Read secret value from the source file into a protected buffer
file, err := cli.fs.Open(sourceFile)
if err != nil {
return fmt.Errorf("failed to open file %s: %w", sourceFile, err)
@@ -679,14 +699,10 @@ func (cli *Instance) ImportSecret(
}
}()
buffers, totalSize, err := readSecretFromReader(file)
valueBuffer, err := readSecretFromReader(file, false)
if err != nil {
return fmt.Errorf("failed to read secret from file %s: %w", sourceFile, err)
}
defer destroyBuffers(buffers)
// Combine all buffers into a single protected buffer
valueBuffer := combineBuffers(buffers, totalSize)
defer valueBuffer.Destroy()
// Locked only now that the file has been read, as in AddSecret: the
+153 -7
View File
@@ -3,17 +3,23 @@ package cli
import (
"bytes"
"context"
"crypto/rand"
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"testing"
"time"
"github.com/spf13/afero"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/secret/internal/secret"
"sneak.berlin/go/secret/internal/vault"
"sneak.berlin/go/secret/pkg/agehd"
)
@@ -208,23 +214,25 @@ func TestImportSecretVariousSizes(t *testing.T) {
}
}
// TestAddSecretBufferGrowth tests that our buffer growth strategy works correctly
// TestAddSecretAroundPageSize tests that secret add, which reads its input
// one memory page (usually 4 KiB) at a time, stores values just under, at and
// just over one and two pages unchanged.
//
//nolint:paralleltest // in parallel, size tests could exceed the memlock limit
func TestAddSecretBufferGrowth(t *testing.T) {
// Test various sizes that should trigger buffer growth
func TestAddSecretAroundPageSize(t *testing.T) {
// Sizes around one and two pages of 4 KiB, then larger ones up to 1 MiB
sizes := []int{
1, // Single byte
100, // Small
4095, // Just under initial 4KB
4095, // Just under 4KB
4096, // Exactly 4KB
4097, // Just over 4KB
8191, // Just under 8KB (first double)
8191, // Just under 8KB
8192, // Exactly 8KB
8193, // Just over 8KB
12288, // 12KB (should trigger second double)
12288, // 12KB
16384, // 16KB
32768, // 32KB (after more doublings)
32768, // 32KB
65536, // 64KB
131072, // 128KB
524288, // 512KB
@@ -315,6 +323,144 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
"Retrieved secret should match original")
}
// newBinaryTestStateDir returns a new state directory holding a vault made
// from the test mnemonic, for the secret binary to use.
func newBinaryTestStateDir(t *testing.T) string {
t.Helper()
stateDir := t.TempDir()
_, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default",
testMnemonicBuffer(t), nil)
require.NoError(t, err)
return stateDir
}
// runAtLockedMemoryLimit runs the secret binary with args, stdin, the state
// directory stateDir and the test mnemonic, its locked-memory limit (ulimit
// -l) lowered to limitKiB, and returns what it printed.
func runAtLockedMemoryLimit(
t *testing.T, stateDir string, limitKiB int, stdin io.Reader,
args ...string,
) (string, error) {
t.Helper()
const commandWait = 10 * time.Second
secretPath, err := filepath.Abs("../../secret")
require.NoError(t, err)
ctx, cancel := context.WithTimeout(t.Context(), commandWait)
defer cancel()
// The shell lowers the limit for itself and the binary only.
script := fmt.Sprintf(`ulimit -l %d && exec "$0" "$@"`, limitKiB)
//nolint:gosec // G204: runs the freshly built secret binary
cmd := exec.CommandContext(ctx, "sh",
append([]string{"-c", script, secretPath}, args...)...)
cmd.Env = []string{
secret.EnvStateDir + "=" + stateDir,
secret.EnvMnemonic + "=" + testMnemonic,
}
cmd.Stdin = stdin
output, err := cmd.CombinedOutput()
return string(output), err
}
// requireRefusedAtLockedMemoryLimit runs the secret binary as
// runAtLockedMemoryLimit does, and requires it to exit with status 1 and
// print wantError, with no panic.
func requireRefusedAtLockedMemoryLimit(
t *testing.T, stateDir string, limitKiB int, wantError string,
stdin io.Reader, args ...string,
) {
t.Helper()
output, err := runAtLockedMemoryLimit(t, stateDir, limitKiB, stdin, args...)
var exitErr *exec.ExitError
require.ErrorAs(t, err, &exitErr, "%v: %s", args, output)
assert.Equal(t, 1, exitErr.ExitCode(), "%v: %s", args, output)
assert.Contains(t, output, wantError, args)
assert.NotContains(t, output, "panic", args)
}
// TestSecretsAtLockedMemoryLimit runs the secret binary with the
// locked-memory limit (ulimit -l) lowered to 1 MiB, where memguard would crash
// the process with a panic instead of returning an error. add must store the
// largest value that fits, which takes holding it in locked memory only once.
// add, import and generate secret must refuse a value one byte larger with
// exit status 1 and an error saying why.
func TestSecretsAtLockedMemoryLimit(t *testing.T) {
t.Parallel()
const limitKiB = 1024
fits := limitKiB*kibibyte - lockedMemoryReservePages*os.Getpagesize()
limitError := fmt.Sprintf("locked-memory limit of %d KiB", limitKiB)
stateDir := newBinaryTestStateDir(t)
tooBigFile := filepath.Join(t.TempDir(), "too-big")
require.NoError(t, os.WriteFile(tooBigFile, make([]byte, fits+1), 0o600))
output, err := runAtLockedMemoryLimit(t, stateDir, limitKiB,
bytes.NewReader(make([]byte, fits)), "add", "fits")
require.NoError(t, err, "the largest value that fits: %s", output)
requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB, limitError,
bytes.NewReader(make([]byte, fits+1)), "add", "too-big")
requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB, limitError,
nil, "import", "too-big", "--source", tooBigFile)
requireRefusedAtLockedMemoryLimit(t, stateDir, limitKiB, limitError,
nil, "generate", "secret", "too-big", "--length", strconv.Itoa(fits+1))
}
// TestSecretsUnderSmallLockedMemoryLimits runs add, import and generate
// secret with a short value under two small locked-memory limits (ulimit
// -l). Under 20 KiB, too little for the buffers they need besides the value,
// each must refuse it with exit status 1 and an error naming the limit, where
// memguard would panic or hang. Under 64 KiB, the default of older Linux
// kernels, each must store it.
func TestSecretsUnderSmallLockedMemoryLimits(t *testing.T) {
t.Parallel()
const (
tooSmallLimitKiB = 20
oldDefaultLimitKiB = 64
shortValue = "hunter2"
)
tooSmallLimitError := fmt.Sprintf("locked-memory limit of %d KiB",
tooSmallLimitKiB)
stateDir := newBinaryTestStateDir(t)
shortFile := filepath.Join(t.TempDir(), "short")
require.NoError(t, os.WriteFile(shortFile, []byte(shortValue), 0o600))
requireRefusedAtLockedMemoryLimit(t, stateDir, tooSmallLimitKiB,
tooSmallLimitError, strings.NewReader(shortValue), "add", "added")
requireRefusedAtLockedMemoryLimit(t, stateDir, tooSmallLimitKiB,
tooSmallLimitError, nil, "import", "imported", "--source", shortFile)
requireRefusedAtLockedMemoryLimit(t, stateDir, tooSmallLimitKiB,
tooSmallLimitError, nil, "generate", "secret", "generated")
output, err := runAtLockedMemoryLimit(t, stateDir, oldDefaultLimitKiB,
strings.NewReader(shortValue), "add", "added")
require.NoError(t, err, "add: %s", output)
output, err = runAtLockedMemoryLimit(t, stateDir, oldDefaultLimitKiB,
nil, "import", "imported", "--source", shortFile)
require.NoError(t, err, "import: %s", output)
output, err = runAtLockedMemoryLimit(t, stateDir, oldDefaultLimitKiB,
nil, "generate", "secret", "generated")
require.NoError(t, err, "generate secret: %s", output)
}
// slowReader simulates a reader that returns data in small chunks
type slowReader struct {
data []byte
+22 -3
View File
@@ -388,7 +388,8 @@ func ResolveGPGKeyFingerprint(keyID string) (string, error) {
return "", fmt.Errorf("%w: %s", ErrGPGKeyNotFound, keyID)
}
return "", fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
return "", fmt.Errorf(
"failed to resolve GPG key fingerprint: %w", withGPGStderr(err))
}
// Parse the output to extract the fingerprint
@@ -418,6 +419,24 @@ func checkGPGAvailable() error {
return nil
}
// withGPGStderr appends gpg's stderr, which says why gpg failed, to err
// from cmd.Output. cmd.Output keeps that stderr in the *exec.ExitError.
// err is returned unchanged when gpg wrote nothing to stderr.
func withGPGStderr(err error) error {
var exitErr *exec.ExitError
if !errors.As(err, &exitErr) {
return err
}
stderr := strings.TrimSpace(string(exitErr.Stderr))
if stderr == "" {
return err
}
return fmt.Errorf("%w: %s", err, stderr)
}
// gpgEncryptDefault is the default implementation of GPG encryption
func gpgEncryptDefault(data *memguard.LockedBuffer, keyID string) ([]byte, error) {
if data == nil {
@@ -437,7 +456,7 @@ func gpgEncryptDefault(data *memguard.LockedBuffer, keyID string) ([]byte, error
output, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("GPG encryption failed: %w", err)
return nil, fmt.Errorf("GPG encryption failed: %w", withGPGStderr(err))
}
return output, nil
@@ -450,7 +469,7 @@ func gpgDecryptDefault(encryptedData []byte) (*memguard.LockedBuffer, error) {
output, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("GPG decryption failed: %w", err)
return nil, fmt.Errorf("GPG decryption failed: %w", withGPGStderr(err))
}
// Create a secure buffer for the decrypted data
+24
View File
@@ -102,3 +102,27 @@ func TestPGPUnlockerAddedTwiceKeepsFirst(t *testing.T) {
assert.NotEqual(t, first.GetDirectory(), second.GetDirectory())
assert.Equal(t, firstFiles, dirFiles(t, fs, first.GetDirectory()))
}
// TestGPGDecryptErrorHasGPGReason runs the real gpg on data that is not an
// OpenPGP message and checks that the error carries the reason gpg writes
// to stderr.
func TestGPGDecryptErrorHasGPGReason(t *testing.T) {
// The empty GNUPGHOME keeps gpg away from the user's own keyring;
// LC_ALL=C keeps gpg's message in English.
t.Setenv("GNUPGHOME", t.TempDir())
t.Setenv("LC_ALL", "C")
_, err := secret.GPGDecryptFunc([]byte("not an OpenPGP message"))
require.ErrorContains(t, err, "gpg: no valid OpenPGP data found")
}
// TestGPGDecryptErrorWithoutGPGStderr checks that when gpg fails without
// writing to stderr, the error ends with the exit status and nothing after it.
//
//nolint:paralleltest // installFakeGPG uses t.Setenv
func TestGPGDecryptErrorWithoutGPGStderr(t *testing.T) {
installFakeGPG(t)
_, err := secret.GPGDecryptFunc([]byte("not an OpenPGP message"))
require.EqualError(t, err, "GPG decryption failed: exit status 1")
}