Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd, secboot: add argon2 out-of-process kdf support #15073

Merged
merged 9 commits into from
Feb 18, 2025
3 changes: 3 additions & 0 deletions cmd/snap-bootstrap/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"github.com/jessevdk/go-flags"

"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/secboot"
)

var (
Expand All @@ -40,6 +41,8 @@
)

func main() {
secboot.MaybeRunArgon2OutOfProcessRequestHandler()

Check warning on line 45 in cmd/snap-bootstrap/main.go

View check run for this annotation

Codecov / codecov/patch

cmd/snap-bootstrap/main.go#L44-L45

Added lines #L44 - L45 were not covered by tests
err := run(os.Args[1:])
if err != nil {
fmt.Fprintf(os.Stderr, "error: %s\n", err)
Expand Down
3 changes: 3 additions & 0 deletions cmd/snapd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"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"
Expand All @@ -57,6 +58,8 @@
snapdtool.ExecInSnapdOrCoreSnap()
}

secboot.MaybeRunArgon2OutOfProcessRequestHandler()

Check warning on line 62 in cmd/snapd/main.go

View check run for this annotation

Codecov / codecov/patch

cmd/snapd/main.go#L61-L62

Added lines #L61 - L62 were not covered by tests
if err := snapdtool.MaybeSetupFIPS(); err != nil {
fmt.Fprintf(os.Stderr, "cannot check or enable FIPS mode: %v", err)
os.Exit(1)
Expand Down
23 changes: 23 additions & 0 deletions secboot/argon2_out_of_process_dummy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

Check failure on line 1 in secboot/argon2_out_of_process_dummy.go

View workflow job for this annotation

GitHub Actions / Inclusive-naming-check

[woke] reported by reviewdog 🐶 [error] Filename finding: `argon2_out_of_process_dummy` may be insensitive, use `placeholder`, `sample` instead Raw Output: secboot/argon2_out_of_process_dummy.go:1:1: [error] Filename finding: `argon2_out_of_process_dummy` may be insensitive, use `placeholder`, `sample` instead
//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 <http://www.gnu.org/licenses/>.
*
*/

package secboot

func MaybeRunArgon2OutOfProcessRequestHandler() {}

Check warning on line 23 in secboot/argon2_out_of_process_dummy.go

View check run for this annotation

Codecov / codecov/patch

secboot/argon2_out_of_process_dummy.go#L23

Added line #L23 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should raise an error/panic if it observes a trigger for out of process secboot runner

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func MaybeRunArgon2OutOfProcessRequestHandler() {}
func MaybeRunArgon2OutOfProcessRequestHandler() {
if !isOutOfProcessArgon2KDFMode() {
panic("internal error: unexpected call to execute as argon2 runner in non-secboot binary")
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should panic when isOutOfProcessArgon2KDFMode is true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, my bad, I copied directly without changing it

97 changes: 97 additions & 0 deletions secboot/argon2_out_of_process_sb.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// -*- 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 <http://www.gnu.org/licenses/>.
*
*/

package secboot

import (
"fmt"
"os"
"os/exec"
"time"

sb "github.com/snapcore/secboot"

"github.com/snapcore/snapd/logger"
)

var (
osExit = os.Exit
osReadlink = os.Readlink

sbWaitForAndRunArgon2OutOfProcessRequest = sb.WaitForAndRunArgon2OutOfProcessRequest
sbNewOutOfProcessArgon2KDF = sb.NewOutOfProcessArgon2KDF
sbSetArgon2KDF = sb.SetArgon2KDF
)

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).
//
// 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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we change the API a little such that it can be used like so, in snapd:

// snapd/main.go

func executeSecbootArgon2Runner {
	logger.Noticef("running argon2 out-of-process helper")
	// Ignore the lock release callback and use implicit release on process termination.
	_, err := secboot.ExecuteAsArgon2Runner(secboot.Argon2RunnerOptions{
		Watchdog: false,
	})
	if err != nil {
		logger.Errorf("cannot execute runner: %v", err)
		os.Exit(1)
	}
	os.Exit(0)
}

func main() {
	...
	InstallSecbootHandlerOnArg([]string{"--secboot-argon2-runner"})
	
	if len(os.Args) == 2 && os.Args[1] == "--secboot-argon2-runner" {
		executeArgon2Runner()
		// not-reached
	}
	...
}

and snap-bootstrap:

// snap-boostrap/main.go

func main() {
	...
	InstallSecbootHandlerOnArg([]string{"secboot-argon2-runner"})
}

// snap-boostrap/cmd_secboot_argon2_runner.go

func init() {
	addCommandBuilder(func(parser *flags.Parser) {
		cmd, err := parser.AddCommand("secboot2_argon2-runner", short, long, &cmdArgon2Runner{})
		if  err != nil {
			panic(err)
		}
		// make it hidden
		cmd.Hidden = true
	})
}

type cmdArgon2Runner struct {}

func (c *cmdArgon2Runner) Execute([]string) error {
	logger.Noticef("running argon2 out-of-process helper")
	// Ignore the lock release callback and use implicit release on process termination.
	_, err := secboot.ExecuteAsArgon2Runner(secboot.Argon2RunnerOptions{
		Watchdog: false,
	})
	return err
}

Copy link
Collaborator

@pedronis pedronis Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a lot of extra code that I'm not keen on. Maybe the compromise is to pass the marker flag to the MaybeRunArgon2OutOfProcessRequestHandler

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
}

Check warning on line 73 in secboot/argon2_out_of_process_sb.go

View check run for this annotation

Codecov / codecov/patch

secboot/argon2_out_of_process_sb.go#L71-L73

Added lines #L71 - L73 were not covered by tests

handlerCmd := func() (*exec.Cmd, error) {
cmd := exec.Command(exe, outOfProcessArgon2Arg)
return cmd, nil
}
argon2KDF := sbNewOutOfProcessArgon2KDF(handlerCmd, outOfProcessArgon2KDFTimeout, nil)
sbSetArgon2KDF(argon2KDF)

return
}

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, sb.NoArgon2OutOfProcessWatchdogHandler())
if err != nil {
fmt.Fprintf(os.Stderr, "cannot run argon2 out-of-process request: %v", err)
osExit(1)
}

