feat(core): Verify the server signature with SASL2

packages/moxxmpp/lib/src/negotiators/negotiator.dart

@@ -134,7 +134,6 @@ abstract class XmppFeatureNegotiatorBase {
   NegotiatorAttributes get attributes => _attributes;
 
   /// Run after all negotiators are registered. Useful for registering callbacks against
-  /// other negotiators.
+  /// other negotiators. By default this function does nothing.
-  @visibleForOverriding
   Future<void> postRegisterCallback() async {}
 }

packages/moxxmpp/lib/src/negotiators/sasl/plain.dart

@@ -93,8 +93,9 @@ class SaslPlainNegotiator extends Sasl2AuthenticationNegotiator {
   }
 
   @override
-  Future<void> onSasl2Success(XMLNode response) async {
+  Future<Result<bool, NegotiatorError>> onSasl2Success(XMLNode response) async {
     state = NegotiatorState.done;
+    return const Result(true);
   }
 
   @override

packages/moxxmpp/lib/src/negotiators/sasl/scram.dart

@@ -15,6 +15,18 @@ import 'package:moxxmpp/src/types/result.dart';
 import 'package:random_string/random_string.dart';
 import 'package:saslprep/saslprep.dart';
 
+abstract class SaslScramError extends NegotiatorError {}
+
+class NoAdditionalDataError extends SaslScramError {
+  @override
+  bool isRecoverable() => false;
+}
+
+class InvalidServerSignatureError extends SaslScramError {
+  @override
+  bool isRecoverable() => false;
+}
+
 // NOTE: Inspired by https://github.com/vukoye/xmpp_dart/blob/3b1a0588562b9e591488c99d834088391840911d/lib/src/features/sasl/ScramSaslHandler.dart
 
 enum ScramHashType { sha1, sha256, sha512 }
@@ -230,6 +242,12 @@ class SaslScramNegotiator extends Sasl2AuthenticationNegotiator {
     return false;
   }
 
+  bool _checkSignature(String base64Signature) {
+    final signature =
+        parseKeyValue(utf8.decode(base64.decode(base64Signature)));
+    return signature['v']! == _serverSignature;
+  }
+
   @override
   Future<Result<NegotiatorState, NegotiatorError>> negotiate(
     XMLNode nonza,
@@ -271,10 +289,7 @@ class SaslScramNegotiator extends Sasl2AuthenticationNegotiator {
           );
         }
 
-        // NOTE: This assumes that the string is always "v=..." and contains no other parameters
+        if (!_checkSignature(nonza.innerText())) {
-        final signature =
-            parseKeyValue(utf8.decode(base64.decode(nonza.innerText())));
-        if (signature['v']! != _serverSignature) {
           // TODO(Unknown): Notify of a signature mismatch
           //final error = nonza.children.first.tag;
           //attributes.sendEvent(AuthenticationFailedEvent(error));
@@ -329,13 +344,32 @@ class SaslScramNegotiator extends Sasl2AuthenticationNegotiator {
     }
   }
 
+  @override
+  Future<void> postRegisterCallback() async {
+    attributes
+        .getNegotiatorById<Sasl2Negotiator>(sasl2Negotiator)
+        ?.registerSaslNegotiator(this);
+  }
+
   @override
   Future<List<XMLNode>> onSasl2FeaturesReceived(XMLNode sasl2Features) async {
     return [];
   }
 
   @override
-  Future<void> onSasl2Success(XMLNode response) async {
+  Future<Result<bool, NegotiatorError>> onSasl2Success(XMLNode response) async {
+    // When we're done with SASL2, check the additional data to verify the server
+    // signature.
     state = NegotiatorState.done;
+    final additionalData = response.firstTag('additional-data');
+    if (additionalData == null) {
+      return Result(NoAdditionalDataError());
+    }
+
+    if (!_checkSignature(additionalData.innerText())) {
+      return Result(InvalidServerSignatureError());
+    }
+
+    return const Result(true);
   }
 }

packages/moxxmpp/lib/src/negotiators/sasl2.dart

@@ -21,7 +21,7 @@ abstract class Sasl2FeatureNegotiator extends XmppFeatureNegotiatorBase {
 
   /// Called by the SASL2 negotiator when the SASL2 negotiations are done. [response]
   /// is the entire response nonza.
-  Future<void> onSasl2Success(XMLNode response);
+  Future<Result<bool, NegotiatorError>> onSasl2Success(XMLNode response);
 }
 
 /// A special type of [SaslNegotiator] that is aware of SASL2.
@@ -184,13 +184,25 @@ class Sasl2Negotiator extends XmppFeatureNegotiatorBase {
         if (nonza.tag == 'success') {
           // Tell the dependent negotiators about the result
           for (final negotiator in _featureNegotiators) {
-            await negotiator.onSasl2Success(nonza);
+            final result = await negotiator.onSasl2Success(nonza);
+            if (!result.isType<bool>()) {
+              return Result(result.get<NegotiatorError>());
+            }
           }
 
           // We're done
           attributes.setAuthenticated();
           attributes.removeNegotiatingFeature(saslXmlns);
           return const Result(NegotiatorState.done);
+        } else if (nonza.tag == 'challenge') {
+          // We still have to negotiate
+          final challenge = nonza.innerText();
+          final response = XMLNode.xmlns(
+            tag: 'response',
+            xmlns: sasl2Xmlns,
+            text: await _currentSaslNegotiator!.getRawStep(challenge),
+          );
+          attributes.sendNonza(response);
         }
     }
 

packages/moxxmpp/test/xeps/xep_0388_test.dart

@@ -74,6 +74,92 @@ void main() {
       ),
     ]);
 
