Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: disable running artifact downloader as nobody #16375

Merged
merged 4 commits into from
Mar 8, 2023
Merged

Conversation

shoenig
Copy link
Member

@shoenig shoenig commented Mar 7, 2023

This PR reverts a change from Nomad 1.5 where artifact downloads were
executed as the nobody user on Linux systems. This was done as an attempt
to improve the security model of artifact downloading where third party
tools such as git or mercurial would be run as the root user with all
the security implications thereof.

However, doing so conflicts with Nomad's own advice for securing the
Client data directory - which when setup with the recommended directory
permissions structure prevents artifact downloads from working as intended.

Artifact downloads are at least still now executed as a child process of
the Nomad agent, and on modern Linux systems make use of the kernel Landlock
feature for limiting filesystem access of the child process.

Note: the tests for this is really the bank of artifact e2e test cases

This PR reverts a change from Nomad 1.5 where artifact downloads were
executed as the nobody user on Linux systems. This was done as an attempt
to improve the security model of artifact downloading where third party
tools such as git or mercurial would be run as the root user with all
the security implications thereof.

However, doing so conflicts with Nomad's own advice for securing the
Client data directory - which when setup with the recommended directory
permissions structure prevents artifact downloads from working as intended.

Artifact downloads are at least still now executed as a child process of
the Nomad agent, and on modern Linux systems make use of the kernel Landlock
feature for limiting filesystem access of the child process.
@bfqrst
Copy link

bfqrst commented Mar 7, 2023

Are set_environment_variables and disable_filesystem_isolation artifact client config entries obsolete once this lands?

@shoenig
Copy link
Member Author

shoenig commented Mar 7, 2023

Hi @bfqrst, those two config values will still be relevant. The artifact downloader is still running as a child process with a limited environment - the set_environment_variables config still acts as before, allowing the passthrough of environment variables set on the Nomad Client. Likewise, on modern Linux systems the landlock kernel feature is still being used to restrict filesystem access to the child process, and that mechanism can be optionally disabled via the disable_filesystem_isolation configuration.

@shoenig
Copy link
Member Author

shoenig commented Mar 7, 2023

Spot check on an e2e instance created with #16399

job file
job "example" {

  group "cache" {
    network {
      port "db" {
        to = 6379
      }
    }

    task "redis" {
      driver = "docker"
      
      artifact {
        source = "https://github.com/shoenig/ssh-key-sync/releases/download/v1.7.0/ssh-key-sync_1.7.0_linux_amd64.tar.gz"
        destination = "local/"
      }

      config {
        image          = "redis:7"
        ports          = ["db"]
        auth_soft_fail = true
      }

      identity {
        env  = true
        file = true
      }

      resources {
        cpu    = 500
        memory = 256
      }
    }
  }
}
2023-03-08T15:02:00-06:00  Started                Task started by client
2023-03-08T15:01:55-06:00  Driver                 Downloading image
2023-03-08T15:01:54-06:00  Downloading Artifacts  Client is downloading artifacts
2023-03-08T15:01:54-06:00  Task Setup             Building Task Directory
2023-03-08T15:01:54-06:00  Received               Task received by client

node has expected directory structure

ubuntu@ip-172-31-92-1:~$ sudo tree -p -d -u --metafirst /opt/nomad
[drwxr-xr-x root    ]  /opt/nomad
[drwx------ root    ]  ├── data
[drwx--x--x root    ]  │   ├── alloc
[drwx------ root    ]  │   └── client
[drwxr-xr-x root    ]  └── plugins

Inspect that the downloaded artifact is owned by root

➜ nomad alloc exec f6 ls -lha /local/ssh-key-sync
-rwxr-xr-x 1 root root 4.8M Feb 12 16:35 /local/ssh-key-sync

And finally passing full artifact e2e suite

