From 936fad1d04771bf1cc4cdeb3586831574cbc9400 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 9 Mar 2023 10:57:11 +0200 Subject: [PATCH 1/3] Move check for privileged images to config/image/ package Signed-off-by: Evan Lezar --- .../container_config.go | 40 +++++++++-------- internal/config/image/privileged.go | 43 +++++++++++++++++++ 2 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 internal/config/image/privileged.go diff --git a/cmd/nvidia-container-runtime-hook/container_config.go b/cmd/nvidia-container-runtime-hook/container_config.go index 531b2b42..20a3b09f 100644 --- a/cmd/nvidia-container-runtime-hook/container_config.go +++ b/cmd/nvidia-container-runtime-hook/container_config.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/NVIDIA/nvidia-container-toolkit/internal/config/image" + "github.com/opencontainers/runtime-spec/specs-go" "golang.org/x/mod/semver" ) @@ -130,7 +131,7 @@ func isPrivileged(s *Spec) bool { } var caps []string - // If v1.1.0-rc1 <= OCI version < v1.0.0-rc5 parse s.Process.Capabilities as: + // If v1.0.0-rc1 <= OCI version < v1.0.0-rc5 parse s.Process.Capabilities as: // github.com/opencontainers/runtime-spec/blob/v1.0.0-rc1/specs-go/config.go#L30-L54 rc1cmp := semver.Compare("v"+*s.Version, "v1.0.0-rc1") rc5cmp := semver.Compare("v"+*s.Version, "v1.0.0-rc5") @@ -139,28 +140,31 @@ func isPrivileged(s *Spec) bool { if err != nil { log.Panicln("could not decode Process.Capabilities in OCI spec:", err) } - // Otherwise, parse s.Process.Capabilities as: - // github.com/opencontainers/runtime-spec/blob/v1.0.0/specs-go/config.go#L30-L54 - } else { - var lc LinuxCapabilities - err := json.Unmarshal(*s.Process.Capabilities, &lc) - if err != nil { - log.Panicln("could not decode Process.Capabilities in OCI spec:", err) + for _, c := range caps { + if c == capSysAdmin { + return true + } } - // We only make sure that the bounding capabibility set has - // CAP_SYS_ADMIN. This allows us to make sure that the container was - // actually started as '--privileged', but also allow non-root users to - // access the privileged NVIDIA capabilities. - caps = lc.Bounding + return false } - for _, c := range caps { - if c == capSysAdmin { - return true - } + // Otherwise, parse s.Process.Capabilities as: + // github.com/opencontainers/runtime-spec/blob/v1.0.0/specs-go/config.go#L30-L54 + process := specs.Process{ + Env: s.Process.Env, } - return false + err := json.Unmarshal(*s.Process.Capabilities, &process.Capabilities) + if err != nil { + log.Panicln("could not decode Process.Capabilities in OCI spec:", err) + } + + fullSpec := specs.Spec{ + Version: *s.Version, + Process: &process, + } + + return image.IsPrivileged(&fullSpec) } func getDevicesFromEnvvar(image image.CUDA, swarmResourceEnvvars []string) *string { diff --git a/internal/config/image/privileged.go b/internal/config/image/privileged.go new file mode 100644 index 00000000..a54598d6 --- /dev/null +++ b/internal/config/image/privileged.go @@ -0,0 +1,43 @@ +/** +# Copyright (c) NVIDIA CORPORATION. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package image + +import ( + "github.com/opencontainers/runtime-spec/specs-go" +) + +const ( + capSysAdmin = "CAP_SYS_ADMIN" +) + +// IsPrivileged returns true if the container is a privileged container. +func IsPrivileged(s *specs.Spec) bool { + if s.Process.Capabilities == nil { + return false + } + + // We only make sure that the bounding capabibility set has + // CAP_SYS_ADMIN. This allows us to make sure that the container was + // actually started as '--privileged', but also allow non-root users to + // access the privileged NVIDIA capabilities. + for _, c := range s.Process.Capabilities.Bounding { + if c == capSysAdmin { + return true + } + } + return false +} From 154cd4ecf3b94a8446dc8a436690225e1e40c8e5 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 9 Mar 2023 11:14:38 +0200 Subject: [PATCH 2/3] Add to config struct Signed-off-by: Evan Lezar --- internal/config/config.go | 5 +++++ internal/config/config_test.go | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/internal/config/config.go b/internal/config/config.go index 6adc604e..b3c6f890 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -45,6 +45,8 @@ var ( // Config represents the contents of the config.toml file for the NVIDIA Container Toolkit // Note: This is currently duplicated by the HookConfig in cmd/nvidia-container-toolkit/hook_config.go type Config struct { + AcceptEnvvarUnprivileged bool `toml:"accept-nvidia-visible-devices-envvar-when-unprivileged"` + NVIDIAContainerCLIConfig ContainerCLIConfig `toml:"nvidia-container-cli"` NVIDIACTKConfig CTKConfig `toml:"nvidia-ctk"` NVIDIAContainerRuntimeConfig RuntimeConfig `toml:"nvidia-container-runtime"` @@ -91,6 +93,8 @@ func getConfigFrom(toml *toml.Tree) (*Config, error) { return cfg, nil } + cfg.AcceptEnvvarUnprivileged = toml.GetDefault("accept-nvidia-visible-devices-envvar-when-unprivileged", cfg.AcceptEnvvarUnprivileged).(bool) + cfg.NVIDIAContainerCLIConfig = *getContainerCLIConfigFrom(toml) cfg.NVIDIACTKConfig = *getCTKConfigFrom(toml) runtimeConfig, err := getRuntimeConfigFrom(toml) @@ -105,6 +109,7 @@ func getConfigFrom(toml *toml.Tree) (*Config, error) { // getDefaultConfig defines the default values for the config func getDefaultConfig() *Config { c := Config{ + AcceptEnvvarUnprivileged: true, NVIDIAContainerCLIConfig: *getDefaultContainerCLIConfig(), NVIDIACTKConfig: *getDefaultCTKConfig(), NVIDIAContainerRuntimeConfig: *GetDefaultRuntimeConfig(), diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 12683ae2..4c378d55 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -57,6 +57,7 @@ func TestGetConfig(t *testing.T) { { description: "empty config is default", expectedConfig: &Config{ + AcceptEnvvarUnprivileged: true, NVIDIAContainerCLIConfig: ContainerCLIConfig{ Root: "", }, @@ -82,6 +83,7 @@ func TestGetConfig(t *testing.T) { { description: "config options set inline", contents: []string{ + "accept-nvidia-visible-devices-envvar-when-unprivileged = false", "nvidia-container-cli.root = \"/bar/baz\"", "nvidia-container-runtime.debug = \"/foo/bar\"", "nvidia-container-runtime.experimental = true", @@ -94,6 +96,7 @@ func TestGetConfig(t *testing.T) { "nvidia-ctk.path = \"/foo/bar/nvidia-ctk\"", }, expectedConfig: &Config{ + AcceptEnvvarUnprivileged: false, NVIDIAContainerCLIConfig: ContainerCLIConfig{ Root: "/bar/baz", }, @@ -119,6 +122,7 @@ func TestGetConfig(t *testing.T) { { description: "config options set in section", contents: []string{ + "accept-nvidia-visible-devices-envvar-when-unprivileged = false", "[nvidia-container-cli]", "root = \"/bar/baz\"", "[nvidia-container-runtime]", @@ -136,6 +140,7 @@ func TestGetConfig(t *testing.T) { "path = \"/foo/bar/nvidia-ctk\"", }, expectedConfig: &Config{ + AcceptEnvvarUnprivileged: false, NVIDIAContainerCLIConfig: ContainerCLIConfig{ Root: "/bar/baz", }, From 973e7bda5edf8c2d0899138d1d9b912b21b57f20 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 9 Mar 2023 11:15:34 +0200 Subject: [PATCH 3/3] Check accept-nvidia-visible-devices-envvar-when-unprivileged option for CDI Signed-off-by: Evan Lezar --- internal/modifier/cdi.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/internal/modifier/cdi.go b/internal/modifier/cdi.go index eb15b4bf..2fcd8eaa 100644 --- a/internal/modifier/cdi.go +++ b/internal/modifier/cdi.go @@ -37,7 +37,7 @@ type cdiModifier struct { // CDI specifications available on the system. The NVIDIA_VISIBLE_DEVICES enviroment variable is // used to select the devices to include. func NewCDIModifier(logger *logrus.Logger, cfg *config.Config, ociSpec oci.Spec) (oci.SpecModifier, error) { - devices, err := getDevicesFromSpec(logger, ociSpec, cfg.NVIDIAContainerRuntimeConfig.Modes.CDI.DefaultKind) + devices, err := getDevicesFromSpec(logger, ociSpec, cfg) if err != nil { return nil, fmt.Errorf("failed to get required devices from OCI specification: %v", err) } @@ -61,7 +61,7 @@ func NewCDIModifier(logger *logrus.Logger, cfg *config.Config, ociSpec oci.Spec) return m, nil } -func getDevicesFromSpec(logger *logrus.Logger, ociSpec oci.Spec, defaultKind string) ([]string, error) { +func getDevicesFromSpec(logger *logrus.Logger, ociSpec oci.Spec, cfg *config.Config) ([]string, error) { rawSpec, err := ociSpec.Load() if err != nil { return nil, fmt.Errorf("failed to load OCI spec: %v", err) @@ -75,17 +75,17 @@ func getDevicesFromSpec(logger *logrus.Logger, ociSpec oci.Spec, defaultKind str return annotationDevices, nil } - image, err := image.NewCUDAImageFromSpec(rawSpec) + container, err := image.NewCUDAImageFromSpec(rawSpec) if err != nil { return nil, err } - envDevices := image.DevicesFromEnvvars(visibleDevicesEnvvar) + envDevices := container.DevicesFromEnvvars(visibleDevicesEnvvar) var devices []string seen := make(map[string]bool) for _, name := range envDevices.List() { if !cdi.IsQualifiedName(name) { - name = fmt.Sprintf("%s=%s", defaultKind, name) + name = fmt.Sprintf("%s=%s", cfg.NVIDIAContainerRuntimeConfig.Modes.CDI.DefaultKind, name) } if seen[name] { logger.Debugf("Ignoring duplicate device %q", name) @@ -94,6 +94,16 @@ func getDevicesFromSpec(logger *logrus.Logger, ociSpec oci.Spec, defaultKind str devices = append(devices, name) } + if len(devices) == 0 { + return nil, nil + } + + if cfg.AcceptEnvvarUnprivileged || image.IsPrivileged(rawSpec) { + return devices, nil + } + + logger.Warningf("Ignoring devices specified in NVIDIA_VISIBLE_DEVICES: %v", devices) + return devices, nil }