Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify report example #3

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft

Verify report example #3

wants to merge 21 commits into from

Conversation

takuro-sato
Copy link
Owner

No description provided.

@takuro-sato takuro-sato marked this pull request as draft April 11, 2023 13:33
github.com/sirupsen/logrus v1.9.0
github.com/veraison/go-cose v1.0.0
google.golang.org/grpc v1.54.0
microsoft/attestation-container v0.0.0-00010101000000-000000000000
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version number was added by go mod tidy

@takuro-sato takuro-sato force-pushed the verify_report_example branch from 1f3478d to 288b622 Compare April 13, 2023 11:54
google.golang.org/protobuf v1.30.0 // indirect
)

replace microsoft/attestation-container => ../
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return r
}

func verifyAttestationEndorsements(endorsementCertificates []byte) *x509.Certificate {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return chipCertificate
}

func verifyAttestationReportSignature(attestation []byte, chipCertificate *x509.Certificate) string {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return deserializedReport.Measurement
}

func verifyUVMEndorsements(uvmEndorsements []byte, measurementInReport string) {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return r
}

func verifyAttestationEndorsements(endorsementCertificates []byte) *x509.Certificate {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Endorsement is a bit overused. In the MAA case that means the UVM reference info. I think you are verifying the platform certs here?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed CCF's convention.

https://github.com/microsoft/CCF/blob/main/attestation-container/protobuf/attestation-container.proto#L20

attestation endorsements: endorses attestation report
UVM endorsements: endorses UVM

I think you are verifying the platform certs here

Yes, I am. So it can be verifyPlatformCertificats or something like that, but in that case, it might bit confusing considering that server and client are using different names for a same thing.
(of course, we can rename the interface carefully though in future)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you make a good point. Leave it so it matches CCF. Maybe some comment block to make it clear locally which set of endorsements are which. AttestationEndorsement is true when you think about it but for someone coming to the code without a deep understanding of what is going on it might be a bit of a leap. We can help them by explaining that the attestation endorsement is the matching certs from AMD, possibly supplied in a round about way etc etc...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to switch naming on the CCF side to better match conventions used elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants