-
Notifications
You must be signed in to change notification settings - Fork 19
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
Align trust vector with draft-ietf-rats-ar4si #37
Conversation
@@ -64,7 +64,10 @@ func (s Scheme) GetFormat() proto.AttestationFormat { | |||
return proto.AttestationFormat_PSA_IOT | |||
} | |||
|
|||
func (s Scheme) SynthKeysFromSwComponent(tenantID string, swComp *proto.Endorsement) ([]string, error) { | |||
func (s Scheme) SynthKeysFromSwComponent( |
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.
There seems to be no change except the newlines introduced. Any specific reason for this change?
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.
To keep the lines short, to prevent wrapping and make them easier to parse.
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.
ok, understood, was just wondering should we consistently do it at least in the file we are currently touching??
SynthKeysFromTrustAnchors is nearly 100+ column width but not modified so was in a little fix hence the question?
@@ -136,7 +139,10 @@ func (s Scheme) GetTrustAnchorID(token *proto.AttestationToken) (string, error) | |||
), nil | |||
} | |||
|
|||
func (s Scheme) ExtractVerifiedClaims(token *proto.AttestationToken, trustAnchor string) (*scheme.ExtractedClaims, error) { | |||
func (s Scheme) ExtractVerifiedClaims( |
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.
Same question as line 67
proto/result.go
Outdated
// Claim values as defined by | ||
// https://datatracker.ietf.org/doc/draft-ietf-rats-ar4si/ | ||
const ( | ||
// common |
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 does //common here means?
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.
The group of consts bellow are common to all claims. (The subsequent groups are titled with the claim they're for).
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.
🏆
|
||
// Claim values as defined by | ||
// https://datatracker.ietf.org/doc/draft-ietf-rats-ar4si/ | ||
const ( |
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.
this is awesome, and it's key information for plugin writers. I am wondering how to make it as visible as possible?
Just a side note:
|
which change are you thinking of? |
I was mentioning about reading the Attestation Results and echoing what has been received in return response from Veraison. At the moment this bit of code is not present. |
If that's the case there should be no change to track, right? What am I missing? |
Well, we have veraison/evcli#12 on evcli -> I think we do not need anything here. So we are ok. |
Discussed the confusion: veraison/evcli#13 will address the core problem. For apiclient, let the composer of the library display this information. Suitable api functions will be provided for users to include so that they have the required information at their disposal. |
Bring the TrustVector within Attestation result into closer alignment with https://datatracker.ietf.org/doc/draft-ietf-rats-ar4si/ - Align the claims inside the TrustVector with those specified by the ar4si spec. - Change their values to be integers with values in the int8 range (note: due to protobuf limitations, actual representation is int32). - Add a TrustTier type that corresponds to the "Trustworthiness Tier" concept from ar4si. ARStatus has methods to covert its value into a tier. - The overall Test status is now a TrustTier. This is now set automatically by the core verifier from the TrustVector, rather than relying on Scheme plugins to update it (note: plugins can still override it, if necessary). - Add "veraison-verifier-added-claims" extension to the attestation result and allow it to be populated by policy. Signed-off-by: Sergei Trofimov <[email protected]> Co-authored-by: Thomas Fossati <[email protected]>
Bring the TrustVector within Attestation result into closer alignment
with https://datatracker.ietf.org/doc/draft-ietf-rats-ar4si/
ar4si spec.
(note: due to protobuf limitations, actual representation is int32).
concept from ar4si. ARStatus has methods to covert its value into a
tier.
automatically by the core verifier from the TrustVector, rather than
relying on Scheme plugins to update it (note: plugins can still
override it, if necessary).
result and allow it to be populated by policy.