-
Notifications
You must be signed in to change notification settings - Fork 7
Add EAT measured component to measurement-values-map
#395
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
base: main
Are you sure you want to change the base?
Conversation
Fix #381 Signed-off-by: Thomas Fossati <[email protected]>
cddl/measurement-values-map.cddl
Outdated
@@ -15,5 +15,6 @@ measurement-values-map = non-empty<{ | |||
? &(name: 11) => text | |||
? &(cryptokeys: 13) => [ + $crypto-key-type-choice ] | |||
? &(integrity-registers: 14) => integrity-registers | |||
? &(measured-component: 15) => measured-component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
15 conflicts with the linear privlevel measurement PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, apologies. Will fix.
Signed-off-by: Thomas Fossati <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I find the compositional aspects of "measured components" confusing. Given the mkey is already a compositional element with is sub-component of an environment, what does it mean to have a sub-sub-element called a "component"? It seems the basic idea of using triples to enumerate various components or sub-components is being side-stepped toward composition expressed in terms of nesting. Most of the values in a measured component already exist in the measurement-values-map like digest, version, flags / raw-value. Mkey can support text identifiers which seems to map to It seems like a straight forward mapping without defining another layer of composition inside of measurement-values-map. |
@@ -970,6 +971,8 @@ The following describes each member of the `measurement-values-map`. | |||
|
|||
* `integrity-registers` (index 14): A group of one or more named measurements associated with the environment. Described in {{sec-comid-integrity-registers}}. | |||
|
|||
* `measured-component` (index 15): A measured component defined in {{Section 4.2 of -rats-eat-mc}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the matching semantics for this component? If I want to write a conditional endorsement over a piece of your measured component, what are those rules?
If you don't want to have to compose with conditional endorsements, I would recommend just making eat-measured-component triples their own thing.
If you do want composition, I think you'll need to do some transformation so component-id is split into the name and version measurements, measurement promoted to digest measurement value, flags promoted to raw-value, and signers promoted to a new measurement-value-map codepoint that has defined matching semantics.
I can appreciate not wanting to have to transform it for the endorsement. I do think you still need to define the matching semantics, and this may even be something you could specify as a request to the IANA to include the codepoint in the new measurement-values-map registry form the rats-eat-mc RFC.
Adding more matching rules to the base document for what's mostly a rejiggering of existing measurements seems a little wasteful of reviewer attention, but you have more experience with RFCs than I do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the matching semantics for this component?
Exact match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even for signers? Seems rough, but okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brutal but honest ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should approve this PR in its current form.
I agree with comments above that there are no matching rules, writing those matching rules will I think expose some gaps in the PR which need to be fixed.
The gap I am most concerned about is that the measured-component
object contains both a name and a value, but this object has been added to mval
which is for values only.
Fix #381