corim/signedcorim: check for mandatory alg and kid #157
Conversation
pranjalkole
force-pushed
the
signedcorim
branch
from
6c337fd to
95eb749
Compare
There was a problem hiding this comment.
we need to change corim/testcases/signed-good-corim.cbor to include the kid parameter.
The test cases should be amended as part of the same commit commit that changes the code to make it necessary. (Doing this requires first updaing https://github.com/veraison/gen-testcases to include the key ID on signing).
pranjalkole
force-pushed
the
signedcorim
branch
3 times, most recently
from
1e94de3 to
b16dda1
Compare
* Added the kid parameter to the failing tests * Added getKidFromJWK in corim/signer.go * Regenerated all testcases * Made regen-from-src.sh executable Signed-off-by: Pranjal Kole <[email protected]>
| @@ -159,6 +179,7 @@ func (o *SignedCorim) Sign(signer cose.Signer) ([]byte, error) { | |||
|
|
|||
| o.message.Headers.Protected.SetAlgorithm(alg) | |||
| o.message.Headers.Protected[cose.HeaderLabelContentType] = ContentType | |||
| o.message.Headers.Protected[cose.HeaderLabelKeyID] = kid | |||
There was a problem hiding this comment.
we should we ensure that kid != nil
There was a problem hiding this comment.
kid is supposed to never be nil. I was thinking we should store kid as part of the cose.Signer while creating it from the JWK (like we do for alg). getKidFromJWK can get a kid from every JWK.
I'll wait for the meeting on kid being mandatory before making any more changes.
There was a problem hiding this comment.
@thomas-fossati Please ping me after the final decision on whether kid should be mandatory.
Before merging, we need to change
corim/testcases/signed-good-corim.cborto include thekidparameter.