Skip to content

Commit 37dcc44

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 a32a4bd commit 37dcc44

2 files changed

Lines changed: 38 additions & 0 deletions

File tree

internal/guest/runtime/hcsv2/uvm.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,6 +1246,24 @@ func (h *Host) modifyMappedVirtualDisk(
12461246
mountCtx, cancel := context.WithTimeout(ctx, time.Second*5)
12471247
defer cancel()
12481248
if mvd.MountPath != "" {
1249+
if h.HasSecurityPolicy() {
1250+
// The only option we allow if there is policy enforcement is
1251+
// "ro", and it must match the readonly field in the request.
1252+
mountOptionHasRo := false
1253+
for _, opt := range mvd.Options {
1254+
if opt == "ro" {
1255+
mountOptionHasRo = true
1256+
continue
1257+
}
1258+
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)
1259+
}
1260+
if mvd.ReadOnly != mountOptionHasRo {
1261+
return errors.Errorf(
1262+
"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",
1263+
mvd.Controller, mvd.Lun, mvd.MountPath, strings.Join(mvd.Options, ","), mvd.ReadOnly, mountOptionHasRo,
1264+
)
1265+
}
1266+
}
12491267
if mvd.ReadOnly {
12501268
var deviceHash string
12511269
if verityInfo != nil {
@@ -1410,6 +1428,12 @@ func (h *Host) modifyMappedDirectory(
14101428
return errors.Wrapf(err, "mounting plan9 device at %s denied by policy", md.MountPath)
14111429
}
14121430

1431+
if h.HasSecurityPolicy() {
1432+
if err = plan9.ValidateShareName(md.ShareName); err != nil {
1433+
return err
1434+
}
1435+
}
1436+
14131437
// Similar to the reasoning in modifyMappedVirtualDisk, since we're
14141438
// rolling back the policy metadata, plan9.Mount here must clean up
14151439
// 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)