From 204eb4f32baf2ec325cd7f78a12ae0f862954aee Mon Sep 17 00:00:00 2001 From: Michal Vyskocil Date: Fri, 23 Oct 2026 15:06:14 +0200 Subject: [PATCH 1/3] identity.gpg: generate the sandbox's gpg.conf files from the host's The sandbox gpg client ran on compiled-in defaults, so a host that tightened digest/cipher preferences, weak-digest, compliance or the trust model signed, encrypted and verified by stricter rules than gpg inside (invariant 7). gpgconf --list-options gpg lists none of the preference options (gpg 2.5.24), so snug reads the files: /gpg.conf, and the first of gpg.conf-..gpg.conf in the host GNUPGHOME, as gpg does. Each is rewritten from an allowlist of crypto/trust options, aliases and compliance shortcuts mapped to the carried spelling, system meta commands carried, into two generated files so gpg inside applies its own precedence. An allowlisted line snug cannot carry refuses the run. Refs #689 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/src/identity.md | 33 ++ internal/cli/gpgconf.go | 214 ++++++++++++ internal/cli/gpgconf_test.go | 215 ++++++++++++ internal/cli/gpgidentity.go | 12 +- internal/cli/gpgidentitygolden_test.go | 6 +- .../cli/testdata/gpg-decrypt.filesystem.txt | 1 + internal/cli/testdata/gpg.filesystem.txt | 1 + .../testdata/identity-gpg-decrypt.bwrap.txt | 13 +- internal/cli/testdata/identity-gpg.bwrap.txt | 13 +- internal/policy/bookgpgconf_test.go | 54 +++ internal/policy/gpgconf.go | 310 ++++++++++++++++++ internal/policy/gpgconf_test.go | 231 +++++++++++++ internal/policy/testdata/gpgconf-empty.txt | 10 + internal/policy/testdata/gpgconf-system.txt | 13 + internal/policy/testdata/gpgconf-user.txt | 12 + test/integration/identitygpg_test.go | 133 ++++++++ 16 files changed, 1256 insertions(+), 15 deletions(-) create mode 100644 internal/cli/gpgconf.go create mode 100644 internal/cli/gpgconf_test.go create mode 100644 internal/policy/bookgpgconf_test.go create mode 100644 internal/policy/gpgconf.go create mode 100644 internal/policy/gpgconf_test.go create mode 100644 internal/policy/testdata/gpgconf-empty.txt create mode 100644 internal/policy/testdata/gpgconf-system.txt create mode 100644 internal/policy/testdata/gpgconf-user.txt diff --git a/docs/src/identity.md b/docs/src/identity.md index 7f477465..d8e0edd9 100644 --- a/docs/src/identity.md +++ b/docs/src/identity.md @@ -190,6 +190,39 @@ keeps one selected key per connection whichever verb selected it, so the proxy also checks that the signing or decrypting command follows a selection made for that purpose. +#### Your gpg.conf + +The gpg client inside makes the digest, cipher and trust choices your host's +gpg makes. snug reads your `gpg.conf` (the one gpg reads: `gpg.conf-` +first, as gpg does) and the system-wide one in gpg's sysconfdir +(`/etc/gnupg/gpg.conf`), and writes the sandbox its own copy of each holding +only these options: + +- digest and cipher choices: `personal-digest-preferences`, + `personal-cipher-preferences`, `personal-aead-preferences`, `digest-algo`, + `cert-digest-algo`, `cipher-algo`, `aead-algo`, `s2k-mode`, `s2k-digest-algo`, + `s2k-cipher-algo`, `s2k-count`, `force-aead`, `no-symkey-cache` +- what verification rejects: `weak-digest`, `disable-cipher-algo`, + `disable-pubkey-algo`, `assert-pubkey-algo`, `require-cross-certification`, + `require-backsigs`, `require-secmem`, `require-pqc-encryption` +- compliance and trust: `compliance`, `require-compliance`, `trust-model`, + `completes-needed`, `marginals-needed`, `max-cert-depth`, `min-cert-level`, + `trusted-key`, `tofu-default-policy`, `assert-signer` + +gpg's other spellings of these (`personal-cipher-prefs`, `force-ocb`, and the +compliance shortcuts such as `openpgp`) are carried too, rewritten to the names +above, in your file's order. + +The system file's `[force]`, `[ignore]` and `[ignore-all]` lines are kept in +place, so gpg inside applies them as yours does. Everything else stays behind: +options that run a program (`exec-path`, `photo-viewer`), that reach the +network (`keyserver`, `auto-key-retrieve`, `auto-key-locate`), and the +relaxations (`allow-weak-digest-algos`, `always-trust`, and the like). Without a +relaxation, gpg inside is stricter than yours, never looser. + +If snug cannot read either file, or a kept option has a value it cannot write +back exactly, it refuses to start and names the file and line. + A hostile process inside the sandbox can use this block to sign anything as you with the pinned signing subkey, and to decrypt anything addressed to the pinned encryption subkey that it can read, for as long as the run lasts. It cannot diff --git a/internal/cli/gpgconf.go b/internal/cli/gpgconf.go new file mode 100644 index 00000000..84a0745a --- /dev/null +++ b/internal/cli/gpgconf.go @@ -0,0 +1,214 @@ +package cli + +import ( + "context" + "errors" + "fmt" + "io/fs" + "os/exec" + "path/filepath" + "regexp" + "strings" + + "github.com/gomoni/snug/internal/hostread" + "github.com/gomoni/snug/internal/policy" +) + +// gpgConfFiles is the host's gpg configuration as gpg itself would read it: +// the system-wide file and the ONE user file gpg picks, each as raw bytes and +// the path they came from. An absent file is a nil content and an empty path. +// The system file's host path is also its guest path: the sandbox runs the +// same gpg, with the same compiled-in sysconfdir. +type gpgConfFiles struct { + system []byte + systemPath string + user []byte + userPath string +} + +// probeGPGConf finds the configuration the host's gpg CLIENT runs on, so the +// sandbox's gpg makes the same digest, cipher and trust choices. +// +// WHY NOT `gpgconf --list-options gpg`: it does not list the options that +// matter. Measured on gpg 2.5.24 with a GNUPGHOME whose gpg.conf set +// personal-digest-preferences, digest-algo, cert-digest-algo, weak-digest, +// personal-cipher-preferences, compliance, trust-model, require-compliance and +// keyserver, the listing carried values for compliance, trust-model and +// keyserver only. So snug reads the files, and asks gpgconf only where they +// are: its homedir honours GNUPGHOME, its sysconfdir is the compiled-in +// /etc/gnupg. +// +// Which user file: gpg reads the first that exists of gpg.conf-, +// gpg.conf-., gpg.conf- and gpg.conf, and only that one +// — measured on 2.5.24, where gpg.conf-2.5.24 beat gpg.conf-2.5, gpg.conf-2.5 +// beat gpg.conf, gpg.conf-2 beat gpg.conf, and a gpg.conf-2.5.2 or +// gpg.conf-2.4 was ignored. The system directory has no versioned variant: a +// /etc/gnupg/gpg.conf-2.5 was ignored beside /etc/gnupg/gpg.conf. +// +// Every failure is an error: a probe that could not run, a version it cannot +// read, a file that exists and cannot be read. Each would otherwise leave the +// sandbox's gpg on compiled-in defaults the host may have tightened. One of +// them is stricter than gpg: gpg skips a file it may not read (measured, mode +// 000: a gpg.conf gives compiled-in defaults, a gpg.conf-2.5 falls back to +// gpg.conf), and snug refuses instead of copying that. +func probeGPGConf() (gpgConfFiles, error) { + var f gpgConfFiles + gpgconf, err := exec.LookPath("gpgconf") + if err != nil { + return f, errors.New("identity.gpg needs gpgconf on the host to find gpg's " + + "configuration, and no gpgconf is on PATH") + } + ctx, cancel := context.WithTimeout(context.Background(), gpgHostTimeout) + defer cancel() + // Bare `--list-dirs`, read by name: with names as arguments gpgconf + // prints the values in ITS order, not theirs (measured: `--list-dirs + // homedir sysconfdir` printed /etc/gnupg first). + out, err := exec.CommandContext(ctx, gpgconf, "--list-dirs").Output() + if err != nil { + return f, fmt.Errorf("identity.gpg: `gpgconf --list-dirs` failed: %v", err) + } + dirs := map[string]string{} + for line := range strings.SplitSeq(string(out), "\n") { + if k, v, ok := strings.Cut(line, ":"); ok { + dirs[k] = v + } + } + // A `%` is gpgconf's percent-escaping of a `:` or `%` in the path; + // refused rather than decoded, as snug has never measured it. + homedir, sysconfdir := dirs["homedir"], dirs["sysconfdir"] + if !cleanAbs(homedir) || !cleanAbs(sysconfdir) || + strings.Contains(homedir+sysconfdir, "%") { + return f, fmt.Errorf("identity.gpg: `gpgconf --list-dirs` gave homedir %q and sysconfdir %q, "+ + "and snug needs both as plain absolute paths", policy.VisibleText(homedir), + policy.VisibleText(sysconfdir)) + } + + gpg, err := exec.LookPath("gpg") + if err != nil { + return f, errors.New("identity.gpg needs gpg on the host, and none is on PATH") + } + vout, err := exec.CommandContext(ctx, gpg, "--batch", "--version").Output() + if err != nil { + return f, fmt.Errorf("identity.gpg: `gpg --version` failed: %v", err) + } + version, err := parseGPGVersion(string(vout)) + if err != nil { + return f, err + } + + f.systemPath = filepath.Join(sysconfdir, policy.GPGSystemConfName) + if f.system, err = readGPGConf(f.systemPath); err != nil { + return f, err + } + if f.system == nil { + f.systemPath = "" + } + for _, name := range gpgUserConfNames(version) { + p := filepath.Join(homedir, name) + b, err := readGPGConf(p) + if err != nil { + return f, err + } + if b != nil { + f.user, f.userPath = b, p + break + } + } + return f, nil +} + +func cleanAbs(p string) bool { return filepath.IsAbs(p) && filepath.Clean(p) == p } + +// maxGPGConfBytes bounds each gpg.conf read. A hand-written gpg.conf is a few +// dozen lines; a megabyte is not one. +const maxGPGConfBytes = 1 << 20 + +// readGPGConf is a file's bytes, or nil when gpg would find nothing there. +// ENOENT is the only absence: a dangling symlink gives it too, and gpg reads +// one as absent (measured). A FIFO, a device, a directory or an oversized file +// is refused by hostread rather than read. +func readGPGConf(p string) ([]byte, error) { + b, err := hostread.Required(p, maxGPGConfBytes) + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("identity.gpg: cannot read %s (%v), so snug cannot carry the "+ + "host's gpg settings into the sandbox and will not start it on gpg's compiled-in "+ + "defaults instead.\n Make it a readable file, or remove it if gpg is not meant "+ + "to read it", policy.VisibleText(p), err) + } + if b == nil { + b = []byte{} + } + return b, nil +} + +var gpgVersionLine = regexp.MustCompile(`^gpg \(GnuPG[^)]*\) ([0-9]+(?:\.[0-9]+)*)$`) + +// parseGPGVersion is the dotted version on `gpg --version`'s first line, +// "gpg (GnuPG) 2.5.24" on the development host. +func parseGPGVersion(out string) (string, error) { + first, _, _ := strings.Cut(out, "\n") + m := gpgVersionLine.FindStringSubmatch(strings.TrimSpace(first)) + if m == nil { + return "", fmt.Errorf("identity.gpg: `gpg --version` begins %q, and snug needs the "+ + "dotted version to know which gpg.conf- gpg reads", policy.VisibleText(first)) + } + return m[1], nil +} + +// gpgUserConfNames is the order gpg looks for its user file: the full version, +// then shorter by one component at a time, then the bare name. +func gpgUserConfNames(version string) []string { + var out []string + for v := version; v != ""; { + out = append(out, policy.GPGSystemConfName+"-"+v) + i := strings.LastIndexByte(v, '.') + if i < 0 { + break + } + v = v[:i] + } + return append(out, policy.GPGSystemConfName) +} + +// probeAndStageGPGConf is probeGPGConf followed by stageGPGConf. +func probeAndStageGPGConf(pol *policy.Policy) error { + f, err := probeGPGConf() + if err != nil { + return err + } + return stageGPGConf(pol, f) +} + +// stageGPGConf mounts the generated gpg.conf files: the user one always, so +// --dry-run shows what the sandbox's gpg reads even on a host that sets +// nothing, and the system one only where the host has one, so the sandbox's +// gpg sees the same files present as the host's. +func stageGPGConf(pol *policy.Policy, f gpgConfFiles) error { + userHost := f.userPath + if userHost == "" { + userHost = "(none: no gpg.conf in the host's GNUPGHOME)" + } + user, err := policy.GPGConfFrom(f.user, userHost, false) + if err != nil { + return fmt.Errorf("identity.gpg: %v", err) + } + if f.system != nil { + system, err := policy.GPGConfFrom(f.system, f.systemPath, true) + if err != nil { + return fmt.Errorf("identity.gpg: %v", err) + } + pol.Replace(policy.Mount{ + Guest: f.systemPath, Kind: policy.KindData, Access: policy.AccessRO, + Content: system, From: []string{identityProvenance(pol, "gpg")}, + }) + } + pol.Replace(policy.Mount{ + Guest: pol.Home + "/" + policy.GPGConfGuest, Kind: policy.KindData, + Access: policy.AccessRO, Content: user, + From: []string{identityProvenance(pol, "gpg")}, + }) + return nil +} diff --git a/internal/cli/gpgconf_test.go b/internal/cli/gpgconf_test.go new file mode 100644 index 00000000..79a844f1 --- /dev/null +++ b/internal/cli/gpgconf_test.go @@ -0,0 +1,215 @@ +package cli + +import ( + "os" + "path/filepath" + "slices" + "strings" + "syscall" + "testing" + + "github.com/gomoni/snug/internal/policy" +) + +// fakeGPGConfHost puts a gpgconf and a gpg on an otherwise empty PATH that +// answer for a host whose homedir and sysconfdir are temporary directories, +// and returns those two. Both start empty, so what the probe finds does not +// depend on the machine running the suite. gpgconf prints sysconfdir before homedir, as the real +// one does whatever order it is asked in. +func fakeGPGConfHost(t *testing.T, version string) (home, sys string) { + t.Helper() + bin, home, sys := t.TempDir(), t.TempDir(), t.TempDir() + scripts := map[string]string{ + "gpgconf": "#!/bin/sh\nprintf 'sysconfdir:%s\\nbindir:/usr/bin\\nhomedir:%s\\n' '" + sys + "' '" + home + "'\n", + // Only --version answers; an export or a key listing fails, as on a + // host whose gpg does not know the key. + "gpg": "#!/bin/sh\nfor a; do [ \"$a\" = --version ] && printf 'gpg (GnuPG) " + version + + "\\nlibgcrypt 1.12.4\\n' && exit 0; done\nexit 2\n", + } + for name, body := range scripts { + if err := os.WriteFile(filepath.Join(bin, name), []byte(body), 0o755); err != nil { + t.Fatal(err) + } + } + t.Setenv("PATH", bin) + return home, sys +} + +func write(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +// TestGPGUserConfNamesIsGPGsOrder: the full version first, then one component +// shorter at a time, then the bare name — the order measured on gpg 2.5.24. +func TestGPGUserConfNamesIsGPGsOrder(t *testing.T) { + got := gpgUserConfNames("2.5.24") + want := []string{"gpg.conf-2.5.24", "gpg.conf-2.5", "gpg.conf-2", "gpg.conf"} + if !slices.Equal(got, want) { + t.Errorf("gpgUserConfNames(2.5.24) = %q, want %q", got, want) + } +} + +func TestParseGPGVersion(t *testing.T) { + if v, err := parseGPGVersion("gpg (GnuPG) 2.5.24\nlibgcrypt 1.12.4\n"); err != nil || v != "2.5.24" { + t.Errorf("parseGPGVersion = %q, %v", v, err) + } + if v, err := parseGPGVersion("gpg (GnuPG/MacGPG2) 2.2.41\n"); err != nil || v != "2.2.41" { + t.Errorf("parseGPGVersion of a vendor build = %q, %v", v, err) + } + for _, bad := range []string{"", "gpg (GnuPG) 2.5.24-beta3\n", "gpg (GnuPG) ../x\n", "sq 1.0\n"} { + if v, err := parseGPGVersion(bad); err == nil { + t.Errorf("parseGPGVersion(%q) = %q, want a refusal", bad, v) + } + } +} + +// TestProbeGPGConfReadsTheFileGPGReads: with both gpg.conf-2.5 and gpg.conf in +// the homedir, the probe takes gpg.conf-2.5 as gpg does, and reads the system +// file from the sysconfdir gpgconf names. +func TestProbeGPGConfReadsTheFileGPGReads(t *testing.T) { + home, sys := fakeGPGConfHost(t, "2.5.24") + write(t, filepath.Join(home, "gpg.conf"), "digest-algo SHA224\n") + write(t, filepath.Join(home, "gpg.conf-2.5"), "digest-algo SHA256\n") + write(t, filepath.Join(home, "gpg.conf-2.4"), "digest-algo MD5\n") + write(t, filepath.Join(sys, "gpg.conf"), "[force]\ncompliance gnupg\n") + f, err := probeGPGConf() + if err != nil { + t.Fatal(err) + } + if f.userPath != filepath.Join(home, "gpg.conf-2.5") || string(f.user) != "digest-algo SHA256\n" { + t.Errorf("user file = %s %q, want gpg.conf-2.5", f.userPath, f.user) + } + if f.systemPath != filepath.Join(sys, "gpg.conf") || !strings.Contains(string(f.system), "[force]") { + t.Errorf("system file = %s %q", f.systemPath, f.system) + } +} + +// TestProbeGPGConfAbsentFilesAreNotAnError: a host with no gpg.conf anywhere +// probes cleanly, and staging still writes the user file (empty of options) +// and no system file. +func TestProbeGPGConfAbsentFilesAreNotAnError(t *testing.T) { + _, sys := fakeGPGConfHost(t, "2.5.24") + f, err := probeGPGConf() + if err != nil { + t.Fatal(err) + } + if f.user != nil || f.system != nil || f.userPath != "" || f.systemPath != "" { + t.Errorf("absent files probed as %+v", f) + } + pol := &policy.Policy{Mounts: map[string]policy.Mount{}, Home: "/home/u"} + if err := stageGPGConf(pol, f); err != nil { + t.Fatal(err) + } + if _, ok := pol.Mounts["/home/u/"+policy.GPGConfGuest]; !ok { + t.Error("no user gpg.conf staged on a host with none") + } + if _, ok := pol.Mounts[filepath.Join(sys, "gpg.conf")]; ok { + t.Error("a system gpg.conf was staged on a host with none") + } +} + +// TestProbeGPGConfRefusesAnUnreadableFile: gpg itself skips a gpg.conf it +// cannot read; snug refuses, naming the file, rather than run the sandbox on +// defaults the file may have tightened. +func TestProbeGPGConfRefusesAnUnreadableFile(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root reads a mode-000 file") + } + home, _ := fakeGPGConfHost(t, "2.5.24") + p := filepath.Join(home, "gpg.conf") + write(t, p, "digest-algo SHA256\n") + if err := os.Chmod(p, 0); err != nil { + t.Fatal(err) + } + if _, err := probeGPGConf(); err == nil || !strings.Contains(err.Error(), p) { + t.Errorf("an unreadable %s probed as %v", p, err) + } +} + +// TestProbeGPGConfRefusesAFailedProbe: a gpgconf that fails, or prints no +// usable homedir, refuses rather than probing nothing. +func TestProbeGPGConfRefusesAFailedProbe(t *testing.T) { + for name, body := range map[string]string{ + "exits non-zero": "#!/bin/sh\nexit 2\n", + "no homedir": "#!/bin/sh\nprintf 'sysconfdir:/etc/gnupg\\n'\n", + "relative homedir": "#!/bin/sh\nprintf 'sysconfdir:/etc/gnupg\\nhomedir:gnupg\\n'\n", + "escaped homedir": "#!/bin/sh\nprintf 'sysconfdir:/etc/gnupg\\nhomedir:/h/a%%3ab\\n'\n", + } { + t.Run(name, func(t *testing.T) { + fakeGPGConfHost(t, "2.5.24") + if err := os.WriteFile(filepath.Join(os.Getenv("PATH"), "gpgconf"), []byte(body), 0o755); err != nil { + t.Fatal(err) + } + if _, err := probeGPGConf(); err == nil || !strings.Contains(err.Error(), "gpgconf --list-dirs") { + t.Errorf("probe = %v, want a refusal naming gpgconf --list-dirs", err) + } + }) + } + t.Run("no gpgconf", func(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + if _, err := probeGPGConf(); err == nil || !strings.Contains(err.Error(), "no gpgconf is on PATH") { + t.Errorf("probe without gpgconf = %v", err) + } + }) +} + +// TestStageGPGConfMountsBothFilesGenerated: the system file lands at the +// host's sysconfdir path and the user file in the sandbox's GNUPGHOME, both +// read-only, both generated, and neither carrying a non-allowlisted line. +func TestStageGPGConfMountsBothFilesGenerated(t *testing.T) { + f := gpgConfFiles{ + system: []byte("[force]\nweak-digest SHA224\nkeyserver hkps://x\n"), systemPath: "/etc/gnupg/gpg.conf", + user: []byte("exec-path /tmp\npersonal-digest-preferences SHA256\n"), userPath: "/h/.gnupg/gpg.conf", + } + pol := &policy.Policy{Mounts: map[string]policy.Mount{}, Home: "/home/u"} + if err := stageGPGConf(pol, f); err != nil { + t.Fatal(err) + } + for guest, want := range map[string]string{ + "/etc/gnupg/gpg.conf": "[force]\nweak-digest SHA224\n", + "/home/u/" + policy.GPGConfGuest: "\npersonal-digest-preferences SHA256\n", + } { + m, ok := pol.Mounts[guest] + if !ok { + t.Errorf("%s not staged", guest) + continue + } + if m.Kind != policy.KindData || m.Access != policy.AccessRO || !m.Authored { + t.Errorf("%s staged as kind %v access %v authored %v", guest, m.Kind, m.Access, m.Authored) + } + c := string(m.Content) + if !strings.HasSuffix(c, want) || strings.Contains(c, "\nkeyserver") || strings.Contains(c, "\nexec-path") { + t.Errorf("%s content:\n%s", guest, c) + } + } +} + +// TestStageGPGConfRefusesAnUncarriableLine: one allowlisted line snug cannot +// render refuses the run, and stages nothing. +func TestStageGPGConfRefusesAnUncarriableLine(t *testing.T) { + f := gpgConfFiles{user: []byte("digest-algo \"SHA256\n"), userPath: "/h/.gnupg/gpg.conf"} + pol := &policy.Policy{Mounts: map[string]policy.Mount{}, Home: "/home/u"} + err := stageGPGConf(pol, f) + if err == nil || !strings.Contains(err.Error(), "/h/.gnupg/gpg.conf:1") { + t.Fatalf("stageGPGConf = %v, want a refusal naming /h/.gnupg/gpg.conf:1", err) + } + if len(pol.Mounts) != 0 { + t.Errorf("a refused stage left mounts: %v", pol.Mounts) + } +} + +// TestProbeGPGConfRefusesAFIFO: a FIFO where gpg.conf goes is refused, not +// opened — an open(2) of a FIFO with no writer never returns. +func TestProbeGPGConfRefusesAFIFO(t *testing.T) { + home, _ := fakeGPGConfHost(t, "2.5.24") + p := filepath.Join(home, "gpg.conf") + if err := syscall.Mkfifo(p, 0o600); err != nil { + t.Fatal(err) + } + if _, err := probeGPGConf(); err == nil || !strings.Contains(err.Error(), p) { + t.Errorf("a FIFO at %s probed as %v", p, err) + } +} diff --git a/internal/cli/gpgidentity.go b/internal/cli/gpgidentity.go index 07c8d36b..b2b1baac 100644 --- a/internal/cli/gpgidentity.go +++ b/internal/cli/gpgidentity.go @@ -170,7 +170,8 @@ func gpgPins(fingerprint, scope string, g gpgGrips) ([]gpgproxy.PinnedGrip, erro } // startGPGIdentity is what identity.gpg needs from the host: the Assuan proxy -// to the host gpg-agent and the pinned key's PUBLIC keyring. +// to the host gpg-agent, the pinned key's PUBLIC keyring, and gpg.conf files +// generated from the host's (probeGPGConf). // // ~/.gnupg itself (a 0700 tmpfs) and GNUPGHOME are Resolve's; this stages what // needs host state inside it. @@ -210,6 +211,11 @@ func startGPGIdentity(pol *policy.Policy, id *policy.Identity, verbose, dryRun b } else { stageGPGKeyring(pol, keyring) } + if cerr := probeAndStageGPGConf(pol); cerr != nil { + fmt.Fprintln(os.Stderr, "snug: "+cerr.Error()) + fmt.Fprintln(os.Stderr, " (dry run: continuing; a real run refuses here, and the "+ + "FILESYSTEM block below therefore has no gpg.conf row)") + } pol.BindSocket(sock, pol.Home+"/"+policy.GPGAgentSocketGuest, identityProvenance(pol, "gpg")) return cleanup, nil @@ -263,6 +269,10 @@ func startGPGIdentity(pol *policy.Policy, id *policy.Identity, verbose, dryRun b return nil, kerr } stageGPGKeyring(pol, keyring) + if err := probeAndStageGPGConf(pol); err != nil { + cleanup() + return nil, err + } pol.BindSocket(sock, pol.Home+"/"+policy.GPGAgentSocketGuest, identityProvenance(pol, "gpg")) diff --git a/internal/cli/gpgidentitygolden_test.go b/internal/cli/gpgidentitygolden_test.go index 29caaf87..13b6dc68 100644 --- a/internal/cli/gpgidentitygolden_test.go +++ b/internal/cli/gpgidentitygolden_test.go @@ -19,12 +19,14 @@ const goldenGPGFingerprint = "6732361300BBFDAE7F22972CD4381E1CE2880244" // gpgGoldenPolicy resolves the builtin defaults plus one profile pinning // goldenGPGFingerprint with the given scope, and stages what a dry run stages: // the agent socket from startGPGIdentity and, standing in for the host's -// `gpg --export`, a fixed keyring. The host gpg is pointed at an empty -// throwaway GNUPGHOME so nothing on the machine running the suite is read. +// `gpg --export`, a fixed keyring. gpg and gpgconf are fakeGPGConfHost's, over +// an empty homedir and sysconfdir, so nothing on the machine running the suite +// is read and the generated gpg.conf row is the same on every machine. func gpgGoldenPolicy(t *testing.T, scope string) *policy.Policy { t.Helper() t.Setenv("XDG_RUNTIME_DIR", "/run/user/1000") t.Setenv("GNUPGHOME", shortGnupgHome(t)) + fakeGPGConfHost(t, "2.5.24") reg, err := profile.Builtins() if err != nil { t.Fatal(err) diff --git a/internal/cli/testdata/gpg-decrypt.filesystem.txt b/internal/cli/testdata/gpg-decrypt.filesystem.txt index 5a9df074..c800eabe 100644 --- a/internal/cli/testdata/gpg-decrypt.filesystem.txt +++ b/internal/cli/testdata/gpg-decrypt.filesystem.txt @@ -33,6 +33,7 @@ FILESYSTEM (deny-by-default; every line is a grant, there are no deny rules) tmpfs /home/u/.local (max 1 GiB) (snug anchor) tmpfs /home/u/proj (max 1 GiB) (snug anchor) rw /home/u/.gnupg/S.gpg-agent (from /run/user/1000/snug/run-N/gpg-agent.sock) identity:signer + data /home/u/.gnupg/gpg.conf identity:signer data /home/u/.gnupg/pubring.kbx identity:signer tmpfs /home/u/.local/share (max 1 GiB) @home tmpfs /home/u/.local/state (max 1 GiB) @home diff --git a/internal/cli/testdata/gpg.filesystem.txt b/internal/cli/testdata/gpg.filesystem.txt index 3a985db9..aa7314e8 100644 --- a/internal/cli/testdata/gpg.filesystem.txt +++ b/internal/cli/testdata/gpg.filesystem.txt @@ -34,6 +34,7 @@ FILESYSTEM (deny-by-default; every line is a grant, there are no deny rules) tmpfs /home/u/.local (max 1 GiB) (snug anchor) tmpfs /home/u/proj (max 1 GiB) (snug anchor) rw /home/u/.gnupg/S.gpg-agent (from /run/user/1000/snug/run-N/gpg-agent.sock) identity:signer + data /home/u/.gnupg/gpg.conf identity:signer data /home/u/.gnupg/pubring.kbx identity:signer tmpfs /home/u/.local/share (max 1 GiB) @home tmpfs /home/u/.local/state (max 1 GiB) @home diff --git a/internal/cli/testdata/identity-gpg-decrypt.bwrap.txt b/internal/cli/testdata/identity-gpg-decrypt.bwrap.txt index 048aeda8..86905ac4 100644 --- a/internal/cli/testdata/identity-gpg-decrypt.bwrap.txt +++ b/internal/cli/testdata/identity-gpg-decrypt.bwrap.txt @@ -24,10 +24,10 @@ --symlink usr/sbin /sbin --size 1073741824 --tmpfs /tmp ---ro-bind-fd 18 /usr +--ro-bind-fd 19 /usr --ro-bind-data 10 /etc/group ---ro-bind-fd 19 /etc/ld.so.cache ---ro-bind-fd 20 /etc/ld.so.conf +--ro-bind-fd 20 /etc/ld.so.cache +--ro-bind-fd 21 /etc/ld.so.conf --ro-bind-data 11 /etc/nsswitch.conf --ro-bind-data 12 /etc/passwd --ro-bind-data 13 /etc/resolv.conf @@ -48,13 +48,14 @@ --tmpfs /home/u/.local --size 1073741824 --tmpfs /home/u/proj ---bind-fd 21 /home/u/.gnupg/S.gpg-agent ---ro-bind-data 17 /home/u/.gnupg/pubring.kbx +--bind-fd 22 /home/u/.gnupg/S.gpg-agent +--ro-bind-data 17 /home/u/.gnupg/gpg.conf +--ro-bind-data 18 /home/u/.gnupg/pubring.kbx --size 1073741824 --tmpfs /home/u/.local/share --size 1073741824 --tmpfs /home/u/.local/state ---bind-fd 22 /home/u/proj/sub +--bind-fd 23 /home/u/proj/sub --remount-ro / --clearenv --setenv GNUPGHOME /home/u/.gnupg diff --git a/internal/cli/testdata/identity-gpg.bwrap.txt b/internal/cli/testdata/identity-gpg.bwrap.txt index 396d069e..27a44e4c 100644 --- a/internal/cli/testdata/identity-gpg.bwrap.txt +++ b/internal/cli/testdata/identity-gpg.bwrap.txt @@ -24,10 +24,10 @@ --symlink usr/sbin /sbin --size 1073741824 --tmpfs /tmp ---ro-bind-fd 19 /usr +--ro-bind-fd 20 /usr --ro-bind-data 10 /etc/group ---ro-bind-fd 20 /etc/ld.so.cache ---ro-bind-fd 21 /etc/ld.so.conf +--ro-bind-fd 21 /etc/ld.so.cache +--ro-bind-fd 22 /etc/ld.so.conf --ro-bind-data 11 /etc/nsswitch.conf --ro-bind-data 12 /etc/passwd --ro-bind-data 13 /etc/resolv.conf @@ -49,13 +49,14 @@ --tmpfs /home/u/.local --size 1073741824 --tmpfs /home/u/proj ---bind-fd 22 /home/u/.gnupg/S.gpg-agent ---ro-bind-data 18 /home/u/.gnupg/pubring.kbx +--bind-fd 23 /home/u/.gnupg/S.gpg-agent +--ro-bind-data 18 /home/u/.gnupg/gpg.conf +--ro-bind-data 19 /home/u/.gnupg/pubring.kbx --size 1073741824 --tmpfs /home/u/.local/share --size 1073741824 --tmpfs /home/u/.local/state ---bind-fd 23 /home/u/proj/sub +--bind-fd 24 /home/u/proj/sub --remount-ro / --clearenv --setenv GIT_CONFIG_GLOBAL /home/u/.gitconfig diff --git a/internal/policy/bookgpgconf_test.go b/internal/policy/bookgpgconf_test.go new file mode 100644 index 00000000..2d69508d --- /dev/null +++ b/internal/policy/bookgpgconf_test.go @@ -0,0 +1,54 @@ +package policy + +import ( + "os" + "regexp" + "slices" + "strings" + "testing" +) + +// TestBookListsExactlyTheCarriedGPGConfOptions checks the bulleted option list +// under "#### Your gpg.conf" in docs/src/identity.md against GPGConfOptions, in +// both directions, and that every meta command the page names is one snug +// carries. +func TestBookListsExactlyTheCarriedGPGConfOptions(t *testing.T) { + const page = "../../docs/src/identity.md" + data, err := os.ReadFile(page) + if err != nil { + t.Fatal(err) + } + _, section, ok := strings.Cut(string(data), "#### Your gpg.conf\n") + if !ok { + t.Fatalf("%s has no \"#### Your gpg.conf\" section", page) + } + section, _, _ = strings.Cut(section, "\n#") + // The list is the first run of "- " lines and their indented continuations. + var bullets strings.Builder + for _, line := range strings.Split(section, "\n") { + if strings.HasPrefix(line, "- ") || (bullets.Len() > 0 && strings.HasPrefix(line, " ")) { + bullets.WriteString(line + "\n") + } else if bullets.Len() > 0 { + break + } + } + var documented []string + for _, m := range regexp.MustCompile("`([a-z0-9-]+)`").FindAllStringSubmatch(bullets.String(), -1) { + documented = append(documented, m[1]) + } + for _, name := range documented { + if _, ok := GPGConfOptions[name]; !ok { + t.Errorf("%s lists %q as carried, and GPGConfOptions does not carry it", page, name) + } + } + for _, name := range SortedGPGConfOptions() { + if !slices.Contains(documented, name) { + t.Errorf("%s does not list %q, which GPGConfOptions carries", page, name) + } + } + for _, m := range regexp.MustCompile("`\\[([a-z+-]+)\\]`").FindAllStringSubmatch(section, -1) { + if !slices.Contains(gpgConfMeta, m[1]) { + t.Errorf("%s names meta command [%s], which snug does not carry", page, m[1]) + } + } +} diff --git a/internal/policy/gpgconf.go b/internal/policy/gpgconf.go new file mode 100644 index 00000000..93d9a3a4 --- /dev/null +++ b/internal/policy/gpgconf.go @@ -0,0 +1,310 @@ +package policy + +import ( + "fmt" + "sort" + "strings" +) + +// GPGConfGuest is the generated per-user gpg.conf inside the sandbox's +// GNUPGHOME, and GPGSystemConfName the file name gpg reads in its sysconfdir +// (`gpgconf --list-dirs sysconfdir`). gpg reads the system file BEFORE the +// user's and lets the user's override it, except for options the system file +// marks `[force]` — measured on gpg 2.5.24 with `digest-algo` and a signature's +// `digest algo` packet field. snug generates both files rather than merging +// them into one so that gpg inside applies that precedence itself. +const ( + GPGConfGuest = GPGHomeGuest + "/gpg.conf" + GPGSystemConfName = "gpg.conf" +) + +// GPGConfOptions is every gpg.conf option snug carries from the host into the +// files it generates, mapped to whether the option takes an argument. Each +// names a cryptographic or trust CHOICE the gpg client makes: which digest +// signs, which cipher encrypts, which digests a signature may not use, which +// trust model and compliance mode decide validity. +// +// It is an ALLOWLIST because gpg.conf is partly a command table: `exec-path`, +// `photo-viewer` and `verify-options show-photos` name programs to run, and +// `keyserver`, `keyserver-options`, `auto-key-retrieve` and `auto-key-locate` +// send the sandbox to the network on gpg's initiative. None of those, and +// nothing naming a file or socket, is on this list. +// +// The `allow-*` relaxations and their kin (`allow-weak-digest-algos`, +// `allow-weak-key-signatures`, `allow-old-cipher-algos`, `ignore-mdc-error`, +// `no-require-cross-certification`, `override-compliance-check`, +// `always-trust`) are absent ON PURPOSE: each only switches a check off, so +// dropping one leaves the sandbox's gpg stricter than the host's, never looser. +// A host that needs one to verify an old signature does that on the host. The +// VALUE of a carried choice is the host's whatever it is: `trust-model always` +// and `compliance pgp6` are carried as written, because the sandbox then +// matches the host, and the rule above is about dropping a line, not about +// second-guessing one. +// +// Dropping a line is safe only if no carried line depends on it, and two kinds +// of gpg option do: an ALIAS of a carried option (gpgConfAliases) and a +// SHORTCUT that sets a carried option's value (gpgConfShortcuts). Both are +// rewritten to the carried spelling in place, so the host's last-one-wins +// order survives. Measured by the red team on gpg 2.5.24: a dropped +// `personal-cipher-prefs AES128` left the sandbox encrypting with AES256, and +// a dropped `gnupg` after a carried `compliance openpgp` left `gpg -c` inside +// on 3DES and SHA-1 where the host used AES256 and SHA-256. +// +// A hostile process inside is not bound by any of this — it can point gpg at a +// GNUPGHOME of its own. The files exist so that git and every other HONEST +// client inside signs and verifies the way the host's gpg would. +var GPGConfOptions = map[string]bool{ + "personal-cipher-preferences": true, + "personal-digest-preferences": true, + "personal-aead-preferences": true, + "cipher-algo": true, + "digest-algo": true, + "cert-digest-algo": true, + "aead-algo": true, + "s2k-mode": true, + "s2k-digest-algo": true, + "s2k-cipher-algo": true, + "s2k-count": true, + "weak-digest": true, + "disable-cipher-algo": true, + "disable-pubkey-algo": true, + "assert-pubkey-algo": true, + "compliance": true, + "trust-model": true, + "completes-needed": true, + "marginals-needed": true, + "max-cert-depth": true, + "min-cert-level": true, + "trusted-key": true, + "tofu-default-policy": true, + "assert-signer": true, + + "require-compliance": false, + "require-cross-certification": false, + "require-backsigs": false, + "require-secmem": false, + "require-pqc-encryption": false, + "force-aead": false, + "no-symkey-cache": false, +} + +// gpgConfAliases maps each second spelling `gpg --dump-options` lists for a +// carried option to the option it is. +var gpgConfAliases = map[string]string{ + "personal-cipher-prefs": "personal-cipher-preferences", + "force-ocb": "force-aead", + "use-ocb-sym": "force-aead", +} + +// gpgConfShortcuts maps each argument-less compliance shortcut `gpg +// --dump-options` lists to the `compliance` line it sets: a `gnupg` after +// `compliance openpgp` overrode it on the host (measured, above). +var gpgConfShortcuts = map[string]string{ + "gnupg": "compliance gnupg", + "openpgp": "compliance openpgp", + "rfc2440": "compliance rfc2440", + "rfc4880": "compliance rfc4880", + "rfc4880bis": "compliance rfc4880bis", + "pgp6": "compliance pgp6", + "pgp7": "compliance pgp7", + "pgp8": "compliance pgp8", +} + +// gpgConfBlanks is what gpg takes as the gap between an option and its +// argument: measured, `digest-algo` followed by VT, FF or CR and then a bogus +// name each failed with "selected digest algorithm is invalid", so all three +// separate as space and tab do. +const gpgConfBlanks = " \t\v\f\r" + +// gpgConfMeta is every meta command snug carries verbatim from the SYSTEM file. +// gpg accepts meta commands only there (in a user file it fails with +// "unexpected meta command") and fails on one it does not know ("unknown meta +// command"); both measured on gpg 2.5.24. They are carried rather than +// interpreted so that gpg inside applies them to the carried lines exactly as +// the host's does: `[force]` lets a system option win over the user file and +// the command line, `[ignore]` drops the system options after it, and +// `[ignore-all]` makes gpg skip the user file. `[user NAME]` is carried with +// its argument checked by gpgConfValueOK. +var gpgConfMeta = []string{ + "force", "+force", "-force", + "ignore", "+ignore", "-ignore", "ignore-all", + "verbose", "+verbose", "-verbose", +} + +// SortedGPGConfOptions is GPGConfOptions' keys in a stable order. +func SortedGPGConfOptions() []string { + out := make([]string, 0, len(GPGConfOptions)) + for k := range GPGConfOptions { + out = append(out, k) + } + sort.Strings(out) + return out +} + +// GPGConfFrom reads one host gpg.conf as DATA and returns the file snug +// generates in its place: the allowlisted option lines and, for the system +// file, the meta commands, in the host's order, each rendered by snug. +// `host` names the file in errors and in the header. +// +// The grammar is the part of gpg's that the host's own gpg accepts, measured +// on gpg 2.5.24: one option per line; leading and trailing blanks ignored; `#` +// starts a comment only at the start of a line (`digest-algo SHA256 # c` is +// the argument "SHA256 # c", and gpg rejects it); option names are exact and +// lowercase (`digest-al`, `DIGEST-ALGO`, `--digest-algo` and +// `digest-algo=SHA256` are each "invalid option"); one `"…"` pair around an +// argument is stripped; the last of a repeated scalar option wins. +// +// It returns an error, never a quieter file, for an allowlisted option it +// cannot carry faithfully: an argument missing, present on a flag, or shaped +// like nothing a crypto or trust option takes. Dropping that line would give +// the sandbox gpg's compiled-in default for the option — the silent downgrade +// this file exists to prevent. Lines naming any other option are left behind +// without comment; that is what the allowlist means. +func GPGConfFrom(src []byte, host string, system bool) ([]byte, error) { + var carried []string + for i, raw := range strings.Split(string(src), "\n") { + line := strings.Trim(raw, gpgConfBlanks) + if line == "" || line[0] == '#' { + continue + } + where := fmt.Sprintf("%s:%d", host, i+1) + if line[0] == '[' { + if !system { + return nil, fmt.Errorf("%s: %q is a meta command, which gpg accepts only in "+ + "the system-wide gpg.conf. The host's gpg refuses this file too; remove the line", + where, VisibleText(line)) + } + m, err := gpgConfMetaLine(line) + if err != nil { + return nil, fmt.Errorf("%s: %v", where, err) + } + carried = append(carried, m) + continue + } + name, arg := line, "" + if i := strings.IndexAny(line, gpgConfBlanks); i >= 0 { + name, arg = line[:i], strings.Trim(line[i:], gpgConfBlanks) + } + if !gpgConfNameOK(name) { + return nil, fmt.Errorf("%s: %q is not spelled like a gpg option (lowercase letters, "+ + "digits and dashes), so snug cannot tell which option the host's gpg reads it as; "+ + "fix or remove the line", where, VisibleText(name)) + } + if short, ok := gpgConfShortcuts[name]; ok && arg == "" { + carried = append(carried, short) + continue + } + if canonical, ok := gpgConfAliases[name]; ok { + name = canonical + } + takesArg, ok := GPGConfOptions[name] + if !ok { + continue + } + if len(arg) >= 2 && arg[0] == '"' && arg[len(arg)-1] == '"' { + arg = arg[1 : len(arg)-1] + } + switch { + case !takesArg && arg != "": + return nil, fmt.Errorf("%s: %s takes no argument, and this line gives it %q. "+ + "snug cannot tell what the host's gpg makes of it; write the option alone", + where, name, VisibleText(arg)) + case takesArg && arg == "": + return nil, fmt.Errorf("%s: %s needs an argument. The host's gpg refuses this "+ + "line too (\"missing argument\"); give it one or remove it", where, name) + case takesArg && !gpgConfValueOK(arg): + return nil, fmt.Errorf("%s: %s %q is not shaped like an algorithm list, a "+ + "number or a key ID, so snug will not write it into the sandbox's gpg.conf, "+ + "and leaving it out would hand the sandbox gpg's compiled-in default instead. "+ + "Spell it as gpg(1) does, e.g. `personal-digest-preferences SHA512 SHA384`", + where, name, VisibleText(arg)) + } + if takesArg { + carried = append(carried, name+" "+arg) + } else { + carried = append(carried, name) + } + } + return renderGPGConf(host, system, carried), nil +} + +func gpgConfNameOK(name string) bool { + for _, r := range name { + if !(r >= 'a' && r <= 'z' || r >= '0' && r <= '9' || r == '-') { + return false + } + } + return name != "" +} + +func gpgConfMetaLine(line string) (string, error) { + if !strings.HasSuffix(line, "]") { + return "", fmt.Errorf("%q opens a meta command and does not close it", VisibleText(line)) + } + inner := strings.Trim(line[1:len(line)-1], " \t") + if user, ok := strings.CutPrefix(inner, "user"); ok && (user == "" || user[0] == ' ' || user[0] == '\t') { + user = strings.Trim(user, " \t") + if user == "" || !gpgConfValueOK(user) || strings.Contains(user, " ") { + return "", fmt.Errorf("[user %s] names a user snug cannot carry; "+ + "want one login name or *", VisibleText(user)) + } + return "[user " + user + "]", nil + } + for _, m := range gpgConfMeta { + if inner == m { + return "[" + m + "]", nil + } + } + return "", fmt.Errorf("[%s] is a meta command snug does not know (it carries %s and [user NAME]); "+ + "without it snug cannot say what the system options around it mean", + VisibleText(inner), "["+strings.Join(gpgConfMeta, "] [")+"]") +} + +// gpgConfValueOK admits exactly what the allowlisted options take — algorithm +// names (`SHA512`, `AES256`, `cv25519`), lists of them separated by blanks or +// commas, numbers, key IDs (`0xD4381E1CE2880244`), the compliance and trust +// words (`de-vs`, `pgp`), `assert-pubkey-algo`'s comparisons (`>=rsa3072`), and +// the `*` of `[user *]` — as an allowed set rather than a list of dangerous +// characters. It refuses `"` and `#`, the two characters gpg.conf's grammar +// gives meaning inside a line, and every control character, so a carried value +// cannot become a second line or a comment. +func gpgConfValueOK(v string) bool { + if v == "" || len(v) > 1024 { + return false + } + for _, r := range v { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9': + case strings.ContainsRune(" ,.-_+<>=*", r): + default: + return false + } + } + return true +} + +func renderGPGConf(host string, system bool, carried []string) []byte { + var b strings.Builder + if system { + b.WriteString("# Generated by snug from this host's system-wide gpg.conf,\n") + } else { + b.WriteString("# Generated by snug from this host's gpg.conf,\n") + } + fmt.Fprintf(&b, "# %s.\n", VisibleText(host)) + b.WriteString("#\n") + b.WriteString("# Only cryptographic and trust choices are carried (digest, cipher,\n") + b.WriteString("# weak-digest, compliance, trust model), each line written by snug.\n") + b.WriteString("# Options that run a program or reach the network (exec-path,\n") + b.WriteString("# photo-viewer, keyserver, auto-key-retrieve, auto-key-locate) and\n") + b.WriteString("# the allow-* relaxations are left behind.\n") + if len(carried) == 0 { + b.WriteString("#\n# The host's file sets none of the carried options.\n") + return []byte(b.String()) + } + b.WriteString("\n") + for _, l := range carried { + b.WriteString(l) + b.WriteString("\n") + } + return []byte(b.String()) +} diff --git a/internal/policy/gpgconf_test.go b/internal/policy/gpgconf_test.go new file mode 100644 index 00000000..d7574ec1 --- /dev/null +++ b/internal/policy/gpgconf_test.go @@ -0,0 +1,231 @@ +package policy + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// carriedLines is the non-comment, non-blank lines of a generated gpg.conf. +func carriedLines(b []byte) []string { + var out []string + for l := range strings.SplitSeq(string(b), "\n") { + if l != "" && !strings.HasPrefix(l, "#") { + out = append(out, l) + } + } + return out +} + +// TestGPGConfOptionsNeverRunOrReach: no option that names a program, sends gpg +// to the network, or only relaxes a check is on the allowlist. Adding one is +// the change this test exists to make loud. +func TestGPGConfOptionsNeverRunOrReach(t *testing.T) { + for _, bad := range []string{ + "exec-path", "photo-viewer", "verify-options", "list-options", + "keyserver", "keyserver-options", "auto-key-retrieve", "auto-key-locate", + "auto-key-import", "options", "homedir", "agent-program", "dirmngr-program", + "keyboxd-program", "trustdb-name", "keyring", "secret-keyring", "load-extension", + "allow-weak-digest-algos", "allow-weak-key-signatures", "allow-old-cipher-algos", + "ignore-mdc-error", "no-require-cross-certification", "no-require-backsigs", + "override-compliance-check", "always-trust", + } { + if _, ok := GPGConfOptions[bad]; ok { + t.Errorf("%s is on GPGConfOptions", bad) + } + } +} + +// TestGPGConfFromCarriesOnlyTheAllowlist: a host file mixing crypto choices +// with every program- and network-naming option comes out holding the crypto +// choices, in the host's order, and nothing else. +func TestGPGConfFromCarriesOnlyTheAllowlist(t *testing.T) { + host := `# a host gpg.conf +personal-digest-preferences SHA512 SHA384 SHA256 +keyserver hkps://keys.openpgp.org +keyserver-options auto-key-retrieve +exec-path /tmp/evil +photo-viewer "xdg-open %i" +auto-key-retrieve +auto-key-locate local,wkd +verify-options show-photos + digest-algo "SHA256" +allow-weak-digest-algos +weak-digest SHA224 +require-compliance +trust-model pgp +compliance de-vs +assert-pubkey-algo >=rsa3072,ed25519 +trusted-key 0xD4381E1CE2880244 +` + got, err := GPGConfFrom([]byte(host), "/h/.gnupg/gpg.conf", false) + if err != nil { + t.Fatal(err) + } + want := []string{ + "personal-digest-preferences SHA512 SHA384 SHA256", + "digest-algo SHA256", + "weak-digest SHA224", + "require-compliance", + "trust-model pgp", + "compliance de-vs", + "assert-pubkey-algo >=rsa3072,ed25519", + "trusted-key 0xD4381E1CE2880244", + } + if lines := carriedLines(got); strings.Join(lines, "\n") != strings.Join(want, "\n") { + t.Errorf("carried:\n%s\nwant:\n%s", strings.Join(lines, "\n"), strings.Join(want, "\n")) + } +} + +// TestGPGConfFromCarriesSystemMetaCommands: the system file's meta commands +// survive in place, so gpg inside applies [force] and [ignore] to the same +// options the host's does. +func TestGPGConfFromCarriesSystemMetaCommands(t *testing.T) { + host := "[force]\ndigest-algo SHA384\nkeyserver hkps://x\n[-force]\n[ user * ]\n[ignore-all]\ncompliance gnupg\n" + got, err := GPGConfFrom([]byte(host), "/etc/gnupg/gpg.conf", true) + if err != nil { + t.Fatal(err) + } + want := "[force]\ndigest-algo SHA384\n[-force]\n[user *]\n[ignore-all]\ncompliance gnupg" + if lines := strings.Join(carriedLines(got), "\n"); lines != want { + t.Errorf("carried:\n%s\nwant:\n%s", lines, want) + } +} + +// TestGPGConfFromRefusesWhatItCannotCarry: every allowlisted line snug cannot +// render faithfully refuses the run, naming the file and line, rather than +// leaving the option to gpg's compiled-in default. +func TestGPGConfFromRefusesWhatItCannotCarry(t *testing.T) { + for _, tc := range []struct { + name, src string + system bool + want string + }{ + {"missing argument", "digest-algo\n", false, "needs an argument"}, + {"flag with argument", "require-compliance yes\n", false, "takes no argument"}, + {"unterminated quote", "digest-algo \"SHA256\n", false, "not shaped like"}, + {"trailing comment", "digest-algo SHA256 # c\n", false, "not shaped like"}, + {"embedded quote", "personal-digest-preferences SHA256\"x\n", false, "not shaped like"}, + {"control character", "digest-algo SHA256\x1b[2J\n", false, "not shaped like"}, + {"meta in user file", "[force]\n", false, "only in"}, + {"unknown meta", "[bogus]\n", true, "does not know"}, + {"unclosed meta", "[force\n", true, "does not close"}, + {"user with two words", "[user a b]\n", true, "cannot carry"}, + {"user with no name", "[user]\n", true, "cannot carry"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := GPGConfFrom([]byte("# first\n"+tc.src), "/h/gpg.conf", tc.system) + if err == nil { + t.Fatalf("%q was carried", tc.src) + } + if !strings.Contains(err.Error(), "/h/gpg.conf:2") || !strings.Contains(err.Error(), tc.want) { + t.Errorf("error %q does not name /h/gpg.conf:2 and %q", err, tc.want) + } + }) + } +} + +// TestGPGConfFromEmptyHostFile: a host that sets nothing gets a file that says +// so and carries no option. +func TestGPGConfFromEmptyHostFile(t *testing.T) { + got, err := GPGConfFrom(nil, "(none)", false) + if err != nil { + t.Fatal(err) + } + if lines := carriedLines(got); len(lines) != 0 { + t.Errorf("an empty host file carried %q", lines) + } + if !strings.Contains(string(got), "sets none of the carried options") { + t.Errorf("the empty file does not say why it is empty:\n%s", got) + } +} + +// TestGPGConfFromEscapesTheHostPathInTheHeader: the host path is written into +// a comment, and a newline in it must not end the comment. +func TestGPGConfFromEscapesTheHostPathInTheHeader(t *testing.T) { + got, err := GPGConfFrom(nil, "/h/x\ndigest-algo MD5", false) + if err != nil { + t.Fatal(err) + } + if lines := carriedLines(got); len(lines) != 0 { + t.Errorf("a newline in the host path became a directive: %q", lines) + } +} + +// TestGoldenGPGConf is the review artifact for the two generated gpg.conf +// files: what the sandbox's gpg reads, byte for byte, from one host file of +// each kind. +func TestGoldenGPGConf(t *testing.T) { + for _, tc := range []struct { + name, host, src string + system bool + }{ + {"gpgconf-user.txt", "/home/u/.gnupg/gpg.conf", + "personal-digest-preferences SHA512 SHA384\nkeyserver hkps://keys.openpgp.org\n" + + "exec-path /tmp\nweak-digest SHA224\nallow-weak-digest-algos\nrequire-compliance\n", false}, + {"gpgconf-system.txt", "/etc/gnupg/gpg.conf", + "[force]\ncompliance de-vs\nauto-key-locate wkd\n[-force]\ntrust-model pgp\n", true}, + {"gpgconf-empty.txt", "(none: no gpg.conf in the host's GNUPGHOME)", "", false}, + } { + t.Run(tc.name, func(t *testing.T) { + b, err := GPGConfFrom([]byte(tc.src), tc.host, tc.system) + if err != nil { + t.Fatal(err) + } + path := filepath.Join("testdata", tc.name) + if *update { + if err := os.WriteFile(path, b, 0o644); err != nil { + t.Fatal(err) + } + return + } + want, err := os.ReadFile(path) + if err != nil { + t.Fatalf("%v (run with -update and read the file)", err) + } + if string(want) != string(b) { + t.Errorf("%s changed; this is the gpg.conf the sandbox's gpg reads. Run with -update "+ + "and read the diff.\n%s", tc.name, b) + } + }) + } +} + +// TestGPGConfFromCarriesWhatTheRedTeamSawDropped: each of these lines is one the +// host's gpg applies, and an earlier GPGConfFrom dropped as unknown, leaving the +// sandbox's gpg looser than the host's. Each must be carried under the +// allowlisted spelling, or refused. +func TestGPGConfFromCarriesWhatTheRedTeamSawDropped(t *testing.T) { + for _, tc := range []struct{ src, want string }{ + {"personal-cipher-prefs AES128\n", "personal-cipher-preferences AES128"}, + {"force-ocb\n", "force-aead"}, + {"use-ocb-sym\n", "force-aead"}, + {"digest-algo\vSHA256\n", "digest-algo SHA256"}, + {"weak-digest\fSHA224\n", "weak-digest SHA224"}, + {"personal-cipher-preferences\rAES128\n", "personal-cipher-preferences AES128"}, + {"force-aead\v\n", "force-aead"}, + // The last compliance wins on the host, so it must be the last here. + {"compliance openpgp\ngnupg\n", "compliance openpgp\ncompliance gnupg"}, + } { + got, err := GPGConfFrom([]byte(tc.src), "/h/gpg.conf", false) + if err != nil { + t.Errorf("%q refused: %v", tc.src, err) + continue + } + if lines := strings.Join(carriedLines(got), "\n"); lines != tc.want { + t.Errorf("%q carried %q, want %q", tc.src, lines, tc.want) + } + } +} + +// TestGPGConfFromRefusesAnOptionNameItCannotRead: a keyword with a byte gpg +// does not spell options with is refused, never dropped as "not allowlisted", +// since snug cannot tell which option the host's gpg makes of it. +func TestGPGConfFromRefusesAnOptionNameItCannotRead(t *testing.T) { + for _, src := range []string{"digest-algo\x00 SHA256\n", "DIGEST-ALGO SHA256\n", "digest-algo=SHA256\n", "digest algo SHA256\n"} { + if _, err := GPGConfFrom([]byte(src), "/h/gpg.conf", false); err == nil { + t.Errorf("%q was not refused", src) + } + } +} diff --git a/internal/policy/testdata/gpgconf-empty.txt b/internal/policy/testdata/gpgconf-empty.txt new file mode 100644 index 00000000..69af458a --- /dev/null +++ b/internal/policy/testdata/gpgconf-empty.txt @@ -0,0 +1,10 @@ +# Generated by snug from this host's gpg.conf, +# (none: no gpg.conf in the host's GNUPGHOME). +# +# Only cryptographic and trust choices are carried (digest, cipher, +# weak-digest, compliance, trust model), each line written by snug. +# Options that run a program or reach the network (exec-path, +# photo-viewer, keyserver, auto-key-retrieve, auto-key-locate) and +# the allow-* relaxations are left behind. +# +# The host's file sets none of the carried options. diff --git a/internal/policy/testdata/gpgconf-system.txt b/internal/policy/testdata/gpgconf-system.txt new file mode 100644 index 00000000..ada3265d --- /dev/null +++ b/internal/policy/testdata/gpgconf-system.txt @@ -0,0 +1,13 @@ +# Generated by snug from this host's system-wide gpg.conf, +# /etc/gnupg/gpg.conf. +# +# Only cryptographic and trust choices are carried (digest, cipher, +# weak-digest, compliance, trust model), each line written by snug. +# Options that run a program or reach the network (exec-path, +# photo-viewer, keyserver, auto-key-retrieve, auto-key-locate) and +# the allow-* relaxations are left behind. + +[force] +compliance de-vs +[-force] +trust-model pgp diff --git a/internal/policy/testdata/gpgconf-user.txt b/internal/policy/testdata/gpgconf-user.txt new file mode 100644 index 00000000..d9aa3151 --- /dev/null +++ b/internal/policy/testdata/gpgconf-user.txt @@ -0,0 +1,12 @@ +# Generated by snug from this host's gpg.conf, +# /home/u/.gnupg/gpg.conf. +# +# Only cryptographic and trust choices are carried (digest, cipher, +# weak-digest, compliance, trust model), each line written by snug. +# Options that run a program or reach the network (exec-path, +# photo-viewer, keyserver, auto-key-retrieve, auto-key-locate) and +# the allow-* relaxations are left behind. + +personal-digest-preferences SHA512 SHA384 +weak-digest SHA224 +require-compliance diff --git a/test/integration/identitygpg_test.go b/test/integration/identitygpg_test.go index 49ab4b93..b5cb25e6 100644 --- a/test/integration/identitygpg_test.go +++ b/test/integration/identitygpg_test.go @@ -17,6 +17,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" "testing" "time" @@ -478,3 +479,135 @@ func TestIdentityGPGProxyDiesWithSnug(t *testing.T) { t.Errorf("the host agent's socket is gone: %v", err) } } + +// digestAlgo is the `digest algo N` of the first signature packet in a +// `gpg --list-packets` dump, or "" when there is none. +func digestAlgo(packets string) string { + for line := range strings.SplitSeq(packets, "\n") { + if _, rest, ok := strings.Cut(line, "digest algo "); ok { + n, _, _ := strings.Cut(rest, ",") + return strings.TrimSpace(n) + } + } + return "" +} + +// TestIdentityGPGCarriesHostGPGConf: the host's gpg.conf asks for SHA-256 +// signatures, and a signature made inside the sandbox uses SHA-256 (OpenPGP +// digest algo 8). The sandbox's generated gpg.conf carries that line and the +// weak-digest one, and leaves behind the lines that name a program, the +// network or a relaxation. +// +// The control is the host itself: the same gpg on the same home signs with +// algo 8 reading the file and with something else under --no-options, which +// skips it — so the assertion inside discriminates, rather than passing on a +// gpg whose default is already SHA-256. +func TestIdentityGPGCarriesHostGPGConf(t *testing.T) { + budget(t, 30*time.Second) + h := newGPGHost(t) + proj, _ := target(t) + // personal-cipher-prefs is gpg's alias for personal-cipher-preferences: + // snug must carry it under the long name, not drop it as unknown. + conf := "personal-digest-preferences SHA256\nweak-digest SHA224\npersonal-cipher-prefs AES128\n" + + "keyserver hkps://keys.example.invalid\nexec-path /nonexistent\nallow-weak-digest-algos\n" + if err := os.WriteFile(filepath.Join(h.home, "gpg.conf"), []byte(conf), 0o600); err != nil { + t.Fatal(err) + } + hostDigest := func(extra ...string) string { + sig, errs, err := h.gpgRun([]byte("hi"), append(extra, "-u", h.fpr, "--sign")...) + if err != nil { + t.Fatalf("control: host gpg could not sign: %v\n%s", err, errs) + } + packets, _, _ := h.gpgRun([]byte(sig), "--list-packets") + return digestAlgo(packets) + } + if d := hostDigest(); d != "8" { + t.Fatalf("control: host gpg reading the file signs with digest algo %q, want 8", d) + } + if d := hostDigest("--no-options"); d == "8" || d == "" { + t.Fatalf("control: host gpg WITHOUT the file signs with digest algo %q; "+ + "the test cannot tell the file was carried", d) + } + // The cipher a message was encrypted with, as the host's `gpg -v -d` + // names it: "AES.OCB encrypted data" for AES128, "AES256.OCB" otherwise. + cipherOf := func(path string) string { + _, errs, err := h.gpgRun(nil, "-v", "-d", path) + if err != nil { + t.Fatalf("host could not decrypt %s: %v\n%s", path, err, errs) + } + for line := range strings.SplitSeq(errs, "\n") { + if c, ok := strings.CutSuffix(strings.TrimPrefix(line, "gpg: "), " encrypted data"); ok { + return c + } + } + return "" + } + hostEnc := func(out string, extra ...string) { + if _, errs, err := h.gpgRun([]byte("hi"), append(extra, "--trust-model", "always", + "-o", out, "-e", "-r", h.fpr)...); err != nil { + t.Fatalf("control: host gpg could not encrypt: %v\n%s", err, errs) + } + } + ctl := t.TempDir() + hostEnc(filepath.Join(ctl, "with.gpg")) + hostEnc(filepath.Join(ctl, "without.gpg"), "--no-options") + if with, without := cipherOf(filepath.Join(ctl, "with.gpg")), cipherOf(filepath.Join(ctl, "without.gpg")); with != "AES.OCB" || without == with { + t.Fatalf("control: host encrypts with %q reading the file and %q without; "+ + "want AES.OCB and something else", with, without) + } + + r := runEnv(t, h.profile(h.fpr, "sign"), []string{"-p", "gpgid"}, proj, ` +echo "CONF-BEGIN"; cat "$GNUPGHOME/gpg.conf"; echo "CONF-END" +echo "PKT-BEGIN"; echo hi | gpg --batch -u `+h.fpr+` --sign | gpg --batch --list-packets 2>&1; echo "PKT-END" +echo hi | gpg --batch --trust-model always -o enc.gpg -e -r `+h.fpr+`; echo "enc-exit=[$?]" +`).mustRun(t) + if c := cipherOf(filepath.Join(proj, "enc.gpg")); c != "AES.OCB" { + t.Errorf("a message encrypted inside uses %q, want AES.OCB (the host's "+ + "personal-cipher-prefs AES128, carried under its long name):\n%s", c, r.out) + } + packets, _ := section(r.out, "PKT") + if d := digestAlgo(packets); d != "8" { + t.Errorf("a signature made inside uses digest algo %q, want 8 (the host's "+ + "personal-digest-preferences SHA256):\n%s", d, r.out) + } + carried, _ := section(r.out, "CONF") + lines := strings.Split(carried, "\n") + for _, want := range []string{"personal-digest-preferences SHA256", "weak-digest SHA224", + "personal-cipher-preferences AES128"} { + if !slices.Contains(lines, want) { + t.Errorf("the sandbox's gpg.conf lacks %q:\n%s", want, carried) + } + } + for _, line := range lines { + for _, bad := range []string{"keyserver", "exec-path", "allow-weak-digest-algos"} { + if strings.HasPrefix(line, bad) { + t.Errorf("the sandbox's gpg.conf carries %q", line) + } + } + } +} + +// TestIdentityGPGRefusesAnUncarriableGPGConf: a host gpg.conf line snug cannot +// render faithfully refuses the run, naming the file and line, instead of +// starting the sandbox on gpg's compiled-in default for that option. The value +// is one the host's gpg ACCEPTS (an unterminated quote), so the refusal is +// snug's and not a copy of gpg's own. +func TestIdentityGPGRefusesAnUncarriableGPGConf(t *testing.T) { + budget(t, 30*time.Second) + h := newGPGHost(t) + proj, _ := target(t) + p := filepath.Join(h.home, "gpg.conf") + if err := os.WriteFile(p, []byte("# host\npersonal-digest-preferences \"SHA256\n"), 0o600); err != nil { + t.Fatal(err) + } + if _, errs, err := h.gpgRun([]byte("hi"), "-u", h.fpr, "--sign"); err != nil { + t.Fatalf("control: the host's gpg rejects the file itself: %v\n%s", err, errs) + } + r := runEnv(t, h.profile(h.fpr, "sign"), []string{"-p", "gpgid"}, proj, "echo started") + if r.ran || r.code == 0 { + t.Fatalf("the sandbox started over an uncarriable gpg.conf (ran=%v exit=%d):\n%s", r.ran, r.code, r.out) + } + if !strings.Contains(r.out, p+":2") || !strings.Contains(r.out, "personal-digest-preferences") { + t.Errorf("the refusal does not name %s:2 and the option:\n%s", p, r.out) + } +} From ad592a52cc5274d4384b5ad2396b7b48ca9bfa53 Mon Sep 17 00:00:00 2001 From: Michal Vyskocil Date: Fri, 23 Oct 2026 15:09:24 +0200 Subject: [PATCH 2/3] identity.gpg: refuse a profile gpg.conf stub beside a carried one; name drops under -v MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pieces from the parallel #689 implementation (kept on feat/689-gpg-conf-files): - A profile grant at ~/.gnupg/gpg.conf was displaced by the generated file. Two authors, one path: refuse when the host user file carries anything; with nothing carried the stub stays and no user file is generated. A grant at ~/.gnupg/gpg.conf-* is refused always: gpg reads the versioned name instead of the generated file. - Options left behind are named under -v, keyword only, never value (GENERATED-CONFIG.md §6a). policy.GPGConfSummary. Refs #689 Co-Authored-By: Claude Opus 5.5 (1M context) --- internal/cli/gpgconf.go | 79 ++++++++++++++++++++++++++++----- internal/cli/gpgconf_test.go | 40 +++++++++++++++++ internal/cli/gpgidentity.go | 4 +- internal/policy/gpgconf.go | 57 +++++++++++++++++++----- internal/policy/gpgconf_test.go | 24 ++++++++++ 5 files changed, 182 insertions(+), 22 deletions(-) diff --git a/internal/cli/gpgconf.go b/internal/cli/gpgconf.go index 84a0745a..f7e877f9 100644 --- a/internal/cli/gpgconf.go +++ b/internal/cli/gpgconf.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io/fs" + "os" "os/exec" "path/filepath" "regexp" @@ -173,19 +174,51 @@ func gpgUserConfNames(version string) []string { return append(out, policy.GPGSystemConfName) } -// probeAndStageGPGConf is probeGPGConf followed by stageGPGConf. -func probeAndStageGPGConf(pol *policy.Policy) error { +// probeAndStageGPGConf is probeGPGConf followed by stageGPGConf, and under -v +// names what each host file carried and what it left behind, by option name +// only (every drop is named: .claude/design/GENERATED-CONFIG.md §6a). +func probeAndStageGPGConf(pol *policy.Policy, verbose bool) error { f, err := probeGPGConf() if err != nil { return err } - return stageGPGConf(pol, f) + if err := stageGPGConf(pol, f); err != nil { + return err + } + if verbose { + for _, src := range []struct { + path string + b []byte + system bool + }{{f.systemPath, f.system, true}, {f.userPath, f.user, false}} { + if src.b == nil { + continue + } + carried, dropped := policy.GPGConfSummary(src.b, src.system) + fmt.Fprintf(os.Stderr, "snug: gpg.conf %s: carried %s; left on the host %s\n", + policy.VisibleText(src.path), gpgConfNames(carried), gpgConfNames(dropped)) + } + } + return nil +} + +func gpgConfNames(names []string) string { + if len(names) == 0 { + return "(none)" + } + return strings.Join(names, " ") } // stageGPGConf mounts the generated gpg.conf files: the user one always, so // --dry-run shows what the sandbox's gpg reads even on a host that sets // nothing, and the system one only where the host has one, so the sandbox's // gpg sees the same files present as the host's. +// +// A profile's own grant at ~/.gnupg/gpg.conf (a stub a human wrote) is +// REFUSED when the host's user file carries anything, never displaced: two +// authors for one file, and either silent winner drops the other's options. +// When the host's user file carries nothing, the stub stays and no user file +// is generated — the stub is then the only author. func stageGPGConf(pol *policy.Policy, f gpgConfFiles) error { userHost := f.userPath if userHost == "" { @@ -195,20 +228,46 @@ func stageGPGConf(pol *policy.Policy, f gpgConfFiles) error { if err != nil { return fmt.Errorf("identity.gpg: %v", err) } + var system []byte if f.system != nil { - system, err := policy.GPGConfFrom(f.system, f.systemPath, true) - if err != nil { + if system, err = policy.GPGConfFrom(f.system, f.systemPath, true); err != nil { return fmt.Errorf("identity.gpg: %v", err) } + } + userGuest := pol.Home + "/" + policy.GPGConfGuest + // gpg reads the first of gpg.conf-…gpg.conf (probeGPGConf), so a + // grant at a versioned name would be read INSTEAD of the generated file, + // carried options and all, whatever the host sets. + for guest, m := range pol.Mounts { + if strings.HasPrefix(guest, userGuest+"-") && !m.Anchor { + return fmt.Errorf("%s grants %s, which the sandbox's gpg reads in place of the %s "+ + "identity.gpg (profile %q) generates from the host's.\n"+ + " Drop the grant", strings.Join(m.From, "+"), policy.VisibleText(guest), + userGuest, pol.IdentityOwner("gpg")) + } + } + stub, haveStub := pol.Mounts[userGuest] + haveStub = haveStub && !stub.Anchor + if haveStub { + if carried, _ := policy.GPGConfSummary(f.user, false); len(carried) > 0 { + return fmt.Errorf("identity.gpg (profile %q) generates %s from the host's %s, and "+ + "%s also grants that path.\n"+ + " Two authors cannot both hold one file. Drop the grant, or move what it "+ + "sets into the host's gpg.conf", pol.IdentityOwner("gpg"), userGuest, + policy.VisibleText(f.userPath), strings.Join(stub.From, "+")) + } + } + if system != nil { pol.Replace(policy.Mount{ Guest: f.systemPath, Kind: policy.KindData, Access: policy.AccessRO, Content: system, From: []string{identityProvenance(pol, "gpg")}, }) } - pol.Replace(policy.Mount{ - Guest: pol.Home + "/" + policy.GPGConfGuest, Kind: policy.KindData, - Access: policy.AccessRO, Content: user, - From: []string{identityProvenance(pol, "gpg")}, - }) + if !haveStub { + pol.Replace(policy.Mount{ + Guest: userGuest, Kind: policy.KindData, Access: policy.AccessRO, Content: user, + From: []string{identityProvenance(pol, "gpg")}, + }) + } return nil } diff --git a/internal/cli/gpgconf_test.go b/internal/cli/gpgconf_test.go index 79a844f1..53e0f44c 100644 --- a/internal/cli/gpgconf_test.go +++ b/internal/cli/gpgconf_test.go @@ -213,3 +213,43 @@ func TestProbeGPGConfRefusesAFIFO(t *testing.T) { t.Errorf("a FIFO at %s probed as %v", p, err) } } + +// TestStageGPGConfRefusesBesideAProfileStub: a profile's own grant at +// ~/.gnupg/gpg.conf and a generated file are two authors for one path, so +// staging refuses, naming both, when the host's user file carries anything — +// never displacing the stub. With nothing carried the stub stays as the only +// author and no user file is generated (control). A grant at a versioned +// name is refused whatever the host sets: gpg would read it instead. +func TestStageGPGConfRefusesBesideAProfileStub(t *testing.T) { + stubbed := func(guest string) *policy.Policy { + return &policy.Policy{Home: "/home/u", Mounts: map[string]policy.Mount{ + guest: {Guest: guest, Host: "/x/stub", Kind: policy.KindBind, Access: policy.AccessRO, + From: []string{"gpg-stub"}}, + }} + } + userGuest := "/home/u/" + policy.GPGConfGuest + carrying := gpgConfFiles{user: []byte("digest-algo SHA256\n"), userPath: "/h/.gnupg/gpg.conf"} + + pol := stubbed(userGuest) + err := stageGPGConf(pol, carrying) + if err == nil || !strings.Contains(err.Error(), "gpg-stub") || !strings.Contains(err.Error(), "/h/.gnupg/gpg.conf") { + t.Fatalf("stageGPGConf beside a stub = %v, want a refusal naming the stub and the host file", err) + } + if pol.Mounts[userGuest].Kind != policy.KindBind { + t.Error("the refused stage displaced the stub") + } + + pol = stubbed(userGuest) + if err := stageGPGConf(pol, gpgConfFiles{user: []byte("keyserver hkps://x\n"), userPath: "/h/.gnupg/gpg.conf"}); err != nil { + t.Fatalf("nothing carried, yet: %v", err) + } + if m := pol.Mounts[userGuest]; m.Kind != policy.KindBind || m.Authored { + t.Errorf("with nothing carried the stub must stay, got %+v", m) + } + + versioned := userGuest + "-2.5" + pol = stubbed(versioned) + if err := stageGPGConf(pol, gpgConfFiles{}); err == nil || !strings.Contains(err.Error(), versioned) { + t.Errorf("a grant at %s = %v, want a refusal naming it", versioned, err) + } +} diff --git a/internal/cli/gpgidentity.go b/internal/cli/gpgidentity.go index b2b1baac..9112528b 100644 --- a/internal/cli/gpgidentity.go +++ b/internal/cli/gpgidentity.go @@ -211,7 +211,7 @@ func startGPGIdentity(pol *policy.Policy, id *policy.Identity, verbose, dryRun b } else { stageGPGKeyring(pol, keyring) } - if cerr := probeAndStageGPGConf(pol); cerr != nil { + if cerr := probeAndStageGPGConf(pol, verbose); cerr != nil { fmt.Fprintln(os.Stderr, "snug: "+cerr.Error()) fmt.Fprintln(os.Stderr, " (dry run: continuing; a real run refuses here, and the "+ "FILESYSTEM block below therefore has no gpg.conf row)") @@ -269,7 +269,7 @@ func startGPGIdentity(pol *policy.Policy, id *policy.Identity, verbose, dryRun b return nil, kerr } stageGPGKeyring(pol, keyring) - if err := probeAndStageGPGConf(pol); err != nil { + if err := probeAndStageGPGConf(pol, verbose); err != nil { cleanup() return nil, err } diff --git a/internal/policy/gpgconf.go b/internal/policy/gpgconf.go index 93d9a3a4..0638b5ef 100644 --- a/internal/policy/gpgconf.go +++ b/internal/policy/gpgconf.go @@ -2,6 +2,7 @@ package policy import ( "fmt" + "slices" "sort" "strings" ) @@ -158,10 +159,43 @@ func SortedGPGConfOptions() []string { // cannot carry faithfully: an argument missing, present on a flag, or shaped // like nothing a crypto or trust option takes. Dropping that line would give // the sandbox gpg's compiled-in default for the option — the silent downgrade -// this file exists to prevent. Lines naming any other option are left behind -// without comment; that is what the allowlist means. +// this file exists to prevent. Lines naming any other option are left behind; +// GPGConfSummary names them (never their values) for the -v log, because +// every drop is named (.claude/design/GENERATED-CONFIG.md §6a). func GPGConfFrom(src []byte, host string, system bool) ([]byte, error) { - var carried []string + carried, _, err := gpgConfRead(src, host, system) + if err != nil { + return nil, err + } + return renderGPGConf(host, system, carried), nil +} + +// GPGConfSummary is the option NAMES one host file contributes (carried) and +// those it leaves behind (dropped), each once, in file order — for the -v log +// and for deciding whether the user file carries anything at all. Values are +// never returned: a keyserver URL or a path is the host user's, not a +// screen's. A file GPGConfFrom refuses summarises as nothing; the refusal is +// what the human sees. +func GPGConfSummary(src []byte, system bool) (carried, dropped []string) { + lines, dropped, err := gpgConfRead(src, "", system) + if err != nil { + return nil, nil + } + for _, l := range lines { + if l[0] == '[' { + continue + } + name, _, _ := strings.Cut(l, " ") + if !slices.Contains(carried, name) { + carried = append(carried, name) + } + } + return carried, dropped +} + +// gpgConfRead is GPGConfFrom's parse: the carried lines as snug renders them, +// and the names of the options left behind. +func gpgConfRead(src []byte, host string, system bool) (carried, dropped []string, err error) { for i, raw := range strings.Split(string(src), "\n") { line := strings.Trim(raw, gpgConfBlanks) if line == "" || line[0] == '#' { @@ -170,13 +204,13 @@ func GPGConfFrom(src []byte, host string, system bool) ([]byte, error) { where := fmt.Sprintf("%s:%d", host, i+1) if line[0] == '[' { if !system { - return nil, fmt.Errorf("%s: %q is a meta command, which gpg accepts only in "+ + return nil, nil, fmt.Errorf("%s: %q is a meta command, which gpg accepts only in "+ "the system-wide gpg.conf. The host's gpg refuses this file too; remove the line", where, VisibleText(line)) } m, err := gpgConfMetaLine(line) if err != nil { - return nil, fmt.Errorf("%s: %v", where, err) + return nil, nil, fmt.Errorf("%s: %v", where, err) } carried = append(carried, m) continue @@ -186,7 +220,7 @@ func GPGConfFrom(src []byte, host string, system bool) ([]byte, error) { name, arg = line[:i], strings.Trim(line[i:], gpgConfBlanks) } if !gpgConfNameOK(name) { - return nil, fmt.Errorf("%s: %q is not spelled like a gpg option (lowercase letters, "+ + return nil, nil, fmt.Errorf("%s: %q is not spelled like a gpg option (lowercase letters, "+ "digits and dashes), so snug cannot tell which option the host's gpg reads it as; "+ "fix or remove the line", where, VisibleText(name)) } @@ -199,6 +233,9 @@ func GPGConfFrom(src []byte, host string, system bool) ([]byte, error) { } takesArg, ok := GPGConfOptions[name] if !ok { + if !slices.Contains(dropped, name) { + dropped = append(dropped, name) + } continue } if len(arg) >= 2 && arg[0] == '"' && arg[len(arg)-1] == '"' { @@ -206,14 +243,14 @@ func GPGConfFrom(src []byte, host string, system bool) ([]byte, error) { } switch { case !takesArg && arg != "": - return nil, fmt.Errorf("%s: %s takes no argument, and this line gives it %q. "+ + return nil, nil, fmt.Errorf("%s: %s takes no argument, and this line gives it %q. "+ "snug cannot tell what the host's gpg makes of it; write the option alone", where, name, VisibleText(arg)) case takesArg && arg == "": - return nil, fmt.Errorf("%s: %s needs an argument. The host's gpg refuses this "+ + return nil, nil, fmt.Errorf("%s: %s needs an argument. The host's gpg refuses this "+ "line too (\"missing argument\"); give it one or remove it", where, name) case takesArg && !gpgConfValueOK(arg): - return nil, fmt.Errorf("%s: %s %q is not shaped like an algorithm list, a "+ + return nil, nil, fmt.Errorf("%s: %s %q is not shaped like an algorithm list, a "+ "number or a key ID, so snug will not write it into the sandbox's gpg.conf, "+ "and leaving it out would hand the sandbox gpg's compiled-in default instead. "+ "Spell it as gpg(1) does, e.g. `personal-digest-preferences SHA512 SHA384`", @@ -225,7 +262,7 @@ func GPGConfFrom(src []byte, host string, system bool) ([]byte, error) { carried = append(carried, name) } } - return renderGPGConf(host, system, carried), nil + return carried, dropped, nil } func gpgConfNameOK(name string) bool { diff --git a/internal/policy/gpgconf_test.go b/internal/policy/gpgconf_test.go index d7574ec1..a1bb0650 100644 --- a/internal/policy/gpgconf_test.go +++ b/internal/policy/gpgconf_test.go @@ -229,3 +229,27 @@ func TestGPGConfFromRefusesAnOptionNameItCannotRead(t *testing.T) { } } } + +// TestGPGConfSummaryNamesDropsNotValues: every option left behind is named +// once, by keyword; no value (a URL, a path) reaches the summary, and the +// carried list uses the carried spelling (an alias resolved). A refused file +// summarises as nothing. +func TestGPGConfSummaryNamesDropsNotValues(t *testing.T) { + src := []byte("keyserver hkps://keys.example\nexec-path /tmp\n" + + "personal-cipher-prefs AES256\nkeyserver hkps://other\nopenpgp\n") + carried, dropped := GPGConfSummary(src, false) + if strings.Join(carried, " ") != "personal-cipher-preferences compliance" { + t.Errorf("carried = %q", carried) + } + if strings.Join(dropped, " ") != "keyserver exec-path" { + t.Errorf("dropped = %q, want each dropped keyword once", dropped) + } + for _, d := range append(carried, dropped...) { + if strings.ContainsAny(d, "/: ") { + t.Errorf("summary entry %q carries a value", d) + } + } + if c, d := GPGConfSummary([]byte("digest-algo \"x\n"), false); c != nil || d != nil { + t.Errorf("a refused file summarised as %q / %q", c, d) + } +} From 23b1202eb139cd11b3bfd8ba27952478aa2f1a3a Mon Sep 17 00:00:00 2001 From: Michal Vyskocil Date: Fri, 23 Oct 2026 15:31:43 +0200 Subject: [PATCH 3/3] identity.gpg: carry gpg.conf spellings as written; carry leak choices; disable dirmngr Red team round on the merged #689 branch (gpg 2.5.24): - F1/F2 sev:medium. Aliases and compliance shortcuts were rewritten to the canonical option. gpg's [force] locks one option-table entry, and a shortcut is a different entry: system `[force] openpgp` + user `compliance gnupg` gave AES256/SHA-256 on the host, 3DES/SHA-1 in the sandbox. no-pgp2/6/7/8 (set compliance back to gnupg) were dropped. Now every alias and shortcut is carried as written. - F3 sev:low. Host no-auto-key-locate dropped; gpg's default did a WKD lookup through a dirmngr it started inside. The keyring is read-only, so no lookup can import: the user file always ends in disable-dirmngr. - F4/F5 sev:low. throw-keyids (keyid 0000000000000000 on host, real ID inside) and compression choices (no compressed packet on host, algo=2 inside) dropped. Carried, with disable-signer-uid. - F6 sev:low. assert-signer and trusted-key also take a FILE name (`assert-signer signers.txt` read ./signers.txt). Key IDs only now. Tests: TestGPGConfKeepsTheHostsSpelling, TestGPGConfCarriesWhatAMessageLeaks, TestGPGConfKeyIDOptionsRefuseAFileName, TestGPGConfUserFileDisablesDirmngr, TestIdentityGPGHidesWhatTheHostHides (dirmngr assertion fails with the line removed: dirmngr=1). Refs #689 Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/src/identity.md | 23 +++- internal/cli/gpgconf_test.go | 5 +- internal/policy/gpgconf.go | 140 ++++++++++++++------ internal/policy/gpgconf_test.go | 107 ++++++++++++++- internal/policy/testdata/gpgconf-empty.txt | 8 +- internal/policy/testdata/gpgconf-system.txt | 5 +- internal/policy/testdata/gpgconf-user.txt | 8 +- test/integration/identitygpg_test.go | 60 ++++++++- 8 files changed, 292 insertions(+), 64 deletions(-) diff --git a/docs/src/identity.md b/docs/src/identity.md index d8e0edd9..e8bffeef 100644 --- a/docs/src/identity.md +++ b/docs/src/identity.md @@ -208,10 +208,25 @@ only these options: - compliance and trust: `compliance`, `require-compliance`, `trust-model`, `completes-needed`, `marginals-needed`, `max-cert-depth`, `min-cert-level`, `trusted-key`, `tofu-default-policy`, `assert-signer` - -gpg's other spellings of these (`personal-cipher-prefs`, `force-ocb`, and the -compliance shortcuts such as `openpgp`) are carried too, rewritten to the names -above, in your file's order. +- what an encrypted message gives away: `throw-keyids`, `no-throw-keyids`, + `disable-signer-uid`, `personal-compress-preferences`, `compress-algo`, + `compress-level`, `bzip2-compress-level`, `no-compress` +- gpg's other spellings of the above: `personal-cipher-prefs`, + `personal-compress-prefs`, `compression-algo`, `force-ocb`, `use-ocb-sym`, + and the compliance shortcuts `gnupg`, `openpgp`, `rfc2440`, `rfc4880`, + `rfc4880bis`, `pgp6`, `pgp7`, `pgp8`, `no-pgp2`, `no-pgp6`, `no-pgp7`, + `no-pgp8` + +Each line keeps your spelling and your file's order. gpg's `[force]` locks one +spelling, not the option behind it, so rewriting `openpgp` to +`compliance openpgp` would change what a forced line overrides. `trusted-key` +and `assert-signer` are carried only as key IDs or fingerprints: gpg also +accepts a file name there, and a file inside the sandbox is not your file. + +The sandbox's own `gpg.conf` also ends with `disable-dirmngr`, which snug adds +itself. The sandbox's keyring is read-only, so a key looked up over the network +could never be imported, and the lookup would only tell the network which +address you asked about. The system file's `[force]`, `[ignore]` and `[ignore-all]` lines are kept in place, so gpg inside applies them as yours does. Everything else stays behind: diff --git a/internal/cli/gpgconf_test.go b/internal/cli/gpgconf_test.go index 53e0f44c..0e31af35 100644 --- a/internal/cli/gpgconf_test.go +++ b/internal/cli/gpgconf_test.go @@ -169,8 +169,9 @@ func TestStageGPGConfMountsBothFilesGenerated(t *testing.T) { t.Fatal(err) } for guest, want := range map[string]string{ - "/etc/gnupg/gpg.conf": "[force]\nweak-digest SHA224\n", - "/home/u/" + policy.GPGConfGuest: "\npersonal-digest-preferences SHA256\n", + "/etc/gnupg/gpg.conf": "[force]\nweak-digest SHA224\n", + "/home/u/" + policy.GPGConfGuest: "\npersonal-digest-preferences SHA256\n\n" + + "# snug: no dirmngr — the sandbox's keyring is read-only.\ndisable-dirmngr\n", } { m, ok := pol.Mounts[guest] if !ok { diff --git a/internal/policy/gpgconf.go b/internal/policy/gpgconf.go index 0638b5ef..0ff14fac 100644 --- a/internal/policy/gpgconf.go +++ b/internal/policy/gpgconf.go @@ -25,11 +25,23 @@ const ( // signs, which cipher encrypts, which digests a signature may not use, which // trust model and compliance mode decide validity. // +// It also carries the choices that decide what an encrypted message LEAKS: +// `throw-keyids` (measured: the host wrote keyid 0000000000000000, the +// sandbox the real key ID until it was carried), `disable-signer-uid`, and +// compression, which hosts turn off against compression-before-encryption +// side channels (`compress-algo none` measured: no compressed packet on the +// host, `compressed packet: algo=2` in the sandbox until carried). +// // It is an ALLOWLIST because gpg.conf is partly a command table: `exec-path`, // `photo-viewer` and `verify-options show-photos` name programs to run, and // `keyserver`, `keyserver-options`, `auto-key-retrieve` and `auto-key-locate` -// send the sandbox to the network on gpg's initiative. None of those, and -// nothing naming a file or socket, is on this list. +// send the sandbox to the network on gpg's initiative. None of those is on +// this list, and nothing naming a file or socket: `assert-signer` and +// `trusted-key` also accept a FILE name (measured: `assert-signer signers.txt` +// read ./signers.txt), so they carry hex key IDs only (gpgConfHexOptions). +// Leaving the network ones behind is not enough on its own — gpg's DEFAULT +// does a WKD lookup through a dirmngr it starts itself — so the user file +// always ends in `disable-dirmngr` (renderGPGConf). // // The `allow-*` relaxations and their kin (`allow-weak-digest-algos`, // `allow-weak-key-signatures`, `allow-old-cipher-algos`, `ignore-mdc-error`, @@ -43,13 +55,15 @@ const ( // second-guessing one. // // Dropping a line is safe only if no carried line depends on it, and two kinds -// of gpg option do: an ALIAS of a carried option (gpgConfAliases) and a -// SHORTCUT that sets a carried option's value (gpgConfShortcuts). Both are -// rewritten to the carried spelling in place, so the host's last-one-wins -// order survives. Measured by the red team on gpg 2.5.24: a dropped -// `personal-cipher-prefs AES128` left the sandbox encrypting with AES256, and -// a dropped `gnupg` after a carried `compliance openpgp` left `gpg -c` inside -// on 3DES and SHA-1 where the host used AES256 and SHA-256. +// of gpg option do: an ALIAS of a carried option (`personal-cipher-prefs`) and +// a SHORTCUT that sets a carried option's value (`gnupg`, `openpgp`, `no-pgp6` +// … set `compliance`). Both are carried AS WRITTEN, never rewritten to the +// canonical spelling: gpg records `[force]` per option-table entry, and an +// alias or a shortcut is a different entry from the option it sets. Measured +// by the red team on gpg 2.5.24: system `[force] openpgp` with user +// `compliance gnupg` gave AES256/SHA-256 on the host and, once both were +// rewritten to `compliance`, 3DES/SHA-1 in the sandbox; a dropped +// `personal-cipher-prefs AES128` or `gnupg` gave the same class of downgrade. // // A hostile process inside is not bound by any of this — it can point gpg at a // GNUPGHOME of its own. The files exist so that git and every other HONEST @@ -80,6 +94,11 @@ var GPGConfOptions = map[string]bool{ "tofu-default-policy": true, "assert-signer": true, + "personal-compress-preferences": true, + "compress-algo": true, + "compress-level": true, + "bzip2-compress-level": true, + "require-compliance": false, "require-cross-certification": false, "require-backsigs": false, @@ -87,28 +106,56 @@ var GPGConfOptions = map[string]bool{ "require-pqc-encryption": false, "force-aead": false, "no-symkey-cache": false, -} + "throw-keyids": false, + "no-throw-keyids": false, + "disable-signer-uid": false, + "no-compress": false, -// gpgConfAliases maps each second spelling `gpg --dump-options` lists for a -// carried option to the option it is. -var gpgConfAliases = map[string]string{ - "personal-cipher-prefs": "personal-cipher-preferences", - "force-ocb": "force-aead", - "use-ocb-sym": "force-aead", + // Aliases, as `gpg --dump-options` lists them. + "personal-cipher-prefs": true, + "personal-compress-prefs": true, + "compression-algo": true, + "force-ocb": false, + "use-ocb-sym": false, + + // Compliance shortcuts: each sets `compliance` (no-pgpN back to gnupg). + "gnupg": false, + "openpgp": false, + "rfc2440": false, + "rfc4880": false, + "rfc4880bis": false, + "pgp6": false, + "pgp7": false, + "pgp8": false, + "no-pgp2": false, + "no-pgp6": false, + "no-pgp7": false, + "no-pgp8": false, } -// gpgConfShortcuts maps each argument-less compliance shortcut `gpg -// --dump-options` lists to the `compliance` line it sets: a `gnupg` after -// `compliance openpgp` overrode it on the host (measured, above). -var gpgConfShortcuts = map[string]string{ - "gnupg": "compliance gnupg", - "openpgp": "compliance openpgp", - "rfc2440": "compliance rfc2440", - "rfc4880": "compliance rfc4880", - "rfc4880bis": "compliance rfc4880bis", - "pgp6": "compliance pgp6", - "pgp7": "compliance pgp7", - "pgp8": "compliance pgp8", +// gpgConfHexOptions take a key ID or fingerprint — and, to gpg, alternatively +// a file name. snug carries the key-ID form only. +var gpgConfHexOptions = map[string]bool{"trusted-key": true, "assert-signer": true} + +// gpgConfHexOK is one or more key IDs or fingerprints (8 to 64 hex digits, +// optional 0x), separated by blanks or commas. +func gpgConfHexOK(v string) bool { + fields := strings.FieldsFunc(v, func(r rune) bool { return r == ' ' || r == ',' }) + if len(fields) == 0 { + return false + } + for _, f := range fields { + f = strings.TrimPrefix(strings.TrimPrefix(f, "0x"), "0X") + if len(f) < 8 || len(f) > 64 { + return false + } + for _, r := range f { + if !(r >= '0' && r <= '9' || r >= 'a' && r <= 'f' || r >= 'A' && r <= 'F') { + return false + } + } + } + return true } // gpgConfBlanks is what gpg takes as the gap between an option and its @@ -224,13 +271,6 @@ func gpgConfRead(src []byte, host string, system bool) (carried, dropped []strin "digits and dashes), so snug cannot tell which option the host's gpg reads it as; "+ "fix or remove the line", where, VisibleText(name)) } - if short, ok := gpgConfShortcuts[name]; ok && arg == "" { - carried = append(carried, short) - continue - } - if canonical, ok := gpgConfAliases[name]; ok { - name = canonical - } takesArg, ok := GPGConfOptions[name] if !ok { if !slices.Contains(dropped, name) { @@ -249,6 +289,11 @@ func gpgConfRead(src []byte, host string, system bool) (carried, dropped []strin case takesArg && arg == "": return nil, nil, fmt.Errorf("%s: %s needs an argument. The host's gpg refuses this "+ "line too (\"missing argument\"); give it one or remove it", where, name) + case takesArg && gpgConfHexOptions[name] && !gpgConfHexOK(arg): + return nil, nil, fmt.Errorf("%s: %s %q is not a key ID or fingerprint. gpg also reads "+ + "a FILE name here, and a file in the sandbox is not the host's file, so snug "+ + "carries key IDs only; leaving the line out would drop the host's check. "+ + "Write the fingerprints in gpg.conf", where, name, VisibleText(arg)) case takesArg && !gpgConfValueOK(arg): return nil, nil, fmt.Errorf("%s: %s %q is not shaped like an algorithm list, a "+ "number or a key ID, so snug will not write it into the sandbox's gpg.conf, "+ @@ -329,19 +374,30 @@ func renderGPGConf(host string, system bool, carried []string) []byte { } fmt.Fprintf(&b, "# %s.\n", VisibleText(host)) b.WriteString("#\n") - b.WriteString("# Only cryptographic and trust choices are carried (digest, cipher,\n") - b.WriteString("# weak-digest, compliance, trust model), each line written by snug.\n") + b.WriteString("# Only cryptographic, trust and leak choices are carried (digest,\n") + b.WriteString("# cipher, weak-digest, compliance, trust model, key-ID hiding,\n") + b.WriteString("# compression), each line written by snug.\n") b.WriteString("# Options that run a program or reach the network (exec-path,\n") b.WriteString("# photo-viewer, keyserver, auto-key-retrieve, auto-key-locate) and\n") b.WriteString("# the allow-* relaxations are left behind.\n") if len(carried) == 0 { b.WriteString("#\n# The host's file sets none of the carried options.\n") - return []byte(b.String()) - } - b.WriteString("\n") - for _, l := range carried { - b.WriteString(l) + } else { b.WriteString("\n") + for _, l := range carried { + b.WriteString(l) + b.WriteString("\n") + } + } + if !system { + // snug's own line, not the host's. gpg's DEFAULT auto-key-locate does a + // WKD lookup through a dirmngr it starts itself (measured inside: with + // the host's no-auto-key-locate left behind, encrypting to + // wk@gnupg.org fetched over WKD and failed only at "no writable + // keyring found"). The staged keyring is read-only, so no lookup can + // import anything: what one does is tell the network which address + // the sandbox asked about. Nothing that works inside is lost. + b.WriteString("\n# snug: no dirmngr — the sandbox's keyring is read-only.\ndisable-dirmngr\n") } return []byte(b.String()) } diff --git a/internal/policy/gpgconf_test.go b/internal/policy/gpgconf_test.go index a1bb0650..8a6b3934 100644 --- a/internal/policy/gpgconf_test.go +++ b/internal/policy/gpgconf_test.go @@ -7,11 +7,13 @@ import ( "testing" ) -// carriedLines is the non-comment, non-blank lines of a generated gpg.conf. +// carriedLines is the non-comment, non-blank lines of a generated gpg.conf +// that came from the host: snug's own trailing disable-dirmngr is left out +// (TestGPGConfUserFileDisablesDirmngr holds it). func carriedLines(b []byte) []string { var out []string for l := range strings.SplitSeq(string(b), "\n") { - if l != "" && !strings.HasPrefix(l, "#") { + if l != "" && !strings.HasPrefix(l, "#") && l != "disable-dirmngr" { out = append(out, l) } } @@ -198,15 +200,16 @@ func TestGoldenGPGConf(t *testing.T) { // allowlisted spelling, or refused. func TestGPGConfFromCarriesWhatTheRedTeamSawDropped(t *testing.T) { for _, tc := range []struct{ src, want string }{ - {"personal-cipher-prefs AES128\n", "personal-cipher-preferences AES128"}, - {"force-ocb\n", "force-aead"}, - {"use-ocb-sym\n", "force-aead"}, + // Carried as written: [force] locks a spelling (TestGPGConfKeepsTheHostsSpelling). + {"personal-cipher-prefs AES128\n", "personal-cipher-prefs AES128"}, + {"force-ocb\n", "force-ocb"}, + {"use-ocb-sym\n", "use-ocb-sym"}, {"digest-algo\vSHA256\n", "digest-algo SHA256"}, {"weak-digest\fSHA224\n", "weak-digest SHA224"}, {"personal-cipher-preferences\rAES128\n", "personal-cipher-preferences AES128"}, {"force-aead\v\n", "force-aead"}, // The last compliance wins on the host, so it must be the last here. - {"compliance openpgp\ngnupg\n", "compliance openpgp\ncompliance gnupg"}, + {"compliance openpgp\ngnupg\n", "compliance openpgp\ngnupg"}, } { got, err := GPGConfFrom([]byte(tc.src), "/h/gpg.conf", false) if err != nil { @@ -238,7 +241,7 @@ func TestGPGConfSummaryNamesDropsNotValues(t *testing.T) { src := []byte("keyserver hkps://keys.example\nexec-path /tmp\n" + "personal-cipher-prefs AES256\nkeyserver hkps://other\nopenpgp\n") carried, dropped := GPGConfSummary(src, false) - if strings.Join(carried, " ") != "personal-cipher-preferences compliance" { + if strings.Join(carried, " ") != "personal-cipher-prefs openpgp" { t.Errorf("carried = %q", carried) } if strings.Join(dropped, " ") != "keyserver exec-path" { @@ -253,3 +256,93 @@ func TestGPGConfSummaryNamesDropsNotValues(t *testing.T) { t.Errorf("a refused file summarised as %q / %q", c, d) } } + +// TestGPGConfUserFileDisablesDirmngr: the user file always ends with snug's +// disable-dirmngr, whatever the host sets (red team: with the host's +// no-auto-key-locate left behind, gpg inside did a WKD lookup through a +// dirmngr it started itself). The system file does not carry it: it is the +// user file's line, after everything the host wrote. +func TestGPGConfUserFileDisablesDirmngr(t *testing.T) { + for _, src := range []string{"", "no-auto-key-locate\n", "digest-algo SHA256\n"} { + got, err := GPGConfFrom([]byte(src), "/h/gpg.conf", false) + if err != nil { + t.Fatal(err) + } + if !strings.HasSuffix(string(got), "\ndisable-dirmngr\n") { + t.Errorf("user file from %q does not end in disable-dirmngr:\n%s", src, got) + } + } + got, err := GPGConfFrom([]byte("digest-algo SHA256\n"), "/etc/gnupg/gpg.conf", true) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(got), "\ndisable-dirmngr") { + t.Errorf("the system file carries disable-dirmngr:\n%s", got) + } +} + +// TestGPGConfKeepsTheHostsSpelling: aliases and compliance shortcuts are +// carried as written, never rewritten. gpg's [force] locks one option-table +// entry, and a shortcut is a different entry from `compliance`: measured by +// the red team, system `[force] openpgp` + user `compliance gnupg` gave +// AES256/SHA-256 on the host and 3DES/SHA-1 in the sandbox once both had been +// rewritten to `compliance`. no-pgp2/6/7/8 are shortcuts too (they set +// compliance back to gnupg) and were dropped before. +func TestGPGConfKeepsTheHostsSpelling(t *testing.T) { + sys, err := GPGConfFrom([]byte("[force]\nopenpgp\npersonal-cipher-preferences AES\n"), "/etc/gnupg/gpg.conf", true) + if err != nil { + t.Fatal(err) + } + if got := strings.Join(carriedLines(sys), "|"); got != "[force]|openpgp|personal-cipher-preferences AES" { + t.Errorf("system file carried %q", got) + } + user, err := GPGConfFrom([]byte("compliance gnupg\nno-pgp6\npersonal-cipher-prefs AES256\n"+ + "personal-compress-prefs Uncompressed\ncompression-algo none\nuse-ocb-sym\n"), "/h/gpg.conf", false) + if err != nil { + t.Fatal(err) + } + want := "compliance gnupg|no-pgp6|personal-cipher-prefs AES256|personal-compress-prefs Uncompressed|" + + "compression-algo none|use-ocb-sym" + if got := strings.Join(carriedLines(user), "|"); got != want { + t.Errorf("user file carried %q, want %q", got, want) + } +} + +// TestGPGConfCarriesWhatAMessageLeaks: throw-keyids, disable-signer-uid and +// the compression choices are carried (red team: the host wrote keyid +// 0000000000000000 and no compressed packet; the sandbox wrote the real key +// ID and compressed with algo 2 while these were dropped). +func TestGPGConfCarriesWhatAMessageLeaks(t *testing.T) { + src := "throw-keyids\ndisable-signer-uid\ncompress-algo none\ncompress-level 0\n" + + "bzip2-compress-level 0\npersonal-compress-preferences Uncompressed\nno-compress\nno-throw-keyids\n" + got, err := GPGConfFrom([]byte(src), "/h/gpg.conf", false) + if err != nil { + t.Fatal(err) + } + if lines := carriedLines(got); strings.Join(lines, "\n")+"\n" != src { + t.Errorf("carried %q, want every line of %q", lines, src) + } +} + +// TestGPGConfKeyIDOptionsRefuseAFileName: assert-signer and trusted-key also +// take a FILE name to gpg (red team: `assert-signer signers.txt` read +// ./signers.txt, which inside the sandbox is the project directory). snug +// carries the key-ID form and refuses anything else rather than drop the +// host's check. +func TestGPGConfKeyIDOptionsRefuseAFileName(t *testing.T) { + for _, ok := range []string{ + "assert-signer 6732361300BBFDAE7F22972CD4381E1CE2880244\n", + "assert-signer 0x6732361300BBFDAE7F22972CD4381E1CE2880244, D4381E1CE2880244\n", + "trusted-key 0xD4381E1CE2880244\n", + } { + if _, err := GPGConfFrom([]byte(ok), "/h/gpg.conf", false); err != nil { + t.Errorf("%q refused: %v", ok, err) + } + } + for _, bad := range []string{"assert-signer signers.txt\n", "trusted-key keys.d\n", "assert-signer ABC\n"} { + _, err := GPGConfFrom([]byte(bad), "/h/gpg.conf", false) + if err == nil || !strings.Contains(err.Error(), "not a key ID or fingerprint") { + t.Errorf("%q = %v, want the key-ID refusal", bad, err) + } + } +} diff --git a/internal/policy/testdata/gpgconf-empty.txt b/internal/policy/testdata/gpgconf-empty.txt index 69af458a..6d9fca68 100644 --- a/internal/policy/testdata/gpgconf-empty.txt +++ b/internal/policy/testdata/gpgconf-empty.txt @@ -1,10 +1,14 @@ # Generated by snug from this host's gpg.conf, # (none: no gpg.conf in the host's GNUPGHOME). # -# Only cryptographic and trust choices are carried (digest, cipher, -# weak-digest, compliance, trust model), each line written by snug. +# Only cryptographic, trust and leak choices are carried (digest, +# cipher, weak-digest, compliance, trust model, key-ID hiding, +# compression), each line written by snug. # Options that run a program or reach the network (exec-path, # photo-viewer, keyserver, auto-key-retrieve, auto-key-locate) and # the allow-* relaxations are left behind. # # The host's file sets none of the carried options. + +# snug: no dirmngr — the sandbox's keyring is read-only. +disable-dirmngr diff --git a/internal/policy/testdata/gpgconf-system.txt b/internal/policy/testdata/gpgconf-system.txt index ada3265d..3c89db9a 100644 --- a/internal/policy/testdata/gpgconf-system.txt +++ b/internal/policy/testdata/gpgconf-system.txt @@ -1,8 +1,9 @@ # Generated by snug from this host's system-wide gpg.conf, # /etc/gnupg/gpg.conf. # -# Only cryptographic and trust choices are carried (digest, cipher, -# weak-digest, compliance, trust model), each line written by snug. +# Only cryptographic, trust and leak choices are carried (digest, +# cipher, weak-digest, compliance, trust model, key-ID hiding, +# compression), each line written by snug. # Options that run a program or reach the network (exec-path, # photo-viewer, keyserver, auto-key-retrieve, auto-key-locate) and # the allow-* relaxations are left behind. diff --git a/internal/policy/testdata/gpgconf-user.txt b/internal/policy/testdata/gpgconf-user.txt index d9aa3151..9c0526fb 100644 --- a/internal/policy/testdata/gpgconf-user.txt +++ b/internal/policy/testdata/gpgconf-user.txt @@ -1,8 +1,9 @@ # Generated by snug from this host's gpg.conf, # /home/u/.gnupg/gpg.conf. # -# Only cryptographic and trust choices are carried (digest, cipher, -# weak-digest, compliance, trust model), each line written by snug. +# Only cryptographic, trust and leak choices are carried (digest, +# cipher, weak-digest, compliance, trust model, key-ID hiding, +# compression), each line written by snug. # Options that run a program or reach the network (exec-path, # photo-viewer, keyserver, auto-key-retrieve, auto-key-locate) and # the allow-* relaxations are left behind. @@ -10,3 +11,6 @@ personal-digest-preferences SHA512 SHA384 weak-digest SHA224 require-compliance + +# snug: no dirmngr — the sandbox's keyring is read-only. +disable-dirmngr diff --git a/test/integration/identitygpg_test.go b/test/integration/identitygpg_test.go index b5cb25e6..3d60371d 100644 --- a/test/integration/identitygpg_test.go +++ b/test/integration/identitygpg_test.go @@ -507,7 +507,7 @@ func TestIdentityGPGCarriesHostGPGConf(t *testing.T) { h := newGPGHost(t) proj, _ := target(t) // personal-cipher-prefs is gpg's alias for personal-cipher-preferences: - // snug must carry it under the long name, not drop it as unknown. + // snug must carry it, as written, not drop it as unknown. conf := "personal-digest-preferences SHA256\nweak-digest SHA224\npersonal-cipher-prefs AES128\n" + "keyserver hkps://keys.example.invalid\nexec-path /nonexistent\nallow-weak-digest-algos\n" if err := os.WriteFile(filepath.Join(h.home, "gpg.conf"), []byte(conf), 0o600); err != nil { @@ -563,7 +563,7 @@ echo hi | gpg --batch --trust-model always -o enc.gpg -e -r `+h.fpr+`; echo "enc `).mustRun(t) if c := cipherOf(filepath.Join(proj, "enc.gpg")); c != "AES.OCB" { t.Errorf("a message encrypted inside uses %q, want AES.OCB (the host's "+ - "personal-cipher-prefs AES128, carried under its long name):\n%s", c, r.out) + "personal-cipher-prefs AES128, carried as written):\n%s", c, r.out) } packets, _ := section(r.out, "PKT") if d := digestAlgo(packets); d != "8" { @@ -573,7 +573,7 @@ echo hi | gpg --batch --trust-model always -o enc.gpg -e -r `+h.fpr+`; echo "enc carried, _ := section(r.out, "CONF") lines := strings.Split(carried, "\n") for _, want := range []string{"personal-digest-preferences SHA256", "weak-digest SHA224", - "personal-cipher-preferences AES128"} { + "personal-cipher-prefs AES128"} { if !slices.Contains(lines, want) { t.Errorf("the sandbox's gpg.conf lacks %q:\n%s", want, carried) } @@ -611,3 +611,57 @@ func TestIdentityGPGRefusesAnUncarriableGPGConf(t *testing.T) { t.Errorf("the refusal does not name %s:2 and the option:\n%s", p, r.out) } } + +// TestIdentityGPGHidesWhatTheHostHides: the host's gpg.conf sets throw-keyids +// and compress-algo none, and a message encrypted inside carries no recipient +// key ID (keyid 0000000000000000) and no compressed packet, as the host's +// does. The control is the host under --no-options: the real key ID and a +// compressed packet, so the assertions inside discriminate. Inside, gpg also +// starts no dirmngr when it cannot find a recipient: snug's disable-dirmngr +// stops the WKD lookup gpg's default would make (red team, #689). +func TestIdentityGPGHidesWhatTheHostHides(t *testing.T) { + budget(t, 30*time.Second) + h := newGPGHost(t) + proj, _ := target(t) + if err := os.WriteFile(filepath.Join(h.home, "gpg.conf"), + []byte("throw-keyids\ncompress-algo none\n"), 0o600); err != nil { + t.Fatal(err) + } + // What a host-encrypted message shows, decrypted by the host so the + // compressed packet (inside the encryption) is visible too. + packetsOf := func(enc string) string { + out, errs, err := h.gpgRun(nil, "--list-packets", enc) + if err != nil { + t.Fatalf("host could not list %s: %v\n%s", enc, err, errs) + } + return out + errs + } + ctl := filepath.Join(t.TempDir(), "ctl.gpg") + if _, errs, err := h.gpgRun(bytes.Repeat([]byte("hi "), 200), "--no-options", "--trust-model", "always", + "-o", ctl, "-e", "-r", h.fpr); err != nil { + t.Fatalf("control: host could not encrypt: %v\n%s", err, errs) + } + if p := packetsOf(ctl); strings.Contains(p, "keyid 0000000000000000") || !strings.Contains(p, "compressed packet") { + t.Fatalf("control: without the file the host hides the key ID or skips compression; "+ + "the test cannot discriminate:\n%s", p) + } + + r := runEnv(t, h.profile(h.fpr, "sign"), []string{"-p", "gpgid"}, proj, ` +yes hi | head -200 | gpg --batch --trust-model always -o enc.gpg -e -r `+h.fpr+`; echo "enc-exit=[$?]" +echo hi | gpg --batch -e -r nobody@example.org >/dev/null 2>&1 +ls -a "$GNUPGHOME" | grep -c dirmngr | sed 's/^/dirmngr=/' +`).mustRun(t) + if !strings.Contains(r.out, "enc-exit=[0]") { + t.Fatalf("encryption inside failed:\n%s", r.out) + } + p := packetsOf(filepath.Join(proj, "enc.gpg")) + if !strings.Contains(p, "keyid 0000000000000000") { + t.Errorf("a message encrypted inside names its recipient's key ID; the host's throw-keyids was dropped:\n%s", p) + } + if strings.Contains(p, "compressed packet") { + t.Errorf("a message encrypted inside is compressed; the host's compress-algo none was dropped:\n%s", p) + } + if !strings.Contains(r.out, "dirmngr=0") { + t.Errorf("gpg inside started a dirmngr for an unknown recipient:\n%s", r.out) + } +}