canmove, Confirmed users, Bureaucrats and Sysops emeriti
2,776
edits
Security/Reviews/SettingsAPI: Difference between revisions
no edit summary
(Created page with "{{SecReviewInfo |SecReview name=Settings API }} {{SecReview}} {{SecReviewActionStatus |SecReview action item status=None }}") |
No edit summary |
||
| (One intermediate revision by the same user not shown) | |||
| Line 1: | Line 1: | ||
{{SecReviewInfo | {{SecReviewInfo | ||
|SecReview name=Settings API | |SecReview name=Settings API | ||
|SecReview target=<bugzilla> | |||
{ | |||
"id":"750992,678695" | |||
} | |||
</bugzilla> | |||
}} | |||
{{SecReview | |||
|SecReview feature goal=* a way to store system wide settings to a backend database | |||
** set:get for settings | |||
** Listen for changes | |||
* Only certified apps have to settings API | |||
** only certified apps can *write* | |||
* Mainly it will only the Settings API | |||
* List of settings: | |||
** https://etherpad.mozilla.org/LuzVJ1iGuO | |||
* Might expose specific settings to untrusted/trusted applications, for them to read them. | |||
* Currently there are two permisions settings: | |||
** read settings | |||
** write settings | |||
* Design might be: | |||
** App would declare in the manifest, what settings it would want access to, based on the type of app | |||
** Only certified apps could set them | |||
*** I think we should allow some settings to be settable by trusted app | |||
* Other apps could only read them | |||
|SecReview alt solutions=* indexdb - this is per app rather than global | |||
|SecReview solution chosen=* effectively a shared database | |||
* add a layer of control for permissions | |||
|SecReview threat brainstorming=* What security model has been implemented for the settings API? | |||
** The current implementation is similar to the SMS API and has a readonly and readwrite permission flag per principal.URI. | |||
* Certified app vulnerability would have access to read or change any setting | |||
** Could be an explicit permission for read/write settings (or even an explicit subset of settings). I.e. the list of settings that the app has access to would be enumerated in manifest. | |||
** Even certified apps should be limited to the settings explicitly listed in its manifest -- improves auditability and a line of defense against compromised apps. | |||
* (future threat?) Untrusted app reading sensitive data from a setting | |||
** Expose only a subset of settings for untrusted app | |||
** Review security & privacy implications of settings prior to exposing settings | |||
* Denial of service - change a setting which breaks a feature (e.g. change RIL settings to stop phone functionality working) | |||
** Mitigation: only certified apps can change settings | |||
** Testing a certified app should include review of what settings it changes | |||
* Storing some passwords in this settings database | |||
** Settings are currently stored in plaintext encoded format | |||
** No plan for a keystore at the moment | |||
* [future threat] If we expose reading/setting to untrusted/trusted app, we should make sure, those app will not be able to reach settings they should not be able to reach. Given that it is a DB in the backend. | |||
}} | }} | ||
{{SecReviewActionStatus | {{SecReviewActionStatus | ||
|SecReview action item status= | |SecReview action item status=In Progress | ||
|SecReview action items=<table border="1"> | |||
<tr> | |||
<td>Who</td> | |||
<td>bug</td> | |||
<td>Action</td> | |||
<td>By When</td> | |||
<td>Completed date | |||
{{new|new}} | |||
{{done|Done}} | |||
{{miss|Miss}} | |||
</td> | |||
</tr> | |||
<tr> | |||
<td>jonas </td> | |||
<td>763965 </td> | |||
<td>cjones/gal Do we want to expose anything at all right now to the web?</td> | |||
<td> </td> | |||
<td>{{new|new}} </td> | |||
</tr> | |||
<tr> | |||
<td>jonas </td> | |||
<td>763969 </td> | |||
<td>Ask the B2G team (consumers of the API) what settings they need to read (and store?) </td> | |||
<td> </td> | |||
<td>{{new|new}} </td> | |||
</tr> | |||
<tr> | |||
<td>pauljt </td> | |||
<td>763970 </td> | |||
<td>Code review (both of the settings API and the consumers) </td> | |||
<td> </td> | |||
<td>{{new|new}} </td> | |||
</tr> | |||
<tr> | |||
<td>pauljt </td> | |||
<td>763972 </td> | |||
<td>Code review (both of the settings API and the consumers) </td> | |||
<td> </td> | |||
<td>{{new|new}} </td> | |||
</tr> | |||
</table> | |||
<bugzilla> | |||
{ | |||
"id":"`763965,763969,763970,763972" | |||
} | |||
</bugzilla> | |||
}} | }} | ||