297
edits
Security/Reviews/SocialAPI: Difference between revisions
Jump to navigation
Jump to search
Added platform issues, open questions, notes
m (formatting changes) |
(Added platform issues, open questions, notes) |
||
| Line 168: | Line 168: | ||
13 Javascript or other active content running in the initial hidden window? | '''13 Javascript or other active content running in the initial hidden window? | ||
Threat | Threat''' | ||
* We create an iframe (for each provider) on the hidden window with the src attribute set to workerURL from the providers manifest. The content retrieved is copied and eval'd in the sandbox. Can code run in the hidden window, prior to being sandboxed? | * We create an iframe (for each provider) on the hidden window with the src attribute set to workerURL from the providers manifest. The content retrieved is copied and eval'd in the sandbox. Can code run in the hidden window, prior to being sandboxed? | ||
* The remote code is loaded into a sandboxed content iframe without access to chrome privileges or the hidden xul window. | * The remote code is loaded into a sandboxed content iframe without access to chrome privileges or the hidden xul window. | ||
| Line 175: | Line 175: | ||
Proposed Remediation | Proposed Remediation | ||
* ? | * ? | ||
'''Open Questions''' | |||
* Is domain in the draft being used the same as origin in SOP? | |||
e.g. Can service provider and and service be on same domain but different port / scheme? (good question, see discussion below under threats brainstorming / "activating the feature") | |||
* Is the notification system different from the notification system being developed by services? | |||
* sandbox == iframe sandbox? | |||
* Enhance user choice by allowing users to "bring their own network" to the web (as opposed to the current system of iframe embedding, which requires the site developer to choose which network to use for social recommendation)<- from notes, I don't understand this, though | |||
* We'd could definitely use a step-by-step explanation of how it is intended to work. Pedantic is good, more detail is better than less. | |||
* We're moving away from using sqlite to using prefs, at least in the near term, for storing the provider information. We expect that any given user will install less than 3 or 4 providers, and most will likely only have 1. (Question: Is this a good assumption? Example: Facebook, Twitter, Google Chat are three. We expect – and hope – more sites will create socialapi functionality. There will likely be toolkits created to make this easy. ) Using a full db for such a small amount of data is overkill and the change to prefs removes a good chunk of code. Any issues/concerns related specifically to the sqlite db can be removed. | |||
'''Platform Issues - Firefox''' | |||
* Data Encryption for preferences data | |||
* Firefox stores a lot of critical metadata in the clear, in text files, such as it's own update url, and the update url for all addons, safebrowsing, etc. It may be a good security improvement to have some crypto signing of that data, which any feature or addon could then also take advantage of. This would be a good bug for the platform or security roadmap, but not a part of the socialapi scope. | |||
'''Platform Issues - Firefox''' | |||
* Non-responsive Scripts (Javascript Engine) | |||
* Cu.Sandbox doesn't provide a way for us to test for non-responsive scripts, so making this a blocker will prevent the feature from moving forward at this time. Shane thinks that kind of functionality should actually be integral to Cu.Sandbox itself rather than features utilizing it, it would be a good addition, but something for js engine. | |||
Placing the non-responsive test at the sandbox level will also provide that protection to the many places sandbox is used throughout firefox. | |||
The real worker implementation should probably be in scope for that as well. | |||
''' Security-Relevent Technical Notes''' | |||
* Initial startup code - https://github.com/mozilla/socialapi-dev/blob/develop/content/main.js | |||
* Other source repo - https://github.com/michaelrhanson/socialapi-hacking | |||
* Q - Where is the definitive source? | |||
Firefox Hidden Window refs – every instance of Firefox has a hidden window - https://mxr.mozilla.org/mozilla-central/search?string=hiddenWindow | |||
* Controls around installing new social providers: For installing new social providers, the safeguards in place include: requiring valid ssl certs, safebrowsing checks, same-origin policy of any urls in the manifest. | |||
* Bug 756591 asks whether this is sufficient, if we have to implement a new service to support blacklisting it will have to be on a future roadmap. For the initial landing, it is likely we will not include the ability to install new providers, but we will ensure that developers can easily add new providers via preferences or addons that set those preferences. | |||
* Right now (an likely not for the initial landing), the only way to install a remote manifest file (other than a malicious addon) is by browsing to a website that has a "link rel=manifest href=path" tag in the the html head section. That link path must be same-origin to the page containing it, and urls within the manifest must be same origin as the manifest file itself. The channel must be secure with a valid ssl cert. The safebrowsing check is just an additional check prior to these measures. | |||
* Sync support - Moving to using prefs for storing the social provider metadata gives us sync for free. | |||
'''Notes for pentest''' | |||
* Sandbox: Cu.Sandbox allows chrome to inject code for content to use, some of which may presumably safely call back into chrome functionality. IMHO The question here is, have we used the sandbox correctly. We had the code looked over in bug 751241, and further again by ddahl (more an off-the-record review for a question I had). As part of the full code review the sandbox use should be scrutinized. If there are risk problems with the sandbox itself, that needs to go to the javascript engine team. | |||
}} | }} | ||
{{SecReviewActionStatus | {{SecReviewActionStatus | ||