chore: Additional validation for verifier (#3522)

Additional validation when processing disclosures to assemble disclosed claims: - If there is more than one place where the digest is included, the Verifier MUST reject the Presentation. - If the claim name already exists at the same level, the Verifier MUST reject the Presentation. - If the claim value contains an object with an _sd key (at the top level or nested deeper), the Verifier MUST reject the Presentation. Closes #3519 Signed-off-by: Sandra Vrtikapa <[email protected]>
hyperledger-archives · Feb 10, 2023 · c509c1e · c509c1e
1 parent a22f53f
commit c509c1e
Show file tree

Hide file tree

Showing 3 changed files with 210 additions and 28 deletions.
diff --git a/pkg/doc/sdjwt/common/common.go b/pkg/doc/sdjwt/common/common.go
@@ -355,24 +355,25 @@ func GetDisclosedClaims(disclosureClaims []*DisclosureClaim, claims map[string]i
 	}
 
 	output := copyMap(claims)
+	includedDigests := make(map[string]bool)
 
-	err = processDisclosedClaims(disclosureClaims, output, hash)
+	err = processDisclosedClaims(disclosureClaims, output, includedDigests, hash)
 	if err != nil {
 		return nil, fmt.Errorf("failed to process disclosed claims: %w", err)
 	}
 
 	return output, nil
 }
 
