Main | My View | View Issues | Change Log | Wiki | Tavern | News
  

Viewing Issue Simple Details [ Jump to Notes ] [ Wiki ] [ View Advanced ] [ Issue History ] [ Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0008607 [DCSS] Bug Report major always 2014-05-28 03:56 2014-07-21 01:54
Reporter nrook View Status public  
Assigned To edlothiol
Priority normal Resolution no change required  
Status new   Product Branch 0.15 ancient branch
Summary 0008607: Users with password 'a' cannot login if crypt_algorithm is "broken"
Description If crypt_algorithm is "broken", the password itself is chosen as a salt. The crypted password is then calculated as:

    crypted_pw = crypt.crypt(passwd, salt)

However, for example, crypt.crypt('anything', 'a') returns None. This causes nil to be written to the sqlite database. Nil goes in, nil comes out:

2014-05-27 18:49:58,664 WARN: #1 Error while handling JSON message!
Traceback (most recent call last):
  File "/home/nrook/code/crawl/crawl/crawl-ref/source/webserver/ws_handler.py", line 548, in on_message
    handler(**obj)
  File "/home/nrook/code/crawl/crawl/crawl-ref/source/webserver/ws_handler.py", line 386, in login
    real_username = user_passwd_match(username, password)
  File "/home/nrook/code/crawl/crawl/crawl-ref/source/webserver/userdb.py", line 26, in user_passwd_match
    elif crypt.crypt(passwd, result[1]) == result[1]:
TypeError: must be string, not None

There are three bugs here. The first comes from the fact that crypt.crypt returns None if the supplied salt is illegal:

http://stackoverflow.com/questions/19127535/python-crypt-import-linux-salt [^]

In order to protect against bogus statuses, register_user() should return an error message rather than serializing None if crypt.crypt returns None.

The second comes from user_passwd_match() handling the case where the serialized password is None poorly. The correct fix here, I think, is to allow such users to login with the empty string. This would allow them, if their situation is discovered, to rectify it with a password change.

And the third, of course, is that an invalid salt is given to crypt.crypt at all. config.py claims "broken" is how it is due to "compatibility with dgamelaunch", so any fix would have to look at how dgl does it, I guess.

I would be willing to implement these fixes if you like.
Additional Information
Tags No tags attached.
Attached Files

- Relationships

-  Notes
(0026299)
edlothiol (developer)
2014-05-28 11:57

 1. It'd be good to check if "broken" is still in use on the servers at all. At least CSZO uses "6"; I can't check directly on the other servers. I'd guess that CAO and CBRO also use "6" since they use the same basic architecture as CSZO; I don't know about CLAN. So it might be better to just remove "broken", since it's, you know, broken.
Anyway, the relevant DGL source code is here: https://github.com/greensnark/dgamelaunch-crawl/blob/master/dgamelaunch.c#L1328 [^] I have no idea how that will deal with 1-letter passwords.

2. If you do any big changes to Webtiles, please do it on the webtiles-changes branch; otherwise they might need complete rewriting when that lands. Especially if you're looking at it for the chat thing. (I can also implement the chat page myself, but only in a few weeks.)
(0026301)
edlothiol (developer)
2014-05-28 22:18

 OK, after talking to |amethyst, I've removed the "broken" algorithm and made "6" the default.
(0026788)
nrook (updater)
2014-07-20 20:57

 This still looks broken:

http://s-z.org/neil/git/?p=crawl.git;a=blob;f=crawl-ref/source/webserver/config.py;h=0f149e26b3f8359136da918ce574747567e1c1f8;hb=HEAD [^]

(ctrl+f broken)
(0026795)
neil (administrator)
2014-07-21 01:54

 The branch in question hasn't been merged into trunk yet, because I've been a bad and lazy sysadmin. We can leave this open until it is.

- Issue History
Date Modified Username Field Change
2014-05-28 03:56 nrook New Issue
2014-05-28 11:57 edlothiol Note Added: 0026299
2014-05-28 22:18 edlothiol Note Added: 0026301
2014-05-28 22:18 edlothiol Status new => resolved
2014-05-28 22:18 edlothiol Fixed in Branch => 0.15 development branch
2014-05-28 22:18 edlothiol Resolution open => no change required
2014-05-28 22:18 edlothiol Assigned To => edlothiol
2014-07-20 20:57 nrook Note Added: 0026788
2014-07-20 20:57 nrook Status resolved => new
2014-07-21 01:54 neil Note Added: 0026795

Mantis 1.1.8[^]
Copyright © 2000 - 2009 Mantis Group
Powered by Mantis Bugtracker