From a41ee2d6a404f89de5a5ef7e33def9a5e25be5d4 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 8 Feb 2023 07:28:50 -0500 Subject: [PATCH 01/54] Move to v1.11.1-dev Signed-off-by: Daniel J Walsh --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 1cac385..095b667 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.11.0 +1.11.1-dev From ea3905d9b24818288457e81598103f7f46601d27 Mon Sep 17 00:00:00 2001 From: ningmingxiao Date: Thu, 27 Jul 2023 23:28:40 +0800 Subject: [PATCH 02/54] fix some obvious error Signed-off-by: ningmingxiao --- go-selinux/selinux_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index f1e9597..0638783 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -1134,7 +1134,7 @@ func rchcon(fpath, label string) error { //revive:disable:cognitive-complexity } return pwalkdir.Walk(fpath, func(p string, _ fs.DirEntry, _ error) error { if fastMode { - if cLabel, err := lFileLabel(fpath); err == nil && cLabel == label { + if cLabel, err := lFileLabel(p); err == nil && cLabel == label { return nil } } From 7c6e67dfa49ab1ef4f804b660a9b8479e474bbac Mon Sep 17 00:00:00 2001 From: Michal Biesek Date: Mon, 14 Aug 2023 02:27:06 +0200 Subject: [PATCH 03/54] ci: update Go 1.21 support Signed-off-by: Michal Biesek --- .github/workflows/validate.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 7331776..41fa9d0 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -33,7 +33,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-go@v3 with: - go-version: 1.20.x + go-version: 1.21.x - uses: golangci/golangci-lint-action@v3 with: version: v1.51 @@ -51,7 +51,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-go@v3 with: - go-version: 1.20.x + go-version: 1.21.x - uses: golangci/golangci-lint-action@v3 with: version: v1.51 @@ -62,7 +62,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: [1.19.x, 1.20.x] + go-version: [1.19.x, 1.20.x, 1.21.x] race: ["-race", ""] runs-on: ubuntu-20.04 steps: From d47b9f731ebff5a8342789d426e664991d169a5c Mon Sep 17 00:00:00 2001 From: Michal Biesek Date: Mon, 14 Aug 2023 17:30:27 +0200 Subject: [PATCH 04/54] ci: update `golangci-lint` to v1.54 Ref: https://github.com/golangci/golangci-lint/releases/tag/v1.54.1 Signed-off-by: Michal Biesek --- .github/workflows/validate.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 41fa9d0..be86e65 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -36,7 +36,7 @@ jobs: go-version: 1.21.x - uses: golangci/golangci-lint-action@v3 with: - version: v1.51 + version: v1.54 cross: runs-on: ubuntu-20.04 @@ -54,7 +54,7 @@ jobs: go-version: 1.21.x - uses: golangci/golangci-lint-action@v3 with: - version: v1.51 + version: v1.54 - name: test-stubs run: make test From 42334c102f068a15aadee02e25d025a2e68a4101 Mon Sep 17 00:00:00 2001 From: Michal Biesek Date: Mon, 14 Aug 2023 17:39:09 +0200 Subject: [PATCH 05/54] go-selinux: fix `if-return` in Relabel Ref: https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#if-return Signed-off-by: Michal Biesek --- go-selinux/label/label_linux.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/go-selinux/label/label_linux.go b/go-selinux/label/label_linux.go index f61a560..e49e6d5 100644 --- a/go-selinux/label/label_linux.go +++ b/go-selinux/label/label_linux.go @@ -120,10 +120,7 @@ func Relabel(path string, fileLabel string, shared bool) error { c["level"] = "s0" fileLabel = c.Get() } - if err := selinux.Chcon(path, fileLabel, true); err != nil { - return err - } - return nil + return selinux.Chcon(path, fileLabel, true) } // DisableSecOpt returns a security opt that can disable labeling From cf0b5a9ee0221df9e9c46e719f433f0a69136873 Mon Sep 17 00:00:00 2001 From: michalbiesek Date: Mon, 14 Aug 2023 18:51:18 +0200 Subject: [PATCH 06/54] go-selinux: fix unused-parameter lint warning Ref: https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter Signed-off-by: Michal Biesek --- go-selinux/label/label_stub.go | 16 ++++++------ go-selinux/selinux_stub.go | 46 +++++++++++++++++----------------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/go-selinux/label/label_stub.go b/go-selinux/label/label_stub.go index f21c80c..1c260cb 100644 --- a/go-selinux/label/label_stub.go +++ b/go-selinux/label/label_stub.go @@ -6,25 +6,25 @@ package label // InitLabels returns the process label and file labels to be used within // the container. A list of options can be passed into this function to alter // the labels. -func InitLabels(options []string) (string, string, error) { +func InitLabels([]string) (string, string, error) { return "", "", nil } // Deprecated: The GenLabels function is only to be used during the transition // to the official API. Use InitLabels(strings.Fields(options)) instead. -func GenLabels(options string) (string, string, error) { +func GenLabels(string) (string, string, error) { return "", "", nil } -func SetFileLabel(path string, fileLabel string) error { +func SetFileLabel(string, string) error { return nil } -func SetFileCreateLabel(fileLabel string) error { +func SetFileCreateLabel(string) error { return nil } -func Relabel(path string, fileLabel string, shared bool) error { +func Relabel(string, string, bool) error { return nil } @@ -35,16 +35,16 @@ func DisableSecOpt() []string { } // Validate checks that the label does not include unexpected options -func Validate(label string) error { +func Validate(string) error { return nil } // RelabelNeeded checks whether the user requested a relabel -func RelabelNeeded(label string) bool { +func RelabelNeeded(string) bool { return false } // IsShared checks that the label includes a "shared" mark -func IsShared(label string) bool { +func IsShared(string) bool { return false } diff --git a/go-selinux/selinux_stub.go b/go-selinux/selinux_stub.go index bc3fd3b..0889fbe 100644 --- a/go-selinux/selinux_stub.go +++ b/go-selinux/selinux_stub.go @@ -7,7 +7,7 @@ func attrPath(string) string { return "" } -func readCon(fpath string) (string, error) { +func readCon(string) (string, error) { return "", nil } @@ -21,27 +21,27 @@ func getEnabled() bool { return false } -func classIndex(class string) (int, error) { +func classIndex(string) (int, error) { return -1, nil } -func setFileLabel(fpath string, label string) error { +func setFileLabel(string, string) error { return nil } -func lSetFileLabel(fpath string, label string) error { +func lSetFileLabel(string, string) error { return nil } -func fileLabel(fpath string) (string, error) { +func fileLabel(string) (string, error) { return "", nil } -func lFileLabel(fpath string) (string, error) { +func lFileLabel(string) (string, error) { return "", nil } -func setFSCreateLabel(label string) error { +func setFSCreateLabel(string) error { return nil } @@ -53,7 +53,7 @@ func currentLabel() (string, error) { return "", nil } -func pidLabel(pid int) (string, error) { +func pidLabel(int) (string, error) { return "", nil } @@ -61,23 +61,23 @@ func execLabel() (string, error) { return "", nil } -func canonicalizeContext(val string) (string, error) { +func canonicalizeContext(string) (string, error) { return "", nil } -func computeCreateContext(source string, target string, class string) (string, error) { +func computeCreateContext(string, string, string) (string, error) { return "", nil } -func calculateGlbLub(sourceRange, targetRange string) (string, error) { +func calculateGlbLub(string, string) (string, error) { return "", nil } -func peerLabel(fd uintptr) (string, error) { +func peerLabel(uintptr) (string, error) { return "", nil } -func setKeyLabel(label string) error { +func setKeyLabel(string) error { return nil } @@ -85,14 +85,14 @@ func (c Context) get() string { return "" } -func newContext(label string) (Context, error) { +func newContext(string) (Context, error) { return Context{}, nil } func clearLabels() { } -func reserveLabel(label string) { +func reserveLabel(string) { } func isMLSEnabled() bool { @@ -103,7 +103,7 @@ func enforceMode() int { return Disabled } -func setEnforceMode(mode int) error { +func setEnforceMode(int) error { return nil } @@ -111,7 +111,7 @@ func defaultEnforceMode() int { return Disabled } -func releaseLabel(label string) { +func releaseLabel(string) { } func roFileLabel() string { @@ -126,27 +126,27 @@ func initContainerLabels() (string, string) { return "", "" } -func containerLabels() (processLabel string, fileLabel string) { +func containerLabels() (string, string) { return "", "" } -func securityCheckContext(val string) error { +func securityCheckContext(string) error { return nil } -func copyLevel(src, dest string) (string, error) { +func copyLevel(string, string) (string, error) { return "", nil } -func chcon(fpath string, label string, recurse bool) error { +func chcon(string, string, bool) error { return nil } -func dupSecOpt(src string) ([]string, error) { +func dupSecOpt(string) ([]string, error) { return nil, nil } -func getDefaultContextWithLevel(user, level, scon string) (string, error) { +func getDefaultContextWithLevel(string, string, string) (string, error) { return "", nil } From e65925b226d26477aad68cbf1a844ec6c2a89099 Mon Sep 17 00:00:00 2001 From: Michal Biesek Date: Mon, 14 Aug 2023 02:22:07 +0200 Subject: [PATCH 07/54] Extend `build-cross` target with `riscv64` arch Signed-off-by: Michal Biesek --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index c39d6e0..23c3d8b 100644 --- a/Makefile +++ b/Makefile @@ -18,6 +18,7 @@ build-cross: $(call go-build,linux,ppc64le) $(call go-build,linux,s390x) $(call go-build,linux,mips64le) + $(call go-build,linux,riscv64) $(call go-build,windows,amd64) $(call go-build,windows,386) From 5ab22c5b42c3efbadc6dda2adf8bbe1ffb4c84f4 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 24 Aug 2023 19:06:14 -0700 Subject: [PATCH 08/54] Remove nolint annotations for unix errno comparisons golangci-lint v1.54.2 comes with errorlint v1.4.4, which contains the fix [1] whitelisting all errno comparisons for errors coming from x/sys/unix. Thus, these annotations are no longer necessary. Remove those. Except, the errors that do not come directly from a function in unix package still need to be annotated (see [2]). [1] https://github.com/polyfloyd/go-errorlint/pull/47 [2] https://github.com/polyfloyd/go-errorlint/issues/55 Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 8 ++++---- go-selinux/xattrs_linux.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 0638783..c1f19c4 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -132,7 +132,7 @@ func verifySELinuxfsMount(mnt string) bool { if err == nil { break } - if err == unix.EAGAIN || err == unix.EINTR { //nolint:errorlint // unix errors are bare + if err == unix.EAGAIN || err == unix.EINTR { continue } return false @@ -263,7 +263,7 @@ func isProcHandle(fh *os.File) error { if err == nil { break } - if err != unix.EINTR { //nolint:errorlint // unix errors are bare + if err != unix.EINTR { return &os.PathError{Op: "fstatfs", Path: fh.Name(), Err: err} } } @@ -328,7 +328,7 @@ func lSetFileLabel(fpath string, label string) error { if err == nil { break } - if err != unix.EINTR { //nolint:errorlint // unix errors are bare + if err != unix.EINTR { return &os.PathError{Op: "lsetxattr", Path: fpath, Err: err} } } @@ -347,7 +347,7 @@ func setFileLabel(fpath string, label string) error { if err == nil { break } - if err != unix.EINTR { //nolint:errorlint // unix errors are bare + if err != unix.EINTR { return &os.PathError{Op: "setxattr", Path: fpath, Err: err} } } diff --git a/go-selinux/xattrs_linux.go b/go-selinux/xattrs_linux.go index 9e473ca..559c851 100644 --- a/go-selinux/xattrs_linux.go +++ b/go-selinux/xattrs_linux.go @@ -31,7 +31,7 @@ func lgetxattr(path, attr string) ([]byte, error) { func doLgetxattr(path, attr string, dest []byte) (int, error) { for { sz, err := unix.Lgetxattr(path, attr, dest) - if err != unix.EINTR { //nolint:errorlint // unix errors are bare + if err != unix.EINTR { return sz, err } } @@ -64,7 +64,7 @@ func getxattr(path, attr string) ([]byte, error) { func dogetxattr(path, attr string, dest []byte) (int, error) { for { sz, err := unix.Getxattr(path, attr, dest) - if err != unix.EINTR { //nolint:errorlint // unix errors are bare + if err != unix.EINTR { return sz, err } } From 2336a0397fd39f94fe5b00aad30571bf370de875 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 25 Aug 2023 18:23:02 -0700 Subject: [PATCH 09/54] ci: bump some actions 1. Bump actions/setup-go to v4 which enables caching by default. Disable cache for cases when golangci-lint is used. 2. Remove 'stable:' property from actions/setup-go (not needed since at least v3). 3. Bump tim-actions/get-pr-commits to v1.3.0 (fixes node compatibility). Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index be86e65..e701fc4 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -16,7 +16,7 @@ jobs: steps: - name: get pr commits id: 'get-pr-commits' - uses: tim-actions/get-pr-commits@v1.0.0 + uses: tim-actions/get-pr-commits@v1.3.0 with: token: ${{ secrets.GITHUB_TOKEN }} @@ -31,9 +31,10 @@ jobs: runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - uses: actions/setup-go@v4 with: go-version: 1.21.x + cache: false # golangci-lint-action does its own caching - uses: golangci/golangci-lint-action@v3 with: version: v1.54 @@ -49,9 +50,10 @@ jobs: runs-on: macos-latest steps: - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + - uses: actions/setup-go@v4 with: go-version: 1.21.x + cache: false # golangci-lint-action does its own caching - uses: golangci/golangci-lint-action@v3 with: version: v1.54 @@ -69,9 +71,8 @@ jobs: - uses: actions/checkout@v3 - name: install go ${{ matrix.go-version }} - uses: actions/setup-go@v3 + uses: actions/setup-go@v4 with: - stable: '!contains(${{ matrix.go-version }}, "beta") && !contains(${{ matrix.go-version }}, "rc")' go-version: ${{ matrix.go-version }} - name: build From d984be7c26b672b878340bd2305736897b11f958 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 25 Aug 2023 17:52:37 -0700 Subject: [PATCH 10/54] pwalk,pwalkdir: simplify test setup and cleanup Use t.Helper and t.Cleanup in prepareTestSet to simplify tests setup and cleanup (no need to call defer os.Remove or check error). Signed-off-by: Kir Kolyshkin --- pkg/pwalk/pwalk_test.go | 42 ++++++++++++++-------------------- pkg/pwalkdir/pwalkdir_test.go | 43 ++++++++++++++--------------------- 2 files changed, 34 insertions(+), 51 deletions(-) diff --git a/pkg/pwalk/pwalk_test.go b/pkg/pwalk/pwalk_test.go index bf50613..5dd910e 100644 --- a/pkg/pwalk/pwalk_test.go +++ b/pkg/pwalk/pwalk_test.go @@ -15,19 +15,14 @@ func TestWalk(t *testing.T) { var count uint32 concurrency := runtime.NumCPU() * 2 - dir, total, err := prepareTestSet(3, 2, 1) - if err != nil { - t.Fatalf("dataset creation failed: %v", err) - } - defer os.RemoveAll(dir) + dir, total := prepareTestSet(t, 3, 2, 1) - err = WalkN(dir, + err := WalkN(dir, func(_ string, _ os.FileInfo, _ error) error { atomic.AddUint32(&count, 1) return nil }, concurrency) - if err != nil { t.Errorf("Walk failed: %v", err) } @@ -41,15 +36,11 @@ func TestWalk(t *testing.T) { func TestWalkManyErrors(t *testing.T) { var count uint32 - dir, total, err := prepareTestSet(3, 3, 2) - if err != nil { - t.Fatalf("dataset creation failed: %v", err) - } - defer os.RemoveAll(dir) + dir, total := prepareTestSet(t, 3, 3, 2) max := uint32(total / 2) e42 := errors.New("42") - err = Walk(dir, + err := Walk(dir, func(p string, i os.FileInfo, _ error) error { if atomic.AddUint32(&count, 1) > max { return e42 @@ -101,17 +92,22 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count int, err error) // // Total dirs: dirs^levels + dirs^(levels-1) + ... + dirs^1 // Total files: total_dirs * files -func prepareTestSet(levels, dirs, files int) (dir string, total int, err error) { +func prepareTestSet(tb testing.TB, levels, dirs, files int) (dir string, total int) { + tb.Helper() + var err error + dir, err = os.MkdirTemp(".", "pwalk-test-") if err != nil { - return + tb.Fatal(err) } + tb.Cleanup(func() { + if err := os.RemoveAll(dir); err != nil && !errors.Is(err, os.ErrNotExist) { + tb.Errorf("cleanup error: %v", err) + } + }) total, err = makeManyDirs(dir, levels, dirs, files) - if err != nil && total > 0 { - _ = os.RemoveAll(dir) - dir = "" - total = 0 - return + if err != nil { + tb.Fatal(err) } total++ // this dir @@ -161,11 +157,7 @@ func BenchmarkWalk(b *testing.B) { {name: "pwalk.Walk256", walker: genWalkN(256)}, } - dir, total, err := prepareTestSet(levels, dirs, files) - if err != nil { - b.Fatalf("dataset creation failed: %v", err) - } - defer os.RemoveAll(dir) + dir, total := prepareTestSet(b, levels, dirs, files) b.Logf("dataset: %d levels x %d dirs x %d files, total entries: %d", levels, dirs, files, total) for _, bm := range benchmarks { diff --git a/pkg/pwalkdir/pwalkdir_test.go b/pkg/pwalkdir/pwalkdir_test.go index c173001..5fa59a0 100644 --- a/pkg/pwalkdir/pwalkdir_test.go +++ b/pkg/pwalkdir/pwalkdir_test.go @@ -18,20 +18,14 @@ import ( func TestWalkDir(t *testing.T) { var count uint32 concurrency := runtime.NumCPU() * 2 + dir, total := prepareTestSet(t, 3, 2, 1) - dir, total, err := prepareTestSet(3, 2, 1) - if err != nil { - t.Fatalf("dataset creation failed: %v", err) - } - defer os.RemoveAll(dir) - - err = WalkN(dir, + err := WalkN(dir, func(_ string, _ fs.DirEntry, _ error) error { atomic.AddUint32(&count, 1) return nil }, concurrency) - if err != nil { t.Errorf("Walk failed: %v", err) } @@ -45,15 +39,11 @@ func TestWalkDir(t *testing.T) { func TestWalkDirManyErrors(t *testing.T) { var count uint32 - dir, total, err := prepareTestSet(3, 3, 2) - if err != nil { - t.Fatalf("dataset creation failed: %v", err) - } - defer os.RemoveAll(dir) + dir, total := prepareTestSet(t, 3, 3, 2) max := uint32(total / 2) e42 := errors.New("42") - err = Walk(dir, + err := Walk(dir, func(p string, e fs.DirEntry, _ error) error { if atomic.AddUint32(&count, 1) > max { return e42 @@ -105,17 +95,22 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count int, err error) // // Total dirs: dirs^levels + dirs^(levels-1) + ... + dirs^1 // Total files: total_dirs * files -func prepareTestSet(levels, dirs, files int) (dir string, total int, err error) { +func prepareTestSet(tb testing.TB, levels, dirs, files int) (dir string, total int) { + tb.Helper() + var err error + dir, err = os.MkdirTemp(".", "pwalk-test-") if err != nil { - return + tb.Fatal(err) } + tb.Cleanup(func() { + if err := os.RemoveAll(dir); err != nil && !errors.Is(err, os.ErrNotExist) { + tb.Errorf("cleanup error: %v", err) + } + }) total, err = makeManyDirs(dir, levels, dirs, files) - if err != nil && total > 0 { - _ = os.RemoveAll(dir) - dir = "" - total = 0 - return + if err != nil { + tb.Fatal(err) } total++ // this dir @@ -165,11 +160,7 @@ func BenchmarkWalk(b *testing.B) { {name: "pwalkdir.Walk256", walker: genWalkN(256)}, } - dir, total, err := prepareTestSet(levels, dirs, files) - if err != nil { - b.Fatalf("dataset creation failed: %v", err) - } - defer os.RemoveAll(dir) + dir, total := prepareTestSet(b, levels, dirs, files) b.Logf("dataset: %d levels x %d dirs x %d files, total entries: %d", levels, dirs, files, total) for _, bm := range benchmarks { From 4c0350337e474c127c437e197e21b2cd2c8a72e0 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 25 Aug 2023 18:05:53 -0700 Subject: [PATCH 11/54] pwalk,pwalkdir: handle ErrNotExist internally In some cases, when file walk is racing with removal, the ENOENT is received by filepath.Walk[Dir], rather than by the callback. Do ignore such errors, except for the root directory (Walk argument). Modify the doc accordingly, and add a couple of test cases. The "Race" test case, when testing the code before the fix, fails 100% of times for pwalk and 90% for pwalkdir. Alas, I was unable to make it fail 100% of the times without significantly increasing the test complexity. Fixes: 199 Signed-off-by: Kir Kolyshkin --- pkg/pwalk/README.md | 4 ++++ pkg/pwalk/pwalk.go | 8 ++++++++ pkg/pwalk/pwalk_test.go | 36 +++++++++++++++++++++++++++++++++-- pkg/pwalkdir/README.md | 4 +++- pkg/pwalkdir/pwalkdir.go | 7 +++++++ pkg/pwalkdir/pwalkdir_test.go | 36 +++++++++++++++++++++++++++++++++-- 6 files changed, 90 insertions(+), 5 deletions(-) diff --git a/pkg/pwalk/README.md b/pkg/pwalk/README.md index 7e78dce..a060ad3 100644 --- a/pkg/pwalk/README.md +++ b/pkg/pwalk/README.md @@ -24,6 +24,10 @@ Please note the following limitations of this code: * filepath.SkipDir is not supported; + * ErrNotExist errors from filepath.Walk are silently ignored for any path + except the top directory (Walk argument); any other error is returned to + the caller of Walk; + * no errors are ever passed to WalkFunc; * once any error is returned from any WalkFunc instance, no more new calls diff --git a/pkg/pwalk/pwalk.go b/pkg/pwalk/pwalk.go index a28b4c4..686c8ba 100644 --- a/pkg/pwalk/pwalk.go +++ b/pkg/pwalk/pwalk.go @@ -1,6 +1,7 @@ package pwalk import ( + "errors" "fmt" "os" "path/filepath" @@ -67,6 +68,13 @@ func WalkN(root string, walkFn WalkFunc, num int) error { go func() { err = filepath.Walk(root, func(p string, info os.FileInfo, err error) error { if err != nil { + // Walking a file tree can race with removal, + // so ignore ENOENT, except for root. + // https://github.com/opencontainers/selinux/issues/199. + if errors.Is(err, os.ErrNotExist) && len(p) != rootLen { + return nil + } + close(files) return err } diff --git a/pkg/pwalk/pwalk_test.go b/pkg/pwalk/pwalk_test.go index 5dd910e..b026681 100644 --- a/pkg/pwalk/pwalk_test.go +++ b/pkg/pwalk/pwalk_test.go @@ -30,10 +30,42 @@ func TestWalk(t *testing.T) { t.Errorf("File count mismatch: found %d, expected %d", count, total) } - t.Logf("concurrency: %d, files found: %d\n", concurrency, count) + t.Logf("concurrency: %d, files found: %d", concurrency, count) } -func TestWalkManyErrors(t *testing.T) { +func TestWalkTopLevelErrNotExistNotIgnored(t *testing.T) { + if WalkN("non-existent-directory", cbEmpty, 8) == nil { + t.Fatal("expected ErrNotExist, got nil") + } +} + +// https://github.com/opencontainers/selinux/issues/199 +func TestWalkRaceWithRemoval(t *testing.T) { + var count uint32 + concurrency := runtime.NumCPU() * 2 + // This test is still on a best-effort basis, meaning it can still pass + // when there is a bug in the code, but the larger the test set is, the + // higher the probability that this test fails (without a fix). + // + // With this set (4, 5, 6), and the fix commented out, it fails + // 100 out of 100 runs on my machine. + dir, total := prepareTestSet(t, 4, 5, 6) + + // Race walk with removal. + go os.RemoveAll(dir) + err := WalkN(dir, + func(_ string, _ os.FileInfo, _ error) error { + atomic.AddUint32(&count, 1) + return nil + }, + concurrency) + t.Logf("found %d of %d files", count, total) + if err != nil { + t.Fatalf("expected nil, got %v", err) + } +} + +func TestWalkDirManyErrors(t *testing.T) { var count uint32 dir, total := prepareTestSet(t, 3, 3, 2) diff --git a/pkg/pwalkdir/README.md b/pkg/pwalkdir/README.md index 068ac40..a4926ee 100644 --- a/pkg/pwalkdir/README.md +++ b/pkg/pwalkdir/README.md @@ -28,7 +28,9 @@ Please note the following limitations of this code: * fs.SkipDir is not supported; - * no errors are ever passed to WalkDirFunc; + * ErrNotExist errors from filepath.WalkDir are silently ignored for any path + except the top directory (WalkDir argument); any other error is returned to + the caller of WalkDir; * once any error is returned from any walkDirFunc instance, no more calls to WalkDirFunc are made, and the error is returned to the caller of WalkDir; diff --git a/pkg/pwalkdir/pwalkdir.go b/pkg/pwalkdir/pwalkdir.go index 0f5d9f5..5d2d09a 100644 --- a/pkg/pwalkdir/pwalkdir.go +++ b/pkg/pwalkdir/pwalkdir.go @@ -4,6 +4,7 @@ package pwalkdir import ( + "errors" "fmt" "io/fs" "path/filepath" @@ -60,6 +61,12 @@ func WalkN(root string, walkFn fs.WalkDirFunc, num int) error { go func() { err = filepath.WalkDir(root, func(p string, entry fs.DirEntry, err error) error { if err != nil { + // Walking a file tree can race with removal, + // so ignore ENOENT, except for root. + // https://github.com/opencontainers/selinux/issues/199. + if errors.Is(err, fs.ErrNotExist) && len(p) != rootLen { + return nil + } close(files) return err } diff --git a/pkg/pwalkdir/pwalkdir_test.go b/pkg/pwalkdir/pwalkdir_test.go index 5fa59a0..0096638 100644 --- a/pkg/pwalkdir/pwalkdir_test.go +++ b/pkg/pwalkdir/pwalkdir_test.go @@ -33,12 +33,44 @@ func TestWalkDir(t *testing.T) { t.Errorf("File count mismatch: found %d, expected %d", count, total) } - t.Logf("concurrency: %d, files found: %d\n", concurrency, count) + t.Logf("concurrency: %d, files found: %d", concurrency, count) } -func TestWalkDirManyErrors(t *testing.T) { +func TestWalkDirTopLevelErrNotExistNotIgnored(t *testing.T) { + err := WalkN("non-existent-directory", cbEmpty, 8) + if err == nil { + t.Fatal("expected ErrNotExist, got nil") + } +} + +// https://github.com/opencontainers/selinux/issues/199 +func TestWalkDirRaceWithRemoval(t *testing.T) { var count uint32 + concurrency := runtime.NumCPU() * 2 + // This test is still on a best-effort basis, meaning it can still pass + // when there is a bug in the code, but the larger the test set is, the + // higher the probability that this test fails (without a fix). + // + // With this set (4, 5, 6), and the fix commented out, it fails + // about 90 out of 100 runs on my machine. + dir, total := prepareTestSet(t, 4, 5, 6) + + // Make walk race with removal. + go os.RemoveAll(dir) + err := WalkN(dir, + func(_ string, _ fs.DirEntry, _ error) error { + atomic.AddUint32(&count, 1) + return nil + }, + concurrency) + t.Logf("found %d of %d files", count, total) + if err != nil { + t.Fatalf("expected nil, got %v", err) + } +} +func TestWalkDirManyErrors(t *testing.T) { + var count uint32 dir, total := prepareTestSet(t, 3, 3, 2) max := uint32(total / 2) From c6c45015c816c3c2c4a29ba07f8ccdbce0c6f14e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 29 Aug 2023 15:24:38 -0700 Subject: [PATCH 12/54] Makefile: use $(GO) for test So one can run e.g. "make GO=go1.21.0 test". Signed-off-by: Kir Kolyshkin --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 23c3d8b..f7b9c3d 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ build-cross: .PHONY: test test: - go test -timeout 3m ${TESTFLAGS} -v ./... + $(GO) test -timeout 3m ${TESTFLAGS} -v ./... .PHONY: lint lint: From 7f5f17e2e36b0c823202ba595588e27c86c7cfb0 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 29 Aug 2023 15:26:09 -0700 Subject: [PATCH 13/54] Add a TODO to remove min/max Go 1.21 has built-in min and max, so there is no need to define our own (OTOH it's not a problem to have it). I tried to move min/max to a separate file having "//go:build !go1.21" build constraint, but go1.21.0 complains: > max requires go1.21 or later (-lang was set to go1.19; check go.mod) > min requires go1.21 or later (-lang was set to go1.19; check go.mod) I have no idea how to work around that; so let's just add a TODO item to remove min/max when it's time. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index c1f19c4..b7462f1 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -639,6 +639,7 @@ func (m mlsRange) String() string { return low + "-" + high } +// TODO: remove min and max once Go < 1.21 is not supported. func max(a, b uint) uint { if a > b { return a From b8db720e201c90655391f492dcba7d13af43e8a2 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 29 Aug 2023 15:34:08 -0700 Subject: [PATCH 14/54] ci: add codespell Signed-off-by: Kir Kolyshkin --- .codespellrc | 2 ++ .github/workflows/validate.yml | 10 ++++++++++ pkg/pwalkdir/README.md | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 .codespellrc diff --git a/.codespellrc b/.codespellrc new file mode 100644 index 0000000..8f0866a --- /dev/null +++ b/.codespellrc @@ -0,0 +1,2 @@ +[codespell] +skip = ./.git,./go.sum,./go-selinux/testdata diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index e701fc4..9e7544e 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -39,6 +39,16 @@ jobs: with: version: v1.54 + codespell: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v3 + - name: install deps + # Version of codespell bundled with Ubuntu is way old, so use pip. + run: pip install codespell + - name: run codespell + run: codespell + cross: runs-on: ubuntu-20.04 steps: diff --git a/pkg/pwalkdir/README.md b/pkg/pwalkdir/README.md index 068ac40..c36e184 100644 --- a/pkg/pwalkdir/README.md +++ b/pkg/pwalkdir/README.md @@ -51,4 +51,4 @@ filepath.WalkDir. Otherwise (if a WalkDirFunc is actually doing something) this is usually faster, except when the WalkDirN(..., 1) is used. Run `go test -bench .` to see how different operations can benefit from it, as well as how the -level of paralellism affects the speed. +level of parallelism affects the speed. From 344e2906144510040c1598698eb2d249f08e09b1 Mon Sep 17 00:00:00 2001 From: Austin Vazquez Date: Sun, 11 Feb 2024 23:40:30 +0000 Subject: [PATCH 15/54] Update GitHub Actions packages to resolve deprecation warnings. This change updates actions/checkout to v4, actions/setup-go to v5, golangci/golangci-lint-action to v4 to resolve NodeJS 16 deprecation warnings. Signed-off-by: Austin Vazquez --- .github/workflows/validate.yml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 9e7544e..e0bcc35 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -21,7 +21,7 @@ jobs: token: ${{ secrets.GITHUB_TOKEN }} - name: check subject line length - uses: tim-actions/commit-message-checker-with-regex@v0.3.1 + uses: tim-actions/commit-message-checker-with-regex@v0.3.2 with: commits: ${{ steps.get-pr-commits.outputs.commits }} pattern: '^.{0,72}(\n.*)*$' @@ -30,19 +30,19 @@ jobs: lint: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v4 + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 with: go-version: 1.21.x cache: false # golangci-lint-action does its own caching - - uses: golangci/golangci-lint-action@v3 + - uses: golangci/golangci-lint-action@v4 with: version: v1.54 codespell: runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: install deps # Version of codespell bundled with Ubuntu is way old, so use pip. run: pip install codespell @@ -52,19 +52,19 @@ jobs: cross: runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: cross run: make build-cross test-stubs: runs-on: macos-latest steps: - - uses: actions/checkout@v3 - - uses: actions/setup-go@v4 + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 with: go-version: 1.21.x cache: false # golangci-lint-action does its own caching - - uses: golangci/golangci-lint-action@v3 + - uses: golangci/golangci-lint-action@v4 with: version: v1.54 - name: test-stubs @@ -78,10 +78,10 @@ jobs: race: ["-race", ""] runs-on: ubuntu-20.04 steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: install go ${{ matrix.go-version }} - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: ${{ matrix.go-version }} From d0a26489fcca2f2273c9dd1289a476a1fb0643a2 Mon Sep 17 00:00:00 2001 From: Austin Vazquez Date: Mon, 12 Feb 2024 00:07:27 +0000 Subject: [PATCH 16/54] Rename unused parameters to resolve linter warnings Signed-off-by: Austin Vazquez --- pkg/pwalk/pwalk_test.go | 2 +- pkg/pwalkdir/pwalkdir_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/pwalk/pwalk_test.go b/pkg/pwalk/pwalk_test.go index b026681..66f6def 100644 --- a/pkg/pwalk/pwalk_test.go +++ b/pkg/pwalk/pwalk_test.go @@ -73,7 +73,7 @@ func TestWalkDirManyErrors(t *testing.T) { max := uint32(total / 2) e42 := errors.New("42") err := Walk(dir, - func(p string, i os.FileInfo, _ error) error { + func(_ string, _ os.FileInfo, _ error) error { if atomic.AddUint32(&count, 1) > max { return e42 } diff --git a/pkg/pwalkdir/pwalkdir_test.go b/pkg/pwalkdir/pwalkdir_test.go index 0096638..35e7655 100644 --- a/pkg/pwalkdir/pwalkdir_test.go +++ b/pkg/pwalkdir/pwalkdir_test.go @@ -76,7 +76,7 @@ func TestWalkDirManyErrors(t *testing.T) { max := uint32(total / 2) e42 := errors.New("42") err := Walk(dir, - func(p string, e fs.DirEntry, _ error) error { + func(_ string, _ fs.DirEntry, _ error) error { if atomic.AddUint32(&count, 1) > max { return e42 } From 8d30f36ad8b9e9eeddc0049500f9e8d6439c6c9a Mon Sep 17 00:00:00 2001 From: Austin Vazquez Date: Sun, 11 Feb 2024 23:59:35 +0000 Subject: [PATCH 17/54] Update GitHub Actions CI Go matrix for Go v1.22 This change adds Go v1.22 to the GitHub Actions CI matrix and drops Go v1.19 and v1.20 which would no longer receive updates. Also it updates the golangci-lint version to v1.56 which adds support for Go v1.22. Signed-off-by: Austin Vazquez --- .github/workflows/validate.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 9e7544e..bf1ad4f 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -33,11 +33,11 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-go@v4 with: - go-version: 1.21.x + go-version: 1.22.x cache: false # golangci-lint-action does its own caching - uses: golangci/golangci-lint-action@v3 with: - version: v1.54 + version: v1.56 codespell: runs-on: ubuntu-22.04 @@ -62,11 +62,11 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-go@v4 with: - go-version: 1.21.x + go-version: 1.22.x cache: false # golangci-lint-action does its own caching - uses: golangci/golangci-lint-action@v3 with: - version: v1.54 + version: v1.56 - name: test-stubs run: make test @@ -74,7 +74,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: [1.19.x, 1.20.x, 1.21.x] + go-version: [1.21.x, 1.22.x] race: ["-race", ""] runs-on: ubuntu-20.04 steps: From 2d0d0929c5d31f2437da04e62914746777cdef8c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 12 Feb 2024 17:06:08 -0800 Subject: [PATCH 18/54] Add dependabot config This way it can create PRs to update github workflow rules deps. Signed-off-by: Kir Kolyshkin --- .github/dependabot.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..b534a2b --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,10 @@ +# Please see the documentation for all configuration options: +# https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file + +version: 2 +updates: + # Dependencies listed in .github/workflows/*.yml + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" From 5f5e8c28c0b2778e9361cfbb279889ae454085bb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 26 Feb 2024 08:36:06 +0000 Subject: [PATCH 19/54] build(deps): bump tim-actions/get-pr-commits from 1.3.0 to 1.3.1 Bumps [tim-actions/get-pr-commits](https://github.com/tim-actions/get-pr-commits) from 1.3.0 to 1.3.1. - [Release notes](https://github.com/tim-actions/get-pr-commits/releases) - [Commits](https://github.com/tim-actions/get-pr-commits/compare/v1.3.0...v1.3.1) --- updated-dependencies: - dependency-name: tim-actions/get-pr-commits dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- .github/workflows/validate.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index d81edb7..9abddfd 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -16,7 +16,7 @@ jobs: steps: - name: get pr commits id: 'get-pr-commits' - uses: tim-actions/get-pr-commits@v1.3.0 + uses: tim-actions/get-pr-commits@v1.3.1 with: token: ${{ secrets.GITHUB_TOKEN }} From 13c8f769155302de57a5e47df9d8d7b3b209aa95 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 7 May 2024 09:06:22 +0000 Subject: [PATCH 20/54] build(deps): bump golangci/golangci-lint-action from 4 to 6 Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 4 to 6. - [Release notes](https://github.com/golangci/golangci-lint-action/releases) - [Commits](https://github.com/golangci/golangci-lint-action/compare/v4...v6) --- updated-dependencies: - dependency-name: golangci/golangci-lint-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/validate.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 9abddfd..e7984e3 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -35,7 +35,7 @@ jobs: with: go-version: 1.22.x cache: false # golangci-lint-action does its own caching - - uses: golangci/golangci-lint-action@v4 + - uses: golangci/golangci-lint-action@v6 with: version: v1.56 @@ -64,7 +64,7 @@ jobs: with: go-version: 1.22.x cache: false # golangci-lint-action does its own caching - - uses: golangci/golangci-lint-action@v4 + - uses: golangci/golangci-lint-action@v6 with: version: v1.56 - name: test-stubs From 5bdefc7e0241c7e0d35b1bee47d27fd765a0535d Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 31 Jul 2024 16:45:47 -0400 Subject: [PATCH 21/54] Show SELinux label on failure We are seeing EINVAL errors with container engines setting SELinux labels. It would be helpful to see what Labels the engines are trying to set. Signed-off-by: Daniel J Walsh --- go-selinux/selinux_linux.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index b7462f1..c80c109 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -329,7 +329,7 @@ func lSetFileLabel(fpath string, label string) error { break } if err != unix.EINTR { - return &os.PathError{Op: "lsetxattr", Path: fpath, Err: err} + return &os.PathError{Op: fmt.Sprintf("lsetxattr(label=%s)", label), Path: fpath, Err: err} } } @@ -348,7 +348,7 @@ func setFileLabel(fpath string, label string) error { break } if err != unix.EINTR { - return &os.PathError{Op: "setxattr", Path: fpath, Err: err} + return &os.PathError{Op: fmt.Sprintf("setxattr(label=%s)", label), Path: fpath, Err: err} } } From 50899337b8c06b0a38b4b981a7a54417c2e4442f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 15 Oct 2024 17:46:38 -0700 Subject: [PATCH 22/54] VERSION: remove This is not used anywhere, and go tools use git tags anyway. Signed-off-by: Kir Kolyshkin --- VERSION | 1 - 1 file changed, 1 deletion(-) delete mode 100644 VERSION diff --git a/VERSION b/VERSION deleted file mode 100644 index 095b667..0000000 --- a/VERSION +++ /dev/null @@ -1 +0,0 @@ -1.11.1-dev From af36dd6820499719d562e789b5304515025e1e52 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 25 Feb 2025 16:10:13 +0900 Subject: [PATCH 23/54] CI: add AlmaLinux 8, CentOS Stream 9, and Fedora Lima (http://lima-vm.io) is used to create VM of these distros. Close issue 220 Signed-off-by: Akihiro Suda --- .github/workflows/validate.yml | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index e7984e3..f9a2d60 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -90,3 +90,41 @@ jobs: - name: test run: make TESTFLAGS="${{ matrix.race }}" test + + vm: + name: "VM" + strategy: + fail-fast: false + matrix: + template: + - template://almalinux-8 + - template://centos-stream-9 + - template://fedora + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + + - name: "Install Lima" + uses: lima-vm/lima-actions/setup@v1 + id: lima-actions-setup + + - name: "Cache ~/.cache/lima" + uses: actions/cache@v4 + with: + path: ~/.cache/lima + key: lima-${{ steps.lima-actions-setup.outputs.version }}-${{ matrix.template }} + + - name: "Start VM" + # --plain is set to disable file sharing, port forwarding, built-in containerd, etc. for faster start up + run: limactl start --plain --name=default ${{ matrix.template }} + + - name: "Initialize VM" + run: | + set -eux -o pipefail + # Sync the current directory to /tmp/selinux in the guest + limactl cp -r . default:/tmp/selinux + # Install packages + lima sudo dnf install -y git make golang + + - name: "make test" + run: lima make -C /tmp/selinux test From 3c3fb5bfe4ca4686203da0d83649eb4b55609b2d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Sun, 9 Mar 2025 18:26:43 -0700 Subject: [PATCH 24/54] ci: install git-core It has less dependencies than the "full" git package. On Fedora, we need --setopt=install_weak_deps=false so that the "full" git package is not installed. Also, setup nodocs transaction flag as we don't need documentation inside CI environment. This might save some time and/or disk space. Before: > Install 77 Packages > Total download size: 164 M > Installed size: 431 M After: > Install 30 Packages > Total download size: 148 M > Installed size: 381 M (Those numbers are for almalinux-8). Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index f9a2d60..ca0d898 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -124,7 +124,7 @@ jobs: # Sync the current directory to /tmp/selinux in the guest limactl cp -r . default:/tmp/selinux # Install packages - lima sudo dnf install -y git make golang + lima sudo dnf install --setopt=install_weak_deps=false --setopt=tsflags=nodocs -y git-core make golang - name: "make test" run: lima make -C /tmp/selinux test From b2b2fb4a447cbd40c87955d475c5741ba89e021a Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Sun, 9 Mar 2025 02:55:11 +0900 Subject: [PATCH 25/54] CI: add openSUSE Tumbleweed Tumbleweed uses SELinux since snapshot 20250211 https://lists.opensuse.org/archives/list/factory@lists.opensuse.org/thread/G3W5NIY3OKRBHPHWTPYEUPSS4LKZN77N/ Signed-off-by: Akihiro Suda --- .github/workflows/validate.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index ca0d898..48c7dd3 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -100,6 +100,7 @@ jobs: - template://almalinux-8 - template://centos-stream-9 - template://fedora + - template://experimental/opensuse-tumbleweed runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 @@ -124,7 +125,14 @@ jobs: # Sync the current directory to /tmp/selinux in the guest limactl cp -r . default:/tmp/selinux # Install packages - lima sudo dnf install --setopt=install_weak_deps=false --setopt=tsflags=nodocs -y git-core make golang + if lima command -v dnf >/dev/null; then + lima sudo dnf install --setopt=install_weak_deps=false --setopt=tsflags=nodocs -y git-core make golang + elif lima command -v zypper >/dev/null; then + lima sudo zypper install -y git make go + else + echo >&2 "Unsupported distribution" + exit 1 + fi - name: "make test" run: lima make -C /tmp/selinux test From f0722f8c3b329cc36d1cc25307a3522860426f61 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 15 Oct 2024 17:53:03 -0700 Subject: [PATCH 26/54] ci/gha: bump ubuntu to 24.04 Ubuntu 20.04 is being deprecated. Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index ca0d898..1581ca3 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -10,7 +10,7 @@ on: jobs: commit: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 # Only check commits on pull requests. if: github.event_name == 'pull_request' steps: @@ -28,7 +28,7 @@ jobs: error: 'Subject too long (max 72)' lint: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 @@ -40,7 +40,7 @@ jobs: version: v1.56 codespell: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - name: install deps @@ -50,7 +50,7 @@ jobs: run: codespell cross: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - name: cross @@ -76,7 +76,7 @@ jobs: matrix: go-version: [1.21.x, 1.22.x] race: ["-race", ""] - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 From 4d978207f8e4f764e357edc1163fa4354710d51c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 15 Oct 2024 18:23:19 -0700 Subject: [PATCH 27/54] pkg/pwalk,pkg/pwalkdir: fix gosec in tests Fix the following gosec warnings in tests by using uint32 everywhere, so we don't have to do a single cast: pkg/pwalk/pwalk_test.go:29:20: G115: integer overflow conversion int -> uint32 (gosec) if count != uint32(total) { ^ pkg/pwalk/pwalk_test.go:73:15: G115: integer overflow conversion int -> uint32 (gosec) max := uint32(total / 2) ^ pkg/pwalk/pwalk_test.go:86:21: G115: integer overflow conversion int -> uint32 (gosec) if count != uint32(total) { ^ pkg/pwalkdir/pwalkdir_test.go:32:20: G115: integer overflow conversion int -> uint32 (gosec) if count != uint32(total) { ^ pkg/pwalkdir/pwalkdir_test.go:76:15: G115: integer overflow conversion int -> uint32 (gosec) max := uint32(total / 2) ^ pkg/pwalkdir/pwalkdir_test.go:89:21: G115: integer overflow conversion int -> uint32 (gosec) if count != uint32(total) { ^ While at it, - switch from atomic op (atomic.AddUint32) to atomic type (atomic.Int32) with methods, which is more error-prone; - rename max to maxFiles as the former is a built-in function in Go >= 1.21. Signed-off-by: Kir Kolyshkin --- pkg/pwalk/pwalk_test.go | 27 +++++++++++++++------------ pkg/pwalkdir/pwalkdir_test.go | 27 +++++++++++++++------------ 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/pkg/pwalk/pwalk_test.go b/pkg/pwalk/pwalk_test.go index 66f6def..21adce4 100644 --- a/pkg/pwalk/pwalk_test.go +++ b/pkg/pwalk/pwalk_test.go @@ -12,21 +12,22 @@ import ( ) func TestWalk(t *testing.T) { - var count uint32 + var ac atomic.Uint32 concurrency := runtime.NumCPU() * 2 dir, total := prepareTestSet(t, 3, 2, 1) err := WalkN(dir, func(_ string, _ os.FileInfo, _ error) error { - atomic.AddUint32(&count, 1) + ac.Add(1) return nil }, concurrency) if err != nil { t.Errorf("Walk failed: %v", err) } - if count != uint32(total) { + count := ac.Load() + if count != total { t.Errorf("File count mismatch: found %d, expected %d", count, total) } @@ -41,7 +42,7 @@ func TestWalkTopLevelErrNotExistNotIgnored(t *testing.T) { // https://github.com/opencontainers/selinux/issues/199 func TestWalkRaceWithRemoval(t *testing.T) { - var count uint32 + var ac atomic.Uint32 concurrency := runtime.NumCPU() * 2 // This test is still on a best-effort basis, meaning it can still pass // when there is a bug in the code, but the larger the test set is, the @@ -55,10 +56,11 @@ func TestWalkRaceWithRemoval(t *testing.T) { go os.RemoveAll(dir) err := WalkN(dir, func(_ string, _ os.FileInfo, _ error) error { - atomic.AddUint32(&count, 1) + ac.Add(1) return nil }, concurrency) + count := int(ac.Load()) t.Logf("found %d of %d files", count, total) if err != nil { t.Fatalf("expected nil, got %v", err) @@ -66,30 +68,31 @@ func TestWalkRaceWithRemoval(t *testing.T) { } func TestWalkDirManyErrors(t *testing.T) { - var count uint32 + var ac atomic.Uint32 dir, total := prepareTestSet(t, 3, 3, 2) - max := uint32(total / 2) + maxFiles := total / 2 e42 := errors.New("42") err := Walk(dir, func(_ string, _ os.FileInfo, _ error) error { - if atomic.AddUint32(&count, 1) > max { + if ac.Add(1) > maxFiles { return e42 } return nil }) + count := ac.Load() t.Logf("found %d of %d files", count, total) if err == nil { t.Errorf("Walk succeeded, but error is expected") - if count != uint32(total) { + if count != total { t.Errorf("File count mismatch: found %d, expected %d", count, total) } } } -func makeManyDirs(prefix string, levels, dirs, files int) (count int, err error) { +func makeManyDirs(prefix string, levels, dirs, files int) (count uint32, err error) { for d := 0; d < dirs; d++ { var dir string dir, err = os.MkdirTemp(prefix, "d-") @@ -109,7 +112,7 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count int, err error) if levels == 0 { continue } - var c int + var c uint32 if c, err = makeManyDirs(dir, levels-1, dirs, files); err != nil { return } @@ -124,7 +127,7 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count int, err error) // // Total dirs: dirs^levels + dirs^(levels-1) + ... + dirs^1 // Total files: total_dirs * files -func prepareTestSet(tb testing.TB, levels, dirs, files int) (dir string, total int) { +func prepareTestSet(tb testing.TB, levels, dirs, files int) (dir string, total uint32) { tb.Helper() var err error diff --git a/pkg/pwalkdir/pwalkdir_test.go b/pkg/pwalkdir/pwalkdir_test.go index 35e7655..7c0aa0f 100644 --- a/pkg/pwalkdir/pwalkdir_test.go +++ b/pkg/pwalkdir/pwalkdir_test.go @@ -16,20 +16,21 @@ import ( ) func TestWalkDir(t *testing.T) { - var count uint32 + var ac atomic.Uint32 concurrency := runtime.NumCPU() * 2 dir, total := prepareTestSet(t, 3, 2, 1) err := WalkN(dir, func(_ string, _ fs.DirEntry, _ error) error { - atomic.AddUint32(&count, 1) + ac.Add(1) return nil }, concurrency) if err != nil { t.Errorf("Walk failed: %v", err) } - if count != uint32(total) { + count := ac.Load() + if count != total { t.Errorf("File count mismatch: found %d, expected %d", count, total) } @@ -45,7 +46,7 @@ func TestWalkDirTopLevelErrNotExistNotIgnored(t *testing.T) { // https://github.com/opencontainers/selinux/issues/199 func TestWalkDirRaceWithRemoval(t *testing.T) { - var count uint32 + var ac atomic.Uint32 concurrency := runtime.NumCPU() * 2 // This test is still on a best-effort basis, meaning it can still pass // when there is a bug in the code, but the larger the test set is, the @@ -59,10 +60,11 @@ func TestWalkDirRaceWithRemoval(t *testing.T) { go os.RemoveAll(dir) err := WalkN(dir, func(_ string, _ fs.DirEntry, _ error) error { - atomic.AddUint32(&count, 1) + ac.Add(1) return nil }, concurrency) + count := ac.Load() t.Logf("found %d of %d files", count, total) if err != nil { t.Fatalf("expected nil, got %v", err) @@ -70,29 +72,30 @@ func TestWalkDirRaceWithRemoval(t *testing.T) { } func TestWalkDirManyErrors(t *testing.T) { - var count uint32 + var ac atomic.Uint32 dir, total := prepareTestSet(t, 3, 3, 2) - max := uint32(total / 2) + maxFiles := total / 2 e42 := errors.New("42") err := Walk(dir, func(_ string, _ fs.DirEntry, _ error) error { - if atomic.AddUint32(&count, 1) > max { + if ac.Add(1) > maxFiles { return e42 } return nil }) + count := ac.Load() t.Logf("found %d of %d files", count, total) if err == nil { t.Error("Walk succeeded, but error is expected") - if count != uint32(total) { + if count != total { t.Errorf("File count mismatch: found %d, expected %d", count, total) } } } -func makeManyDirs(prefix string, levels, dirs, files int) (count int, err error) { +func makeManyDirs(prefix string, levels, dirs, files int) (count uint32, err error) { for d := 0; d < dirs; d++ { var dir string dir, err = os.MkdirTemp(prefix, "d-") @@ -112,7 +115,7 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count int, err error) if levels == 0 { continue } - var c int + var c uint32 if c, err = makeManyDirs(dir, levels-1, dirs, files); err != nil { return } @@ -127,7 +130,7 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count int, err error) // // Total dirs: dirs^levels + dirs^(levels-1) + ... + dirs^1 // Total files: total_dirs * files -func prepareTestSet(tb testing.TB, levels, dirs, files int) (dir string, total int) { +func prepareTestSet(tb testing.TB, levels, dirs, files int) (dir string, total uint32) { tb.Helper() var err error From 0e280f7bb02a66cdabafe0a43ac7eecde276ccf0 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Oct 2024 11:11:07 -0700 Subject: [PATCH 28/54] parseLevelItem: limit to 31 bits, return int Most of parseLevelItem users will cast its result to int. On a 32-bit platform this means we may end up with a negative number. So, let's limit bitSize to 31 in a call to ParseUint, and return int so there are less typecasts in the code. Also, change MLS level to use int, for the same reason (less typecasts). This fixes the following gosec warnings: go-selinux/selinux_linux.go:505:30: G115: integer overflow conversion uint -> int (gosec) bitset.SetBit(bitset, int(i), 1) ^ go-selinux/selinux_linux.go:512:29: G115: integer overflow conversion uint -> int (gosec) bitset.SetBit(bitset, int(cat), 1) ^ go-selinux/selinux_linux.go:626:31: G115: integer overflow conversion uint -> int (gosec) low := "s" + strconv.Itoa(int(m.low.sens)) ^ go-selinux/selinux_linux.go:635:32: G115: integer overflow conversion uint -> int (gosec) high := "s" + strconv.Itoa(int(m.high.sens)) ^ Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index c80c109..95afdcd 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -45,7 +45,7 @@ type selinuxState struct { type level struct { cats *big.Int - sens uint + sens int } type mlsRange struct { @@ -501,14 +501,14 @@ func catsToBitset(cats string) (*big.Int, error) { return nil, err } for i := catstart; i <= catend; i++ { - bitset.SetBit(bitset, int(i), 1) + bitset.SetBit(bitset, i, 1) } } else { cat, err := parseLevelItem(ranges[0], category) if err != nil { return nil, err } - bitset.SetBit(bitset, int(cat), 1) + bitset.SetBit(bitset, cat, 1) } } @@ -516,16 +516,17 @@ func catsToBitset(cats string) (*big.Int, error) { } // parseLevelItem parses and verifies that a sensitivity or category are valid -func parseLevelItem(s string, sep levelItem) (uint, error) { +func parseLevelItem(s string, sep levelItem) (int, error) { if len(s) < minSensLen || levelItem(s[0]) != sep { return 0, ErrLevelSyntax } - val, err := strconv.ParseUint(s[1:], 10, 32) + const bitSize = 31 // Make sure the result fits into signed int32. + val, err := strconv.ParseUint(s[1:], 10, bitSize) if err != nil { return 0, err } - return uint(val), nil + return int(val), nil } // parseLevel fills a level from a string that contains @@ -622,7 +623,7 @@ func (l *level) equal(l2 *level) bool { // String returns an mlsRange as a string. func (m mlsRange) String() string { - low := "s" + strconv.Itoa(int(m.low.sens)) + low := "s" + strconv.Itoa(m.low.sens) if m.low.cats != nil && m.low.cats.BitLen() > 0 { low += ":" + bitsetToStr(m.low.cats) } @@ -631,7 +632,7 @@ func (m mlsRange) String() string { return low } - high := "s" + strconv.Itoa(int(m.high.sens)) + high := "s" + strconv.Itoa(m.high.sens) if m.high.cats != nil && m.high.cats.BitLen() > 0 { high += ":" + bitsetToStr(m.high.cats) } @@ -640,14 +641,14 @@ func (m mlsRange) String() string { } // TODO: remove min and max once Go < 1.21 is not supported. -func max(a, b uint) uint { +func max(a, b int) int { if a > b { return a } return b } -func min(a, b uint) uint { +func min(a, b int) int { if a < b { return a } From adb37bc5f127be00941fdbaecab392a154e7200b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 11 Mar 2025 23:36:51 -0700 Subject: [PATCH 29/54] Rename min/max to not clash with Go 1.21 built-ins This fixes the following linter warnings: go-selinux/selinux_linux.go:645:6: function max has same name as predeclared identifier (predeclared) func max(a, b int) int { ^ go-selinux/selinux_linux.go:652:6: function min has same name as predeclared identifier (predeclared) func min(a, b int) int { ^ Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 95afdcd..7b3944c 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -640,15 +640,16 @@ func (m mlsRange) String() string { return low + "-" + high } -// TODO: remove min and max once Go < 1.21 is not supported. -func max(a, b int) int { +// TODO: remove these in favor of built-in min/max +// once we stop supporting Go < 1.21. +func maxInt(a, b int) int { if a > b { return a } return b } -func min(a, b int) int { +func minInt(a, b int) int { if a < b { return a } @@ -677,10 +678,10 @@ func calculateGlbLub(sourceRange, targetRange string) (string, error) { outrange := &mlsRange{low: &level{}, high: &level{}} /* take the greatest of the low */ - outrange.low.sens = max(s.low.sens, t.low.sens) + outrange.low.sens = maxInt(s.low.sens, t.low.sens) /* take the least of the high */ - outrange.high.sens = min(s.high.sens, t.high.sens) + outrange.high.sens = minInt(s.high.sens, t.high.sens) /* find the intersecting categories */ if s.low.cats != nil && t.low.cats != nil { From 0a4868c70b2329168ec00f69b7c5f858c81ac53f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Oct 2024 15:01:43 -0700 Subject: [PATCH 30/54] Silence a SELINUX_MAGIC gosec warning Gosec doesn't like this code: go-selinux/selinux_linux.go:141:11: G115: integer overflow conversion int64 -> uint32 (gosec) if uint32(buf.Type) != uint32(unix.SELINUX_MAGIC) { ^ But it is correct because - buf.Type is int64 or int32, depending on the platform; - unix.SELINUX_MAGIC is untyped int which overflows int32 (i.e. it becomes negative). So the best type to use here is uint32. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 7b3944c..962de59 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -138,6 +138,7 @@ func verifySELinuxfsMount(mnt string) bool { return false } + //#nosec G115 -- there is no overflow here. if uint32(buf.Type) != uint32(unix.SELINUX_MAGIC) { return false } From b42a48c2d92dfb5539467bedc767d9952927c3fb Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Oct 2024 15:10:53 -0700 Subject: [PATCH 31/54] Silence a gosec warning Gosec complains: go-selinux/selinux_linux.go:587:14: G115: integer overflow conversion uint -> int (gosec) for i := int(c.TrailingZeroBits()); i < c.BitLen(); i++ { ^ This is indeed a valid concern in case TrailingZeroBits returns a value which uses a highest bit (i.e. more than MaxInt32 or MaxInt64, depending on the platform). But I think this is highly unlikely. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 962de59..84e1d75 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -584,7 +584,8 @@ func bitsetToStr(c *big.Int) string { var str string length := 0 - for i := int(c.TrailingZeroBits()); i < c.BitLen(); i++ { + i0 := int(c.TrailingZeroBits()) //#nosec G115 -- don't expect TralingZeroBits to return values with highest bit set. + for i := i0; i < c.BitLen(); i++ { if c.Bit(i) == 0 { continue } From 07df017f13bd7eb6517fd7640a83282f8991e1a4 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 11 Mar 2025 23:47:27 -0700 Subject: [PATCH 32/54] Remove unneeded nolint:gosec annotations In both of these cases we're writing to a file in /sys/fs/selinux virtual file system, the file is pre-existing (and if it isn't, it could not be created), so the third WriteFile argument is never used and can as well be 0. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 84e1d75..22ecd01 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -813,8 +813,7 @@ func enforceMode() int { // setEnforceMode sets the current SELinux mode Enforcing, Permissive. // Disabled is not valid, since this needs to be set at boot time. func setEnforceMode(mode int) error { - //nolint:gosec // ignore G306: permissions to be 0600 or less. - return os.WriteFile(selinuxEnforcePath(), []byte(strconv.Itoa(mode)), 0o644) + return os.WriteFile(selinuxEnforcePath(), []byte(strconv.Itoa(mode)), 0) } // defaultEnforceMode returns the systems default SELinux mode Enforcing, @@ -1021,8 +1020,7 @@ func addMcs(processLabel, fileLabel string) (string, string) { // securityCheckContext validates that the SELinux label is understood by the kernel func securityCheckContext(val string) error { - //nolint:gosec // ignore G306: permissions to be 0600 or less. - return os.WriteFile(filepath.Join(getSelinuxMountPoint(), "context"), []byte(val), 0o644) + return os.WriteFile(filepath.Join(getSelinuxMountPoint(), "context"), []byte(val), 0) } // copyLevel returns a label with the MLS/MCS level from src label replaced on From 1cdb81223a5b92bb5f03c473ca8ee5c769a83ad7 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 3 Mar 2025 13:02:55 -0800 Subject: [PATCH 33/54] pkg/pwalk*: use t.TempDir in tests While at it, fix naked returns. Signed-off-by: Kir Kolyshkin --- pkg/pwalk/pwalk_test.go | 12 ++---------- pkg/pwalkdir/pwalkdir_test.go | 12 ++---------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/pkg/pwalk/pwalk_test.go b/pkg/pwalk/pwalk_test.go index 21adce4..f28fa28 100644 --- a/pkg/pwalk/pwalk_test.go +++ b/pkg/pwalk/pwalk_test.go @@ -131,22 +131,14 @@ func prepareTestSet(tb testing.TB, levels, dirs, files int) (dir string, total u tb.Helper() var err error - dir, err = os.MkdirTemp(".", "pwalk-test-") - if err != nil { - tb.Fatal(err) - } - tb.Cleanup(func() { - if err := os.RemoveAll(dir); err != nil && !errors.Is(err, os.ErrNotExist) { - tb.Errorf("cleanup error: %v", err) - } - }) + dir = tb.TempDir() total, err = makeManyDirs(dir, levels, dirs, files) if err != nil { tb.Fatal(err) } total++ // this dir - return + return dir, total } type walkerFunc func(root string, walkFn WalkFunc) error diff --git a/pkg/pwalkdir/pwalkdir_test.go b/pkg/pwalkdir/pwalkdir_test.go index 7c0aa0f..eb1653d 100644 --- a/pkg/pwalkdir/pwalkdir_test.go +++ b/pkg/pwalkdir/pwalkdir_test.go @@ -134,22 +134,14 @@ func prepareTestSet(tb testing.TB, levels, dirs, files int) (dir string, total u tb.Helper() var err error - dir, err = os.MkdirTemp(".", "pwalk-test-") - if err != nil { - tb.Fatal(err) - } - tb.Cleanup(func() { - if err := os.RemoveAll(dir); err != nil && !errors.Is(err, os.ErrNotExist) { - tb.Errorf("cleanup error: %v", err) - } - }) + dir = tb.TempDir() total, err = makeManyDirs(dir, levels, dirs, files) if err != nil { tb.Fatal(err) } total++ // this dir - return + return dir, total } type walkerFunc func(root string, walkFn fs.WalkDirFunc) error From 1d50a6584819ab4ecee0338cdc8648836ede63d4 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Oct 2024 15:13:07 -0700 Subject: [PATCH 34/54] ci: bump golangci-lint to v1.64 The new version produces the following warnings: WARN [config_reader] The configuration option `linters.govet.check-shadowing` is deprecated. Please enable `shadow` instead, if you are not using `enable-all`. WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar. WARN The linter 'tenv' is deprecated (since v1.64.0) due to: Duplicate feature another linter. Replaced by usetesting. so fix the configuration accordingly. Note we do not enable copyloopvar since it requires Go 1.22 and we're currently have it set to Go 1.21 in go.mod. Also, remove "cache: false" from actions/setup-go@v5 since golangci-lint-action@v6 now relies on setup-go caching. Also, rename "deadline" to "timeout" as the former is no longer supported. Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 6 ++---- .golangci.yml | 7 +++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 1581ca3..33d6ae7 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -34,10 +34,9 @@ jobs: - uses: actions/setup-go@v5 with: go-version: 1.22.x - cache: false # golangci-lint-action does its own caching - uses: golangci/golangci-lint-action@v6 with: - version: v1.56 + version: v1.64 codespell: runs-on: ubuntu-24.04 @@ -63,10 +62,9 @@ jobs: - uses: actions/setup-go@v5 with: go-version: 1.22.x - cache: false # golangci-lint-action does its own caching - uses: golangci/golangci-lint-action@v6 with: - version: v1.56 + version: v1.64 - name: test-stubs run: make test diff --git a/.golangci.yml b/.golangci.yml index a570a2e..2083fda 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,12 +1,12 @@ --- run: concurrency: 6 - deadline: 5m + timeout: 5m linters: enable: + # - copyloopvar # Detects places where loop variables are copied. TODO enable for Go 1.22+ - dupword # Detects duplicate words. - errorlint # Detects code that may cause problems with Go 1.13 error wrapping. - - exportloopref # Detects pointers to enclosing loop variables. - gocritic # Metalinter; detects bugs, performance, and styling issues. - gofumpt # Detects whether code was gofumpt-ed. - gosec # Detects security problems. @@ -16,13 +16,12 @@ linters: - prealloc # Detects slice declarations that could potentially be pre-allocated. - predeclared # Detects code that shadows one of Go's predeclared identifiers - revive # Metalinter; drop-in replacement for golint. - - tenv # Detects using os.Setenv instead of t.Setenv. - thelper # Detects test helpers without t.Helper(). - tparallel # Detects inappropriate usage of t.Parallel(). - unconvert # Detects unnecessary type conversions. + - usetesting # Reports uses of functions with replacement inside the testing package. linters-settings: govet: - check-shadowing: true enable-all: true settings: shadow: From f5339bac46975bddc0efafe1afe38c0c836b8760 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Oct 2024 15:37:51 -0700 Subject: [PATCH 35/54] ci: add Go 1.23, 1.24 to test matrix Also, replace Go 1.21 with Go 1.19 as this is the minimally supported version for now. Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 33d6ae7..4f745cf 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -72,7 +72,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: [1.21.x, 1.22.x] + go-version: [1.19.x, 1.23.x, 1.24.x] race: ["-race", ""] runs-on: ubuntu-24.04 steps: From 336f2dd40eae25af4021142b4e9d07aafc3191d6 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Oct 2024 16:51:36 -0700 Subject: [PATCH 36/54] ci: add 32-bit test Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 4f745cf..2402715 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -126,3 +126,6 @@ jobs: - name: "make test" run: lima make -C /tmp/selinux test + + - name: "32-bit test" + run: lima make -C /tmp/selinux GOARCH=386 test From 9e7caf66240fb21a44de66654fa3398a8c5ef1e6 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 16 Oct 2024 17:07:39 -0700 Subject: [PATCH 37/54] ci: add all-done job The sole reason is to simplify branch protection rules, requiring just this one to be passed. I tried but could not find a way to list all other jobs, so had to add all of them manually. Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 2402715..32f4b75 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -129,3 +129,16 @@ jobs: - name: "32-bit test" run: lima make -C /tmp/selinux GOARCH=386 test + + all-done: + needs: + - commit + - lint + - codespell + - cross + - test-stubs + - test + - vm + runs-on: ubuntu-24.04 + steps: + - run: echo "All jobs completed" From 6ef149a2aa73db2644c265dc181205956e87e5f3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 3 Mar 2025 12:12:33 -0800 Subject: [PATCH 38/54] deps: bump golang.org/x/sys to v0.1.0 Use the oldest tagged version here, because all the functionality this package needs is in there already. Signed-off-by: Kir Kolyshkin --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 56328f1..12f9f64 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,4 @@ module github.com/opencontainers/selinux go 1.19 -require golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 +require golang.org/x/sys v0.1.0 diff --git a/go.sum b/go.sum index 28ab7f7..b69ea85 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,2 @@ -golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 h1:0A+M6Uqn+Eje4kHMK80dtF3JCXC4ykBgQG4Fe06QRhQ= -golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= From c826d9e11db4f0f915b915d37fa52635c34cfcf3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Sun, 9 Mar 2025 18:23:27 -0700 Subject: [PATCH 39/54] ci: show avc denials after tests Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 32f4b75..0fbd7e7 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -125,11 +125,16 @@ jobs: lima sudo dnf install --setopt=install_weak_deps=false --setopt=tsflags=nodocs -y git-core make golang - name: "make test" + continue-on-error: true run: lima make -C /tmp/selinux test - name: "32-bit test" + continue-on-error: true run: lima make -C /tmp/selinux GOARCH=386 test + - name: "Show AVC denials" + run: lima sudo ausearch -m AVC,USER_AVC || true + all-done: needs: - commit From 931542d18d42d3a58f3a4967db392d02b7ff32cc Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 12 Mar 2025 09:55:20 -0700 Subject: [PATCH 40/54] label: stop using deprecated stuff in tests Move 4 Linux tests that only test stuff from the selinux pkg (TestDuplicateLabel, TestSELinuxNoLevel, TestSocketLabel, TestKeyLabel) from label pkg to selinux, following commit 15c906f ("label: deprecate selinux wrappers"). Remove some stub tests in label pkg (TestSocketLabel, TestKeyLabel, TestProcessLabel, and parts of TestCheckLabelCompile) that already have equivalents in selinux (TestSELinuxStubs). Fix a few remaining issues in label to not use deprecated stuff (ROMountLabel -> selinux.ROFileLabel, GenLabels -> InitLabels). Signed-off-by: Kir Kolyshkin --- go-selinux/label/label_linux_test.go | 98 +-------------------------- go-selinux/label/label_stub_test.go | 61 +++-------------- go-selinux/selinux_linux_test.go | 99 ++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 149 deletions(-) diff --git a/go-selinux/label/label_linux_test.go b/go-selinux/label/label_linux_test.go index 0200810..e25ead7 100644 --- a/go-selinux/label/label_linux_test.go +++ b/go-selinux/label/label_linux_test.go @@ -3,7 +3,6 @@ package label import ( "errors" "os" - "strings" "testing" "github.com/opencontainers/selinux/go-selinux" @@ -25,9 +24,8 @@ func TestInit(t *testing.T) { t.Fatalf("InitLabels failed: %v:", err) } testDisabled := []string{"disable"} - roMountLabel := ROMountLabel() - if roMountLabel == "" { - t.Fatal("ROMountLabel: empty") + if selinux.ROFileLabel() == "" { + t.Fatal("selinux.ROFileLabel: empty") } plabel, mlabel, err := InitLabels(testDisabled) if err != nil { @@ -55,45 +53,6 @@ func TestInit(t *testing.T) { } } -func TestDuplicateLabel(t *testing.T) { - secopt, err := DupSecOpt("system_u:system_r:container_t:s0:c1,c2") - if err != nil { - t.Fatalf("DupSecOpt: %v", err) - } - for _, opt := range secopt { - con := strings.SplitN(opt, ":", 2) - if con[0] == "user" { - if con[1] != "system_u" { - t.Errorf("DupSecOpt Failed user incorrect") - } - continue - } - if con[0] == "role" { - if con[1] != "system_r" { - t.Errorf("DupSecOpt Failed role incorrect") - } - continue - } - if con[0] == "type" { - if con[1] != "container_t" { - t.Errorf("DupSecOpt Failed type incorrect") - } - continue - } - if con[0] == "level" { - if con[1] != "s0:c1,c2" { - t.Errorf("DupSecOpt Failed level incorrect") - } - continue - } - t.Errorf("DupSecOpt failed: invalid field %q", con[0]) - } - secopt = DisableSecOpt() - if secopt[0] != "disable" { - t.Errorf("DisableSecOpt failed: expected \"disable\", got %q", secopt[0]) - } -} - func TestRelabel(t *testing.T) { needSELinux(t) @@ -157,59 +116,6 @@ func TestIsShared(t *testing.T) { } } -func TestSELinuxNoLevel(t *testing.T) { - needSELinux(t) - - tlabel := "system_u:system_r:container_t" - dup, err := DupSecOpt(tlabel) - if err != nil { - t.Fatal(err) - } - - if len(dup) != 3 { - t.Errorf("DupSecOpt failed on non mls label: expected 3, got %d", len(dup)) - } - con, err := selinux.NewContext(tlabel) - if err != nil { - t.Fatal(err) - } - if con.Get() != tlabel { - t.Errorf("NewContaxt and con.Get() failed on non mls label: expected %q, got %q", tlabel, con.Get()) - } -} - -func TestSocketLabel(t *testing.T) { - needSELinux(t) - - label := "system_u:object_r:container_t:s0:c1,c2" - if err := selinux.SetSocketLabel(label); err != nil { - t.Fatal(err) - } - nlabel, err := selinux.SocketLabel() - if err != nil { - t.Fatal(err) - } - if label != nlabel { - t.Errorf("SocketLabel %s != %s", nlabel, label) - } -} - -func TestKeyLabel(t *testing.T) { - needSELinux(t) - - label := "system_u:object_r:container_t:s0:c1,c2" - if err := selinux.SetKeyLabel(label); err != nil { - t.Fatal(err) - } - nlabel, err := selinux.KeyLabel() - if err != nil { - t.Fatal(err) - } - if label != nlabel { - t.Errorf("KeyLabel %s != %s", nlabel, label) - } -} - func TestFileLabel(t *testing.T) { needSELinux(t) diff --git a/go-selinux/label/label_stub_test.go b/go-selinux/label/label_stub_test.go index 9742e6e..e92cc8b 100644 --- a/go-selinux/label/label_stub_test.go +++ b/go-selinux/label/label_stub_test.go @@ -3,7 +3,11 @@ package label -import "testing" +import ( + "testing" + + "github.com/opencontainers/selinux/go-selinux" +) const testLabel = "system_u:object_r:container_file_t:s0:c1,c2" @@ -15,9 +19,8 @@ func TestInit(t *testing.T) { t.Fatal(err) } testDisabled := []string{"disable"} - roMountLabel := ROMountLabel() - if roMountLabel != "" { - t.Errorf("ROMountLabel Failed") + if selinux.ROFileLabel() != "" { + t.Error("selinux.ROFileLabel Failed") } plabel, mlabel, err := InitLabels(testDisabled) if err != nil { @@ -44,45 +47,12 @@ func TestRelabel(t *testing.T) { } } -func TestSocketLabel(t *testing.T) { - label := testLabel - if err := SetSocketLabel(label); err != nil { - t.Fatal(err) - } - if _, err := SocketLabel(); err != nil { - t.Fatal(err) - } -} - -func TestKeyLabel(t *testing.T) { - label := testLabel - if err := SetKeyLabel(label); err != nil { - t.Fatal(err) - } - if _, err := KeyLabel(); err != nil { - t.Fatal(err) - } -} - -func TestProcessLabel(t *testing.T) { - label := testLabel - if err := SetProcessLabel(label); err != nil { - t.Fatal(err) - } - if _, err := ProcessLabel(); err != nil { - t.Fatal(err) - } -} - func TestCheckLabelCompile(t *testing.T) { - if _, _, err := GenLabels(""); err != nil { + if _, _, err := InitLabels(nil); err != nil { t.Fatal(err) } tmpDir := t.TempDir() - if _, err := FileLabel(tmpDir); err != nil { - t.Fatal(err) - } if err := SetFileLabel(tmpDir, "foobar"); err != nil { t.Fatal(err) @@ -92,21 +62,6 @@ func TestCheckLabelCompile(t *testing.T) { t.Fatal(err) } - if _, err := PidLabel(0); err != nil { - t.Fatal(err) - } - - ClearLabels() - - if err := ReserveLabel("foobar"); err != nil { - t.Fatal(err) - } - - if err := ReleaseLabel("foobar"); err != nil { - t.Fatal(err) - } - - _, _ = DupSecOpt("foobar") DisableSecOpt() if err := Validate("foobar"); err != nil { diff --git a/go-selinux/selinux_linux_test.go b/go-selinux/selinux_linux_test.go index c49e2bf..a4563f0 100644 --- a/go-selinux/selinux_linux_test.go +++ b/go-selinux/selinux_linux_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "testing" ) @@ -119,6 +120,104 @@ func TestInitLabels(t *testing.T) { ReleaseLabel(plabel) } +func TestDuplicateLabel(t *testing.T) { + secopt, err := DupSecOpt("system_u:system_r:container_t:s0:c1,c2") + if err != nil { + t.Fatalf("DupSecOpt: %v", err) + } + for _, opt := range secopt { + con := strings.SplitN(opt, ":", 2) + if con[0] == "user" { + if con[1] != "system_u" { + t.Errorf("DupSecOpt Failed user incorrect") + } + continue + } + if con[0] == "role" { + if con[1] != "system_r" { + t.Errorf("DupSecOpt Failed role incorrect") + } + continue + } + if con[0] == "type" { + if con[1] != "container_t" { + t.Errorf("DupSecOpt Failed type incorrect") + } + continue + } + if con[0] == "level" { + if con[1] != "s0:c1,c2" { + t.Errorf("DupSecOpt Failed level incorrect") + } + continue + } + t.Errorf("DupSecOpt failed: invalid field %q", con[0]) + } + secopt = DisableSecOpt() + if secopt[0] != "disable" { + t.Errorf(`DisableSecOpt failed: want "disable", got %q`, secopt[0]) + } +} + +func TestSELinuxNoLevel(t *testing.T) { + if !GetEnabled() { + t.Skip("SELinux not enabled, skipping.") + } + + tlabel := "system_u:system_r:container_t" + dup, err := DupSecOpt(tlabel) + if err != nil { + t.Fatal(err) + } + + if len(dup) != 3 { + t.Errorf("DupSecOpt failed on non mls label: want 3, got %d", len(dup)) + } + con, err := NewContext(tlabel) + if err != nil { + t.Fatal(err) + } + if con.Get() != tlabel { + t.Errorf("NewContext and con.Get() failed on non mls label: want %q, got %q", tlabel, con.Get()) + } +} + +func TestSocketLabel(t *testing.T) { + if !GetEnabled() { + t.Skip("SELinux not enabled, skipping.") + } + + label := "system_u:object_r:container_t:s0:c1,c2" + if err := SetSocketLabel(label); err != nil { + t.Fatal(err) + } + nlabel, err := SocketLabel() + if err != nil { + t.Fatal(err) + } + if label != nlabel { + t.Errorf("SocketLabel %s != %s", nlabel, label) + } +} + +func TestKeyLabel(t *testing.T) { + if !GetEnabled() { + t.Skip("SELinux not enabled, skipping.") + } + + label := "system_u:object_r:container_t:s0:c1,c2" + if err := SetKeyLabel(label); err != nil { + t.Fatal(err) + } + nlabel, err := KeyLabel() + if err != nil { + t.Fatal(err) + } + if label != nlabel { + t.Errorf("KeyLabel: want %q, got %q", label, nlabel) + } +} + func BenchmarkContextGet(b *testing.B) { ctx, err := NewContext("system_u:object_r:container_file_t:s0:c1022,c1023") if err != nil { From 6f9de938844647b793942b1f5cdfd52354df329a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 11 Mar 2025 22:56:12 -0700 Subject: [PATCH 41/54] label: remove deprecated stuff This was mostly deprecated in 2020 and I was not able to find any existing users (except for very old vendor directories in a dozen of unmaintained repos). Time to move on. Signed-off-by: Kir Kolyshkin --- go-selinux/label/label.go | 67 --------------------------------- go-selinux/label/label_linux.go | 11 ------ go-selinux/label/label_stub.go | 6 --- 3 files changed, 84 deletions(-) diff --git a/go-selinux/label/label.go b/go-selinux/label/label.go index 07e0f77..884a8b8 100644 --- a/go-selinux/label/label.go +++ b/go-selinux/label/label.go @@ -6,78 +6,11 @@ import ( "github.com/opencontainers/selinux/go-selinux" ) -// Deprecated: use selinux.ROFileLabel -var ROMountLabel = selinux.ROFileLabel - -// SetProcessLabel takes a process label and tells the kernel to assign the -// label to the next program executed by the current process. -// Deprecated: use selinux.SetExecLabel -var SetProcessLabel = selinux.SetExecLabel - -// ProcessLabel returns the process label that the kernel will assign -// to the next program executed by the current process. If "" is returned -// this indicates that the default labeling will happen for the process. -// Deprecated: use selinux.ExecLabel -var ProcessLabel = selinux.ExecLabel - -// SetSocketLabel takes a process label and tells the kernel to assign the -// label to the next socket that gets created -// Deprecated: use selinux.SetSocketLabel -var SetSocketLabel = selinux.SetSocketLabel - -// SocketLabel retrieves the current default socket label setting -// Deprecated: use selinux.SocketLabel -var SocketLabel = selinux.SocketLabel - -// SetKeyLabel takes a process label and tells the kernel to assign the -// label to the next kernel keyring that gets created -// Deprecated: use selinux.SetKeyLabel -var SetKeyLabel = selinux.SetKeyLabel - -// KeyLabel retrieves the current default kernel keyring label setting -// Deprecated: use selinux.KeyLabel -var KeyLabel = selinux.KeyLabel - -// FileLabel returns the label for specified path -// Deprecated: use selinux.FileLabel -var FileLabel = selinux.FileLabel - -// PidLabel will return the label of the process running with the specified pid -// Deprecated: use selinux.PidLabel -var PidLabel = selinux.PidLabel - // Init initialises the labeling system func Init() { _ = selinux.GetEnabled() } -// ClearLabels will clear all reserved labels -// Deprecated: use selinux.ClearLabels -var ClearLabels = selinux.ClearLabels - -// ReserveLabel will record the fact that the MCS label has already been used. -// This will prevent InitLabels from using the MCS label in a newly created -// container -// Deprecated: use selinux.ReserveLabel -func ReserveLabel(label string) error { - selinux.ReserveLabel(label) - return nil -} - -// ReleaseLabel will remove the reservation of the MCS label. -// This will allow InitLabels to use the MCS label in a newly created -// containers -// Deprecated: use selinux.ReleaseLabel -func ReleaseLabel(label string) error { - selinux.ReleaseLabel(label) - return nil -} - -// DupSecOpt takes a process label and returns security options that -// can be used to set duplicate labels on future container processes -// Deprecated: use selinux.DupSecOpt -var DupSecOpt = selinux.DupSecOpt - // FormatMountLabel returns a string to be used by the mount command. Using // the SELinux `context` mount option. Changing labels of files on mount // points with this option can never be changed. diff --git a/go-selinux/label/label_linux.go b/go-selinux/label/label_linux.go index e49e6d5..1258c98 100644 --- a/go-selinux/label/label_linux.go +++ b/go-selinux/label/label_linux.go @@ -79,12 +79,6 @@ func InitLabels(options []string) (plabel string, mlabel string, retErr error) { return processLabel, mountLabel, nil } -// Deprecated: The GenLabels function is only to be used during the transition -// to the official API. Use InitLabels(strings.Fields(options)) instead. -func GenLabels(options string) (string, string, error) { - return InitLabels(strings.Fields(options)) -} - // SetFileLabel modifies the "path" label to the specified file label func SetFileLabel(path string, fileLabel string) error { if !selinux.GetEnabled() || fileLabel == "" { @@ -123,11 +117,6 @@ func Relabel(path string, fileLabel string, shared bool) error { return selinux.Chcon(path, fileLabel, true) } -// DisableSecOpt returns a security opt that can disable labeling -// support for future container processes -// Deprecated: use selinux.DisableSecOpt -var DisableSecOpt = selinux.DisableSecOpt - // Validate checks that the label does not include unexpected options func Validate(label string) error { if strings.Contains(label, "z") && strings.Contains(label, "Z") { diff --git a/go-selinux/label/label_stub.go b/go-selinux/label/label_stub.go index 1c260cb..7a54afc 100644 --- a/go-selinux/label/label_stub.go +++ b/go-selinux/label/label_stub.go @@ -10,12 +10,6 @@ func InitLabels([]string) (string, string, error) { return "", "", nil } -// Deprecated: The GenLabels function is only to be used during the transition -// to the official API. Use InitLabels(strings.Fields(options)) instead. -func GenLabels(string) (string, string, error) { - return "", "", nil -} - func SetFileLabel(string, string) error { return nil } From 13b180a2d3d43dda50b6ce8e73cb65e4d4e92107 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 10 Mar 2025 17:45:25 -0700 Subject: [PATCH 42/54] TestSELinux: use LockOSThread to avoid flakes The SetFSCreateLabel documentation says the caller should use runtime.LockOSThread. Indeed, if not used, there is an occasional flake: > $ go test -run TestSELinux -count 10000 . > selinux_linux_test.go:168: SetFSCreateLabel failed write /proc/thread-self/attr/fscreate: permission denied > selinux_linux_test.go:169: write /proc/thread-self/attr/fscreate: permission denied Add LockOSThread to fix the flake. While at it, simplify code flow to avoid using "else", and fix logging to avoid printing the same error twice. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux_test.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/go-selinux/selinux_linux_test.go b/go-selinux/selinux_linux_test.go index a4563f0..7681c5d 100644 --- a/go-selinux/selinux_linux_test.go +++ b/go-selinux/selinux_linux_test.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strconv" "strings" "testing" @@ -235,6 +236,12 @@ func TestSELinux(t *testing.T) { t.Skip("SELinux not enabled, skipping.") } + // Ensure the thread stays the same for duration of the test. + // Otherwise Go runtime can switch this to a different thread, + // which results in EACCES in call to SetFSCreateLabel. + runtime.LockOSThread() + defer runtime.UnlockOSThread() + var ( err error plabel, flabel string @@ -259,21 +266,17 @@ func TestSELinux(t *testing.T) { ReleaseLabel(plabel) pid := os.Getpid() - t.Logf("PID:%d MCS:%s\n", pid, intToMcs(pid, 1023)) + t.Logf("PID:%d MCS:%s", pid, intToMcs(pid, 1023)) err = SetFSCreateLabel("unconfined_u:unconfined_r:unconfined_t:s0") - if err == nil { - t.Log(FSCreateLabel()) - } else { - t.Log("SetFSCreateLabel failed", err) - t.Fatal(err) + if err != nil { + t.Fatal("SetFSCreateLabel failed:", err) } + t.Log(FSCreateLabel()) err = SetFSCreateLabel("") - if err == nil { - t.Log(FSCreateLabel()) - } else { - t.Log("SetFSCreateLabel failed", err) - t.Fatal(err) + if err != nil { + t.Fatal("SetFSCreateLabel failed:", err) } + t.Log(FSCreateLabel()) t.Log(PidLabel(1)) } From 4c76c019cd748e4efcaff17d837da42e9bb66f98 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 11 Mar 2025 19:39:21 -0700 Subject: [PATCH 43/54] TestSocketLabel: use LockOSThread to avoid flakes The SetSocketLabel documentation says the caller should use runtime.LockOSThread. Indeed, if not used, there is an occasional flake: > $ go test -count 10000 -run SocketLabel . > --- FAIL: TestSocketLabel (0.00s) > label_linux_test.go:189: write /proc/thread-self/attr/sockcreate: permission denied Add LockOSThread to fix the flake. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux_linux_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/go-selinux/selinux_linux_test.go b/go-selinux/selinux_linux_test.go index 7681c5d..02259ce 100644 --- a/go-selinux/selinux_linux_test.go +++ b/go-selinux/selinux_linux_test.go @@ -188,6 +188,12 @@ func TestSocketLabel(t *testing.T) { t.Skip("SELinux not enabled, skipping.") } + // Ensure the thread stays the same for duration of the test. + // Otherwise Go runtime can switch this to a different thread, + // which results in EACCES in call to SetSocketLabel. + runtime.LockOSThread() + defer runtime.UnlockOSThread() + label := "system_u:object_r:container_t:s0:c1,c2" if err := SetSocketLabel(label); err != nil { t.Fatal(err) From 965323e01c4e6b2e5046968d194f520e1502498a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 11 Mar 2025 19:44:49 -0700 Subject: [PATCH 44/54] SetKeyLabel: add thread group leader requirement Since commit bbbc51cbc717 ("Need to set process attributes not task") SetKeyLabel and KeyLabel operate on /proc/self/attr/keycreate, which can only be modified by the thread group leader; a non thread group leader thread will get EACCES: > write /proc/self/attr/keycreate: permission denied Let's document that, and return a more meaningful error (ErrNotTGLeader) if that is the case. Modify the test case accordingly, i.e. skip it if the current thread is not the thread group leader. Signed-off-by: Kir Kolyshkin --- go-selinux/selinux.go | 16 ++++++++++++---- go-selinux/selinux_linux.go | 3 +++ go-selinux/selinux_linux_test.go | 12 ++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/go-selinux/selinux.go b/go-selinux/selinux.go index af058b8..9f0740e 100644 --- a/go-selinux/selinux.go +++ b/go-selinux/selinux.go @@ -41,6 +41,10 @@ var ( // ErrVerifierNil is returned when a context verifier function is nil. ErrVerifierNil = errors.New("verifier function is nil") + // ErrNotTGLeader is returned by [SetKeyLabel] if the calling thread + // is not the thread group leader. + ErrNotTGLeader = errors.New("calling thread is not the thread group leader") + // CategoryRange allows the upper bound on the category range to be adjusted CategoryRange = DefaultCategoryRange @@ -180,10 +184,14 @@ func PeerLabel(fd uintptr) (string, error) { } // SetKeyLabel takes a process label and tells the kernel to assign the -// label to the next kernel keyring that gets created. Calls to SetKeyLabel -// should be wrapped in runtime.LockOSThread()/runtime.UnlockOSThread() until -// the kernel keyring is created to guarantee another goroutine does not migrate -// to the current thread before execution is complete. +// label to the next kernel keyring that gets created. +// +// Calls to SetKeyLabel should be wrapped in +// runtime.LockOSThread()/runtime.UnlockOSThread() until the kernel keyring is +// created to guarantee another goroutine does not migrate to the current +// thread before execution is complete. +// +// Only the thread group leader can set key label. func SetKeyLabel(label string) error { return setKeyLabel(label) } diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 22ecd01..a607980 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -735,6 +735,9 @@ func setKeyLabel(label string) error { if label == "" && errors.Is(err, os.ErrPermission) { return nil } + if errors.Is(err, unix.EACCES) && unix.Getuid() != unix.Gettid() { + return ErrNotTGLeader + } return err } diff --git a/go-selinux/selinux_linux_test.go b/go-selinux/selinux_linux_test.go index 02259ce..71aa0b8 100644 --- a/go-selinux/selinux_linux_test.go +++ b/go-selinux/selinux_linux_test.go @@ -11,6 +11,8 @@ import ( "strconv" "strings" "testing" + + "golang.org/x/sys/unix" ) func TestSetFileLabel(t *testing.T) { @@ -212,6 +214,16 @@ func TestKeyLabel(t *testing.T) { t.Skip("SELinux not enabled, skipping.") } + // Ensure the thread stays the same for duration of the test. + // Otherwise Go runtime can switch this to a different thread, + // which results in EACCES in call to SetKeyLabel. + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + if unix.Getpid() != unix.Gettid() { + t.Skip(ErrNotTGLeader) + } + label := "system_u:object_r:container_t:s0:c1,c2" if err := SetKeyLabel(label); err != nil { t.Fatal(err) From 2a69eaf6b09e6e98ff6c8f62db67dea189d14804 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 11 Mar 2025 19:59:25 -0700 Subject: [PATCH 45/54] ci: add tests to check for races This is not the kind of race that Go race detector would catch; this is a race when a /proc file of one thread is opened and when Go changes the underlying thread, and that different thread tries to write to that file descriptor. In order to catch those reliably, we need quite a big number of iterations, and we only need to test go-selinux stuff. I chose the count to be 100000 because it takes about 1 minute on my machine: [kir@kir-tp1 selinux]$ time go test -timeout 3m -count 100000 ./go-selinux ok github.com/opencontainers/selinux/go-selinux 53.763s real 0m53.983s user 0m30.030s sys 0m30.339s Also note that this only makes sense to run on a SELinux enabled systems (i.e. those we run under lima). Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 6a87e61..1395498 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -140,6 +140,12 @@ jobs: continue-on-error: true run: lima make -C /tmp/selinux GOARCH=386 test + # https://github.com/opencontainers/selinux/issues/222 + # https://github.com/opencontainers/selinux/issues/225 + - name: "racy test" + continue-on-error: true + run: lima bash -c 'cd /tmp/selinux && go test -timeout 10m -count 100000 ./go-selinux' + - name: "Show AVC denials" run: lima sudo ausearch -m AVC,USER_AVC || true From 36bf2333ac15653bd480b310108e22c23a6755f4 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Mar 2025 19:35:49 -0700 Subject: [PATCH 46/54] label: don't capitalize error strings This fixes the following linter warnings: > go-selinux/label/label_linux.go:21:28: ST1005: error strings should not be capitalized (staticcheck) > var ErrIncompatibleLabel = errors.New("Bad SELinux option z and Z can not be used together") > ^ > go-selinux/label/label_linux.go:55:20: ST1005: error strings should not be capitalized (staticcheck) > return "", "", fmt.Errorf("Bad label option %q, valid options 'disable' or \n'user, role, level, type, filetype' followed by ':' and a value", opt) > ^ > go-selinux/label/label_linux.go:59:20: ST1005: error strings should not be capitalized (staticcheck) > return "", "", fmt.Errorf("Bad label option %q, valid options 'disable, user, role, level, type, filetype'", con[0]) > ^ Signed-off-by: Kir Kolyshkin --- go-selinux/label/label_linux.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go-selinux/label/label_linux.go b/go-selinux/label/label_linux.go index 1258c98..95f29e2 100644 --- a/go-selinux/label/label_linux.go +++ b/go-selinux/label/label_linux.go @@ -18,7 +18,7 @@ var validOptions = map[string]bool{ "level": true, } -var ErrIncompatibleLabel = errors.New("Bad SELinux option z and Z can not be used together") +var ErrIncompatibleLabel = errors.New("bad SELinux option: z and Z can not be used together") // InitLabels returns the process label and file labels to be used within // the container. A list of options can be passed into this function to alter @@ -52,11 +52,11 @@ func InitLabels(options []string) (plabel string, mlabel string, retErr error) { return "", selinux.PrivContainerMountLabel(), nil } if i := strings.Index(opt, ":"); i == -1 { - return "", "", fmt.Errorf("Bad label option %q, valid options 'disable' or \n'user, role, level, type, filetype' followed by ':' and a value", opt) + return "", "", fmt.Errorf("bad label option %q, valid options 'disable' or \n'user, role, level, type, filetype' followed by ':' and a value", opt) } con := strings.SplitN(opt, ":", 2) if !validOptions[con[0]] { - return "", "", fmt.Errorf("Bad label option %q, valid options 'disable, user, role, level, type, filetype'", con[0]) + return "", "", fmt.Errorf("bad label option %q, valid options 'disable, user, role, level, type, filetype'", con[0]) } if con[0] == "filetype" { mcon["type"] = con[1] From 0a30d59753bda334b90101255b9ecfe64dccb13b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 25 Mar 2025 19:38:35 -0700 Subject: [PATCH 47/54] ci: switch to golangci-lint v2 The configuration was migrated using golangci-lint migrate and when tweaked manually trying to minimize it. Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 12 +++++------ .golangci.yml | 39 +++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 1395498..d31d4b4 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -33,10 +33,10 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.22.x - - uses: golangci/golangci-lint-action@v6 + go-version: 1.24.x + - uses: golangci/golangci-lint-action@v7 with: - version: v1.64 + version: v2.0 codespell: runs-on: ubuntu-24.04 @@ -61,10 +61,10 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.22.x - - uses: golangci/golangci-lint-action@v6 + go-version: 1.24.x + - uses: golangci/golangci-lint-action@v7 with: - version: v1.64 + version: v2.0 - name: test-stubs run: make test diff --git a/.golangci.yml b/.golangci.yml index 2083fda..b1b9892 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,14 +1,15 @@ ---- -run: - concurrency: 6 - timeout: 5m +version: "2" + +formatters: + enable: + - gofumpt + linters: enable: # - copyloopvar # Detects places where loop variables are copied. TODO enable for Go 1.22+ - dupword # Detects duplicate words. - errorlint # Detects code that may cause problems with Go 1.13 error wrapping. - gocritic # Metalinter; detects bugs, performance, and styling issues. - - gofumpt # Detects whether code was gofumpt-ed. - gosec # Detects security problems. - misspell # Detects commonly misspelled English words in comments. - nilerr # Detects code that returns nil even if it checks that the error is not nil. @@ -20,16 +21,24 @@ linters: - tparallel # Detects inappropriate usage of t.Parallel(). - unconvert # Detects unnecessary type conversions. - usetesting # Reports uses of functions with replacement inside the testing package. -linters-settings: - govet: - enable-all: true - settings: - shadow: - strict: true + settings: + govet: + enable-all: true + settings: + shadow: + strict: true + exclusions: + generated: strict + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - govet + text: '^shadow: declaration of "err" shadows declaration' + issues: max-issues-per-linter: 0 max-same-issues: 0 - exclude-rules: - - text: '^shadow: declaration of "err" shadows declaration' - linters: - - govet From 1c8c9700599f22a37033aee75cc3b0fd9499492b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 12 Aug 2025 14:45:16 +0000 Subject: [PATCH 48/54] build(deps): bump actions/checkout from 4 to 5 Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/validate.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index d31d4b4..75dd37e 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -30,7 +30,7 @@ jobs: lint: runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: actions/setup-go@v5 with: go-version: 1.24.x @@ -41,7 +41,7 @@ jobs: codespell: runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: install deps # Version of codespell bundled with Ubuntu is way old, so use pip. run: pip install codespell @@ -51,14 +51,14 @@ jobs: cross: runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: cross run: make build-cross test-stubs: runs-on: macos-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: actions/setup-go@v5 with: go-version: 1.24.x @@ -76,7 +76,7 @@ jobs: race: ["-race", ""] runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: install go ${{ matrix.go-version }} uses: actions/setup-go@v5 @@ -101,7 +101,7 @@ jobs: - template://experimental/opensuse-tumbleweed runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: "Install Lima" uses: lima-vm/lima-actions/setup@v1 From 3c1bd9a95bfcc23ae12ed024d1bc9db9537b2d4f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 4 Sep 2025 12:33:08 +0000 Subject: [PATCH 49/54] build(deps): bump actions/setup-go from 5 to 6 Bumps [actions/setup-go](https://github.com/actions/setup-go) from 5 to 6. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](https://github.com/actions/setup-go/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/validate.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 75dd37e..fab1cb4 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -31,7 +31,7 @@ jobs: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v5 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@v6 with: go-version: 1.24.x - uses: golangci/golangci-lint-action@v7 @@ -59,7 +59,7 @@ jobs: runs-on: macos-latest steps: - uses: actions/checkout@v5 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@v6 with: go-version: 1.24.x - uses: golangci/golangci-lint-action@v7 @@ -79,7 +79,7 @@ jobs: - uses: actions/checkout@v5 - name: install go ${{ matrix.go-version }} - uses: actions/setup-go@v5 + uses: actions/setup-go@v6 with: go-version: ${{ matrix.go-version }} From 6ec194b9a845fa4a14cd6bd1c0458ddc44d21407 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sun, 22 Jun 2025 23:52:23 +1000 Subject: [PATCH 50/54] keyring: fix typo in EACCES check Commit 965323e01c4e ("SetKeyLabel: add thread group leader requirement") added verification that the caller of SetKeyLabel is the thread-group leader, however the check had a typo in it, which would almost always cause all errors to be treated as ErrNotTGLeader. It's a bit of a shame that os.Getuid() and os.Getpid() are untyped, as a one-character typo like this can really easily cause bugs without type checking... Signed-off-by: Aleksa Sarai --- go-selinux/selinux_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index a607980..0875205 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -735,7 +735,7 @@ func setKeyLabel(label string) error { if label == "" && errors.Is(err, os.ErrPermission) { return nil } - if errors.Is(err, unix.EACCES) && unix.Getuid() != unix.Gettid() { + if errors.Is(err, unix.EACCES) && unix.Getpid() != unix.Gettid() { return ErrNotTGLeader } return err From b42e5c8eff8eab7ee590cc61d78fd3e2d38e3309 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 7 Oct 2025 16:52:06 -0700 Subject: [PATCH 51/54] all: format sources with latest gofumpt A new rule was introduced in gofumpt v0.9.0 to "clothe" naked returns. Signed-off-by: Kir Kolyshkin --- pkg/pwalk/pwalk_test.go | 6 +++--- pkg/pwalkdir/pwalkdir_test.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/pwalk/pwalk_test.go b/pkg/pwalk/pwalk_test.go index f28fa28..9cca3b6 100644 --- a/pkg/pwalk/pwalk_test.go +++ b/pkg/pwalk/pwalk_test.go @@ -97,7 +97,7 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count uint32, err err var dir string dir, err = os.MkdirTemp(prefix, "d-") if err != nil { - return + return count, err } count++ for f := 0; f < files; f++ { @@ -114,12 +114,12 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count uint32, err err } var c uint32 if c, err = makeManyDirs(dir, levels-1, dirs, files); err != nil { - return + return count, err } count += c } - return + return count, err } // prepareTestSet() creates a directory tree of shallow files, diff --git a/pkg/pwalkdir/pwalkdir_test.go b/pkg/pwalkdir/pwalkdir_test.go index eb1653d..e66a80d 100644 --- a/pkg/pwalkdir/pwalkdir_test.go +++ b/pkg/pwalkdir/pwalkdir_test.go @@ -100,7 +100,7 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count uint32, err err var dir string dir, err = os.MkdirTemp(prefix, "d-") if err != nil { - return + return count, err } count++ for f := 0; f < files; f++ { @@ -117,12 +117,12 @@ func makeManyDirs(prefix string, levels, dirs, files int) (count uint32, err err } var c uint32 if c, err = makeManyDirs(dir, levels-1, dirs, files); err != nil { - return + return count, err } count += c } - return + return count, err } // prepareTestSet() creates a directory tree of shallow files, From 916cab932c940e0fc55f0c8404d503665160dd9c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 7 Oct 2025 16:43:07 -0700 Subject: [PATCH 52/54] ci: bump golangci-lint to v2.5 Also, bump golangci-lint-action to v8. Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index fab1cb4..ac559d9 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -34,9 +34,9 @@ jobs: - uses: actions/setup-go@v6 with: go-version: 1.24.x - - uses: golangci/golangci-lint-action@v7 + - uses: golangci/golangci-lint-action@v8 with: - version: v2.0 + version: v2.5 codespell: runs-on: ubuntu-24.04 @@ -62,9 +62,9 @@ jobs: - uses: actions/setup-go@v6 with: go-version: 1.24.x - - uses: golangci/golangci-lint-action@v7 + - uses: golangci/golangci-lint-action@v8 with: - version: v2.0 + version: v2.5 - name: test-stubs run: make test From 648ce7f0f85f4a310d1cd7317986fc1d6c8ff41c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 7 Oct 2025 16:44:57 -0700 Subject: [PATCH 53/54] ci: add go 1.25 Switch from go 1.24 to go 1.25 where we use a single go version. Drop go 1.23, add go 1.25 to the test matrix. (Note most testing is done in a VM job which uses whatever Go version is shipped with a distro). Signed-off-by: Kir Kolyshkin --- .github/workflows/validate.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index ac559d9..6604128 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -33,7 +33,7 @@ jobs: - uses: actions/checkout@v5 - uses: actions/setup-go@v6 with: - go-version: 1.24.x + go-version: 1.25.x - uses: golangci/golangci-lint-action@v8 with: version: v2.5 @@ -61,7 +61,7 @@ jobs: - uses: actions/checkout@v5 - uses: actions/setup-go@v6 with: - go-version: 1.24.x + go-version: 1.25.x - uses: golangci/golangci-lint-action@v8 with: version: v2.5 @@ -72,7 +72,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: [1.19.x, 1.23.x, 1.24.x] + go-version: [1.19.x, 1.24.x, 1.25.x] race: ["-race", ""] runs-on: ubuntu-24.04 steps: From c8cfa6fd2d285a96022203163c2075eda85bff54 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 23 Jun 2025 00:15:24 +1000 Subject: [PATCH 54/54] selinux: migrate to pathrs-lite procfs API The previous isProcHandle approach introduced in 03b517dc4fd5 ("selinux: verify that writes to /proc/... are on procfs") was a fairly naive solution to CVE-2019-16884 style bugs, as it only checked that the target was a procfs file without any verification what exact procfs file it is. A far more insidious attack (as discussed at the time) would be to instead bind-mount something like /proc/self/sched on top of /proc/self/attr/... which would not be detectable using a simple filesystem type check. The new pathrs-lite API (provided by filepath-securejoin) can correctly detect this and includes many other hardenings to avoid attacks of this kind. Fixes: CVE-2025-52881 Signed-off-by: Aleksa Sarai --- go-selinux/selinux.go | 10 +- go-selinux/selinux_linux.go | 266 +++++++++++++++++++++++++----------- go-selinux/selinux_stub.go | 12 +- go.mod | 7 +- go.sum | 12 +- 5 files changed, 210 insertions(+), 97 deletions(-) diff --git a/go-selinux/selinux.go b/go-selinux/selinux.go index 9f0740e..15150d4 100644 --- a/go-selinux/selinux.go +++ b/go-selinux/selinux.go @@ -153,7 +153,7 @@ func CalculateGlbLub(sourceRange, targetRange string) (string, error) { // of the program is finished to guarantee another goroutine does not migrate to the current // thread before execution is complete. func SetExecLabel(label string) error { - return writeCon(attrPath("exec"), label) + return writeConThreadSelf("attr/exec", label) } // SetTaskLabel sets the SELinux label for the current thread, or an error. @@ -161,7 +161,7 @@ func SetExecLabel(label string) error { // be wrapped in runtime.LockOSThread()/runtime.UnlockOSThread() to guarantee // the current thread does not run in a new mislabeled thread. func SetTaskLabel(label string) error { - return writeCon(attrPath("current"), label) + return writeConThreadSelf("attr/current", label) } // SetSocketLabel takes a process label and tells the kernel to assign the @@ -170,12 +170,12 @@ func SetTaskLabel(label string) error { // the socket is created to guarantee another goroutine does not migrate // to the current thread before execution is complete. func SetSocketLabel(label string) error { - return writeCon(attrPath("sockcreate"), label) + return writeConThreadSelf("attr/sockcreate", label) } // SocketLabel retrieves the current socket label setting func SocketLabel() (string, error) { - return readCon(attrPath("sockcreate")) + return readConThreadSelf("attr/sockcreate") } // PeerLabel retrieves the label of the client on the other side of a socket @@ -198,7 +198,7 @@ func SetKeyLabel(label string) error { // KeyLabel retrieves the current kernel keyring label setting func KeyLabel() (string, error) { - return readCon("/proc/self/attr/keycreate") + return keyLabel() } // Get returns the Context as a string diff --git a/go-selinux/selinux_linux.go b/go-selinux/selinux_linux.go index 0875205..6d7f8e2 100644 --- a/go-selinux/selinux_linux.go +++ b/go-selinux/selinux_linux.go @@ -17,8 +17,11 @@ import ( "strings" "sync" - "github.com/opencontainers/selinux/pkg/pwalkdir" + "github.com/cyphar/filepath-securejoin/pathrs-lite" + "github.com/cyphar/filepath-securejoin/pathrs-lite/procfs" "golang.org/x/sys/unix" + + "github.com/opencontainers/selinux/pkg/pwalkdir" ) const ( @@ -73,10 +76,6 @@ var ( mcsList: make(map[string]bool), } - // for attrPath() - attrPathOnce sync.Once - haveThreadSelf bool - // for policyRoot() policyRootOnce sync.Once policyRootVal string @@ -256,48 +255,183 @@ func readConfig(target string) string { return "" } -func isProcHandle(fh *os.File) error { - var buf unix.Statfs_t +func readConFd(in *os.File) (string, error) { + data, err := io.ReadAll(in) + if err != nil { + return "", err + } + return string(bytes.TrimSuffix(data, []byte{0})), nil +} - for { - err := unix.Fstatfs(int(fh.Fd()), &buf) - if err == nil { - break - } - if err != unix.EINTR { - return &os.PathError{Op: "fstatfs", Path: fh.Name(), Err: err} - } +func writeConFd(out *os.File, val string) error { + var err error + if val != "" { + _, err = out.Write([]byte(val)) + } else { + _, err = out.Write(nil) } - if buf.Type != unix.PROC_SUPER_MAGIC { - return fmt.Errorf("file %q is not on procfs", fh.Name()) + return err +} + +// openProcThreadSelf is a small wrapper around [procfs.Handle.OpenThreadSelf] +// and [pathrs.Reopen] to make "one-shot opens" slightly more ergonomic. The +// provided mode must be os.O_* flags to indicate what mode the returned file +// should be opened with (flags like os.O_CREAT and os.O_EXCL are not +// supported). +// +// If no error occurred, the returned handle is guaranteed to be exactly +// /proc/thread-self/ with no tricky mounts or symlinks causing you to +// operate on an unexpected path (with some caveats on pre-openat2 or +// pre-fsopen kernels). +func openProcThreadSelf(subpath string, mode int) (*os.File, procfs.ProcThreadSelfCloser, error) { + if subpath == "" { + return nil, nil, ErrEmptyPath } - return nil -} + proc, err := procfs.OpenProcRoot() + if err != nil { + return nil, nil, err + } + defer proc.Close() -func readCon(fpath string) (string, error) { - if fpath == "" { - return "", ErrEmptyPath + handle, closer, err := proc.OpenThreadSelf(subpath) + if err != nil { + return nil, nil, fmt.Errorf("open /proc/thread-self/%s handle: %w", subpath, err) + } + defer handle.Close() // we will return a re-opened handle + + file, err := pathrs.Reopen(handle, mode) + if err != nil { + closer() + return nil, nil, fmt.Errorf("reopen /proc/thread-self/%s handle (%#x): %w", subpath, mode, err) } + return file, closer, nil +} - in, err := os.Open(fpath) +// Read the contents of /proc/thread-self/. +func readConThreadSelf(fpath string) (string, error) { + in, closer, err := openProcThreadSelf(fpath, os.O_RDONLY|unix.O_CLOEXEC) if err != nil { return "", err } + defer closer() defer in.Close() - if err := isProcHandle(in); err != nil { + return readConFd(in) +} + +// Write to /proc/thread-self/. +func writeConThreadSelf(fpath, val string) error { + if val == "" { + if !getEnabled() { + return nil + } + } + + out, closer, err := openProcThreadSelf(fpath, os.O_WRONLY|unix.O_CLOEXEC) + if err != nil { + return err + } + defer closer() + defer out.Close() + + return writeConFd(out, val) +} + +// openProcSelf is a small wrapper around [procfs.Handle.OpenSelf] and +// [pathrs.Reopen] to make "one-shot opens" slightly more ergonomic. The +// provided mode must be os.O_* flags to indicate what mode the returned file +// should be opened with (flags like os.O_CREAT and os.O_EXCL are not +// supported). +// +// If no error occurred, the returned handle is guaranteed to be exactly +// /proc/self/ with no tricky mounts or symlinks causing you to +// operate on an unexpected path (with some caveats on pre-openat2 or +// pre-fsopen kernels). +func openProcSelf(subpath string, mode int) (*os.File, error) { + if subpath == "" { + return nil, ErrEmptyPath + } + + proc, err := procfs.OpenProcRoot() + if err != nil { + return nil, err + } + defer proc.Close() + + handle, err := proc.OpenSelf(subpath) + if err != nil { + return nil, fmt.Errorf("open /proc/self/%s handle: %w", subpath, err) + } + defer handle.Close() // we will return a re-opened handle + + file, err := pathrs.Reopen(handle, mode) + if err != nil { + return nil, fmt.Errorf("reopen /proc/self/%s handle (%#x): %w", subpath, mode, err) + } + return file, nil +} + +// Read the contents of /proc/self/. +func readConSelf(fpath string) (string, error) { + in, err := openProcSelf(fpath, os.O_RDONLY|unix.O_CLOEXEC) + if err != nil { return "", err } + defer in.Close() + return readConFd(in) } -func readConFd(in *os.File) (string, error) { - data, err := io.ReadAll(in) +// Write to /proc/self/. +func writeConSelf(fpath, val string) error { + if val == "" { + if !getEnabled() { + return nil + } + } + + out, err := openProcSelf(fpath, os.O_WRONLY|unix.O_CLOEXEC) if err != nil { - return "", err + return err } - return string(bytes.TrimSuffix(data, []byte{0})), nil + defer out.Close() + + return writeConFd(out, val) +} + +// openProcPid is a small wrapper around [procfs.Handle.OpenPid] and +// [pathrs.Reopen] to make "one-shot opens" slightly more ergonomic. The +// provided mode must be os.O_* flags to indicate what mode the returned file +// should be opened with (flags like os.O_CREAT and os.O_EXCL are not +// supported). +// +// If no error occurred, the returned handle is guaranteed to be exactly +// /proc/self/ with no tricky mounts or symlinks causing you to +// operate on an unexpected path (with some caveats on pre-openat2 or +// pre-fsopen kernels). +func openProcPid(pid int, subpath string, mode int) (*os.File, error) { + if subpath == "" { + return nil, ErrEmptyPath + } + + proc, err := procfs.OpenProcRoot() + if err != nil { + return nil, err + } + defer proc.Close() + + handle, err := proc.OpenPid(pid, subpath) + if err != nil { + return nil, fmt.Errorf("open /proc/%d/%s handle: %w", pid, subpath, err) + } + defer handle.Close() // we will return a re-opened handle + + file, err := pathrs.Reopen(handle, mode) + if err != nil { + return nil, fmt.Errorf("reopen /proc/%d/%s handle (%#x): %w", pid, subpath, mode, err) + } + return file, nil } // classIndex returns the int index for an object class in the loaded policy, @@ -393,78 +527,34 @@ func lFileLabel(fpath string) (string, error) { } func setFSCreateLabel(label string) error { - return writeCon(attrPath("fscreate"), label) + return writeConThreadSelf("attr/fscreate", label) } // fsCreateLabel returns the default label the kernel which the kernel is using // for file system objects created by this task. "" indicates default. func fsCreateLabel() (string, error) { - return readCon(attrPath("fscreate")) + return readConThreadSelf("attr/fscreate") } // currentLabel returns the SELinux label of the current process thread, or an error. func currentLabel() (string, error) { - return readCon(attrPath("current")) + return readConThreadSelf("attr/current") } // pidLabel returns the SELinux label of the given pid, or an error. func pidLabel(pid int) (string, error) { - return readCon(fmt.Sprintf("/proc/%d/attr/current", pid)) + it, err := openProcPid(pid, "attr/current", os.O_RDONLY|unix.O_CLOEXEC) + if err != nil { + return "", nil + } + defer it.Close() + return readConFd(it) } // ExecLabel returns the SELinux label that the kernel will use for any programs // that are executed by the current process thread, or an error. func execLabel() (string, error) { - return readCon(attrPath("exec")) -} - -func writeCon(fpath, val string) error { - if fpath == "" { - return ErrEmptyPath - } - if val == "" { - if !getEnabled() { - return nil - } - } - - out, err := os.OpenFile(fpath, os.O_WRONLY, 0) - if err != nil { - return err - } - defer out.Close() - - if err := isProcHandle(out); err != nil { - return err - } - - if val != "" { - _, err = out.Write([]byte(val)) - } else { - _, err = out.Write(nil) - } - if err != nil { - return err - } - return nil -} - -func attrPath(attr string) string { - // Linux >= 3.17 provides this - const threadSelfPrefix = "/proc/thread-self/attr" - - attrPathOnce.Do(func() { - st, err := os.Stat(threadSelfPrefix) - if err == nil && st.Mode().IsDir() { - haveThreadSelf = true - } - }) - - if haveThreadSelf { - return filepath.Join(threadSelfPrefix, attr) - } - - return filepath.Join("/proc/self/task", strconv.Itoa(unix.Gettid()), "attr", attr) + return readConThreadSelf("exec") } // canonicalizeContext takes a context string and writes it to the kernel @@ -728,7 +818,9 @@ func peerLabel(fd uintptr) (string, error) { // setKeyLabel takes a process label and tells the kernel to assign the // label to the next kernel keyring that gets created func setKeyLabel(label string) error { - err := writeCon("/proc/self/attr/keycreate", label) + // Rather than using /proc/thread-self, we want to use /proc/self to + // operate on the thread-group leader. + err := writeConSelf("attr/keycreate", label) if errors.Is(err, os.ErrNotExist) { return nil } @@ -741,6 +833,14 @@ func setKeyLabel(label string) error { return err } +// KeyLabel retrieves the current kernel keyring label setting for this +// thread-group. +func keyLabel() (string, error) { + // Rather than using /proc/thread-self, we want to use /proc/self to + // operate on the thread-group leader. + return readConSelf("attr/keycreate") +} + // get returns the Context as a string func (c Context) get() string { if l := c["level"]; l != "" { diff --git a/go-selinux/selinux_stub.go b/go-selinux/selinux_stub.go index 0889fbe..382244e 100644 --- a/go-selinux/selinux_stub.go +++ b/go-selinux/selinux_stub.go @@ -3,15 +3,11 @@ package selinux -func attrPath(string) string { - return "" -} - -func readCon(string) (string, error) { +func readConThreadSelf(string) (string, error) { return "", nil } -func writeCon(string, string) error { +func writeConThreadSelf(string, string) error { return nil } @@ -81,6 +77,10 @@ func setKeyLabel(string) error { return nil } +func keyLabel() (string, error) { + return "", nil +} + func (c Context) get() string { return "" } diff --git a/go.mod b/go.mod index 12f9f64..6d9e3b9 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,9 @@ module github.com/opencontainers/selinux go 1.19 -require golang.org/x/sys v0.1.0 +require ( + github.com/cyphar/filepath-securejoin v0.6.0 + golang.org/x/sys v0.26.0 +) + +require cyphar.com/go-pathrs v0.2.1 // indirect diff --git a/go.sum b/go.sum index b69ea85..432e727 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,10 @@ -golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= -golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +cyphar.com/go-pathrs v0.2.1 h1:9nx1vOgwVvX1mNBWDu93+vaceedpbsDqo+XuBGL40b8= +cyphar.com/go-pathrs v0.2.1/go.mod h1:y8f1EMG7r+hCuFf/rXsKqMJrJAUoADZGNh5/vZPKcGc= +github.com/cyphar/filepath-securejoin v0.6.0 h1:BtGB77njd6SVO6VztOHfPxKitJvd/VPT+OFBFMOi1Is= +github.com/cyphar/filepath-securejoin v0.6.0/go.mod h1:A8hd4EnAeyujCJRrICiOWqjS1AX0a9kM5XL+NwKoYSc= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= +golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=