From f86dbe6af88658415c0c3fa02cfc240d2fe7a0d9 Mon Sep 17 00:00:00 2001 From: "Alexander \"PapaTutuWawa" Date: Fri, 31 Mar 2023 23:52:48 +0200 Subject: [PATCH] feat(core): Verify the server signature with SASL2 --- .../lib/src/negotiators/negotiator.dart | 3 +- .../lib/src/negotiators/sasl/plain.dart | 3 +- .../lib/src/negotiators/sasl/scram.dart | 44 ++++- .../moxxmpp/lib/src/negotiators/sasl2.dart | 16 +- packages/moxxmpp/test/xeps/xep_0388_test.dart | 166 ++++++++++++++++++ 5 files changed, 222 insertions(+), 10 deletions(-) diff --git a/packages/moxxmpp/lib/src/negotiators/negotiator.dart b/packages/moxxmpp/lib/src/negotiators/negotiator.dart index 50cc220..d60c4a6 100644 --- a/packages/moxxmpp/lib/src/negotiators/negotiator.dart +++ b/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. - @visibleForOverriding + /// other negotiators. By default this function does nothing. Future postRegisterCallback() async {} } diff --git a/packages/moxxmpp/lib/src/negotiators/sasl/plain.dart b/packages/moxxmpp/lib/src/negotiators/sasl/plain.dart index 17a54de..d3e02c2 100644 --- a/packages/moxxmpp/lib/src/negotiators/sasl/plain.dart +++ b/packages/moxxmpp/lib/src/negotiators/sasl/plain.dart @@ -93,8 +93,9 @@ class SaslPlainNegotiator extends Sasl2AuthenticationNegotiator { } @override - Future onSasl2Success(XMLNode response) async { + Future> onSasl2Success(XMLNode response) async { state = NegotiatorState.done; + return const Result(true); } @override diff --git a/packages/moxxmpp/lib/src/negotiators/sasl/scram.dart b/packages/moxxmpp/lib/src/negotiators/sasl/scram.dart index 1273366..360ebf7 100644 --- a/packages/moxxmpp/lib/src/negotiators/sasl/scram.dart +++ b/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> 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 - final signature = - parseKeyValue(utf8.decode(base64.decode(nonza.innerText()))); - if (signature['v']! != _serverSignature) { + if (!_checkSignature(nonza.innerText())) { // 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 postRegisterCallback() async { + attributes + .getNegotiatorById(sasl2Negotiator) + ?.registerSaslNegotiator(this); + } + @override Future> onSasl2FeaturesReceived(XMLNode sasl2Features) async { return []; } @override - Future onSasl2Success(XMLNode response) async { + Future> 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); } } diff --git a/packages/moxxmpp/lib/src/negotiators/sasl2.dart b/packages/moxxmpp/lib/src/negotiators/sasl2.dart index 861b33c..fdd2139 100644 --- a/packages/moxxmpp/lib/src/negotiators/sasl2.dart +++ b/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 onSasl2Success(XMLNode response); + Future> 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()) { + return Result(result.get()); + } } // 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); } } diff --git a/packages/moxxmpp/test/xeps/xep_0388_test.dart b/packages/moxxmpp/test/xeps/xep_0388_test.dart index 8cd96e6..544343c 100644 --- a/packages/moxxmpp/test/xeps/xep_0388_test.dart +++ b/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(), false); + }); + + test('Test SCRAM-SHA-1 SASL2 negotiation with a valid signature', () async { + final fakeSocket = StubTCPSocket([ + StringExpectation( + "", + ''' + + + + PLAIN + SCRAM-SHA-256 + + + PLAIN + SCRAM-SHA-256 + + + + + ''', + ), + StanzaExpectation( + "moxxmppPapaTutuWawa's awesome devicebiwsbj11c2VyLHI9ck9wck5HZndFYmVSV2diTkVrcU8=", + ''' +cj1yT3ByTkdmd0ViZVJXZ2JORWtxTyVodllEcFdVYTJSYVRDQWZ1eEZJbGopaE5sRiRrMCxzPVcyMlphSjBTTlk3c29Fc1VFamI2Z1E9PSxpPTQwOTY= + ''', + ), + StanzaExpectation( + 'Yz1iaXdzLHI9ck9wck5HZndFYmVSV2diTkVrcU8laHZZRHBXVWEyUmFUQ0FmdXhGSWxqKWhObEYkazAscD1kSHpiWmFwV0lrNGpVaE4rVXRlOXl0YWc5empmTUhnc3FtbWl6N0FuZFZRPQ==', + 'dj02cnJpVFJCaTIzV3BSUi93dHVwK21NaFVaVW4vZEI1bkxUSlJzamw5NUc0PQ==user@server', + ), + StanzaExpectation( + "", + ''' +'polynomdivision@test.server/MU29eEZn', + ''', + 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(), false); }); + + test('Test SCRAM-SHA-1 SASL2 negotiation with an invalid signature', + () async { + final fakeSocket = StubTCPSocket([ + StringExpectation( + "", + ''' + + + + PLAIN + SCRAM-SHA-256 + + + PLAIN + SCRAM-SHA-256 + + + + + ''', + ), + StanzaExpectation( + "moxxmppPapaTutuWawa's awesome devicebiwsbj11c2VyLHI9ck9wck5HZndFYmVSV2diTkVrcU8=", + ''' +cj1yT3ByTkdmd0ViZVJXZ2JORWtxTyVodllEcFdVYTJSYVRDQWZ1eEZJbGopaE5sRiRrMCxzPVcyMlphSjBTTlk3c29Fc1VFamI2Z1E9PSxpPTQwOTY= + ''', + ), + StanzaExpectation( + 'Yz1iaXdzLHI9ck9wck5HZndFYmVSV2diTkVrcU8laHZZRHBXVWEyUmFUQ0FmdXhGSWxqKWhObEYkazAscD1kSHpiWmFwV0lrNGpVaE4rVXRlOXl0YWc5empmTUhnc3FtbWl6N0FuZFZRPQ==', + 'dj1zbUY5cHFWOFM3c3VBb1pXamE0ZEpSa0ZzS1E9user@server', + ), + ]); + 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(), true); + expect(result.get() is InvalidServerSignatureError, true); + }); }