505
edits
Labs/Weave/Sync Client Security Review: Difference between revisions
Jump to navigation
Jump to search
Labs/Weave/Sync Client Security Review (view source)
Revision as of 23:54, 10 February 2010
, 10 February 2010→Review comments
| Line 90: | Line 90: | ||
== Review comments == | == Review comments == | ||
* In account creation, should we protect against an SSL hijack? | |||
* Can we show special UI for users that do not use password manager (have turned it off?) | |||
* Passwords are unsalted MD5, should be MD-256 or SHA-256. | |||
* Some questions about what the captcha can do. | |||
* We don't have any official agreements with recaptcha - should we have one in case we become ultra popular? | |||
* '''Action:'''We should do a security audit for the client, hasn't been done yet. | |||
* What happens if we get bad certs? We silently eat them and get a generic error. | |||
* Is the user warned that you are storing password/passphrase in password manager? | |||
* '''Action:''' Have a blacklist of prefs, things we would never sync even if the user asks us to. Also, should we not sync Weave prefs? | |||
* Recommend users that have their own | |||
* '''Action:''' Password is Basic inside SSL. Should be Digest. | |||
* Is there a privacy problem - a network listener can figure out when you are syncing? | |||
* '''Action:''' For form data, we use the hash as the GUID since Places doesn't use GUIDs for form data. | |||
* '''Action:''' Can the captcha being loaded in a browser element do something to the rest of the browser? | |||
* Enumerating user names is trivial. Was designed to enable discovery - can we make this more secure without compromising UX? | |||
* '''Action:''' Don't differentiate between user name is wrong and user name doesn't exist. | |||
* '''Action:''' We should not sync the passphrase. | |||
* '''Action:''' We should have the user fill out or pick a client name. User doesn't realize that they are sharing this information to us. Also affects the Privacy Policy. If this information is actually not useful to us, then we should encrypt it. | |||
* '''Action:''' IV should be stored along side the record and should be changed every time the record is changed. | |||