Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XEP-0357 communication with app server fails #551

Closed
weiss opened this issue Aug 18, 2016 · 3 comments
Closed

XEP-0357 communication with app server fails #551

weiss opened this issue Aug 18, 2016 · 3 comments
Assignees

Comments

@weiss
Copy link

weiss commented Aug 18, 2016

We tested ChatSecure-iOS 3.2.3 with the XEP-0357 module for ejabberd I'm working on. Enabling notifications on the XMPP server works fine, but actual notifications do not (while they do with Conversations).

The XMPP server (jabber.fu-berlin.de) initiates an s2s connection with pubsub.chatsecure.org and sends the notification IQ stanza successfully. However, pubsub.chatsecure.org then silently closes the TCP connection (I don't see a connection attempt from your app server to deliver either an IQ-result or an IQ-error), and the app is not woken up.

This is an example of a notification generated by my server:

#2016-08-16 17:06:44 (from jabber.fu-berlin.de, 130.133.8.10)

<iq from='[email protected]' to='pubsub.chatsecure.org' type='set' id='15786152335197260825'>
  <pubsub xmlns='http://jabber.org/protocol/pubsub'>
    <publish node='9ED98833-2D5D-49FE-9F04-0438272F2E3C'>
      <item>
        <notification xmlns='urn:xmpp:push:0'>
          <x xmlns='jabber:x:data' type='submit'>
            <field var='message-count'><value>1</value></field>
            <field type='hidden' var='FORM_TYPE'><value>urn:xmpp:push:summary</value></field>
          </x>
        </notification>
      </item>
    </publish>
    <publish-options>
      <x xmlns='jabber:x:data' type='submit'>
        <field type='hidden' var='FORM_TYPE'><value>http://jabber.org/protocol/pubsub#publish-options</value></field>
        <field var='token'><value>ccabad[...]</value></field>
      </x>
    </publish-options>
  </pubsub>
</iq>

The same module is running on conversations.im, if you'd like to test this yourself.

Let me know if there's additional information I could provide, or contact me directly if you like ([email protected]).

@davidchiles
Copy link
Member

It looks like in this case there's not enough information in the stanza. It should have both a var='token' and var='endpoint' in the publish-options.

I'll make changes to RubDub to properly report this back as an error.

We also need to make sure we're including this information when we're registering for push notifications on the ChatSecure-iOS side.

@weiss
Copy link
Author

weiss commented Aug 19, 2016

It looks like in this case there's not enough information in the stanza. It should have both a var='token' and var='endpoint' in the publish-options.

D'oh! ChatSecure-iOS actually specified both while enabling notifications, the endpoint was missing from the notification due to a mod_push bug. Thanks, and sorry about that.

After fixing this, I get an IQ-result for each notification reliably. This sometimes but not always wakes the app for me (I don't see a pattern yet), but that's a separate issue.

I'll make changes to RubDub to properly report this back as an error.

Yes, that would be nice. I think RubDub should really respond to all IQ requests with either an IQ-result or an IQ-error (a generic error will be much better then none at all).

@weiss weiss closed this as completed Aug 19, 2016
@chrisballinger
Copy link
Member

I think the bigger problem is that this reveals potential DoS vectors when
processing unexpected input.

On Fri, Aug 19, 2016 at 7:16 AM, Holger Weiß [email protected]
wrote:

Closed #551 #551.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#551 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAfqH9ixvKLW9KHtbXfZLcpuMpZmDxCoks5qhbqjgaJpZM4JnnPu
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants