Author SHA1 Message Date
clawbot f081505f78 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. The value is read a page at a time into a memguard.Stream,
which keeps it encrypted, then copied once into locked memory after its size is
checked against 100 MiB and against the limit less 16 pages for the command's
keys. It comes out of the stream through Next, not Read: Read re-encrypts a
leftover piece, and memguard hangs if that runs out of locked memory.

Judgement call: with a limit of 64 KiB or less, nothing can be stored.

Model: opus-5-5
2026-10-14 20:04:22 +00:00
10 changed files with 232 additions and 194 deletions
+12 -2
View File
@@ -127,6 +127,17 @@ one.
### Secret Management
#### Secret Size
A secret's value is held in locked memory, which is never written to swap.
`secret` raises its locked-memory limit (`ulimit -l`) to the hard limit when it
starts, and `secret add`, `secret import` and `secret generate secret` refuse a
value that does not fit in that limit with 16 memory pages to spare (64 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.
@@ -197,8 +208,7 @@ for confirmation, naming the version, the secret and its vault (see
#### `secret generate mnemonic`
Generates a cryptographically secure BIP39 mnemonic phrase and prints it to
stdout. The warning to keep it safe goes to stderr.
Generates a cryptographically secure BIP39 mnemonic phrase.
#### `secret generate secret <name> [--length=16] [--type=base58] [--force]`
+11 -8
View File
@@ -18,14 +18,17 @@ https://git.eeqj.de/sneak/secret/milestone/12
# Completed Steps
- 2026-10-14: `secret generate mnemonic`, `secret vault list` and
`secret unlocker list` print their output to stdout, as `secret list` does, so
it can be piped or captured (https://git.eeqj.de/sneak/secret/issues/138).
They printed with cobra's `cmd.Println` and `cmd.Printf`, which write to
stderr when no output writer is set, so `secret vault list --json | jq`
received nothing. "No versions found" from `secret version list` moved to
stdout too. Messages such as "Created vault", and the warning after the
mnemonic, stay on stderr.
- 2026-10-14: A secret too large for the locked-memory limit (`ulimit -l`) makes
`secret add`, `secret import` and `secret generate secret` fail with an error
that names the limit, not crash with a panic
(https://git.eeqj.de/sneak/secret/issues/145). memguard panics when it cannot
lock memory, and the value was held twice, so with the usual 8 MiB limit any
value over about 4 MiB crashed. `secret` now raises the limit to the hard
limit at startup. The value is read a page at a time into a `memguard.Stream`,
which keeps it encrypted, and copied once into locked memory only after its
size is checked against 100 MiB and against the limit less 16 pages for the
command's keys. `generate secret` checks the length before it generates
anything.
- 2026-10-14: Removing the current unlocker no longer leaves the vault unable to
open through the unlockers it still has
(https://git.eeqj.de/sneak/secret/issues/137). `current-unlocker` went on
+11 -5
View File
@@ -103,7 +103,7 @@ func (cli *Instance) GenerateMnemonic(cmd *cobra.Command) error {
}
// Output mnemonic to stdout
_, _ = fmt.Fprintln(cmd.OutOrStdout(), mnemonic)
cmd.Println(mnemonic)
// Output helpful information to stderr
fmt.Fprintln(os.Stderr, "")
@@ -133,10 +133,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 terminal echo
// off until the read finishes, so a signal there would leave echo off.
@@ -38,6 +40,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")
+93 -96
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,28 @@ 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 a command
// needs besides the value it stores, for its keys and its read buffer,
// with room to spare: memguard locks whole pages, at least one for each
// buffer, and `secret add` needs 7.
lockedMemoryReservePages = 16
// 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
@@ -270,88 +273,100 @@ 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 the process when it cannot lock more, so this check comes
// 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
}
// destroyBuffers destroys every buffer in the list
func destroyBuffers(buffers []bufferInfo) {
for _, b := range buffers {
b.buffer.Destroy()
//nolint:gosec // G115: a size and a page count are never negative
needed := uint64(size + lockedMemoryReservePages*os.Getpagesize())
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 +385,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.
@@ -653,7 +654,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)
@@ -665,14 +666,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
+77
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"
)
@@ -315,6 +321,77 @@ func TestAddSecretStreamingBehavior(t *testing.T) {
"Retrieved secret should match original")
}
// TestSecretsAtLockedMemoryLimit runs the secret binary with the
// locked-memory limit (ulimit -l) lowered. 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, and generate secret
// one over 100 MB, with exit status 1 and an error saying why, where memguard
// would crash the process with a panic.
func TestSecretsAtLockedMemoryLimit(t *testing.T) {
t.Parallel()
const (
limitKiB = 1024
commandWait = 10 * time.Second
)
fits := limitKiB*kibibyte - lockedMemoryReservePages*os.Getpagesize()
limitError := fmt.Sprintf("locked-memory limit of %d KiB", limitKiB)
stateDir := t.TempDir()
_, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default",
testMnemonicBuffer(t), nil)
require.NoError(t, err)
secretPath, err := filepath.Abs("../../secret")
require.NoError(t, err)
tooBigFile := filepath.Join(t.TempDir(), "too-big")
require.NoError(t, os.WriteFile(tooBigFile, make([]byte, fits+1), 0o600))
run := func(stdin []byte, args ...string) (string, error) {
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 = bytes.NewReader(stdin)
output, err := cmd.CombinedOutput()
return string(output), err
}
output, err := run(make([]byte, fits), "add", "fits")
require.NoError(t, err, "the largest value that fits: %s", output)
refused := func(wantError string, stdin []byte, args ...string) {
output, err := run(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)
}
refused(limitError, make([]byte, fits+1), "add", "too-big")
refused(limitError, nil, "import", "too-big", "--source", tooBigFile)
refused(limitError, nil, "generate", "secret", "too-big",
"--length", strconv.Itoa(fits+1))
refused(errSecretTooLarge.Error(), nil, "generate", "secret", "too-big",
"--length", strconv.Itoa(maxSecretSize+1))
}
// slowReader simulates a reader that returns data in small chunks
type slowReader struct {
data []byte
-65
View File
@@ -2,7 +2,6 @@ package cli_test
import (
"bytes"
"encoding/json"
"os/exec"
"path/filepath"
"strings"
@@ -10,7 +9,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/secret/internal/bip39"
"sneak.berlin/go/secret/internal/secret"
)
@@ -92,66 +90,3 @@ func TestGetCommandOutputsToStdout(t *testing.T) {
// Nothing should be in stderr
assert.Empty(t, stderr.String(), "stderr should be empty")
}
// TestMnemonicAndListingsOutputToStdout tests that 'secret generate
// mnemonic', 'secret vault list' and 'secret unlocker list' print their
// output to stdout, so that it can be piped
func TestMnemonicAndListingsOutputToStdout(t *testing.T) {
t.Parallel()
tempDir := t.TempDir()
secretPath := secretBinaryPath(t)
// run runs the secret binary and returns what it wrote to stdout and to
// stderr
run := func(args ...string) (string, string) {
//nolint:gosec // G204: test executes the freshly built secret binary
cmd := exec.CommandContext(t.Context(), secretPath, args...)
cmd.Env = []string{
secret.EnvStateDir + "=" + tempDir,
secret.EnvMnemonic + "=" + testMnemonic,
secret.EnvUnlockPassphrase + "=test-passphrase",
"PATH=/usr/bin:/bin",
}
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr
err := cmd.Run()
require.NoError(t, err, "%v should succeed: %s", args, stderr.String())
return stdout.String(), stderr.String()
}
stdout, stderr := run("generate", "mnemonic")
assert.True(t, bip39.IsMnemonicValid(strings.TrimSpace(stdout)),
"stdout should hold the mnemonic, not %q", stdout)
assert.Contains(t, stderr, "Save this mnemonic phrase securely",
"the warning should be on stderr")
run("init")
stdout, _ = run("vault", "list", "--json")
var vaults map[string]any
require.NoError(t, json.Unmarshal([]byte(stdout), &vaults),
"stdout should hold the vaults as JSON, not %q", stdout)
assert.Equal(t, "default", vaults["currentVault"])
stdout, _ = run("unlocker", "list", "--json")
var unlockers map[string]any
require.NoError(t, json.Unmarshal([]byte(stdout), &unlockers),
"stdout should hold the unlockers as JSON, not %q", stdout)
assert.Len(t, unlockers["unlockers"], 1)
stdout, _ = run("vault", "list")
assert.Contains(t, stdout, "default (current)")
stdout, _ = run("unlocker", "list")
assert.Contains(t, stdout, "Total: 1 unlocker(s)")
}
+7 -10
View File
@@ -326,25 +326,22 @@ func (cli *Instance) printUnlockersJSON(
return fmt.Errorf("failed to marshal JSON: %w", err)
}
_, _ = fmt.Fprintln(cli.cmd.OutOrStdout(), string(jsonBytes))
cli.cmd.Println(string(jsonBytes))
return nil
}
// printUnlockersTable prints unlockers in a formatted table
func (cli *Instance) printUnlockersTable(unlockers []UnlockerInfo) error {
out := cli.cmd.OutOrStdout()
if len(unlockers) == 0 {
_, _ = fmt.Fprintln(out, "No unlockers found in current vault.")
_, _ = fmt.Fprintln(out, "Run 'secret unlocker add passphrase' to create one.")
cli.cmd.Println("No unlockers found in current vault.")
cli.cmd.Println("Run 'secret unlocker add passphrase' to create one.")
return nil
}
_, _ = fmt.Fprintf(out, " %-40s %-12s %-20s %s\n",
"UNLOCKER ID", "TYPE", "CREATED", "FLAGS")
_, _ = fmt.Fprintf(out, " %-40s %-12s %-20s %s\n",
cli.cmd.Printf(" %-40s %-12s %-20s %s\n", "UNLOCKER ID", "TYPE", "CREATED", "FLAGS")
cli.cmd.Printf(" %-40s %-12s %-20s %s\n",
strings.Repeat("-", unlockerIDWidth), strings.Repeat("-", unlockerTypeWidth),
strings.Repeat("-", unlockerDateWidth), strings.Repeat("-", unlockerFlagsWidth))
@@ -359,7 +356,7 @@ func (cli *Instance) printUnlockersTable(unlockers []UnlockerInfo) error {
prefix = "* "
}
_, _ = fmt.Fprintf(out, "%s%-40s %-12s %-20s %s\n",
cli.cmd.Printf("%s%-40s %-12s %-20s %s\n",
prefix,
unlocker.ID,
unlocker.Type,
@@ -367,7 +364,7 @@ func (cli *Instance) printUnlockersTable(unlockers []UnlockerInfo) error {
flags)
}
_, _ = fmt.Fprintf(out, "\nTotal: %d unlocker(s)\n", len(unlockers))
cli.cmd.Printf("\nTotal: %d unlocker(s)\n", len(unlockers))
return nil
}
+5 -7
View File
@@ -186,8 +186,6 @@ func (cli *Instance) ListVaults(cmd *cobra.Command, jsonOutput bool) error {
return err
}
out := cmd.OutOrStdout()
if jsonOutput { //nolint:nestif // Separate JSON and text output formatting logic
// Get current vault name for context
currentVault := ""
@@ -207,13 +205,13 @@ func (cli *Instance) ListVaults(cmd *cobra.Command, jsonOutput bool) error {
return err
}
_, _ = fmt.Fprintln(out, string(jsonBytes))
cmd.Println(string(jsonBytes))
} else {
// Text output
_, _ = fmt.Fprintln(out, "Available vaults:")
cmd.Println("Available vaults:")
if len(vaults) == 0 {
_, _ = fmt.Fprintln(out, " (none)")
cmd.Println(" (none)")
} else {
// Try to get current vault for marking
currentVault := ""
@@ -225,9 +223,9 @@ func (cli *Instance) ListVaults(cmd *cobra.Command, jsonOutput bool) error {
for _, vaultName := range vaults {
if vaultName == currentVault {
_, _ = fmt.Fprintf(out, " %s (current)\n", vaultName)
cmd.Printf(" %s (current)\n", vaultName)
} else {
_, _ = fmt.Fprintf(out, " %s\n", vaultName)
cmd.Printf(" %s\n", vaultName)
}
}
}
+1 -1
View File
@@ -167,7 +167,7 @@ func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error {
}
if len(versions) == 0 {
_, _ = fmt.Fprintln(cmd.OutOrStdout(), "No versions found")
cmd.Println("No versions found")
return nil
}