A secret too large for the locked-memory limit fails with an error, not a panic (closes #145)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
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
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user