-func processDisclosedClaims(disclosureClaims []*DisclosureClaim, claims map[string]interface{}, hash crypto.Hash) error { // nolint:lll
+func processDisclosedClaims(disclosureClaims []*DisclosureClaim, claims map[string]interface{}, includedDigests map[string]bool, hash crypto.Hash) error { // nolint:lll
 	digests, err := GetDisclosureDigests(claims)
 	if err != nil {
 		return err
 	}
 
 	for key, value := range claims {
 		if obj, ok := value.(map[string]interface{}); ok {
-			err := processDisclosedClaims(disclosureClaims, obj, hash)
+			err := processDisclosedClaims(disclosureClaims, obj, includedDigests, hash)
 			if err != nil {
 				return err
 			}
@@ -387,9 +388,25 @@ func processDisclosedClaims(disclosureClaims []*DisclosureClaim, claims map[stri
 			return err
 		}
 
-		if _, ok := digests[digest]; ok {
-			claims[dc.Name] = dc.Value
+		if _, ok := digests[digest]; !ok {
+			continue
 		}
+
+		_, digestAlreadyIncluded := includedDigests[digest]
+		if digestAlreadyIncluded {
+			// If there is more than one place where the digest is included,
+			// the Verifier MUST reject the Presentation.
+			return fmt.Errorf("digest '%s' has been included in more than one place", digest)
+		}
+
+		err = validateClaim(dc, claims)
+		if err != nil {
+			return err
+		}
+
+		claims[dc.Name] = dc.Value
+
+		includedDigests[digest] = true
 	}
 
 	delete(claims, SDKey)
@@ -398,6 +415,31 @@ func processDisclosedClaims(disclosureClaims []*DisclosureClaim, claims map[stri
 	return nil
 }
 
+func validateClaim(dc *DisclosureClaim, claims map[string]interface{}) error {
+	_, claimNameExists := claims[dc.Name]
+	if claimNameExists {
+		// If the claim name already exists at the same level, the Verifier MUST reject the Presentation.
+		return fmt.Errorf("claim name '%s' already exists at the same level", dc.Name)
+	}
+
+	m, ok := getMap(dc.Value)
+	if ok {
+		if KeyExistsInMap(SDKey, m) {
+			// If the claim value contains an object with an _sd key (at the top level or nested deeper),
+			// the Verifier MUST reject the Presentation.
+			return fmt.Errorf("claim value contains an object with an '%s' key", SDKey)
+		}
+	}
+
+	return nil
+}
+
+func getMap(value interface{}) (map[string]interface{}, bool) {
+	val, ok := value.(map[string]interface{})
+
+	return val, ok
+}
+
 func stringArray(entry interface{}) ([]string, error) {
 	if entry == nil {
 		return nil, nil
@@ -446,3 +488,21 @@ func copyMap(m map[string]interface{}) map[string]interface{} {
 
 	return cm
 }
+
+// KeyExistsInMap checks if key exists in map.
+func KeyExistsInMap(key string, m map[string]interface{}) bool {
+	for k, v := range m {
+		if k == key {
+			return true
+		}
+
+		if obj, ok := v.(map[string]interface{}); ok {
+			exists := KeyExistsInMap(key, obj)
+			if exists {
+				return true
+			}
+		}
+	}
+
+	return false
+}
diff --git a/pkg/doc/sdjwt/common/common_test.go b/pkg/doc/sdjwt/common/common_test.go
@@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0
 package common
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/ed25519"
 	"crypto/rand"
@@ -366,19 +367,93 @@ func TestGetDisclosedClaims(t *testing.T) {
 	t.Run("success - with complex object", func(t *testing.T) {
 		testClaims := copyMap(claims)
 
+		additionalDigest, err := GetHash(crypto.SHA256, additionalDisclosure)
+		r.NoError(err)
+
 		parentObj := make(map[string]interface{})
-		parentObj["given_name"] = "Albert"
-		parentObj[SDKey] = claims[SDKey]
+		parentObj["last_name"] = "Brown"
+		parentObj[SDKey] = []interface{}{additionalDigest}
 
 		testClaims["father"] = parentObj
 
-		disclosedClaims, err := GetDisclosedClaims(disclosureClaims, testClaims)
+		printObject(t, "Complex Claims", testClaims)
+
+		disclosedClaims, err := GetDisclosedClaims(append(disclosureClaims,
+			&DisclosureClaim{
+				Disclosure: additionalDisclosure,
+				Name:       "key-x",
+				Value:      "value-y"}),
+			testClaims)
 		r.NoError(err)
 		r.NotNil(disclosedClaims)
 
+		printObject(t, "Disclosed Claims", disclosedClaims)
+
 		r.Equal(6, len(disclosedClaims))
 		r.Equal("John", disclosedClaims["given_name"])
-		r.Equal("John", disclosedClaims["father"].(map[string]interface{})["given_name"])
+		r.Equal("value-y", disclosedClaims["father"].(map[string]interface{})["key-x"])
+	})
+
+	t.Run("error - claim value contains _sd", func(t *testing.T) {
+		testClaims := copyMap(claims)
+
+		additionalDigest, err := GetHash(crypto.SHA256, additionalDisclosure)
+		r.NoError(err)
+
+		parentObj := make(map[string]interface{})
+		parentObj["last_name"] = "Smith"
+		parentObj[SDKey] = []interface{}{additionalDigest}
+
+		testClaims["father"] = parentObj
+
+		disclosedClaims, err := GetDisclosedClaims(append(disclosureClaims,
+			&DisclosureClaim{
+				Disclosure: additionalDisclosure,
+				Name:       "key-x",
+				Value: map[string]interface{}{
+					"_sd": []interface{}{"test-digest"},
+				},
+			}),
+			testClaims)
+		r.Error(err)
+		r.Nil(disclosedClaims)
+		r.Contains(err.Error(), "failed to process disclosed claims: claim value contains an object with an '_sd' key")
+	})
+
+	t.Run("error - same claim key at the same level ", func(t *testing.T) {
+		testClaims := copyMap(claims)
+
+		parentObj := make(map[string]interface{})
+		parentObj["given_name"] = "Albert"
+		parentObj[SDKey] = claims[SDKey]
+
+		testClaims["father"] = parentObj
+
+		printObject(t, "Complex Claims", testClaims)
+
+		disclosedClaims, err := GetDisclosedClaims(disclosureClaims, testClaims)
+		r.Error(err)
+		r.Nil(disclosedClaims)
+		r.Contains(err.Error(),
+			"failed to process disclosed claims: claim name 'given_name' already exists at the same level")
+	})
+
+	t.Run("error - digest included in more than one spot ", func(t *testing.T) {
+		testClaims := copyMap(claims)
+
+		parentObj := make(map[string]interface{})
+		parentObj["last_name"] = "Smith"
+		parentObj[SDKey] = claims[SDKey]
+
+		testClaims["father"] = parentObj
+
+		printObject(t, "Complex Claims", testClaims)
+
+		disclosedClaims, err := GetDisclosedClaims(disclosureClaims, testClaims)
+		r.Error(err)
+		r.Nil(disclosedClaims)
+		r.Contains(err.Error(),
+			"failed to process disclosed claims: digest 'qqvcqnczAMgYx7EykI6wwtspyvyvK790ge7MBbQ-Nus' has been included in more than one place") //nolint:lll
 	})
 
 	t.Run("error - with complex object", func(t *testing.T) {
@@ -438,7 +513,7 @@ func TestGetDisclosedClaims(t *testing.T) {
 		testClaims[SDAlgorithmKey] = "sha-256"
 		testClaims[SDKey] = []interface{}{"abc"}
 
-		err := processDisclosedClaims(disclosureClaims, testClaims, 0)
+		err := processDisclosedClaims(disclosureClaims, testClaims, make(map[string]bool), 0)
 		r.Error(err)
 
 		r.Contains(err.Error(),
@@ -605,6 +680,70 @@ func TestGetCNF(t *testing.T) {
 	})
 }
 
+func TestKeyExistInMap(t *testing.T) {
+	r := require.New(t)
+
+	key := "_sd"
+
+	t.Run("true - claims contain _sd key (top level object)", func(t *testing.T) {
+		claims := map[string]interface{}{
+			key: "whatever",
+		}
+
+		exists := KeyExistsInMap(key, claims)
+		r.True(exists)
+	})
+
+	t.Run("true - claims contain _sd key (inner object)", func(t *testing.T) {
+		claims := map[string]interface{}{
+			"degree": map[string]interface{}{
+				key:    "whatever",
+				"type": "BachelorDegree",
+			},
+		}
+
+		exists := KeyExistsInMap(key, claims)
+		r.True(exists)
+	})
+
+	t.Run("false - _sd key not present in claims", func(t *testing.T) {
+		claims := map[string]interface{}{
+			"key-x": "value-y",
+			"degree": map[string]interface{}{
+				"key-x": "whatever",
+				"type":  "BachelorDegree",
+			},
+		}
+
+		exists := KeyExistsInMap(key, claims)
+		r.False(exists)
+	})
+}
+
+func printObject(t *testing.T, name string, obj interface{}) {
+	t.Helper()
+
+	objBytes, err := json.Marshal(obj)
+	require.NoError(t, err)
+
+	prettyJSON, err := prettyPrint(objBytes)
+	require.NoError(t, err)
+
+	fmt.Println(name + ":")
+	fmt.Println(prettyJSON)
+}
+
+func prettyPrint(msg []byte) (string, error) {
+	var prettyJSON bytes.Buffer
+
+	err := json.Indent(&prettyJSON, msg, "", "\t")
+	if err != nil {
+		return "", err
+	}
+
+	return prettyJSON.String(), nil
+}
+
 type NoopSignatureVerifier struct {
 }
 

diff --git a/pkg/doc/sdjwt/issuer/issuer.go b/pkg/doc/sdjwt/issuer/issuer.go
@@ -200,7 +200,7 @@ func New(issuer string, claims interface{}, headers jose.Headers,
 	}
 
 	// check for the presence of the _sd claim in claims map
-	found := keyExistsInMap(common.SDKey, claimsMap)
+	found := common.KeyExistsInMap(common.SDKey, claimsMap)
 	if found {
 		return nil, fmt.Errorf("key '%s' cannot be present in the claims", common.SDKey)
 	}
@@ -454,23 +454,6 @@ func generateSalt() (string, error) {
 	return base64.RawURLEncoding.EncodeToString(salt), nil
 }
 
-func keyExistsInMap(key string, claims map[string]interface{}) bool {
-	for k, v := range claims {
-		if k == key {
-			return true
-		}
-
-		if obj, ok := v.(map[string]interface{}); ok {
-			exists := keyExistsInMap(key, obj)
-			if exists {
-				return true
-			}
-		}
-	}
-
-	return false
-}
-
 // payload represents SD-JWT payload.
 type payload struct {
 	// registered claim names