From b16dda171027d02f831add3c54e34607f5446530 Mon Sep 17 00:00:00 2001 From: Pranjal Kole Date: Mon, 20 Jan 2025 04:33:07 +0530 Subject: [PATCH] corim/signedcorim: check for mandatory alg and kid * 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 --- corim/signedcorim.go | 31 +++++++++++--- corim/signedcorim_test.go | 38 +++++++++++------- corim/signer.go | 19 +++++++++ corim/testcases/regen-from-src.sh | 2 +- .../signed-corim-with-extensions.cbor | Bin 681 -> 684 bytes corim/testcases/signed-example-corim.cbor | Bin 684 -> 687 bytes corim/testcases/signed-good-corim.cbor | Bin 607 -> 610 bytes .../unsigned-corim-with-extensions.cbor | Bin 514 -> 514 bytes corim/testcases/unsigned-example-corim.cbor | Bin 517 -> 517 bytes corim/testcases/unsigned-good-corim.cbor | Bin 440 -> 440 bytes 10 files changed, 69 insertions(+), 21 deletions(-) mode change 100644 => 100755 corim/testcases/regen-from-src.sh diff --git a/corim/signedcorim.go b/corim/signedcorim.go index abe12364..ad7ee1fe 100644 --- a/corim/signedcorim.go +++ b/corim/signedcorim.go @@ -59,7 +59,21 @@ func (o *SignedCorim) processHdrs() error { return errors.New("missing mandatory protected header") } - v, ok := hdr.Protected[cose.HeaderLabelContentType] + v, ok := hdr.Protected[cose.HeaderLabelAlgorithm] + if !ok { + return errors.New("missing mandatory algorithm") + } + + // TODO: make this consistent, either int64 or cose.Algorithm + // cose.Algorithm is an alias to int64 defined in veraison/go-cose + switch v.(type) { + case int64: + case cose.Algorithm: + default: + return fmt.Errorf("expecting integer CoRIM Algorithm, got %T instead", v) + } + + v, ok = hdr.Protected[cose.HeaderLabelContentType] if !ok { return errors.New("missing mandatory content type") } @@ -68,9 +82,15 @@ func (o *SignedCorim) processHdrs() error { return fmt.Errorf("expecting content type %q, got %q instead", ContentType, v) } - // TODO(tho) key id is apparently mandatory, which doesn't look right. - // TODO(tho) Check with the CoRIM design team. - // See https://github.com/veraison/corim/issues/14 + v, ok = hdr.Protected[cose.HeaderLabelKeyID] + if !ok { + return errors.New("missing mandatory key id") + } + + _, ok = v.([]byte) + if !ok { + return fmt.Errorf("expecting byte string CoRIM Key ID, got %T instead", v) + } v, ok = hdr.Protected[HeaderLabelCorimMeta] if !ok { @@ -129,7 +149,7 @@ func (o *SignedCorim) FromCOSE(buf []byte) error { // Sign returns the serialized signed-corim, signed by the supplied cose Signer. // The target SignedCorim must have its UnsignedCorim field correctly // populated. -func (o *SignedCorim) Sign(signer cose.Signer) ([]byte, error) { +func (o *SignedCorim) Sign(signer cose.Signer, kid []byte) ([]byte, error) { if signer == nil { return nil, errors.New("nil signer") } @@ -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 o.message.Headers.Protected[HeaderLabelCorimMeta] = metaCBOR err = o.message.Sign(rand.Reader, NoExternalData, signer) diff --git a/corim/signedcorim_test.go b/corim/signedcorim_test.go index 2e286489..a0774034 100644 --- a/corim/signedcorim_test.go +++ b/corim/signedcorim_test.go @@ -286,6 +286,7 @@ func TestSignedCorim_FromCOSE_fail_corim_bad_cbor(t *testing.T) { / protected / << { / alg / 1: -7, / ECDSA 256 / / content-type / 3: "application/rim+cbor", + / kid / 4: h'1', / corim-meta / 8: h'a200a1006941434d45204c74642e01a101c11a5fad2056' } >>, / unprotected / {}, @@ -295,12 +296,12 @@ func TestSignedCorim_FromCOSE_fail_corim_bad_cbor(t *testing.T) { ) */ tv := []byte{ - 0xd2, 0x84, 0x58, 0x32, 0xa3, 0x01, 0x26, 0x03, 0x74, 0x61, 0x70, 0x70, + 0xd2, 0x84, 0x58, 0x35, 0xa4, 0x01, 0x26, 0x03, 0x74, 0x61, 0x70, 0x70, 0x6c, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2f, 0x72, 0x69, 0x6d, - 0x2b, 0x63, 0x62, 0x6f, 0x72, 0x08, 0x57, 0xa2, 0x00, 0xa1, 0x00, 0x69, - 0x41, 0x43, 0x4d, 0x45, 0x20, 0x4c, 0x74, 0x64, 0x2e, 0x01, 0xa1, 0x01, - 0xc1, 0x1a, 0x5f, 0xad, 0x20, 0x56, 0xa0, 0x44, 0xba, 0xdc, 0xb0, 0x30, - 0x44, 0xde, 0xad, 0xbe, 0xef, + 0x2b, 0x63, 0x62, 0x6f, 0x72, 0x04, 0x41, 0x31, 0x08, 0x57, 0xa2, 0x00, + 0xa1, 0x00, 0x69, 0x41, 0x43, 0x4d, 0x45, 0x20, 0x4c, 0x74, 0x64, 0x2e, + 0x01, 0xa1, 0x01, 0xc1, 0x1a, 0x5f, 0xad, 0x20, 0x56, 0xa0, 0x44, 0xba, + 0xdc, 0xb0, 0x30, 0x44, 0xde, 0xad, 0xbe, 0xef, } var actual SignedCorim @@ -316,6 +317,7 @@ func TestSignedCorim_FromCOSE_fail_invalid_corim(t *testing.T) { / protected / << { / alg / 1: -7, / ECDSA 256 / / content-type / 3: "application/rim+cbor", + / kid / 4: h'1', / corim-meta / 8: h'a200a1006941434d45204c74642e01a101c11a5fad2056' } >>, / unprotected / {}, @@ -327,13 +329,13 @@ func TestSignedCorim_FromCOSE_fail_invalid_corim(t *testing.T) { ) */ tv := []byte{ - 0xd2, 0x84, 0x58, 0x32, 0xa3, 0x01, 0x26, 0x03, 0x74, 0x61, 0x70, 0x70, + 0xd2, 0x84, 0x58, 0x35, 0xa4, 0x01, 0x26, 0x03, 0x74, 0x61, 0x70, 0x70, 0x6c, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x2f, 0x72, 0x69, 0x6d, - 0x2b, 0x63, 0x62, 0x6f, 0x72, 0x08, 0x57, 0xa2, 0x00, 0xa1, 0x00, 0x69, - 0x41, 0x43, 0x4d, 0x45, 0x20, 0x4c, 0x74, 0x64, 0x2e, 0x01, 0xa1, 0x01, - 0xc1, 0x1a, 0x5f, 0xad, 0x20, 0x56, 0xa0, 0x50, 0xa1, 0x00, 0x6d, 0x69, - 0x6e, 0x76, 0x61, 0x6c, 0x69, 0x64, 0x20, 0x63, 0x6f, 0x72, 0x69, 0x6d, - 0x44, 0xde, 0xad, 0xbe, 0xef, + 0x2b, 0x63, 0x62, 0x6f, 0x72, 0x04, 0x41, 0x31, 0x08, 0x57, 0xa2, 0x00, + 0xa1, 0x00, 0x69, 0x41, 0x43, 0x4d, 0x45, 0x20, 0x4c, 0x74, 0x64, 0x2e, + 0x01, 0xa1, 0x01, 0xc1, 0x1a, 0x5f, 0xad, 0x20, 0x56, 0xa0, 0x50, 0xa1, + 0x00, 0x6d, 0x69, 0x6e, 0x76, 0x61, 0x6c, 0x69, 0x64, 0x20, 0x63, 0x6f, + 0x72, 0x69, 0x6d, 0x44, 0xde, 0xad, 0xbe, 0xef, } var actual SignedCorim @@ -435,13 +437,15 @@ func TestSignedCorim_SignVerify_ok(t *testing.T) { } { signer, err := NewSignerFromJWK(key) require.NoError(t, err) + kid, err := getKidFromJWK(key) + require.NoError(t, err) var SignedCorimIn SignedCorim SignedCorimIn.UnsignedCorim = *unsignedCorimFromCBOR(t, testGoodUnsignedCorimCBOR) SignedCorimIn.Meta = *metaGood(t) - cbor, err := SignedCorimIn.Sign(signer) + cbor, err := SignedCorimIn.Sign(signer, kid) assert.Nil(t, err) var SignedCorimOut SignedCorim @@ -462,12 +466,14 @@ func TestSignedCorim_SignVerify_ok(t *testing.T) { func TestSignedCorim_SignVerify_fail_tampered(t *testing.T) { signer, err := NewSignerFromJWK(testES256Key) require.NoError(t, err) + kid, err := getKidFromJWK(testES256Key) + require.NoError(t, err) var SignedCorimIn SignedCorim SignedCorimIn.UnsignedCorim = *unsignedCorimFromCBOR(t, testGoodUnsignedCorimCBOR) - cbor, err := SignedCorimIn.Sign(signer) + cbor, err := SignedCorimIn.Sign(signer, kid) assert.Nil(t, err) var SignedCorimOut SignedCorim @@ -493,6 +499,8 @@ func TestSignedCorim_SignVerify_fail_tampered(t *testing.T) { func TestSignedCorim_Sign_fail_bad_corim(t *testing.T) { signer, err := NewSignerFromJWK(testES256Key) require.NoError(t, err) + kid, err := getKidFromJWK(testES256Key) + require.NoError(t, err) var SignedCorimIn SignedCorim @@ -501,7 +509,7 @@ func TestSignedCorim_Sign_fail_bad_corim(t *testing.T) { SignedCorimIn.UnsignedCorim = *emptyCorim - _, err = SignedCorimIn.Sign(signer) + _, err = SignedCorimIn.Sign(signer, kid) assert.EqualError(t, err, "failed validation of unsigned CoRIM: empty id") } @@ -513,7 +521,7 @@ func TestSignedCorim_Sign_fail_no_signer(t *testing.T) { SignedCorimIn.UnsignedCorim = *emptyCorim - _, err := SignedCorimIn.Sign(nil) + _, err := SignedCorimIn.Sign(nil, nil) assert.EqualError(t, err, "nil signer") } diff --git a/corim/signer.go b/corim/signer.go index cf971292..9916cc3d 100644 --- a/corim/signer.go +++ b/corim/signer.go @@ -154,6 +154,25 @@ func getAlgAndKeyFromJWK(j []byte) (cose.Algorithm, crypto.Signer, error) { return alg, key, nil } +func getKidFromJWK(j []byte) ([]byte, error) { + k, err := jwk.ParseKey(j) + if err != nil { + return nil, err + } + + if k.KeyID() != "" { + return []byte(k.KeyID()), nil + } + + // Generate a key ID from the JWK Thumbprint if none exist + // See https://datatracker.ietf.org/doc/html/rfc7638 + kid, err := k.Thumbprint(crypto.SHA256) + if err != nil { + return nil, err + } + return kid, nil +} + func ellipticCurveToAlg(c elliptic.Curve) cose.Algorithm { switch c { case elliptic.P256(): diff --git a/corim/testcases/regen-from-src.sh b/corim/testcases/regen-from-src.sh old mode 100644 new mode 100755 index f119e952..f5b6c065 --- a/corim/testcases/regen-from-src.sh +++ b/corim/testcases/regen-from-src.sh @@ -7,7 +7,7 @@ GEN_TESTCASE=$(go env GOPATH)/bin/gen-testcase if [[ ! -f ${GEN_TESTCASE} ]]; then echo "installing gen-testcase" - go install github.com/veraison/gen-testcase@v0.0.1 + go install github.com/veraison/gen-testcase@v0.0.2 fi testcases=( diff --git a/corim/testcases/signed-corim-with-extensions.cbor b/corim/testcases/signed-corim-with-extensions.cbor index 4bfca682959be82d3ace80585e5e20241a5f7759..32f1a04d9e3212a46906a2bdfd688835dffa9107 100644 GIT binary patch delta 274 zcmZ3AE{HE@TLB-u<{;*ogZ*q$8$vr44za6K+vx4&-Vn+h#DI06mdq*#H0l delta 247 zcmZ3(x{_7)QcFbaVn#LQlEi|7oXq6JlFa-({i4iV?c}8VqKOKglRcP}_!N@U^79!R zBN>m~Wc;;cvK*7N0#oA>#v2MH86_nJ#a8V6spzemktD^7_k6zA$I*Gge0dc+FhX diff --git a/corim/testcases/signed-example-corim.cbor b/corim/testcases/signed-example-corim.cbor index f6c4f46185582023dc04983cbef0118c99ed3c42..5f2c9b420af23eb02ad60e07cd30ea523e734791 100644 GIT binary patch delta 249 zcmV*AaiMFZfS03AZulLkx^HZPXZeQ z0)eEF$Q+TY9U26o0D*#`0HXn9K|@VN0%}rkVPsNuZf<3Ak?bY{gQAmJ0WJ&x*#cRk z1Z6TVF)lEZ{sANdVnR%lhXHDn_W>&fWI zOWC#9_32V*S?|gJqBQ0XV-d=Q4o2y(4=evJx^x}*J!LKk{{8X`!C1hU3 delta 250 zcmZ3_x`tKxQcFbaVn#LQlEi|7oXq6JlFa-({i4iV?c}8VB8~{hMGT83+KG$bP$A_7 znBwT{>&lcBl%JRqRGOEUTGYIV;U-h$;>mT4>g+73MtX*N29sSF^(HT6%whwo4$2Ri z%+I7f*?~!S;%aH92#4FPjneg9JKx^kwIF^*cIxzLag0HVz5!ynCDxN2tl!ma?>0RB hIwT_LTshzBkWY5o@}8D%+OXR~@zkeX7BW1`O8_ceT{r*$ diff --git a/corim/testcases/signed-good-corim.cbor b/corim/testcases/signed-good-corim.cbor index 50dae69118fff2b0e11127df304879307902dcb1..38762116d5dc14dda40b5e92159e41589bfa2c62 100644 GIT binary patch delta 210 zcmV;@04@LD1mXl_(u7!GqyZ)abYXCCY-wX*bZKvHFLG&ZD`R4Bas)v!2v|X)0ipoG z8ez^z00F@oX;f!`0HOidAarPSbZ~PzFE3$ZZDlTHcwudDY-Ip+K|@VNAWU>*AaiMF zZfS03AZulLkzpN^-T^<6enOG+C=3AE0$HO3Wil=?E-;fW0VD%rLQInz0mDLAK)T!X zH6^=}IV3h>?a1Li;cv z1pwIsS)-G10VE1yLQDi@GA=PLFp~lSD3e_Q!;yhMLRdhr1nE1?az`M3RzBiVD_D6* yXBu6KCL7VIG3HBOIbDhqp$}m=jSU25E55Ov{am^F1Gj6A*j@Y9xdB0Ed5_@GGfTbz diff --git a/corim/testcases/unsigned-corim-with-extensions.cbor b/corim/testcases/unsigned-corim-with-extensions.cbor index 0e056df92a7d13cf5caff8df0858e92e82bb1894..4cd240a9b7a0947413ee65cac7f99a4402b1c32f 100644 GIT binary patch delta 104 zcmZo-X<`vr!jM~%T3n)#oL`igtB{#8kd?udKV_Y;j xkx>sSZpWx1pArz1AHtGqsAsHax(KYCF~t!qWT0oLXENE2QG4=PM)iqCVE~gyA5{PV delta 104 zcmZo-X<}hqGEu-_@@mHLi4IN^x9V{+++>Pe%$VflGqGJucoBq`5)hOh!jfvJXRK#B yu}@`k9V7e1H)^~PS(a1-JwrVcpg3!aBT(JsT1I`o+>+Gd5{2aaqRd={%oG6bP9f?5 diff --git a/corim/testcases/unsigned-example-corim.cbor b/corim/testcases/unsigned-example-corim.cbor index 0c715c8f24d0886906a5f8df3eb40b02d6b4d4dd..8047897e4f48331278358af99ef8777e06e34877 100644 GIT binary patch delta 150 zcmZo=X=PztGWj;6mJwrPB;&E0jK7vJq^9QSx;rs0WC(EH{kUE)(c5_4<>2!low~)C z8kaC+Iy(EhD)^M7=uMu?s5g-2;dfh8GJ9E0*hSW*puQYJu^lieBh`EpBAi%S%e^NTWb6*5x*fDtou delta 151 zcmZo=X=UMB!q^zecAvM`vGG1)q`>J%-$p)Z!9_Vxt_SW*puf+irr$rBmX0UE{|tN;K2 delta 90 zcmdnNyn}gyIV)2$10&N!b7c;eg$#{N3mFzqtW=X^OmTGfb!Ey5%1=xQD$UDFEoxrG taFZ!=@x+ZPTuDwoEU89%hI$4Quk-)~cv2jL@lP550008cb98mxO