search

ChatSecure / ChatSecure-iOS

ChatSecure is a free and open source encrypted chat client for iOS that supports OTR and OMEMO encryption over XMPP.
https://chatsecure.org
Other
3.13k stars 1.03k forks source link

Push node id (device id) not stable across app usage #1022

Open tmolitor-stud-tu opened 6 years ago

tmolitor-stud-tu commented 6 years ago

A friend of mine installed the newest version of Chatsecure from iTunes today and after only a few hours I got 5 different devices registered for push on my private server out of a total of 17 push enable iqs. And no, she didn't reinstall the app at all and she didn't configure the account multiple times, too.

I can send you the IQs along with timestamps for debugging purposes but I don't want to post them here due to privacy reasons.

My guess is, that the device ID offered by apple is not stable at all (contrary to what is explained in their API documentation). This could be related to the roughly the same problem Monal has over here: https://github.com/tmolitor-stud-tu/mod_push_appserver/issues/6#issuecomment-391176730

tmolitor-stud-tu commented 6 years ago

Small addition: the token value used for registration looks like a python3 bytestring casted to string without proper encoding/decoding (the value was changed due to privacy reasons): <value>b'a500a8519aca3cf94877bab0f340aa2205f5a840'</value>

chrisballinger commented 6 years ago

Thanks for the report! Looks like some issues slipped through during the Python 3 migration...

chrisballinger commented 6 years ago

@tmolitor-stud-tu Alright, the Python 3 string issue is now fixed on production. My guess is that it didn't really affect much because the tokens are just opaque strings anyway.

The tokens your XMPP server sees are not APNS tokens, but random tokens generated by the ChatSecure Push API server. They expire after 30 days so they must be rotated over time, but you shouldn't be seeing such a large number of registrations unless there is a bug in the client refresh logic.

tmolitor-stud-tu commented 6 years ago

@chrisballinger Well, a token rotation shouldn't change the node id, am I right? And only a changed node ID will register a new device in prosody's push implementation (and in ejabberd, too, I guess)

tmolitor-stud-tu commented 6 years ago

Much of the load on your push server is probably coming from this double and triple registrations.

tmolitor-stud-tu commented 6 years ago

I sent you my debug log (the xml sent by ChatSecure) via mail. Maybe it helps tackling down this issue.

chrisballinger commented 6 years ago

Thanks, I'll take a look

tmolitor-stud-tu commented 6 years ago

Did you manage to take a look at my mail? If you need some help or more information go ahead and just ask for it. I'm happy to help if I can :)

licaon-kter commented 6 years ago

ejabberd spit this out today:

[error] <0.320.0>@ejabberd_sql:check_error:1138 SQL query 'Q21699820' at {mod_push_sql,59} failed: [{severity,'ERROR'},{86,<<"ERROR">>},{code,<<"23505">>},{message,<<"duplicate key value violates unique constraint \"push_session_pkey\"">>},{detail,<<"Key (server_host, username, \"timestamp\")=(domain.tld, username, 1540741783202646) already exists.">>},{115,<<"public">>},{116,<<"push_session">>},{110,<<"push_session_pkey">>},{file,<<"nbtinsert.c">>},{line,433},{routine,<<"_bt_check_unique">>}]
[error] <0.6513.0>@mod_push:enable:312 Cannot enable push for username@domain.tld/chatsecure28173: database error

ChatSecure 4.3.5, newly installed app on new device, existing account

/LE: not sure if this is a sign: https://github.com/ChatSecure/ChatSecure-iOS/issues/883#issuecomment-433729470