go.mod: experimental integration of gomodjail (library sandbox) #4012

AkihiroSuda · 2025-03-17T17:25:18Z

https://github.com/AkihiroSuda/gomodjail

gomodjail imposes syscall restrictions on a specific set of Go modules (excepts ones that use unsafe pointers, reflections, etc.), so as to mitigate their potential vulnerabilities and supply chain attack vectors.

Usage:

gomodjail run --go-mod=./go.mod -- nerdctl run hello-world

or

gomodjail pack --go-mod=./go.mod /usr/local/bin/nerdctl
./nerdctl.gomodjail run hello-world

Hint: use git diff --word-diff for reviewing the changes in this commit

.github/workflows/test.yml

fahedouch · 2025-03-18T09:15:07Z

@AkihiroSuda in the gomodjail project is there a plan to restrict specific system calls (e.g declarative configuration) for confined modules?

AkihiroSuda · 2025-03-18T09:19:49Z

@AkihiroSuda in the gomodjail project is there a plan to restrict specific system calls (e.g declarative configuration) for confined modules?

Eventually we may have fine-grained policies, but practically just having "confined" and "unconfined" might be enough to avoid complicating it

Dockerfile

AkihiroSuda · 2025-05-01T07:37:01Z

The more I think about this, the more I believe any temp location is a bad idea - they will get gc-ed eventually (on logout, on reboot) and this will break.

Sure, fixed in

unpack: cache: avoid using XDG_RUNTIME_DIR AkihiroSuda/gomodjail#49

https://github.com/AkihiroSuda/gomodjail gomodjail imposes syscall restrictions on a specific set of Go modules (excepts ones that use unsafe pointers, reflections, etc.), so as to mitigate their potential vulnerabilities and supply chain attack vectors. Usage: ``` gomodjail run --go-mod=./go.mod -- nerdctl run hello-world ``` or ``` gomodjail pack --go-mod=./go.mod /usr/local/bin/nerdctl ./nerdctl.gomodjail run hello-world ``` Hint: use `git diff --word-diff` for reviewing the changes in this commit Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

apostasie · 2025-05-01T09:18:35Z

The more I think about this, the more I believe any temp location is a bad idea - they will get gc-ed eventually (on logout, on reboot) and this will break.

Sure, fixed in

unpack: cache: avoid using XDG_RUNTIME_DIR AkihiroSuda/gomodjail#49

Also thinking about this: is there a reason we are using (in nerdctl) os.Executable - instead of os.Args[0] + resolve path - for the binary path?

If there is no serious reason, then we may want to use os.Args[0] indeed - that way gomodjail could just spoof the first arg - from nerdctl perspective this would be transparent, and the logger would point to the gomodjail binary without relying on any non-portable inspection of the parent or any other change in nerdctl itself.

AkihiroSuda · 2025-05-01T09:31:12Z

Also thinking about this: is there a reason we are using (in nerdctl) os.Executable - instead of os.Args[0] + resolve path - for the binary path?

os.Executable is more deterministic.

Anyway, gomodjail is not specifically made for nerdctl, and should support any application that depends on os.Executable

apostasie · 2025-05-01T16:41:28Z

Also thinking about this: is there a reason we are using (in nerdctl) os.Executable - instead of os.Args[0] + resolve path - for the binary path?

os.Executable is more deterministic.

Anyway, gomodjail is not specifically made for nerdctl, and should support any application that depends on os.Executable

To clarify: the suggestion was not to change anything for os.Executable neither to "not support it", but to enhance things for os.Args.

Anyhow, cool to have this.

apostasie · 2025-05-01T17:09:30Z

That got merged fast...

I wish we just had a few hours to review after it moved out of draft?
Will leave some comments so that hopefully things can be addressed as a follow-up.

apostasie · 2025-05-01T17:11:12Z

cmd/nerdctl/container/container_restart_linux_test.go

