Skip to content

Commit fb118fe

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 cd94b7f commit fb118fe

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
@@ -1225,6 +1225,24 @@ func (h *Host) modifyMappedVirtualDisk(
12251225
mountCtx, cancel := context.WithTimeout(ctx, time.Second*5)
12261226
defer cancel()
12271227
if mvd.MountPath != "" {
1228+
if h.HasSecurityPolicy() {
1229+
// The only option we allow if there is policy enforcement is
1230+
// "ro", and it must match the readonly field in the request.
1231+
mountOptionHasRo := false
1232+
for _, opt := range mvd.Options {
1233+
if opt == "ro" {
1234+
mountOptionHasRo = true
1235+
continue
1236+
}
1237+
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)
1238+
}
1239+
if mvd.ReadOnly != mountOptionHasRo {
1240+
return errors.Errorf(
1241+
"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",
1242+
mvd.Controller, mvd.Lun, mvd.MountPath, strings.Join(mvd.Options, ","), mvd.ReadOnly, mountOptionHasRo,
1243+
)
1244+
}
1245+
}
12281246
if mvd.ReadOnly {
12291247
var deviceHash string
12301248
if verityInfo != nil {
@@ -1389,6 +1407,12 @@ func (h *Host) modifyMappedDirectory(
13891407
return errors.Wrapf(err, "mounting plan9 device at %s denied by policy", md.MountPath)
13901408
}
13911409

1410+
if h.HasSecurityPolicy() {
1411+
if err = plan9.ValidateShareName(md.ShareName); err != nil {
1412+
return err
1413+
}
1414+
}
1415+
13921416
// Similar to the reasoning in modifyMappedVirtualDisk, since we're
13931417
// rolling back the policy metadata, plan9.Mount here must clean up
13941418
// 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)