Skip to content

Commit

Permalink
Merge pull request #1220 from buildkite/v5-backport-pr1219
Browse files Browse the repository at this point in the history
Backport: Replace Bash fix-permissions script with Go #1219
  • Loading branch information
DrJosh9000 committed Sep 20, 2023
2 parents 35a3fba + cd773c3 commit 944044b
Show file tree
Hide file tree
Showing 22 changed files with 537 additions and 280 deletions.
21 changes: 21 additions & 0 deletions .buildkite/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
version: '3'

services:
fixperms-tests:
image: golang:latest
working_dir: /code
environment:
CGO_ENABLED: 0
volumes:
- ..:/code:ro
command: go test -v ./...

fixperms-build:
image: golang:latest
working_dir: /code
environment:
CGO_ENABLED: 0
volumes:
- ..:/code
- /var/lib/buildkite-agent/git-mirrors:/var/lib/buildkite-agent/git-mirrors
command: .buildkite/steps/build-fixperms.sh
36 changes: 27 additions & 9 deletions .buildkite/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,28 @@ steps:
agents:
queue: "${BUILDKITE_AGENT_META_DATA_QUEUE}"

- id: "bats-tests"
name: ":bash: Unit tests"
- id: "fixperms-tests"
name: ":go: fixperms tests"
agents:
queue: "${BUILDKITE_AGENT_META_DATA_QUEUE}"
plugins:
docker-compose#v2.1.0:
run: unit-tests
config: docker-compose.unit-tests.yml
- docker-compose#v2.1.0:
run: fixperms-tests
config: .buildkite/docker-compose.yml

- id: "fixperms-build"
name: ":go: fixperms build"
agents:
queue: "${BUILDKITE_AGENT_META_DATA_QUEUE}"
depends_on:
- "fixperms-tests"
artifact_paths: "build/fix-perms-*"
plugins:
- docker-compose#v2.1.0:
run: fixperms-build
config: .buildkite/docker-compose.yml
- artifacts#v1.9.0:
upload: "builds/fix-perms-*"

- id: "deploy-service-role-stack"
name: ":aws-iam: :cloudformation:"
Expand All @@ -23,7 +37,8 @@ steps:
command: .buildkite/steps/deploy-service-role-stack.sh
depends_on:
- "lint"
- "bats-tests"
- "fixperms-tests"
- "fixperms-build"

- id: "packer-windows-amd64"
name: ":packer: :windows:"
Expand All @@ -34,7 +49,8 @@ steps:
queue: "${BUILDKITE_AGENT_META_DATA_QUEUE}"
depends_on:
- "lint"
- "bats-tests"
- "fixperms-tests"
- "fixperms-build"

- id: "launch-windows-amd64"
name: ":cloudformation: :windows: AMD64 Launch"
Expand Down Expand Up @@ -77,7 +93,8 @@ steps:
queue: "${BUILDKITE_AGENT_META_DATA_QUEUE}"
depends_on:
- "lint"
- "bats-tests"
- "fixperms-tests"
- "fixperms-build"

- id: "launch-linux-amd64"
name: ":cloudformation: :linux: AMD64 Launch"
Expand Down Expand Up @@ -119,7 +136,8 @@ steps:
queue: "${BUILDKITE_AGENT_META_DATA_QUEUE}"
depends_on:
- "lint"
- "bats-tests"
- "fixperms-tests"
- "fixperms-build"

- id: "launch-linux-arm64"
name: ":cloudformation: :linux: ARM64 Launch"
Expand Down
5 changes: 5 additions & 0 deletions .buildkite/steps/build-fixperms.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/usr/bin/env bash
set -euo pipefail
for arch in amd64 arm64; do
GOOS=linux GOARCH="${arch}" go build -v -o "build/fix-perms-linux-${arch}" ./internal/fixperms
done
6 changes: 6 additions & 0 deletions .buildkite/steps/packer.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ fi

mkdir -p "build/"

