Repository navigation
Add CBOR and CWT support - #63
Conversation
7aa446b to
72e51ae
Compare
5e23c9e to
7631cca
Compare
5da44c5 to
460224a
Compare
|
Sergei, did you see #63 (comment) ? |
460224a to
19fc8ca
Compare
Missed it. Now addressed. |
19fc8ca to
9a3f773
Compare
| // AttestationResult represents the result of one or more evidence Appraisals | ||
| // by the verifier. It is serialized to JSON and signed by the verifier using | ||
| // JWT. | ||
| type AttestationResult struct { |
There was a problem hiding this comment.
A minor point but checking the EAR spec, the CDDL says EAR = { }
and EAR means Eat Attestation Results,
the line 51 should be re-named to type AttestationResults {
There was a problem hiding this comment.
No. Go types do not have to match the spec names exactly, and in this case, renaming would only lead to confusion. Pluralised names typically refer to collections so AttestationResults would imply a collection of AttestatonResult.
Also, FWIW, the section is called "EAT Attestation Result" (singular): https://www.ietf.org/archive/id/draft-ietf-rats-ear-04.html#name-eat-attestation-result. The the CDDL symbol EAR is representing a single result, regardless of how the acronym "EAR" usually decodes.
yogeshbdeshpande
left a comment
There was a problem hiding this comment.
Have reviewed it and left some comments for you to have a look at!
| @@ -255,8 +290,11 @@ func (o *AttestationResult) Map() map[eat.IntOrString]any { | |||
| // trustworthy if had been manually set that way). If submods contain nested | |||
| // tokens, an attempt is made to verify its signature using provided keys. If | |||
| // no keys were provided, or if verification fails using all provided keys, | |||
There was a problem hiding this comment.
I am not sure the original intent at Line 291 that Verification Fails, the the status TrustTierIncomplete is justifiable. A quick check on Ar4Si states the following:
`Incomplete: The Verifier was unable to complete its appraisal of this aspect of trustworthiness.
Value 1: The Evidence received contains unknown elements which the Verifier is unable to evaluate. An example might be that the wrong type of Evidence has been delivered. Another case is that of Evidence coming from a composite Attester: a Verifier may understand only part of it and leave as "unknown" the Trustworthiness claims related to features it can't appraise.
Value -1: A verifier malfunction occurred during the Verifier's appraisal processing.`
None fits the cause. For Verification Failure it should be "ContraIndicated"
There was a problem hiding this comment.
Not sure I fully understand. Is this an objection to the term "failure"? If so then that absolutely applies to TrustTierIncomplete, both in plain English (if a full appraisal was not possible, it by definition failed) and by intent in AR4SI (TrustTierIncomplete is the highest severity tier, higher even than TrustTierContraindicated).
| return o.fromJWTToken(token) | ||
| } | ||
|
|
||
| // SignCWT validates the AttestationResult object, encodes it to CBOR and wraps it |
There was a problem hiding this comment.
Just a question, is the X.509, Signing and subsequent Verification will also be part of the flow in future?
There was a problem hiding this comment.
No. That does not belong here. Both CWT and JWT paths take a key that is used to sign/verify. That key could be obtained from a cert, but verification of the cert chain and extraction of the key from it are operations that are completely orthogonal to EAR (the spec itself does not mention certs).
Add CBOR encoding support to EAR types. Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
Add support for creating, verifying, and extracting attestation results form CWTs. Attetation result now has SignCWT, as well as verified and unverified FromXXXCWT() methods and functions that exist for JWT. arc has also been updated to work with CWTs. "create" command now has a --cwt flag to indicate that a CWT, rather than JWT, should be created. "verify" command determines whether it has a CWT or JWT based on initial bytes. Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
Previous commit pushed coverage of arc/cmd to just below 90%. There is nothing that can be sensibly tested in the additions -- the drop is due to a switch that translates algorithm. To exercise other paths would require generating (valid!) keys that use those algorithms and adding them to test vars. Instead I'm bumping the coverage of the "root" command (which the culprit for low overall arc/cmd coverage) by exercising paths that are not used in other tests. These paths do not error, so there is nothing to check. The new test is purely to bump reported coverage for root.go (and therefore arc/cmd subpackage). Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
9a3f773 to
d7d0e5e
Compare
Support CBOR encoding of attestation results, and creation and verification of signed CWTs.