Security/Reviews/SettingsAPI
Item Reviewed
Settings API | |||||||||||||
Target |
2 Total; 0 Open (0%); 2 Resolved (100%); 0 Verified (0%); |
{{#set:SecReview name=Settings API
|SecReview target=
ID | Summary | Priority | Status |
---|---|---|---|
678695 | Settings API | -- | RESOLVED |
750992 | [Security Review] Settings API | P2 | RESOLVED |
2 Total; 0 Open (0%); 2 Resolved (100%); 0 Verified (0%);
}}
Introduce the Feature
Goal of Feature, what is trying to be achieved (problem solved, use cases, etc)
- 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:
- 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
What solutions/approaches were considered other than the proposed solution?
- indexdb - this is per app rather than global
Why was this solution chosen?
- effectively a shared database
- add a layer of control for permissions
Any security threats already considered in the design and why?
`
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.
{{#set: 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:
- 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 threats considered=' |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.
}}
Action Items
Action Item Status | In Progress | |||||||||||||||||||||||||||||||||||||||||
Release Target | ` | |||||||||||||||||||||||||||||||||||||||||
Action Items | ||||||||||||||||||||||||||||||||||||||||||
3 Total; 0 Open (0%); 3 Resolved (100%); 0 Verified (0%); |
{{#set:|SecReview action item status=In Progress
|Feature version=`
|SecReview action items=
Who | bug | Action | By When | Completed date
[NEW] new [DONE] Done [MISSED] Miss |
jonas | 763965 | cjones/gal Do we want to expose anything at all right now to the web? | [NEW] new | |
jonas | 763969 | Ask the B2G team (consumers of the API) what settings they need to read (and store?) | [NEW] new | |
pauljt | 763970 | Code review (both of the settings API and the consumers) | [NEW] new | |
pauljt | 763972 | Code review (both of the settings API and the consumers) | [NEW] new |
ID | Summary | Priority | Status |
---|---|---|---|
763969 | SecReview: Settings API - Read & Store items | -- | RESOLVED |
763970 | SecReview: Settings API - code review | -- | RESOLVED |
763972 | SecReview: Settings API - code review (settings & consumers) | -- | RESOLVED |
3 Total; 0 Open (0%); 3 Resolved (100%); 0 Verified (0%);
}}