Commit 3d2795d9c2
Verified · cmc ci/build: success ci/test: success
Layout: unified · split
.gitbay/wiki/Threat-Model.org +7 −3
| @@ -130,9 +130,13 @@ runner, polling over SSH, clones the commit and runs its steps. | |||
| 130 | no secrets, so a stranger's branch cannot read the target's deploy | 130 | no secrets, so a stranger's branch cannot read the target's deploy |
| 131 | credentials. The step environment is *constructed*, not inherited: a | 131 | credentials. The step environment is *constructed*, not inherited: a |
| 132 | build gets =PATH=, =HOME=, =LANG=, =CI=, its own variables and its | 132 | build gets =PATH=, =HOME=, =LANG=, =CI=, its own variables and its |
| 133 | secrets, and nothing the operator set on the service. =HOME= is the | 133 | secrets, and nothing the operator set on the service. =HOME= is a build |
| 134 | workspace, so a build cannot read the runner's =.netrc=, =.npmrc= or | 134 | home under the runner's =-workdir=, not the runner's own home, so a |
| 135 | =.gitconfig=, where tools keep credentials. | 135 | build cannot read the =.netrc=, =.npmrc= or =.gitconfig= where tools |
| 136 | keep credentials. That home is shared by every build on the runner — | ||
| 137 | one build can poison a cache another reads, which is no more than | ||
| 138 | anything a step can already do as this user, and is what isolation | ||
| 139 | (krz/gitbay#144) is for. | ||
| 136 | - *Where it runs.* Steps run as the runner's own user on the runner | 140 | - *Where it runs.* Steps run as the runner's own user on the runner |
| 137 | host, with no container; the systemd drop-in adds =NoNewPrivileges=, | 141 | host, with no container; the systemd drop-in adds =NoNewPrivileges=, |
| 138 | =ProtectSystem=full= and the kernel and cgroup protections. =-repos= | 142 | =ProtectSystem=full= and the kernel and cgroup protections. =-repos= |
cmd/gitbay-runner/env_test.go +21 −7
| @@ -12,7 +12,7 @@ func TestStepEnvDoesNotInherit(t *testing.T) { | |||
| 12 | t.Setenv("GITBAY_RUNNER_TOKEN", "a-secret-the-service-was-given") | 12 | t.Setenv("GITBAY_RUNNER_TOKEN", "a-secret-the-service-was-given") |
| 13 | t.Setenv("AWS_SECRET_ACCESS_KEY", "also-not-for-builds") | 13 | t.Setenv("AWS_SECRET_ACCESS_KEY", "also-not-for-builds") |
| 14 | 14 | ||
| 15 | env := stepEnv(job{Repo: "alice/app", SHA: "abc", Ref: "main", Job: "test"}, "/tmp/ws") | 15 | env := stepEnv(job{Repo: "alice/app", SHA: "abc", Ref: "main", Job: "test"}, "/tmp/buildhome") |
| 16 | 16 | ||
| 17 | for _, e := range env { | 17 | for _, e := range env { |
| 18 | if strings.HasPrefix(e, "GITBAY_RUNNER_TOKEN=") || strings.HasPrefix(e, "AWS_SECRET_ACCESS_KEY=") { | 18 | if strings.HasPrefix(e, "GITBAY_RUNNER_TOKEN=") || strings.HasPrefix(e, "AWS_SECRET_ACCESS_KEY=") { |
| @@ -22,9 +22,11 @@ func TestStepEnvDoesNotInherit(t *testing.T) { | |||
| 22 | want := map[string]string{ | 22 | want := map[string]string{ |
| 23 | "CI": "true", "GITBAY_REPO": "alice/app", "GITBAY_SHA": "abc", | 23 | "CI": "true", "GITBAY_REPO": "alice/app", "GITBAY_SHA": "abc", |
| 24 | "GITBAY_REF": "main", "GITBAY_JOB": "test", | 24 | "GITBAY_REF": "main", "GITBAY_JOB": "test", |
| 25 | // HOME is the workspace so a build cannot read the runner's | 25 | // HOME is the shared build home, not the runner's own, so a |
| 26 | // dotfiles, where tools keep credentials. | 26 | // build cannot read the dotfiles where tools keep credentials — |
| 27 | "HOME": "/tmp/ws", | 27 | // and not the workspace, which is deleted after every build, |
| 28 | // taking every tool cache with it. | ||
| 29 | "HOME": "/tmp/buildhome", | ||
| 28 | } | 30 | } |
| 29 | got := map[string]string{} | 31 | got := map[string]string{} |
| 30 | for _, e := range env { | 32 | for _, e := range env { |
| @@ -44,11 +46,11 @@ func TestStepEnvDoesNotInherit(t *testing.T) { | |||
| 44 | // Secrets are passed through when the server sent them, which it does | 46 | // Secrets are passed through when the server sent them, which it does |
| 45 | // only for a trusted build. | 47 | // only for a trusted build. |
| 46 | func TestStepEnvCarriesSecrets(t *testing.T) { | 48 | func TestStepEnvCarriesSecrets(t *testing.T) { |
| 47 | env := stepEnv(job{Secrets: map[string]string{"TOKEN": "s3cret"}}, "/tmp/ws") | 49 | env := stepEnv(job{Secrets: map[string]string{"TOKEN": "s3cret"}}, "/tmp/buildhome") |
| 48 | if !containsEnv(env, "TOKEN=s3cret") { | 50 | if !containsEnv(env, "TOKEN=s3cret") { |
| 49 | t.Error("a trusted build's secret did not reach the step") | 51 | t.Error("a trusted build's secret did not reach the step") |
| 50 | } | 52 | } |
| 51 | env = stepEnv(job{}, "/tmp/ws") | 53 | env = stepEnv(job{}, "/tmp/buildhome") |
| 52 | for _, e := range env { | 54 | for _, e := range env { |
| 53 | if strings.HasPrefix(e, "TOKEN=") { | 55 | if strings.HasPrefix(e, "TOKEN=") { |
| 54 | t.Errorf("a secret appeared with none sent: %q", e) | 56 | t.Errorf("a secret appeared with none sent: %q", e) |
| @@ -61,7 +63,7 @@ func TestStepEnvPathFallback(t *testing.T) { | |||
| 61 | old := os.Getenv("PATH") | 63 | old := os.Getenv("PATH") |
| 62 | os.Unsetenv("PATH") | 64 | os.Unsetenv("PATH") |
| 63 | defer os.Setenv("PATH", old) | 65 | defer os.Setenv("PATH", old) |
| 64 | if env := stepEnv(job{}, "/tmp/ws"); !containsEnv(env, "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") { | 66 | if env := stepEnv(job{}, "/tmp/buildhome"); !containsEnv(env, "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin") { |
| 65 | t.Errorf("no PATH fallback: %v", env) | 67 | t.Errorf("no PATH fallback: %v", env) |
| 66 | } | 68 | } |
| 67 | } | 69 | } |
| @@ -74,3 +76,15 @@ func containsEnv(env []string, want string) bool { | |||
| 74 | } | 76 | } |
| 75 | return false | 77 | return false |
| 76 | } | 78 | } |
| 79 | |||
| 80 | // The build home must outlive a build. It was briefly the workspace, | ||
| 81 | // which run() removes when the build ends, so every build re-downloaded | ||
| 82 | // the Go module cache and the ~50MB sonar scanner. | ||
| 83 | func TestStepEnvHomeIsNotTheWorkspace(t *testing.T) { | ||
| 84 | env := stepEnv(job{ID: 7}, "/var/lib/gitbay-runner/work/home") | ||
| 85 | for _, e := range env { | ||
| 86 | if strings.HasPrefix(e, "HOME=") && strings.Contains(e, "build-7") { | ||
| 87 | t.Errorf("HOME is the per-build workspace, which is deleted after the build: %q", e) | ||
| 88 | } | ||
| 89 | } | ||
| 90 | } | ||
cmd/gitbay-runner/main.go +26 −7
| @@ -203,6 +203,24 @@ func (r *runner) run(j job) bool { | |||
| 203 | dir := filepath.Join(r.workdir, fmt.Sprintf("build-%d", j.ID)) | 203 | dir := filepath.Join(r.workdir, fmt.Sprintf("build-%d", j.ID)) |
| 204 | defer os.RemoveAll(dir) | 204 | defer os.RemoveAll(dir) |
| 205 | 205 | ||
| 206 | // A build's HOME. Not the workspace, which is removed after every | ||
| 207 | // build: the Go module cache, the sonar scanner and every other tool | ||
| 208 | // cache live under HOME, so a per-build one re-downloads the world | ||
| 209 | // each time. Not the runner's own home either, where its SSH key and | ||
| 210 | // credential dotfiles are. A directory beside the workspaces is | ||
| 211 | // neither. | ||
| 212 | // | ||
| 213 | // It is shared by every build on this runner, so a step can poison a | ||
| 214 | // cache another repository's build will read. That is already true of | ||
| 215 | // anything a step can reach as this user — see the wiki's | ||
| 216 | // Threat-Model on the runner — and is what container isolation (#144) | ||
| 217 | // is for; -repos is the control until then. | ||
| 218 | buildHome := filepath.Join(r.workdir, "home") | ||
| 219 | if err := os.MkdirAll(buildHome, 0o700); err != nil { | ||
| 220 | log.Printf("build %d: build home: %v", j.ID, err) | ||
| 221 | return false | ||
| 222 | } | ||
| 223 | |||
| 206 | // One long-lived `runner log` session receives the whole stream. | 224 | // One long-lived `runner log` session receives the whole stream. |
| 207 | logCmd := exec.Command(toolpath.Look("ssh"), append(r.sshOpts, r.remote, "runner", "log", fmt.Sprint(j.ID))...) | 225 | logCmd := exec.Command(toolpath.Look("ssh"), append(r.sshOpts, r.remote, "runner", "log", fmt.Sprint(j.ID))...) |
| 208 | pipe, err := logCmd.StdinPipe() | 226 | pipe, err := logCmd.StdinPipe() |
| @@ -304,7 +322,7 @@ func (r *runner) run(j job) bool { | |||
| 304 | } | 322 | } |
| 305 | } | 323 | } |
| 306 | 324 | ||
| 307 | env := stepEnv(j, dir) | 325 | env := stepEnv(j, buildHome) |
| 308 | for _, step := range j.Steps { | 326 | for _, step := range j.Steps { |
| 309 | fmt.Fprintf(sink, "$ %s\n", step) | 327 | fmt.Fprintf(sink, "$ %s\n", step) |
| 310 | cmd := exec.Command(toolpath.Look("sh"), "-c", step) | 328 | cmd := exec.Command(toolpath.Look("sh"), "-c", step) |
| @@ -324,21 +342,22 @@ func (r *runner) run(j job) bool { | |||
| 324 | // the runner's entire environment, including anything an operator set on | 342 | // the runner's entire environment, including anything an operator set on |
| 325 | // the service (#144). | 343 | // the service (#144). |
| 326 | // | 344 | // |
| 327 | // HOME is the workspace, not the runner's home. Tools read credentials | 345 | // HOME is a build home shared by this runner's builds, not the runner's |
| 328 | // out of dotfiles — .netrc, .npmrc, .gitconfig — and a build has no | 346 | // own: tools read credentials out of dotfiles — .netrc, .npmrc, |
| 329 | // business finding the runner's. It also means a build's caches land in | 347 | // .gitconfig — and a build has no business finding the runner's. It is |
| 330 | // the workspace and go away with it. | 348 | // not the workspace either, because the workspace is deleted after every |
| 349 | // build and every tool cache lives under HOME. | ||
| 331 | // | 350 | // |
| 332 | // PATH is the one thing carried over: without it a step cannot find the | 351 | // PATH is the one thing carried over: without it a step cannot find the |
| 333 | // tools the host was provisioned with. | 352 | // tools the host was provisioned with. |
| 334 | func stepEnv(j job, dir string) []string { | 353 | func stepEnv(j job, home string) []string { |
| 335 | path := os.Getenv("PATH") | 354 | path := os.Getenv("PATH") |
| 336 | if path == "" { | 355 | if path == "" { |
| 337 | path = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" | 356 | path = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" |
| 338 | } | 357 | } |
| 339 | env := []string{ | 358 | env := []string{ |
| 340 | "PATH=" + path, | 359 | "PATH=" + path, |
| 341 | "HOME=" + dir, | 360 | "HOME=" + home, |
| 342 | "LANG=C.UTF-8", | 361 | "LANG=C.UTF-8", |
| 343 | "CI=true", | 362 | "CI=true", |
| 344 | "GITBAY_REPO=" + j.Repo, | 363 | "GITBAY_REPO=" + j.Repo, |