VHD with dm-verity by SeanTAllen · Pull Request #985 · microsoft/hcsshim

SeanTAllen · 2021-03-25T12:57:01Z

Adds a dm-verity integrity option to VHDs created from tar files.

allow for the creation of dmverity superblocks
getting the complete merkle tree
getting a roothash from an existing tree

tar2ext4 now takes an additional option to include tree information in the
generated VHD. Information will be stored as:

ext4 / verity superblock / merkle tree / VHD footer

assuming that all optional content is included.

At time, the salt value for all verity code is hardcoded. It could be
changed later as with the usage of a superblock, we no longer need
to know the salt on the use side as it will be obainted from the superblock.

The included cmd/dmverity-vhd cli app can be used for testing.

create takes the name of a docker image which it will download
and for each layer, it will generate a VHD in the provided output directory.
The layer VHDs are named after the SHA for the given layer.
create outputs the name of each vhd created.
roothash takes the nane of a docker image which it will download
and for each layer, it will output the roothash for the layer.

For example:

> ./dmverity-vhd create -i alpine:3.10 -o test
Layer 0: test/eb37acd07598b5162968a995ec7eef4caf64f46cf825b2f97a21eaba03c6d5f8.vhd

> ./dmverity-vhd roothash -i alpine:3.10
Layer 0
root hash: 135db6d67677a19c3bf95ac5d57fe313c153c2d07d49fefbc1aa1da50df88cac

> dumpe2fs test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286
dumpe2fs 1.45.5 (07-Jan-2020)
Filesystem volume name:   <none>
Last mounted on:          <not available>
Filesystem UUID:          <none>
Filesystem magic number:  0xEF53
Filesystem revision #:    1 (dynamic)
Filesystem features:      ext_attr sparse_super2 filetype extent flex_bg large_file huge_file extra_isize read-only
Default mount options:    (none)
Filesystem state:         clean
Errors behavior:          Continue
Filesystem OS type:       Linux
Inode count:              496
Block count:              1544
Reserved block count:     0
Free blocks:              7
Free inodes:              3
First block:              0
Block size:               4096
Fragment size:            4096
Blocks per group:         32768
Fragments per group:      32768
Inodes per group:         496
Inode blocks per group:   31
Flex block group size:    2147483648
Last mount time:          n/a
Last write time:          Wed Dec 31 19:00:00 1969
Mount count:              0
Maximum mount count:      0
Last checked:             Wed Dec 31 19:00:00 1969
Check interval:           0 (<none>)
Reserved blocks uid:      0 (user root)
Reserved blocks gid:      0 (group root)
First inode:              11
Inode size:               256
Required extra isize:     24
Desired extra isize:      24

Group 0: (Blocks 0-1543)
  Primary superblock at 0, Group descriptors at 1-1
  Block bitmap at 1542 (+1542)
  Inode bitmap at 1543 (+1543)
  Inode table at 1511-1541 (+1511)
  7 free blocks, 3 free inodes, 90 directories
  Free blocks: 2-8
  Free inodes: 494-496

> veritysetup verify --data-blocks=1544 --hash-offset=6324224 test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 680edf0d62d42085f446efc20f34d02f5d21f4a2eec1ab79506809321105a13a

where no output from the verifysetup command means that everything is working as expected.

SeanTAllen · 2021-03-25T13:09:35Z

I assume that

Run go test -gcflags=all=-d=checkptr -v ./... -tags admin
cmd\dmverity-vhd\main.go:9:2: cannot find package "." in:
	D:\a\hcsshim\hcsshim\vendor\github.com\google\go-containerregistry\pkg\authn
cmd\dmverity-vhd\main.go:10:2: cannot find package "." in:
	D:\a\hcsshim\hcsshim\vendor\github.com\google\go-containerregistry\pkg\name
cmd\dmverity-vhd\main.go:11:2: cannot find package "." in:
	D:\a\hcsshim\hcsshim\vendor\github.com\google\go-containerregistry\pkg\v1\remote
Error: Process completed with exit code 1.

for the test failure is related to something I didn't do correctly related to go.mod when adding the new dependency.

Is this because I didn't vendor the new dependency?

SeanTAllen · 2021-03-25T13:13:29Z

.github/workflows/ci.yml

+      - run: go build ./cmd/dmverity-vhd
+      - run: go build ./cmd/roothash


I didn't add these to the artifact upload that appears directly below this, should I? I'm not sure what happens with those artifacts so I left out for now.

ext4/dmverity/dmverity.go

SeanTAllen · 2021-03-25T13:19:23Z

ext4/internal/compactext4/compact.go


-	blockSize               = 4096
-	blocksPerGroup          = blockSize * 8
+	BlockSize               = 4096


Made BlockSize exported as this value needs to be the same both during ext4 creation and when generating the dm-verity information.

anmaxvl · 2021-03-25T16:40:50Z

for the error, did you try go mod tidy followed by go mod vendor?

SeanTAllen · 2021-03-25T16:53:42Z

@anmaxvl I did not because I wasnt sure what to do. Will do that now.

SeanTAllen · 2021-03-25T17:09:34Z

that worked @anmaxvl. thanks.

cmd/dmverity-vhd/README.md

cmd/dmverity-vhd/main.go

ext4/dmverity/dmverity.go

anmaxvl · 2021-03-26T06:28:38Z

ext4/dmverity/dmverity.go

+	DataBlocks    uint64
+}
+
+func Tree(data []byte) []byte {


not in scope of this PR, but this should probably be some sort of a Reader and we'd read the VHD data sequentially in blocks, rather than everything into memory?

Perhaps. I went with the "easiest thing to get working". I'm not really sure whether it should be or not.

If its a Reader then you need to write the tree somewhere... for the most common use case that would mean, writing the tree somewhere to save a probably small amount of memory so you can read the tree from that location to write it to its final location. Not really sure that is a win. It's more complicated, unlike to have much memory pressure and would be doing extra IO.

SeanTAllen · 2021-03-26T12:06:25Z

This should be squashed on merge maintaining only the first commit comment. Do you normally do that via the GitHub UI or should I be handling that locally and then force pushing?

katiewasnothere · 2021-04-06T20:23:20Z

This should be squashed on merge maintaining only the first commit comment. Do you normally do that via the GitHub UI or should I be handling that locally and then force pushing?

@SeanTAllen we normally do that locally and force push

ext4/dmverity/dmverity.go

katiewasnothere · 2021-04-06T21:02:42Z

ext4/dmverity/dmverity.go

+		nextLevel.Write(padding)
+
+		currentLevel = nextLevel
+		layers = append(layers, currentLevel.Bytes())


Why don't we write the first layer?

the first layer is the VHD data itself, so we're skipping it here.

what do you mean by "first layer" @katiewasnothere?

ext4/dmverity/dmverity.go

katiewasnothere · 2021-04-06T21:56:34Z

ext4/dmverity/dmverity.go

+	}
+	rootHash := hash2(dmvSB.Salt[:dmvSB.SaltSize], block)
+	return &VerityInfo{
+		RootDigest:         fmt.Sprintf("%x", rootHash),


instead of doing an sprintf, you can use golang's built in string func to convert a byte slice to a string.

string() will have it try and be decoded as valid utf-8 I think. Here's a run with the alpine image in the README for both methods.

fmt.Sprintf("%x", rootHash): ea1303202ccc2e71b4f40d13537c0dda1058b232293d42c6cd18258df0210642

Convert via string(): �►X�2)=B��↑%��!♠B

It seems like this was intentional

yeah, this was intentional. I initially did try to just convert via string.

katiewasnothere · 2021-04-06T21:58:58Z

ext4/dmverity/dmverity.go

