From 51edb614433dddf0fe782f1d848d42ddfc2cc9af Mon Sep 17 00:00:00 2001 From: "Alexander \"PapaTutuWawa" Date: Sat, 1 Apr 2023 15:50:13 +0200 Subject: [PATCH] feat(xep): Implement SASL2 inline stream resumption --- packages/moxxmpp/lib/src/connection.dart | 9 +- .../moxxmpp/lib/src/negotiators/sasl2.dart | 61 +++---- .../lib/src/xeps/xep_0198/negotiator.dart | 154 ++++++++++++++---- packages/moxxmpp/lib/src/xeps/xep_0386.dart | 9 + packages/moxxmpp/test/xeps/xep_0198_test.dart | 94 +++++++++++ packages/moxxmpp/test/xeps/xep_0386_test.dart | 3 +- packages/moxxmpp/test/xeps/xep_0388_test.dart | 9 + 7 files changed, 271 insertions(+), 68 deletions(-) diff --git a/packages/moxxmpp/lib/src/connection.dart b/packages/moxxmpp/lib/src/connection.dart index a517826..c092c57 100644 --- a/packages/moxxmpp/lib/src/connection.dart +++ b/packages/moxxmpp/lib/src/connection.dart @@ -279,7 +279,7 @@ class XmppConnection { () => _socket, () => _isAuthenticated, _setAuthenticated, - _setResource, + setResource, _removeNegotiatingFeature, ), ); @@ -675,7 +675,8 @@ class XmppConnection { } /// Sets the resource of the connection - void _setResource(String resource, {bool triggerEvent = true}) { + @visibleForTesting + void setResource(String resource, {bool triggerEvent = true}) { _log.finest('Updating _resource to $resource'); _resource = resource; @@ -1134,9 +1135,7 @@ class XmppConnection { } if (lastResource != null) { - _setResource(lastResource, triggerEvent: false); - } else { - _setResource('', triggerEvent: false); + setResource(lastResource, triggerEvent: false); } _enableReconnectOnSuccess = enableReconnectOnSuccess; diff --git a/packages/moxxmpp/lib/src/negotiators/sasl2.dart b/packages/moxxmpp/lib/src/negotiators/sasl2.dart index 5a4dc6f..bfb186e 100644 --- a/packages/moxxmpp/lib/src/negotiators/sasl2.dart +++ b/packages/moxxmpp/lib/src/negotiators/sasl2.dart @@ -1,4 +1,3 @@ -import 'package:collection/collection.dart'; import 'package:moxxmpp/src/jid.dart'; import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; @@ -28,6 +27,11 @@ abstract class Sasl2FeatureNegotiator extends XmppFeatureNegotiatorBase { /// This method is only called when the previous element contains an /// item with xmlns equal to [negotiatingXmlns]. Future> onSasl2Success(XMLNode response); + + /// Called by the SASL2 negotiator to find out whether the negotiator is willing + /// to inline a feature. [features] is the list of elements inside the + /// element. + bool canInlineFeature(List features); } /// A special type of [SaslNegotiator] that is aware of SASL2. @@ -38,6 +42,11 @@ abstract class Sasl2AuthenticationNegotiator extends SaslNegotiator /// Perform a SASL step with [input] as the already parsed input data. Returns /// the base64-encoded response data. Future getRawStep(String input); + + @override + bool canInlineFeature(List features) { + return true; + } } class NoSASLMechanismSelectedError extends NegotiatorError { @@ -125,6 +134,8 @@ class Sasl2Negotiator extends XmppFeatureNegotiatorBase { /// The SASL2 element we received with the stream features. XMLNode? _sasl2Data; + final List _activeSasl2Negotiators = + List.empty(growable: true); /// Register a SASL negotiator so that we can use that SASL implementation during /// SASL2. @@ -141,18 +152,6 @@ class Sasl2Negotiator extends XmppFeatureNegotiatorBase { _featureNegotiators.add(negotiator); } - /// Returns true, if an item with xmlns of [xmlns] is contained inside [_sasl2Data]'s - /// block. If not, returns false. - bool _isInliningPossible(String xmlns) { - final inline = _sasl2Data!.firstTag('inline'); - if (inline == null) { - return false; - } - - return inline.children.firstWhereOrNull((child) => child.xmlns == xmlns) != - null; - } - @override bool matchesFeature(List features) { // Only do SASL2 when the socket is secure @@ -185,12 +184,16 @@ class Sasl2Negotiator extends XmppFeatureNegotiatorBase { } // Collect additional data by interested negotiators + final inline = _sasl2Data!.firstTag('inline'); final children = List.empty(growable: true); - for (final negotiator in _featureNegotiators) { - if (_isInliningPossible(negotiator.negotiatingXmlns)) { - children.addAll( - await negotiator.onSasl2FeaturesReceived(_sasl2Data!), - ); + if (inline != null && inline.children.isNotEmpty) { + for (final negotiator in _featureNegotiators) { + if (negotiator.canInlineFeature(inline.children)) { + _activeSasl2Negotiators.add(negotiator.id); + children.addAll( + await negotiator.onSasl2FeaturesReceived(_sasl2Data!), + ); + } } } @@ -217,19 +220,18 @@ class Sasl2Negotiator extends XmppFeatureNegotiatorBase { case Sasl2State.authenticateSent: if (nonza.tag == 'success') { // Tell the dependent negotiators about the result - // TODO(Unknown): This can be written in a better way - for (final negotiator in _featureNegotiators) { - if (_isInliningPossible(negotiator.negotiatingXmlns)) { - final result = await negotiator.onSasl2Success(nonza); - if (!result.isType()) { - return Result(result.get()); - } + final negotiators = _featureNegotiators + .where( + (negotiator) => _activeSasl2Negotiators.contains(negotiator.id), + ) + .toList() + ..add(_currentSaslNegotiator!); + for (final negotiator in negotiators) { + final result = await negotiator.onSasl2Success(nonza); + if (!result.isType()) { + return Result(result.get()); } } - final result = await _currentSaslNegotiator!.onSasl2Success(nonza); - if (!result.isType()) { - return Result(result.get()); - } // We're done attributes.setAuthenticated(); @@ -264,6 +266,7 @@ class Sasl2Negotiator extends XmppFeatureNegotiatorBase { _currentSaslNegotiator = null; _sasl2State = Sasl2State.idle; _sasl2Data = null; + _activeSasl2Negotiators.clear(); super.reset(); } diff --git a/packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart b/packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart index 274f345..7e007f1 100644 --- a/packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart +++ b/packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart @@ -1,9 +1,11 @@ +import 'package:collection/collection.dart'; import 'package:logging/logging.dart'; import 'package:moxxmpp/src/events.dart'; import 'package:moxxmpp/src/managers/namespaces.dart'; import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; import 'package:moxxmpp/src/negotiators/negotiator.dart'; +import 'package:moxxmpp/src/negotiators/sasl2.dart'; import 'package:moxxmpp/src/stringxml.dart'; import 'package:moxxmpp/src/types/result.dart'; import 'package:moxxmpp/src/xeps/xep_0198/nonzas.dart'; @@ -23,27 +25,51 @@ enum _StreamManagementNegotiatorState { /// NOTE: The stream management negotiator requires that loadState has been called on the /// StreamManagementManager at least once before connecting, if stream resumption /// is wanted. -class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { +class StreamManagementNegotiator extends Sasl2FeatureNegotiator { StreamManagementNegotiator() - : _state = _StreamManagementNegotiatorState.ready, - _supported = false, - _resumeFailed = false, - _isResumed = false, - _log = Logger('StreamManagementNegotiator'), - super(10, false, smXmlns, streamManagementNegotiator); - _StreamManagementNegotiatorState _state; - bool _resumeFailed; - bool _isResumed; + : super(10, false, smXmlns, streamManagementNegotiator); - final Logger _log; + /// Stream Management negotiation state. + _StreamManagementNegotiatorState _state = + _StreamManagementNegotiatorState.ready; + + /// Flag indicating whether the resume failed (true) or succeeded (false). + bool _resumeFailed = false; + + /// Flag indicating whether the current stream is resumed (true) or not (false). + bool _isResumed = false; + + /// Logger + final Logger _log = Logger('StreamManagementNegotiator'); /// True if Stream Management is supported on this stream. - bool _supported; + bool _supported = false; bool get isSupported => _supported; /// True if the current stream is resumed. False if not. bool get isResumed => _isResumed; + @override + bool canInlineFeature(List features) { + final sm = attributes.getManagerById(smManager)!; + + // We do not check here for authentication as enabling/resuming happens inline + // with the authentication. + if (sm.state.streamResumptionId != null && !_resumeFailed) { + // We can try to resume the stream or enable the stream + return features.firstWhereOrNull( + (child) => child.xmlns == smXmlns, + ) != + null; + } else { + // We can try to enable SM + return features.firstWhereOrNull( + (child) => child.tag == 'enable' && child.xmlns == smXmlns, + ) != + null; + } + } + @override bool matchesFeature(List features) { final sm = attributes.getManagerById(smManager)!; @@ -53,13 +79,37 @@ class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { return super.matchesFeature(features) && attributes.isAuthenticated(); } else { // We cannot do a stream resumption - final br = attributes.getNegotiatorById(resourceBindingNegotiator); return super.matchesFeature(features) && - br?.state == NegotiatorState.done && + attributes.getConnection().resource.isNotEmpty && attributes.isAuthenticated(); } } + Future _onStreamResumptionFailed() async { + await attributes.sendEvent(StreamResumeFailedEvent()); + final sm = attributes.getManagerById(smManager)!; + + // We have to do this because we otherwise get a stanza stuck in the queue, + // thus spamming the server on every nonza we receive. + // ignore: cascade_invocations + await sm.setState(StreamManagementState(0, 0)); + await sm.commitState(); + + _resumeFailed = true; + _isResumed = false; + _state = _StreamManagementNegotiatorState.ready; + } + + Future _onStreamResumptionSuccessful(XMLNode resumed) async { + assert(resumed.tag == 'resumed', 'The correct element must be passed'); + + final h = int.parse(resumed.attributes['h']! as String); + await attributes.sendEvent(StreamResumedEvent(h: h)); + + _resumeFailed = false; + _isResumed = true; + } + @override Future> negotiate( XMLNode nonza, @@ -103,30 +153,14 @@ class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { csi.restoreCSIState(); } - final h = int.parse(nonza.attributes['h']! as String); - await attributes.sendEvent(StreamResumedEvent(h: h)); - - _resumeFailed = false; - _isResumed = true; + await _onStreamResumptionSuccessful(nonza); return const Result(NegotiatorState.skipRest); } else { // We assume it is _log.info( 'Stream resumption failed. Expected , got ${nonza.tag}, Proceeding with new stream...', ); - await attributes.sendEvent(StreamResumeFailedEvent()); - final sm = - attributes.getManagerById(smManager)!; - - // We have to do this because we otherwise get a stanza stuck in the queue, - // thus spamming the server on every nonza we receive. - // ignore: cascade_invocations - await sm.setState(StreamManagementState(0, 0)); - await sm.commitState(); - - _resumeFailed = true; - _isResumed = false; - _state = _StreamManagementNegotiatorState.ready; + await _onStreamResumptionFailed(); return const Result(NegotiatorState.retryLater); } case _StreamManagementNegotiatorState.enableRequested: @@ -165,4 +199,60 @@ class StreamManagementNegotiator extends XmppFeatureNegotiatorBase { super.reset(); } + + @override + Future> onSasl2FeaturesReceived(XMLNode sasl2Features) async { + final inline = sasl2Features.firstTag('inline')!; + final resume = inline.firstTag('resume', xmlns: smXmlns); + + if (resume == null) { + return []; + } + + final sm = attributes.getManagerById(smManager)!; + final srid = sm.state.streamResumptionId; + final h = sm.state.s2c; + if (srid == null) { + _log.finest('No srid'); + return []; + } + + return [ + XMLNode.xmlns( + tag: 'resume', + xmlns: smXmlns, + attributes: { + 'h': h.toString(), + 'previd': srid, + }, + ), + ]; + } + + @override + Future> onSasl2Success(XMLNode response) async { + final resumed = response.firstTag('resumed', xmlns: smXmlns); + if (resumed == null) { + _log.warning('Inline stream resumption failed'); + await _onStreamResumptionFailed(); + state = NegotiatorState.retryLater; + return const Result(true); + } + + _log.finest('Inline stream resumption successful'); + await _onStreamResumptionSuccessful(resumed); + state = NegotiatorState.skipRest; + + attributes.removeNegotiatingFeature(smXmlns); + attributes.removeNegotiatingFeature(bindXmlns); + + return const Result(true); + } + + @override + Future postRegisterCallback() async { + attributes + .getNegotiatorById(sasl2Negotiator) + ?.registerNegotiator(this); + } } diff --git a/packages/moxxmpp/lib/src/xeps/xep_0386.dart b/packages/moxxmpp/lib/src/xeps/xep_0386.dart index 033a82f..25deb7a 100644 --- a/packages/moxxmpp/lib/src/xeps/xep_0386.dart +++ b/packages/moxxmpp/lib/src/xeps/xep_0386.dart @@ -1,3 +1,4 @@ +import 'package:collection/collection.dart'; import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; import 'package:moxxmpp/src/negotiators/negotiator.dart'; @@ -37,6 +38,14 @@ class Bind2Negotiator extends Sasl2FeatureNegotiator { ]; } + @override + bool canInlineFeature(List features) { + return features.firstWhereOrNull( + (child) => child.tag == 'bind' && child.xmlns == bind2Xmlns, + ) != + null; + } + @override Future> onSasl2Success(XMLNode response) async { attributes.removeNegotiatingFeature(bindXmlns); diff --git a/packages/moxxmpp/test/xeps/xep_0198_test.dart b/packages/moxxmpp/test/xeps/xep_0198_test.dart index 6c1dc73..a11987d 100644 --- a/packages/moxxmpp/test/xeps/xep_0198_test.dart +++ b/packages/moxxmpp/test/xeps/xep_0198_test.dart @@ -788,4 +788,98 @@ void main() { expect(sm.streamResumed, true); }); }); + + test('Test SASL2 inline stream resumption', () async { + final fakeSocket = StubTCPSocket([ + StringExpectation( + "", + ''' + + + + PLAIN + + + PLAIN + + + + + + + + ''', + ), + StanzaExpectation( + "moxxmppPapaTutuWawa's awesome deviceAHBvbHlub21kaXZpc2lvbgBhYWFh", + ''' + + polynomdivision@test.server + + + ''', + ), + ]); + final sm = StreamManagementManager(); + await sm.setState( + sm.state.copyWith( + c2s: 25, + s2c: 2, + streamResumptionId: 'test-prev-id', + ), + ); + + final conn = XmppConnection( + TestingReconnectionPolicy(), + AlwaysConnectedConnectivityManager(), + fakeSocket, + ) + ..setConnectionSettings( + ConnectionSettings( + jid: JID.fromString('polynomdivision@test.server'), + password: 'aaaa', + useDirectTLS: true, + ), + ) + ..setResource('test-resource', triggerEvent: false); + await conn.registerManagers([ + RosterManager(TestingRosterStateManager('', [])), + DiscoManager([]), + sm, + ]); + await conn.registerFeatureNegotiators([ + SaslPlainNegotiator(), + ResourceBindingNegotiator(), + StreamManagementNegotiator(), + 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(), false); + + expect( + sm.state.c2s, + 25, + ); + expect( + sm.state.s2c, + 2, + ); + expect(conn.resource, 'test-resource'); + }); } diff --git a/packages/moxxmpp/test/xeps/xep_0386_test.dart b/packages/moxxmpp/test/xeps/xep_0386_test.dart index b7b5864..ebb70fa 100644 --- a/packages/moxxmpp/test/xeps/xep_0386_test.dart +++ b/packages/moxxmpp/test/xeps/xep_0386_test.dart @@ -132,8 +132,7 @@ void main() { await conn.registerFeatureNegotiators([ SaslPlainNegotiator(), ResourceBindingNegotiator(), - Bind2Negotiator() - ..tag = 'moxxmpp', + Bind2Negotiator()..tag = 'moxxmpp', Sasl2Negotiator( userAgent: const UserAgent( id: 'd4565fa7-4d72-4749-b3d3-740edbf87770', diff --git a/packages/moxxmpp/test/xeps/xep_0388_test.dart b/packages/moxxmpp/test/xeps/xep_0388_test.dart index 0941e8c..1973e45 100644 --- a/packages/moxxmpp/test/xeps/xep_0388_test.dart +++ b/packages/moxxmpp/test/xeps/xep_0388_test.dart @@ -1,3 +1,4 @@ +import 'package:collection/collection.dart'; import 'package:moxxmpp/moxxmpp.dart'; import 'package:test/test.dart'; import '../helpers/logging.dart'; @@ -16,6 +17,14 @@ class ExampleNegotiator extends Sasl2FeatureNegotiator { return const Result(NegotiatorState.done); } + @override + bool canInlineFeature(List features) { + return features.firstWhereOrNull( + (child) => child.xmlns == 'invalid:example:dont:use', + ) != + null; + } + @override Future postRegisterCallback() async { attributes