Compare commits

...
3 Commits
Author SHA1 Message Date
sneak ebb1d3d5a4 Check vault names in every command that takes one (closes #68)
check / check (push) Failing after 2s
A vault name may use only lowercase ASCII letters, digits, `.`, `-` and
`_`, and must not be empty, `.` or `..`; the error now states that rule.
`vault create`, `vault import`, `vault select`, `vault remove`, both
vault names of `mv` and shell completion of a `vault:secret` argument
check the name as typed before building any path from it. Before,
`vault import ..` wrote a long-term key and an unlocker into the state
directory itself, and `vault select ..` made that the current vault.

Model: opus-5-5
2026-10-04 11:27:42 +00:00
clawbot eb596b8be6 Run the checks again on every script/cibuild (closes #54)
check / check (push) Failing after 2s
On an unchanged tree docker served every check step of the Dockerfile
from its build cache, so a second script/cibuild ran no lint, tests or
build and still succeeded.

script/cibuild now passes the current time as the CHECK_EPOCH build
argument. The lint and build stages each declare it after their module
download and before `COPY . .`. A build argument whose value changes
makes every RUN step after its declaration miss the cache, so the
checks run on each build while the base images, the apk install and the
module downloads stay cached.

Model: opus-5-5
2026-10-04 13:25:24 +02:00
clawbot 596b978cb1 Leave no partial unlocker directory when adding an unlocker fails (closes #48)
check / check (push) Failing after 3s
CreatePGPUnlocker looked up the GPG key's fingerprint, and the keychain
unlocker got the long-term key, only after writing part of the unlocker,
so a failure there left a directory with no metadata. Both now do every
step that can fail before writing anything. `secret unlocker add pgp`
looks the fingerprint up once, for its duplicate check, and passes it to
CreatePGPUnlocker to record. All four unlocker types write their files
through the new secret.WriteDir, which builds a new directory in a
temporary directory, renames it into place when complete and removes it
on a failure. A directory that already exists, as when an unlocker
replaces one of the same name, is written in place and never removed.

Model: opus-5-5
2026-10-04 12:58:50 +02:00
22 changed files with 655 additions and 250 deletions
+8
View File
@@ -6,6 +6,11 @@ WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN go mod download RUN go mod download
# script/cibuild sets CHECK_EPOCH to the current time, so the RUN steps
# below run again on each build, an unchanged tree included, while the
# steps above stay cached. ARG is per stage: the build stage declares it too.
ARG CHECK_EPOCH
COPY . . COPY . .
RUN make fmt-check RUN make fmt-check
@@ -25,6 +30,9 @@ WORKDIR /build
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN go mod download RUN go mod download
# As in the lint stage: the RUN steps below run again on each script/cibuild.
ARG CHECK_EPOCH
COPY . . COPY . .
RUN make test RUN make test
+5 -1
View File
@@ -91,6 +91,9 @@ Lists all available vaults. The current vault is marked.
Creates a new vault with the specified name. Creates a new vault with the specified name.
**Vault Name Format:** only lowercase ASCII letters, digits, `.`, `-` and `_`
are allowed, and a name must not be empty, `.` or `..`.
#### `secret vault select <name>` #### `secret vault select <name>`
Switches to the specified vault for subsequent operations. Switches to the specified vault for subsequent operations.
@@ -523,7 +526,8 @@ them. We provide:
- `script/docker` — build the Docker image tagged with the project name - `script/docker` — build the Docker image tagged with the project name
- `script/cibuild` — CI entrypoint: `docker build --ulimit - `script/cibuild` — CI entrypoint: `docker build --ulimit
memlock=-1:-1 .` (memguard needs mlock; the Dockerfile runs the memlock=-1:-1 .` (memguard needs mlock; the Dockerfile runs the
checks) checks), with a new `CHECK_EPOCH` build argument on every run so the
checks run again on an unchanged tree
- `script/precommit` — pre-commit checks: `go mod tidy` verification, - `script/precommit` — pre-commit checks: `go mod tidy` verification,
then `script/check` then `script/check`
- `script/install-precommit` — install the git pre-commit hook that - `script/install-precommit` — install the git pre-commit hook that
+32 -7
View File
@@ -25,6 +25,34 @@ Bring the repo into policy compliance in one commit:
# Completed Steps # Completed Steps
- 2026-10-04: A vault name may use only lowercase ASCII letters, digits,
`.`, `-` and `_`, and must not be empty, `.` or `..`
(https://git.eeqj.de/sneak/secret/issues/68); the error and `README.md`
state the rule. `vault create`, `vault import`, `vault select`,
`vault remove`, both vault names of `mv` and shell completion of a
`vault:secret` argument check the name as typed with
`vault.ValidateVaultName` before building any path from it. Before,
`vault import ..` wrote a long-term key and an unlocker into the state
directory itself, and `vault select ..` made that the current vault.
- 2026-10-04: `script/cibuild` runs the checks again on an unchanged
tree (https://git.eeqj.de/sneak/secret/issues/54). It passes the
current time as the `CHECK_EPOCH` build argument, which both the lint
and the build stage of the `Dockerfile` declare after their module
download, so the `RUN` steps below the argument run again on each
build while the base images and module downloads stay cached. Before,
a second run on the same tree took every check from the build cache
and reported success having run nothing.
- 2026-10-04: A failed unlocker add no longer leaves a partial unlocker
directory (https://git.eeqj.de/sneak/secret/issues/48).
`secret unlocker add pgp` resolves the GPG key's fingerprint once, for
its duplicate check, and passes it to `CreatePGPUnlocker` to record.
`CreatePGPUnlocker` and `CreateKeychainUnlocker` get the long-term key
and encrypt everything before writing anything. All four unlocker
types write their files through `secret.WriteDir`: a new unlocker is
built in a temporary directory, renamed into place when complete and
removed on a failure. One added under the directory name of an
existing unlocker is still written into that directory in place
(https://git.eeqj.de/sneak/secret/issues/71).
- 2026-10-04: `secret unlocker select` and `secret unlocker remove` - 2026-10-04: `secret unlocker select` and `secret unlocker remove`
skip, with the warning `unlocker list` gives, an unlocker directory skip, with the warning `unlocker list` gives, an unlocker directory
whose metadata file cannot be checked for, read or parsed, instead of whose metadata file cannot be checked for, read or parsed, instead of
@@ -129,13 +157,10 @@ Bring the repo into policy compliance in one commit:
- from `init` or `vault create` killed after the passphrase prompt - from `init` or `vault create` killed after the passphrase prompt
but before the unlocker is written, a vault with no unlocker, but before the unlocker is written, a vault with no unlocker,
which `vault create` has already made the current vault; which `vault create` has already made the current vault;
- from an unlocker add stopped before its metadata is written, a - data under a `.tmp-` name in the state directory: a secret,
directory that `unlocker list` warns about and `unlocker rm` version or unlocker being added, or the secret, version, unlocker
removes only by its directory name; or vault being removed, encrypted keys included. Nothing deletes
- data under a `.tmp-` name in the state directory: a secret or it; it must be deleted by hand
version being added, or the secret, version, unlocker or vault
being removed, encrypted keys included. Nothing deletes it; it
must be deleted by hand
(https://git.eeqj.de/sneak/secret/issues/75). (https://git.eeqj.de/sneak/secret/issues/75).
- 2026-10-03: The checks run before changing a vault now stop with an - 2026-10-03: The checks run before changing a vault now stop with an
error naming the path and cause when they cannot read what they error naming the path and cause when they cannot read what they
+7 -1
View File
@@ -123,7 +123,9 @@ func getVaultNamesCompletionFunc(fs afero.Fs, stateDir string) func(
} }
// completeVaultQualifiedSecrets completes "vault:secret" references once a // completeVaultQualifiedSecrets completes "vault:secret" references once a
// colon is present in the input // colon is present in the input. It completes nothing when the vault part
// is not a valid vault name, so that a name such as ".." cannot list a
// directory outside vaults.d.
func completeVaultQualifiedSecrets( func completeVaultQualifiedSecrets(
fs afero.Fs, stateDir, toComplete string, fs afero.Fs, stateDir, toComplete string,
) []string { ) []string {
@@ -134,6 +136,10 @@ func completeVaultQualifiedSecrets(
vaultName := parts[0] vaultName := parts[0]
secretPrefix := parts[1] secretPrefix := parts[1]
if vault.ValidateVaultName(vaultName) != nil {
return nil
}
vlt := vault.NewVault(fs, stateDir, vaultName) vlt := vault.NewVault(fs, stateDir, vaultName)
secrets, err := vlt.ListSecrets() secrets, err := vlt.ListSecrets()
+41
View File
@@ -0,0 +1,41 @@
//nolint:testpackage // white-box test of unexported internals
package cli
import (
"path/filepath"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestVaultSecretCompletionRejectsInvalidVaultName is a regression test for
// https://git.eeqj.de/sneak/secret/issues/68: completing a `vault:secret`
// argument lists nothing when the vault part is not a valid vault name, even
// where that name, joined onto vaults.d, leads to a secrets.d directory.
func TestVaultSecretCompletionRejectsInvalidVaultName(t *testing.T) {
t.Parallel()
const (
stateDir = "/state"
dirPerm = 0o700
)
fs := afero.NewMemMapFs()
// The vault "work" holds the secret "x". So does every directory an
// invalid name below would lead to from vaults.d.
for _, vaultName := range []string{"work", ".", "..", "a/b"} {
secretDir := filepath.Join(stateDir, "vaults.d", vaultName, "secrets.d", "x")
require.NoError(t, fs.MkdirAll(secretDir, dirPerm))
}
assert.Equal(t, []string{"work:x"},
completeVaultQualifiedSecrets(fs, stateDir, "work:"))
for _, toComplete := range []string{".:", "..:", "a/b:"} {
assert.Empty(t, completeVaultQualifiedSecrets(fs, stateDir, toComplete),
"completing %q", toComplete)
}
}
+9 -10
View File
@@ -48,26 +48,25 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
"mv work:nosuch work:y", "work:nosuch", "work:y", false, "mv work:nosuch work:y", "work:nosuch", "work:y", false,
"secret 'nosuch' not found", "secret 'nosuch' not found",
}, },
// Only an existing vault is used, so ".." cannot reach the state // Only an existing vault is used.
// directory itself.
{ {
"mv --force ..:x ..:y", "..:x", "..:y", true, "mv --force nosuch:x nosuch:y", "nosuch:x", "nosuch:y", true,
"vault '..' does not exist", "vault 'nosuch' does not exist",
}, },
// Each of these spells "work" a second way. The spelling is not an // Each of these spells "work" a second way. The spelling is not a
// existing vault name, so the move is not taken for a move between // valid vault name, so the move is not taken for a move between two
// two vaults, which would delete the destination, here the source. // vaults, which would delete the destination, here the source.
{ {
"mv --force work:x work/:x", workX, "work/:x", true, "mv --force work:x work/:x", workX, "work/:x", true,
"vault 'work/' does not exist", vault.ValidateVaultName("work/").Error(),
}, },
{ {
"mv --force work/:x work:", "work/:x", "work:", true, "mv --force work/:x work:", "work/:x", "work:", true,
"vault 'work/' does not exist", vault.ValidateVaultName("work/").Error(),
}, },
{ {
"mv --force work:x ./work:x", workX, "./work:x", true, "mv --force work:x ./work:x", workX, "./work:x", true,
"vault './work' does not exist", vault.ValidateVaultName("./work").Error(),
}, },
} }
+52
View File
@@ -292,6 +292,58 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
} }
} }
// TestInvalidVaultNameLeavesStateUnchanged is a regression test for
// https://git.eeqj.de/sneak/secret/issues/68, where
// `secret vault import ..` wrote a long-term key and an unlocker into the
// state directory itself, and `secret vault select ..` made it the current
// vault. Each command that takes a vault name must reject an invalid one
// before building a path from it. The mnemonic and the passphrase are set,
// and moves and removals use --force, so that only the name check stands
// in the way.
//
//nolint:paralleltest // newTwoVaultFs uses t.Setenv
func TestInvalidVaultNameLeavesStateUnchanged(t *testing.T) {
before := snapshotStateDir(t, newTwoVaultFs(t))
t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
cmd := &cobra.Command{}
// Each command is a format with %q where the vault name goes.
commands := []struct {
command string
run func(c *cli.Instance, name string) error
}{
{"vault create %q", func(c *cli.Instance, name string) error {
return c.CreateVault(cmd, name)
}},
{"vault import %q", func(c *cli.Instance, name string) error {
return c.VaultImport(cmd, name)
}},
{"vault select %q", func(c *cli.Instance, name string) error {
return c.SelectVault(cmd, name)
}},
{"vault remove --force %q", func(c *cli.Instance, name string) error {
return c.RemoveVault(cmd, name, true)
}},
{"mv --force %q:x work:x", func(c *cli.Instance, name string) error {
return c.MoveSecret(cmd, name+":x", "work:x", true)
}},
{"mv --force default:x %q:x", func(c *cli.Instance, name string) error {
return c.MoveSecret(cmd, "default:x", name+":x", true)
}},
}
for _, tt := range commands {
for _, name := range []string{"", ".", "..", "a/b"} {
t.Run(fmt.Sprintf(tt.command, name), func(t *testing.T) {
requireRejectedAndUnchanged(t, before, vault.ValidateVaultName(name),
func(c *cli.Instance) error { return tt.run(c, name) })
})
}
}
}
// TestRemoveVersionRemovesOnlyThatVersion checks that `secret version rm` // TestRemoveVersionRemovesOnlyThatVersion checks that `secret version rm`
// with a version that is not the current one removes that version and // with a version that is not the current one removes that version and
// changes nothing else. // changes nothing else.
+11 -5
View File
@@ -811,9 +811,9 @@ func (cli *Instance) moveSecret(
cmd, vlt, srcSecretName, destSecretName, force) cmd, vlt, srcSecretName, destSecretName, force)
} }
// Both vaults must be existing vaults by exact name, so that two // Both vault names must be valid and name existing vaults exactly, so
// spellings of one vault, such as "work" and "work/", are never taken for // that two spellings of one vault, such as "work" and "work/", are never
// two vaults. A named vault does not become the current vault. // taken for two vaults. A named vault does not become the current vault.
srcVault, err := cli.existingVault(srcVaultName) srcVault, err := cli.existingVault(srcVaultName)
if err != nil { if err != nil {
return err return err
@@ -833,9 +833,15 @@ func (cli *Instance) moveSecret(
cmd, srcVault, srcSecretName, destVault, destSecretName, force) cmd, srcVault, srcSecretName, destVault, destSecretName, force)
} }
// existingVault returns the vault with the given name, or an error if there // existingVault returns the vault with the given name, or an error if the
// is none. Unlike vault.SelectVault, it leaves the current vault as it is. // name is not a valid vault name or there is no such vault. Unlike
// vault.SelectVault, it leaves the current vault as it is.
func (cli *Instance) existingVault(name string) (*vault.Vault, error) { func (cli *Instance) existingVault(name string) (*vault.Vault, error) {
err := vault.ValidateVaultName(name)
if err != nil {
return nil, err
}
vaults, err := vault.ListVaults(cli.fs, cli.stateDir) vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to list vaults: %w", err) return nil, fmt.Errorf("failed to list vaults: %w", err)
+4 -2
View File
@@ -685,7 +685,8 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
return fmt.Errorf("failed to get current vault: %w", err) return fmt.Errorf("failed to get current vault: %w", err)
} }
// Resolve the GPG key ID to its fingerprint // Resolve the GPG key ID to its fingerprint, once: the duplicate check
// and the new unlocker's metadata both use this result
fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID) fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID)
if err != nil { if err != nil {
return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err) return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
@@ -706,7 +707,8 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker) return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
} }
pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir, gpgKeyID) pgpUnlocker, err := secret.CreatePGPUnlocker(
cli.fs, cli.stateDir, gpgKeyID, fingerprint)
if err != nil { if err != nil {
return err return err
} }
+35
View File
@@ -0,0 +1,35 @@
//nolint:testpackage // white-box test of unexported internals
package cli
import (
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
)
// unknownTestGPGUserID is a GPG user ID that no key in the test keyring has.
const unknownTestGPGUserID = "not-in-keyring@example.com"
// TestAddPGPUnlockerUnknownKey asserts that adding a PGP unlocker for a key
// the keyring does not hold fails at looking up the key's fingerprint and
// leaves no new unlocker directory. The error must come from the lookup: a
// lookup moved after anything is written would also come after getting the
// vault's long-term key, which fails first on every platform but macOS
// (https://git.eeqj.de/sneak/secret/issues/88).
//
//nolint:paralleltest // t.Setenv (GNUPGHOME) forbids parallel tests
func TestAddPGPUnlockerUnknownKey(t *testing.T) {
newTestGPGKey(t)
base := newListTestVault(t, 1)
instance, cmd := newTestInstance(base)
cmd.Flags().String("keyid", unknownTestGPGUserID, "")
err := instance.addPGPUnlocker(cmd)
require.ErrorContains(t, err, "failed to resolve GPG key fingerprint")
assertDirEntries(t, base,
filepath.Join(testVaultDir(listTestVaultName), listTestUnlockersDirName),
listTestUnlockerDirOne)
}
+10
View File
@@ -462,6 +462,11 @@ func updateVaultImportMetadata(
// VaultImport imports a mnemonic into a specific vault, holding the state // VaultImport imports a mnemonic into a specific vault, holding the state
// directory lock while importMnemonic runs // directory lock while importMnemonic runs
func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error { func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error {
err := vault.ValidateVaultName(vaultName)
if err != nil {
return err
}
release, err := vault.LockStateDir(cli.fs, cli.stateDir) release, err := vault.LockStateDir(cli.fs, cli.stateDir)
if err != nil { if err != nil {
return err return err
@@ -617,6 +622,11 @@ func (cli *Instance) switchAwayFromVault(
// RemoveVault removes a vault, holding the state directory lock while // RemoveVault removes a vault, holding the state directory lock while
// removeVault runs // removeVault runs
func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error { func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error {
err := vault.ValidateVaultName(name)
if err != nil {
return err
}
release, err := vault.LockStateDir(cli.fs, cli.stateDir) release, err := vault.LockStateDir(cli.fs, cli.stateDir)
if err != nil { if err != nil {
return err return err
+47
View File
@@ -1,6 +1,7 @@
package secret package secret
import ( import (
"errors"
"fmt" "fmt"
"path/filepath" "path/filepath"
@@ -61,6 +62,52 @@ func TempDirFor(fs afero.Fs, target string) (string, error) {
return dir, nil return dir, nil
} }
// WriteDir calls write to write the files of the directory dir. When dir does
// not exist yet, write writes them into a temporary directory from TempDirFor,
// which is then renamed to dir, so that neither a failure nor a crash leaves
// dir half-written; on a failure the temporary directory is removed, and a
// failure to remove it is returned along with the first. A directory cannot be
// renamed over one that has files in it, so when dir already exists, write
// writes into it in place; dir is then never removed.
func WriteDir(fs afero.Fs, dir string, write func(dir string) error) error {
exists, err := afero.Exists(fs, dir)
if err != nil {
return fmt.Errorf("failed to check for %s: %w", dir, err)
}
if exists {
return write(dir)
}
// Create the directory the finished one is renamed into
err = fs.MkdirAll(filepath.Dir(dir), DirPerms)
if err != nil {
return fmt.Errorf("failed to create %s: %w", filepath.Dir(dir), err)
}
tmp, err := TempDirFor(fs, dir)
if err != nil {
return err
}
err = write(tmp)
if err == nil {
err = fs.Rename(tmp, dir)
}
if err != nil {
removeErr := fs.RemoveAll(tmp)
if removeErr != nil {
err = errors.Join(err,
fmt.Errorf("failed to remove %s: %w", tmp, removeErr))
}
return err
}
return nil
}
// RemoveDirAtomic deletes the directory dir so that it disappears in one // RemoveDirAtomic deletes the directory dir so that it disappears in one
// rename: dir is moved into a new directory from TempDirFor, which is then // rename: dir is moved into a new directory from TempDirFor, which is then
// deleted. A crash part-way leaves only that temporary directory behind. // deleted. A crash part-way leaves only that temporary directory behind.
+101 -15
View File
@@ -35,6 +35,10 @@ const currentFile = "current"
// unlockerMetadataFile is the file a new unlocker writes last. // unlockerMetadataFile is the file a new unlocker writes last.
const unlockerMetadataFile = "unlocker-metadata.json" const unlockerMetadataFile = "unlocker-metadata.json"
// privKeyFile is the file that holds the encrypted private key of a version
// or of a passphrase unlocker.
const privKeyFile = "priv.age"
// unlockerPassphrase protects the passphrase unlockers the tests create. // unlockerPassphrase protects the passphrase unlockers the tests create.
// //
//nolint:gosec // G101: test data, not a real credential //nolint:gosec // G101: test data, not a real credential
@@ -453,7 +457,7 @@ func TestVersionSaveIsWholeOrAbsent(t *testing.T) {
if exists { if exists {
assert.ElementsMatch(t, assert.ElementsMatch(t,
[]string{"pub.age", "value.age", "priv.age", "metadata.age"}, []string{"pub.age", "value.age", privKeyFile, "metadata.age"},
dirNames(t, base, versionDir), dirNames(t, base, versionDir),
"version directory visible before it was complete") "version directory visible before it was complete")
} }
@@ -496,7 +500,7 @@ func TestVersionSaveFailureLeavesNothing(t *testing.T) {
writeLongTermKey(t, base, stateDir) writeLongTermKey(t, base, stateDir)
fs := hookFs{Fs: base, before: func(op, path string) error { fs := hookFs{Fs: base, before: func(op, path string) error {
if op == opRename && filepath.Base(path) == "priv.age" { if op == opRename && filepath.Base(path) == privKeyFile {
return errInjected return errInjected
} }
@@ -642,15 +646,21 @@ func TestPassphraseUnlockerGetsKeyFirst(t *testing.T) {
require.Error(t, err) require.Error(t, err)
} }
// TestPassphraseUnlockerWritesMetadataLast checks that the last file a new // TestPassphraseUnlockerIsWholeOrAbsent checks, before every change that
// passphrase unlocker writes in its directory is its metadata: an unlocker // creating a passphrase unlocker makes, that the unlocker's directory either
// directory without metadata is never used, so one interrupted earlier // does not exist or holds all of its files: a crash or a failure at any point
// cannot be. // leaves no partial unlocker.
func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) { //
//nolint:paralleltest // t.Setenv forbids t.Parallel
func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) {
t.Setenv(secret.EnvMnemonic, testMnemonic) t.Setenv(secret.EnvMnemonic, testMnemonic)
base := afero.NewMemMapFs() files := []string{"pub.age", privKeyFile, "longterm.age", unlockerMetadataFile}
vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
for _, tfs := range testFilesystems {
t.Run(tfs.name, func(t *testing.T) {
base, stateDir := tfs.open(t)
vlt, err := vault.CreateVault(base, stateDir, testVaultName)
require.NoError(t, err) require.NoError(t, err)
vaultDir, err := vlt.GetDirectory() vaultDir, err := vlt.GetDirectory()
@@ -658,11 +668,13 @@ func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) {
unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase") unlockerDir := filepath.Join(vaultDir, "unlockers.d", "passphrase")
var last string fs := hookFs{Fs: base, before: func(string, string) error {
exists, err := afero.DirExists(base, unlockerDir)
require.NoError(t, err)
fs := hookFs{Fs: base, before: func(_, path string) error { if exists {
if filepath.Dir(path) == unlockerDir { assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir),
last = filepath.Base(path) "unlocker directory visible before it was complete")
} }
return nil return nil
@@ -671,8 +683,82 @@ func TestPassphraseUnlockerWritesMetadataLast(t *testing.T) {
passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase)) passphrase := memguard.NewBufferFromBytes([]byte(unlockerPassphrase))
defer passphrase.Destroy() defer passphrase.Destroy()
_, err = vault.NewVault(fs, testVaultStateDir, testVaultName). _, err = vault.NewVault(fs, stateDir, testVaultName).
CreatePassphraseUnlocker(passphrase) CreatePassphraseUnlocker(passphrase)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, unlockerMetadataFile, last) assert.ElementsMatch(t, files, dirNames(t, base, unlockerDir))
})
}
}
// TestWriteDirFailureLeavesNothing makes writing a new directory fail after
// a file has been written in it, and checks that neither the directory nor
// its temporary directory is left behind; and, when the temporary directory
// cannot be removed either, that both failures are reported.
func TestWriteDirFailureLeavesNothing(t *testing.T) {
t.Parallel()
for _, tfs := range testFilesystems {
t.Run(tfs.name, func(t *testing.T) {
t.Parallel()
base, dir := tfs.open(t)
listed := filepath.Join(dir, "unlockers.d")
target := filepath.Join(listed, "new")
writeThenFail := func(tmp string) error {
require.NoError(t, secret.WriteFileAtomic(base,
filepath.Join(tmp, unlockerMetadataFile), []byte("{}")))
return errInjected
}
err := secret.WriteDir(base, target, writeThenFail)
require.ErrorIs(t, err, errInjected)
// Nothing in the directory that is listed, nor beside it
assert.Empty(t, dirNames(t, base, listed))
assert.Equal(t, []string{"unlockers.d"}, dirNames(t, base, dir))
fs := hookFs{Fs: base, before: func(op, _ string) error {
if op == opRemove {
return os.ErrPermission
}
return nil
}}
err = secret.WriteDir(fs, target, writeThenFail)
require.ErrorIs(t, err, errInjected)
require.ErrorIs(t, err, os.ErrPermission)
assert.Empty(t, dirNames(t, base, listed))
})
}
}
// TestWriteDirKeepsExistingDir makes writing into a directory that already
// exists fail, and checks that the directory, with what was in it, is still
// there: WriteDir writes into it in place and never removes it.
func TestWriteDirKeepsExistingDir(t *testing.T) {
t.Parallel()
for _, tfs := range testFilesystems {
t.Run(tfs.name, func(t *testing.T) {
t.Parallel()
fs, dir := tfs.open(t)
target := filepath.Join(dir, "unlockers.d", "passphrase")
require.NoError(t, fs.MkdirAll(target, secret.DirPerms))
require.NoError(t, secret.WriteFileAtomic(fs,
filepath.Join(target, unlockerMetadataFile), []byte("{}")))
err := secret.WriteDir(fs, target, func(got string) error {
assert.Equal(t, target, got)
return errInjected
})
require.ErrorIs(t, err, errInjected)
assert.Equal(t, []string{unlockerMetadataFile}, dirNames(t, fs, target))
})
}
} }
+39 -36
View File
@@ -341,16 +341,13 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
return nil, fmt.Errorf("failed to generate keychain item name: %w", err) return nil, fmt.Errorf("failed to generate keychain item name: %w", err)
} }
// Create unlocker directory using the keychain item name as the directory name // The unlocker directory is named after the keychain item
vaultDir, err := vault.GetDirectory() vaultDir, err := vault.GetDirectory()
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to get vault directory: %w", err) return nil, fmt.Errorf("failed to get vault directory: %w", err)
} }
unlockerDir := filepath.Join(vaultDir, "unlockers.d", keychainItemName) unlockerDir := filepath.Join(vaultDir, "unlockers.d", keychainItemName)
if err := fs.MkdirAll(unlockerDir, DirPerms); err != nil {
return nil, fmt.Errorf("failed to create unlocker directory: %w", err)
}
// Step 1: Generate a new age keypair for the keychain unlocker // Step 1: Generate a new age keypair for the keychain unlocker
ageIdentity, err := age.GenerateX25519Identity() ageIdentity, err := age.GenerateX25519Identity()
@@ -358,6 +355,8 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
return nil, fmt.Errorf("failed to generate age keypair: %w", err) return nil, fmt.Errorf("failed to generate age keypair: %w", err)
} }
ageRecipient := ageIdentity.Recipient().String()
// Step 2: Generate a random passphrase for encrypting the age private key // Step 2: Generate a random passphrase for encrypting the age private key
agePrivKeyPassphrase, err := generateRandomPassphrase(agePrivKeyPassphraseLength) agePrivKeyPassphrase, err := generateRandomPassphrase(agePrivKeyPassphraseLength)
if err != nil { if err != nil {
@@ -365,14 +364,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
} }
defer agePrivKeyPassphrase.Destroy() defer agePrivKeyPassphrase.Destroy()
// Step 3: Store age recipient as plaintext // Step 3: Encrypt age private key with the generated passphrase
ageRecipient := ageIdentity.Recipient().String()
recipientPath := filepath.Join(unlockerDir, "pub.txt")
if err := WriteFileAtomic(fs, recipientPath, []byte(ageRecipient)); err != nil {
return nil, fmt.Errorf("failed to write age recipient: %w", err)
}
// Step 4: Encrypt age private key with the generated passphrase and store on disk
// Create a secure buffer for the private key // Create a secure buffer for the private key
agePrivKeyStr := ageIdentity.String() agePrivKeyStr := ageIdentity.String()
agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr)) agePrivKeyBuffer := memguard.NewBufferFromBytes([]byte(agePrivKeyStr))
@@ -383,31 +375,20 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err) return nil, fmt.Errorf("failed to encrypt age private key with passphrase: %w", err)
} }
agePrivKeyPath := filepath.Join(unlockerDir, "priv.age") // Step 4: Get or derive the long-term private key
if err := WriteFileAtomic(fs, agePrivKeyPath, encryptedAgePrivKey); err != nil {
return nil, fmt.Errorf("failed to write encrypted age private key: %w", err)
}
// Step 5: Get or derive the long-term private key
ltPrivKeyData, err := getLongTermPrivateKey(fs, vault) ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
if err != nil { if err != nil {
return nil, err return nil, err
} }
defer ltPrivKeyData.Destroy() defer ltPrivKeyData.Destroy()
// Step 6: Encrypt long-term private key to the new age unlocker // Step 5: Encrypt long-term private key to the new age unlocker
encryptedLtPrivKeyToAge, err := EncryptToRecipient(ltPrivKeyData, ageIdentity.Recipient()) encryptedLtPrivKeyToAge, err := EncryptToRecipient(ltPrivKeyData, ageIdentity.Recipient())
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to encrypt long-term private key to age unlocker: %w", err) return nil, fmt.Errorf("failed to encrypt long-term private key to age unlocker: %w", err)
} }
// Write encrypted long-term private key // Step 6: Prepare keychain data
ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
if err := WriteFileAtomic(fs, ltPrivKeyPath, encryptedLtPrivKeyToAge); err != nil {
return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
}
// Step 7: Prepare keychain data
keychainData := KeychainData{ keychainData := KeychainData{
AgePublicKey: ageRecipient, AgePublicKey: ageRecipient,
AgePrivKeyPassphrase: agePrivKeyPassphrase, AgePrivKeyPassphrase: agePrivKeyPassphrase,
@@ -420,12 +401,7 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
} }
defer keychainDataBuffer.Destroy() defer keychainDataBuffer.Destroy()
// Step 8: Store data in keychain // Step 7: Prepare enhanced metadata
if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil {
return nil, fmt.Errorf("failed to store data in keychain: %w", err)
}
// Step 9: Create and write enhanced metadata
keychainMetadata := KeychainUnlockerMetadata{ keychainMetadata := KeychainUnlockerMetadata{
UnlockerMetadata: UnlockerMetadata{ UnlockerMetadata: UnlockerMetadata{
Type: "keychain", Type: "keychain",
@@ -440,10 +416,37 @@ func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, er
return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err) return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
} }
if err := WriteFileAtomic(fs, // Step 8: Write the unlocker's files and store the data in the keychain,
filepath.Join(unlockerDir, "unlocker-metadata.json"), // the metadata last
metadataBytes); err != nil { err = WriteDir(fs, unlockerDir, func(dir string) error {
return nil, fmt.Errorf("failed to write unlocker metadata: %w", err) pubPath := filepath.Join(dir, "pub.txt")
if err := WriteFileAtomic(fs, pubPath, []byte(ageRecipient)); err != nil {
return fmt.Errorf("failed to write age recipient: %w", err)
}
privPath := filepath.Join(dir, "priv.age")
if err := WriteFileAtomic(fs, privPath, encryptedAgePrivKey); err != nil {
return fmt.Errorf("failed to write encrypted age private key: %w", err)
}
ltKeyPath := filepath.Join(dir, "longterm.age")
if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtPrivKeyToAge); err != nil {
return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
}
if err := storeInKeychain(keychainItemName, keychainDataBuffer); err != nil {
return fmt.Errorf("failed to store data in keychain: %w", err)
}
metadataPath := filepath.Join(dir, "unlocker-metadata.json")
if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
return fmt.Errorf("failed to write unlocker metadata: %w", err)
}
return nil
})
if err != nil {
return nil, err
} }
return &KeychainUnlocker{ return &KeychainUnlocker{
+1 -1
View File
@@ -290,7 +290,7 @@ Passphrase: ` + testPassphrase + `
} }
// Now create a PGP unlock key (this will use our custom GPGEncryptFunc) // Now create a PGP unlock key (this will use our custom GPGEncryptFunc)
pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID) pgpUnlocker, err := secret.CreatePGPUnlocker(fs, stateDir, keyID, fingerprint)
if err != nil { if err != nil {
t.Fatalf("Failed to create PGP unlock key: %v", err) t.Fatalf("Failed to create PGP unlock key: %v", err)
} }
+97 -94
View File
@@ -222,20 +222,13 @@ func generatePGPUnlockerName() (string, error) {
return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentDate), nil return fmt.Sprintf("%s-pgp-%s", hostname, enrollmentDate), nil
} }
// preparePGPUnlockerDir checks GPG availability and creates the // pgpUnlockerDir returns the current vault and the directory in it for a
// unlocker directory in the current vault, returning the vault and the // new PGP unlocker, named after the host and the day.
// directory path.
// //
//nolint:ireturn // the vault is only available behind VaultInterface //nolint:ireturn // the vault is only available behind VaultInterface
func preparePGPUnlockerDir( func pgpUnlockerDir(
fs afero.Fs, stateDir string, fs afero.Fs, stateDir string,
) (VaultInterface, string, error) { ) (VaultInterface, string, error) {
// Check if GPG is available
err := checkGPGAvailable()
if err != nil {
return nil, "", err
}
// Get current vault // Get current vault
vault, err := GetCurrentVault(fs, stateDir) vault, err := GetCurrentVault(fs, stateDir)
if err != nil { if err != nil {
@@ -248,27 +241,29 @@ func preparePGPUnlockerDir(
return nil, "", fmt.Errorf("failed to generate unlocker name: %w", err) return nil, "", fmt.Errorf("failed to generate unlocker name: %w", err)
} }
// Create unlocker directory using the generated name
vaultDir, err := vault.GetDirectory() vaultDir, err := vault.GetDirectory()
if err != nil { if err != nil {
return nil, "", fmt.Errorf("failed to get vault directory: %w", err) return nil, "", fmt.Errorf("failed to get vault directory: %w", err)
} }
unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerName) return vault, filepath.Join(vaultDir, "unlockers.d", unlockerName), nil
err = fs.MkdirAll(unlockerDir, DirPerms)
if err != nil {
return nil, "", fmt.Errorf("failed to create unlocker directory: %w", err)
}
return vault, unlockerDir, nil
} }
// CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault // CreatePGPUnlocker creates a new PGP unlocker and stores it in the vault.
// It encrypts to the GPG key gpgKeyID and records fingerprint, that key's
// fingerprint as ResolveGPGKeyFingerprint returns it, in the metadata.
// Everything that can fail short of writing a file is done before anything
// is written, and the files are written through WriteDir, so a failure
// leaves no partial unlocker.
func CreatePGPUnlocker( func CreatePGPUnlocker(
fs afero.Fs, stateDir string, gpgKeyID string, fs afero.Fs, stateDir, gpgKeyID, fingerprint string,
) (*PGPUnlocker, error) { ) (*PGPUnlocker, error) {
vault, unlockerDir, err := preparePGPUnlockerDir(fs, stateDir) err := checkGPGAvailable()
if err != nil {
return nil, err
}
vault, unlockerDir, err := pgpUnlockerDir(fs, stateDir)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -279,77 +274,13 @@ func CreatePGPUnlocker(
return nil, fmt.Errorf("failed to generate age keypair: %w", err) return nil, fmt.Errorf("failed to generate age keypair: %w", err)
} }
// Step 2: Store age recipient as plaintext // Step 2: Encrypt the long-term private key to the new keypair, and the
ageRecipient := ageIdentity.Recipient().String() // keypair's private key to the GPG key
recipientPath := filepath.Join(unlockerDir, "pub.txt") encryptedLtPrivKey, encryptedAgePrivKey, err := encryptPGPUnlockerKeys(
fs, vault, ageIdentity, gpgKeyID)
err = WriteFileAtomic(fs, recipientPath, []byte(ageRecipient))
if err != nil {
return nil, fmt.Errorf("failed to write age recipient: %w", err)
}
// Step 3: Get or derive the long-term private key
ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
if err != nil { if err != nil {
return nil, err return nil, err
} }
defer ltPrivKeyData.Destroy()
// Step 7: Encrypt long-term private key to the new age unlocker
encryptedLtPrivKeyToAge, err := EncryptToRecipient(
ltPrivKeyData, ageIdentity.Recipient())
if err != nil {
return nil, fmt.Errorf(
"failed to encrypt long-term private key to age unlocker: %w", err)
}
// Write encrypted long-term private key
ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
err = WriteFileAtomic(fs, ltPrivKeyPath, encryptedLtPrivKeyToAge)
if err != nil {
return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
}
// Step 8: Encrypt age private key to the GPG key ID
// Use memguard to protect the private key in memory
agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
defer agePrivateKeyBuffer.Destroy()
encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
if err != nil {
return nil, fmt.Errorf("failed to encrypt age private key with GPG: %w", err)
}
agePrivKeyPath := filepath.Join(unlockerDir, "priv.age.gpg")
err = WriteFileAtomic(fs, agePrivKeyPath, encryptedAgePrivKey)
if err != nil {
return nil, fmt.Errorf("failed to write encrypted age private key: %w", err)
}
// Steps 9-10: Resolve the fingerprint and write enhanced metadata
pgpMetadata, err := writePGPUnlockerMetadata(fs, unlockerDir, gpgKeyID)
if err != nil {
return nil, err
}
return &PGPUnlocker{
Directory: unlockerDir,
Metadata: pgpMetadata.UnlockerMetadata,
fs: fs,
}, nil
}
// writePGPUnlockerMetadata resolves the GPG key fingerprint and writes
// the unlocker metadata file, returning the metadata written.
func writePGPUnlockerMetadata(
fs afero.Fs, unlockerDir string, gpgKeyID string,
) (*PGPUnlockerMetadata, error) {
fingerprint, err := ResolveGPGKeyFingerprint(gpgKeyID)
if err != nil {
return nil, fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
}
pgpMetadata := PGPUnlockerMetadata{ pgpMetadata := PGPUnlockerMetadata{
UnlockerMetadata: UnlockerMetadata{ UnlockerMetadata: UnlockerMetadata{
@@ -365,13 +296,85 @@ func writePGPUnlockerMetadata(
return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err) return nil, fmt.Errorf("failed to marshal unlocker metadata: %w", err)
} }
err = WriteFileAtomic(fs, // Step 3: Write the unlocker's files, the metadata last
filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes) err = WriteDir(fs, unlockerDir, func(dir string) error {
return writePGPUnlockerFiles(fs, dir, ageIdentity.Recipient(),
encryptedLtPrivKey, encryptedAgePrivKey, metadataBytes)
})
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to write unlocker metadata: %w", err) return nil, err
} }
return &pgpMetadata, nil return &PGPUnlocker{
Directory: unlockerDir,
Metadata: pgpMetadata.UnlockerMetadata,
fs: fs,
}, nil
}
// encryptPGPUnlockerKeys returns the vault's long-term private key encrypted
// to the new PGP unlocker's age keypair, and that keypair's private key
// encrypted to the GPG key gpgKeyID.
func encryptPGPUnlockerKeys(
fs afero.Fs, vault VaultInterface,
ageIdentity *age.X25519Identity, gpgKeyID string,
) ([]byte, []byte, error) {
// Get or derive the long-term private key
ltPrivKeyData, err := getLongTermPrivateKey(fs, vault)
if err != nil {
return nil, nil, err
}
defer ltPrivKeyData.Destroy()
encryptedLtPrivKey, err := EncryptToRecipient(
ltPrivKeyData, ageIdentity.Recipient())
if err != nil {
return nil, nil, fmt.Errorf(
"failed to encrypt long-term private key to age unlocker: %w", err)
}
// Use memguard to protect the private key in memory
agePrivateKeyBuffer := memguard.NewBufferFromBytes([]byte(ageIdentity.String()))
defer agePrivateKeyBuffer.Destroy()
encryptedAgePrivKey, err := GPGEncryptFunc(agePrivateKeyBuffer, gpgKeyID)
if err != nil {
return nil, nil, fmt.Errorf(
"failed to encrypt age private key with GPG: %w", err)
}
return encryptedLtPrivKey, encryptedAgePrivKey, nil
}
// writePGPUnlockerFiles writes the files of a PGP unlocker into dir, the
// metadata last.
func writePGPUnlockerFiles(
fs afero.Fs, dir string, ageRecipient *age.X25519Recipient,
encryptedLtPrivKey, encryptedAgePrivKey, metadataBytes []byte,
) error {
err := WriteFileAtomic(fs, filepath.Join(dir, "pub.txt"),
[]byte(ageRecipient.String()))
if err != nil {
return fmt.Errorf("failed to write age recipient: %w", err)
}
err = WriteFileAtomic(fs, filepath.Join(dir, "longterm.age"), encryptedLtPrivKey)
if err != nil {
return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
}
err = WriteFileAtomic(fs, filepath.Join(dir, "priv.age.gpg"), encryptedAgePrivKey)
if err != nil {
return fmt.Errorf("failed to write encrypted age private key: %w", err)
}
err = WriteFileAtomic(fs,
filepath.Join(dir, "unlocker-metadata.json"), metadataBytes)
if err != nil {
return fmt.Errorf("failed to write unlocker metadata: %w", err)
}
return nil
} }
// validateGPGKeyID validates that a GPG key ID is safe for command execution // validateGPGKeyID validates that a GPG key ID is safe for command execution
+67
View File
@@ -0,0 +1,67 @@
package secret_test
import (
"os"
"path/filepath"
"testing"
"git.eeqj.de/sneak/secret/internal/secret"
"git.eeqj.de/sneak/secret/internal/vault"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// The GPG key ID and fingerprint passed to CreatePGPUnlocker.
const (
testGPGKeyID = "0123456789ABCDEF"
testGPGFingerprint = "0123456789ABCDEF0123456789ABCDEF01234567"
)
// fakeGPGScript is a gpg for which `gpg --version` succeeds and anything
// else fails.
const fakeGPGScript = `#!/bin/sh
[ "$*" = --version ]
`
// installFakeGPG makes fakeGPGScript the only gpg on PATH for the test.
func installFakeGPG(t *testing.T) {
t.Helper()
dir := t.TempDir()
//nolint:gosec // G306: the script must be executable
err := os.WriteFile(filepath.Join(dir, "gpg"), []byte(fakeGPGScript), 0o700)
require.NoError(t, err)
t.Setenv("PATH", dir)
}
// TestCreatePGPUnlockerFailureWritesNothing makes CreatePGPUnlocker fail at
// getting the vault's long-term key, which used to come after part of the
// unlocker was written, and asserts that nothing is written. Getting the key
// fails because on macOS there is no mnemonic and no current unlocker, and
// on every other platform it always fails
// (https://git.eeqj.de/sneak/secret/issues/88).
func TestCreatePGPUnlockerFailureWritesNothing(t *testing.T) {
installFakeGPG(t)
t.Setenv(secret.EnvMnemonic, "")
base := afero.NewMemMapFs()
vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName)
require.NoError(t, err)
fs := hookFs{Fs: base, before: func(_, path string) error {
t.Errorf("changed %s", path)
return nil
}}
_, err = secret.CreatePGPUnlocker(
fs, testVaultStateDir, testGPGKeyID, testGPGFingerprint)
require.Error(t, err)
vaultDir, err := vlt.GetDirectory()
require.NoError(t, err)
assert.Empty(t, dirNames(t, base, filepath.Join(vaultDir, "unlockers.d")))
}
+19 -19
View File
@@ -254,7 +254,7 @@ func CreateSecureEnclaveUnlocker(
) )
} }
// Step 4: Create unlocker directory and write files // Step 4: Prepare the unlocker directory's path and metadata
vaultDir, err := vault.GetDirectory() vaultDir, err := vault.GetDirectory()
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to get vault directory: %w", err) return nil, fmt.Errorf("failed to get vault directory: %w", err)
@@ -262,23 +262,7 @@ func CreateSecureEnclaveUnlocker(
unlockerDirName := fmt.Sprintf("se-%s", filepath.Base(seKeyLabel)) unlockerDirName := fmt.Sprintf("se-%s", filepath.Base(seKeyLabel))
unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerDirName) unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerDirName)
if err := fs.MkdirAll(unlockerDir, DirPerms); err != nil {
return nil, fmt.Errorf(
"failed to create unlocker directory: %w",
err,
)
}
// Write SE-encrypted long-term key
ltKeyPath := filepath.Join(unlockerDir, seLongtermFilename)
if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtKey); err != nil {
return nil, fmt.Errorf(
"failed to write SE-encrypted long-term key: %w",
err,
)
}
// Write metadata
seMetadata := SecureEnclaveUnlockerMetadata{ seMetadata := SecureEnclaveUnlockerMetadata{
UnlockerMetadata: UnlockerMetadata{ UnlockerMetadata: UnlockerMetadata{
Type: seUnlockerType, Type: seUnlockerType,
@@ -294,9 +278,25 @@ func CreateSecureEnclaveUnlocker(
return nil, fmt.Errorf("failed to marshal metadata: %w", err) return nil, fmt.Errorf("failed to marshal metadata: %w", err)
} }
metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json") // Step 5: Write the SE-encrypted long-term key, then the metadata
err = WriteDir(fs, unlockerDir, func(dir string) error {
ltKeyPath := filepath.Join(dir, seLongtermFilename)
if err := WriteFileAtomic(fs, ltKeyPath, encryptedLtKey); err != nil {
return fmt.Errorf(
"failed to write SE-encrypted long-term key: %w",
err,
)
}
metadataPath := filepath.Join(dir, "unlocker-metadata.json")
if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil { if err := WriteFileAtomic(fs, metadataPath, metadataBytes); err != nil {
return nil, fmt.Errorf("failed to write metadata: %w", err) return fmt.Errorf("failed to write metadata: %w", err)
}
return nil
})
if err != nil {
return nil, err
} }
return &SecureEnclaveUnlocker{ return &SecureEnclaveUnlocker{
+4 -3
View File
@@ -17,9 +17,10 @@ var (
"derived public key does not match vault: mnemonic may be incorrect", "derived public key does not match vault: mnemonic may be incorrect",
) )
// ErrInvalidVaultName indicates a vault name that does not match the // ErrInvalidVaultName indicates a vault name that breaks the naming
// allowed pattern [a-z0-9.\-_]+. Composed as // rule: only lowercase ASCII letters, digits, '.', '-' and '_'; not
// "invalid vault name '<name>': must match pattern [a-z0-9.\-_]+". // empty, "." or "..". Composed by ValidateVaultName as
// "invalid vault name '<name>': <the rule>".
ErrInvalidVaultName = errors.New("invalid vault name") ErrInvalidVaultName = errors.New("invalid vault name")
// ErrVaultNotFound indicates the named vault does not exist. Composed // ErrVaultNotFound indicates the named vault does not exist. Composed
+26 -15
View File
@@ -24,10 +24,12 @@ func init() {
}) })
} }
// isValidVaultName validates vault names according to the format [a-z0-9\.\-\_]+ // isValidVaultName reports whether name is a valid vault name: only
// Note: We don't allow slashes in vault names unlike secret names // lowercase ASCII letters, digits, '.', '-' and '_', and not empty, "." or
// "..". With no path separator allowed, a vault is always one directory
// directly under vaults.d.
func isValidVaultName(name string) bool { func isValidVaultName(name string) bool {
if name == "" { if name == "" || name == "." || name == ".." {
return false return false
} }
@@ -36,6 +38,21 @@ func isValidVaultName(name string) bool {
return matched return matched
} }
// ValidateVaultName returns an error wrapping ErrInvalidVaultName when name
// is not a valid vault name. Call it on the name exactly as the user gave it,
// before building any path from it.
func ValidateVaultName(name string) error {
if !isValidVaultName(name) {
return fmt.Errorf(
"%w '%s': only lowercase ASCII letters, digits, '.', '-' and '_' "+
"are allowed, and a name must not be empty, '.' or '..'",
ErrInvalidVaultName, name,
)
}
return nil
}
// ResolveVaultSymlink reads the currentvault file to get the path to the current vault // ResolveVaultSymlink reads the currentvault file to get the path to the current vault
// The file contains just the vault name (e.g., "default") // The file contains just the vault name (e.g., "default")
func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) { func ResolveVaultSymlink(fs afero.Fs, currentVaultPath string) (string, error) {
@@ -199,14 +216,11 @@ func processMnemonicForVault(
func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) { func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
secret.Debug("Creating new vault", "name", name, "state_dir", stateDir) secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
// Validate vault name err := ValidateVaultName(name)
if !isValidVaultName(name) { if err != nil {
secret.Debug("Invalid vault name provided", "vault_name", name) secret.Debug("Invalid vault name provided", "vault_name", name)
return nil, fmt.Errorf( return nil, err
"%w '%s': must match pattern [a-z0-9.\\-_]+",
ErrInvalidVaultName, name,
)
} }
secret.Debug("Vault name validation passed", "vault_name", name) secret.Debug("Vault name validation passed", "vault_name", name)
@@ -285,14 +299,11 @@ func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
func SelectVault(fs afero.Fs, stateDir string, name string) error { func SelectVault(fs afero.Fs, stateDir string, name string) error {
secret.Debug("Selecting vault", "vault_name", name, "state_dir", stateDir) secret.Debug("Selecting vault", "vault_name", name, "state_dir", stateDir)
// Validate vault name err := ValidateVaultName(name)
if !isValidVaultName(name) { if err != nil {
secret.Debug("Invalid vault name provided", "vault_name", name) secret.Debug("Invalid vault name provided", "vault_name", name)
return fmt.Errorf( return err
"%w '%s': must match pattern [a-z0-9.\\-_]+",
ErrInvalidVaultName, name,
)
} }
secret.Debug("Vault name validation passed", "vault_name", name) secret.Debug("Vault name validation passed", "vault_name", name)
+23 -28
View File
@@ -357,26 +357,14 @@ func (v *Vault) CreatePassphraseUnlocker(
return nil, fmt.Errorf("failed to get long-term key: %w", err) return nil, fmt.Errorf("failed to get long-term key: %w", err)
} }
// Create unlocker directory
unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerTypePassphrase) unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerTypePassphrase)
err = v.fs.MkdirAll(unlockerDir, secret.DirPerms)
if err != nil {
return nil, fmt.Errorf("failed to create unlocker directory: %w", err)
}
// Generate new age keypair for unlocker // Generate new age keypair for unlocker
unlockerIdentity, err := age.GenerateX25519Identity() unlockerIdentity, err := age.GenerateX25519Identity()
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to generate unlocker: %w", err) return nil, fmt.Errorf("failed to generate unlocker: %w", err)
} }
// Write the unlocker keypair (public and passphrase-encrypted private)
err = v.writeUnlockerKeypair(unlockerDir, unlockerIdentity, passphrase)
if err != nil {
return nil, err
}
// Encrypt long-term private key to this unlocker // Encrypt long-term private key to this unlocker
ltPrivKeyBuffer := memguard.NewBufferFromBytes([]byte(ltIdentity.String())) ltPrivKeyBuffer := memguard.NewBufferFromBytes([]byte(ltIdentity.String()))
defer ltPrivKeyBuffer.Destroy() defer ltPrivKeyBuffer.Destroy()
@@ -387,15 +375,6 @@ func (v *Vault) CreatePassphraseUnlocker(
return nil, fmt.Errorf("failed to encrypt long-term private key: %w", err) return nil, fmt.Errorf("failed to encrypt long-term private key: %w", err)
} }
ltPrivKeyPath := filepath.Join(unlockerDir, "longterm.age")
err = secret.WriteFileAtomic(v.fs, ltPrivKeyPath, encryptedLtPrivKey)
if err != nil {
return nil, fmt.Errorf("failed to write encrypted long-term private key: %w", err)
}
// Write the metadata last: readers skip an unlocker directory without
// it, so an unlocker interrupted before this point is never used.
metadata := UnlockerMetadata{ metadata := UnlockerMetadata{
Type: unlockerTypePassphrase, Type: unlockerTypePassphrase,
CreatedAt: time.Now(), CreatedAt: time.Now(),
@@ -407,11 +386,13 @@ func (v *Vault) CreatePassphraseUnlocker(
return nil, fmt.Errorf("failed to marshal metadata: %w", err) return nil, fmt.Errorf("failed to marshal metadata: %w", err)
} }
metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json") // Write the unlocker's files, the metadata last
err = secret.WriteDir(v.fs, unlockerDir, func(dir string) error {
err = secret.WriteFileAtomic(v.fs, metadataPath, metadataBytes) return v.writeUnlockerFiles(dir, unlockerIdentity, passphrase,
encryptedLtPrivKey, metadataBytes)
})
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to write unlocker metadata: %w", err) return nil, err
} }
// Create the unlocker instance // Create the unlocker instance
@@ -457,12 +438,14 @@ func (v *Vault) readUnlockerMetadata(unlockerDir string) (UnlockerMetadata, erro
return metadata, nil return metadata, nil
} }
// writeUnlockerKeypair writes the unlocker's public key and its // writeUnlockerFiles writes the files of a passphrase unlocker into
// passphrase-encrypted private key into the unlocker directory. // unlockerDir: its public key, its passphrase-encrypted private key, the
func (v *Vault) writeUnlockerKeypair( // long-term private key encrypted to it, and its metadata, last.
func (v *Vault) writeUnlockerFiles(
unlockerDir string, unlockerDir string,
unlockerIdentity *age.X25519Identity, unlockerIdentity *age.X25519Identity,
passphrase *memguard.LockedBuffer, passphrase *memguard.LockedBuffer,
encryptedLtPrivKey, metadataBytes []byte,
) error { ) error {
// Write public key // Write public key
pubKeyPath := filepath.Join(unlockerDir, "pub.age") pubKeyPath := filepath.Join(unlockerDir, "pub.age")
@@ -492,5 +475,17 @@ func (v *Vault) writeUnlockerKeypair(
return fmt.Errorf("failed to write encrypted unlocker private key: %w", err) return fmt.Errorf("failed to write encrypted unlocker private key: %w", err)
} }
err = secret.WriteFileAtomic(v.fs,
filepath.Join(unlockerDir, "longterm.age"), encryptedLtPrivKey)
if err != nil {
return fmt.Errorf("failed to write encrypted long-term private key: %w", err)
}
err = secret.WriteFileAtomic(v.fs,
filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes)
if err != nil {
return fmt.Errorf("failed to write unlocker metadata: %w", err)
}
return nil return nil
} }
+5 -1
View File
@@ -4,13 +4,17 @@
# The Gitea workflow runs this on push. The memlock ulimit lets the tests # The Gitea workflow runs this on push. The memlock ulimit lets the tests
# that lock large secrets in memory (memguard mlocks them) run; under the # that lock large secrets in memory (memguard mlocks them) run; under the
# lower limit of a plain `docker build .` they are skipped. # lower limit of a plain `docker build .` they are skipped.
# A cached build checks nothing: a new CHECK_EPOCH on every run makes the
# Dockerfile's check steps run again on an unchanged tree, while its base
# images and module downloads stay cached.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
docker build --ulimit memlock=-1:-1 . docker build --ulimit memlock=-1:-1 \
--build-arg CHECK_EPOCH="$(date +%s)" .
} }
main "$@" main "$@"