if [[ "$os" == "linux" ]] ; then
buildkite-agent artifact download "build/fix-perms-linux-${arch}" ./build
mv "build/fix-perms-linux-${arch}" packer/linux/conf/buildkite-agent/scripts/fix-buildkite-agent-builds-permissions
chmod 755 packer/linux/conf/buildkite-agent/scripts/fix-buildkite-agent-builds-permissions
fi

# Build a hash of packer files and the agent versions
packer_files_sha=$(find Makefile "packer/${os}" plugins/ -type f -print0 | xargs -0 sha1sum | awk '{print $1}' | sort | sha1sum | awk '{print $1}')
stable_agent_sha=$(curl -Lfs "https://download.buildkite.com/agent/stable/latest/${agent_binary}.sha256")
Expand Down
9 changes: 0 additions & 9 deletions docker-compose.unit-tests.yml

This file was deleted.

8 changes: 8 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module github.com/buildkite/elastic-ci-stack-for-aws/v5

go 1.20

require (
github.com/google/go-cmp v0.5.9
golang.org/x/sys v0.12.0
)
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o=
golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
122 changes: 122 additions & 0 deletions internal/fixperms/fdfs/fdfs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
//go:build linux

// Package fdfs is like os.DirFS, but with a file descriptor and openat(2),
// fchownat(2), etc, to ensure symlinks do not escape.
package fdfs

import (
"fmt"
"io/fs"
"os"

"golang.org/x/sys/unix"
)

const resolveFlags = unix.RESOLVE_BENEATH | unix.RESOLVE_NO_SYMLINKS | unix.RESOLVE_NO_MAGICLINKS | unix.RESOLVE_NO_XDEV

// FS uses a file descriptor for a directory as the base of a fs.FS.
type FS struct {
file *os.File
}

