diff options
author | Chong Cai <chongc@google.com> | 2020-10-09 17:27:24 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-10-09 17:29:34 -0700 |
commit | 5389e441a5480450eb430adf10a552dd3dd1d4f7 (patch) | |
tree | 963102776a916f423eaa71aff80d29c396eb2376 /pkg/sentry | |
parent | 79a5910c04ed18901f755588003ca62d0646b763 (diff) |
Include stat in Verity hash
PiperOrigin-RevId: 336395445
Diffstat (limited to 'pkg/sentry')
-rw-r--r-- | pkg/sentry/fsimpl/verity/filesystem.go | 145 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/verity/verity.go | 74 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/verity/verity_test.go | 103 |
3 files changed, 280 insertions, 42 deletions
diff --git a/pkg/sentry/fsimpl/verity/filesystem.go b/pkg/sentry/fsimpl/verity/filesystem.go index a560b0797..7779271a9 100644 --- a/pkg/sentry/fsimpl/verity/filesystem.go +++ b/pkg/sentry/fsimpl/verity/filesystem.go @@ -20,6 +20,7 @@ import ( "io" "strconv" "strings" + "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -251,11 +252,35 @@ func (fs *filesystem) verifyChild(ctx context.Context, parent *dentry, child *de Ctx: ctx, } + parentStat, err := vfsObj.StatAt(ctx, fs.creds, &vfs.PathOperation{ + Root: parent.lowerVD, + Start: parent.lowerVD, + }, &vfs.StatOptions{}) + if err == syserror.ENOENT { + return nil, alertIntegrityViolation(err, fmt.Sprintf("Failed to get parent stat for %s: %v", childPath, err)) + } + if err != nil { + return nil, err + } + // Since we are verifying against a directory Merkle tree, buf should // contain the root hash of the children in the parent Merkle tree when // Verify returns with success. var buf bytes.Buffer - if _, err := merkletree.Verify(&buf, &fdReader, &fdReader, int64(parentSize), int64(offset), int64(merkletree.DigestSize()), parent.rootHash, true /* dataAndTreeInSameFile */); err != nil && err != io.EOF { + if _, err := merkletree.Verify(&merkletree.VerifyParams{ + Out: &buf, + File: &fdReader, + Tree: &fdReader, + Size: int64(parentSize), + Name: parent.name, + Mode: uint32(parentStat.Mode), + UID: parentStat.UID, + GID: parentStat.GID, + ReadOffset: int64(offset), + ReadSize: int64(merkletree.DigestSize()), + ExpectedRoot: parent.rootHash, + DataAndTreeInSameFile: true, + }); err != nil && err != io.EOF { return nil, alertIntegrityViolation(syserror.EIO, fmt.Sprintf("Verification for %s failed: %v", childPath, err)) } @@ -266,6 +291,84 @@ func (fs *filesystem) verifyChild(ctx context.Context, parent *dentry, child *de return child, nil } +// verifyStat verifies the stat against the verified root hash. The mode/uid/gid +// of the file is cached after verified. +func (fs *filesystem) verifyStat(ctx context.Context, d *dentry, stat linux.Statx) error { + vfsObj := fs.vfsfs.VirtualFilesystem() + + // Get the path to the child dentry. This is only used to provide path + // information in failure case. + childPath, err := vfsObj.PathnameWithDeleted(ctx, d.fs.rootDentry.lowerVD, d.lowerVD) + if err != nil { + return err + } + + verityMu.RLock() + defer verityMu.RUnlock() + + fd, err := vfsObj.OpenAt(ctx, fs.creds, &vfs.PathOperation{ + Root: d.lowerMerkleVD, + Start: d.lowerMerkleVD, + }, &vfs.OpenOptions{ + Flags: linux.O_RDONLY, + }) + if err == syserror.ENOENT { + return alertIntegrityViolation(err, fmt.Sprintf("Failed to open merkle file for %s: %v", childPath, err)) + } + if err != nil { + return err + } + + merkleSize, err := fd.GetXattr(ctx, &vfs.GetXattrOptions{ + Name: merkleSizeXattr, + Size: sizeOfStringInt32, + }) + + if err == syserror.ENODATA { + return alertIntegrityViolation(err, fmt.Sprintf("Failed to get xattr %s for merkle file of %s: %v", merkleSizeXattr, childPath, err)) + } + if err != nil { + return err + } + + size, err := strconv.Atoi(merkleSize) + if err != nil { + return alertIntegrityViolation(syserror.EINVAL, fmt.Sprintf("Failed to convert xattr %s for %s to int: %v", merkleSizeXattr, childPath, err)) + } + + fdReader := vfs.FileReadWriteSeeker{ + FD: fd, + Ctx: ctx, + } + + var buf bytes.Buffer + params := &merkletree.VerifyParams{ + Out: &buf, + Tree: &fdReader, + Size: int64(size), + Name: d.name, + Mode: uint32(stat.Mode), + UID: stat.UID, + GID: stat.GID, + ReadOffset: 0, + // Set read size to 0 so only the metadata is verified. + ReadSize: 0, + ExpectedRoot: d.rootHash, + DataAndTreeInSameFile: false, + } + if atomic.LoadUint32(&d.mode)&linux.S_IFMT == linux.S_IFDIR { + params.DataAndTreeInSameFile = true + } + + if _, err := merkletree.Verify(params); err != nil && err != io.EOF { + return alertIntegrityViolation(err, fmt.Sprintf("Verification stat for %s failed: %v", childPath, err)) + } + d.mode = uint32(stat.Mode) + d.uid = stat.UID + d.gid = stat.GID + return nil +} + // Preconditions: fs.renameMu must be locked. d.dirMu must be locked. func (fs *filesystem) getChildLocked(ctx context.Context, parent *dentry, name string, ds **[]*dentry) (*dentry, error) { if child, ok := parent.children[name]; ok { @@ -274,9 +377,27 @@ func (fs *filesystem) getChildLocked(ctx context.Context, parent *dentry, name s // runtime enable is allowed and the parent directory is // enabled, we should verify the child root hash here because // it may be cached before enabled. - if fs.allowRuntimeEnable && len(parent.rootHash) != 0 { - if _, err := fs.verifyChild(ctx, parent, child); err != nil { - return nil, err + if fs.allowRuntimeEnable { + if isEnabled(parent) { + if _, err := fs.verifyChild(ctx, parent, child); err != nil { + return nil, err + } + } + if isEnabled(child) { + vfsObj := fs.vfsfs.VirtualFilesystem() + mask := uint32(linux.STATX_TYPE | linux.STATX_MODE | linux.STATX_UID | linux.STATX_GID) + stat, err := vfsObj.StatAt(ctx, fs.creds, &vfs.PathOperation{ + Root: child.lowerVD, + Start: child.lowerVD, + }, &vfs.StatOptions{ + Mask: mask, + }) + if err != nil { + return nil, err + } + if err := fs.verifyStat(ctx, child, stat); err != nil { + return nil, err + } } } return child, nil @@ -426,7 +547,6 @@ func (fs *filesystem) lookupAndVerifyLocked(ctx context.Context, parent *dentry, child.parent = parent child.name = name - // TODO(b/162788573): Verify child metadata. child.mode = uint32(stat.Mode) child.uid = stat.UID child.gid = stat.GID @@ -434,12 +554,18 @@ func (fs *filesystem) lookupAndVerifyLocked(ctx context.Context, parent *dentry, // Verify child root hash. This should always be performed unless in // allowRuntimeEnable mode and the parent directory hasn't been enabled // yet. - if !(fs.allowRuntimeEnable && len(parent.rootHash) == 0) { + if isEnabled(parent) { if _, err := fs.verifyChild(ctx, parent, child); err != nil { child.destroyLocked(ctx) return nil, err } } + if isEnabled(child) { + if err := fs.verifyStat(ctx, child, stat); err != nil { + child.destroyLocked(ctx) + return nil, err + } + } return child, nil } @@ -771,6 +897,8 @@ func (fs *filesystem) SetStatAt(ctx context.Context, rp *vfs.ResolvingPath, opts } // StatAt implements vfs.FilesystemImpl.StatAt. +// TODO(b/170157489): Investigate whether stats other than Mode/UID/GID should +// be verified. func (fs *filesystem) StatAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.StatOptions) (linux.Statx, error) { var ds *[]*dentry fs.renameMu.RLock() @@ -788,6 +916,11 @@ func (fs *filesystem) StatAt(ctx context.Context, rp *vfs.ResolvingPath, opts vf if err != nil { return linux.Statx{}, err } + if isEnabled(d) { + if err := fs.verifyStat(ctx, d, stat); err != nil { + return linux.Statx{}, err + } + } return stat, nil } diff --git a/pkg/sentry/fsimpl/verity/verity.go b/pkg/sentry/fsimpl/verity/verity.go index fc5eabbca..3eb972237 100644 --- a/pkg/sentry/fsimpl/verity/verity.go +++ b/pkg/sentry/fsimpl/verity/verity.go @@ -142,6 +142,14 @@ func (FilesystemType) Name() string { return Name } +// isEnabled checks whether the target is enabled with verity features. It +// should always be true if runtime enable is not allowed. In runtime enable +// mode, it returns true if the target has been enabled with +// ioctl(FS_IOC_ENABLE_VERITY). +func isEnabled(d *dentry) bool { + return !d.fs.allowRuntimeEnable || len(d.rootHash) != 0 +} + // alertIntegrityViolation alerts a violation of integrity, which usually means // unexpected modification to the file system is detected. In // noCrashOnVerificationFailure mode, it returns an error, otherwise it panic. @@ -245,12 +253,17 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt return nil, nil, err } - // TODO(b/162788573): Verify Metadata. d.mode = uint32(stat.Mode) d.uid = stat.UID d.gid = stat.GID - d.rootHash = make([]byte, len(iopts.RootHash)) + + if !fs.allowRuntimeEnable { + if err := fs.verifyStat(ctx, d, stat); err != nil { + return nil, nil, err + } + } + copy(d.rootHash, iopts.RootHash) d.vfsd.Init(d) @@ -488,6 +501,11 @@ func (fd *fileDescription) Stat(ctx context.Context, opts vfs.StatOptions) (linu if err != nil { return linux.Statx{}, err } + if isEnabled(fd.d) { + if err := fd.d.fs.verifyStat(ctx, fd.d, stat); err != nil { + return linux.Statx{}, err + } + } return stat, nil } @@ -516,8 +534,10 @@ func (fd *fileDescription) generateMerkle(ctx context.Context) ([]byte, uint64, FD: fd.merkleWriter, Ctx: ctx, } - var rootHash []byte - var dataSize uint64 + params := &merkletree.GenerateParams{ + TreeReader: &merkleReader, + TreeWriter: &merkleWriter, + } switch atomic.LoadUint32(&fd.d.mode) & linux.S_IFMT { case linux.S_IFREG: @@ -528,12 +548,14 @@ func (fd *fileDescription) generateMerkle(ctx context.Context) ([]byte, uint64, if err != nil { return nil, 0, err } - dataSize = stat.Size - rootHash, err = merkletree.Generate(&fdReader, int64(dataSize), &merkleReader, &merkleWriter, false /* dataAndTreeInSameFile */) - if err != nil { - return nil, 0, err - } + params.File = &fdReader + params.Size = int64(stat.Size) + params.Name = fd.d.name + params.Mode = uint32(stat.Mode) + params.UID = stat.UID + params.GID = stat.GID + params.DataAndTreeInSameFile = false case linux.S_IFDIR: // For a directory, generate a Merkle tree based on the root // hashes of its children that has already been written to the @@ -542,18 +564,27 @@ func (fd *fileDescription) generateMerkle(ctx context.Context) ([]byte, uint64, if err != nil { return nil, 0, err } - dataSize = merkleStat.Size - rootHash, err = merkletree.Generate(&merkleReader, int64(dataSize), &merkleReader, &merkleWriter, true /* dataAndTreeInSameFile */) + params.Size = int64(merkleStat.Size) + + stat, err := fd.lowerFD.Stat(ctx, vfs.StatOptions{}) if err != nil { return nil, 0, err } + + params.File = &merkleReader + params.Name = fd.d.name + params.Mode = uint32(stat.Mode) + params.UID = stat.UID + params.GID = stat.GID + params.DataAndTreeInSameFile = true default: // TODO(b/167728857): Investigate whether and how we should // enable other types of file. return nil, 0, syserror.EINVAL } - return rootHash, dataSize, nil + rootHash, err := merkletree.Generate(params) + return rootHash, uint64(params.Size), err } // enableVerity enables verity features on fd by generating a Merkle tree file @@ -688,7 +719,7 @@ func (fd *fileDescription) Ioctl(ctx context.Context, uio usermem.IO, args arch. func (fd *fileDescription) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts vfs.ReadOptions) (int64, error) { // No need to verify if the file is not enabled yet in // allowRuntimeEnable mode. - if fd.d.fs.allowRuntimeEnable && len(fd.d.rootHash) == 0 { + if !isEnabled(fd.d) { return fd.lowerFD.PRead(ctx, dst, offset, opts) } @@ -725,9 +756,22 @@ func (fd *fileDescription) PRead(ctx context.Context, dst usermem.IOSequence, of Ctx: ctx, } - n, err := merkletree.Verify(dst.Writer(ctx), &dataReader, &merkleReader, int64(size), offset, dst.NumBytes(), fd.d.rootHash, false /* dataAndTreeInSameFile */) + n, err := merkletree.Verify(&merkletree.VerifyParams{ + Out: dst.Writer(ctx), + File: &dataReader, + Tree: &merkleReader, + Size: int64(size), + Name: fd.d.name, + Mode: fd.d.mode, + UID: fd.d.uid, + GID: fd.d.gid, + ReadOffset: offset, + ReadSize: dst.NumBytes(), + ExpectedRoot: fd.d.rootHash, + DataAndTreeInSameFile: false, + }) if err != nil { - return 0, alertIntegrityViolation(syserror.EINVAL, fmt.Sprintf("Verification failed: %v", err)) + return 0, alertIntegrityViolation(syserror.EIO, fmt.Sprintf("Verification failed: %v", err)) } return n, err } diff --git a/pkg/sentry/fsimpl/verity/verity_test.go b/pkg/sentry/fsimpl/verity/verity_test.go index 8bcc14131..8d0926bc4 100644 --- a/pkg/sentry/fsimpl/verity/verity_test.go +++ b/pkg/sentry/fsimpl/verity/verity_test.go @@ -41,11 +41,11 @@ const maxDataSize = 100000 // newVerityRoot creates a new verity mount, and returns the root. The // underlying file system is tmpfs. If the error is not nil, then cleanup // should be called when the root is no longer needed. -func newVerityRoot(ctx context.Context) (*vfs.VirtualFilesystem, vfs.VirtualDentry, func(), error) { +func newVerityRoot(ctx context.Context, t *testing.T) (*vfs.VirtualFilesystem, vfs.VirtualDentry, error) { rand.Seed(time.Now().UnixNano()) vfsObj := &vfs.VirtualFilesystem{} if err := vfsObj.Init(ctx); err != nil { - return nil, vfs.VirtualDentry{}, nil, fmt.Errorf("VFS init: %v", err) + return nil, vfs.VirtualDentry{}, fmt.Errorf("VFS init: %v", err) } vfsObj.MustRegisterFilesystemType("verity", FilesystemType{}, &vfs.RegisterFilesystemTypeOptions{ @@ -67,13 +67,15 @@ func newVerityRoot(ctx context.Context) (*vfs.VirtualFilesystem, vfs.VirtualDent }, }) if err != nil { - return nil, vfs.VirtualDentry{}, nil, fmt.Errorf("NewMountNamespace: %v", err) + return nil, vfs.VirtualDentry{}, fmt.Errorf("NewMountNamespace: %v", err) } root := mntns.Root() - return vfsObj, root, func() { + t.Helper() + t.Cleanup(func() { root.DecRef(ctx) mntns.DecRef(ctx) - }, nil + }) + return vfsObj, root, nil } // newFileFD creates a new file in the verity mount, and returns the FD. The FD @@ -143,11 +145,10 @@ func corruptRandomBit(ctx context.Context, fd *vfs.FileDescription, size int) er // file and the root Merkle tree file exist. func TestOpen(t *testing.T) { ctx := contexttest.Context(t) - vfsObj, root, cleanup, err := newVerityRoot(ctx) + vfsObj, root, err := newVerityRoot(ctx, t) if err != nil { t.Fatalf("newVerityRoot: %v", err) } - defer cleanup() filename := "verity-test-file" if _, _, err := newFileFD(ctx, vfsObj, root, filename, 0644); err != nil { @@ -178,15 +179,14 @@ func TestOpen(t *testing.T) { } } -// TestUntouchedFileSucceeds ensures that read from an untouched verity file +// TestUnmodifiedFileSucceeds ensures that read from an untouched verity file // succeeds after enabling verity for it. -func TestReadUntouchedFileSucceeds(t *testing.T) { +func TestReadUnmodifiedFileSucceeds(t *testing.T) { ctx := contexttest.Context(t) - vfsObj, root, cleanup, err := newVerityRoot(ctx) + vfsObj, root, err := newVerityRoot(ctx, t) if err != nil { t.Fatalf("newVerityRoot: %v", err) } - defer cleanup() filename := "verity-test-file" fd, size, err := newFileFD(ctx, vfsObj, root, filename, 0644) @@ -212,15 +212,14 @@ func TestReadUntouchedFileSucceeds(t *testing.T) { } } -// TestReopenUntouchedFileSucceeds ensures that reopen an untouched verity file +// TestReopenUnmodifiedFileSucceeds ensures that reopen an untouched verity file // succeeds after enabling verity for it. -func TestReopenUntouchedFileSucceeds(t *testing.T) { +func TestReopenUnmodifiedFileSucceeds(t *testing.T) { ctx := contexttest.Context(t) - vfsObj, root, cleanup, err := newVerityRoot(ctx) + vfsObj, root, err := newVerityRoot(ctx, t) if err != nil { t.Fatalf("newVerityRoot: %v", err) } - defer cleanup() filename := "verity-test-file" fd, _, err := newFileFD(ctx, vfsObj, root, filename, 0644) @@ -251,11 +250,10 @@ func TestReopenUntouchedFileSucceeds(t *testing.T) { // TestModifiedFileFails ensures that read from a modified verity file fails. func TestModifiedFileFails(t *testing.T) { ctx := contexttest.Context(t) - vfsObj, root, cleanup, err := newVerityRoot(ctx) + vfsObj, root, err := newVerityRoot(ctx, t) if err != nil { t.Fatalf("newVerityRoot: %v", err) } - defer cleanup() filename := "verity-test-file" fd, size, err := newFileFD(ctx, vfsObj, root, filename, 0644) @@ -298,11 +296,10 @@ func TestModifiedFileFails(t *testing.T) { // corresponding Merkle tree file is modified. func TestModifiedMerkleFails(t *testing.T) { ctx := contexttest.Context(t) - vfsObj, root, cleanup, err := newVerityRoot(ctx) + vfsObj, root, err := newVerityRoot(ctx, t) if err != nil { t.Fatalf("newVerityRoot: %v", err) } - defer cleanup() filename := "verity-test-file" fd, size, err := newFileFD(ctx, vfsObj, root, filename, 0644) @@ -353,11 +350,10 @@ func TestModifiedMerkleFails(t *testing.T) { // the parent Merkle tree file is modified. func TestModifiedParentMerkleFails(t *testing.T) { ctx := contexttest.Context(t) - vfsObj, root, cleanup, err := newVerityRoot(ctx) + vfsObj, root, err := newVerityRoot(ctx, t) if err != nil { t.Fatalf("newVerityRoot: %v", err) } - defer cleanup() filename := "verity-test-file" fd, _, err := newFileFD(ctx, vfsObj, root, filename, 0644) @@ -427,3 +423,68 @@ func TestModifiedParentMerkleFails(t *testing.T) { t.Errorf("OpenAt file with modified parent Merkle succeeded") } } + +// TestUnmodifiedStatSucceeds ensures that stat of an untouched verity file +// succeeds after enabling verity for it. +func TestUnmodifiedStatSucceeds(t *testing.T) { + ctx := contexttest.Context(t) + vfsObj, root, err := newVerityRoot(ctx, t) + if err != nil { + t.Fatalf("newVerityRoot: %v", err) + } + + filename := "verity-test-file" + fd, _, err := newFileFD(ctx, vfsObj, root, filename, 0644) + if err != nil { + t.Fatalf("newFileFD: %v", err) + } + + // Enable verity on the file and confirms stat succeeds. + var args arch.SyscallArguments + args[1] = arch.SyscallArgument{Value: linux.FS_IOC_ENABLE_VERITY} + if _, err := fd.Ioctl(ctx, nil /* uio */, args); err != nil { + t.Fatalf("fd.Ioctl: %v", err) + } + + if _, err := fd.Stat(ctx, vfs.StatOptions{}); err != nil { + t.Errorf("fd.Stat: %v", err) + } +} + +// TestModifiedStatFails checks that getting stat for a file with modified stat +// should fail. +func TestModifiedStatFails(t *testing.T) { + ctx := contexttest.Context(t) + vfsObj, root, err := newVerityRoot(ctx, t) + if err != nil { + t.Fatalf("newVerityRoot: %v", err) + } + + filename := "verity-test-file" + fd, _, err := newFileFD(ctx, vfsObj, root, filename, 0644) + if err != nil { + t.Fatalf("newFileFD: %v", err) + } + + // Enable verity on the file. + var args arch.SyscallArguments + args[1] = arch.SyscallArgument{Value: linux.FS_IOC_ENABLE_VERITY} + if _, err := fd.Ioctl(ctx, nil /* uio */, args); err != nil { + t.Fatalf("fd.Ioctl: %v", err) + } + + lowerFD := fd.Impl().(*fileDescription).lowerFD + // Change the stat of the underlying file, and check that stat fails. + if err := lowerFD.SetStat(ctx, vfs.SetStatOptions{ + Stat: linux.Statx{ + Mask: uint32(linux.STATX_MODE), + Mode: 0777, + }, + }); err != nil { + t.Fatalf("lowerFD.SetStat: %v", err) + } + + if _, err := fd.Stat(ctx, vfs.StatOptions{}); err == nil { + t.Errorf("fd.Stat succeeded when it should fail") + } +} |