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
6 changes: 6 additions & 0 deletions cmd/snap-bootstrap/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/jessevdk/go-flags"

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

var (
Expand All @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

it's a bit unclear to me why we don't want to deal with this inside the helper directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated!

os.Exit(1)
}

err := run(os.Args[1:])
if err != nil {
fmt.Fprintf(os.Stderr, "error: %s\n", err)
Expand Down
6 changes: 6 additions & 0 deletions cmd/snapd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

os.Exit(1)
}

if err := snapdtool.MaybeSetupFIPS(); err != nil {
fmt.Fprintf(os.Stderr, "cannot check or enable FIPS mode: %v", err)
os.Exit(1)
Expand Down
25 changes: 25 additions & 0 deletions secboot/argon2_out_of_process_dummy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// -*- 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() error {
return errBuildWithoutSecboot
}
91 changes: 91 additions & 0 deletions secboot/argon2_out_of_process_sb.go
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.
*
*/

package secboot

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

sb "github.com/snapcore/secboot"

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

func init() {
setArgon2KDF()
Copy link
Contributor

Choose a reason for hiding this comment

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

this will now affect every program in which one of the packages imports secboot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

switched to setting the kdf implementation from MaybeRunArgon2OutOfProcessRequestHandler itself as described here #15073 (comment)

}

const outOfProcessArgon2KDFTimeout = 100 * time.Millisecond
const argon2Cmd = "run-argon2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

my preference is something like an option "--argon2-proc"

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 don't have strong opinions on this, I went with a sub-command to be ready in case we want to pass args in the future to customize how the out-of-process helper is configured, but since this will be internal and be self invoking we can easily switch later if need arise and not worry about the binaries being in sync.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

switched to --argon2-proc


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")
}
127 changes: 127 additions & 0 deletions secboot/argon2_out_of_process_sb_test.go
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.
*
*/

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

import (
"io"
"os"

"github.com/canonical/go-tpm2"
sb "github.com/snapcore/secboot"
Expand Down Expand Up @@ -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)
}
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
9 changes: 1 addition & 8 deletions secboot/secboot_tpm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading