Bug 16647 - Incorrect credentials can lead to thousands of connections attempts
: Incorrect credentials can lead to thousands of connections attempts
Status: RESOLVED FIXED
Product: MySqueezebox.com
Classification: Unclassified
Component: WiMP
: MySB
: PC Other
: P1 major (vote)
: Hotfix
Assigned To: Michael Herger
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-05 01:39 UTC by Michael Herger
Modified: 2011-11-07 08:43 UTC (History)
4 users (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Herger 2010-11-05 01:39:18 UTC
WiMP has reported several cases where a user's SB would have hit their servers thousands of times trying to connect with incorrect credentials:

From Ruben Chadien:

I have been looking in the logs for the last two "login loops". It is actually only login attempts, the password doesn't match.
So this leads me to think that the user changed his password, this can be done on our web site, maybe while the Squeezebox was playing or
at least logged in. I also see that the user is switching between the desktop app and squeezebox a lot, causing them to log out each other , and in one case the desktop app is playing while the Squeezebox is in the login loop.
Comment 1 Michael Herger 2010-11-05 02:06:00 UTC
Unfortunately I can't reproduce this issue: if I manipulate my password in the DB, I'll get a UserLoggedOutException and the player would stop. Do you see whether some other request was done before the failed login attempt?
Comment 2 Michael Herger 2010-11-05 02:08:23 UTC
Oh, actually I'm not getting the UserLoggedOutException, that's a follow up issue due to the failed authentication:

Calling WiMP:{
  args      => [
                 "41794075018",
                 "]s3\x83cCs",
                 49,
                 "Squeezebox",
                 "00:04:20:22:01:0c",
               ],
  dontretry => 0,
  method    => "login",
  player    => "00:04:20:22:01:0c",
  sid       => "nosid",
  user      => "41794075018",
} at /Users/mh/Documents/workspace/SNqa/script/../lib/Net/WiMP.pm line 570.
Error calling 'login' method: "Can't use string (\"0\") as a SCALAR ref while \"strict refs\" in use at /Users/mh/Documents/workspace/SNqa/script/../lib/Thrift/BinaryProtocol.pm line 376.\n" at /Users/mh/Documents/workspace/SNqa/script/../lib/Net/WiMP.pm line 615.
Error calling 'getTrack' method: "UserLoggedOutException - 41794075018" at /Users/mh/Documents/workspace/SNqa/script/../lib/Net/WiMP.pm line 615.


Should I see something more explicit than the "Can't use string (\"0\") as a SCALAR" when things fail? I'm seeing it whenever I eg. send a wrong parameter or as in this case an invalid password. It's hard to trigger some action based on it to tell the user to check his password, as it's too generic.
Comment 3 Ruben Chadien 2010-11-05 04:22:07 UTC
Oh, looks like we have a bug in the login command, it should have returned a NO_RESULT status.
Unfortunately we can´t change this right now because of other clients accepting this as login fault.

We are in the final stages of deploying a new API for partners, it would be great if we could update the Squeezebox to use this. This should not be a to big job, we have converted our Android app and are preparing a migration guide. Also there is documentation for the new API...

But in the meantime it should be possible to stop the loop ? Maybe just react to this error in the login as an "Wrong username or password" and stop trying to login.
Comment 4 Michael Herger 2011-01-13 02:34:52 UTC
Ruben - can we consider this issue fixed?
Comment 5 Nils Juell 2011-01-13 02:43:34 UTC
Hello Michael

Ruben is on paternity leave and only sporadically on mail.

Unfortunately the login loop from "Squeezeboxes gone wild" is still very much alive. In the last 48 hrs we've had a continuous period where there have been 5000 logins pr / 10 minutes from a few SB'es.

Our system can handle it fine (fortunately), but it's of ocourse not optimal.

Hopefully Remco and you will be able to schedule a transition to the new API (received from Kenneth a few days ago I hope ?), and in this process we can see if login loops like the ones we're seeing now can be eliminated.
Comment 6 Michael Herger 2011-01-13 03:04:52 UTC
Ok, let's review this case when I start working on the API transition (can't give any date yet - just had a call with Remco about it).
Comment 7 Michael Herger 2011-10-25 02:50:28 UTC
We need to address this soon.
Comment 8 Michael Herger 2011-10-25 21:58:51 UTC
Ok, here's my latest finding: we do not repeat the login attempt more than once _per_ _request_. This usually would result in the server giving up quickly with a UserLoggedOutException for eg. navigation requests.

But if I have queued up a bunch of WIMP tracks, then change my credentials and re-connect my player, the server would try to request metadata (getTrack) for every single track. Unfortunately SBS would not recognize the failure and request the same metadata over and over again.
Comment 9 SVN Bot 2011-10-25 23:51:55 UTC
 == Auto-comment from SVN commit #11061 to the network repo by mherger ==
 == http://svn.slimdevices.com/network?view=revision&revision=11061 ==

Bug: 16647
Description: improve behaviour with invalid credentials (eg. user has changed password on WiMP web page, but not mysb.com)
- don't run actual request if login fails
- do not return empty data to getBulkMetadata caller to prevent repeated requests. Return error message instead.
Comment 10 SVN Bot 2011-10-26 00:51:39 UTC
 == Auto-comment from SVN commit #11062 to the network repo by mherger ==
 == http://svn.slimdevices.com/network?view=revision&revision=11062 ==

Bug: 16647
Description: improve behaviour with invalid credentials (eg. user has changed password on WiMP web page, but not mysb.com)
- return error message when requesting stream url
Comment 11 SVN Bot 2011-10-26 04:50:59 UTC
 == Auto-comment from SVN commit #11063 to the network repo by mherger ==
 == http://svn.slimdevices.com/network?view=revision&revision=11063 ==

Bug: 16647
Description: improve behaviour with invalid credentials. Always set an error message.
Comment 12 SVN Bot 2011-10-26 05:58:34 UTC
 == Auto-comment from SVN commit #33622 to the slim repo by mherger ==
 == http://svn.slimdevices.com/slim?view=revision&revision=33622 ==

Bug: 16647
Description: only cache error messages for a few minutes, giving the user the chance to fix the issue and not have to wait 24h before the cached response expires...
Comment 13 SVN Bot 2011-10-27 03:49:38 UTC
 == Auto-comment from SVN commit #11067 to the network repo by mherger ==
 == http://svn.slimdevices.com/network?view=revision&revision=11067 ==

Bug: 16647
Description: port fixes to production system to be rolled out with next update.