Add SEVSNP plugin for Veraison server #307

jraman567 · 2025-03-07T19:06:19Z

This PR implements the SEVSNP scheme for Veraison.

jraman567 · 2025-03-07T19:34:27Z

I think the linters and integration tests are failing because I bumped go version to 1.24.1. ratsd needs it. Let me try to fix that

jraman567 · 2025-03-07T21:52:35Z

golangci-lint version 1.64.2 introduces support for golang 1.24. Bumping up its version fixes the linters CI error.

scheme/sevsnp/evidence_handler.go

jraman567 · 2025-03-07T22:00:46Z

scheme/sevsnp/evidence_handler.go

+	"github.com/google/go-sev-guest/proto/sevsnp"
+	"github.com/google/go-sev-guest/verify"
+	"github.com/google/go-sev-guest/verify/trust"
+	sevsnpParser "github.com/jraman567/go-gen-ref/cmd/sevsnp"


I need to merge github.com/jraman567/go-gen-ref with github.com/veraison/gen-corim. It's in my pipeline but has the overhead of refactoring gen-corim.

scheme/sevsnp/scheme.go

yogeshbdeshpande · 2025-03-11T10:04:23Z

Thank you for the great work:

I request you add the README.md similar to, here and also add a link to any profile documentation you may have for the reader of this profile!

jraman567 · 2025-03-26T19:16:25Z

I have addressed the comments in this PR so far. I'm working on addressing Dionna's latest changes to SEV-SNP profile: deeglaze/draft-deeglaze-amd-sev-snp-corim-profile@150696c

I'll send a new rev out shortly.

Signed-off-by: Jagannathan Raman <[email protected]>

define SEV-SNP scheme for Veraison. Switch to CoRIM version v1.1.3-0.20250307044607-0bbdd6c78526 Signed-off-by: Jagannathan Raman <[email protected]>

store the trust anchors and reference values in the CoMID's "Attest Key Triple" and "Reference Value Triple" formats. Signed-off-by: Jagannathan Raman <[email protected]>

accept CoRIM endorsements, reference values & trust anchors, and save them in the database. Signed-off-by: Jagannathan Raman <[email protected]>

implement parts of the store handler that synthesize keys from trust anchors and reference values. Signed-off-by: Jagannathan Raman <[email protected]>

Implement an evidence handler to extract claims from the evidence token and store them in an internal representation format ( CoRIM for SEV-SNP). Additionally, implement the GetLevel interface for HCLogger, which was introduced with v1.5.0. Signed-off-by: Jagannathan Raman <[email protected]>

Update the store handler to get Trust Anchor and Reference Value keys from evidence. Add helper routines to parse the TSM report's auxblob to extract AMD keys. Signed-off-by: Jagannathan Raman <[email protected]>

Implement the ValidateEvidenceIntegrity routine of the EvidenceHandler interface. Ensure the root key in auxblob matches the ARK in provisioned trust anchors. Confirm the integrity of the certificate chain in the auxblob and the validity of the signature in the evidence. Signed-off-by: Jagannathan Raman <[email protected]>

Implement the AppraiseEvidence routine in the EvidenceHandler interface to confirm the claims match with the evidence. Signed-off-by: Jagannathan Raman <[email protected]>

jraman567 · 2025-03-28T19:13:04Z

I'm working on addressing Dionna's latest changes to SEV-SNP profile: deeglaze/draft-deeglaze-amd-sev-snp-corim-profile@150696c

This is taking longer I thought, so I'm pushing my current version while I work on Dionna's latest update to the SEV-SNP profile.

Add unit tests for endorsement, evidence and storage handlers Signed-off-by: Jagannathan Raman <[email protected]>

Add README document for SEVSNP scheme Signed-off-by: Jagannathan Raman <[email protected]>

deeglaze · 2025-03-11T16:25:51Z

scheme/sevsnp/common.go

+func getKey(auxblob []byte, guid []byte) ([]byte, error) {
+	for i := 0; i < len(auxblob); i += 24 {
+		var entry certTableEntry
+		b := auxblob[i : i+24]
+		buf := bytes.NewReader(b)
+		err := binary.Read(buf, binary.LittleEndian, &entry)
+		if err != nil {
+			return nil, err
+		}
+
+		if entry.Guid[0] == 0x0 {
+			break
+		}
+
+		if bytes.Equal(guid, entry.Guid[:]) {
+			return auxblob[entry.Offset : entry.Offset+entry.Length], nil
+		}
+	}
+
+	return nil, fmt.Errorf("key not found: %v", guid)
+}