+	return &VerityInfo{
+		RootDigest:         fmt.Sprintf("%x", rootHash),
+		Algorithm:          string(bytes.Trim(dmvSB.Algorithm[:], "\x00")),
+		Salt:               fmt.Sprintf("%x", dmvSB.Salt[:dmvSB.SaltSize]),


same here, you can use the string func

#985 (comment) Same as this. I'd assume this was intentional but someone chime in

dcantah · 2021-04-06T22:09:25Z

@SeanTAllen We'd generally prefer to have any dependency additions/updates in a separate commit from changes themselves. Makes it a lot easier to track down

cmd/dmverity-vhd/main.go

dcantah · 2021-05-05T08:39:22Z

cmd/dmverity-vhd/main.go

+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to set remote")
+		}
+		if verbose {


nit: maybe a wrapper func to get rid of the eight if verbose's. Or I guess we could use logrus and set the log level to debug if verbose was specified and log these with logrus.Debug.

dcantah

I'd still like to see the new dependency for the dmverity-vhd binary vendored in in a separate commit, but the changes lgtm.

anmaxvl · 2021-05-10T17:00:33Z

@microsoft/containerplat , PTaL

- allow for the creation of dmverity superblocks - getting the complete merkle tree - getting a roothash from an existing tree tar2ext4 now takes an additional option to include tree information in the generated VHD. Information will be stored as: ext4 / verity superblock / merkle tree / VHD footer assuming that all optional content is included. At time, the salt value for all verity code is hardcoded. It could be changed later as with the usage of a superblock, we no longer need to know the salt on the use side as it will be obainted from the superblock. The included `cmd/dmverity-vhd` and `cmd/roothash` commands can be used for testing. dmverity-hd takes the name of a docker image which it will download and for each layer, it will generate a VHD in the provided output directory. The layer VHDs are named after the SHA for the given layer. dmverity-vhd outputs the name of each vhd created. roothash takes the nane of a docker image which it will download and for each layer, it will output the roothash for the layer. For example: ``` > ./dmverity-vhd -i alpine:3.10 -o test test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 > ./roothash -i alpine:3.10 680edf0d62d42085f446efc20f34d02f5d21f4a2eec1ab79506809321105a13a > dumpe2fs test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 dumpe2fs 1.45.5 (07-Jan-2020) Filesystem volume name: <none> Last mounted on: <not available> Filesystem UUID: <none> Filesystem magic number: 0xEF53 Filesystem revision #: 1 (dynamic) Filesystem features: ext_attr sparse_super2 filetype extent flex_bg large_file huge_file extra_isize read-only Default mount options: (none) Filesystem state: clean Errors behavior: Continue Filesystem OS type: Linux Inode count: 496 Block count: 1544 Reserved block count: 0 Free blocks: 7 Free inodes: 3 First block: 0 Block size: 4096 Fragment size: 4096 Blocks per group: 32768 Fragments per group: 32768 Inodes per group: 496 Inode blocks per group: 31 Flex block group size: 2147483648 Last mount time: n/a Last write time: Wed Dec 31 19:00:00 1969 Mount count: 0 Maximum mount count: 0 Last checked: Wed Dec 31 19:00:00 1969 Check interval: 0 (<none>) Reserved blocks uid: 0 (user root) Reserved blocks gid: 0 (group root) First inode: 11 Inode size: 256 Required extra isize: 24 Desired extra isize: 24 Group 0: (Blocks 0-1543) Primary superblock at 0, Group descriptors at 1-1 Block bitmap at 1542 (+1542) Inode bitmap at 1543 (+1543) Inode table at 1511-1541 (+1511) 7 free blocks, 3 free inodes, 90 directories Free blocks: 2-8 Free inodes: 494-496 > veritysetup verify --data-blocks=1544 \ --hash-offset=6324224 \ test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 \ test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 \ 680edf0d62d42085f446efc20f34d02f5d21f4a2eec1ab79506809321105a13a ``` where no output from the verifysetup command means that everything is working as expected.

additionally combine dmverity-vhd and roothash into a single app with corresponding subcommands. Signed-off-by: Maksim An <maksiman@microsoft.com>

katiewasnothere

LGTM

* Adds a dm-verity integrity option to VHDs created from tar files. - allow for the creation of dmverity superblocks - getting the complete merkle tree - getting a roothash from an existing tree tar2ext4 now takes an additional option to include tree information in the generated VHD. Information will be stored as: ext4 / verity superblock / merkle tree / VHD footer assuming that all optional content is included. At time, the salt value for all verity code is hardcoded. It could be changed later as with the usage of a superblock, we no longer need to know the salt on the use side as it will be obainted from the superblock. The included `cmd/dmverity-vhd` and `cmd/roothash` commands can be used for testing. dmverity-hd takes the name of a docker image which it will download and for each layer, it will generate a VHD in the provided output directory. The layer VHDs are named after the SHA for the given layer. dmverity-vhd outputs the name of each vhd created. roothash takes the nane of a docker image which it will download and for each layer, it will output the roothash for the layer. For example: ``` > ./dmverity-vhd -i alpine:3.10 -o test test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 > ./roothash -i alpine:3.10 680edf0d62d42085f446efc20f34d02f5d21f4a2eec1ab79506809321105a13a > dumpe2fs test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 dumpe2fs 1.45.5 (07-Jan-2020) Filesystem volume name: <none> Last mounted on: <not available> Filesystem UUID: <none> Filesystem magic number: 0xEF53 Filesystem revision #: 1 (dynamic) Filesystem features: ext_attr sparse_super2 filetype extent flex_bg large_file huge_file extra_isize read-only Default mount options: (none) Filesystem state: clean Errors behavior: Continue Filesystem OS type: Linux Inode count: 496 Block count: 1544 Reserved block count: 0 Free blocks: 7 Free inodes: 3 First block: 0 Block size: 4096 Fragment size: 4096 Blocks per group: 32768 Fragments per group: 32768 Inodes per group: 496 Inode blocks per group: 31 Flex block group size: 2147483648 Last mount time: n/a Last write time: Wed Dec 31 19:00:00 1969 Mount count: 0 Maximum mount count: 0 Last checked: Wed Dec 31 19:00:00 1969 Check interval: 0 (<none>) Reserved blocks uid: 0 (user root) Reserved blocks gid: 0 (group root) First inode: 11 Inode size: 256 Required extra isize: 24 Desired extra isize: 24 Group 0: (Blocks 0-1543) Primary superblock at 0, Group descriptors at 1-1 Block bitmap at 1542 (+1542) Inode bitmap at 1543 (+1543) Inode table at 1511-1541 (+1511) 7 free blocks, 3 free inodes, 90 directories Free blocks: 2-8 Free inodes: 494-496 > veritysetup verify --data-blocks=1544 \ --hash-offset=6324224 \ test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 \ test/483b65c07faaf8ee7f1f57c6d7de0eda9dfd61a34f03337926650b8178db5286 \ 680edf0d62d42085f446efc20f34d02f5d21f4a2eec1ab79506809321105a13a ``` where no output from the verifysetup command means that everything is working as expected. * vendor new dependencies * create a new cli app for dmverity-vhd additionally combine dmverity-vhd and roothash into a single app with corresponding subcommands. Signed-off-by: Maksim An <maksiman@microsoft.com> Co-authored-by: Maksim An <maksiman@microsoft.com>

Related work items: microsoft#955, microsoft#985, microsoft#1023, microsoft#1026, microsoft#1032, microsoft#1034, microsoft#1038, microsoft#1041, microsoft#1044

SeanTAllen requested a review from a team as a code owner March 25, 2021 12:57

SeanTAllen commented Mar 25, 2021

View reviewed changes

ext4/dmverity/dmverity.go Outdated Show resolved Hide resolved

SeanTAllen commented Mar 25, 2021

View reviewed changes

ext4/dmverity/dmverity.go Outdated Show resolved Hide resolved

SeanTAllen commented Mar 25, 2021

View reviewed changes

anmaxvl reviewed Mar 26, 2021

View reviewed changes