Skip to content

Commit bada12e

Browse files
anmaxvlanmaxvl
authored
hcsv2: allowlist NVIDIA GPU capability annotation values (#2915)
The guest-side NVIDIA device hook comma-split the untrusted io.microsoft.container.gpu.capabilities annotation and appended every token verbatim as a --<token> argument to nvidia-container-cli configure. Because the tokens followed the fixed --ldconfig=@/sbin/ldconfig, a value-bearing token such as ldconfig=@<path> produced a second --ldconfig that won under the tool's last-flag-wins argument parsing, allowing an attacker-controlled executable path to be resolved before the target container's confinement was applied. Validate capability values against a fail-closed allowlist of the NVIDIA driver-capability vocabulary (all, compat32, compute, display, graphics, ngx, utility, video). Unknown, empty, and value-bearing tokens now abort hook creation before any argument reaches the tool. Omitting every non-listed option also blocks bare isolation-weakening flags such as no-cgroups. Valid capability sets are unchanged. Extract the injection-prone argv prefix into nvidiaConfigureArgs so it can be unit tested, and add regression tests covering the ldconfig injection payload, a valued option, unknown/empty tokens, and the legitimate capability set. Signed-off-by: Maksim An <maksiman@microsoft.com>
1 parent d94399a commit bada12e

3 files changed

Lines changed: 185 additions & 14 deletions

File tree

‎internal/guest/runtime/hcsv2/nvidia_utils.go‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,33 @@ import (
2525
const nvidiaDebugFilePath = "nvidia-container.log"
2626
const nvidiaToolBinary = "nvidia-container-cli"
2727

28-
// addNvidiaDeviceHook builds the arguments for nvidia-container-cli and creates the createRuntime [OCI hooks].
29-
//
30-
// [OCI hooks]: https://github.com/opencontainers/runtime-spec/blob/39c287c415bf86fb5b7506528d471db5405f8ca8/config.md#posix-platform-hooks
31-
func addNvidiaDeviceHook(ctx context.Context, spec *oci.Spec, ociBundlePath string) error {
32-
genericHookBinary := "generichook"
33-
genericHookPath, err := exec.LookPath(genericHookBinary)
34-
if err != nil {
35-
return errors.Wrapf(err, "failed to find %s for container device support", genericHookBinary)
28+
var nvidiaCapabilities = map[string]struct{}{
29+
"all": {},
30+
"compat32": {},
31+
"compute": {},
32+
"display": {},
33+
"graphics": {},
34+
"ngx": {},
35+
"utility": {},
36+
"video": {},
37+
}
38+
39+
func nvidiaCapabilityArgs(capabilities string) ([]string, error) {
40+
caps := strings.Split(capabilities, ",")
41+
args := make([]string, 0, len(caps))
42+
for _, capability := range caps {
43+
if _, ok := nvidiaCapabilities[capability]; !ok {
44+
return nil, fmt.Errorf("unsupported NVIDIA GPU capability %q", capability)
45+
}
46+
args = append(args, "--"+capability)
3647
}
48+
return args, nil
49+
}
3750

38-
toolDebugPath := filepath.Join(ociBundlePath, nvidiaDebugFilePath)
39-
debugOption := fmt.Sprintf("--debug=%s", toolDebugPath)
51+
// nvidiaConfigureArgs builds the fixed nvidia-container-cli configure arguments and
52+
// appends the validated GPU capabilities. The fixed --ldconfig must never be
53+
// overridable by the untrusted capabilities annotation.
54+
func nvidiaConfigureArgs(genericHookPath, debugOption string, spec *oci.Spec) ([]string, error) {
4055
args := []string{
4156
genericHookPath,
4257
nvidiaToolBinary,
@@ -46,10 +61,30 @@ func addNvidiaDeviceHook(ctx context.Context, spec *oci.Spec, ociBundlePath stri
4661
"--ldconfig=@/sbin/ldconfig",
4762
}
4863
if capabilities, ok := spec.Annotations[annotations.ContainerGPUCapabilities]; ok {
49-
caps := strings.Split(capabilities, ",")
50-
for _, c := range caps {
51-
args = append(args, fmt.Sprintf("--%s", c))
64+
capabilityArgs, err := nvidiaCapabilityArgs(capabilities)
65+
if err != nil {
66+
return nil, fmt.Errorf("invalid %s annotation: %w", annotations.ContainerGPUCapabilities, err)
5267
}
68+
args = append(args, capabilityArgs...)
69+
}
70+
return args, nil
71+
}
72+
73+
// addNvidiaDeviceHook builds the arguments for nvidia-container-cli and creates the createRuntime [OCI hooks].
74+
//
75+
// [OCI hooks]: https://github.com/opencontainers/runtime-spec/blob/39c287c415bf86fb5b7506528d471db5405f8ca8/config.md#posix-platform-hooks
76+
func addNvidiaDeviceHook(ctx context.Context, spec *oci.Spec, ociBundlePath string) error {
77+
genericHookBinary := "generichook"
78+
genericHookPath, err := exec.LookPath(genericHookBinary)
79+
if err != nil {
80+
return errors.Wrapf(err, "failed to find %s for container device support", genericHookBinary)
81+
}
82+
83+
toolDebugPath := filepath.Join(ociBundlePath, nvidiaDebugFilePath)
84+
debugOption := fmt.Sprintf("--debug=%s", toolDebugPath)
85+
args, err := nvidiaConfigureArgs(genericHookPath, debugOption, spec)
86+
if err != nil {
87+
return err
5388
}
5489

5590
for _, d := range spec.Windows.Devices {

‎internal/guest/runtime/hcsv2/nvidia_utils_test.go‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
//go:build linux
2+
// +build linux
3+
4+
package hcsv2
5+
6+
import (
7+
"reflect"
8+
"strings"
9+
"testing"
10+
11+
oci "github.com/opencontainers/runtime-spec/specs-go"
12+
13+
"github.com/Microsoft/hcsshim/pkg/annotations"
14+
)
15+
16+
func countLdconfig(args []string) int {
17+
n := 0
18+
for _, a := range args {
19+
if strings.HasPrefix(a, "--ldconfig=") {
20+
n++
21+
}
22+
}
23+
return n
24+
}
25+
26+
// TestNvidiaConfigureArgs_LdconfigInjection is a regression test for argument
27+
// injection via the untrusted GPU capabilities annotation. Setting it to
28+
// "utility,compute,ldconfig=@<payload>" used to append the value verbatim after
29+
// the fixed --ldconfig=@/sbin/ldconfig, producing a second --ldconfig that won
30+
// under nvidia-container-cli's last-flag-wins parsing.
31+
func TestNvidiaConfigureArgs_LdconfigInjection(t *testing.T) {
32+
const payload = "utility,compute,ldconfig=@/attacker/controlled/payload"
33+
34+
// Demonstrate the previous behavior: the naive comma-split the hook used to
35+
// perform yields a second, attacker-controlled --ldconfig after the default.
36+
legacy := []string{"--ldconfig=@/sbin/ldconfig"}
37+
for _, c := range strings.Split(payload, ",") {
38+
legacy = append(legacy, "--"+c)
39+
}
40+
if got := countLdconfig(legacy); got != 2 {
41+
t.Fatalf("precondition: legacy construction should inject a second --ldconfig, got %d", got)
42+
}
43+
if legacy[len(legacy)-1] != "--ldconfig=@/attacker/controlled/payload" {
44+
t.Fatalf("precondition: legacy construction should leave the injected --ldconfig last, got %q", legacy[len(legacy)-1])
45+
}
46+
47+
// New behavior: the same payload is rejected before any argv is produced.
48+
spec := &oci.Spec{Annotations: map[string]string{
49+
annotations.ContainerGPUCapabilities: payload,
50+
}}
51+
if _, err := nvidiaConfigureArgs("/path/generichook", "--debug=/tmp/log", spec); err == nil {
52+
t.Fatal("nvidiaConfigureArgs() accepted ldconfig injection payload, want error")
53+
}
54+
}
55+
56+
// TestNvidiaConfigureArgs_Valid confirms a legitimate capability set keeps
57+
// exactly the single fixed --ldconfig and appends the expected flags in order.
58+
func TestNvidiaConfigureArgs_Valid(t *testing.T) {
59+
spec := &oci.Spec{Annotations: map[string]string{
60+
annotations.ContainerGPUCapabilities: "compute,utility",
61+
}}
62+
args, err := nvidiaConfigureArgs("/path/generichook", "--debug=/tmp/log", spec)
63+
if err != nil {
64+
t.Fatalf("nvidiaConfigureArgs() error = %v", err)
65+
}
66+
if got := countLdconfig(args); got != 1 {
67+
t.Fatalf("expected exactly one --ldconfig, got %d in %v", got, args)
68+
}
69+
want := []string{
70+
"/path/generichook",
71+
"nvidia-container-cli",
72+
"--debug=/tmp/log",
73+
"--no-pivot",
74+
"configure",
75+
"--ldconfig=@/sbin/ldconfig",
76+
"--compute",
77+
"--utility",
78+
}
79+
if !reflect.DeepEqual(args, want) {
80+
t.Errorf("nvidiaConfigureArgs() = %v, want %v", args, want)
81+
}
82+
}
83+
84+
func TestNvidiaCapabilityArgs(t *testing.T) {
85+
tests := []struct {
86+
name string
87+
capabilities string
88+
want []string
89+
wantErr string
90+
}{
91+
{
92+
name: "valid capabilities",
93+
capabilities: "all,compat32,compute,display,graphics,ngx,utility,video",
94+
want: []string{"--all", "--compat32", "--compute", "--display", "--graphics", "--ngx", "--utility", "--video"},
95+
},
96+
{
97+
name: "valued option rejected",
98+
capabilities: "compute,no-cgroups",
99+
wantErr: `unsupported NVIDIA GPU capability "no-cgroups"`,
100+
},
101+
{
102+
name: "argument injection",
103+
capabilities: "utility,compute,ldconfig=@/attacker/controlled/payload",
104+
wantErr: `unsupported NVIDIA GPU capability "ldconfig=@/attacker/controlled/payload"`,
105+
},
106+
{
107+
name: "unknown capability",
108+
capabilities: "network",
109+
wantErr: `unsupported NVIDIA GPU capability "network"`,
110+
},
111+
{
112+
name: "empty capability",
113+
capabilities: "",
114+
wantErr: `unsupported NVIDIA GPU capability ""`,
115+
},
116+
}
117+
118+
for _, test := range tests {
119+
t.Run(test.name, func(t *testing.T) {
120+
got, err := nvidiaCapabilityArgs(test.capabilities)
121+
if test.wantErr != "" {
122+
if err == nil || !strings.Contains(err.Error(), test.wantErr) {
123+
t.Fatalf("nvidiaCapabilityArgs() error = %v, want error containing %q", err, test.wantErr)
124+
}
125+
return
126+
}
127+
if err != nil {
128+
t.Fatalf("nvidiaCapabilityArgs() error = %v", err)
129+
}
130+
if !reflect.DeepEqual(got, test.want) {
131+
t.Errorf("nvidiaCapabilityArgs() = %v, want %v", got, test.want)
132+
}
133+
})
134+
}
135+
}

‎pkg/annotations/annotations.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,8 @@ const (
553553
// Deprecated: GPU VHDs are no longer supported.
554554
GPUVHDPath = "io.microsoft.lcow.gpuvhdpath"
555555

556-
// ContainerGPUCapabilities is used to find the gpu capabilities on the container spec.
556+
// ContainerGPUCapabilities specifies a comma-separated list of NVIDIA GPU capabilities.
557+
// Supported values are all, compat32, compute, display, graphics, ngx, utility, and video.
557558
ContainerGPUCapabilities = "io.microsoft.container.gpu.capabilities"
558559
)
559560

0 commit comments

Comments
 (0)