this is implemented in go-sev-guest/abi, no?

GitHub says you posted this comment 3 weeks ago. For some reason, I didn't see this until now. Sorry about that.

Yes, go-sev-guest/abi has this. I'll use that instead of implementing my own.

GitHub says you posted this comment 3 weeks ago. For some reason, I didn't see this until now. Sorry about that.

GitHub says you posted this comment 3 weeks ago, but an email notification says you just added this comment. Weird.

Yeah, I started the review 3 weeks ago and sent it today.

Got it, thanks

deeglaze · 2025-03-31T16:23:30Z

scheme/sevsnp/common.go

+	if len(extractedCorim.Tags) > 1 {
+		return nil, errors.New("too many tags")
+	}


Why can't a CoRIM have multiple CoMIDs? Seems like a strange restriction.

deeglaze · 2025-03-31T16:24:20Z

scheme/sevsnp/evidence_handler.go

+	token *proto.AttestationToken,
+	_ []string,
+) (map[string]interface{}, error) {
+	var claimsSet map[string]interface{}


Suggested change

var claimsSet map[string]interface{}

var claimsSet map[string]any

deeglaze · 2025-03-31T16:26:00Z

scheme/sevsnp/evidence_handler.go

+	}
+
+	refValCorim := corim.UnsignedCorim{}
+	refValCorim.SetProfile("http://amd.com/2024/snp-corim-profile")


Suggested change

refValCorim.SetProfile("http://amd.com/2024/snp-corim-profile")

refValCorim.SetProfile("tag:amd.com,2025:snp-corim-profile")

deeglaze · 2025-03-31T16:28:31Z

scheme/sevsnp/evidence_handler.go

+		return err
+	}
+
+	ark, err := getARK(tsm.AuxBlob)


ARK and ASK certs may not be in the AuxBlob and should always be provided by the user as their trust anchor in order to not be tricked with a fake SEV platform. I do see you have compareTAs, so the current logic is safe. I know I do the same thing is go-sev-guest, but I just filed a bug against myself to remove this.

deeglaze · 2025-03-31T16:36:24Z

scheme/sevsnp/evidence_handler.go