// DirFS opens the directory dir, and returns an FS rooted at that directory.
// It uses open(2) with O_RDONLY+O_DIRECTORY+O_CLOEXEC. Note that this will
// resolve symlinks in the path, so only use this to open a trusted base path.
func DirFS(dir string) (*FS, error) {
f, err := os.OpenFile(dir, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
if err != nil {
return nil, err
}
return &FS{file: f}, nil
}

// Close closes the file descriptor.
func (s *FS) Close() error {
return s.file.Close()
}

// Open wraps openat2(2) with O_RDONLY+O_NOFOLLOW+O_CLOEXEC, and prohibits
// symlinks etc within the path.
func (s *FS) Open(path string) (fs.File, error) {
fd, err := unix.Openat2(int(s.file.Fd()), path, &unix.OpenHow{
Flags: unix.O_RDONLY | unix.O_NOFOLLOW | unix.O_CLOEXEC,
Mode: 0,
Resolve: resolveFlags,
})
if err != nil {
return nil, fmt.Errorf("openat2(%d, %q): %w", s.file.Fd(), path, err)
}
return os.NewFile(uintptr(fd), path), nil
}

// Lchown wraps fchownat(2) (with AT_SYMLINK_NOFOLLOW).
func (s *FS) Lchown(path string, uid, gid int) error {
if err := unix.Fchownat(int(s.file.Fd()), path, uid, gid, unix.AT_SYMLINK_NOFOLLOW); err != nil {
return fmt.Errorf("fchownat(%d, %q, %d, %d): %w", s.file.Fd(), path, uid, gid, err)
}
return nil
}

// Sub wraps openat2(2) (with O_RDONLY+O_DIRECTORY+O_NOFOLLOW+O_CLOEXEC), and
// returns an FS.
func (s *FS) Sub(dir string) (*FS, error) {
fd, err := unix.Openat2(int(s.file.Fd()), dir, &unix.OpenHow{
Flags: unix.O_RDONLY | unix.O_DIRECTORY | unix.O_NOFOLLOW | unix.O_CLOEXEC,
Mode: 0,
Resolve: resolveFlags,
})
if err != nil {
return nil, fmt.Errorf("openat2(%d, %q): %w", s.file.Fd(), dir, err)
}
return &FS{os.NewFile(uintptr(fd), dir)}, nil
}

// RecursiveChown lchowns everything within the receiver.
func (s *FS) RecursiveChown(uid, gid int) error {
// Q: Why not fs.WalkDir(... s.Lchown(path, uid, gid) ... ) ?
// A: fs.WalkDir gives the callback a subpath to each item. So although
// fs.WalkDir doesn't traverse symlinks, there's a race between walking
// each path (no intermediate symlinks), and passing that path to lchown
// (has possibly changed).
// Solution: More openat.

if err := s.Lchown(".", uid, gid); err != nil {
return err
}

// This closure exists so sd.Close happens before the next loop iteration,
// rather than at the end of RecursiveChown.
chownSubdir := func(name string) error {
sd, err := s.Sub(name)
if err != nil {
return err
}
defer sd.Close()
return sd.RecursiveChown(uid, gid)
}

// The "file" within an *FS should always be a directory.
ds, err := s.file.ReadDir(-1)
if err != nil {
return err
}
for _, d := range ds {
if !d.IsDir() {
if err := s.Lchown(d.Name(), uid, gid); err != nil {
return err
}
continue
}

// Defensively check we're not about to recurse on a symlink.
// (The openat2 call in s.Sub will block it anyway.)
if d.Type()&fs.ModeSymlink != 0 {
continue
}

if err := chownSubdir(d.Name()); err != nil {
return err
}
}
return nil
}
61 changes: 61 additions & 0 deletions internal/fixperms/fdfs/fdfs_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//go:build linux

package fdfs

import (
"io/fs"
"os"
"path/filepath"
"testing"
)

func TestTOCTOUShenanigans(t *testing.T) {
path := "/tmp/TestTOCTOUShenanigans/foo"
if err := os.MkdirAll(path, 0o777); err != nil {
t.Fatalf("os.MkdirAll(%s, %o) = %v", path, 0o777, err)
}
fp := filepath.Join(path, "data")
if err := os.WriteFile(fp, []byte("innocent"), 0o666); err != nil {
t.Fatalf("os.WriteFile(%s, nil, 0o666) = %v", fp, err)
}

path2 := "/tmp/TestTOCTOUShenanigans/crimes"
if err := os.MkdirAll(path2, 0o777); err != nil {
t.Fatalf("os.MkdirAll(%s, %o) = %v", path2, 0o777, err)
}
fp2 := filepath.Join(path2, "data")
if err := os.WriteFile(fp2, []byte("guilty"), 0o666); err != nil {
t.Fatalf("os.WriteFile(%s, nil, 0o666) = %v", fp2, err)
}

// Do it in two steps, to simulate a trusted directory and an untrusted
// subpath.
fsys, err := DirFS("/tmp/TestTOCTOUShenanigans")
if err != nil {
t.Fatalf("DirFS(/tmp/TestTOCTOUShenanigans) error = %v", err)
}
defer fsys.Close()
fooFS, err := fsys.Sub("foo")
if err != nil {
t.Fatalf("DirFS(/tmp/TestTOCTOUShenanigans).Sub(foo) error = %v", err)
}
defer fooFS.Close()

// Replace foo with a symlink to crimes...
path3 := "/tmp/TestTOCTOUShenanigans/foo.bak"
if err := os.Rename(path, path3); err != nil {
t.Fatalf("os.Rename(%s, %s) = %v", path, path3, err)
}
if err := os.Symlink(path2, path); err != nil {
t.Fatalf("os.Symlink(%s, %s) = %v", path2, path, err)
}

// What do we get?
df, err := fs.ReadFile(fooFS, "data")
if err != nil {
t.Fatalf("fs.ReadFile(DirFS(%s), data) error = %v", path, err)
}
if got, want := string(df), "innocent"; got != want {
t.Fatalf("fs.ReadFile(DirFS(%s), data) contents = %q, want %q", path, got, want)
}
}

0 comments on commit 944044b

Please sign in to comment.