fix(waku_keystore): sigsegv on different appInfo (#2654)

* fix(waku_keystore): sigsegv on different appInfo

* fix: field specific errors

* fix: more verbose error logs

tests/test_waku_keystore.nim

@@ -246,3 +246,60 @@ procSuite "Credentials test suite":
     check:
       recoveredCredentialsRes.isErr()
       recoveredCredentialsRes.error.kind == KeystoreCredentialNotFoundError
+
+  test "if a keystore exists, but the keystoreQuery doesn't match it":
+    let filepath = "./testAppKeystore.txt"
+    defer:
+      removeFile(filepath)
+
+    # We generate random identity credentials (inter-value constrains are not enforced, otherwise we need to load e.g. zerokit RLN keygen)
+    let
+      idTrapdoor = randomSeqByte(rng[], 32)
+      idNullifier = randomSeqByte(rng[], 32)
+      idSecretHash = randomSeqByte(rng[], 32)
+      idCommitment = randomSeqByte(rng[], 32)
+      idCredential = IdentityCredential(
+        idTrapdoor: idTrapdoor,
+        idNullifier: idNullifier,
+        idSecretHash: idSecretHash,
+        idCommitment: idCommitment,
+      )
+
+    # We generate two distinct membership groups
+    let contract = MembershipContract(
+      chainId: "5", address: "0x0123456789012345678901234567890123456789"
+    )
+    let index = MembershipIndex(1)
+    var membershipCredential = KeystoreMembership(
+      membershipContract: contract, treeIndex: index, identityCredential: idCredential
+    )
+
+    let password = "%m0um0ucoW%"
+
+    let keystoreRes = addMembershipCredentials(
+      path = filepath,
+      membership = membershipCredential,
+      password = password,
+      appInfo = testAppInfo,
+    )
+
+    assert(keystoreRes.isOk(), $keystoreRes.error)
+
+    let badTestAppInfo =
+      AppInfo(application: "_bad_test_", appIdentifier: "1234", version: "0.1")
+
+    # We test retrieval of credentials.
+    let membershipQuery = KeystoreMembership(membershipContract: contract)
+
+    let recoveredCredentialsRes = getMembershipCredentials(
+      path = filepath,
+      password = password,
+      query = membershipQuery,
+      appInfo = badTestAppInfo,
+    )
+
+    check:
+      recoveredCredentialsRes.isErr()
+      recoveredCredentialsRes.error.kind == KeystoreJsonValueMismatchError
+      recoveredCredentialsRes.error.msg ==
+        "Application does not match. Expected '_bad_test_' but got 'test'"

waku/waku_keystore/keystore.nim

@@ -45,7 +45,6 @@ proc loadAppKeystore*(
 ): KeystoreResult[JsonNode] =
   ## Load and decode JSON keystore from pathname
   var data: JsonNode
-  var matchingAppKeystore: JsonNode
 
   # If no keystore exists at path we create a new empty one with passed keystore parameters
   if fileExists(path) == false:
@@ -77,14 +76,45 @@ proc loadAppKeystore*(
 
         # We check if parsed json contains the relevant keystore credentials fields and if these are set to the passed parameters
         # (note that "if" is lazy, so if one of the .contains() fails, the json fields contents will not be checked and no ResultDefect will be raised due to accessing unavailable fields)
-        if data.hasKeys(["application", "appIdentifier", "credentials", "version"]) and
-            data["application"].getStr() == appInfo.application and
-            data["appIdentifier"].getStr() == appInfo.appIdentifier and
-            data["version"].getStr() == appInfo.version:
+        if not data.hasKeys(["application", "appIdentifier", "credentials", "version"]):
+          return err(
+            AppKeystoreError(
+              kind: KeystoreJsonKeyError, msg: "Missing required fields in keystore"
+            )
+          )
+
+        if data["application"].getStr() != appInfo.application:
+          return err(
+            AppKeystoreError(
+              kind: KeystoreJsonValueMismatchError,
+              msg:
+                "Application does not match. Expected '" & appInfo.application &
+                "' but got '" & data["application"].getStr() & "'",
+            )
+          )
+
+        if data["appIdentifier"].getStr() != appInfo.appIdentifier:
+          return err(
+            AppKeystoreError(
+              kind: KeystoreJsonValueMismatchError,
+              msg:
+                "AppIdentifier does not match. Expected '" & appInfo.appIdentifier &
+                "' but got '" & data["appIdentifier"].getStr() & "'",
+            )
+          )
+
+        if data["version"].getStr() != appInfo.version:
+          return err(
+            AppKeystoreError(
+              kind: KeystoreJsonValueMismatchError,
+              msg:
+                "Version does not match. Expected '" & appInfo.version & "' but got '" &
+                data["version"].getStr() & "'",
+            )
+          )
           # We return the first json keystore that matches the passed app parameters
           # We assume a unique kesytore with such parameters is present in the file
-          matchingAppKeystore = data
-          break
+        return ok(data)
       # TODO: we might continue rather than return for some of these errors
       except JsonParsingError:
         return
@@ -101,7 +131,12 @@ proc loadAppKeystore*(
   except IOError:
     return err(AppKeystoreError(kind: KeystoreIoError, msg: getCurrentExceptionMsg()))
 
-  return ok(matchingAppKeystore)
+  return err(
+    AppKeystoreError(
+      kind: KeystoreKeystoreDoesNotExist,
+      msg: "No keystore found for the passed parameters",
+    )
+  )
 
 # Adds a membership credential to the keystore matching the application, appIdentifier and version filters.
 proc addMembershipCredentials*(

waku/waku_keystore/protocol_types.nim

@@ -168,6 +168,7 @@ type
     KeystoreOsError = "keystore error: OS specific error"
     KeystoreIoError = "keystore error: IO specific error"
     KeystoreJsonKeyError = "keystore error: fields not present in JSON"
+    KeystoreJsonValueMismatchError = "keystore error: JSON value mismatch"
     KeystoreJsonError = "keystore error: JSON encoder/decoder error"
     KeystoreKeystoreDoesNotExist = "keystore error: file does not exist"
     KeystoreCreateKeystoreError = "Error while creating application keystore"