+	}
+
+	if refValEndorsement.Type != handler.EndorsementType_REFERENCE_VALUE {
+		return nil, fmt.Errorf("reference values unavailable")


Errorf without formatting is wasteful. Better to have a global var like ErrRefValUnavailable = errors.New("reference values unavailable") so a caller can compare for equality if needed.

OK, I'll add a global variable so a caller can compare for equality.

deeglaze · 2025-03-31T16:38:29Z

scheme/sevsnp/evidence_handler.go

+}
+
+// compareMeasurements checks if two given comid.Measurement variables are the same.
+func compareMeasurements(refM comid.Measurement, evM comid.Measurement) bool {


This seems like a generic function for the base spec's measurement-map. Shouldn't this be something we can import from a generic library for appraisal against a base CoRIM?

That makes sense. It's not specific to SEV-SNP per se, it will benefit from being in a library. Let me do that change.

As also discussed in one of the Veraison weeklies, while eventually this kind of comparison logic should be encapsulated in the CoRIM library, this version of the function is not generic enough to be ported.

deeglaze · 2025-03-31T16:39:52Z

scheme/sevsnp/evidence_handler.go

+	// SVN comparison
+	if refM.Val.SVN != nil {
+		if evM.Val.SVN == nil {
+			log.Debugf("evidence doesn't have SVN")


This is an expected case when the host doesn't use an IDBLOCK_AUTH.

deeglaze · 2025-03-31T16:42:35Z

scheme/sevsnp/evidence_handler.go

+func (o EvidenceHandler) ValidateEvidenceIntegrity(
+	token *proto.AttestationToken,
+	trustAnchors []string,
+	endorsementsStrings []string,


Why are endorsements strings instead of CoMIDs for example? This is also not used, so should be _.

The IEvidenceHandler interface defines it that way.

deeglaze · 2025-03-31T16:45:28Z

scheme/sevsnp/test/corim/unsignedCorimSevSnp.cbor

.cbor files should be created with go generate from .diag files for understandability. Binary test files are write-once-read-never. See // go:generate documentation

OK, got it. That makes sense. I'll give //go:generate directive a shot for creating .cbor files.

thomas-fossati

Nice work, LGTM.

(I’ve left a few non-blocking comments inline)

thomas-fossati · 2025-04-25T10:52:17Z

scheme/sevsnp/evidence_handler.go

+	err = verify.SnpAttestation(&attestation, snpAttestationOptions())
+
+	if err != nil {
+		log.Errorf("failed to validate certificate chain: %+v\n", err)


Just a curiosity: why is the NL needed?

I'll remove it.

thomas-fossati · 2025-04-25T10:56:01Z

scheme/sevsnp/evidence_handler.go

+}
+
+// compareMeasurements checks if two given comid.Measurement variables are the same.
+func compareMeasurements(refM comid.Measurement, evM comid.Measurement) bool {


As also discussed in one of the Veraison weeklies, while eventually this kind of comparison logic should be encapsulated in the CoRIM library, this version of the function is not generic enough to be ported.

thomas-fossati · 2025-04-25T11:00:47Z

scheme/sevsnp/evidence_handler.go

+
+	appraisal := result.Submods[SchemeName]
+
+	appraisal.TrustVector.InstanceIdentity = ear.NoClaim


Quick (ignorant) question: Does a verified certificate contain information about the SEV-SNP instance?
If so, this should be populated accordingly (i.e., ear.TrustworthyInstanceClaim).

With SEV-SNP alone, we don't have evidence to verify the instance identity. We need evidence from attesters like TPM to establish instance identity.

thomas-fossati · 2025-04-25T11:01:57Z

scheme/sevsnp/evidence_handler.go

+		// REPORT_ID is ephemeral, so we can't use it for verification.
+		// REPORT_DATA is client-supplied , which we aren't using for
+		// verification in this scheme.


Shouldn't this be linked to the verification session nonce?

You're correct, it should be linked to a nonce.

When verifying as a relying party, this should be the session nonce. Whereas, when verifying as an attester, it should could be any nonce.

The RATSd token specifies a nonce in the outer envelope, which acts a binding between multiple evidence in a composite evidence scenario. I'll ensure that this matches the nonce specified in the envelope.

Thank you, I appreciate the review. :)

scheme/sevsnp/store_handler.go

yogeshbdeshpande · 2025-07-08T17:36:12Z

scheme/sevsnp/scheme.go

+const (
+	SchemeName             = "SEVSNP"
+	EndorsementMediaTypeRV = `application/corim-unsigned+cbor; profile="tag:amd.com,2024:snp-corim-profile"`
+	// ToDo: check media type for AMD ARK


Is Line 8 still a TO DO, if so please track it using github issue, else please Delete this,...

yogeshbdeshpande · 2025-07-08T17:37:38Z

scheme/sevsnp/scheme.go

+	SchemeName             = "SEVSNP"
+	EndorsementMediaTypeRV = `application/corim-unsigned+cbor; profile="tag:amd.com,2024:snp-corim-profile"`
+	// ToDo: check media type for AMD ARK
+	EndorsementMediaTypeTA = `application/corim-unsigned+cbor; profile="https://amd.com/ark"`


Any reason for Line 9: MediaType Usage different for the profile, as compared to one on Line 7..?

Is it better to use the same media type for both Trust Anchors and Reference Values?

The SEV-SNP profile defined the media type for Reference Values, but I couldn't find a media type for Trust Anchors. The TA is an AMD Root Key, so I'm using the base CoRIM format for it.

In summary, the TA is using the base CoRIM format, and RV is using the format specified in the SEV-SNP profile.

scheme/sevsnp/store_handler.go

scheme/sevsnp/common.go

yogeshbdeshpande

some minor .nits to look at!

Co-authored-by: Yogesh Deshpande <[email protected]>

jraman567 · 2025-07-09T11:36:57Z

some minor .nits to look at!

Thank you for reviewing the PR, @yogeshbdeshpande ! :) I have a newer version of this; I'll refresh this PR shortly with it.

jraman567 · 2025-09-03T15:53:23Z

Closing this, please refer to PR #333.

jraman567 force-pushed the sevsnp-plugin branch 5 times, most recently from 1106e48 to 94456b3 Compare March 7, 2025 21:48

jraman567 commented Mar 7, 2025

View reviewed changes

scheme/sevsnp/evidence_handler.go Outdated Show resolved Hide resolved

jraman567 commented Mar 7, 2025

View reviewed changes

scheme/sevsnp/scheme.go Outdated Show resolved Hide resolved

jraman567 requested review from setrofim, thomas-fossati and yogeshbdeshpande March 10, 2025 09:20

jraman567 added 9 commits March 28, 2025 15:09

fix(github): bump golang version to 1.24.1

38f760f

Signed-off-by: Jagannathan Raman <[email protected]>

feat(scheme): create plugin for AMD SEV-SNP

f10d1c5

define SEV-SNP scheme for Veraison. Switch to CoRIM version v1.1.3-0.20250307044607-0bbdd6c78526 Signed-off-by: Jagannathan Raman <[email protected]>

feat(sevsnp): implement endorsement extractor

41d5755

store the trust anchors and reference values in the CoMID's "Attest Key Triple" and "Reference Value Triple" formats. Signed-off-by: Jagannathan Raman <[email protected]>

feat(sevsnp): implement an endorsement handler

b45ed74

accept CoRIM endorsements, reference values & trust anchors, and save them in the database. Signed-off-by: Jagannathan Raman <[email protected]>

feat(sevsnp): implement the store handler

b55f395

implement parts of the store handler that synthesize keys from trust anchors and reference values. Signed-off-by: Jagannathan Raman <[email protected]>

feat(sevsnp): get keys from evidence

46edc0c

Update the store handler to get Trust Anchor and Reference Value keys from evidence. Add helper routines to parse the TSM report's auxblob to extract AMD keys. Signed-off-by: Jagannathan Raman <[email protected]>

feat(sevsnp): appraise the evidence

3f2318a

Implement the AppraiseEvidence routine in the EvidenceHandler interface to confirm the claims match with the evidence. Signed-off-by: Jagannathan Raman <[email protected]>

jraman567 force-pushed the sevsnp-plugin branch from 94456b3 to 55d86bd Compare March 28, 2025 19:10

jraman567 added 2 commits March 28, 2025 16:16

sevsnp tests: add unit tests

53cb52e

Add unit tests for endorsement, evidence and storage handlers Signed-off-by: Jagannathan Raman <[email protected]>

sevsnp: Add README

54d5bd8

Add README document for SEVSNP scheme Signed-off-by: Jagannathan Raman <[email protected]>

jraman567 force-pushed the sevsnp-plugin branch from 55d86bd to 54d5bd8 Compare March 28, 2025 20:17

deeglaze suggested changes Mar 31, 2025

View reviewed changes

jraman567 self-assigned this Apr 1, 2025

thomas-fossati approved these changes Apr 25, 2025

View reviewed changes

jraman567 closed this Jun 25, 2025

jraman567 deleted the sevsnp-plugin branch June 25, 2025 19:44

jraman567 restored the sevsnp-plugin branch July 8, 2025 13:48

jraman567 reopened this Jul 8, 2025

yogeshbdeshpande reviewed Jul 8, 2025

View reviewed changes

scheme/sevsnp/store_handler.go Outdated Show resolved Hide resolved

yogeshbdeshpande reviewed Jul 8, 2025

View reviewed changes

scheme/sevsnp/common.go Outdated Show resolved Hide resolved

yogeshbdeshpande reviewed Jul 8, 2025

View reviewed changes

scheme/sevsnp/common.go Outdated Show resolved Hide resolved

yogeshbdeshpande reviewed Jul 8, 2025

View reviewed changes

scheme/sevsnp/common.go Outdated Show resolved Hide resolved

yogeshbdeshpande reviewed Jul 8, 2025

View reviewed changes

jraman567 and others added 4 commits July 9, 2025 07:35

Update scheme/sevsnp/store_handler.go

76a0bbb

Co-authored-by: Yogesh Deshpande <[email protected]>

Update scheme/sevsnp/common.go

5df0774

Co-authored-by: Yogesh Deshpande <[email protected]>

Update scheme/sevsnp/common.go

16cae83

Co-authored-by: Yogesh Deshpande <[email protected]>

Update scheme/sevsnp/common.go

1b7ce0e

Co-authored-by: Yogesh Deshpande <[email protected]>

jraman567 closed this Sep 3, 2025

	var claimsSet map[string]interface{}
	var claimsSet map[string]any

	refValCorim.SetProfile("http://amd.com/2024/snp-corim-profile")
	refValCorim.SetProfile("tag:amd.com,2025:snp-corim-profile")


		appraisal := result.Submods[SchemeName]

		appraisal.TrustVector.InstanceIdentity = ear.NoClaim

Add SEVSNP plugin for Veraison server #307

Add SEVSNP plugin for Veraison server #307

Uh oh!

Conversation

jraman567 commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jraman567 commented Mar 7, 2025

Uh oh!

jraman567 commented Mar 7, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yogeshbdeshpande commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jraman567 commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jraman567 commented Mar 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thomas-fossati left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

jraman567 commented Mar 7, 2025 •

edited

Loading

yogeshbdeshpande commented Mar 11, 2025 •

edited

Loading

jraman567 commented Mar 26, 2025 •

edited

Loading