Add guest package for fetching attestation report via syscall #1341
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@SeanTAllen @KenGordon @svolos FYI. |
|
This is the replacement for #1329? |
helsaawy
approved these changes
Mar 31, 2022
|
Github doesn't want me to reply to the question about formats, but a json version isn't totally useful as the binary data is signed. Change the format and checking the signature becomes difficult. As a client I cannot rely on, for example, the hostdata or boot measurement fields unless I can validate that the whole report is real and produced by a real device. |
anmaxvl
force-pushed
the
snp-report
branch
2 times, most recently
from
17e266b
to
d86bdfc
Compare
kevpar
reviewed
Apr 1, 2022
kevpar
reviewed
Apr 1, 2022
kevpar
reviewed
Apr 1, 2022
kevpar
reviewed
Apr 1, 2022
|
Imagine you need to boot a naked UVM image to see that it works and that the measurement is as expected. There is no gcs running to mount a device and having one adds complication to understanding that the kernel/rootfs are as expected.
The C version of this code results in a 17k binary.
From: Kevin Parsons ***@***.***>
Sent: 01 April 2022 18:37
To: microsoft/hcsshim ***@***.***>
Cc: Ken Gordon ***@***.***>; Mention ***@***.***>
Subject: Re: [microsoft/hcsshim] Add guest package for fetching attestation report via syscall (PR #1341)
@kevpar commented on this pull request.
@@ -49,6 +49,7 @@ out/delta.tar.gz: bin/init bin/vsockexec bin/cmd/gcs bin/cmd/gcstools bin/cmd/ho
cp bin/cmd/gcs rootfs/bin/
cp bin/cmd/gcstools rootfs/bin/
cp bin/cmd/hooks/wait-paths rootfs/bin/
+ cp bin/internal/tools/snp-report rootfs/bin/
Is this just a debug tool? Could we leave it out of the rootfs and just mount it in when needed?
-
Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fhcsshim%2Fpull%2F1341%23pullrequestreview-929237907&data=04%7C01%7Cken.gordon%40microsoft.com%7C2d87b1f782284c9d9eed08da14063163%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637844314098266222%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rLHDmi%2FF1osVbD3RuPY7gWJ7hxupxwrWVjoN29cGTJ0%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABI7RIHELYH4PD45MFUFTJTVC4X23ANCNFSM5SEMNH2A&data=04%7C01%7Cken.gordon%40microsoft.com%7C2d87b1f782284c9d9eed08da14063163%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637844314098266222%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=XW4F4knEUTHCJI4cwX0jN7ODJnkHK1qHJHeiKKwLg3c%3D&reserved=0>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
anmaxvl
force-pushed
the
snp-report
branch
4 times, most recently
from
378d00e
to
47a2fcf
Compare
kevpar
reviewed
Apr 4, 2022
kevpar
reviewed
Apr 4, 2022
kevpar
reviewed
Apr 4, 2022
kevpar
reviewed
Apr 4, 2022
|
It looks like you're checking in the binary |
thanks for catching this! definitely not! |
kevpar
reviewed
Apr 4, 2022
kevpar
reviewed
Apr 4, 2022
kevpar
reviewed
Apr 4, 2022
kevpar
reviewed
Apr 4, 2022
kevpar
reviewed
Apr 4, 2022
kevpar
reviewed
Apr 4, 2022
anmaxvl
force-pushed
the
snp-report
branch
3 times, most recently
from
2cddb4d
to
c74e7c2
Compare
kevpar
reviewed
Apr 8, 2022
kevpar
reviewed
Apr 8, 2022
kevpar
reviewed
Apr 8, 2022
kevpar
reviewed
Apr 8, 2022
|
No, hcs takes a base64, makes a binary blob and puts that nto a hardware register. Maksim then get it out of the hardware register in process via an ioctl and converts it in wild ways. Previously the standalone C program converted it to hex so that it could be consumed by go code that would do whatever from just a series of bytes that were hex encoded to make debugging easier and avoid all the pain of passing binary via stdout.
Really, once it is in process, everything ought to be an appropriate sized blob, with a type etc to make sue it is impossible to compare the wrongs things in the wrong format.
From: Kevin Parsons ***@***.***>
Sent: 08 April 2022 19:11
To: microsoft/hcsshim ***@***.***>
Cc: Ken Gordon ***@***.***>; Mention ***@***.***>
Subject: Re: [microsoft/hcsshim] Add guest package for fetching attestation report via syscall (PR #1341)
@kevpar commented on this pull request.
________________________________
In internal/guest/runtime/hcsv2/uvm.go<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fhcsshim%2Fpull%2F1341%23discussion_r846370419&data=05%7C01%7Cken.gordon%40microsoft.com%7Cbe63e82903394263010e08da198b2ed4%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637850382849253649%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=tbwGgbCZXASEEGjwfJv5jVylYPOmq4OQLfcjfegLYKo%3D&reserved=0>:
@@ -95,6 +95,15 @@ func (h *Host) SetSecurityPolicy(base64Policy string) error {
return err
}
+ hostData, err := securitypolicy.NewSecurityPolicyDigest(base64Policy)
+ if err != nil {
+ return err
+ }
+
+ if err := validateHostData(fmt.Sprintf("%x", hostData[:])); err != nil {
That's terrible lol. So HCS is decoding the base64 and formatting it as a hex string? If that's the case let's make a comment both here and in create_lcow.go since that is super confusing.
-
Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2Fhcsshim%2Fpull%2F1341%23discussion_r846370419&data=05%7C01%7Cken.gordon%40microsoft.com%7Cbe63e82903394263010e08da198b2ed4%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637850382849253649%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=tbwGgbCZXASEEGjwfJv5jVylYPOmq4OQLfcjfegLYKo%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABI7RICWODH4MCVI6CKIGPTVEBZERANCNFSM5SEMNH2A&data=05%7C01%7Cken.gordon%40microsoft.com%7Cbe63e82903394263010e08da198b2ed4%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637850382849253649%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=tfNlWQdOSAYqwIYLZV1xWL7nq8iDcj%2FFdAX52QDVtHY%3D&reserved=0>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
msscotb
reviewed
Apr 9, 2022
|
Make sure you either rebase your commits into a curated set, or use "Squash and merge". |
kevpar
reviewed
Apr 12, 2022
kevpar
reviewed
Apr 12, 2022
Add internal/guest/linux package, which contains linux ioctl definitions. Devicemapper code is refactored to use the new package. Introduce ioctl wrapper and structs required to fetch attestation report. Validate that LaunchData provided to HCS and HostData returned as part of attestation report match. Add utility binary to fetch SNP report and update Makefile to support DEV_BUILD parameter, which includes test utilities inside LCOW image. Fake attestation report can be used when testing integrations. Signed-off-by: Maksim An <maksiman@microsoft.com>
Signed-off-by: Maksim An <maksiman@microsoft.com>
msscotb
approved these changes
Apr 13, 2022
|
lgtm |
anmaxvl
added a commit
that referenced
this pull request
Feb 7, 2023
princepereira
pushed a commit
to princepereira/hcsshim
that referenced
this pull request
Aug 29, 2024
…oft#1341) Add `internal/guest/linux package`, which contains linux ioctl definitions. Devicemapper code is refactored to use the new package. Introduce new `amdsevsnp` package with Introduce ioctl wrappers and structs required to fetch attestation report. Validate that `LaunchData` provided to HCS during UVM boot and `HostData` returned as part of attestation report match. Add utility binary to fetch SNP report and update Makefile to support `DEV_BUILD` parameter, which includes test utilities inside LCOW image. Fake attestation report can be used when testing integrations. Signed-off-by: Maksim An <maksiman@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add ioctl wrapper and structs required to fetch attestation report.
Add a command line tool to fetch real and fake attestation
reports.
Fake attestation report can be used when testing integrations.
Signed-off-by: Maksim An maksiman@microsoft.com