From 11223a9267de1259adf7fe8d906c7b169221e8ef Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Thu, 13 Feb 2025 16:31:24 +0200 Subject: [PATCH 1/9] secboot: add argon2 out-of-process command helpers Signed-off-by: Zeyad Gouda --- secboot/argon2_out_of_process_dummy.go | 25 +++++ secboot/argon2_out_of_process_sb.go | 91 ++++++++++++++++ secboot/argon2_out_of_process_sb_test.go | 127 +++++++++++++++++++++++ secboot/export_sb_test.go | 13 +++ 4 files changed, 256 insertions(+) create mode 100644 secboot/argon2_out_of_process_dummy.go create mode 100644 secboot/argon2_out_of_process_sb.go create mode 100644 secboot/argon2_out_of_process_sb_test.go diff --git a/secboot/argon2_out_of_process_dummy.go b/secboot/argon2_out_of_process_dummy.go new file mode 100644 index 00000000000..5d04eeffe3b --- /dev/null +++ b/secboot/argon2_out_of_process_dummy.go @@ -0,0 +1,25 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- +//go:build nosecboot + +/* + * Copyright (C) 2025 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package secboot + +func MaybeRunArgon2OutOfProcessRequestHandler() error { + return errBuildWithoutSecboot +} diff --git a/secboot/argon2_out_of_process_sb.go b/secboot/argon2_out_of_process_sb.go new file mode 100644 index 00000000000..cb1c9b96fc0 --- /dev/null +++ b/secboot/argon2_out_of_process_sb.go @@ -0,0 +1,91 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2025 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package secboot + +import ( + "fmt" + "os" + "os/exec" + "time" + + sb "github.com/snapcore/secboot" + + "github.com/snapcore/snapd/logger" +) + +func init() { + setArgon2KDF() +} + +const outOfProcessArgon2KDFTimeout = 100 * time.Millisecond +const argon2Cmd = "run-argon2" + +func setArgon2KDF() error { + // This assumes that the calling binary uses MaybeRunArgon2OutOfProcessRequestHandler early in main(). + exe, err := os.Readlink("/proc/self/exe") + if err != nil { + return err + } + + handlerCmd := func() (*exec.Cmd, error) { + cmd := exec.Command(exe, argon2Cmd) + return cmd, nil + } + argon2KDF := sb.NewOutOfProcessArgon2KDF(handlerCmd, outOfProcessArgon2KDFTimeout, nil) + sb.SetArgon2KDF(argon2KDF) + + return nil +} + +var osExit = os.Exit +var sbWaitForAndRunArgon2OutOfProcessRequest = sb.WaitForAndRunArgon2OutOfProcessRequest + +// MaybeRunArgon2OutOfProcessRequestHandler is supposed to be called +// from the main() of binaries involved with sealing/unsealing of +// keys (i.e. snapd and snap-bootstrap). +// +// This switches the binary to a special mode where it acts as an +// argon2 out-of-process helper command, and exits when its work +// is done. +// +// For more context, check docs for sb.WaitForAndRunArgon2OutOfProcessRequest +// and sb.NewOutOfProcessArgon2KDF for details on how the flow works +// in secboot. +func MaybeRunArgon2OutOfProcessRequestHandler() error { + if len(os.Args) < 2 || os.Args[1] != argon2Cmd { + return nil + } + + watchdog := sb.NoArgon2OutOfProcessWatchdogHandler() + + logger.Noticef("running argon2 out-of-process helper") + // Lock will be released implicitly on process termination, but let's also + // explicitly release lock. + lockRelease, err := sbWaitForAndRunArgon2OutOfProcessRequest(os.Stdin, os.Stdout, watchdog) + defer lockRelease() + if err != nil { + return fmt.Errorf("cannot run request: %w", err) + } + + // Argon2 request was processed successfully + osExit(0) + + panic("internal error: not reachable") +} diff --git a/secboot/argon2_out_of_process_sb_test.go b/secboot/argon2_out_of_process_sb_test.go new file mode 100644 index 00000000000..ffb19ce11b3 --- /dev/null +++ b/secboot/argon2_out_of_process_sb_test.go @@ -0,0 +1,127 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2025 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package secboot_test + +import ( + "errors" + "io" + + sb "github.com/snapcore/secboot" + . "gopkg.in/check.v1" + + "github.com/snapcore/snapd/secboot" +) + +type argon2Suite struct { +} + +var _ = Suite(&argon2Suite{}) + +func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandler(c *C) { + argon2Called := 0 + lockReleaseCalled := 0 + restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { + argon2Called++ + return func() { + lockReleaseCalled++ + }, nil + }) + defer restore() + + exitCalled := 0 + restore = secboot.MockOsExit(func(code int) { + exitCalled++ + c.Assert(code, Equals, 0) + }) + defer restore() + + restore = secboot.MockOsArgs([]string{"/path/to/cmd", "run-argon2"}) + defer restore() + + // Since we override os.Exit(0), we expect to panic + c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "internal error: not reachable") + + c.Check(argon2Called, Equals, 1) + c.Check(exitCalled, Equals, 1) + c.Check(lockReleaseCalled, Equals, 1) +} + +func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNotTriggered(c *C) { + argon2Called := 0 + lockReleaseCalled := 0 + restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { + argon2Called++ + return func() { + lockReleaseCalled++ + }, nil + }) + defer restore() + + exitCalled := 0 + restore = secboot.MockOsExit(func(code int) { + exitCalled++ + c.Assert(code, Equals, 0) + }) + defer restore() + + for _, args := range [][]string{ + {}, + {"/path/to/cmd"}, + {"/path/to/cmd", "not-run-argon2"}, + {"/path/to/cmd", "not-run-argon2", "run-argon2"}, + } { + restore := secboot.MockOsArgs(args) + defer restore() + err := secboot.MaybeRunArgon2OutOfProcessRequestHandler() + c.Assert(err, IsNil) + } + + c.Check(argon2Called, Equals, 0) + c.Check(exitCalled, Equals, 0) + c.Check(lockReleaseCalled, Equals, 0) +} + +func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerError(c *C) { + argon2Called := 0 + lockReleaseCalled := 0 + restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { + argon2Called++ + return func() { + lockReleaseCalled++ + }, errors.New("boom!") + }) + defer restore() + + exitCalled := 0 + restore = secboot.MockOsExit(func(code int) { + exitCalled++ + c.Assert(code, Equals, 0) + }) + defer restore() + + restore = secboot.MockOsArgs([]string{"/path/to/cmd", "run-argon2"}) + defer restore() + + c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler(), ErrorMatches, "cannot run request: boom!") + + c.Check(argon2Called, Equals, 1) + c.Check(exitCalled, Equals, 0) + c.Check(lockReleaseCalled, Equals, 1) +} diff --git a/secboot/export_sb_test.go b/secboot/export_sb_test.go index 1dad283556f..ee0dd8ef0db 100644 --- a/secboot/export_sb_test.go +++ b/secboot/export_sb_test.go @@ -22,6 +22,7 @@ package secboot import ( "io" + "os" "github.com/canonical/go-tpm2" sb "github.com/snapcore/secboot" @@ -449,3 +450,15 @@ func MockDisksDevlinks(f func(node string) ([]string, error)) (restore func()) { disksDevlinks = old } } + +func MockOsArgs(args []string) (restore func()) { + return testutil.Mock(&os.Args, args) +} + +func MockOsExit(f func(code int)) (restore func()) { + return testutil.Mock(&osExit, f) +} + +func MockSbWaitForAndRunArgon2OutOfProcessRequest(f func(in io.Reader, out io.WriteCloser, watchdog sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error)) (restore func()) { + return testutil.Mock(&sbWaitForAndRunArgon2OutOfProcessRequest, f) +} From ed2415915d1c3e58c2f3d7e0393455e7d1e85276 Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Thu, 13 Feb 2025 16:32:37 +0200 Subject: [PATCH 2/9] cmd/{snapd,snap-bootstrap}: add argon2 out-of-process special mode sub-commands Signed-off-by: Zeyad Gouda --- cmd/snap-bootstrap/main.go | 6 ++++++ cmd/snapd/main.go | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/cmd/snap-bootstrap/main.go b/cmd/snap-bootstrap/main.go index 82a649cd95c..c770c0e8d03 100644 --- a/cmd/snap-bootstrap/main.go +++ b/cmd/snap-bootstrap/main.go @@ -26,6 +26,7 @@ import ( "github.com/jessevdk/go-flags" "github.com/snapcore/snapd/logger" + "github.com/snapcore/snapd/secboot" ) var ( @@ -40,6 +41,11 @@ such as initramfs. ) func main() { + if err := secboot.MaybeRunArgon2OutOfProcessRequestHandler(); err != nil { + fmt.Fprintf(os.Stderr, "cannot run argon2 out-of-process helper command: %v", err) + os.Exit(1) + } + err := run(os.Args[1:]) if err != nil { fmt.Fprintf(os.Stderr, "error: %s\n", err) diff --git a/cmd/snapd/main.go b/cmd/snapd/main.go index bf5b6e7b1c2..1dd9cf251ca 100644 --- a/cmd/snapd/main.go +++ b/cmd/snapd/main.go @@ -32,6 +32,7 @@ import ( "github.com/snapcore/snapd/logger" "github.com/snapcore/snapd/osutil" "github.com/snapcore/snapd/sandbox" + "github.com/snapcore/snapd/secboot" "github.com/snapcore/snapd/snapdenv" "github.com/snapcore/snapd/snapdtool" "github.com/snapcore/snapd/syscheck" @@ -57,6 +58,11 @@ func main() { snapdtool.ExecInSnapdOrCoreSnap() } + if err := secboot.MaybeRunArgon2OutOfProcessRequestHandler(); err != nil { + fmt.Fprintf(os.Stderr, "cannot run argon2 out-of-process helper command: %v", err) + os.Exit(1) + } + if err := snapdtool.MaybeSetupFIPS(); err != nil { fmt.Fprintf(os.Stderr, "cannot check or enable FIPS mode: %v", err) os.Exit(1) From 4d9e5c0641d65bbe30f09f25e03e6d9f04dfc60b Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Thu, 13 Feb 2025 16:34:03 +0200 Subject: [PATCH 3/9] Revert "secboot: default to pbkdf2 instead of argon2id (#15058)" This reverts commit ec60555ccf29535ceb1c542aca043d56c5665f4b. --- secboot/secboot_sb_test.go | 2 +- secboot/secboot_tpm.go | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/secboot/secboot_sb_test.go b/secboot/secboot_sb_test.go index 64cc3d48dee..d2e32a44fff 100644 --- a/secboot/secboot_sb_test.go +++ b/secboot/secboot_sb_test.go @@ -1161,7 +1161,7 @@ func (s *secbootSuite) TestSealKey(c *C) { expectedKDFOptions = &sb.Argon2Options{Mode: sb.Argon2id, TargetDuration: tc.volumesAuth.KDFTime} case "argon2i": expectedKDFOptions = &sb.Argon2Options{Mode: sb.Argon2i, TargetDuration: tc.volumesAuth.KDFTime} - case "pbkdf2", "": + case "pbkdf2": expectedKDFOptions = &sb.PBKDF2Options{TargetDuration: tc.volumesAuth.KDFTime} } c.Assert(params.KDFOptions, DeepEquals, expectedKDFOptions) diff --git a/secboot/secboot_tpm.go b/secboot/secboot_tpm.go index 9e46b979320..798efec35d4 100644 --- a/secboot/secboot_tpm.go +++ b/secboot/secboot_tpm.go @@ -487,20 +487,13 @@ func ProvisionForCVM(initramfsUbuntuSeedDir string) error { func kdfOptions(volumesAuth *device.VolumesAuthOptions) (sb.KDFOptions, error) { switch volumesAuth.KDFType { case "": - // TODO:FDEM:FIX: default to out-of-process argon2id implementation - return &sb.PBKDF2Options{ - TargetDuration: volumesAuth.KDFTime, - }, nil + return nil, nil case "argon2id": - // TODO:FDEM:FIX: sb.SetArgon2KDF(sb.InProcessArgon2KDF) or intentionally fail - // until out-of-process variant is implemented? return &sb.Argon2Options{ Mode: sb.Argon2id, TargetDuration: volumesAuth.KDFTime, }, nil case "argon2i": - // TODO:FDEM:FIX: sb.SetArgon2KDF(sb.InProcessArgon2KDF) or intentionally fail - // until out-of-process variant is implemented? return &sb.Argon2Options{ Mode: sb.Argon2i, TargetDuration: volumesAuth.KDFTime, From 6df58d89bbcfd7b12077be03b1206c3d9c7c48af Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Fri, 14 Feb 2025 10:05:16 +0200 Subject: [PATCH 4/9] fixup! secboot: use --argon2-proc arg instead of run-argon2 sub-command Signed-off-by: Zeyad Gouda --- secboot/argon2_out_of_process_sb.go | 12 ++++++------ secboot/argon2_out_of_process_sb_test.go | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/secboot/argon2_out_of_process_sb.go b/secboot/argon2_out_of_process_sb.go index cb1c9b96fc0..8cfe49d8d59 100644 --- a/secboot/argon2_out_of_process_sb.go +++ b/secboot/argon2_out_of_process_sb.go @@ -35,7 +35,7 @@ func init() { } const outOfProcessArgon2KDFTimeout = 100 * time.Millisecond -const argon2Cmd = "run-argon2" +const outOfProcessArgon2Arg = "--argon2-proc" func setArgon2KDF() error { // This assumes that the calling binary uses MaybeRunArgon2OutOfProcessRequestHandler early in main(). @@ -45,7 +45,7 @@ func setArgon2KDF() error { } handlerCmd := func() (*exec.Cmd, error) { - cmd := exec.Command(exe, argon2Cmd) + cmd := exec.Command(exe, outOfProcessArgon2Arg) return cmd, nil } argon2KDF := sb.NewOutOfProcessArgon2KDF(handlerCmd, outOfProcessArgon2KDFTimeout, nil) @@ -61,15 +61,15 @@ var sbWaitForAndRunArgon2OutOfProcessRequest = sb.WaitForAndRunArgon2OutOfProces // from the main() of binaries involved with sealing/unsealing of // keys (i.e. snapd and snap-bootstrap). // -// This switches the binary to a special mode where it acts as an -// argon2 out-of-process helper command, and exits when its work -// is done. +// This switches the binary to a special mode when the --argon2-proc arg +// is detected where it acts as an argon2 out-of-process helper command +// and exits when its work is done. // // For more context, check docs for sb.WaitForAndRunArgon2OutOfProcessRequest // and sb.NewOutOfProcessArgon2KDF for details on how the flow works // in secboot. func MaybeRunArgon2OutOfProcessRequestHandler() error { - if len(os.Args) < 2 || os.Args[1] != argon2Cmd { + if len(os.Args) < 2 || os.Args[1] != outOfProcessArgon2Arg { return nil } diff --git a/secboot/argon2_out_of_process_sb_test.go b/secboot/argon2_out_of_process_sb_test.go index ffb19ce11b3..c286017eb2a 100644 --- a/secboot/argon2_out_of_process_sb_test.go +++ b/secboot/argon2_out_of_process_sb_test.go @@ -52,7 +52,7 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandler(c *C) { }) defer restore() - restore = secboot.MockOsArgs([]string{"/path/to/cmd", "run-argon2"}) + restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"}) defer restore() // Since we override os.Exit(0), we expect to panic @@ -85,7 +85,7 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNotTriggered(c * {}, {"/path/to/cmd"}, {"/path/to/cmd", "not-run-argon2"}, - {"/path/to/cmd", "not-run-argon2", "run-argon2"}, + {"/path/to/cmd", "not-run-argon2", "--argon2-proc"}, } { restore := secboot.MockOsArgs(args) defer restore() @@ -116,7 +116,7 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerError(c *C) { }) defer restore() - restore = secboot.MockOsArgs([]string{"/path/to/cmd", "run-argon2"}) + restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"}) defer restore() c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler(), ErrorMatches, "cannot run request: boom!") From 02ca93371a76f41a973c701e2bae46a5b1e9b390 Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Fri, 14 Feb 2025 11:19:28 +0200 Subject: [PATCH 5/9] fixup! secboot: handle error directly in MaybeRunArgon2OutOfProcessRequestHandler Signed-off-by: Zeyad Gouda --- cmd/snap-bootstrap/main.go | 5 +--- cmd/snapd/main.go | 5 +--- secboot/argon2_out_of_process_dummy.go | 4 +-- secboot/argon2_out_of_process_sb.go | 14 +++++----- secboot/argon2_out_of_process_sb_test.go | 35 +++++++++--------------- 5 files changed, 23 insertions(+), 40 deletions(-) diff --git a/cmd/snap-bootstrap/main.go b/cmd/snap-bootstrap/main.go index c770c0e8d03..95301821a52 100644 --- a/cmd/snap-bootstrap/main.go +++ b/cmd/snap-bootstrap/main.go @@ -41,10 +41,7 @@ such as initramfs. ) func main() { - if err := secboot.MaybeRunArgon2OutOfProcessRequestHandler(); err != nil { - fmt.Fprintf(os.Stderr, "cannot run argon2 out-of-process helper command: %v", err) - os.Exit(1) - } + secboot.MaybeRunArgon2OutOfProcessRequestHandler() err := run(os.Args[1:]) if err != nil { diff --git a/cmd/snapd/main.go b/cmd/snapd/main.go index 1dd9cf251ca..ace9c7ea1e9 100644 --- a/cmd/snapd/main.go +++ b/cmd/snapd/main.go @@ -58,10 +58,7 @@ func main() { snapdtool.ExecInSnapdOrCoreSnap() } - if err := secboot.MaybeRunArgon2OutOfProcessRequestHandler(); err != nil { - fmt.Fprintf(os.Stderr, "cannot run argon2 out-of-process helper command: %v", err) - os.Exit(1) - } + secboot.MaybeRunArgon2OutOfProcessRequestHandler() if err := snapdtool.MaybeSetupFIPS(); err != nil { fmt.Fprintf(os.Stderr, "cannot check or enable FIPS mode: %v", err) diff --git a/secboot/argon2_out_of_process_dummy.go b/secboot/argon2_out_of_process_dummy.go index 5d04eeffe3b..5631ba413db 100644 --- a/secboot/argon2_out_of_process_dummy.go +++ b/secboot/argon2_out_of_process_dummy.go @@ -20,6 +20,4 @@ package secboot -func MaybeRunArgon2OutOfProcessRequestHandler() error { - return errBuildWithoutSecboot -} +func MaybeRunArgon2OutOfProcessRequestHandler() {} diff --git a/secboot/argon2_out_of_process_sb.go b/secboot/argon2_out_of_process_sb.go index 8cfe49d8d59..52a9a3b250b 100644 --- a/secboot/argon2_out_of_process_sb.go +++ b/secboot/argon2_out_of_process_sb.go @@ -1,4 +1,5 @@ // -*- Mode: Go; indent-tabs-mode: t -*- +//go:build !nosecboot /* * Copyright (C) 2025 Canonical Ltd @@ -68,20 +69,19 @@ var sbWaitForAndRunArgon2OutOfProcessRequest = sb.WaitForAndRunArgon2OutOfProces // For more context, check docs for sb.WaitForAndRunArgon2OutOfProcessRequest // and sb.NewOutOfProcessArgon2KDF for details on how the flow works // in secboot. -func MaybeRunArgon2OutOfProcessRequestHandler() error { +func MaybeRunArgon2OutOfProcessRequestHandler() { if len(os.Args) < 2 || os.Args[1] != outOfProcessArgon2Arg { - return nil + return } watchdog := sb.NoArgon2OutOfProcessWatchdogHandler() logger.Noticef("running argon2 out-of-process helper") - // Lock will be released implicitly on process termination, but let's also - // explicitly release lock. - lockRelease, err := sbWaitForAndRunArgon2OutOfProcessRequest(os.Stdin, os.Stdout, watchdog) - defer lockRelease() + // Ignore the lock release callback and use implicit release on process termination. + _, err := sbWaitForAndRunArgon2OutOfProcessRequest(os.Stdin, os.Stdout, watchdog) if err != nil { - return fmt.Errorf("cannot run request: %w", err) + fmt.Fprintf(os.Stderr, "cannot run argon2 out-of-process request: %v", err) + osExit(1) } // Argon2 request was processed successfully diff --git a/secboot/argon2_out_of_process_sb_test.go b/secboot/argon2_out_of_process_sb_test.go index c286017eb2a..73c69d64a4c 100644 --- a/secboot/argon2_out_of_process_sb_test.go +++ b/secboot/argon2_out_of_process_sb_test.go @@ -36,12 +36,9 @@ var _ = Suite(&argon2Suite{}) func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandler(c *C) { argon2Called := 0 - lockReleaseCalled := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { argon2Called++ - return func() { - lockReleaseCalled++ - }, nil + return nil, nil }) defer restore() @@ -49,28 +46,25 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandler(c *C) { restore = secboot.MockOsExit(func(code int) { exitCalled++ c.Assert(code, Equals, 0) + panic("os.Exit(0)") }) defer restore() restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"}) defer restore() - // Since we override os.Exit(0), we expect to panic - c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "internal error: not reachable") + // Since we override os.Exit(0), we expect to panic (injected above) + c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(0)") c.Check(argon2Called, Equals, 1) c.Check(exitCalled, Equals, 1) - c.Check(lockReleaseCalled, Equals, 1) } func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNotTriggered(c *C) { argon2Called := 0 - lockReleaseCalled := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { argon2Called++ - return func() { - lockReleaseCalled++ - }, nil + return nil, nil }) defer restore() @@ -78,6 +72,7 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNotTriggered(c * restore = secboot.MockOsExit(func(code int) { exitCalled++ c.Assert(code, Equals, 0) + panic("injected panic") }) defer restore() @@ -89,39 +84,35 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNotTriggered(c * } { restore := secboot.MockOsArgs(args) defer restore() - err := secboot.MaybeRunArgon2OutOfProcessRequestHandler() - c.Assert(err, IsNil) + // This should exit early before running the argon2 helper and calling os.Exit (and no injected panic) + secboot.MaybeRunArgon2OutOfProcessRequestHandler() } c.Check(argon2Called, Equals, 0) c.Check(exitCalled, Equals, 0) - c.Check(lockReleaseCalled, Equals, 0) } func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerError(c *C) { argon2Called := 0 - lockReleaseCalled := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { argon2Called++ - return func() { - lockReleaseCalled++ - }, errors.New("boom!") + return nil, errors.New("boom!") }) defer restore() exitCalled := 0 restore = secboot.MockOsExit(func(code int) { exitCalled++ - c.Assert(code, Equals, 0) + c.Assert(code, Equals, 1) + panic("os.Exit(1)") }) defer restore() restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"}) defer restore() - c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler(), ErrorMatches, "cannot run request: boom!") + c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(1)") c.Check(argon2Called, Equals, 1) - c.Check(exitCalled, Equals, 0) - c.Check(lockReleaseCalled, Equals, 1) + c.Check(exitCalled, Equals, 1) } From 3e1485de687a6b479d1a31fbc108495d0f8f4931 Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Fri, 14 Feb 2025 11:33:42 +0200 Subject: [PATCH 6/9] fixup! secboot: disable argon2 out-of-process tests when built with nosecboot Signed-off-by: Zeyad Gouda --- secboot/argon2_out_of_process_sb_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/secboot/argon2_out_of_process_sb_test.go b/secboot/argon2_out_of_process_sb_test.go index 73c69d64a4c..7af76e0b515 100644 --- a/secboot/argon2_out_of_process_sb_test.go +++ b/secboot/argon2_out_of_process_sb_test.go @@ -1,4 +1,5 @@ // -*- Mode: Go; indent-tabs-mode: t -*- +//go:build !nosecboot /* * Copyright (C) 2025 Canonical Ltd From ce2f4851271295de165819cbc3704434eacc6869 Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Fri, 14 Feb 2025 13:06:05 +0200 Subject: [PATCH 7/9] fixup! secboot: set argon2 kdf from MaybeRunArgon2OutOfProcessRequestHandler Signed-off-by: Zeyad Gouda --- secboot/argon2_out_of_process_sb.go | 64 ++++++------ secboot/argon2_out_of_process_sb_test.go | 125 +++++++++++++++++------ secboot/export_sb_test.go | 14 +++ 3 files changed, 141 insertions(+), 62 deletions(-) diff --git a/secboot/argon2_out_of_process_sb.go b/secboot/argon2_out_of_process_sb.go index 52a9a3b250b..c0d9c1176d0 100644 --- a/secboot/argon2_out_of_process_sb.go +++ b/secboot/argon2_out_of_process_sb.go @@ -31,54 +31,60 @@ import ( "github.com/snapcore/snapd/logger" ) -func init() { - setArgon2KDF() -} - -const outOfProcessArgon2KDFTimeout = 100 * time.Millisecond -const outOfProcessArgon2Arg = "--argon2-proc" +var ( + osExit = os.Exit + osReadlink = os.Readlink -func setArgon2KDF() error { - // This assumes that the calling binary uses MaybeRunArgon2OutOfProcessRequestHandler early in main(). - exe, err := os.Readlink("/proc/self/exe") - if err != nil { - return err - } + sbWaitForAndRunArgon2OutOfProcessRequest = sb.WaitForAndRunArgon2OutOfProcessRequest + sbNewOutOfProcessArgon2KDF = sb.NewOutOfProcessArgon2KDF + sbSetArgon2KDF = sb.SetArgon2KDF +) - handlerCmd := func() (*exec.Cmd, error) { - cmd := exec.Command(exe, outOfProcessArgon2Arg) - return cmd, nil - } - argon2KDF := sb.NewOutOfProcessArgon2KDF(handlerCmd, outOfProcessArgon2KDFTimeout, nil) - sb.SetArgon2KDF(argon2KDF) +const ( + outOfProcessArgon2KDFTimeout = 100 * time.Millisecond + outOfProcessArgon2Arg = "--argon2-proc" +) - return nil +func isOutOfProcessArgon2KDFMode() bool { + return len(os.Args) >= 2 && os.Args[1] == outOfProcessArgon2Arg } -var osExit = os.Exit -var sbWaitForAndRunArgon2OutOfProcessRequest = sb.WaitForAndRunArgon2OutOfProcessRequest - // MaybeRunArgon2OutOfProcessRequestHandler is supposed to be called // from the main() of binaries involved with sealing/unsealing of // keys (i.e. snapd and snap-bootstrap). // -// This switches the binary to a special mode when the --argon2-proc arg -// is detected where it acts as an argon2 out-of-process helper command -// and exits when its work is done. +// This switches the binary to a special argon2 mode when the --argon2-proc arg +// is detected where it acts as an argon2 out-of-process helper command and +// exits when its work is done, otherwise (in normal mode) it sets the default +// argon2 kdf implementation be self-invoking into the special argon2 mode of +// the calling binary. // // For more context, check docs for sb.WaitForAndRunArgon2OutOfProcessRequest // and sb.NewOutOfProcessArgon2KDF for details on how the flow works // in secboot. func MaybeRunArgon2OutOfProcessRequestHandler() { - if len(os.Args) < 2 || os.Args[1] != outOfProcessArgon2Arg { + if !isOutOfProcessArgon2KDFMode() { + // Binary was invoked in normal mode, let's setup default argon2 kdf implementation + // to point to this binary when invoked using special args. + exe, err := osReadlink("/proc/self/exe") + if err != nil { + logger.Noticef("internal error: failed to read symlink of /proc/self/exe: %v", err) + return + } + + handlerCmd := func() (*exec.Cmd, error) { + cmd := exec.Command(exe, outOfProcessArgon2Arg) + return cmd, nil + } + argon2KDF := sbNewOutOfProcessArgon2KDF(handlerCmd, outOfProcessArgon2KDFTimeout, nil) + sbSetArgon2KDF(argon2KDF) + return } - watchdog := sb.NoArgon2OutOfProcessWatchdogHandler() - logger.Noticef("running argon2 out-of-process helper") // Ignore the lock release callback and use implicit release on process termination. - _, err := sbWaitForAndRunArgon2OutOfProcessRequest(os.Stdin, os.Stdout, watchdog) + _, err := sbWaitForAndRunArgon2OutOfProcessRequest(os.Stdin, os.Stdout, sb.NoArgon2OutOfProcessWatchdogHandler()) if err != nil { fmt.Fprintf(os.Stderr, "cannot run argon2 out-of-process request: %v", err) osExit(1) diff --git a/secboot/argon2_out_of_process_sb_test.go b/secboot/argon2_out_of_process_sb_test.go index 7af76e0b515..1cc29861332 100644 --- a/secboot/argon2_out_of_process_sb_test.go +++ b/secboot/argon2_out_of_process_sb_test.go @@ -23,6 +23,8 @@ package secboot_test import ( "errors" "io" + "os/exec" + "time" sb "github.com/snapcore/secboot" . "gopkg.in/check.v1" @@ -35,14 +37,21 @@ type argon2Suite struct { var _ = Suite(&argon2Suite{}) -func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandler(c *C) { - argon2Called := 0 +func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2Mode(c *C) { + runArgon2Called := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { - argon2Called++ + runArgon2Called++ return nil, nil }) defer restore() + setArgon2Called := 0 + restore = secboot.MockSbSetArgon2KDF(func(kdf sb.Argon2KDF) sb.Argon2KDF { + setArgon2Called++ + return nil + }) + defer restore() + exitCalled := 0 restore = secboot.MockOsExit(func(code int) { exitCalled++ @@ -57,63 +66,113 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandler(c *C) { // Since we override os.Exit(0), we expect to panic (injected above) c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(0)") - c.Check(argon2Called, Equals, 1) + c.Check(setArgon2Called, Equals, 0) + c.Check(runArgon2Called, Equals, 1) c.Check(exitCalled, Equals, 1) } -func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNotTriggered(c *C) { - argon2Called := 0 +func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2ModeError(c *C) { + runArgon2Called := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { - argon2Called++ - return nil, nil + runArgon2Called++ + return nil, errors.New("boom!") + }) + defer restore() + + setArgon2Called := 0 + restore = secboot.MockSbSetArgon2KDF(func(kdf sb.Argon2KDF) sb.Argon2KDF { + setArgon2Called++ + return nil }) defer restore() exitCalled := 0 restore = secboot.MockOsExit(func(code int) { exitCalled++ - c.Assert(code, Equals, 0) - panic("injected panic") + c.Assert(code, Equals, 1) + panic("os.Exit(1)") }) defer restore() - for _, args := range [][]string{ - {}, - {"/path/to/cmd"}, - {"/path/to/cmd", "not-run-argon2"}, - {"/path/to/cmd", "not-run-argon2", "--argon2-proc"}, - } { - restore := secboot.MockOsArgs(args) - defer restore() - // This should exit early before running the argon2 helper and calling os.Exit (and no injected panic) - secboot.MaybeRunArgon2OutOfProcessRequestHandler() - } + restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"}) + defer restore() - c.Check(argon2Called, Equals, 0) - c.Check(exitCalled, Equals, 0) + c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(1)") + + c.Check(setArgon2Called, Equals, 0) + c.Check(runArgon2Called, Equals, 1) + c.Check(exitCalled, Equals, 1) } -func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerError(c *C) { - argon2Called := 0 +type mockArgon2KDF struct{} + +func (*mockArgon2KDF) Derive(passphrase string, salt []byte, mode sb.Argon2Mode, params *sb.Argon2CostParams, keyLen uint32) ([]byte, error) { + return nil, nil +} + +func (*mockArgon2KDF) Time(mode sb.Argon2Mode, params *sb.Argon2CostParams) (time.Duration, error) { + return 0, nil +} + +func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNormalMode(c *C) { + runArgon2Called := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { - argon2Called++ - return nil, errors.New("boom!") + runArgon2Called++ + return nil, nil }) defer restore() exitCalled := 0 restore = secboot.MockOsExit(func(code int) { exitCalled++ - c.Assert(code, Equals, 1) - panic("os.Exit(1)") + c.Assert(code, Equals, 0) + panic("injected panic") }) defer restore() - restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"}) + restore = secboot.MockOsReadlink(func(name string) (string, error) { + c.Assert(name, Equals, "/proc/self/exe") + return "/path/to/cmd", nil + }) defer restore() - c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(1)") + argon2KDF := &mockArgon2KDF{} - c.Check(argon2Called, Equals, 1) - c.Check(exitCalled, Equals, 1) + restore = secboot.MockSbNewOutOfProcessArgon2KDF(func(newHandlerCmd func() (*exec.Cmd, error), timeout time.Duration, watchdog sb.Argon2OutOfProcessWatchdogMonitor) sb.Argon2KDF { + c.Check(timeout, Equals, 100*time.Millisecond) + c.Check(watchdog, IsNil) + + cmd, err := newHandlerCmd() + c.Assert(err, IsNil) + c.Check(cmd.Path, Equals, "/path/to/cmd") + c.Check(cmd.Args, DeepEquals, []string{"/path/to/cmd", "--argon2-proc"}) + + return argon2KDF + }) + defer restore() + + setArgon2Called := 0 + restore = secboot.MockSbSetArgon2KDF(func(kdf sb.Argon2KDF) sb.Argon2KDF { + setArgon2Called++ + // Check pointer points to mock implementation + c.Assert(kdf, Equals, argon2KDF) + return nil + }) + defer restore() + + for _, args := range [][]string{ + {}, + {"/path/to/cmd"}, + {"/path/to/cmd", "not-run-argon2"}, + {"/path/to/cmd", "not-run-argon2", "--argon2-proc"}, + } { + restore := secboot.MockOsArgs(args) + defer restore() + // This should exit early before running the argon2 helper and calling os.Exit (and no injected panic) + secboot.MaybeRunArgon2OutOfProcessRequestHandler() + } + + c.Check(setArgon2Called, Equals, 4) + c.Check(runArgon2Called, Equals, 0) + c.Check(exitCalled, Equals, 0) } diff --git a/secboot/export_sb_test.go b/secboot/export_sb_test.go index ee0dd8ef0db..0b8a554b173 100644 --- a/secboot/export_sb_test.go +++ b/secboot/export_sb_test.go @@ -23,6 +23,8 @@ package secboot import ( "io" "os" + "os/exec" + "time" "github.com/canonical/go-tpm2" sb "github.com/snapcore/secboot" @@ -459,6 +461,18 @@ func MockOsExit(f func(code int)) (restore func()) { return testutil.Mock(&osExit, f) } +func MockOsReadlink(f func(name string) (string, error)) (restore func()) { + return testutil.Mock(&osReadlink, f) +} + func MockSbWaitForAndRunArgon2OutOfProcessRequest(f func(in io.Reader, out io.WriteCloser, watchdog sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error)) (restore func()) { return testutil.Mock(&sbWaitForAndRunArgon2OutOfProcessRequest, f) } + +func MockSbNewOutOfProcessArgon2KDF(f func(newHandlerCmd func() (*exec.Cmd, error), timeout time.Duration, watchdog sb.Argon2OutOfProcessWatchdogMonitor) sb.Argon2KDF) (restore func()) { + return testutil.Mock(&sbNewOutOfProcessArgon2KDF, f) +} + +func MockSbSetArgon2KDF(f func(kdf sb.Argon2KDF) sb.Argon2KDF) (restore func()) { + return testutil.Mock(&sbSetArgon2KDF, f) +} From 3cebcc4523431d376cd9c3e9e5173bdc35498564 Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Fri, 14 Feb 2025 14:52:12 +0200 Subject: [PATCH 8/9] fixup! secboot: allow passing matching args for argon2 special mode Signed-off-by: Zeyad Gouda --- cmd/snap-bootstrap/main.go | 2 +- cmd/snapd/main.go | 2 +- secboot/argon2_out_of_process_dummy.go | 6 +++++- secboot/argon2_out_of_process_sb.go | 27 ++++++++++-------------- secboot/argon2_out_of_process_sb_test.go | 14 ++++++------ secboot/secboot.go | 15 +++++++++++++ 6 files changed, 41 insertions(+), 25 deletions(-) diff --git a/cmd/snap-bootstrap/main.go b/cmd/snap-bootstrap/main.go index 95301821a52..a9fce8a8f58 100644 --- a/cmd/snap-bootstrap/main.go +++ b/cmd/snap-bootstrap/main.go @@ -41,7 +41,7 @@ such as initramfs. ) func main() { - secboot.MaybeRunArgon2OutOfProcessRequestHandler() + secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"--argon2-proc"}) err := run(os.Args[1:]) if err != nil { diff --git a/cmd/snapd/main.go b/cmd/snapd/main.go index ace9c7ea1e9..5892069e702 100644 --- a/cmd/snapd/main.go +++ b/cmd/snapd/main.go @@ -58,7 +58,7 @@ func main() { snapdtool.ExecInSnapdOrCoreSnap() } - secboot.MaybeRunArgon2OutOfProcessRequestHandler() + secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"--argon2-proc"}) if err := snapdtool.MaybeSetupFIPS(); err != nil { fmt.Fprintf(os.Stderr, "cannot check or enable FIPS mode: %v", err) diff --git a/secboot/argon2_out_of_process_dummy.go b/secboot/argon2_out_of_process_dummy.go index 5631ba413db..4dae4db8e09 100644 --- a/secboot/argon2_out_of_process_dummy.go +++ b/secboot/argon2_out_of_process_dummy.go @@ -20,4 +20,8 @@ package secboot -func MaybeRunArgon2OutOfProcessRequestHandler() {} +func HijackAndRunArgon2OutOfProcessHandlerOnArg(args []string) { + if isOutOfProcessArgon2KDFMode(args) { + panic("internal error: unexpected call to execute as argon2 runner in non-secboot binary") + } +} diff --git a/secboot/argon2_out_of_process_sb.go b/secboot/argon2_out_of_process_sb.go index c0d9c1176d0..da289b4b18a 100644 --- a/secboot/argon2_out_of_process_sb.go +++ b/secboot/argon2_out_of_process_sb.go @@ -42,28 +42,23 @@ var ( const ( outOfProcessArgon2KDFTimeout = 100 * time.Millisecond - outOfProcessArgon2Arg = "--argon2-proc" ) -func isOutOfProcessArgon2KDFMode() bool { - return len(os.Args) >= 2 && os.Args[1] == outOfProcessArgon2Arg -} - -// MaybeRunArgon2OutOfProcessRequestHandler is supposed to be called -// from the main() of binaries involved with sealing/unsealing of -// keys (i.e. snapd and snap-bootstrap). +// HijackAndRunArgon2OutOfProcessHandlerOnArg is supposed to be called from the +// main() of binaries involved with sealing/unsealing of keys (i.e. snapd and +// snap-bootstrap). // -// This switches the binary to a special argon2 mode when the --argon2-proc arg -// is detected where it acts as an argon2 out-of-process helper command and -// exits when its work is done, otherwise (in normal mode) it sets the default -// argon2 kdf implementation be self-invoking into the special argon2 mode of -// the calling binary. +// This switches the binary to a special argon2 mode when the matching args are +// detected where it hijacks the process and acts as an argon2 out-of-process +// helper command and exits when its work is done, otherwise (in normal mode) +// it sets the default argon2 kdf implementation to be self-invoking into the +// special argon2 mode of the calling binary. // // For more context, check docs for sb.WaitForAndRunArgon2OutOfProcessRequest // and sb.NewOutOfProcessArgon2KDF for details on how the flow works // in secboot. -func MaybeRunArgon2OutOfProcessRequestHandler() { - if !isOutOfProcessArgon2KDFMode() { +func HijackAndRunArgon2OutOfProcessHandlerOnArg(args []string) { + if !isOutOfProcessArgon2KDFMode(args) { // Binary was invoked in normal mode, let's setup default argon2 kdf implementation // to point to this binary when invoked using special args. exe, err := osReadlink("/proc/self/exe") @@ -73,7 +68,7 @@ func MaybeRunArgon2OutOfProcessRequestHandler() { } handlerCmd := func() (*exec.Cmd, error) { - cmd := exec.Command(exe, outOfProcessArgon2Arg) + cmd := exec.Command(exe, args...) return cmd, nil } argon2KDF := sbNewOutOfProcessArgon2KDF(handlerCmd, outOfProcessArgon2KDFTimeout, nil) diff --git a/secboot/argon2_out_of_process_sb_test.go b/secboot/argon2_out_of_process_sb_test.go index 1cc29861332..0ffd29c0c1d 100644 --- a/secboot/argon2_out_of_process_sb_test.go +++ b/secboot/argon2_out_of_process_sb_test.go @@ -60,11 +60,12 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2Mode(c *C) }) defer restore() - restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"}) + restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--test-special-argon2-mode"}) defer restore() // Since we override os.Exit(0), we expect to panic (injected above) - c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(0)") + f := func() { secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"--test-special-argon2-mode"}) } + c.Assert(f, Panics, "os.Exit(0)") c.Check(setArgon2Called, Equals, 0) c.Check(runArgon2Called, Equals, 1) @@ -94,10 +95,11 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2ModeError( }) defer restore() - restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"}) + restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--test-special-argon2-mode"}) defer restore() - c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(1)") + f := func() { secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"--test-special-argon2-mode"}) } + c.Assert(f, Panics, "os.Exit(1)") c.Check(setArgon2Called, Equals, 0) c.Check(runArgon2Called, Equals, 1) @@ -145,7 +147,7 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNormalMode(c *C) cmd, err := newHandlerCmd() c.Assert(err, IsNil) c.Check(cmd.Path, Equals, "/path/to/cmd") - c.Check(cmd.Args, DeepEquals, []string{"/path/to/cmd", "--argon2-proc"}) + c.Check(cmd.Args, DeepEquals, []string{"/path/to/cmd", "--test-special-argon2-mode"}) return argon2KDF }) @@ -169,7 +171,7 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNormalMode(c *C) restore := secboot.MockOsArgs(args) defer restore() // This should exit early before running the argon2 helper and calling os.Exit (and no injected panic) - secboot.MaybeRunArgon2OutOfProcessRequestHandler() + secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"--test-special-argon2-mode"}) } c.Check(setArgon2Called, Equals, 4) diff --git a/secboot/secboot.go b/secboot/secboot.go index 9b251842fd8..e08ed0f1d4e 100644 --- a/secboot/secboot.go +++ b/secboot/secboot.go @@ -26,6 +26,7 @@ package secboot import ( "errors" + "os" "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/bootloader" @@ -268,3 +269,17 @@ func MarkSuccessful() error { return nil } + +func isOutOfProcessArgon2KDFMode(args []string) bool { + if len(os.Args) != len(args)+1 { + return false + } + + for i := 0; i < len(args); i++ { + if os.Args[i+1] != args[i] { + return false + } + } + + return true +} From b52bbd27d0bde2ef955f63a444ab0991f79edce8 Mon Sep 17 00:00:00 2001 From: Zeyad Gouda Date: Mon, 17 Feb 2025 11:41:04 +0200 Subject: [PATCH 9/9] fixup! secboot: address review comments Signed-off-by: Zeyad Gouda --- cmd/snap-bootstrap/main.go | 2 +- cmd/snapd/main.go | 2 +- secboot/argon2_out_of_process.go | 36 ++++++++++++++++++++++++ secboot/argon2_out_of_process_sb_test.go | 6 ++-- secboot/secboot.go | 15 ---------- 5 files changed, 41 insertions(+), 20 deletions(-) create mode 100644 secboot/argon2_out_of_process.go diff --git a/cmd/snap-bootstrap/main.go b/cmd/snap-bootstrap/main.go index a9fce8a8f58..42a7b0407bc 100644 --- a/cmd/snap-bootstrap/main.go +++ b/cmd/snap-bootstrap/main.go @@ -41,7 +41,7 @@ such as initramfs. ) func main() { - secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"--argon2-proc"}) + secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"argon2-proc"}) err := run(os.Args[1:]) if err != nil { diff --git a/cmd/snapd/main.go b/cmd/snapd/main.go index 5892069e702..693d7b7ddad 100644 --- a/cmd/snapd/main.go +++ b/cmd/snapd/main.go @@ -58,7 +58,7 @@ func main() { snapdtool.ExecInSnapdOrCoreSnap() } - secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"--argon2-proc"}) + secboot.HijackAndRunArgon2OutOfProcessHandlerOnArg([]string{"argon2-proc"}) if err := snapdtool.MaybeSetupFIPS(); err != nil { fmt.Fprintf(os.Stderr, "cannot check or enable FIPS mode: %v", err) diff --git a/secboot/argon2_out_of_process.go b/secboot/argon2_out_of_process.go new file mode 100644 index 00000000000..7dbc1e254ae --- /dev/null +++ b/secboot/argon2_out_of_process.go @@ -0,0 +1,36 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2025 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package secboot + +import "os" + +func isOutOfProcessArgon2KDFMode(args []string) bool { + if len(os.Args) != len(args)+1 { + return false + } + + for i := 0; i < len(args); i++ { + if os.Args[i+1] != args[i] { + return false + } + } + + return true +} diff --git a/secboot/argon2_out_of_process_sb_test.go b/secboot/argon2_out_of_process_sb_test.go index 0ffd29c0c1d..d1db0884d90 100644 --- a/secboot/argon2_out_of_process_sb_test.go +++ b/secboot/argon2_out_of_process_sb_test.go @@ -37,7 +37,7 @@ type argon2Suite struct { var _ = Suite(&argon2Suite{}) -func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2Mode(c *C) { +func (*argon2Suite) TestHijackAndRunArgon2OutOfProcessHandlerOnArgArgon2Mode(c *C) { runArgon2Called := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { runArgon2Called++ @@ -72,7 +72,7 @@ func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2Mode(c *C) c.Check(exitCalled, Equals, 1) } -func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2ModeError(c *C) { +func (*argon2Suite) TestHijackAndRunArgon2OutOfProcessHandlerOnArgArgon2ModeError(c *C) { runArgon2Called := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { runArgon2Called++ @@ -116,7 +116,7 @@ func (*mockArgon2KDF) Time(mode sb.Argon2Mode, params *sb.Argon2CostParams) (tim return 0, nil } -func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerNormalMode(c *C) { +func (*argon2Suite) TestHijackAndRunArgon2OutOfProcessHandlerOnArgNormalMode(c *C) { runArgon2Called := 0 restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) { runArgon2Called++ diff --git a/secboot/secboot.go b/secboot/secboot.go index e08ed0f1d4e..9b251842fd8 100644 --- a/secboot/secboot.go +++ b/secboot/secboot.go @@ -26,7 +26,6 @@ package secboot import ( "errors" - "os" "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/bootloader" @@ -269,17 +268,3 @@ func MarkSuccessful() error { return nil } - -func isOutOfProcessArgon2KDFMode(args []string) bool { - if len(os.Args) != len(args)+1 { - return false - } - - for i := 0; i < len(args); i++ { - if os.Args[i+1] != args[i] { - return false - } - } - - return true -}