Idea: Make Woodpecker CI pipelines run as the user that triggered it instead of the woodpecker-agent user

a commented

2022-08-22 21:54:26 +00:00

Owner

Currently, our Woodpecker CI instance is using the local backend (that I wrote) which doesn't use containers. However, all pipelines are run by the woodpecker-agent user, which is terrible for security since any pipeline can take over other pipelines or even the Woodpecker agent itself. To mitigate this, we have currently disabled access to the CI for exozyme users unless you have been manually whitelisted.

I have an idea for improving this. Instead of running pipelines using the woodpecker-agent user, the Woodpecker agent should first switch to the user who triggered it using sudo, and then run the pipeline. This also enables us to imitate Sourcehut CI's feature of being able to SSH into failed CI runs, since you can just SSH in and cd to the pipeline directory. We'll also be able to open up the CI to all exozyme users.

However, we still have to be careful about security, since this idea doesn't entirely fix the security problems with the local backend. I propose we use the following sudoers config to limit the security exposure of the woodpecker-agent:

woodpecker-agent ALL=(!root) NOPASSWD: ALL

Other possible security issues include that pipelines will now have access to all your files, so someone could make a malicious pipeline, send your repo a PR, then if you merge it, you'll run the malicious pipeline with full access to your account 😱. Our current system of running everything as woodpecker-agent avoids this, but it isn't much better since pipelines can access each others' files.

Another thing to worry about is if someone gains shell access to the woodpecker-agent after we implement this new idea. They will be able to become system users, but we might able able to mitigate this with runas_check_shell in sudoers.

I think this idea should be simple to implement once we solve all the security issues and will probably be just a few lines of code. Should we contribute this upstream to Woodpecker CI, or is this too exozyme-specific?

Currently, our Woodpecker CI instance is using the local backend (that I wrote) which doesn't use containers. However, all pipelines are run by the `woodpecker-agent` user, which is terrible for security since any pipeline can take over other pipelines or even the Woodpecker agent itself. To mitigate this, we have currently disabled access to the CI for exozyme users unless you have been manually whitelisted. I have an idea for improving this. Instead of running pipelines using the `woodpecker-agent` user, the Woodpecker agent should first switch to the user who triggered it using `sudo`, and then run the pipeline. This also enables us to imitate Sourcehut CI's feature of being able to SSH into failed CI runs, since you can just SSH in and `cd` to the pipeline directory. We'll also be able to open up the CI to all exozyme users. However, we still have to be careful about security, since this idea doesn't entirely fix the security problems with the local backend. I propose we use the following `sudoers` config to limit the security exposure of the `woodpecker-agent`: ``` woodpecker-agent ALL=(!root) NOPASSWD: ALL ``` Other possible security issues include that pipelines will now have access to all your files, so someone could make a malicious pipeline, send your repo a PR, then if you merge it, you'll run the malicious pipeline with full access to your account 😱. Our current system of running everything as `woodpecker-agent` avoids this, but it isn't much better since pipelines can access each others' files. Another thing to worry about is if someone gains shell access to the `woodpecker-agent` after we implement this new idea. They will be able to become system users, but we might able able to mitigate this with `runas_check_shell` in `sudoers`. I think this idea should be simple to implement once we solve all the security issues and will probably be just a few lines of code. Should we contribute this upstream to Woodpecker CI, or is this too exozyme-specific?

a added this to the v9.0 milestone 2022-08-22 21:54:26 +00:00

a added the

enhancement

help wanted

security

labels 2022-08-22 21:54:26 +00:00

a added this to the (deleted) project 2022-08-22 21:54:26 +00:00

a commented

2022-09-13 04:07:29 +00:00

Author

Owner

Does anyone want to take a stab at implementing this? It should be pretty easy and only require changing a few lines in the local backend code. We could also do it during a future hackathon.