➜ go test -v -run TestArtifact
=== RUN   TestArtifact
=== RUN   TestArtifact/testLinux
=== RUN   TestArtifact/testLinux/rawexec_file_default
=== RUN   TestArtifact/testLinux/rawexec_file_custom
=== RUN   TestArtifact/testLinux/rawexec_file_alloc_dots
=== RUN   TestArtifact/testLinux/rawexec_file_alloc_env
=== RUN   TestArtifact/testLinux/rawexec_zip_default
=== RUN   TestArtifact/testLinux/rawexec_zip_custom
=== RUN   TestArtifact/testLinux/rawexec_git_custom
=== RUN   TestArtifact/testLinux/exec_file_default
=== RUN   TestArtifact/testLinux/exec_file_custom
=== RUN   TestArtifact/testLinux/exec_file_alloc
=== RUN   TestArtifact/testLinux/exec_zip_default
=== RUN   TestArtifact/testLinux/exec_zip_custom
=== RUN   TestArtifact/testLinux/exec_git_custom
=== RUN   TestArtifact/testLinux/docker_file_default
=== RUN   TestArtifact/testLinux/docker_file_custom
=== RUN   TestArtifact/testLinux/docker_file_alloc
=== RUN   TestArtifact/testLinux/docker_zip_default
=== RUN   TestArtifact/testLinux/docker_zip_custom
=== RUN   TestArtifact/testLinux/docker_git_custom
=== RUN   TestArtifact/testWindows
=== RUN   TestArtifact/testWindows/rawexec_file_default
=== RUN   TestArtifact/testWindows/rawexec_file_custom
=== RUN   TestArtifact/testWindows/rawexec_zip_default
=== RUN   TestArtifact/testWindows/rawexec_zip_custom
=== RUN   TestArtifact/testLimits
--- PASS: TestArtifact (75.40s)
    --- PASS: TestArtifact/testLinux (54.49s)
        --- PASS: TestArtifact/testLinux/rawexec_file_default (1.43s)
        --- PASS: TestArtifact/testLinux/rawexec_file_custom (1.52s)
        --- PASS: TestArtifact/testLinux/rawexec_file_alloc_dots (1.46s)
        --- PASS: TestArtifact/testLinux/rawexec_file_alloc_env (1.40s)
        --- PASS: TestArtifact/testLinux/rawexec_zip_default (1.44s)
        --- PASS: TestArtifact/testLinux/rawexec_zip_custom (1.59s)
        --- PASS: TestArtifact/testLinux/rawexec_git_custom (1.92s)
        --- PASS: TestArtifact/testLinux/exec_file_default (1.35s)
        --- PASS: TestArtifact/testLinux/exec_file_custom (1.35s)
        --- PASS: TestArtifact/testLinux/exec_file_alloc (1.39s)
        --- PASS: TestArtifact/testLinux/exec_zip_default (1.35s)
        --- PASS: TestArtifact/testLinux/exec_zip_custom (1.36s)
        --- PASS: TestArtifact/testLinux/exec_git_custom (1.36s)
        --- PASS: TestArtifact/testLinux/docker_file_default (1.36s)
        --- PASS: TestArtifact/testLinux/docker_file_custom (1.36s)
        --- PASS: TestArtifact/testLinux/docker_file_alloc (1.35s)
        --- PASS: TestArtifact/testLinux/docker_zip_default (1.35s)
        --- PASS: TestArtifact/testLinux/docker_zip_custom (1.38s)
        --- PASS: TestArtifact/testLinux/docker_git_custom (1.35s)
    --- PASS: TestArtifact/testWindows (13.39s)
        --- PASS: TestArtifact/testWindows/rawexec_file_default (1.35s)
        --- PASS: TestArtifact/testWindows/rawexec_file_custom (1.34s)
        --- PASS: TestArtifact/testWindows/rawexec_zip_default (1.34s)
        --- PASS: TestArtifact/testWindows/rawexec_zip_custom (1.35s)
    --- PASS: TestArtifact/testLimits (7.17s)
PASS
ok  	github.com/hashicorp/nomad/e2e/artifact	75.404s

@bfqrst
Copy link

bfqrst commented Mar 8, 2023

Hi @bfqrst, those two config values will still be relevant. The artifact downloader is still running as a child process with a limited environment - the set_environment_variables config still acts as before, allowing the passthrough of environment variables set on the Nomad Client. Likewise, on modern Linux systems the landlock kernel feature is still being used to restrict filesystem access to the child process, and that mechanism can be optionally disabled via the disable_filesystem_isolation configuration.

Thanks for the clarification @shoenig, that makes sense! And seeing the tree output of what the directory structure should optimally look like is useful too (to me anyway)!

@shoenig shoenig marked this pull request as ready for review March 8, 2023 21:13
@shoenig shoenig requested review from tgross and lgfa29 March 8, 2023 21:13
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@jeffawang
Copy link

This is in the 1.5.1 upgrade guide, but it seems to be slated for 1.5.2. Right?

@shoenig
Copy link
Member Author

shoenig commented Mar 23, 2023

Hi @jeffawang, this particular change is in 1.5.1; in 1.5.2 we fixed more bugs related to artifact sandboxing, but specific to downloading artifacts using git-ssh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.5.x backport to 1.5.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants