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

Adds ability to restrict uid and gids in exec and raw_exec #24343

Merged
merged 8 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/20073.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
drivers: add posibility to restrict user and group for exec and rawexec
```
3 changes: 3 additions & 0 deletions .changelog/24157.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
getter: Added option to chown artifact(s) to task user
```
3 changes: 3 additions & 0 deletions .changelog/24169.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
fingerprint gce: fingerprint preemptibility
```
6 changes: 3 additions & 3 deletions .release/versions.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@

schema = 1
active_versions {
version "1.9.x" {
ce_active = true
}
version "1.8.x" {
ce_active = true
lts = true
}
version "1.7.x" {
ce_active = true
}
version "1.6.x" {
ce_active = true
}
}
409 changes: 409 additions & 0 deletions CHANGELOG-unsupported.md

Large diffs are not rendered by default.

516 changes: 44 additions & 472 deletions CHANGELOG.md

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion api/allocations.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,10 @@ type AllocPauseRequest struct {
}

type AllocGetPauseResponse struct {
// ScheduleState will be one of "pause", "run", "scheduled".
// ScheduleState will be one of "" (run), "force_run", "scheduled_pause",
// "force_pause", or "schedule_resume".
//
// See nomad/structs/task_sched.go for details.
ScheduleState string
}

Expand Down
1 change: 1 addition & 0 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ type TaskArtifact struct {
GetterMode *string `mapstructure:"mode" hcl:"mode,optional"`
GetterInsecure *bool `mapstructure:"insecure" hcl:"insecure,optional"`
RelativeDest *string `mapstructure:"destination" hcl:"destination,optional"`
Chown bool `mapstructure:"chown" hcl:"chown,optional"`
}

func (a *TaskArtifact) Canonicalize() {
Expand Down
1 change: 1 addition & 0 deletions api/tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ func TestTask_Artifact(t *testing.T) {
must.Eq(t, "local/foo.txt", filepath.ToSlash(*a.RelativeDest))
must.Nil(t, a.GetterOptions)
must.Nil(t, a.GetterHeaders)
must.Eq(t, false, a.Chown)
}

func TestTask_VolumeMount(t *testing.T) {
Expand Down
11 changes: 9 additions & 2 deletions client/allocrunner/taskrunner/artifact_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ func newArtifactHook(e ti.EventEmitter, getter ci.ArtifactGetter, logger log.Log
return h
}

func (h *artifactHook) doWork(req *interfaces.TaskPrestartRequest, resp *interfaces.TaskPrestartResponse, jobs chan *structs.TaskArtifact, errorChannel chan error, wg *sync.WaitGroup, responseStateMutex *sync.Mutex) {
func (h *artifactHook) doWork(
req *interfaces.TaskPrestartRequest,
resp *interfaces.TaskPrestartResponse,
jobs chan *structs.TaskArtifact,
errorChannel chan error,
wg *sync.WaitGroup,
responseStateMutex *sync.Mutex,
) {
defer wg.Done()
for artifact := range jobs {
aid := artifact.Hash()
Expand All @@ -45,7 +52,7 @@ func (h *artifactHook) doWork(req *interfaces.TaskPrestartRequest, resp *interfa

h.logger.Debug("downloading artifact", "artifact", artifact.GetterSource, "aid", aid)

if err := h.getter.Get(req.TaskEnv, artifact); err != nil {
if err := h.getter.Get(req.TaskEnv, artifact, req.Task.User); err != nil {
wrapped := structs.NewRecoverableError(
fmt.Errorf("failed to download artifact %q: %v", artifact.GetterSource, err),
true,
Expand Down
2 changes: 2 additions & 0 deletions client/allocrunner/taskrunner/getter/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ type parameters struct {
// Task Filesystem
AllocDir string `json:"alloc_dir"`
TaskDir string `json:"task_dir"`
User string `json:"user"`
Chown bool `json:"chown"`
}

func (p *parameters) reader() io.Reader {
Expand Down
6 changes: 5 additions & 1 deletion client/allocrunner/taskrunner/getter/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ const paramsAsJSON = `
"X-Nomad-Artifact": ["hi"]
},
"alloc_dir": "/path/to/alloc",
"task_dir": "/path/to/alloc/task"
"task_dir": "/path/to/alloc/task",
"chown": true,
"user":"nobody"
}`

var paramsAsStruct = &parameters{
Expand All @@ -65,6 +67,8 @@ var paramsAsStruct = &parameters{
Headers: map[string][]string{
"X-Nomad-Artifact": {"hi"},
},
User: "nobody",
Chown: true,
}

func TestParameters_reader(t *testing.T) {
Expand Down
7 changes: 5 additions & 2 deletions client/allocrunner/taskrunner/getter/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ type Sandbox struct {
ac *config.ArtifactConfig
}

func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact) error {
s.logger.Debug("get", "source", artifact.GetterSource, "destination", artifact.RelativeDest)
func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact, user string) error {
s.logger.Debug("get", "source", artifact.GetterSource, "destination", artifact.RelativeDest, "user", user)

source, err := getURL(env, artifact)
if err != nil {
Expand Down Expand Up @@ -66,10 +66,13 @@ func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact
// task filesystem
AllocDir: allocDir,
TaskDir: taskDir,
User: user,
Chown: artifact.Chown,
}

if err = s.runCmd(params); err != nil {
return err
}

return nil
}
33 changes: 30 additions & 3 deletions client/allocrunner/taskrunner/getter/sandbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http/httptest"
"os"
"path/filepath"
"syscall"
"testing"
"time"

Expand Down Expand Up @@ -46,7 +47,7 @@ func TestSandbox_Get_http(t *testing.T) {
RelativeDest: "local/downloads",
}

err := sbox.Get(env, artifact)
err := sbox.Get(env, artifact, "nobody")
must.NoError(t, err)

b, err := os.ReadFile(filepath.Join(taskDir, "local", "downloads", "go.mod"))
Expand Down Expand Up @@ -74,11 +75,37 @@ func TestSandbox_Get_insecure_http(t *testing.T) {
RelativeDest: "local/downloads",
}

err := sbox.Get(env, artifact)
err := sbox.Get(env, artifact, "nobody")
must.Error(t, err)
must.StrContains(t, err.Error(), "x509: certificate signed by unknown authority")

artifact.GetterInsecure = true
err = sbox.Get(env, artifact)
err = sbox.Get(env, artifact, "nobody")
must.NoError(t, err)
}

func TestSandbox_Get_chown(t *testing.T) {
testutil.RequireRoot(t)
logger := testlog.HCLogger(t)

ac := artifactConfig(10 * time.Second)
sbox := New(ac, logger)

_, taskDir := SetupDir(t)
env := noopTaskEnv(taskDir)

artifact := &structs.TaskArtifact{
GetterSource: "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod",
RelativeDest: "local/downloads",
Chown: true,
}

err := sbox.Get(env, artifact, "nobody")
must.NoError(t, err)

info, err := os.Stat(filepath.Join(taskDir, "local", "downloads"))
must.NoError(t, err)

uid := info.Sys().(*syscall.Stat_t).Uid
must.Eq(t, 65534, uid) // nobody's conventional uid
}
28 changes: 28 additions & 0 deletions client/allocrunner/taskrunner/getter/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"sort"
"strings"
"unicode"

"github.com/hashicorp/go-getter"
"github.com/hashicorp/nomad/client/interfaces"
"github.com/hashicorp/nomad/helper/subproc"
"github.com/hashicorp/nomad/helper/users"
"github.com/hashicorp/nomad/nomad/structs"
)

Expand Down Expand Up @@ -84,6 +86,32 @@ func getMode(artifact *structs.TaskArtifact) getter.ClientMode {
}
}

func chownDestination(destination, username string) error {
if destination == "" || username == "" {
return nil
}

if os.Geteuid() != 0 {
return nil
}

if runtime.GOOS == "windows" {
return nil
}

uid, gid, _, err := users.LookupUnix(username)
if err != nil {
return err
}

return filepath.Walk(destination, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
return os.Chown(path, uid, gid)
})
}

func isInsecure(artifact *structs.TaskArtifact) bool {
return artifact.GetterInsecure
}
Expand Down
10 changes: 10 additions & 0 deletions client/allocrunner/taskrunner/getter/z_getter_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ func init() {
return subproc.ExitFailure
}

// chown the resulting artifact to the task user, but only if configured
// to do so in the artifact block (for compatibility)
if env.Chown {
err := chownDestination(env.Destination, env.User)
if err != nil {
subproc.Print("failed to chown artifact: %v", err)
return subproc.ExitFailure
}
}

subproc.Print("artifact download was a success")
return subproc.ExitSuccess
})
Expand Down
1 change: 1 addition & 0 deletions client/fingerprint/env_gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func (f *EnvGCEFingerprint) Fingerprint(req *FingerprintRequest, resp *Fingerpri
"cpu-platform": false,
"scheduling/automatic-restart": false,
"scheduling/on-host-maintenance": false,
"scheduling/preemptible": false,
}

for k, unique := range keys {
Expand Down
6 changes: 6 additions & 0 deletions client/fingerprint/env_gce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ func testFingerprint_GCE(t *testing.T, withExternalIp bool) {

assertNodeAttributeEquals(t, response.Attributes, "platform.gce.scheduling.automatic-restart", "TRUE")
assertNodeAttributeEquals(t, response.Attributes, "platform.gce.scheduling.on-host-maintenance", "MIGRATE")
assertNodeAttributeEquals(t, response.Attributes, "platform.gce.scheduling.preemptible", "FALSE")
assertNodeAttributeEquals(t, response.Attributes, "platform.gce.cpu-platform", "Intel Ivy Bridge")
assertNodeAttributeEquals(t, response.Attributes, "platform.gce.tag.abc", "true")
assertNodeAttributeEquals(t, response.Attributes, "platform.gce.tag.def", "true")
Expand Down Expand Up @@ -200,6 +201,11 @@ const GCE_routes = `
"content-type": "text/plain",
"body": "MIGRATE"
},
{
"uri": "/computeMetadata/v1/instance/scheduling/preemptible",
"content-type": "text/plain",
"body": "FALSE"
},
{
"uri": "/computeMetadata/v1/instance/cpu-platform",
"content-type": "text/plain",
Expand Down
2 changes: 1 addition & 1 deletion client/interfaces/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type EnvReplacer interface {
// ArtifactGetter is an interface satisfied by the getter package.
type ArtifactGetter interface {
// Get artifact and put it in the task directory.
Get(EnvReplacer, *structs.TaskArtifact) error
Get(EnvReplacer, *structs.TaskArtifact, string) error
}

// ProcessWranglers is an interface satisfied by the proclib package.
Expand Down
2 changes: 1 addition & 1 deletion client/lib/numalib/hw/ids.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type (
// Must be an alias because go-msgpack cannot handle the real type.
NodeID = uint8

// A SocketID represents a physicsl CPU socket.
// A SocketID represents a physical CPU socket.
SocketID uint8

// A CoreID represents one logical (vCPU) core.
Expand Down
1 change: 1 addition & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
GetterMode: *ta.GetterMode,
GetterInsecure: *ta.GetterInsecure,
RelativeDest: *ta.RelativeDest,
Chown: ta.Chown,
})
}
}
Expand Down
2 changes: 2 additions & 0 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2932,6 +2932,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
},
GetterMode: pointer.Of("dir"),
RelativeDest: pointer.Of("dest"),
Chown: true,
},
},
Vault: &api.Vault{
Expand Down Expand Up @@ -3387,6 +3388,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
},
GetterMode: "dir",
RelativeDest: "dest",
Chown: true,
},
},
Vault: &structs.Vault{
Expand Down
Loading
Loading