Merge pull request #445 from atsign-foundation/reserved_key_format

fix: stricter regex for reserved keys

packages/at_commons/CHANGELOG.md

@@ -1,3 +1,5 @@
+## 4.0.0
+- fix: Improved regex for Reserved keys (Internal keys used by the server)
 ## 3.0.58
 - fix: Deprecate encryptedDefaultEncryptedPrivateKey in EnrollParams and introduce encryptedDefaultEncryptedPrivateKey for readability
 - fix: Replace encryptedDefaultEncryptedPrivateKey with encryptedDefaultEncryptionPrivateKey in EnrollVerbBuilder

packages/at_commons/lib/src/at_constants.dart

@@ -20,6 +20,7 @@ class AtConstants {
   static const String atSigningPublicKey = 'public:signing_publickey';
   static const String atCramSecret = 'privatekey:at_secret';
   static const String atCramSecretDeleted = 'privatekey:at_secret_deleted';
+  static const String atBlocklist = 'private:blocklist'; // contains @atsign postfix
   static const String atSigningKeypairGenerated =
       'privatekey:signing_keypair_generated';
   static const String statId = 'statId';

packages/at_commons/lib/src/utils/at_key_regex_utils.dart

@@ -8,19 +8,31 @@ abstract class Regexes {
   static const charsInEntity = r'''[\w\.\-_'*"]''';
   static const allowedEmoji =
       r'''((\u00a9|\u00ae|[\u2000-\u3300]|[\ufe00-\ufe0f]|\ud83c[\ud000-\udfff]|\ud83d[\ud000-\udfff]|\ud83e[\ud000-\udfff]))''';
-  static const _charsInReservedKey =
-      r'(shared_key|publickey|privatekey|self_encryption_key'
-      r'|commitLogCompactionStats|accessLogCompactionStats'
-      r'|notificationCompactionStats|signing_privatekey|signing_publickey'
-      r'|signing_keypair_generated|at_pkam_privatekey|at_pkam_publickey'
-      r'|at_secret_deleted|at_secret'
-      r'|configkey'
-      r'|_[\w-]+|)';
+
+  static const _reservedKeysWithoutAtsignSuffix =
+      r'(((?<=privatekey:)(at_pkam_publickey|at_pkam_privatekey'
+      '|privatekey|self_encryption_key'
+      '|at_secret|at_secret_deleted'
+      '|signing_keypair_generated|commitLogCompactionStats'
+      '|accessLogCompactionStats'
+      '|notificationCompactionStats)\$)|^(configkey)\$|(?:^_($charsInEntity)+)\$)';
+  // the last part of the above regex is to match internal keys such as
+  // _latestNotificationId (keys that start with an underscore)
+
+  /// The following reserved keys are suffixed by the atsign. [ownershipFragment]
+  /// at the end represents the atsign
+  static const _reservedKeysWithAtsignSuffix = r'(((?<=private:)blocklist'
+      '|(?<=public:)signing_publickey'
+      '|(?<=$ownershipFragmentWithoutNamedGroup:)signing_privatekey'
+      '|(?<=^@($sharedWithFragment))shared_key'
+      '|(?<=public:)publickey)(?=$ownershipFragment))';
 
   static const String namespaceFragment =
       '''\\.(?<namespace>$charsInNamespace)''';
   static const String ownershipFragment =
       '''@(?<owner>($charsInAtSign|$allowedEmoji){1,55})''';
+  static const String ownershipFragmentWithoutNamedGroup =
+      '''@($charsInAtSign|$allowedEmoji){1,55}''';
   static const String sharedWithFragment =
       '''((?<sharedWith>($charsInAtSign|$allowedEmoji){1,55}):)''';
   static const String entityFragment =
@@ -39,7 +51,7 @@ abstract class Regexes {
   static const String cachedPublicKeyStartFragment =
       '''(?<visibility>(cached:public:){1})$entityFragment''';
   static const String reservedKeyFragment =
-      '''(((@(?<sharedWith>($charsInAtSign|$allowedEmoji){1,55}))|public|privatekey):)?(?<atKey>$_charsInReservedKey)(@(?<owner>($charsInAtSign|$allowedEmoji){1,55}))?''';
+      '''(?<atKey>($_reservedKeysWithoutAtsignSuffix|$_reservedKeysWithAtsignSuffix))''';
   static const String localKeyFragment =
       '''(?<visibility>(local:){1})$entityFragment''';
 
@@ -154,11 +166,9 @@ class RegexUtil {
   /// Returns a first matching key type after matching the key against regexes for each of the key type
   static KeyType keyType(String key, bool enforceNamespace) {
     Regexes regexes = Regexes(enforceNamespace);
-
-    if (matchAll(regexes.reservedKey, key)) {
+    if (isPartialMatch(regexes.reservedKey, key)) {
       return KeyType.reservedKey;
     }
-
     // matches the key with public key regex.
     if (matchAll(regexes.publicKey, key)) {
       return KeyType.publicKey;
@@ -193,14 +203,20 @@ class RegexUtil {
     return KeyType.invalidKey;
   }
 
-  /// Matches a regex against the input.
-  /// Returns a true if the regex is matched and a false otherwise
+  /// Matches a regex against the input
+  /// Returns a true if the regex is matched to the ENTIRE string, false otherwise
   static bool matchAll(String regex, String input) {
     var regExp = RegExp(regex, caseSensitive: false);
     return regExp.hasMatch(input) &&
         regExp.stringMatch(input)!.length == input.length;
   }
 
+  /// Checks if the the [input] is a partial match to the [regex]
+  static bool isPartialMatch(String regex, String input) {
+    RegExp regExp = RegExp(regex, caseSensitive: false);
+    return regExp.hasMatch(input);
+  }
+
   /// Returns a [Map] containing named groups and the matched values in the input
   /// Returns an empty [Map] if no matches are found
   static Map<String, String> matchesByGroup(String regex, String input) {

packages/at_commons/pubspec.yaml

@@ -1,6 +1,6 @@
 name: at_commons
 description: A library of Dart and Flutter utility classes that are used across other components of the atPlatform.
-version: 3.0.58
+version: 4.0.0
 repository: https://github.com/atsign-foundation/at_libraries
 homepage: https://atsign.dev
 

packages/at_commons/test/at_key_regex_test.dart

@@ -1,4 +1,4 @@
-import 'package:at_commons/src/keystore/key_type.dart';
+import 'package:at_commons/at_commons.dart';
 import 'package:at_commons/src/utils/at_key_regex_utils.dart';
 import 'package:test/test.dart';
 
@@ -225,17 +225,51 @@ void main() {
         expect(type == KeyType.cachedSharedKey, true);
       }
     });
+  });
 
-    test('A test to validate reserved key type', () {
-      var keyTypeList = [];
+  group('Validate reserved keys regex', () {
 
-      keyTypeList.add('public:signing_publickey@alice');
-      keyTypeList.add('public:signing_publickey@☎️_0002');
+    test('Validate appropriate parts of signing_pub_key are identified correctly', () {
+      String key = 'public:signing_publickey@owner';
+      var type = RegexUtil.keyType(key, false);
+      expect(type, KeyType.reservedKey);
 
-      for (var key in keyTypeList) {
-        var type = RegexUtil.keyType(key, false);
-        expect(type == KeyType.reservedKey, true);
-      }
+      var matches = RegexUtil.matchesByGroup(Regexes(false).reservedKey, key);
+      expect(matches['owner'], 'owner');
+      expect(matches['atKey'], 'signing_publickey');
+    });
+
+    test('Validate appropriate parts of enc_shared_key are identified correctly', () {
+      String key = '@reno:${AtConstants.atEncryptionSharedKey}@ajax';
+      var type = RegexUtil.keyType(key, false);
+      expect(type, KeyType.reservedKey);
+
+      var matches = RegexUtil.matchesByGroup(Regexes(false).reservedKey, key);
+      expect(matches['owner'], 'ajax');
+      expect(matches['atKey'], AtConstants.atEncryptionSharedKey);
+      expect(matches['sharedWith'], 'reno');
+    });
+
+    test('Validate appropriate parts of pkam_pub_key are identified correctly', () {
+      String key = 'privatekey:at_pkam_publickey';
+      var type = RegexUtil.keyType(key, false);
+      expect(type, KeyType.reservedKey);
+
+      var matches = RegexUtil.matchesByGroup(Regexes(false).reservedKey, key);
+      expect(matches['owner'], '');
+      expect(matches['atKey'], 'at_pkam_publickey');
+      expect(matches['sharedWith'], '');
+    });
+
+    test('Validate appropriate parts of _latestNotificationId are identified correctly', () {
+      String key = '_latestNotificationId';
+      var type = RegexUtil.keyType(key, false);
+      expect(type, KeyType.reservedKey);
+
+      var matches = RegexUtil.matchesByGroup(Regexes(false).reservedKey, key);
+      expect(matches['owner'], '');
+      expect(matches['atKey'], '_latestNotificationId');
+      expect(matches['sharedWith'], '');
     });
   });
 

packages/at_commons/test/at_key_type_test.dart

@@ -1,4 +1,5 @@
 import 'package:at_commons/at_commons.dart';
+import 'package:at_commons/src/utils/at_key_regex_utils.dart';
 import 'package:test/test.dart';
 
 void main() {
@@ -34,6 +35,7 @@ void main() {
       expect(keyType, equals(KeyType.localKey));
     });
   });
+
   group('A group of tests to check invalid key types', () {
     test('Test public key type without namespace', () {
       var keyType =
@@ -92,65 +94,135 @@ void main() {
   });
 
   group('A group of tests to check reserved key types', () {
-    test('Test reserved key type for shared_key', () {
-      var keyType = AtKey.getKeyType('@bob:shared_key@alice');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for encryption publickey', () {
-      var keyType = AtKey.getKeyType('public:publickey@alice');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for self encryption key', () {
-      var keyType = AtKey.getKeyType('privatekey:self_encryption_key');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for signing private key', () {
-      var keyType = AtKey.getKeyType('@alice:signing_privatekey@alice');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for public session key', () {
-      var keyType = AtKey.getKeyType(
-          'public:_a29464d0-1f2d-4216-b903-031963bc4ab3@alice');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for latest notification id', () {
-      var keyType = AtKey.getKeyType('_latestNotificationIdv2');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for signing public key', () {
-      var keyType = AtKey.getKeyType('public:signing_publickey@colin');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for commit log compaction key', () {
-      var keyType = AtKey.getKeyType('privatekey:commitLogCompactionStats');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for access log compaction key', () {
-      var keyType = AtKey.getKeyType('privatekey:accessLogCompactionStats');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for cram secret deleted', () {
-      var keyType = AtKey.getKeyType('privatekey:at_secret_deleted');
-      expect(keyType, equals(KeyType.reservedKey));
-    });
-
-    test('Test reserved key type for cram secret', () {
-      var keyType = AtKey.getKeyType('privatekey:at_secret');
-      expect(keyType, equals(KeyType.reservedKey));
+    test('A positive test to validate reserved key type', () {
+      var keyTypeList = [];
+      var fails = [];
+      // keys with atsign
+      keyTypeList.add('${AtConstants.atBlocklist}@☎️_0002');
+      keyTypeList.add('@bob:${AtConstants.atEncryptionSharedKey}@alice');
+      keyTypeList.add('@allen:${AtConstants.atSigningPrivateKey}@allen');
+      keyTypeList.add('${AtConstants.atEncryptionPublicKey}@owner');
+      keyTypeList.add('public:signing_publickey@alice');
+      keyTypeList.add('public:signing_publickey@☎️_0002');
+      // keys without atsign
+      keyTypeList.add(AtConstants.atPkamPublicKey);
+      keyTypeList.add(AtConstants.atPkamPrivateKey);
+      keyTypeList.add(AtConstants.atEncryptionPrivateKey);
+      keyTypeList.add(AtConstants.atEncryptionSelfKey);
+      keyTypeList.add(AtConstants.atCramSecret);
+      keyTypeList.add(AtConstants.atCramSecretDeleted);
+      keyTypeList.add(AtConstants.atSigningKeypairGenerated);
+      keyTypeList.add(AtConstants.commitLogCompactionKey);
+      keyTypeList.add(AtConstants.accessLogCompactionKey);
+      keyTypeList.add(AtConstants.notificationCompactionKey);
+      keyTypeList.add('configkey');
+      keyTypeList.add('_latestNotificationIdv2');
+
+      for (var key in keyTypeList) {
+        var type = RegexUtil.keyType(key, false);
+        print(key);
+        if(type != KeyType.reservedKey){
+          fails.add('$key classified as $type - actually a reserved key');
+        }
+      }
+      expect(fails, []);
+    });
+
+    test('Validate no false positives for reserved keys with atsign', () {
+      var keyTypeList = [];
+      var fails = [];
+      // these keys are supposed to have an atsign at the end
+      // to test a negative case, the @atsign at the end has been removed
+      keyTypeList.add('public:publickey');
+      keyTypeList.add('public:signing_publickey');
+      keyTypeList.add(AtConstants.atBlocklist);
+      keyTypeList.add('@bob:${AtConstants.atEncryptionSharedKey}');
+      keyTypeList.add(AtConstants.atEncryptionSharedKey);
+      keyTypeList.add('@allen:${AtConstants.atSigningPrivateKey}');
+      keyTypeList.add(AtConstants.atSigningPrivateKey);
+
+      for (var key in keyTypeList) {
+        var type = RegexUtil.keyType(key, false);
+        if (type == KeyType.reservedKey) {
+          fails.add('got $type for $key - which is not a reserved key');
+        }
+      }
+      expect(fails, []);
+    });
+
+    test('Validate no false positives for reserved keys without atsign', () {
+      var keysList = [];
+      var fails = [];
+      // the following keys are not supposed to have an atsign at the end
+      // for the sake of testing a negative case, atsigns have been appended
+      // to the keys
+      keysList.add('${AtConstants.atPkamPublicKey}@alice123');
+      keysList.add('${AtConstants.atPkamPrivateKey}@alice123');
+      keysList.add('${AtConstants.atEncryptionPrivateKey}@alice123');
+      keysList.add('${AtConstants.atEncryptionSelfKey}@alice123');
+      keysList.add('${AtConstants.atCramSecret}@alice123');
+      keysList.add('${AtConstants.atCramSecretDeleted}@alice123');
+      keysList.add('${AtConstants.atSigningKeypairGenerated}@alice123');
+      keysList.add('${AtConstants.commitLogCompactionKey}@alice123');
+      keysList.add('${AtConstants.accessLogCompactionKey}@alice123');
+      keysList.add('${AtConstants.notificationCompactionKey}@alice123');
+      keysList.add('configkey@alice123');
+      keysList.add('_latestNotificationIdv2@client');
+
+      for (var key in keysList) {
+        var type = RegexUtil.keyType(key, false);
+        if (type == KeyType.reservedKey) {
+          fails.add('got $type for $key - which is not a reserved key');
+        }
+      }
+      expect(fails, []);
+    });
+
+    test('Validate no false positives for reserved keys with incorrect visibility', (){
+      var keysList = [];
+      var fails = [];
+      // negative test to validate that e.g. only public:publickey@owner is a
+      // reserved key. @owner:publickey@owner is NOT a reserved key
+      keysList.add('public:blocklist@☎️_0002');
+      keysList.add('public:shared_key@alice');
+      keysList.add('public:signing_privatekey@allen');
+      keysList.add('☎️@owner:publickey@owner');
+      keysList.add('@alice:signing_publickey@alice');
+      keysList.add('@☎️_0002:signing_publickey@☎️_0002');
+      keysList.add('public:at_pkam_publickey');
+      keysList.add('public:at_pkam_privatekey');
+      keysList.add('public:privatekey');
+      keysList.add('public:self_encryption_key');
+      keysList.add('public:at_secret');
+      keysList.add('public:at_secret_deleted');
+      keysList.add('public:signing_keypair_generated');
+      keysList.add('public:commitLogCompactionStats');
+      keysList.add('public:accessLogCompactionStats');
+      keysList.add('public:notificationCompactionStats');
+      keysList.add('privatekey:configkey');
+      keysList.add('privatekey:_latestNotificationIdv2');
+
+      for (var key in keysList) {
+        var type = RegexUtil.keyType(key, false);
+        if (type == KeyType.reservedKey) {
+          fails.add('got $type for $key - which is not a reserved key');
+        }
+      }
+      expect(fails, []);
+    });
+
+    test('Ensure public hidden keys should NOT be classified as reserved keys', (){
+      var keyType = AtKey.getKeyType('public:__secretKey@cia', //double underscore after 'public:'
+          enforceNameSpace: false);
+      expect(keyType, isNot(KeyType.reservedKey));
+      expect(keyType, KeyType.publicKey);
     });
 
-    test('Test reserved key type for config key (blocklist/allowlist)', () {
-      var keyType = AtKey.getKeyType('configkey');
-      expect(keyType, equals(KeyType.reservedKey));
+    test('Ensure underscore keys should NOT be classified as reserved keys', (){
+      var keyType = AtKey.getKeyType('public:_secretKey@test',
+          enforceNameSpace: false);
+      expect(keyType, isNot(KeyType.reservedKey));
+      expect(keyType, KeyType.publicKey);
     });
   });
 }