Skip to content

Commit 0bd76ae

Browse files
committed
Merged PR 13627088: guest: Don't allow host to set mount options
[cherry-picked from 1dd0b7ea0b0f91d3698f6008fb0bd5b0de777da6] Blocks mount option passing for 9p (which is accidental) and SCSI disks. - guest: Restrict plan9 share names to digits only on Confidential mode - hcsv2/uvm: Restrict SCSI mount options in confidential mode (The only one we allow is `ro`) Related work items: #34370380 Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
1 parent e024175 commit 0bd76ae

File tree

2 files changed

+38
-0
lines changed

2 files changed

+38
-0
lines changed

internal/guest/runtime/hcsv2/uvm.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,24 @@ func (h *Host) modifyMappedVirtualDisk(
12341234
mountCtx, cancel := context.WithTimeout(ctx, time.Second*5)
12351235
defer cancel()
12361236
if mvd.MountPath != "" {
1237+
if h.HasSecurityPolicy() {
1238+
// The only option we allow if there is policy enforcement is
1239+
// "ro", and it must match the readonly field in the request.
1240+
mountOptionHasRo := false
1241+
for _, opt := range mvd.Options {
1242+
if opt == "ro" {
1243+
mountOptionHasRo = true
1244+
continue
1245+
}
1246+
return errors.Errorf("mounting scsi device controller %d lun %d onto %s: mount option %q denied by policy", mvd.Controller, mvd.Lun, mvd.MountPath, opt)
1247+
}
1248+
if mvd.ReadOnly != mountOptionHasRo {
1249+
return errors.Errorf(
1250+
"mounting scsi device controller %d lun %d onto %s with mount option %q failed due to mount option mismatch: mvd.ReadOnly=%t but mountOptionHasRo=%t",
1251+
mvd.Controller, mvd.Lun, mvd.MountPath, strings.Join(mvd.Options, ","), mvd.ReadOnly, mountOptionHasRo,
1252+
)
1253+
}
1254+
}
12371255
if mvd.ReadOnly {
12381256
var deviceHash string
12391257
if verityInfo != nil {
@@ -1398,6 +1416,12 @@ func (h *Host) modifyMappedDirectory(
13981416
return errors.Wrapf(err, "mounting plan9 device at %s denied by policy", md.MountPath)
13991417
}
14001418

1419+
if h.HasSecurityPolicy() {
1420+
if err = plan9.ValidateShareName(md.ShareName); err != nil {
1421+
return err
1422+
}
1423+
}
1424+
14011425
// Similar to the reasoning in modifyMappedVirtualDisk, since we're
14021426
// rolling back the policy metadata, plan9.Mount here must clean up
14031427
// everything if it fails, which it does do.

internal/guest/storage/plan9/plan9.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"fmt"
99
"os"
10+
"regexp"
1011
"syscall"
1112

1213
"github.com/Microsoft/hcsshim/internal/guest/transport"
@@ -25,6 +26,19 @@ var (
2526
unixMount = unix.Mount
2627
)
2728

29+
// c.f. v9fs_parse_options in linux/fs/9p/v9fs.c - technically anything other
30+
// than ',' is ok (quoting is not handled), however, this name is generated from
31+
// a counter in AddPlan9 (internal/uvm/plan9.go), and therefore we expect only
32+
// digits from a normal hcsshim host.
33+
var validShareNameRegex = regexp.MustCompile(`^[0-9]+$`)
34+
35+
func ValidateShareName(name string) error {
36+
if !validShareNameRegex.MatchString(name) {
37+
return fmt.Errorf("invalid plan9 share name %q: must match regex %q", name, validShareNameRegex.String())
38+
}
39+
return nil
40+
}
41+
2842
// Mount dials a connection from `vsock` and mounts a Plan9 share to `target`.
2943
//
3044
// `target` will be created. On mount failure the created `target` will be

0 commit comments

Comments
 (0)