Does anyone want to take a stab at implementing this? It should be pretty easy and only require changing a few lines in the [local backend code](https://github.com/woodpecker-ci/woodpecker/blob/master/pipeline/backend/local/local.go). We could also do it during a future hackathon.

a added a new dependency 2022-09-15 00:28:42 +00:00

#175 Ideas for improving exozyme security

a referenced this issue

2022-09-15 00:28:55 +00:00

Ideas for improving exozyme security #175

a commented

2022-09-25 17:50:52 +00:00

Author

Owner

Bad news:

int main() {
	setuid(0);
	system("whoami");
}

prints root if the binary has cap_setuid so setuid won't work without creating a security hole and we should use sudo instead.

Bad news: ```c int main() { setuid(0); system("whoami"); } ``` prints `root` if the binary has `cap_setuid` so `setuid` won't work without creating a security hole and we should use `sudo` instead.

a commented

2022-09-25 22:01:17 +00:00

Author

Owner

I finished this in today's hackathon. Here's the changeset:

diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go
index cc636fb99..25142d52e 100644
--- a/pipeline/backend/local/local.go
+++ b/pipeline/backend/local/local.go
@@ -15,7 +15,6 @@ import (
 type local struct {
        cmd        *exec.Cmd
        output     io.ReadCloser
-       workingdir string
 }
 
 // make sure local implements Engine
@@ -35,9 +34,7 @@ func (e *local) IsAvailable() bool {
 }
 
 func (e *local) Load() error {
-       dir, err := os.MkdirTemp("", "woodpecker-local-*")
-       e.workingdir = dir
-       return err
+       return nil
 }
 
 // Setup the pipeline environment.
@@ -52,14 +49,16 @@ func (e *local) Exec(ctx context.Context, proc *types.Step) error {
        for a, b := range proc.Environment {
                if a != "HOME" && a != "SHELL" { // Don't override $HOME and $SHELL
                        Env = append(Env, a+"="+b)
+               } else if a == "HOME" {
+                       Env = append(Env, "HOME=/home/"+proc.Environment["CI_COMMIT_AUTHOR"])
                }
        }
 
-       Command := []string{}
+       Command := []string{"sudo", "-u", proc.Environment["CI_COMMIT_AUTHOR"]}
        if proc.Image == constant.DefaultCloneImage {
                // Default clone step
-               Env = append(Env, "CI_WORKSPACE="+e.workingdir+"/"+proc.Environment["CI_REPO"])
-               Command = append(Command, "plugin-git")
+               Env = append(Env, "CI_WORKSPACE=/tmp/"+proc.Environment["CI_REPO"])
+               Command = append(Command, "-E", "plugin-git")
        } else {
                // Use "image name" as run command
                Command = append(Command, proc.Image)
@@ -77,14 +76,11 @@ func (e *local) Exec(ctx context.Context, proc *types.Step) error {
 
        // Prepare working directory
        if proc.Image == constant.DefaultCloneImage {
-               e.cmd.Dir = e.workingdir + "/" + proc.Environment["CI_REPO_OWNER"]
+               e.cmd.Dir = "/tmp/" + proc.Environment["CI_REPO_OWNER"]
        } else {
-               e.cmd.Dir = e.workingdir + "/" + proc.Environment["CI_REPO"]
-       }
-       err := os.MkdirAll(e.cmd.Dir, 0o700)
-       if err != nil {
-               return err
+               e.cmd.Dir = "/tmp/" + proc.Environment["CI_REPO"]
        }
+       exec.Command("sudo", "-u", proc.Environment["CI_COMMIT_AUTHOR"], "mkdir", "-p", e.cmd.Dir).Run()
        // Get output and redirect Stderr to Stdout
        e.output, _ = e.cmd.StdoutPipe()
        e.cmd.Stderr = e.cmd.Stdout

We also need to rewrite the CI wiki article, but that's another issue. Enjoy the new CI and please report bugs!

I finished this in today's hackathon. Here's the changeset: ```go diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go index cc636fb99..25142d52e 100644 --- a/pipeline/backend/local/local.go +++ b/pipeline/backend/local/local.go @@ -15,7 +15,6 @@ import ( type local struct { cmd *exec.Cmd output io.ReadCloser - workingdir string } // make sure local implements Engine @@ -35,9 +34,7 @@ func (e *local) IsAvailable() bool { } func (e *local) Load() error { - dir, err := os.MkdirTemp("", "woodpecker-local-*") - e.workingdir = dir - return err + return nil } // Setup the pipeline environment. @@ -52,14 +49,16 @@ func (e *local) Exec(ctx context.Context, proc *types.Step) error { for a, b := range proc.Environment { if a != "HOME" && a != "SHELL" { // Don't override $HOME and $SHELL Env = append(Env, a+"="+b) + } else if a == "HOME" { + Env = append(Env, "HOME=/home/"+proc.Environment["CI_COMMIT_AUTHOR"]) } } - Command := []string{} + Command := []string{"sudo", "-u", proc.Environment["CI_COMMIT_AUTHOR"]} if proc.Image == constant.DefaultCloneImage { // Default clone step - Env = append(Env, "CI_WORKSPACE="+e.workingdir+"/"+proc.Environment["CI_REPO"]) - Command = append(Command, "plugin-git") + Env = append(Env, "CI_WORKSPACE=/tmp/"+proc.Environment["CI_REPO"]) + Command = append(Command, "-E", "plugin-git") } else { // Use "image name" as run command Command = append(Command, proc.Image) @@ -77,14 +76,11 @@ func (e *local) Exec(ctx context.Context, proc *types.Step) error { // Prepare working directory if proc.Image == constant.DefaultCloneImage { - e.cmd.Dir = e.workingdir + "/" + proc.Environment["CI_REPO_OWNER"] + e.cmd.Dir = "/tmp/" + proc.Environment["CI_REPO_OWNER"] } else { - e.cmd.Dir = e.workingdir + "/" + proc.Environment["CI_REPO"] - } - err := os.MkdirAll(e.cmd.Dir, 0o700) - if err != nil { - return err + e.cmd.Dir = "/tmp/" + proc.Environment["CI_REPO"] } + exec.Command("sudo", "-u", proc.Environment["CI_COMMIT_AUTHOR"], "mkdir", "-p", e.cmd.Dir).Run() // Get output and redirect Stderr to Stdout e.output, _ = e.cmd.StdoutPipe() e.cmd.Stderr = e.cmd.Stdout ``` We also need to rewrite the [CI wiki article](https://git.exozy.me/exozyme/exozyme/wiki/CI), but that's another issue. Enjoy the new CI and please report bugs!

a closed this issue

2022-09-25 22:01:17 +00:00

a commented

2022-09-25 22:31:03 +00:00

Author

Owner

Updated diff:

diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go
index cc636fb99..aebd73576 100644
--- a/pipeline/backend/local/local.go
+++ b/pipeline/backend/local/local.go
@@ -15,7 +15,6 @@ import (
 type local struct {
        cmd        *exec.Cmd
        output     io.ReadCloser
-       workingdir string
 }
 
 // make sure local implements Engine
@@ -35,9 +34,7 @@ func (e *local) IsAvailable() bool {
 }
 
 func (e *local) Load() error {
-       dir, err := os.MkdirTemp("", "woodpecker-local-*")
-       e.workingdir = dir
-       return err
+       return nil
 }
 
 // Setup the pipeline environment.
@@ -49,17 +46,20 @@ func (e *local) Setup(ctx context.Context, proc *types.Config) error {
 func (e *local) Exec(ctx context.Context, proc *types.Step) error {
        // Get environment variables
        Env := os.Environ()
-       for a, b := range proc.Environment {
-               if a != "HOME" && a != "SHELL" { // Don't override $HOME and $SHELL
-                       Env = append(Env, a+"="+b)
+       for i := 0; i < len(Env); i++ {
+               if strings.HasPrefix(Env[i], "HOME") {
+                       Env[i] = "HOME=/home/"+proc.Environment["CI_COMMIT_AUTHOR"]
                }
        }
+       for a, b := range proc.Environment {
+               Env = append(Env, a+"="+b)
+       }
 
-       Command := []string{}
+       Command := []string{"sudo", "-u", proc.Environment["CI_COMMIT_AUTHOR"]}
        if proc.Image == constant.DefaultCloneImage {
                // Default clone step
-               Env = append(Env, "CI_WORKSPACE="+e.workingdir+"/"+proc.Environment["CI_REPO"])
-               Command = append(Command, "plugin-git")
+               Env = append(Env, "CI_WORKSPACE=/tmp/"+proc.Environment["CI_COMMIT_AUTHOR"]+"/"+proc.Environment["CI_REPO_NAME"])
+               Command = append(Command, "-E", "plugin-git")
        } else {
                // Use "image name" as run command
                Command = append(Command, proc.Image)
@@ -76,15 +76,11 @@ func (e *local) Exec(ctx context.Context, proc *types.Step) error {
        e.cmd.Env = Env
 
        // Prepare working directory
-       if proc.Image == constant.DefaultCloneImage {
-               e.cmd.Dir = e.workingdir + "/" + proc.Environment["CI_REPO_OWNER"]
-       } else {
-               e.cmd.Dir = e.workingdir + "/" + proc.Environment["CI_REPO"]
-       }
-       err := os.MkdirAll(e.cmd.Dir, 0o700)
-       if err != nil {
-               return err
+       e.cmd.Dir = "/tmp/" + proc.Environment["CI_COMMIT_AUTHOR"]
+       if proc.Image != constant.DefaultCloneImage {
+               e.cmd.Dir += "/" + proc.Environment["CI_REPO_NAME"]
        }
+       exec.Command("sudo", "-u", proc.Environment["CI_COMMIT_AUTHOR"], "mkdir", "-p", e.cmd.Dir).Run()
        // Get output and redirect Stderr to Stdout
        e.output, _ = e.cmd.StdoutPipe()
        e.cmd.Stderr = e.cmd.Stdout
@@ -115,5 +111,5 @@ func (e *local) Tail(context.Context, *types.Step) (io.ReadCloser, error) {
 
 // Destroy the pipeline environment.
 func (e *local) Destroy(context.Context, *types.Config) error {
-       return os.RemoveAll(e.cmd.Dir)
+       return nil
 }

Does anyone think we should upstream this?

Updated diff: ```go diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go index cc636fb99..aebd73576 100644 --- a/pipeline/backend/local/local.go +++ b/pipeline/backend/local/local.go @@ -15,7 +15,6 @@ import ( type local struct { cmd *exec.Cmd output io.ReadCloser - workingdir string } // make sure local implements Engine @@ -35,9 +34,7 @@ func (e *local) IsAvailable() bool { } func (e *local) Load() error { - dir, err := os.MkdirTemp("", "woodpecker-local-*") - e.workingdir = dir - return err + return nil } // Setup the pipeline environment. @@ -49,17 +46,20 @@ func (e *local) Setup(ctx context.Context, proc *types.Config) error { func (e *local) Exec(ctx context.Context, proc *types.Step) error { // Get environment variables Env := os.Environ() - for a, b := range proc.Environment { - if a != "HOME" && a != "SHELL" { // Don't override $HOME and $SHELL - Env = append(Env, a+"="+b) + for i := 0; i < len(Env); i++ { + if strings.HasPrefix(Env[i], "HOME") { + Env[i] = "HOME=/home/"+proc.Environment["CI_COMMIT_AUTHOR"] } } + for a, b := range proc.Environment { + Env = append(Env, a+"="+b) + } - Command := []string{} + Command := []string{"sudo", "-u", proc.Environment["CI_COMMIT_AUTHOR"]} if proc.Image == constant.DefaultCloneImage { // Default clone step - Env = append(Env, "CI_WORKSPACE="+e.workingdir+"/"+proc.Environment["CI_REPO"]) - Command = append(Command, "plugin-git") + Env = append(Env, "CI_WORKSPACE=/tmp/"+proc.Environment["CI_COMMIT_AUTHOR"]+"/"+proc.Environment["CI_REPO_NAME"]) + Command = append(Command, "-E", "plugin-git") } else { // Use "image name" as run command Command = append(Command, proc.Image) @@ -76,15 +76,11 @@ func (e *local) Exec(ctx context.Context, proc *types.Step) error { e.cmd.Env = Env // Prepare working directory - if proc.Image == constant.DefaultCloneImage { - e.cmd.Dir = e.workingdir + "/" + proc.Environment["CI_REPO_OWNER"] - } else { - e.cmd.Dir = e.workingdir + "/" + proc.Environment["CI_REPO"] - } - err := os.MkdirAll(e.cmd.Dir, 0o700) - if err != nil { - return err + e.cmd.Dir = "/tmp/" + proc.Environment["CI_COMMIT_AUTHOR"] + if proc.Image != constant.DefaultCloneImage { + e.cmd.Dir += "/" + proc.Environment["CI_REPO_NAME"] } + exec.Command("sudo", "-u", proc.Environment["CI_COMMIT_AUTHOR"], "mkdir", "-p", e.cmd.Dir).Run() // Get output and redirect Stderr to Stdout e.output, _ = e.cmd.StdoutPipe() e.cmd.Stderr = e.cmd.Stdout @@ -115,5 +111,5 @@ func (e *local) Tail(context.Context, *types.Step) (io.ReadCloser, error) { // Destroy the pipeline environment. func (e *local) Destroy(context.Context, *types.Config) error { - return os.RemoveAll(e.cmd.Dir) + return nil } ``` Does anyone think we should upstream this?

fogti commented

2022-09-27 20:10:59 +00:00

Member

regarding your setuid question: https://www.oreilly.com/library/view/secure-programming-cookbook/0596003943/ch01s03.html (has example C code at the end)

a commented

2022-09-27 20:16:00 +00:00

Author

Owner

regarding your setuid question: https://www.oreilly.com/library/view/secure-programming-cookbook/0596003943/ch01s03.html (has example C code at the end)

The problem is that the woodpecker-agent process can't just drop its setuid privileges at some point. It needs it at all times so it can run pipelines as other users.

We solved this instead using sudo, since sudo can be configured to allow woodpecker-agent to run pipelines as normal users but not root.

> regarding your setuid question: https://www.oreilly.com/library/view/secure-programming-cookbook/0596003943/ch01s03.html (has example C code at the end) The problem is that the `woodpecker-agent` process can't just drop its setuid privileges at some point. It needs it at all times so it can run pipelines as other users. We solved this instead using `sudo`, since `sudo` can be configured to allow `woodpecker-agent` to run pipelines as normal users but not root.

fogti commented

2022-09-27 20:23:07 +00:00

Member

Could we put the woodpecker CI agent into a separate mount namespace, and present it with a limited view of users $HOME's, so that it can access their binaries (e.g. Nix stuff or so), but configs and everything is at least guarded so that exploits injected into a pipeline of a user can just mangle the pipelines of that user, but can't e.g. inject malicious code into that user's bashrc or so. Another question would be if we can prevent other ways of e.g. keylogging using pipelines.
A way to reduce/avoid VM startup costs would be just letting the woodpecker CI run in a separate container, where users can log into, but the CI $HOME's would use this patch, but also be separate (the user should be able to provide a env init script, which initializes e.g. $PATH), so that malicious pipelines can't mess with anything except other pipelines, but the user can still log into the pipeline VM and e.g. test stuff. The "malicious pipeline" stuff with PRs applies anyways, but that problem is inherent to CIs (e.g. think about secrets to which CIs often have access), and happens to all of them (e.g. afaik happened already with some GitHub, GitLab projects, etc.).

Could we put the woodpecker CI agent into a separate mount namespace, and present it with a limited view of users $HOME's, so that it can access their binaries (e.g. Nix stuff or so), but configs and everything is at least guarded so that exploits injected into a pipeline of a user can just mangle the pipelines of that user, but can't e.g. inject malicious code into that user's bashrc or so. Another question would be if we can prevent other ways of e.g. keylogging using pipelines. A way to reduce/avoid VM startup costs would be just letting the woodpecker CI run in a separate container, where users can log into, but the CI $HOME's would use this patch, but also be separate (the user should be able to provide a env init script, which initializes e.g. `$PATH`), so that malicious pipelines can't mess with anything except other pipelines, but the user can still log into the pipeline VM and e.g. test stuff. The "malicious pipeline" stuff with PRs applies anyways, but that problem is inherent to CIs (e.g. think about secrets to which CIs often have access), and happens to all of them (e.g. afaik happened already with some GitHub, GitLab projects, etc.).

a commented

2022-09-27 23:54:07 +00:00

Author

Owner

I'm sure we could use namespaces or containers to isolate pipelines so they can't do much harm, but on the other hand I don't really want any complicated or arbitrary restrictions on what pipelines can do. It kind of reminds me of https://xkcd.com/2044/

Also, I'm assuming that I trust my pipelines not to do malicious things since I'm writing my own pipelines, but yeah if someone sends me a PR to modify my project's Woodpecker pipeline, I would have to look at it in detail to ensure it's not malicious.

I'm sure we could use namespaces or containers to isolate pipelines so they can't do much harm, but on the other hand I don't really want any complicated or arbitrary restrictions on what pipelines can do. It kind of reminds me of https://xkcd.com/2044/ Also, I'm assuming that I trust my pipelines not to do malicious things since I'm writing my own pipelines, but yeah if someone sends me a PR to modify my project's Woodpecker pipeline, I would have to look at it in detail to ensure it's not malicious.

a commented

2022-09-28 00:01:00 +00:00

Author

Owner

I just realized there's a possible vulnerability in our CI: if you push a commit impersonating another user, Woodpecker might run arbitrary code as that user!

OK, so I tried exploiting this, and fortunately that doesn't actually seem to happen. Gitea still knows the pusher of the commit and Woodpecker runs the pipeline as the pusher, not the user that supposedly created the commit.

I just realized there's a possible vulnerability in our CI: if you push a commit impersonating another user, Woodpecker might run arbitrary code as that user! OK, so I tried [exploiting this](https://ci.exozy.me/a/Hello-world/build/26), and fortunately that doesn't actually seem to happen. Gitea still knows the pusher of the commit and Woodpecker runs the pipeline as the pusher, not the user that supposedly created the commit.

Idea: Make Woodpecker CI pipelines run as the user that triggered it instead of the woodpecker-agent user #181