+    final result = await conn.connect(
+      waitUntilLogin: true,
+      shouldReconnect: false,
+      enableReconnectOnSuccess: false,
+    );
+    expect(result.isType<NegotiatorError>(), false);
+  });
+
+  test('Test SCRAM-SHA-1 SASL2 negotiation with a valid signature', () async {
+    final fakeSocket = StubTCPSocket([
+      StringExpectation(
+        "<stream:stream xmlns='jabber:client' version='1.0' xmlns:stream='http://etherx.jabber.org/streams' to='server' xml:lang='en'>",
+        '''
+<stream:stream
+    xmlns="jabber:client"
+    version="1.0"
+    xmlns:stream="http://etherx.jabber.org/streams"
+    from="server"
+    xml:lang="en">
+  <stream:features xmlns="http://etherx.jabber.org/streams">
+    <mechanisms xmlns="urn:ietf:params:xml:ns:xmpp-sasl">
+      <mechanism>PLAIN</mechanism>
+      <mechanism>SCRAM-SHA-256</mechanism>
+    </mechanisms>
+    <authentication xmlns='urn:xmpp:sasl:2'>
+      <mechanism>PLAIN</mechanism>
+      <mechanism>SCRAM-SHA-256</mechanism>
+    </authentication>
+    <bind xmlns="urn:ietf:params:xml:ns:xmpp-bind">
+      <required/>
+    </bind>
+  </stream:features>''',
+      ),
+      StanzaExpectation(
+        "<authenticate xmlns='urn:xmpp:sasl:2' mechanism='SCRAM-SHA-256'><user-agent id='d4565fa7-4d72-4749-b3d3-740edbf87770'><software>moxxmpp</software><device>PapaTutuWawa's awesome device</device></user-agent><initial-response>biwsbj11c2VyLHI9ck9wck5HZndFYmVSV2diTkVrcU8=</initial-response></authenticate>",
+        '''
+<challenge xmlns='urn:xmpp:sasl:2'>cj1yT3ByTkdmd0ViZVJXZ2JORWtxTyVodllEcFdVYTJSYVRDQWZ1eEZJbGopaE5sRiRrMCxzPVcyMlphSjBTTlk3c29Fc1VFamI2Z1E9PSxpPTQwOTY=</challenge>
+        ''',
+      ),
+      StanzaExpectation(
+        '<response xmlns="urn:xmpp:sasl:2">Yz1iaXdzLHI9ck9wck5HZndFYmVSV2diTkVrcU8laHZZRHBXVWEyUmFUQ0FmdXhGSWxqKWhObEYkazAscD1kSHpiWmFwV0lrNGpVaE4rVXRlOXl0YWc5empmTUhnc3FtbWl6N0FuZFZRPQ==</response>',
+        '<success xmlns="urn:xmpp:sasl:2"><additional-data>dj02cnJpVFJCaTIzV3BSUi93dHVwK21NaFVaVW4vZEI1bkxUSlJzamw5NUc0PQ==</additional-data><authorization-identifier>user@server</authorization-identifier></success>',
+      ),
+      StanzaExpectation(
+        "<iq xmlns='jabber:client' type='set' id='aaaa'><bind xmlns='urn:ietf:params:xml:ns:xmpp-bind' /></iq>",
+        '''
+'<iq xmlns="jabber:client" type="result" id="a"><bind xmlns="urn:ietf:params:xml:ns:xmpp-bind"><jid>polynomdivision@test.server/MU29eEZn</jid></bind></iq>',
+        ''',
+        adjustId: true,
+        ignoreId: true,
+      ),
+    ]);
+    final conn = XmppConnection(
+      TestingReconnectionPolicy(),
+      AlwaysConnectedConnectivityManager(),
+      fakeSocket,
+    )..setConnectionSettings(
+        ConnectionSettings(
+          jid: JID.fromString('user@server'),
+          password: 'pencil',
+          useDirectTLS: true,
+        ),
+      );
+    await conn.registerManagers([
+      PresenceManager(),
+      RosterManager(TestingRosterStateManager('', [])),
+      DiscoManager([]),
+    ]);
+    await conn.registerFeatureNegotiators([
+      SaslPlainNegotiator(),
+      SaslScramNegotiator(
+        10,
+        'n=user,r=rOprNGfwEbeRWgbNEkqO',
+        'rOprNGfwEbeRWgbNEkqO',
+        ScramHashType.sha256,
+      ),
+      ResourceBindingNegotiator(),
+      Sasl2Negotiator(
+        userAgent: const UserAgent(
+          id: 'd4565fa7-4d72-4749-b3d3-740edbf87770',
+          software: 'moxxmpp',
+          device: "PapaTutuWawa's awesome device",
+        ),
+      ),
+    ]);
+
     final result = await conn.connect(
       waitUntilLogin: true,
       shouldReconnect: false,
@@ -81,4 +167,84 @@ void main() {
     );
     expect(result.isType<XmppError>(), false);
   });
+
+  test('Test SCRAM-SHA-1 SASL2 negotiation with an invalid signature',
+      () async {
+    final fakeSocket = StubTCPSocket([
+      StringExpectation(
+        "<stream:stream xmlns='jabber:client' version='1.0' xmlns:stream='http://etherx.jabber.org/streams' to='server' xml:lang='en'>",
+        '''
+<stream:stream
+    xmlns="jabber:client"
+    version="1.0"
+    xmlns:stream="http://etherx.jabber.org/streams"
+    from="server"
+    xml:lang="en">
+  <stream:features xmlns="http://etherx.jabber.org/streams">
+    <mechanisms xmlns="urn:ietf:params:xml:ns:xmpp-sasl">
+      <mechanism>PLAIN</mechanism>
+      <mechanism>SCRAM-SHA-256</mechanism>
+    </mechanisms>
+    <authentication xmlns='urn:xmpp:sasl:2'>
+      <mechanism>PLAIN</mechanism>
+      <mechanism>SCRAM-SHA-256</mechanism>
+    </authentication>
+    <bind xmlns="urn:ietf:params:xml:ns:xmpp-bind">
+      <required/>
+    </bind>
+  </stream:features>''',
+      ),
+      StanzaExpectation(
+        "<authenticate xmlns='urn:xmpp:sasl:2' mechanism='SCRAM-SHA-256'><user-agent id='d4565fa7-4d72-4749-b3d3-740edbf87770'><software>moxxmpp</software><device>PapaTutuWawa's awesome device</device></user-agent><initial-response>biwsbj11c2VyLHI9ck9wck5HZndFYmVSV2diTkVrcU8=</initial-response></authenticate>",
+        '''
+<challenge xmlns='urn:xmpp:sasl:2'>cj1yT3ByTkdmd0ViZVJXZ2JORWtxTyVodllEcFdVYTJSYVRDQWZ1eEZJbGopaE5sRiRrMCxzPVcyMlphSjBTTlk3c29Fc1VFamI2Z1E9PSxpPTQwOTY=</challenge>
+        ''',
+      ),
+      StanzaExpectation(
+        '<response xmlns="urn:xmpp:sasl:2">Yz1iaXdzLHI9ck9wck5HZndFYmVSV2diTkVrcU8laHZZRHBXVWEyUmFUQ0FmdXhGSWxqKWhObEYkazAscD1kSHpiWmFwV0lrNGpVaE4rVXRlOXl0YWc5empmTUhnc3FtbWl6N0FuZFZRPQ==</response>',
+        '<success xmlns="urn:xmpp:sasl:2"><additional-data>dj1zbUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9</additional-data><authorization-identifier>user@server</authorization-identifier></success>',
+      ),
+    ]);
+    final conn = XmppConnection(
+      TestingReconnectionPolicy(),
+      AlwaysConnectedConnectivityManager(),
+      fakeSocket,
+    )..setConnectionSettings(
+        ConnectionSettings(
+          jid: JID.fromString('user@server'),
+          password: 'pencil',
+          useDirectTLS: true,
+        ),
+      );
+    await conn.registerManagers([
+      PresenceManager(),
+      RosterManager(TestingRosterStateManager('', [])),
+      DiscoManager([]),
+    ]);
+    await conn.registerFeatureNegotiators([
+      SaslPlainNegotiator(),
+      SaslScramNegotiator(
+        10,
+        'n=user,r=rOprNGfwEbeRWgbNEkqO',
+        'rOprNGfwEbeRWgbNEkqO',
+        ScramHashType.sha256,
+      ),
+      ResourceBindingNegotiator(),
+      Sasl2Negotiator(
+        userAgent: const UserAgent(
+          id: 'd4565fa7-4d72-4749-b3d3-740edbf87770',
+          software: 'moxxmpp',
+          device: "PapaTutuWawa's awesome device",
+        ),
+      ),
+    ]);
+
+    final result = await conn.connect(
+      waitUntilLogin: true,
+      shouldReconnect: false,
+      enableReconnectOnSuccess: false,
+    );
+    expect(result.isType<NegotiatorError>(), true);
+    expect(result.get<NegotiatorError>() is InvalidServerSignatureError, true);
+  });
 }