Merge branch 'soru/verify-device-signatures' into 'main'

fix: Verify device signatures before storing devices and block those with invalid signatures

See merge request famedly/famedlysdk!578

lib/matrix_api.dart

@@ -20,6 +20,7 @@ library matrix_api;
 
 export 'matrix_api/matrix_api.dart';
 export 'matrix_api/utils/logs.dart';
+export 'matrix_api/utils/try_get_map_extension.dart';
 export 'matrix_api/model/algorithm_types.dart';
 export 'matrix_api/model/basic_event.dart';
 export 'matrix_api/model/basic_event_with_sender.dart';

lib/src/client.dart

@@ -1419,6 +1419,8 @@ sort order of ${prevState.sortOrder}. This should never happen...''');
                 _userDeviceKeys[userId].deviceKeys[deviceId] =
                     oldKeys[deviceId];
               }
+            } else {
+              Logs().w('Invalid device ${entry.userId}:${entry.deviceId}');
             }
             if (database != null) {
               dbActions.add(() => database.storeUserDeviceKey(

lib/src/utils/device_keys_list.dart

@@ -164,8 +164,17 @@ abstract class SignableKey extends MatrixSignableKey {
     return String.fromCharCodes(canonicalJson.encode(data));
   }
 
-  bool _verifySignature(String pubKey, String signature) {
+  bool _verifySignature(String pubKey, String signature,
-    final olmutil = olm.Utility();
+      {bool isSignatureWithoutLibolmValid = false}) {
+    olm.Utility olmutil;
+    try {
+      olmutil = olm.Utility();
+    } on NoSuchMethodError {
+      // if no libolm is present we land in this catch block, and return the default
+      // set if no libolm is there. Some signatures should be assumed-valid while others
+      // should be assumed-invalid
+      return isSignatureWithoutLibolmValid;
+    }
     var valid = false;
     try {
       olmutil.ed25519_verify(pubKey, signingContent, signature);
@@ -209,6 +218,10 @@ abstract class SignableKey extends MatrixSignableKey {
           continue;
         }
         final keyId = fullKeyId.substring('ed25519:'.length);
+        // we ignore self-signatures here
+        if (otherUserId == userId && keyId == identifier) {
+          continue;
+        }
         SignableKey key;
         if (client.userDeviceKeys[otherUserId].deviceKeys.containsKey(keyId)) {
           key = client.userDeviceKeys[otherUserId].deviceKeys[keyId];
@@ -356,12 +369,26 @@ class DeviceKeys extends SignableKey {
   String get deviceDisplayName =>
       unsigned != null ? unsigned['device_display_name'] : null;
 
+  bool get selfSigned => signatures
+              ?.tryGet<Map<String, dynamic>>(userId)
+              ?.tryGet<String>('ed25519:$deviceId') ==
+          null
+      ? false
+      // without libolm we still want to be able to add devices. In that case we ofc just can't
+      // verify the signature
+      : _verifySignature(ed25519Key, signatures[userId]['ed25519:$deviceId'],
+          isSignatureWithoutLibolmValid: true);
+
+  @override
+  bool get blocked => super.blocked || !selfSigned;
+
   bool get isValid =>
       userId != null &&
       deviceId != null &&
       keys != null &&
       curve25519Key != null &&
-      ed25519Key != null;
+      ed25519Key != null &&
+      selfSigned;
 
   @override
   Future<void> setVerified(bool newVerified, [bool sign = true]) {

test/device_keys_list_test.dart

@@ -92,6 +92,42 @@ void main() {
       client = await getClient();
     });
 
+    test('reject devices without self-signature', () async {
+      var key = DeviceKeys.fromJson({
+        'user_id': '@test:fakeServer.notExisting',
+        'device_id': 'BADDEVICE',
+        'algorithms': [
+          AlgorithmTypes.olmV1Curve25519AesSha2,
+          AlgorithmTypes.megolmV1AesSha2
+        ],
+        'keys': {
+          'curve25519:BADDEVICE': 'ds6+bItpDiWyRaT/b0ofoz1R+GCy7YTbORLJI4dmYho',
+          'ed25519:BADDEVICE': 'CdDKVf44LO2QlfWopP6VWmqedSrRaf9rhHKvdVyH38w'
+        },
+      }, client);
+      expect(key.isValid, false);
+      expect(key.selfSigned, false);
+      key = DeviceKeys.fromJson({
+        'user_id': '@test:fakeServer.notExisting',
+        'device_id': 'BADDEVICE',
+        'algorithms': [
+          AlgorithmTypes.olmV1Curve25519AesSha2,
+          AlgorithmTypes.megolmV1AesSha2
+        ],
+        'keys': {
+          'curve25519:BADDEVICE': 'ds6+bItpDiWyRaT/b0ofoz1R+GCy7YTbORLJI4dmYho',
+          'ed25519:BADDEVICE': 'CdDKVf44LO2QlfWopP6VWmqedSrRaf9rhHKvdVyH38w'
+        },
+        'signatures': {
+          '@test:fakeServer.notExisting': {
+            'ed25519:BADDEVICE': 'invalid',
+          },
+        },
+      }, client);
+      expect(key.isValid, false);
+      expect(key.selfSigned, false);
+    });
+
     test('set blocked / verified', () async {
       final key =
           client.userDeviceKeys[client.userID].deviceKeys['OTHERDEVICE'];
@@ -104,10 +140,17 @@ void main() {
           AlgorithmTypes.megolmV1AesSha2
         ],
         'keys': {
-          'curve25519:UNSIGNEDDEVICE': 'blah',
+          'curve25519:UNSIGNEDDEVICE':
-          'ed25519:UNSIGNEDDEVICE': 'blah'
+              'ds6+bItpDiWyRaT/b0ofoz1R+GCy7YTbORLJI4dmYho',
+          'ed25519:UNSIGNEDDEVICE':
+              'CdDKVf44LO2QlfWopP6VWmqedSrRaf9rhHKvdVyH38w'
+        },
+        'signatures': {
+          '@test:fakeServer.notExisting': {
+            'ed25519:UNSIGNEDDEVICE':
+                'f2p1kv6PIz+hnoFYnHEurhUKIyRsdxwR2RTKT1EnQ3aF2zlZOjmnndOCtIT24Q8vs2PovRw+/jkHKj4ge2yDDw',
+          },
         },
-        'signatures': <String, dynamic>{},
       }, client);
       final masterKey = client.userDeviceKeys[client.userID].masterKey;
       masterKey.setDirectVerified(true);
@@ -185,7 +228,8 @@ void main() {
       user.deviceKeys['OTHERDEVICE'].setDirectVerified(false);
 
       user.masterKey.setDirectVerified(true);
-      user.deviceKeys['GHTYAJCE'].signatures.clear();
+      user.deviceKeys['GHTYAJCE'].signatures[client.userID]
+          .removeWhere((k, v) => k != 'ed25519:GHTYAJCE');
       expect(user.deviceKeys['GHTYAJCE'].verified,
           true); // it's our own device, should be direct verified
       expect(

test/encryption/key_manager_test.dart

@@ -170,10 +170,15 @@ void main() {
           AlgorithmTypes.megolmV1AesSha2
         ],
         'keys': {
-          'curve25519:JLAFKJWSCS':
+          'curve25519:NEWDEVICE': 'bnKQp6pPW0l9cGoIgHpBoK5OUi4h0gylJ7upc4asFV8',
-              '3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI',
+          'ed25519:NEWDEVICE': 'ZZhPdvWYg3MRpGy2MwtI+4MHXe74wPkBli5hiEOUi8Y'
-          'ed25519:JLAFKJWSCS': 'lEuiRJBit0IG6nUf5pUzWTUEsRVVe/HJkoKuEww9ULI'
         },
+        'signatures': {
+          '@alice:example.com': {
+            'ed25519:NEWDEVICE':
+                '94GSg8N9vNB8wyWHJtKaaX3MGNWPVOjBatJM+TijY6B1RlDFJT5Cl1h/tjr17AoQz0CDdOf6uFhrYsBkH1/ABg'
+          }
+        }
       }, client);
       await client.encryption.keyManager.clearOrUseOutboundGroupSession(roomId);
       expect(

test/encryption/key_request_test.dart

@@ -302,7 +302,7 @@ void main() {
             'forwarding_curve25519_key_chain': [],
           },
           encryptedContent: {
-            'sender_key': '3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI',
+            'sender_key': 'L+4+JCl8MD63dgo8z5Ta+9QAHXiANyOVSfgbHA5d3H8',
           });
       await matrix.encryption.keyManager.handleToDeviceEvent(event);
       expect(
@@ -327,7 +327,7 @@ void main() {
             'forwarding_curve25519_key_chain': [],
           },
           encryptedContent: {
-            'sender_key': '3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI',
+            'sender_key': 'L+4+JCl8MD63dgo8z5Ta+9QAHXiANyOVSfgbHA5d3H8',
           });
       await matrix.encryption.keyManager.handleToDeviceEvent(event);
       expect(

test/fake_matrix_api.dart

@@ -26,6 +26,16 @@ import 'package:famedlysdk/matrix_api.dart';
 import 'package:http/http.dart';
 import 'package:http/testing.dart';
 
+Map<String, dynamic> decodeJson(dynamic data) {
+  if (data is String) {
+    return json.decode(data);
+  }
+  if (data.isEmpty) {
+    return <String, dynamic>{};
+  }
+  return data;
+}
+
 class FakeMatrixApi extends MockClient {
   static final calledEndpoints = <String, List<dynamic>>{};
   static int eventCounter = 0;
@@ -98,7 +108,7 @@ class FakeMatrixApi extends MockClient {
             final syncUpdate = sdk.SyncUpdate()
               ..accountData = [
                 sdk.BasicEvent()
-                  ..content = json.decode(data)
+                  ..content = decodeJson(data)
                   ..type = type
               ];
             if (client.database != null) {
@@ -1743,32 +1753,37 @@ class FakeMatrixApi extends MockClient {
       '/client/r0/keys/claim': (var req) => {
             'failures': {},
             'one_time_keys': {
-              '@alice:example.com': {
+              if (decodeJson(req)['one_time_keys']['@alice:example.com'] !=
-                'JLAFKJWSCS': {
+                  null)
-                  'signed_curve25519:AAAAHg': {
+                '@alice:example.com': {
-                    'key': 'zKbLg+NrIjpnagy+pIY6uPL4ZwEG2v+8F9lmgsnlZzs',
+                  'JLAFKJWSCS': {
-                    'signatures': {
+                    'signed_curve25519:AAAAAQ': {
-                      '@alice:example.com': {
+                      'key': 'ikMXajRlkS7Xi9CROrAh3jXnbygk8mLBdSaY9/al0X0',
-                        'ed25519:JLAFKJWSCS':
+                      'signatures': {
-                            'FLWxXqGbwrb8SM3Y795eB6OA8bwBcoMZFXBqnTn58AYWZSqiD45tlBVcDa2L7RwdKXebW/VzDlnfVJ+9jok1Bw'
+                        '@alice:example.com': {
+                          'ed25519:JLAFKJWSCS':
+                              'XdboCa0Ljoh0Y0i/IVnmMqy/+T1hJyu8BA/nRYniJMQ7QWh/pGS5AsWswdARD+MAX+r4u98Qzk0y27HUddZXDA'
+                        }
                       }
                     }
                   }
-                }
+                },
-              },
+              if (decodeJson(req)['one_time_keys']
-              '@test:fakeServer.notExisting': {
+                      ['@test:fakeServer.notExisting'] !=
-                'GHTYAJCE': {
+                  null)
-                  'signed_curve25519:AAAAAQ': {
+                '@test:fakeServer.notExisting': {
-                    'key': 'qc72ve94cA28iuE0fXa98QO3uls39DHWdQlYyvvhGh0',
+                  'GHTYAJCE': {
-                    'signatures': {
+                    'signed_curve25519:AAAAAQ': {
-                      '@test:fakeServer.notExisting': {
+                      'key': 'qc72ve94cA28iuE0fXa98QO3uls39DHWdQlYyvvhGh0',
-                        'ed25519:GHTYAJCE':
+                      'signatures': {
-                            'dFwffr5kTKefO7sjnWLMhTzw7oV31nkPIDRxFy5OQT2OP5++Ao0KRbaBZ6qfuT7lW1owKK0Xk3s7QTBvc/eNDA',
+                        '@test:fakeServer.notExisting': {
+                          'ed25519:GHTYAJCE':
+                              'dFwffr5kTKefO7sjnWLMhTzw7oV31nkPIDRxFy5OQT2OP5++Ao0KRbaBZ6qfuT7lW1owKK0Xk3s7QTBvc/eNDA',
+                        },
                       },
                     },
                   },
                 },
-              },
             }
           },
       '/client/r0/rooms/!localpart%3Aexample.com/invite': (var req) => {},
@@ -1786,7 +1801,7 @@ class FakeMatrixApi extends MockClient {
             'one_time_key_counts': {
               'curve25519': 10,
               'signed_curve25519':
-                  json.decode(req)['one_time_keys']?.keys?.length ?? 0,
+                  decodeJson(req)['one_time_keys']?.keys?.length ?? 0,
             }
           },
       '/client/r0/keys/query': (var req) => {
@@ -1802,14 +1817,14 @@ class FakeMatrixApi extends MockClient {
                   ],
                   'keys': {
                     'curve25519:JLAFKJWSCS':
-                        '3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI',
+                        'L+4+JCl8MD63dgo8z5Ta+9QAHXiANyOVSfgbHA5d3H8',
                     'ed25519:JLAFKJWSCS':
-                        'lEuiRJBit0IG6nUf5pUzWTUEsRVVe/HJkoKuEww9ULI'
+                        'rUFJftIWpFF/jqqz3bexGGYiG8UobKhzkeabqw1v0zM'
                   },
                   'signatures': {
                     '@alice:example.com': {
                       'ed25519:JLAFKJWSCS':
-                          'dSO80A01XiigH3uBiDVx/EjzaoycHcjq9lfQX0uWsqxl2giMIiSPR8a4d291W1ihKJL/a+myXS367WT6NAIcBA'
+                          'go3mi5o3Ile+Ik+lCEpHmBmyJmKWfnRDCBBvfaVlKsMyha5IORuYcxwEUrAeLyAeeeHvkWDFX+No5eY1jYeKBw'
                     }
                   },
                   'unsigned': {'device_display_name': 'Alices mobile phone'}
@@ -1822,10 +1837,17 @@ class FakeMatrixApi extends MockClient {
                     AlgorithmTypes.megolmV1AesSha2
                   ],
                   'keys': {
-                    'curve25519:OTHERDEVICE': 'blah',
+                    'curve25519:OTHERDEVICE':
-                    'ed25519:OTHERDEVICE': 'blah'
+                        'wMIDhiQl5jEXQrTB03ePOSQfR8sA/KMrW0CIfFfXKEE',
+                    'ed25519:OTHERDEVICE':
+                        '2Lyaj5NB7HPqKZMjZpA/pECXuQ+9wi8AGFdw33y3DuQ'
+                  },
+                  'signatures': {
+                    '@alice:example.com': {
+                      'ed25519:OTHERDEVICE':
+                          'bwHd6ylISP13AICdDPd0HQd4V6dvvd4vno8/OwUNdm9UAprr3YjkDqVw425I74u2UQAarq9bytBqVqFyD6trAw',
+                    }
                   },
-                  'signatures': {},
                 },
               },
               '@test:fakeServer.notExisting': {
@@ -1844,6 +1866,8 @@ class FakeMatrixApi extends MockClient {
                   },
                   'signatures': {
                     '@test:fakeServer.notExisting': {
+                      'ed25519:GHTYAJCE':
+                          'NEQeTgv7ew1IZSLQphWd0y60EdHdcNfHgvoaMQco5XKeIYyiUZIWd7F4x/mkPDjUizv6yWMbTDCWdSg5XcgNBA',
                       'ed25519:F9ypFzgbISXCzxQhhSnXMkc1vq12Luna3Nw5rqViOJY':
                           'Q4/55vZjEJD7M2EC40bgZqd9Zuy/4C75UPVopJdXeioQVaKtFf6EF0nUUuql0yD+r3hinsZcock0wO6Q2xcoAQ',
                     },
@@ -1857,13 +1881,17 @@ class FakeMatrixApi extends MockClient {
                     AlgorithmTypes.megolmV1AesSha2
                   ],
                   'keys': {
-                    'curve25519:OTHERDEVICE': 'blah',
+                    'curve25519:OTHERDEVICE':
-                    'ed25519:OTHERDEVICE': 'blah'
+                        'R96BA0qE1+QAWLp7E1jyWSTJ1VXMLpEdiM2SZHlKMXM',
+                    'ed25519:OTHERDEVICE':
+                        'EQo9eYbSygIbOR+tVJziqAY1NI6Gga+JQOVIqJe4mr4'
                   },
                   'signatures': {
                     '@test:fakeServer.notExisting': {
+                      'ed25519:OTHERDEVICE':
+                          '/rT6pVRypJWxGos1QcI7jHL9HwcA83nkHLHqMcRPeLSxXHh4oHWvC0/tl0Xg06ogyiGw4NuB7TpOISvJBdt7BA',
                       'ed25519:F9ypFzgbISXCzxQhhSnXMkc1vq12Luna3Nw5rqViOJY':
-                          'o7ucKPWrF2VKx7wYqP1f+aw4QohLMz7kX+SIw6aWCYsLC3XyIlg8rX/7QQ9B8figCVnRK7IjtjWvQodBCfWCAA',
+                          'qnjiLl36h/1jlLvcAgt46Igaod2T9lOSnoSVkV0KC+c7vYIjG4QBzXpH+hycfufOT/y+a/kl52dUTLQWctMKCA',
                     },
                   },
                 },
@@ -1882,7 +1910,12 @@ class FakeMatrixApi extends MockClient {
                     'ed25519:FOXDEVICE':
                         'R5/p04tticvdlNIxiiBIP0j9OQWv8ep6eEU6/lWKDxw',
                   },
-                  'signatures': {},
+                  'signatures': {
+                    '@othertest:fakeServer.notExisting': {
+                      'ed25519:FOXDEVICE':
+                          '2lJ3atmRIWgkyQNC9gvWEpxwuozsBQsg33M2IMDJqLhx/+g3Ds1vQ683dJsYIu04ORa4U0L9TqieHVpV/7qqDA',
+                    },
+                  },
                 },
               },
             },
@@ -2002,7 +2035,7 @@ class FakeMatrixApi extends MockClient {
       '/client/r0/rooms/!localpart%3Aserver.abc/invite': (var reqI) => {},
       '/client/unstable/keys/device_signing/upload': (var reqI) {
         if (client != null) {
-          final jsonBody = json.decode(reqI);
+          final jsonBody = decodeJson(reqI);
           for (final keyType in {
             'master_key',
             'self_signing_key',

test/matrix_api_test.dart

@@ -1113,7 +1113,12 @@ void main() {
         },
         timeout: 10,
       );
-      expect(FakeMatrixApi.api['POST']['/client/r0/keys/claim']({}),
+      expect(
+          FakeMatrixApi.api['POST']['/client/r0/keys/claim']({
+            'one_time_keys': {
+              '@alice:example.com': {'JLAFKJWSCS': 'signed_curve25519'}
+            }
+          }),
           response.toJson());
 
       matrixApi.homeserver = matrixApi.accessToken = null;