// Argon2 request was processed successfully
osExit(0)

panic("internal error: not reachable")
}
178 changes: 178 additions & 0 deletions secboot/argon2_out_of_process_sb_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
// -*- 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 <http://www.gnu.org/licenses/>.
*
*/

package secboot_test

import (
"errors"
"io"
"os/exec"
"time"

sb "github.com/snapcore/secboot"
. "gopkg.in/check.v1"

"github.com/snapcore/snapd/secboot"
)

type argon2Suite struct {
}

var _ = Suite(&argon2Suite{})

func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2Mode(c *C) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests might need to be renamed now

runArgon2Called := 0
restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) {
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++
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 (injected above)
c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(0)")

c.Check(setArgon2Called, Equals, 0)
c.Check(runArgon2Called, Equals, 1)
c.Check(exitCalled, Equals, 1)
}

func (*argon2Suite) TestMaybeRunArgon2OutOfProcessRequestHandlerArgon2ModeError(c *C) {
runArgon2Called := 0
restore := secboot.MockSbWaitForAndRunArgon2OutOfProcessRequest(func(_ io.Reader, _ io.WriteCloser, _ sb.Argon2OutOfProcessWatchdogHandler) (lockRelease func(), err error) {
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, 1)
panic("os.Exit(1)")
})
defer restore()

restore = secboot.MockOsArgs([]string{"/path/to/cmd", "--argon2-proc"})
defer restore()

c.Assert(secboot.MaybeRunArgon2OutOfProcessRequestHandler, Panics, "os.Exit(1)")

c.Check(setArgon2Called, Equals, 0)
c.Check(runArgon2Called, Equals, 1)
c.Check(exitCalled, Equals, 1)
}

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) {
runArgon2Called++
return nil, nil
})
defer restore()

exitCalled := 0
restore = secboot.MockOsExit(func(code int) {
exitCalled++
c.Assert(code, Equals, 0)
panic("injected panic")
})
defer restore()

restore = secboot.MockOsReadlink(func(name string) (string, error) {
c.Assert(name, Equals, "/proc/self/exe")
return "/path/to/cmd", nil
})
defer restore()

argon2KDF := &mockArgon2KDF{}

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)
}
27 changes: 27 additions & 0 deletions secboot/export_sb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ package secboot

import (
"io"
"os"
"os/exec"
"time"

"github.com/canonical/go-tpm2"
sb "github.com/snapcore/secboot"
Expand Down Expand Up @@ -449,3 +452,27 @@ 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 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)
}
2 changes: 1 addition & 1 deletion secboot/secboot_sb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading