621283 - unicode passwords result in python traceback on sreg

Status: VERIFIED FIXED

(Cloud Services :: Server: Registration, defect)

Description

[:Atoll]

I accidentally entered some 8-bit characters into my password and ended up crashing sreg.

Steps to reproduce on OS X:

1. Start Minefield 2010-12-22.
2. Set up sync for the first time.
3. For your password, press the keystroke Option-8 at least 14 times.
4. Repeat the same in the second password dialog.
5. Complete the signup process.
BUG: The python server will put the below traceback into the logs:

Traceback (most recent call last):
  File "/usr/lib/python2.6/site-packages/services/util.py", line 402, in __call__
    return self.app(environ, start_response)
  File "/usr/lib/python2.6/site-packages/paste/translogger.py", line 68, in __call__
    return self.application(environ, replacement_start_response)
  File "/usr/lib/python2.6/site-packages/webob/dec.py", line 147, in __call__
    resp = self.call_func(req, *args, **self.kwargs)
  File "/usr/lib/python2.6/site-packages/webob/dec.py", line 208, in call_func
    return self.func(req, *args, **kwargs)
  File "/usr/lib/python2.6/site-packages/services/baseapp.py", line 160, in __call__
    result = function(request, **params)
  File "/usr/lib/python2.6/site-packages/syncreg/controllers/user.py", line 193, in create_user
    if not self.auth.create_user(user_name, password, email):
  File "/usr/lib/python2.6/site-packages/services/auth/ldapsql.py", line 208, in create_user
    password_hash = ssha(password)
  File "/usr/lib/python2.6/site-packages/services/util.py", line 171, in ssha
    ssha = base64.b64encode(sha1(password + salt).digest()
UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-13: ordinal not in range(128)

Comment 1

[:Atoll]

Verified this bug against the staging rpm versions below using Aurora 2011-04-28.

python26-services-1.2-1
python26-syncsreg-0.4-1
python26-cef-0.2-1

Current trackback with line numbers for the curious:

2011-05-23 04:26:27,001 ERROR [syncserver] Traceback (most recent call last):
  File "/usr/lib/python2.6/site-packages/services/util.py", line 452, in __call__
    return self.app(environ, start_response)
  File "/usr/lib/python2.6/site-packages/paste/translogger.py", line 68, in __call__
    return self.application(environ, replacement_start_response)
  File "/usr/lib/python2.6/site-packages/webob/dec.py", line 147, in __call__
    resp = self.call_func(req, *args, **self.kwargs)
  File "/usr/lib/python2.6/site-packages/webob/dec.py", line 208, in call_func
    return self.func(req, *args, **kwargs)
  File "/usr/lib/python2.6/site-packages/services/baseapp.py", line 206, in __call__
    result = function(request, **params)
  File "/usr/lib/python2.6/site-packages/syncsreg/controller.py", line 158, in create_user
    if not self.auth.create_user(username, password, email):
  File "/usr/lib/python2.6/site-packages/services/auth/ldapsql.py", line 222, in create_user
    password_hash = ssha(password)
  File "/usr/lib/python2.6/site-packages/services/util.py", line 195, in ssha
    ssha = base64.b64encode(sha1(password + salt).digest()
UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-7: ordinal not in range(128)

Comment 2

[Tarek Ziadé (:tarek)]

will look, thanks for the feedback

Comment 3

[Tarek Ziadé (:tarek)]

Attachment: Fix for non-ascii passwords

Comment 4

[Tarek Ziadé (:tarek)]

Attachment: extra functional test

Comment 5

[:Atoll]

Note for QA, we should test that existing passwords prior to this fix continue to work.

Comment 6

[Tarek Ziadé (:tarek)]

Being unable to register a user with unicode passwords is a regression, so I propose that we derail the Python reg/sreg push in production and have that fix included.

Note #2 to QA: all future tests involving passwords (like changing the password) should include a test with non ascii chars

Comment 7

[:Atoll]

Comment on attachment 534416 [details] [diff] [review]
Fix for non-ascii passwords

User API 1.0 does not specify that passwords must be valid UTF-8, but the above code and tests appear to imply such a requirement.  Please either add tests for passwords containing invalid UTF-8 characters in the range 0x80-0xFF, or implement user API 1.1 with the new UTF-8 restriction.

Comment 8

[:Atoll]

Comment on attachment 534416 [details] [diff] [review]
Fix for non-ascii passwords

turns out the spec declares UTF-8 at the top, and that's what php implements, so works for me

Comment 9

[Tony Chung [:tchung]]

(In reply to comment #6)
> Being unable to register a user with unicode passwords is a regression, so I
> propose that we derail the Python reg/sreg push in production and have that
> fix included.
> 
> Note #2 to QA: all future tests involving passwords (like changing the
> password) should include a test with non ascii chars

yikes.  thanks, this will need to be added to the regression test suite.

Comment 10

[Tarek Ziadé (:tarek)]

Pushed at :
- https://hg.mozilla.org/services/server-core/rev/f34cdfdc6937
- https://hg.mozilla.org/services/server-reg/rev/6c2f8769d717

For the next train a new tag on core should be enough,

Comment 11

[Tracy Walker [:tracy]]

(In reply to comment #5)
> Note for QA, we should test that existing passwords prior to this fix
> continue to work.

existing passwords work

However, attempting to change password to one with character option+8 at the end fails. log below


1306425120015	Net.Resource	TRACE	In _doRequest.
1306425120016	Net.Resource	TRACE	HTTP Header authorization: ***** (suppressed)
1306425120016	Net.Resource	DEBUG	POST Length: 11
1306425120016	Net.Resource	TRACE	POST Body: mozilla5•
1306425120409	Net.Resource	TRACE	onStartRequest called for channel [xpconnect wrapped nsIRequest].
1306425120410	Net.Resource	TRACE	onStartRequest: POST https://stage-auth.services.mozilla.com/user/1.0/nvifatgjze4lnj27xzphgvfugnewwhl5/password
1306425120410	Net.Resource	TRACE	Channel for POST https://stage-auth.services.mozilla.com/user/1.0/nvifatgjze4lnj27xzphgvfugnewwhl5/password: isSuccessCode(0)? true
1306425120410	Net.Resource	TRACE	Channel: flags = 640, URI = https://stage-auth.services.mozilla.com/user/1.0/nvifatgjze4lnj27xzphgvfugnewwhl5/password, HTTP success? false
1306425120410	Net.Resource	TRACE	In _onComplete. Error is null.
1306425120410	Net.Resource	TRACE	Channel: [xpconnect wrapped (nsISupports, nsIChannel, nsIRequest, nsIHttpChannel, nsIUploadChannel)]
1306425120410	Net.Resource	TRACE	Action: POST
1306425120410	Net.Resource	TRACE	Status: 500
1306425120410	Net.Resource	TRACE	Success: false
1306425120410	Net.Resource	DEBUG	mesg: POST fail 500 https://stage-auth.services.mozilla.com/user/1.0/nvifatgjze4lnj27xzphgvfugnewwhl5/password
1306425120410	Net.Resource	DEBUG	POST fail 500 https://stage-auth.services.mozilla.com/user/1.0/nvifatgjze4lnj27xzphgvfugnewwhl5/password
1306425120410	Net.Resource	TRACE	POST body: An unexpected error occurred
1306425120410	Net.Resource	TRACE	Processing response headers.
1306425120410	Service.Main	DEBUG	Password change failed: An unexpected error occurred

Comment 12

[Tarek Ziadé (:tarek)]

So it looks like it works via Account portal but the password cannot be changed via Firefox (with or without non-ascii char) So this looks like an issue on the reg server. investigating

Comment 13

[Tony Chung [:tchung]]

Verified fix on staging.  AP accepts and changes unicode and non-ascii passwords.  

stil unable to clear sync data, but thats bug 660089