diff --git a/ext4/tar2ext4/tar2ext4.go b/ext4/tar2ext4/tar2ext4.go index 14ff66eee6..6938683385 100644 --- a/ext4/tar2ext4/tar2ext4.go +++ b/ext4/tar2ext4/tar2ext4.go @@ -215,19 +215,19 @@ func Convert(r io.Reader, w io.ReadWriteSeeker, options ...Option) error { // More details can be found here https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout // // Our goal is to skip the Group 0 padding, read and return the ext4 SuperBlock -func ReadExt4SuperBlock(vhdPath string) (*format.SuperBlock, error) { - vhd, err := os.OpenFile(vhdPath, os.O_RDONLY, 0) +func ReadExt4SuperBlock(devicePath string) (*format.SuperBlock, error) { + dev, err := os.OpenFile(devicePath, os.O_RDONLY, 0) if err != nil { return nil, err } - defer vhd.Close() + defer dev.Close() // Skip padding at the start - if _, err := vhd.Seek(1024, io.SeekStart); err != nil { + if _, err := dev.Seek(1024, io.SeekStart); err != nil { return nil, err } var sb format.SuperBlock - if err := binary.Read(vhd, binary.LittleEndian, &sb); err != nil { + if err := binary.Read(dev, binary.LittleEndian, &sb); err != nil { return nil, err } // Make sure the magic bytes are correct. @@ -237,6 +237,15 @@ func ReadExt4SuperBlock(vhdPath string) (*format.SuperBlock, error) { return &sb, nil } +// IsDeviceExt4 is will read the device's superblock and determine if it is +// and ext4 superblock. +func IsDeviceExt4(devicePath string) bool { + // ReadExt4SuperBlock will check the superblock magic number for us, + // so we know if no error is returned, this is an ext4 device. + _, err := ReadExt4SuperBlock(devicePath) + return err == nil +} + // ConvertAndComputeRootDigest writes a compact ext4 file system image that contains the files in the // input tar stream, computes the resulting file image's cryptographic hashes (merkle tree) and returns // merkle tree root digest. Convert is called with minimal options: ConvertWhiteout and MaximumDiskSize diff --git a/internal/guest/runtime/hcsv2/uvm.go b/internal/guest/runtime/hcsv2/uvm.go index f1c5540a54..a1d46c0458 100644 --- a/internal/guest/runtime/hcsv2/uvm.go +++ b/internal/guest/runtime/hcsv2/uvm.go @@ -980,9 +980,14 @@ func modifyMappedVirtualDisk( return errors.Wrapf(err, "mounting scsi device controller %d lun %d onto %s denied by policy", mvd.Controller, mvd.Lun, mvd.MountPath) } } - + config := &scsi.Config{ + Encrypted: mvd.Encrypted, + VerityInfo: mvd.VerityInfo, + EnsureFilesystem: mvd.EnsureFilesystem, + Filesystem: mvd.Filesystem, + } return scsi.Mount(mountCtx, mvd.Controller, mvd.Lun, mvd.Partition, mvd.MountPath, - mvd.ReadOnly, mvd.Encrypted, mvd.Options, mvd.VerityInfo) + mvd.ReadOnly, mvd.Options, config) } return nil case guestrequest.RequestTypeRemove: @@ -992,9 +997,14 @@ func modifyMappedVirtualDisk( return fmt.Errorf("unmounting scsi device at %s denied by policy: %w", mvd.MountPath, err) } } - + config := &scsi.Config{ + Encrypted: mvd.Encrypted, + VerityInfo: mvd.VerityInfo, + EnsureFilesystem: mvd.EnsureFilesystem, + Filesystem: mvd.Filesystem, + } if err := scsi.Unmount(ctx, mvd.Controller, mvd.Lun, mvd.Partition, - mvd.MountPath, mvd.Encrypted, mvd.VerityInfo); err != nil { + mvd.MountPath, config); err != nil { return err } } diff --git a/internal/guest/storage/crypt/crypt.go b/internal/guest/storage/crypt/crypt.go index 4bad3c4b95..69e879a66a 100644 --- a/internal/guest/storage/crypt/crypt.go +++ b/internal/guest/storage/crypt/crypt.go @@ -21,7 +21,6 @@ var ( _cryptsetupOpen = cryptsetupOpen _generateKeyFile = generateKeyFile _osMkdirTemp = os.MkdirTemp - _mkfsXfs = mkfsXfs _osRemoveAll = os.RemoveAll _zeroFirstBlock = zeroFirstBlock ) @@ -88,17 +87,6 @@ func cryptsetupClose(deviceName string) error { return cryptsetupCommand(closeArgs) } -// invoke mkfs.xfs for the given device. -func mkfsXfs(devicePath string) error { - args := []string{"-f", devicePath} - cmd := exec.Command("mkfs.xfs", args...) - output, err := cmd.CombinedOutput() - if err != nil { - return errors.Wrapf(err, "failed to execute mkfs.ext4: %s", string(output)) - } - return nil -} - // EncryptDevice creates a dm-crypt target for a container scratch vhd. // // In order to mount a block device as an encrypted device: @@ -121,12 +109,11 @@ func mkfsXfs(devicePath string) error { // /dev/mapper/`cryptDeviceTemplate`. This can be mounted directly, but it // doesn't have any format yet. // -// 4. Format the unencrypted block device as xfs. +// 4. Prepare the unecrypted block device to be later formatted as xfs // 4.1. Zero the first block. It appears that mkfs.xfs reads this before formatting. -// 4.2. Format the device as xfs. func EncryptDevice(ctx context.Context, source string, dmCryptName string) (path string, err error) { - // Create temporary directory to store the keyfile and EXT4 image + // Create temporary directory to store the keyfile and xfs image tempDir, err := _osMkdirTemp("", "dm-crypt") if err != nil { return "", errors.Wrapf(err, "failed to create temporary folder: %s", source) @@ -172,11 +159,6 @@ func EncryptDevice(ctx context.Context, source string, dmCryptName string) (path return "", fmt.Errorf("failed to zero first block: %w", err) } - // 4.2. Format it as xfs - if err = _mkfsXfs(deviceNamePath); err != nil { - return "", fmt.Errorf("mkfs.xfs failed to format %s: %w", deviceNamePath, err) - } - return deviceNamePath, nil } diff --git a/internal/guest/storage/crypt/crypt_test.go b/internal/guest/storage/crypt/crypt_test.go index b05afb1559..fa85b5abbb 100644 --- a/internal/guest/storage/crypt/crypt_test.go +++ b/internal/guest/storage/crypt/crypt_test.go @@ -125,46 +125,6 @@ func Test_Encrypt_Cryptsetup_Open_Error(t *testing.T) { } } -func Test_Encrypt_Mkfs_Error(t *testing.T) { - clearCryptTestDependencies() - - // Test what happens when mkfs fails to format the unencrypted device. - // Verify that the arguments passed to it are the right ones. - _generateKeyFile = func(path string, size int64) error { - return nil - } - _osRemoveAll = func(path string) error { - return nil - } - _cryptsetupFormat = func(source string, keyFilePath string) error { - return nil - } - _cryptsetupOpen = func(source string, deviceName string, keyFilePath string) error { - return nil - } - _cryptsetupClose = func(deviceName string) error { - return nil - } - _zeroFirstBlock = func(_ string, _ int) error { - return nil - } - - source := "/dev/sda" - formatTarget := "/dev/mapper/dm-crypt-name" - - expectedErr := errors.New("expected error message") - _mkfsXfs = func(arg string) error { - if arg != formatTarget { - t.Fatalf("expected args: '%v' got: '%v'", formatTarget, arg) - } - return expectedErr - } - - if _, err := EncryptDevice(context.Background(), source, "dm-crypt-name"); errors.Unwrap(err) != expectedErr { - t.Fatalf("expected err: '%v' got: '%v'", expectedErr, err) - } -} - func Test_Encrypt_Success(t *testing.T) { clearCryptTestDependencies() @@ -184,9 +144,6 @@ func Test_Encrypt_Success(t *testing.T) { _zeroFirstBlock = func(_ string, _ int) error { return nil } - _mkfsXfs = func(arg string) error { - return nil - } source := "/dev/sda" dmCryptName := "dm-crypt-name" diff --git a/internal/guest/storage/ext4/format.go b/internal/guest/storage/ext4/format.go index a8230c57a1..71f6fc014c 100644 --- a/internal/guest/storage/ext4/format.go +++ b/internal/guest/storage/ext4/format.go @@ -18,7 +18,7 @@ func mkfsExt4Command(args []string) error { } return nil } -func FormatExt4(ctx context.Context, source string) error { +func Format(ctx context.Context, source string) error { // Format source as ext4 if err := mkfsExt4Command([]string{source}); err != nil { return fmt.Errorf("mkfs.ext4 failed to format %s: %w", source, err) diff --git a/internal/guest/storage/scsi/scsi.go b/internal/guest/storage/scsi/scsi.go index ffcfb24521..5a588eaeb7 100644 --- a/internal/guest/storage/scsi/scsi.go +++ b/internal/guest/storage/scsi/scsi.go @@ -18,9 +18,12 @@ import ( "go.opencensus.io/trace" "golang.org/x/sys/unix" + "github.com/Microsoft/hcsshim/ext4/tar2ext4" "github.com/Microsoft/hcsshim/internal/guest/storage" "github.com/Microsoft/hcsshim/internal/guest/storage/crypt" dm "github.com/Microsoft/hcsshim/internal/guest/storage/devicemapper" + "github.com/Microsoft/hcsshim/internal/guest/storage/ext4" + "github.com/Microsoft/hcsshim/internal/guest/storage/xfs" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oc" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" @@ -47,8 +50,18 @@ var ( encryptDevice = crypt.EncryptDevice // cleanupCryptDevice is stubbed for unit testing `mount` cleanupCryptDevice = crypt.CleanupCryptDevice + // getDeviceFsType is stubbed for unit testing `mount` + _getDeviceFsType = getDeviceFsType // storageUnmountPath is stubbed for unit testing `unmount` storageUnmountPath = storage.UnmountPath + // tar2ext4.IsDeviceExt4 is stubbed for unit testing `getDeviceFsType` + _tar2ext4IsDeviceExt4 = tar2ext4.IsDeviceExt4 + // ext4Format is stubbed for unit testing the `EnsureFilesystem` flow + // in `mount` + ext4Format = ext4.Format + // ext4Format is stubbed for unit testing the `EnsureFilesystem` and + // `Encrypt` flow in `mount` + xfsFormat = xfs.Format ) const ( @@ -91,14 +104,24 @@ func ActualControllerNumber(_ context.Context, passedController uint8) (uint8, e return 0, fmt.Errorf("host directory not found inside %s", controllerDirPath) } +// Config represents options that are used as part of setup/cleanup before +// mounting or after unmounting a device. This does not include options +// that are sent to the mount or unmount calls. +type Config struct { + Encrypted bool + VerityInfo *guestresource.DeviceVerityInfo + EnsureFilesystem bool + Filesystem string +} + // Mount creates a mount from the SCSI device on `controller` index `lun` to // `target` // // `target` will be created. On mount failure the created `target` will be // automatically cleaned up. // -// If `encrypted` is set to true, the SCSI device will be encrypted using -// dm-crypt. +// If the config has `encrypted` is set to true, the SCSI device will be +// encrypted using dm-crypt. func Mount( ctx context.Context, controller, @@ -106,9 +129,8 @@ func Mount( partition uint64, target string, readonly bool, - encrypted bool, options []string, - verityInfo *guestresource.DeviceVerityInfo) (err error) { + config *Config) (err error) { spnCtx, span := oc.StartSpan(ctx, "scsi::Mount") defer span.End() defer func() { oc.SetSpanStatus(span, err) }() @@ -124,15 +146,35 @@ func Mount( return err } + // The `source` found by GetDevicePath can take some time + // before its actually available under `/dev/sd*`. Retry while we + // wait for `source` to show up. + for { + if _, err := osStat(source); err != nil { + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, unix.ENXIO) { + select { + case <-ctx.Done(): + log.G(ctx).Warnf("context timed out while retrying to find device %s: %v", source, err) + return err + default: + time.Sleep(10 * time.Millisecond) + continue + } + } + return err + } + break + } + if readonly { var deviceHash string - if verityInfo != nil { - deviceHash = verityInfo.RootDigest + if config.VerityInfo != nil { + deviceHash = config.VerityInfo.RootDigest } - if verityInfo != nil { + if config.VerityInfo != nil { dmVerityName := fmt.Sprintf(verityDeviceFmt, controller, lun, partition, deviceHash) - if source, err = createVerityTarget(spnCtx, source, dmVerityName, verityInfo); err != nil { + if source, err = createVerityTarget(spnCtx, source, dmVerityName, config.VerityInfo); err != nil { return err } defer func() { @@ -162,8 +204,8 @@ func Mount( data = "noload" } - mountType := "ext4" - if encrypted { + var deviceFS string + if config.Encrypted { cryptDeviceName := fmt.Sprintf(cryptDeviceFmt, controller, lun, partition) encryptedSource, err := encryptDevice(spnCtx, source, cryptDeviceName) if err != nil { @@ -176,27 +218,50 @@ func Mount( } } source = encryptedSource - mountType = "xfs" + } else { + // Get the filesystem that is already on the device (if any) and use that + // as the mountType unless `Filesystem` was given. + deviceFS, err = _getDeviceFsType(source) + if err != nil { + if config.Filesystem == "" || !errors.Is(err, ErrUnknownFilesystem) { + return fmt.Errorf("getting device's filesystem: %w", err) + } + } + log.G(ctx).WithField("filesystem", deviceFS).Debug("filesystem found on device") } - for { - if err := unixMount(source, target, mountType, flags, data); err != nil { - // The `source` found by GetDevicePath can take some time - // before its actually available under `/dev/sd*`. Retry while we - // wait for `source` to show up. - if errors.Is(err, unix.ENOENT) || errors.Is(err, unix.ENXIO) { - select { - case <-ctx.Done(): - log.G(ctx).Warnf("mount system call failed with %s, context timed out while retrying", err) - return err - default: - time.Sleep(10 * time.Millisecond) - continue + mountType := deviceFS + if config.Filesystem != "" { + mountType = config.Filesystem + } + + // if EnsureFilesystem is set, then we need to check if the device has the + // correct filesystem configured on it. If it does not, format the device + // with the corect filesystem. Right now, we only support formatting ext4 + // and xfs. + if config.EnsureFilesystem { + // compare the actual fs found on the device to the filesystem requested + if deviceFS != config.Filesystem { + // re-format device to the correct fs + switch config.Filesystem { + case "ext4": + if err := ext4Format(ctx, source); err != nil { + return fmt.Errorf("ext4 format: %w", err) } + case "xfs": + if err = xfsFormat(source); err != nil { + return fmt.Errorf("xfs format: %w", err) + } + default: + return fmt.Errorf("unsupported filesystem %s requested for device", config.Filesystem) } - return err } - break + } + + // device should already be present under /dev, so we should not get an error + // unless the command has actually errored out + if err := unixMount(source, target, mountType, flags, data); err != nil { + return fmt.Errorf("mounting: %w", err) } // remount the target to account for propagation flags @@ -220,8 +285,7 @@ func Unmount( lun uint8, partition uint64, target string, - encrypted bool, - verityInfo *guestresource.DeviceVerityInfo, + config *Config, ) (err error) { ctx, span := oc.StartSpan(ctx, "scsi::Unmount") defer span.End() @@ -238,15 +302,15 @@ func Unmount( return errors.Wrapf(err, "unmount failed: %s", target) } - if verityInfo != nil { - dmVerityName := fmt.Sprintf(verityDeviceFmt, controller, lun, partition, verityInfo.RootDigest) + if config.VerityInfo != nil { + dmVerityName := fmt.Sprintf(verityDeviceFmt, controller, lun, partition, config.VerityInfo.RootDigest) if err := removeDevice(dmVerityName); err != nil { // Ignore failures, since the path has been unmounted at this point. log.G(ctx).WithError(err).Debugf("failed to remove dm verity target: %s", dmVerityName) } } - if encrypted { + if config.Encrypted { dmCryptName := fmt.Sprintf(cryptDeviceFmt, controller, lun, partition) if err := cleanupCryptDevice(dmCryptName); err != nil { return fmt.Errorf("failed to cleanup dm-crypt target %s: %w", dmCryptName, err) @@ -358,3 +422,16 @@ func UnplugDevice(ctx context.Context, controller, lun uint8) (err error) { } return nil } + +var ErrUnknownFilesystem = errors.New("could not get device filesystem type") + +// getDeviceFsType finds a device's filesystem. +// Right now we only support checking for ext4. In the future, this may +// be expanded to support xfs or other fs types. +func getDeviceFsType(devicePath string) (string, error) { + if _tar2ext4IsDeviceExt4(devicePath) { + return "ext4", nil + } + + return "", ErrUnknownFilesystem +} diff --git a/internal/guest/storage/scsi/scsi_test.go b/internal/guest/storage/scsi/scsi_test.go index b5a176fcdb..becb94a55a 100644 --- a/internal/guest/storage/scsi/scsi_test.go +++ b/internal/guest/storage/scsi/scsi_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "io/fs" "os" "path/filepath" "testing" @@ -27,6 +28,84 @@ func clearTestDependencies() { encryptDevice = nil cleanupCryptDevice = nil storageUnmountPath = nil + _getDeviceFsType = nil + _tar2ext4IsDeviceExt4 = nil + ext4Format = nil + xfsFormat = nil +} + +// fakeFileInfo is a mock os.FileInfo that can be used to return +// in mock os calls +var _ fs.FileInfo = &fakeFileInfo{} + +type fakeFileInfo struct { + name string +} + +func (f *fakeFileInfo) Name() string { + return f.name +} + +func (f *fakeFileInfo) Size() int64 { + // fake size + return 100 +} + +func (f *fakeFileInfo) Mode() os.FileMode { + // fake mode + return os.ModeDir +} + +func (f *fakeFileInfo) ModTime() time.Time { + // fake time + return time.Now() +} + +func (f *fakeFileInfo) IsDir() bool { + // fake isDir + return false +} + +func (f *fakeFileInfo) Sys() interface{} { + return nil +} + +// fakeDirEntry is a mock os.DirEntry that can be used to return in +// the response from the mocked os.ReadDir call. +var _ os.DirEntry = &fakeDirEntry{} + +type fakeDirEntry struct { + name string +} + +func (d *fakeDirEntry) Name() string { + return d.name +} + +func (d *fakeDirEntry) IsDir() bool { + return true +} + +func (d *fakeDirEntry) Type() os.FileMode { + return os.ModeDir +} + +func (d *fakeDirEntry) Info() (os.FileInfo, error) { + return &fakeFileInfo{name: d.name}, nil +} + +func osStatNoop(source string) (os.FileInfo, error) { + return &fakeFileInfo{ + name: source, + }, nil +} + +func getDeviceFsTypeExt4(source string) (string, error) { + return "ext4", nil +} + +func getDeviceFsTypeUnknown(source string) (string, error) { + return "", ErrUnknownFilesystem } func Test_Mount_Mkdir_Fails_Error(t *testing.T) { @@ -36,11 +115,18 @@ func Test_Mount_Mkdir_Fails_Error(t *testing.T) { osMkdirAll = func(path string, perm os.FileMode) error { return expectedErr } - + _getDeviceFsType = getDeviceFsTypeExt4 getDevicePath = func(ctx context.Context, controller, lun uint8, partition uint64) (string, error) { return "", nil } + osStat = osStatNoop + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -48,9 +134,8 @@ func Test_Mount_Mkdir_Fails_Error(t *testing.T) { 0, "", false, - false, - nil, nil, + config, ); !errors.Is(err, expectedErr) { t.Fatalf("expected err: %v, got: %v", expectedErr, err) } @@ -70,6 +155,7 @@ func Test_Mount_Mkdir_ExpectedPath(t *testing.T) { } return nil } + _getDeviceFsType = getDeviceFsTypeExt4 getDevicePath = func(ctx context.Context, controller, lun uint8, partition uint64) (string, error) { return "", nil } @@ -77,7 +163,14 @@ func Test_Mount_Mkdir_ExpectedPath(t *testing.T) { // Fake the mount success return nil } + osStat = osStatNoop + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -85,9 +178,8 @@ func Test_Mount_Mkdir_ExpectedPath(t *testing.T) { 0, target, false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil error got: %v", err) } @@ -107,6 +199,7 @@ func Test_Mount_Mkdir_ExpectedPerm(t *testing.T) { } return nil } + _getDeviceFsType = getDeviceFsTypeExt4 getDevicePath = func(ctx context.Context, controller, lun uint8, partition uint64) (string, error) { return "", nil } @@ -114,6 +207,13 @@ func Test_Mount_Mkdir_ExpectedPerm(t *testing.T) { // Fake the mount success return nil } + osStat = osStatNoop + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), @@ -122,9 +222,8 @@ func Test_Mount_Mkdir_ExpectedPerm(t *testing.T) { 0, target, false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil error got: %v", err) } @@ -151,7 +250,15 @@ func Test_Mount_GetDevicePath_Valid_Controller(t *testing.T) { // Fake the mount success return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), expectedController, @@ -159,9 +266,8 @@ func Test_Mount_GetDevicePath_Valid_Controller(t *testing.T) { 0, "/fake/path", false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil error got: %v", err) } @@ -188,6 +294,15 @@ func Test_Mount_GetDevicePath_Valid_Lun(t *testing.T) { // Fake the mount success return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), @@ -196,9 +311,8 @@ func Test_Mount_GetDevicePath_Valid_Lun(t *testing.T) { 0, "/fake/path", false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil error got: %v", err) } @@ -225,6 +339,15 @@ func Test_Mount_GetDevicePath_Valid_Partition(t *testing.T) { // Fake the mount success return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), @@ -233,9 +356,8 @@ func Test_Mount_GetDevicePath_Valid_Partition(t *testing.T) { expectedPartition, "/fake/path", false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil error got: %v", err) } @@ -265,7 +387,15 @@ func Test_Mount_Calls_RemoveAll_OnMountFailure(t *testing.T) { // Fake the mount failure to test remove is called return expectedErr } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -273,9 +403,8 @@ func Test_Mount_Calls_RemoveAll_OnMountFailure(t *testing.T) { 0, target, false, - false, - nil, nil, + config, ); !errors.Is(err, expectedErr) { t.Fatalf("expected err: %v, got: %v", expectedErr, err) } @@ -304,7 +433,25 @@ func Test_Mount_Valid_Source(t *testing.T) { } return nil } - err := Mount(context.Background(), 0, 0, 0, "/fake/path", false, false, nil, nil) + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } + err := Mount( + context.Background(), + 0, + 0, + 0, + "/fake/path", + false, + nil, + config, + ) if err != nil { t.Fatalf("expected nil err, got: %v", err) } @@ -330,7 +477,15 @@ func Test_Mount_Valid_Target(t *testing.T) { } return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -338,9 +493,8 @@ func Test_Mount_Valid_Target(t *testing.T) { 0, expectedTarget, false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil err, got: %v", err) } @@ -366,7 +520,15 @@ func Test_Mount_Valid_FSType(t *testing.T) { } return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -374,9 +536,8 @@ func Test_Mount_Valid_FSType(t *testing.T) { 0, "/fake/path", false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil err, got: %v", err) } @@ -402,7 +563,15 @@ func Test_Mount_Valid_Flags(t *testing.T) { } return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -410,9 +579,8 @@ func Test_Mount_Valid_Flags(t *testing.T) { 0, "/fake/path", false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil err, got: %v", err) } @@ -438,7 +606,15 @@ func Test_Mount_Readonly_Valid_Flags(t *testing.T) { } return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -446,9 +622,8 @@ func Test_Mount_Readonly_Valid_Flags(t *testing.T) { 0, "/fake/path", true, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil err, got: %v", err) } @@ -473,7 +648,15 @@ func Test_Mount_Valid_Data(t *testing.T) { } return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -481,9 +664,8 @@ func Test_Mount_Valid_Data(t *testing.T) { 0, "/fake/path", false, - false, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil err, got: %v", err) } @@ -509,7 +691,15 @@ func Test_Mount_Readonly_Valid_Data(t *testing.T) { } return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -517,14 +707,91 @@ func Test_Mount_Readonly_Valid_Data(t *testing.T) { 0, "/fake/path", true, - false, nil, + config, + ); err != nil { + t.Fatalf("expected nil err, got: %v", err) + } +} + +func Test_Mount_EnsureFilesystem_Success(t *testing.T) { + clearTestDependencies() + + // NOTE: Do NOT set osRemoveAll because the mount succeeds. Expect it not to + // be called. + + osMkdirAll = func(path string, perm os.FileMode) error { + return nil + } + getDevicePath = func(ctx context.Context, controller, lun uint8, partition uint64) (string, error) { + return "", nil + } + unixMount = func(source string, target string, fstype string, flags uintptr, data string) error { + return nil + } + osStat = osStatNoop + ext4Format = func(ctx context.Context, source string) error { + return nil + } + + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: true, + Filesystem: "ext4", + } + if err := Mount( + context.Background(), + 0, + 0, + 0, + "/fake/path", + true, nil, + config, ); err != nil { t.Fatalf("expected nil err, got: %v", err) } } +func Test_Mount_EnsureFilesystem_Unsupported(t *testing.T) { + clearTestDependencies() + + osMkdirAll = func(path string, perm os.FileMode) error { + return nil + } + getDevicePath = func(ctx context.Context, controller, lun uint8, partition uint64) (string, error) { + return "", nil + } + unixMount = func(source string, target string, fstype string, flags uintptr, data string) error { + return nil + } + osStat = osStatNoop + osRemoveAll = func(string) error { + return nil + } + _getDeviceFsType = getDeviceFsTypeUnknown + config := &Config{ + Encrypted: false, + VerityInfo: nil, + EnsureFilesystem: true, + Filesystem: "fake", + } + if err := Mount( + context.Background(), + 0, + 0, + 0, + "/fake/path", + true, + nil, + config, + ); err == nil { + t.Fatal("expected to get an error from unsupported fs type") + } +} + // dm-verity tests func Test_CreateVerityTarget_And_Mount_Called_With_Correct_Parameters(t *testing.T) { @@ -567,7 +834,15 @@ func Test_CreateVerityTarget_And_Mount_Called_With_Correct_Parameters(t *testing } return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: vInfo, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -575,9 +850,8 @@ func Test_CreateVerityTarget_And_Mount_Called_With_Correct_Parameters(t *testing 0, expectedTarget, true, - false, nil, - vInfo, + config, ); err != nil { t.Fatalf("unexpected error during Mount: %s", err) } @@ -616,7 +890,15 @@ func Test_osMkdirAllFails_And_RemoveDevice_Called(t *testing.T) { } return nil } + osStat = osStatNoop + _getDeviceFsType = getDeviceFsTypeExt4 + config := &Config{ + Encrypted: false, + VerityInfo: verityInfo, + EnsureFilesystem: false, + Filesystem: "", + } if err := Mount( context.Background(), 0, @@ -624,9 +906,8 @@ func Test_osMkdirAllFails_And_RemoveDevice_Called(t *testing.T) { 0, "/foo", true, - false, nil, - verityInfo, + config, ); err != expectedError { t.Fatalf("expected Mount error %s, got %s", expectedError, err) } @@ -648,13 +929,30 @@ func Test_Mount_EncryptDevice_Called(t *testing.T) { return nil } encryptDeviceCalled := false + expectedCryptTarget := fmt.Sprintf(cryptDeviceFmt, 0, 0, 0) + expectedDevicePath := "/dev/mapper/" + expectedCryptTarget + encryptDevice = func(_ context.Context, source string, devName string) (string, error) { - expectedCryptTarget := fmt.Sprintf(cryptDeviceFmt, 0, 0, 0) if devName != expectedCryptTarget { t.Fatalf("expected crypt device %q got %q", expectedCryptTarget, devName) } encryptDeviceCalled = true - return "", nil + return expectedDevicePath, nil + } + osStat = osStatNoop + + xfsFormat = func(arg string) error { + if arg != expectedDevicePath { + t.Fatalf("expected args: '%v' got: '%v'", expectedDevicePath, arg) + } + return nil + } + + config := &Config{ + Encrypted: true, + VerityInfo: nil, + EnsureFilesystem: true, + Filesystem: "xfs", } if err := Mount( context.Background(), @@ -663,9 +961,8 @@ func Test_Mount_EncryptDevice_Called(t *testing.T) { 0, "/fake/path", false, - true, - nil, nil, + config, ); err != nil { t.Fatalf("expected nil error, got: %s", err) } @@ -674,6 +971,59 @@ func Test_Mount_EncryptDevice_Called(t *testing.T) { } } +func Test_Mount_EncryptDevice_Mkfs_Error(t *testing.T) { + clearTestDependencies() + + osMkdirAll = func(string, os.FileMode) error { + return nil + } + getDevicePath = func(context.Context, uint8, uint8, uint64) (string, error) { + return "", nil + } + unixMount = func(string, string, string, uintptr, string) error { + return nil + } + expectedCryptTarget := fmt.Sprintf(cryptDeviceFmt, 0, 0, 0) + expectedDevicePath := "/dev/mapper/" + expectedCryptTarget + + encryptDevice = func(_ context.Context, source string, devName string) (string, error) { + if devName != expectedCryptTarget { + t.Fatalf("expected crypt device %q got %q", expectedCryptTarget, devName) + } + return expectedDevicePath, nil + } + osStat = osStatNoop + + xfsFormat = func(arg string) error { + if arg != expectedDevicePath { + t.Fatalf("expected args: '%v' got: '%v'", expectedDevicePath, arg) + } + return fmt.Errorf("fake error") + } + osRemoveAll = func(string) error { + return nil + } + + config := &Config{ + Encrypted: true, + VerityInfo: nil, + EnsureFilesystem: true, + Filesystem: "xfs", + } + if err := Mount( + context.Background(), + 0, + 0, + 0, + "/fake/path", + false, + nil, + config, + ); err == nil { + t.Fatalf("expected to fail") + } +} + func Test_Mount_RemoveAllCalled_When_EncryptDevice_Fails(t *testing.T) { clearTestDependencies() @@ -695,7 +1045,18 @@ func Test_Mount_RemoveAllCalled_When_EncryptDevice_Fails(t *testing.T) { removeAllCalled = true return nil } + osStat = osStatNoop + xfsFormat = func(arg string) error { + return nil + } + + config := &Config{ + Encrypted: true, + VerityInfo: nil, + EnsureFilesystem: true, + Filesystem: "xfs", + } err := Mount( context.Background(), 0, @@ -703,9 +1064,8 @@ func Test_Mount_RemoveAllCalled_When_EncryptDevice_Fails(t *testing.T) { 0, "/fake/path", false, - true, - nil, nil, + config, ) if err == nil { t.Fatalf("expected to fail") @@ -733,8 +1093,19 @@ func Test_Unmount_CleanupCryptDevice_Called(t *testing.T) { cleanupCryptDeviceCalled = true return nil } + osStat = osStatNoop - if err := Unmount(context.Background(), 0, 0, 0, "/fake/path", true, nil); err != nil { + config := &Config{ + Encrypted: true, + } + if err := Unmount( + context.Background(), + 0, + 0, + 0, + "/fake/path", + config, + ); err != nil { t.Fatalf("unexpected error: %s", err) } if !cleanupCryptDeviceCalled { @@ -742,62 +1113,6 @@ func Test_Unmount_CleanupCryptDevice_Called(t *testing.T) { } } -// fakeFileInfo is a mock os.FileInfo that can be used to return -// in mock os calls -type fakeFileInfo struct { - name string -} - -func (f *fakeFileInfo) Name() string { - return f.name -} - -func (f *fakeFileInfo) Size() int64 { - // fake size - return 100 -} - -func (f *fakeFileInfo) Mode() os.FileMode { - // fake mode - return os.ModeDir -} - -func (f *fakeFileInfo) ModTime() time.Time { - // fake time - return time.Now() -} - -func (f *fakeFileInfo) IsDir() bool { - // fake isDir - return false -} - -func (f *fakeFileInfo) Sys() interface{} { - return nil -} - -// fakeDirEntry is a mock os.DirEntry that can be used to return in -// the response from the mocked os.ReadDir call. -type fakeDirEntry struct { - name string -} - -func (d *fakeDirEntry) Name() string { - return d.name -} - -func (d *fakeDirEntry) IsDir() bool { - return true -} - -func (d *fakeDirEntry) Type() os.FileMode { - return os.ModeDir -} - -func (d *fakeDirEntry) Info() (os.FileInfo, error) { - return &fakeFileInfo{name: d.name}, nil -} - func Test_GetDevicePath_Device_With_Partition(t *testing.T) { clearTestDependencies() @@ -899,3 +1214,34 @@ func Test_GetDevicePath_Device_No_Partition_Error(t *testing.T) { t.Fatalf("expected to get an error, instead got %v", actualPath) } } + +func Test_GetDeviceFsType_Success(t *testing.T) { + clearTestDependencies() + + devicePath := "/dev/sda" + _tar2ext4IsDeviceExt4 = func(string) bool { + return true + } + + fsType, err := getDeviceFsType(devicePath) + if err != nil { + t.Fatal(err) + } + if fsType != "ext4" { + t.Fatalf("expected to get a filesystem type of ext4, instead got %s", fsType) + } +} + +func Test_GetDeviceFsType_Error(t *testing.T) { + clearTestDependencies() + + devicePath := "/dev/sda" + _tar2ext4IsDeviceExt4 = func(string) bool { + return false + } + + fsType, err := getDeviceFsType(devicePath) + if err == nil { + t.Fatalf("expected to return a failure from call to getDeviceFsType, instead got %s", fsType) + } +} diff --git a/internal/guest/storage/xfs/format.go b/internal/guest/storage/xfs/format.go new file mode 100644 index 0000000000..69c567bd12 --- /dev/null +++ b/internal/guest/storage/xfs/format.go @@ -0,0 +1,17 @@ +package xfs + +import ( + "fmt" + "os/exec" +) + +// Format formats `source` by invoking mkfs.xfs. +func Format(devicePath string) error { + args := []string{"-f", devicePath} + cmd := exec.Command("mkfs.xfs", args...) + output, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("mkfs.xfs failed with %s: %w", string(output), err) + } + return nil +} diff --git a/internal/layers/layers.go b/internal/layers/layers.go index 8eeee5f27b..ab33857e84 100644 --- a/internal/layers/layers.go +++ b/internal/layers/layers.go @@ -124,12 +124,23 @@ func MountLCOWLayers(ctx context.Context, containerID string, layers *LCOWLayers } log.G(ctx).WithField("hostPath", hostPath).Debug("mounting scratch VHD") + mConfig := &scsi.MountConfig{ + Encrypted: vm.ScratchEncryptionEnabled(), + // For scratch disks, we support formatting the disk if it is not already + // formatted. + EnsureFileystem: true, + Filesystem: "ext4", + } + if vm.ScratchEncryptionEnabled() { + // Encrypted scratch devices are formatted with xfs + mConfig.Filesystem = "xfs" + } scsiMount, err := vm.SCSIManager.AddVirtualDisk( ctx, hostPath, false, vm.ID(), - &scsi.MountConfig{Encrypted: vm.ScratchEncryptionEnabled()}, + mConfig, ) if err != nil { return "", "", nil, fmt.Errorf("failed to add SCSI scratch VHD: %s", err) diff --git a/internal/protocol/guestresource/resources.go b/internal/protocol/guestresource/resources.go index 07c4319696..270e508640 100644 --- a/internal/protocol/guestresource/resources.go +++ b/internal/protocol/guestresource/resources.go @@ -78,14 +78,16 @@ type SCSIDevice struct { // LCOWMappedVirtualDisk represents a disk on the host which is mapped into a // directory in the guest in the V2 schema. type LCOWMappedVirtualDisk struct { - MountPath string `json:"MountPath,omitempty"` - Lun uint8 `json:"Lun,omitempty"` - Controller uint8 `json:"Controller,omitempty"` - Partition uint64 `json:"Partition,omitempty"` - ReadOnly bool `json:"ReadOnly,omitempty"` - Encrypted bool `json:"Encrypted,omitempty"` - Options []string `json:"Options,omitempty"` - VerityInfo *DeviceVerityInfo `json:"VerityInfo,omitempty"` + MountPath string `json:"MountPath,omitempty"` + Lun uint8 `json:"Lun,omitempty"` + Controller uint8 `json:"Controller,omitempty"` + Partition uint64 `json:"Partition,omitempty"` + ReadOnly bool `json:"ReadOnly,omitempty"` + Encrypted bool `json:"Encrypted,omitempty"` + Options []string `json:"Options,omitempty"` + VerityInfo *DeviceVerityInfo `json:"VerityInfo,omitempty"` + EnsureFilesystem bool `json:"EnsureFilesystem,omitempty"` + Filesystem string `json:"Filesystem,omitempty"` } type WCOWMappedVirtualDisk struct { diff --git a/internal/uvm/scsi/backend.go b/internal/uvm/scsi/backend.go index f4c4fb0867..7002f2f1b7 100644 --- a/internal/uvm/scsi/backend.go +++ b/internal/uvm/scsi/backend.go @@ -174,8 +174,10 @@ func mountRequest(controller, lun uint, path string, config *mountConfig, osType if controller != 0 { return guestrequest.ModificationRequest{}, errors.New("WCOW only supports SCSI controller 0") } - if config.encrypted || config.verity != nil || len(config.options) != 0 { - return guestrequest.ModificationRequest{}, errors.New("WCOW does not support encrypted, verity, or guest options on mounts") + if config.encrypted || config.verity != nil || len(config.options) != 0 || + config.ensureFileystem || config.filesystem != "" || config.partition != 0 { + return guestrequest.ModificationRequest{}, + errors.New("WCOW does not support encrypted, verity, guest options, partitions, specifying mount filesystem, or ensuring filesystem on mounts") } req.Settings = guestresource.WCOWMappedVirtualDisk{ ContainerPath: path, @@ -183,14 +185,16 @@ func mountRequest(controller, lun uint, path string, config *mountConfig, osType } case "linux": req.Settings = guestresource.LCOWMappedVirtualDisk{ - MountPath: path, - Controller: uint8(controller), - Lun: uint8(lun), - Partition: config.partition, - ReadOnly: config.readOnly, - Encrypted: config.encrypted, - Options: config.options, - VerityInfo: config.verity, + MountPath: path, + Controller: uint8(controller), + Lun: uint8(lun), + Partition: config.partition, + ReadOnly: config.readOnly, + Encrypted: config.encrypted, + Options: config.options, + VerityInfo: config.verity, + EnsureFilesystem: config.ensureFileystem, + Filesystem: config.filesystem, } default: return guestrequest.ModificationRequest{}, fmt.Errorf("unsupported os type: %s", osType) diff --git a/internal/uvm/scsi/manager.go b/internal/uvm/scsi/manager.go index ddd285cd4c..642d7988b0 100644 --- a/internal/uvm/scsi/manager.go +++ b/internal/uvm/scsi/manager.go @@ -66,9 +66,25 @@ func NewManager( // MountConfig specifies the options to apply for mounting a SCSI device in // the guest OS. type MountConfig struct { + // Partition is the target partition index on a partitioned device to + // mount. Partitions are 1-based indexed. + // This is only supported for LCOW. Partition uint64 + // Encrypted indicates if we should encrypt the device with dm-crypt. + // This is only supported for LCOW. Encrypted bool - Options []string + // Options are options such as propagation options, flags, or data to + // pass to the mount call. + // This is only supported for LCOW. + Options []string + // EnsureFilesystem indicates to format the mount as `Filesystem` + // if it is not already formatted with that fs type. + // This is only supported for LCOW. + EnsureFileystem bool + // Filesystem is the target filesystem that a device will be + // mounted as. + // This is only supported for LCOW. + Filesystem string } // Mount represents a SCSI device that has been attached to a VM, and potentially @@ -137,11 +153,13 @@ func (m *Manager) AddVirtualDisk( var mcInternal *mountConfig if mc != nil { mcInternal = &mountConfig{ - partition: mc.Partition, - readOnly: readOnly, - encrypted: mc.Encrypted, - options: mc.Options, - verity: readVerityInfo(ctx, hostPath), + partition: mc.Partition, + readOnly: readOnly, + encrypted: mc.Encrypted, + options: mc.Options, + verity: readVerityInfo(ctx, hostPath), + ensureFileystem: mc.EnsureFileystem, + filesystem: mc.Filesystem, } } return m.add(ctx, @@ -181,11 +199,13 @@ func (m *Manager) AddPhysicalDisk( var mcInternal *mountConfig if mc != nil { mcInternal = &mountConfig{ - partition: mc.Partition, - readOnly: readOnly, - encrypted: mc.Encrypted, - options: mc.Options, - verity: readVerityInfo(ctx, hostPath), + partition: mc.Partition, + readOnly: readOnly, + encrypted: mc.Encrypted, + options: mc.Options, + verity: readVerityInfo(ctx, hostPath), + ensureFileystem: mc.EnsureFileystem, + filesystem: mc.Filesystem, } } return m.add(ctx, @@ -224,10 +244,12 @@ func (m *Manager) AddExtensibleVirtualDisk( var mcInternal *mountConfig if mc != nil { mcInternal = &mountConfig{ - partition: mc.Partition, - readOnly: readOnly, - encrypted: mc.Encrypted, - options: mc.Options, + partition: mc.Partition, + readOnly: readOnly, + encrypted: mc.Encrypted, + options: mc.Options, + ensureFileystem: mc.EnsureFileystem, + filesystem: mc.Filesystem, } } return m.add(ctx, diff --git a/internal/uvm/scsi/mount.go b/internal/uvm/scsi/mount.go index 849a711b09..408e62b5ec 100644 --- a/internal/uvm/scsi/mount.go +++ b/internal/uvm/scsi/mount.go @@ -38,11 +38,13 @@ type mount struct { } type mountConfig struct { - partition uint64 - readOnly bool - encrypted bool - verity *guestresource.DeviceVerityInfo - options []string + partition uint64 + readOnly bool + encrypted bool + verity *guestresource.DeviceVerityInfo + options []string + ensureFileystem bool + filesystem string } func (mm *mountManager) mount(ctx context.Context, controller, lun uint, c *mountConfig) (_ string, err error) {