Security String
I think this whole page can be condensed to:
If a user logs in but forgets to log out on a public terminal, he cannot revoke that login later, and people could steal the cookie there. My recent cookie changes (see commits 1933:19e3af7cabcd and a few before that) in 1.6 would allow revoking other logins trivially by simply invalidating all stored cookie secrets other than the current one. This just needs some UI.
This patch achieves this by introducing a "security string" feature that is hashed into the cookie signature so you can change the security string and thereby log out all your possible sessions.
By simply removing all other cookie secrets, the same is achieved and the user is even kept logged on at the current machine. -- JohannesBerg 2007-04-03 16:50:52
TODO for this page: polish english, remove redundancy, refactor
This patch in moin-1.5 and for security string.
|
|
|
![/!\](/moin_static19/modernized/img/alert.png "/!\"){kind=small}
If find any bug. Please let me <frankie AT FREESPAM openworkshop DOT org> to know. 
= Happy New Year. = 
I am sorry, I need to alter MoinMoin code for security's reason.
A Story
One day a company boss sat in a coffee shop. Suddenly he had a very good idea. Then he wanted to enter it in his company's MoinMoin wiki.
The Boss said: "Yes, It is a very great idea."
But he didn't have a private computer, he was using Coffee Shop's computer to do it. He entered the idea. He was very happy.
His phone rang. He left the coffee shop, but he forgot to logout. ...
What time MoinMoin Change or Create the user's security_string
- when the user create.
- when the user profile hasn't security_string.
- . and When the user done password_auth.
- when the user request account_sendmail.
- when the user logout.
- when the user after moin_url auth.
Why security_string is good for security
- If a cracker knows the user.id, he can not begin to attack, because he doesn't know the security_string.
- Change security_string, if possible. It can keep not enough time for cracker to guess user's security_string.
- Every user have himself security_string, and don't pass it through internet.
Why change the user in logout
- First every member need a way to change his security_string.
- I think logout is a better session then login for change it.
- Member think logout is a global logout.
- If member login in a remote computer like above story. he can logout in another computer, then the remote computer cann't do cookie_auth again.
What does happen with the open connection on the system were he has forgotten to log out? How does it protect for someone using the account or changing the password on the running system? Is probably an automatic vigilance device needed (dead-man switch)? -- ReimarBauer 2006-02-01 22:03:20
All problem is in the cookie, because the moin_id contents is fix, cann't easy to change it. -- FrankieChow 2006-03-22 16:10:50
- Of Course, you can add another contents in Cookie, like Security_String and don't modify the moin_id. ( But Modify is simple. )
Now MoinMoin's Security Policy is very bad, Not Administrator like to Change it. ( Include me ) so .. dead-man switch .. -- FrankieChow 2006-03-22 16:10:50
- If member logout mean that: He will do password_auth in next time.
That's difficult too, because some people do only logout on a foreigner computer and probably not on theirs. -- ReimarBauer 2006-02-01 22:34:02
I know it. but this is simple way to modify the cookie contents for MoinMoin Users. I don't like modify in userform, maybe not people like modify in there. -- FrankieChow 2006-03-22 16:10:50
- If member login in a remote computer like above story. he can logout in another computer, then the remote computer cann't do cookie_auth again.
- Member think logout is a global logout.
Why change the user in account_sendmail
I think another way to change security_string on account_sendmail. because maybe the member lost info, so he want to MoinMoin to send it for mail. Of course he lose include security_string and cookie info.
Why still pass the user.id in cookie
Please see MoinMoinBugs/Cookie is not secure enough, the cracker know user.id is not a security problem, because the cookie have another part: security_string. it is keep security for cracker.
Send the user.id in cookie, it will keep the code simple.
About the Patch
- Applies to
- base core code.
Modify
M MoinMoin/auth.py M MoinMoin/request.py M MoinMoin/user.py M MoinMoin/userform.py M MoinMoin/multiconfig.py
Add
A util/securitystring.py
Maybe
You don't need to change multiconfig.py, Maybe you can just modify in wikiconfig.py ( Modify multiconfig.py for moin_url auth module ) .
- Add a line to wikiconfig.py
- Like it.
auth = [authmodule.moin_cookie, authmodule.moin_url]
- Purpose
Protect the Cookie for MoinMoin.auth.moin_cookie .
- Description
Patch Job
I want to record my patch job in here.
TODO
- freeze code.
Done
- Care the cache, when the security_string change. then need to update it.
When member input the utf-8 string in the securty_string field in UserPreferences
- Redesign cal_security_userid module. ( now have bug. )
- Don't change anything in wiki/config/farmconfig.py
- Move Active the url_auth to another patch.
ReActive in here.
- Change the user's security_string, When the user require mail account
- Change the userform, Keep setCookie, when user's security_string isn't same of user datafile. ( not all time. )
Don't modify the MoinMoin.request.setCookie Module.
- When the user logout, then change the user.security_string.
Disable the user security_string handle in User's UserPreferences.
- Group luck string into securitystring handle.
- Add the make_cache module to securitystring to create the caching obj.
- Add the update_cache module to securitystring to handle the cache update
- update_cache module will use securitystring.cal_* to update the cache.
- Change user.save() to do securitystring.update_cache when change user.security_string.
- ?Need do it:: Change the securitystring to a class not modules.
- Check the cache content before update it.
- active the auth_url and disable the {SHA} string auth.
- change the cal_security_userid module, keep it return the User, not just uid.
- represent securitystring.py use patch format.
- Do the Document.
see mod_env。
I think make the security_string
os.environ['MOIN_SECURITY_STRING'] + user.security_string
is better then just user.security_string.
1 #!python 2 def make_security_key(request, securitystring, userid): 3 """ 4 Make the hmac value for 5 Key: securitystring 6 msg: userid 7 8 HOWTO set env['MOIN_SECURITY_STRING'] in cgi mode or mod_python mode. 9 Please see: http://httpd.apache.org/docs/1.3/mod/mod_env.html 10 """ 11 if request.env.has_key('MOIN_SECURITY_STRING'): 12 return hmac.new( str(request.env['MOIN_SECURITY_STRING']) + securitystring, userid).hexdigest() 13 return hmac.new(securitystring, userid).hexdigest()
- If you use the patch in moin-1.5.0 then you can change above code to
+ if check_hmac_string: + if not user_data.has_key('security_string'): + return
Patch
1 * looking for arch@arch.thinkmo.de--2003-archives/moin--main--1.5--patch-361 to compare with
2 * comparing to arch@arch.thinkmo.de--2003-archives/moin--main--1.5--patch-361
3 M MoinMoin/auth.py
4 M MoinMoin/multiconfig.py
5 M MoinMoin/request.py
6 M MoinMoin/user.py
7 M MoinMoin/userform.py
8 A MoinMoin/util/securitystring.py
9
10 * modified files
11
12 --- orig/MoinMoin/auth.py
13 +++ mod/MoinMoin/auth.py
14 @@ -45,6 +45,7 @@
15
16 import Cookie
17 from MoinMoin import user
18 +from MoinMoin.util import securitystring
19
20 def moin_cookie(request, **kw):
21 """ authenticate via the MOIN_ID cookie """
22 @@ -54,17 +55,17 @@
23 u = user.User(request, name=name, password=password,
24 auth_method='login_userpassword')
25 if u.valid:
26 + # Update the User security_string,
27 + # If he don't have security_string.
28 + try:
29 + u.security_string
30 + except AttributeError:
31 + u.security_string = securitystring.gen(30)
32 + u.save()
33 request.user = u # needed by setCookie
34 request.setCookie()
35 return u, False
36 return None, True
37 -
38 - if kw.get('logout'):
39 - # clear the cookie in the browser and locally. Does not
40 - # check if we have a valid user logged, just make sure we
41 - # don't have one after this call.
42 - request.deleteCookie()
43 - return None, True
44
45 try:
46 cookie = Cookie.SimpleCookie(request.saved_cookie)
47 @@ -72,23 +73,59 @@
48 # ignore invalid cookies, else user can't relogin
49 cookie = None
50 if cookie and cookie.has_key('MOIN_ID'):
51 - u = user.User(request, id=cookie['MOIN_ID'].value,
52 + # Use security_string to handle the Cookie.
53 + u = user.User(request, hmac_uid_string=cookie['MOIN_ID'].value,
54 auth_method='moin_cookie', auth_attribs=())
55 if u.valid:
56 - return u, False
57 + if kw.get('logout'):
58 + # FrankieChow: Why Does not check it?
59 + # I think need to check it, or pass it to another auth handle.
60 + #
61 + ### clear the cookie in the browser and locally. Does not
62 + ### check if we have a valid user logged, just make sure we
63 + ### don't have one after this call.
64 + request.deleteCookie()
65 + # When the user do global logout then change the
66 + # security_string. ( in here. All logout is global logout. )
67 + u.security_string = securitystring.gen(30)
68 + u.save()
69 + return None, True
70 + else:
71 + return u, False
72 +
73 + # If the brower don't have MOIN_ID cookie, just delete the cookie.
74 + if kw.get('logout'):
75 + request.deleteCookie()
76 + return None, True
77 +
78 return None, True
79
80
81 +def moin_url(request, **kw):
82 + # The url syntax is like this: action=userform&uid=
83 + action = request.form.get('action',[None])[0]
84 + uid = request.form.get('uid',[None])[0]
85 + user_obj = user.User(request)
86 + if action == 'userform' :
87 + u = user.User(request, auth_method='moin_url', hmac_uid_string=uid )
88 + if u.valid:
89 + u.security_string = securitystring.gen(30)
90 + u.save()
91 + request.user = u
92 + request.setCookie()
93 + return u, False
94 + return None, True
95 +
96 #
97 # idea: maybe we should call back to the request object like:
98 # username, password, authenticated, authtype = request.getUserPassAuth()
99 -# WhoEver geheim false basic (twisted, doityourself pw check)
100 -# WhoEver None true basic/... (apache)
101 -#
102 +# WhoEver geheim false basic (twisted, doityourself pw check)
103 +# WhoEver None true basic/... (apache)
104 +#
105 # thus, the server specific code would stay in request object implementation.
106 #
107 # THIS IS NOT A WIKI PAGE ;-)
108 -
109 +
110 def http(request, **kw):
111 """ authenticate via http basic/digest/ntlm auth """
112 from MoinMoin.request import RequestTwisted, RequestCLI
113
114
115 --- orig/MoinMoin/multiconfig.py
116 +++ mod/MoinMoin/multiconfig.py
117 @@ -172,7 +172,7 @@
118 actions_excluded = [] # ['DeletePage', 'AttachFile', 'RenamePage']
119 allow_xslt = 0
120 attachments = None # {'dir': path, 'url': url-prefix}
121 - auth = [authmodule.moin_cookie]
122 + auth = [authmodule.moin_cookie, authmodule.moin_url]
123
124 backup_compression = 'gz'
125 backup_users = []
126
127
128 --- orig/MoinMoin/request.py
129 +++ mod/MoinMoin/request.py
130 @@ -9,7 +9,7 @@
131
132 import os, time, sys, cgi, StringIO
133 from MoinMoin import config, wikiutil, user
134 -from MoinMoin.util import MoinMoinNoFooter, IsWin9x
135 +from MoinMoin.util import MoinMoinNoFooter, IsWin9x, securitystring
136
137 # Timing ---------------------------------------------------------------
138
139 @@ -1216,7 +1216,16 @@
140 # Set the cookie
141 from Cookie import SimpleCookie
142 c = SimpleCookie()
143 - c['MOIN_ID'] = self.user.id
144 + # Modify the Cookie String Syntax.
145 + # Keep the self.user.id in Cookie.
146 + # 1. easy for auth.
147 + # 2. and don't need to care the
148 + # securitystring.make_security_key(self, security_string, self.user.id)
149 + # is unique.
150 + c['MOIN_ID'] = '%s%s%s' %(
151 + securitystring.make_security_key(self, self.user.security_string, self.user.id),
152 + securitystring.luck(self),
153 + self.user.id )
154 c['MOIN_ID']['max-age'] = maxage
155 if self.cfg.cookie_domain:
156 c['MOIN_ID']['domain'] = self.cfg.cookie_domain
157
158
159 --- orig/MoinMoin/user.py
160 +++ mod/MoinMoin/user.py
161 @@ -17,7 +17,7 @@
162 PICKLE_PROTOCOL = pickle.HIGHEST_PROTOCOL
163
164 from MoinMoin import config, caching, wikiutil
165 -from MoinMoin.util import datetime, filesys
166 +from MoinMoin.util import datetime, filesys, securitystring
167
168
169 def getUserList(request):
170 @@ -191,7 +191,9 @@
171 class User:
172 """A MoinMoin User"""
173
174 - def __init__(self, request, id=None, name="", password=None, auth_username="", **kw):
175 + # FrankieChow: I think keep the hmac_uid_string auth in User,
176 + # It is better then other, just alter more core code.
177 + def __init__(self, request, id=None, name="", password=None, auth_username="", hmac_uid_string=None, **kw):
178 """ Initialize User object
179
180 @param request: the request object
181 @@ -255,6 +257,7 @@
182 # attrs not saved to profile
183 self._request = request
184 self._trail = []
185 + self._hmac_uid_string = hmac_uid_string
186
187 # we got an already authenticated username:
188 check_pass = 0
189 @@ -272,6 +275,11 @@
190 self.load_from_id(1)
191 else:
192 self.id = self.make_id()
193 + elif self._hmac_uid_string:
194 + try:
195 + self._hmac_string, self.id = self._hmac_uid_string.split( securitystring.luck(self._request) )[:2]
196 + except: pass
197 + self.load_from_id(check_pass=0, check_hmac_string=1)
198 else:
199 self.id = self.make_id()
200
201 @@ -331,7 +339,9 @@
202 """
203 return os.path.exists(self.__filename())
204
205 - def load_from_id(self, check_pass=0):
206 + # In User Class, here is handle User Auth.
207 + # So change it to require the check_hmac_string.
208 + def load_from_id(self, check_pass=0, check_hmac_string=0):
209 """ Load user account data from disk.
210
211 Can only load user data if the id number is already known.
212 @@ -375,6 +385,17 @@
213 return
214 else:
215 self.trusted = 1
216 +
217 + # If User haven't security_string, Don't handle in here,
218 + # Just handle in auth.py in moin_cookie after the user done passd auth.
219 + if check_hmac_string:
220 + if not user_data.has_key('security_string'):
221 + return
222 + valid, changed = self._validateHmacString(user_data)
223 + if not valid:
224 + return
225 + else:
226 + self.trusted = 1
227
228 # Remove ignored checkbox values from user data
229 for key, label in self._cfg.user_checkbox_fields:
230 @@ -488,6 +509,11 @@
231 # No encoded password match, this must be wrong password
232 return False, False
233
234 + def _validateHmacString(self, data):
235 + if self._hmac_string == securitystring.make_security_key(self._request, data['security_string'], self.id):
236 + return True, False
237 + return False, False
238 +
239 def save(self):
240 """ Save user account data to user account file on disk.
241
242 @@ -935,14 +961,18 @@
243 from MoinMoin.util import mail
244 _ = self._request.getText
245
246 + # If MoinMoin use security_string logic to do url_auth.
247 + # When use SSHA to disable the Login Password.
248 text = '\n' + _("""\
249 Login Name: %s
250
251 -Login Password: %s
252 -
253 -Login URL: %s/?action=userform&uid=%s
254 +Login URL: %s/?action=userform&uid=%s%s%s
255 """, formatted=False) % (
256 - self.name, self.enc_password, self._request.getBaseURL(), self.id)
257 + self.name, self._request.getBaseURL(),
258 + securitystring.make_security_key(self._request, self.security_string, self.id),
259 + securitystring.luck(self._request),
260 + self.id
261 + )
262
263 text = _("""\
264 Somebody has requested to submit your account data to this email address.
265
266
267 --- orig/MoinMoin/userform.py
268 +++ mod/MoinMoin/userform.py
269 @@ -8,7 +8,7 @@
270
271 import string, time, re
272 from MoinMoin import user, util, wikiutil
273 -from MoinMoin.util import web, mail, datetime
274 +from MoinMoin.util import web, mail, datetime, securitystring
275 from MoinMoin.widget import html
276
277 _debug = 0
278 @@ -77,6 +77,10 @@
279 for uid in users:
280 theuser = user.User(self.request, uid)
281 if theuser.valid and theuser.email.lower() == email:
282 + # Change the security_string
283 + # When the user request the account_sendmail.
284 + theuser.security_string = securitystring.gen(30)
285 + theuser.save()
286 msg = theuser.mailAccountData()
287 return wikiutil.escape(msg)
288
289 @@ -148,6 +152,8 @@
290 if thisuser.email == theuser.email and not thisuser.disabled:
291 return _("This email already belongs to somebody else.")
292
293 + # Before create the user's profile, create the user's security_string.
294 + theuser.security_string = securitystring.gen(30)
295 # save data
296 theuser.save()
297 if form.has_key('create_and_mail'):
298
299
300 --- orig/MoinMoin/util/securitystring.py
301 +++ mod/MoinMoin/util/securitystring.py
302 @@ -0,0 +1,36 @@
303 +"""
304 + MoinMoin - Handle the Security String
305 + @copyright: (c) Bastian Blank, Florian Festi, Thomas Waldmann
306 + @copyright: MoinMoin:FrankieChow
307 + @license: GNU GPL, see COPYING for details.
308 +"""
309 +import os, hmac, string, random
310 +
311 +# Follow http://moinmoin.wikiwikiweb.de/MoinMoinBugs/Cookie_is_not_secure_enough
312 +# This code is write by Nir Soffer
313 +
314 +def gen(number):
315 + # Make the number is more security.
316 + random_number = random.randint(number/2,number)
317 + safe = string.ascii_letters + string.digits + '_-'
318 + return "%s" % (''.join([random.choice(safe) for i in range(random_number)]))
319 +
320 +###
321 +
322 +def luck(request):
323 + # ':=:' is FrankieChow luck string. maybe you can change this to
324 + # self.cfg.site_luck_string
325 + return ':=:'
326 +
327 +def make_security_key(request, securitystring, userid):
328 + """
329 + Make the hmac value for
330 + Key: securitystring
331 + msg: userid
332 +
333 + HOWTO set env['MOIN_SECURITY_STRING'] in cgi mode or mod_python mode.
334 + Please see: http://httpd.apache.org/docs/1.3/mod/mod_env.html
335 + """
336 + if request.env.has_key('MOIN_SECURITY_STRING'):
337 + return hmac.new( str(request.env['MOIN_SECURITY_STRING']) + securitystring, userid).hexdigest()
338 + return hmac.new(securitystring, userid).hexdigest()
339
Discussion
- In this patch. I don't disable the {SHA} auth.
- Please wait my {SSHA} patch.
Should we still wait for something or begin with what's here???
Did you see: http://www.xml.com/pub/a/2003/12/17/dive.html
I am sorry. I will later to post the {SSHA} patch. ( but I think you don't need to wait for me, maybe I should post it in next version. ) -- FrankieChow 2006-04-07 17:10:00
- because in my site this isn't the main problem.
- Thank you for your information.
-- FrankieChow 2006-04-07 09:12:00
Plan
- Priority:
- Assigned to:
- Status: