update: updated COSE extended attributes logic (#89)

Fixed issue #88. After this update: 1. JWS extended attribute keys still enforce string type. 2. COSE extended attribute keys will accept string and int type. Signed-off-by: Patrick Zheng <[email protected]>
notaryproject · Oct 28, 2022 · 0e98fb1 · 0e98fb1
1 parent 56bd40a
commit 0e98fb1
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 30 deletions.
diff --git a/signature/cose/conformance_test.go b/signature/cose/conformance_test.go
@@ -185,12 +185,8 @@ func verifyPayload(payload *signature.Payload, request *signature.SignRequest, t
 }
 
 func areAttrEqual(u []signature.Attribute, v []signature.Attribute) bool {
-	sort.Slice(u, func(p, q int) bool {
-		return u[p].Key < u[q].Key
-	})
-	sort.Slice(v, func(p, q int) bool {
-		return v[p].Key < v[q].Key
-	})
+	sortCOSEAttributes(u)
+	sortCOSEAttributes(v)
 	return reflect.DeepEqual(u, v)
 }
 
@@ -202,3 +198,25 @@ func generateSign1(msg *cose.Sign1Message) *cose.Sign1Message {
 	newMsg.Signature = hexToBytes("31b6cb0cd9c974b39d603465811c2aa3d96a5dff89f80b33cb4e321dc6e68a29b4ba65c00f0f9f22ee4376abfaec2cba6fd21c6881ecaab25775e3fb9226a88cf41660b2d6fd14184540d07ded3744e19ff9dbdd081e15c8f77bb6ca3072ef57141594fad4ea57d206c6b8dd3a6e0a0a7ed764ff08dbcc439bd722e1b3d282921a579a3d860cceea37d633184f9316cb6b4fa4ea550da5ad9e5bf3c2d768a787da76e594290cb10b5b1ead8b7e75967de28e9ff429fe9db814380608a15674f9741563902a620f312213d9dce5c264017cbcb3bb4f8cebee0d5ef32b364f68c11cba5630fac8e3165d06fdebaca095267223c552fe605b4529f25b65f8fa47b010b9096cec275307e82b1062f660a73e07d0b85b978b4a59b5cde51fc9a031b488a3deb38fc312a64ef2ec1250238ae16cfefc00d9aa1ceb938fe6de51f265eebe975c29f4cff8ab0afb40c45e8c985d17347bf20f455851c1a46ab655f51a159cf8910a424c5a8bbdd239e49e43a73c7b5174de29e835063e5e64b459558de5")
 	return newMsg
 }
+
+func sortCOSEAttributes(u []signature.Attribute) {
+	sort.Slice(u, func(p, q int) bool {
+		switch k1 := u[p].Key.(type) {
+		case int:
+			switch k2 := u[q].Key.(type) {
+			case int:
+				return k1 < k2
+			case string:
+				return false
+			}
+		case string:
+			switch k2 := u[q].Key.(type) {
+			case int:
+				return true
+			case string:
+				return k1 < k2
+			}
+		}
+		return false
+	})
+}
diff --git a/signature/cose/envelope.go b/signature/cose/envelope.go
@@ -495,7 +495,7 @@ func parseProtectedHeaders(protected cose.ProtectedHeader, signerInfo *signature
 // 1. validate that all critical headers are present in the protected bucket
 // 2. validate that all required headers(as per spec) are marked critical
 // Returns list of extended attribute keys
-func validateCritHeaders(protected cose.ProtectedHeader) ([]string, error) {
+func validateCritHeaders(protected cose.ProtectedHeader) ([]interface{}, error) {
 	// This ensures all critical headers are present in the protected bucket.
 	labels, err := protected.Critical()
 	if err != nil {
@@ -531,15 +531,11 @@ func validateCritHeaders(protected cose.ProtectedHeader) ([]string, error) {
 	// fetch all the extended signed attributes
 	systemHeaders := []interface{}{cose.HeaderLabelAlgorithm, cose.HeaderLabelCritical, cose.HeaderLabelContentType,
 		headerLabelExpiry, headerLabelSigningScheme, headerLabelSigningTime, headerLabelAuthenticSigningTime}
-	var extendedAttributeKeys []string
+	var extendedAttributeKeys []interface{}
 	for label := range protected {
 		if contains(systemHeaders, label) {
 			continue
 		}
-		label, ok := label.(string)
-		if !ok {
-			return nil, &signature.InvalidSignatureError{Msg: "extendedAttributes key requires string type"}
-		}
 		extendedAttributeKeys = append(extendedAttributeKeys, label)
 	}
 
@@ -548,7 +544,7 @@ func validateCritHeaders(protected cose.ProtectedHeader) ([]string, error) {
 
 // generateExtendedAttributes generates []signature.Attribute during
 // SignerInfo process.
-func generateExtendedAttributes(extendedAttributeKeys []string, protected cose.ProtectedHeader) ([]signature.Attribute, error) {
+func generateExtendedAttributes(extendedAttributeKeys []interface{}, protected cose.ProtectedHeader) ([]signature.Attribute, error) {
 	criticalHeaders, ok := protected[cose.HeaderLabelCritical].([]interface{})
 	if !ok {
 		return nil, &signature.InvalidSignatureError{Msg: "invalid critical headers"}

diff --git a/signature/cose/envelope_test.go b/signature/cose/envelope_test.go
@@ -499,19 +499,6 @@ func TestSignerInfoErrors(t *testing.T) {
 		}
 	})
 
-	t.Run("when COSE envelope has customized protected header key that is not of string type", func(t *testing.T) {
-		env, err := getVerifyCOSE("notary.x509", signature.KeyTypeRSA, 3072)
-		if err != nil {
-			t.Fatalf("getVerifyCOSE() failed. Error = %s", err)
-		}
-		env.base.Headers.Protected[0] = "unsupported"
-		_, err = env.Content()
-		expected := errors.New("extendedAttributes key requires string type")
-		if !isErrEqual(expected, err) {
-			t.Fatalf("Content() expects error: %v, but got: %v.", expected, err)
-		}
-	})
-
 	t.Run("when COSE envelope protected header missing algorithm", func(t *testing.T) {
 		env, err := getVerifyCOSE("notary.x509", signature.KeyTypeRSA, 3072)
 		if err != nil {

diff --git a/signature/jws/conformance_test.go b/signature/jws/conformance_test.go
@@ -177,7 +177,8 @@ func verifyAttributes(t *testing.T, signerInfo *signature.SignerInfo) {
 
 func sortAttributes(attributes []signature.Attribute) []signature.Attribute {
 	sort.Slice(attributes, func(i, j int) bool {
-		return strings.Compare(attributes[i].Key, attributes[j].Key) < 0
+		key1, key2 := attributes[i].Key.(string), attributes[j].Key.(string)
+		return strings.Compare(key1, key2) < 0
 	})
 	return attributes
 }
diff --git a/signature/jws/jws.go b/signature/jws/jws.go
@@ -189,9 +189,16 @@ func getSignedAttributes(req *signature.SignRequest, algorithm string) (map[stri
 
 	// write extended signed attributes to the extAttrs map
 	for _, elm := range req.ExtendedSignedAttributes {
-		extAttrs[elm.Key] = elm.Value
+		key, ok := elm.Key.(string)
+		if !ok {
+			return nil, &signature.InvalidSignRequestError{Msg: "JWS envelope format only supports key of type string"}
+		}
+		if _, ok := extAttrs[key]; ok {
+			return nil, &signature.InvalidSignRequestError{Msg: fmt.Sprintf("%q already exists in the extAttrs", key)}
+		}
+		extAttrs[key] = elm.Value
 		if elm.Critical {
-			crit = append(crit, elm.Key)
+			crit = append(crit, key)
 		}
 	}
 

diff --git a/signature/types.go b/signature/types.go
@@ -53,7 +53,7 @@ type UnsignedAttributes struct {
 // Attribute represents metadata in the Signature envelope.
 type Attribute struct {
 	// Key is the key name of the attribute.
-	Key string
+	Key interface{}
 
 	// Critical marks the attribute that MUST be processed by a verifier.
 	Critical bool