@@ -129,6 +130,8 @@ func TestRestartWithTime(t *testing.T) {
 func TestRestartWithSignal(t *testing.T) {
 	testCase := nerdtest.Setup()

+	testCase.Require = require.Not(nerdtest.Gomodjail) // FIXME


Are there details on this, and other similar disabled tests?

The test now seems to pass on my laptop (aarch64).

Probably it was failing due to a flakiness or something specific to CI.

Yeah, felt like leftover.

apostasie · 2025-05-01T17:16:51Z

pkg/testutil/testutil.go

@@ -721,13 +721,21 @@ func newBase(t *testing.T, ns string, ipv6Compatible bool, kubernetesCompatible
 	var err error
 	switch base.Target {
 	case Nerdctl:
-		base.Binary, err = exec.LookPath("nerdctl")


Curious if there was something preventing us to use the existing mechanism (-test.target=nerdctl.gomodjail) instead of adding this extra layer?

Thanks, saw you opened a PR

[FIX]: #4012 follow-ups (fixes and target test binaries) #4180

You are welcome ;-).

apostasie · 2025-05-01T17:20:39Z

mod/tigron/test/interfaces.go

@@ -101,6 +101,8 @@ type Helpers interface {
 // a Setup or Cleanup routine, and as the basis of any type of helper.
 // For more powerful use-cases outside of test cases, see below CustomizableCommand.
 type TestableCommand interface {
+	// Binary returns what binary to execute.
+	Binary() string


I don't think we should have this, at this point.
Unless I am missing something, the existing mechanism (--test.target) was meant to allow customization of the tested binary for use cases like this, without having to resort to augmentation of the TestableCommand interface.

Plus getTarget would have worked just fine instead of introducing this.

apostasie · 2025-05-01T18:50:11Z

More generally, this also ignore the fact that tests do hardcode "nerdctl" when they want to bypass the default command wrapping with namespace=nerdctl-test.
This is the case specifically for completion tests.
So, currently, some tests are just not testing nerdctl.gomodjail.

These should have been modified too to accommodate for testing a differently named "nerdish-ctl" binary.

This ^ in itself is desirable (and needed) for people who want to roll their own cli, but should be done right.

apostasie · 2025-05-01T20:15:21Z

pkg/testutil/testutil.go

+		if env := os.Getenv("DOCKER"); env != "" {
+			docker = env
+		}
+		base.Binary, err = exec.LookPath(docker)
 		if err != nil {
 			t.Fatal(err)
 		}


The same modifications as in nerdtest/command should have been applied here (eg: exec.Command(base.Binary))

Signed-off-by: apostasie <spam_blackhole@farcloser.world>

[FIX]: #4012 follow-ups (fixes and target test binaries)

AkihiroSuda force-pushed the gomodjail branch 2 times, most recently from 3ec186f to 295ee0f Compare March 17, 2025 18:03

AkihiroSuda added this to the v2.x.x milestone Mar 18, 2025

AkihiroSuda force-pushed the gomodjail branch 11 times, most recently from 98c3108 to 8587b52 Compare March 18, 2025 08:27

AkihiroSuda commented Mar 18, 2025

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

AkihiroSuda force-pushed the gomodjail branch from 8587b52 to a2394ef Compare March 18, 2025 09:06

AkihiroSuda force-pushed the gomodjail branch from a2394ef to d7ac06a Compare March 18, 2025 09:46

AkihiroSuda commented Mar 18, 2025

View reviewed changes

Dockerfile Show resolved Hide resolved

AkihiroSuda mentioned this pull request Mar 26, 2025

pack: relax file permissions AkihiroSuda/gomodjail#36

Merged

AkihiroSuda force-pushed the gomodjail branch 2 times, most recently from 347b5c5 to 4d6326d Compare March 26, 2025 10:25

AkihiroSuda commented Mar 26, 2025

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

AkihiroSuda mentioned this pull request Mar 26, 2025

Dockerfile: consistently use Alpine or Debian for building the binaries #4037

Closed

AkihiroSuda force-pushed the gomodjail branch from 4d6326d to 9322435 Compare April 15, 2025 12:47

This comment was marked as resolved.

Sign in to view

AkihiroSuda mentioned this pull request Apr 15, 2025

Support specifying multiple policies (when a Go program executes another Go program) AkihiroSuda/gomodjail#39

Open

AkihiroSuda force-pushed the gomodjail branch 2 times, most recently from cba5a2e to c4cb682 Compare April 22, 2025 08:45

AkihiroSuda mentioned this pull request May 1, 2025

unpack: keep cache AkihiroSuda/gomodjail#48

Merged

AkihiroSuda force-pushed the gomodjail branch 2 times, most recently from cbaa9da to d849498 Compare May 1, 2025 07:13

AkihiroSuda marked this pull request as ready for review May 1, 2025 07:27

AkihiroSuda force-pushed the gomodjail branch from d849498 to f15c755 Compare May 1, 2025 07:30

AkihiroSuda requested a review from ktock May 1, 2025 07:38

AkihiroSuda force-pushed the gomodjail branch from f15c755 to ac3940f Compare May 1, 2025 07:47

AkihiroSuda force-pushed the gomodjail branch from ac3940f to 81d830b Compare May 1, 2025 09:03

ktock approved these changes May 1, 2025

View reviewed changes

ktock merged commit 08b4cfb into containerd:main May 1, 2025
80 of 82 checks passed

apostasie reviewed May 1, 2025

View reviewed changes

apostasie added a commit to apostasie/nerdctl that referenced this pull request May 1, 2025

Follow-up to containerd#4012: address tests issues

0f91766

Signed-off-by: apostasie <spam_blackhole@farcloser.world>

apostasie added a commit to apostasie/nerdctl that referenced this pull request May 1, 2025

Follow-up to containerd#4012: address tests issues

4a59607

Signed-off-by: apostasie <spam_blackhole@farcloser.world>

apostasie added a commit to apostasie/nerdctl that referenced this pull request May 2, 2025

Follow-up to containerd#4012: address tests issues

b9e19f7

Signed-off-by: apostasie <spam_blackhole@farcloser.world>

apostasie added a commit to apostasie/nerdctl that referenced this pull request May 2, 2025

Follow-up to containerd#4012: address tests issues

4a83d6d

Signed-off-by: apostasie <spam_blackhole@farcloser.world>

apostasie added a commit to apostasie/nerdctl that referenced this pull request May 2, 2025

Follow-up to containerd#4012: address tests issues

f15db06

Signed-off-by: apostasie <spam_blackhole@farcloser.world>

apostasie mentioned this pull request May 2, 2025

[FIX]: #4012 follow-ups (fixes and target test binaries) #4180

Merged

apostasie added a commit to apostasie/nerdctl that referenced this pull request May 2, 2025

Follow-up to containerd#4012: address tests issues

836ab28

Signed-off-by: apostasie <spam_blackhole@farcloser.world>

apostasie added a commit to apostasie/nerdctl that referenced this pull request May 2, 2025

Follow-up to containerd#4012: address tests issues

4dd9ad9

Signed-off-by: apostasie <spam_blackhole@farcloser.world>

AkihiroSuda added a commit that referenced this pull request May 3, 2025

Merge pull request #4180 from apostasie/fix-gomodjail-integration

944dbe6

[FIX]: #4012 follow-ups (fixes and target test binaries)

go.mod: experimental integration of gomodjail (library sandbox) #4012

go.mod: experimental integration of gomodjail (library sandbox) #4012

Uh oh!

Conversation

AkihiroSuda commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fahedouch commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AkihiroSuda commented Mar 18, 2025

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

AkihiroSuda commented May 1, 2025

Uh oh!

apostasie commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AkihiroSuda commented May 1, 2025

Uh oh!

Uh oh!

apostasie commented May 1, 2025

Uh oh!

apostasie commented May 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apostasie commented May 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AkihiroSuda commented Mar 17, 2025 •

edited

Loading

fahedouch commented Mar 18, 2025 •

edited

Loading

apostasie commented May 1, 2025 •

edited

Loading