Introduce CCA Realm provisioning plugin #222

yogeshbdeshpande · 2024-05-01T16:37:42Z

This change introduces Realm Provisioning Plugin

Signed-off-by: Yogesh Deshpande <[email protected]>

thomas-fossati · 2024-05-08T08:56:15Z

scheme/cca-realm-provisioning/test/store/refvalEndorsements.json

+    "scheme": "CCA_REALM",
+    "type": "REFERENCE_VALUE",
+    "attributes": {
+        "CCA_REALM.vendor": "Worload Client Ltd",


s/worload/workload/

thomas-fossati · 2024-05-08T08:56:38Z

scheme/cca-realm-provisioning/README.md

+    "scheme": "CCA_REALM",
+    "type": "REFERENCE_VALUE",
+    "attributes": {
+        "CCA_REALM.vendor": "Worload Client Ltd",


s/worload/workload/

scheme/cca-realm-provisioning/corim_test_vectors.go

thomas-fossati · 2024-05-08T08:57:58Z

scheme/cca-realm-provisioning/endorsement_handler.go

@@ -0,0 +1,34 @@
+// Copyright 2022-2024 Contributors to the Veraison project.


thomas-fossati · 2024-05-08T08:59:20Z

scheme/cca-realm-provisioning/corim_extractor.go

+	var attrs = map[string]interface{}{
+		"CCA_REALM.vendor":      cAttr.Vendor,
+		"CCA_REALM-class-id":    cAttr.UUID,
+		"CCA_REALM-instance-id": rAttr.Rim,


this should be "CCA_REALM-realm-initial-measurement"

Yes, it can be expressed this way and perhaps better, hence renamed it now!

thomas-fossati · 2024-05-08T09:00:03Z

scheme/cca-realm-provisioning/corim_extractor.go

+
+	var attrs = map[string]interface{}{
+		"CCA_REALM.vendor":      cAttr.Vendor,
+		"CCA_REALM-class-id":    cAttr.UUID,


Is it the right choice? Would "model" be a better identifier for what is essentially a software product?

the intent here was to identify a Workload provider Identity which is launching N number of Realms, each with a Unique RIM Value.

setrofim · 2024-05-08T08:36:27Z

scheme/Makefile

@@ -8,6 +8,7 @@ SUBDIR += psa-iot
 SUBDIR += tpm-enacttrust
 SUBDIR += parsec-tpm
 SUBDIR += parsec-cca
+SUBDIR += cca-realm-provisioning


Even though this currently only implements the provisioning part, shouldn't the scheme be just cca-realm (and any future verification implementation would go under here as well)?

@setrofim : As discussed only the provisioning will be a separate scheme, as it is coming from a different supply chain, the Verification will be taken care inside a combined CCA plugin, which verifies both Realm and Platform part together. Hence I chosen the name cca-realm-provisioning.

If I keep it just cca-realm then it will assume both Provisioning and Verifications are handled inside!

Mmm ok, that would mean that the StoreHandler implementation would essentially be split/duplicated across two schemes, as the key formats need to be aligned across both schemes (which breaks the conceptual integrity of a scheme -- schems ought to be independent of each other).

Are we sure this is the right way forward? Would it not be better to implement as single cca scheme that handles everything in a single set of plugins, and then use configuration and/or policy to control whether the platform or realm parts, or both, are handled by a particular deployment?

Sure, I thought about StoreHandler part, while implementing the provisioning scheme and came to conclusion that the internal logic will be implemented in the common set of functions and both the schemes will invoke the same internal implementation thus essentially avoiding duplication! This is the best way of approaching as the CCA Scheme StoreHandler needs to fetch in sequence the RefValues for Platform and Realm in sequence, however this scheme will only do that latter!

Conceptually, a "scheme" is supposed to encompass the entire attestation "lifecycle", i.e. provisioning and attestion. It doesn't make sense to have a scheme with only provisioning, after all, the sole purpose of provisioning endorsements is so that they would later be used in verification.

the Verification will be taken care inside a combined CCA plugin, which verifies both Realm and Platform part together.

If that is the case, then this should be part of that combined plugin as well (going back to my intial suggestion of renaming this). When implementing the verification part of it, we can then re-use the commmon functionality between this and the cca-ssd-platform scheme.

On the other hand, if the goal is to modify cca-ssd-platfom scheme to optinally handle the realm part as well, then (it should probably be renamed, and) this should be part of that too.

Basically, it doesn't make sense for provisioning to exist in a scheme of its own, as the only purpose of provisioning is to enable verification. So if realm verification on its own is not a thing, and we can only have plaform and platform+realm, then we should have two schemes: cca-ssd-platform and cca (or cca-combined).

Using the same media type for both is incorrect, isn't it?

It's not incorrect, as long as the same CoRIM profile takes care of both platform and realm.

Instead, if we want to have separate profiles, we will need two media types.

Yes, they should be separate media types/profiles.

Yes, CCA Platform and CCA Realm are two separate profiles and hence we will need two media types!

It's not incorrect, as long as the same CoRIM profile takes care of both platform and realm.

They technically could be handled with a single profile. However, I belive the arguement is that they should be handled by separate profiles, as they're expected to be privided by different entitied within the supply chain, and that would simplify the profile that each entity needs to understand.

there are clear practical advantages to separating the two. I was just noting that it's not "incorrect" per se.

I think we are all in agreement about "two media types for one scheme"

setrofim · 2024-05-08T08:39:06Z

scheme/cca-realm-provisioning/classattributes.go

+	if classID == nil {
+		log.Debug("no classID in the environment")
+	}
+	if classID != nil {


Should be an else clause for the if statement above, rather than a separate if statement.

setrofim · 2024-05-08T08:41:40Z

scheme/cca-realm-provisioning/classattributes.go

+		log.Debug("no classID in the environment")
+	}
+	if classID != nil {
+		uuID, err := classID.GetUUID()


nit: rename to uuid, the whole thing, "UUID" is an initialism, and it doesn't make sense to only keep the "ID" part capitalized; so either UUID or uuid.

scheme/cca-realm-provisioning/corim_extractor.go

setrofim · 2024-05-08T09:05:17Z

scheme/cca-realm-provisioning/corim_extractor.go

+
+type CorimExtractor struct{}
+
+func (o CorimExtractor) RefValExtractor(


Rename to ExtractRefVal. In general, methods should be verbs (unless constructors, factories, properties, etc). The name as-is suggests that this should return an extractor, rather than a slice of ref vals.

(note: I realise that this is meant to be implementing common.IExtractor, however there is no need for that interface -- this is only used within this code, and is never passed via the interface.)

IExtractor is the interface which is been implemented by CorimExtractor. To your comment: however there is no need for that interface -- this is only used within this code

is not true, IExtractor interfaces are invoked from: UnsignedCorimDecoder.go
hence this is not a local function!

scheme/cca-realm-provisioning/corim_extractor.go

scheme/cca-realm-provisioning/plugin/combined/main.go

setrofim · 2024-05-08T09:09:13Z

scheme/cca-realm-provisioning/store_handler.go

+
+}
+
+func RefValLookupKey(schemeName, tenantID, instID string) string {


Can be changed to a local function (i. e. refValLookupKey)?

thomas-fossati

a couple of questions:

is it possible to provide an incomplete integrity register (e.g., if I don't care about REMs and only care about RIM)?
why is PV left out from the realm reference values?

thomas-fossati · 2024-05-08T09:33:48Z

scheme/cca-realm-provisioning/realmattributes.go

+	for k, val := range r.M {
+		a, d, err := o.extractRealmDigest(val)
+		if err != nil {
+			return errors.New("unable to extract digest for ")


incomplete sentence

scheme/cca-realm-provisioning/realmattributes.go

thomas-fossati · 2024-05-08T10:02:09Z

scheme/cca-realm-provisioning/realmattributes.go

+		return "", nil, fmt.Errorf("invalid digest: %v", err)
+	}
+	if len(digests) != 1 {
+		return "", nil, fmt.Errorf("invalid number %d for digest", len(digests))


Suggested change

return "", nil, fmt.Errorf("invalid number %d for digest", len(digests))

return "", nil, fmt.Errorf("expecting 1 digest, got %d", len(digests))

thomas-fossati · 2024-05-08T10:02:24Z

scheme/cca-realm-provisioning/realmattributes.go

+}
+
+func (o *RealmAttributes) extractRealmDigest(digests comid.Digests) (algID string, hash []byte, err error) {
+


spurious NL

Suggested change

thomas-fossati · 2024-05-08T10:07:47Z

Follow up on this:

is it possible to provide an incomplete integrity register (e.g., if I don't care about REMs and only care about RIM)?

I think there should be a RealmAttributes::Valid() method that makes it clear what are the exact requirements in terms of what we are willing to ingest.

yogeshbdeshpande · 2024-05-08T10:12:14Z

RealmAttributes

@thomas-fossati: I will look into this further! Basically my initial thought was REM slots will always be 4 in token, so we provide 4 to Supply Chain. They can use whatever they want to use 1,2,3, or all. The rest will be filled with nil values same as received in the token. When comparing, we compare the whole blob and should work, isn't it?

thomas-fossati · 2024-05-08T10:25:22Z

RealmAttributes

@thomas-fossati: I will look into this further! Basically my initial thought was REM slots will always be 4 in token, so we provide 4 to Supply Chain. They can use whatever they want to use 1,2,3, or all. The rest will be filled with nil values same as received in the token. When comparing, we compare the whole blob and should work, isn't it?

Not really. Suppose at run-time the workload app extends the REMs but the verifier is only in the "pure" TCB (i.e., platform + RIM).

If both RIM and REMs must be specified there is no way to tell the verifier to "ignore REMs".

Signed-off-by: Yogesh Deshpande <[email protected]>

yogeshbdeshpande · 2024-06-17T11:13:23Z

Closing this PR as the changes intended for this Pull Request will be addressed using the PR #233

CCA Realm Provisioning Plugin

d2de33e

Signed-off-by: Yogesh Deshpande <[email protected]>

yogeshbdeshpande force-pushed the realm-provisioning-plugin branch from 344ff79 to d2de33e Compare May 1, 2024 16:42

yogeshbdeshpande marked this pull request as ready for review May 1, 2024 16:48

yogeshbdeshpande requested review from thomas-fossati and setrofim May 1, 2024 16:48

thomas-fossati reviewed May 8, 2024

View reviewed changes

setrofim requested changes May 8, 2024

View reviewed changes

thomas-fossati reviewed May 8, 2024

View reviewed changes

yogeshbdeshpande added 6 commits May 15, 2024 18:36

Incorporating some review comments

e4afbd1

Signed-off-by: Yogesh Deshpande <[email protected]>

Correcting Copyright Header Template

2abfea9

Signed-off-by: Yogesh Deshpande <[email protected]>

More review comments!

0c71082

Signed-off-by: Yogesh Deshpande <[email protected]>

Add Realm Personalization support

12644ad

Signed-off-by: Yogesh Deshpande <[email protected]>

Add go modules

86c4206

Signed-off-by: Yogesh Deshpande <[email protected]>

Add Realm Personalization Value

b7e1e5b

Signed-off-by: Yogesh Deshpande <[email protected]>

yogeshbdeshpande closed this Jun 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce CCA Realm provisioning plugin #222

Introduce CCA Realm provisioning plugin #222

yogeshbdeshpande commented May 1, 2024 •

edited

Loading

thomas-fossati May 8, 2024

thomas-fossati May 8, 2024

thomas-fossati May 8, 2024

thomas-fossati May 8, 2024

yogeshbdeshpande May 15, 2024

thomas-fossati May 8, 2024

yogeshbdeshpande May 15, 2024

setrofim May 8, 2024

yogeshbdeshpande May 8, 2024

setrofim May 8, 2024

yogeshbdeshpande May 8, 2024

setrofim May 8, 2024

thomas-fossati May 8, 2024 •

edited

Loading

setrofim May 8, 2024

yogeshbdeshpande May 8, 2024

setrofim May 8, 2024

thomas-fossati May 8, 2024 •

edited

Loading

setrofim May 8, 2024

setrofim May 8, 2024

setrofim May 8, 2024

yogeshbdeshpande May 15, 2024

setrofim May 8, 2024

thomas-fossati left a comment •

edited

Loading

thomas-fossati May 8, 2024

thomas-fossati May 8, 2024

thomas-fossati May 8, 2024

thomas-fossati commented May 8, 2024

yogeshbdeshpande commented May 8, 2024

thomas-fossati commented May 8, 2024

yogeshbdeshpande commented Jun 17, 2024

		@@ -0,0 +1,34 @@
		// Copyright 2022-2024 Contributors to the Veraison project.


		type CorimExtractor struct{}

		func (o CorimExtractor) RefValExtractor(


		}

		func RefValLookupKey(schemeName, tenantID, instID string) string {

	return "", nil, fmt.Errorf("invalid number %d for digest", len(digests))
	return "", nil, fmt.Errorf("expecting 1 digest, got %d", len(digests))

		}

		func (o *RealmAttributes) extractRealmDigest(digests comid.Digests) (algID string, hash []byte, err error) {

Introduce CCA Realm provisioning plugin #222

Introduce CCA Realm provisioning plugin #222

Conversation

yogeshbdeshpande commented May 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati May 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati May 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomas-fossati commented May 8, 2024

yogeshbdeshpande commented May 8, 2024

thomas-fossati commented May 8, 2024

yogeshbdeshpande commented Jun 17, 2024

yogeshbdeshpande commented May 1, 2024 •

edited

Loading

thomas-fossati May 8, 2024 •

edited

Loading

thomas-fossati May 8, 2024 •

edited

Loading

thomas-fossati left a comment •

edited

Loading