SecReview: Resource Lock API

RESOLVED FIXED

Status

mozilla.org
Security Assurance: Review Request
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: curtisk, Assigned: mgoodwin)

Tracking

Details

(URL)

1. Who is/are the point of contact(s) for this review?
2. Please provide a short description of the feature / application (e.g. problem solved, use cases, etc.):
3. Please provide links to additional information (e.g. feature page, wiki) if available and not yet included in feature description:
4. Does this request block another bug? If so, please indicate the bug number
5. This review will be scheduled amongst other requested reviews. What is the urgency or needed completion date of this review?
6. Please answer the following few questions: (Note: If you are asked to describe anything, 1-2 sentences shall suffice.)
6a. Does this feature or code change affect Firefox, Thunderbird or any product or service the Mozilla ships to end users?
6b. Are there any portions of the project that interact with 3rd party services?
6c. Will your application/service collect user data? If so, please describe 
7. If you feel something is missing here or you would like to provide other kind of feedback, feel free to do so here (no limits on size):
8. Desired Date of review (if known from https://mail.mozilla.com/home/ckoenig@mozilla.com/Security%20Review.html) and whom to invite.
I'd appreciate if you'd fill in answers to the questions for which you already know the answer, or can make a reasonable guess.  I'll then be happy to fill in whatever gaps remain.
Honestly, that is supposed to be done by the dev/pm/whomever on the product side.
1) to minimize guessing, as guessing is dangerous for us security folk
2) I honestly don't have time to hunt for things that may or may not exist and that I more than likely don't know where they would be if they did exist.
3) This stuff can change over time, so we wait for the product team to answer the questions as a trigger that a review is ready.
> Honestly, that is supposed to be done by the dev/pm/whomever on the product side.

That's exactly what I disagree with.  :)

> 1) to minimize guessing, as guessing is dangerous for us security folk

I suspect the rate of error for many of these questions, such as "does this affect Firefox, Thunderbird, or anything else we ship" would be quite low.  But I would be more than happy to correct any mistakes you might make.

> 3) This stuff can change over time, so we wait for the product team to answer the questions as a 
> trigger that a review is ready.

I would be more than happy to update this bug as things change over time.  But note that in this case, the bug in question is already closed, so I suspect it's done changing.

> 2) I honestly don't have time to hunt for things that may or may not exist and that I more than 
> likely don't know where they would be if they did exist.

As I said, I'm not asking for you to spend a lot of time hunting for things.  I'm happy to fill in whatever gaps exist in the top-of-your-head knowledge.

I suspect that for this bug you could correctly fill in:

  (2) from bug 697132's summary
  (4) from the list of bugs which block/depend on bug 697132
  (5) from the fact that bug 697132 is already resolved
  (6a) from the fact that bug 697132 has a patch, or from the component of the bug
  (6b) from the summary of bug 697132
  (6c) from the summary of bug 697132

Additionally, you could make a reasonable stab at the answer to (1) from the author and reviewer of the patches in the bug.

That leaves (3), (7), and (8), and confirmation on whether anyone was left out of your guess to (1), all of which I think are perfectly reasonable things to ask.
I wrote a script to fill this information in: http://people.mozilla.org/~jlebar/sec-review.html?697132

(1) Points of contact: justin.lebar+bug, kchen, sicking
(2) Create API for content to keep the screensaver from turning on (or to prevent phone/tablet's screen from turning off)
(3) No additional information.locat
(4) Blocking bugs: [bug 517870, bug 673923, bug 708964, bug 715782]
(5) Priority: Normal
(6a) Affects Firefox.
(6b) No interaction with third-party services.
(6c) Does not collect user data.
(7) No further comments.
(8) Desired date of review: I'm on the East Coast of the US, and Kan-Ru is in Taiwan.  Kan-Ru, can you propose a time which works for you?
Whiteboard: [pending secreview][needs info] → [pending secreview]
Looking at http://time.is/compare/New_York/PST/Taipei the time I'm still awake would be 10:00am PST
Let's do Friday, May 4 at 10a Pacific, then.  Jonas, you're welcome to come, but no big deal if you can't make it.  I don't think this will be a particularly involved security review.
Whiteboard: [pending secreview] → [pending secreview 2012.05.04]
Assignee: nobody → mgoodwin
Oh, I'm bad with international times.  That's Friday night for you, isn't it, Kan-Ru?  Let us know if you'd prefer Thursday.  That works for me just fine.
per https://bugzilla.mozilla.org/show_bug.cgi?id=697132#c148 is this review ready to go or do we need to wait. I'm confused as to whether this is ready for review or not?
Further work on this topic would be implemented in separate bugs.  So I thought that by marking bug 697132 as sec-review-needed, you were asserting that you wanted a security review on this bug as it stood.

If that's not the case, I certainly don't feel a pressing need to discuss this.
(In reply to Justin Lebar [:jlebar] from comment #7)
> Oh, I'm bad with international times.  That's Friday night for you, isn't
> it, Kan-Ru?  Let us know if you'd prefer Thursday.  That works for me just
> fine.

Friday is fine to me.
Upon reflection, if we have a time that works for all of us and Kan-Ru doesn't mind getting up / staying awake, I think it would be useful to talk about this feature.
OK, :mgoodwin please fill out the required reading so the team can be prepared for this review https://etherpad.mozilla.org/requiredreading
Summary: SecReview: Create API for content to keep the screensaver from turning on (or to prevent phone/tablet's screen from turning off) → SecReview: Resource Lock API
Depends on: 751001
No longer depends on: 751001
Duplicate of this bug: 751001
When: 20120504
Link to calendar entry: https://mail.mozilla.com/home/ckoenig@mozilla.com/Security%20Review.html?view=month&action=view&invId=39f7eb29-6e7d-4e4d-97b6-550567eab25c%3a110490-110489&pstat=AC&exInvId=39f7eb29-6e7d-4e4d-97b6-550567eab25c%3a110490-171831&useInstance=1&instStartTime=1336150800000&instDuration=3600000
SecReview Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=749376
Security Lead: Mark Goodwin
Required Reading List:
* The secreview request bug: https://bugzilla.mozilla.org/show_bug.cgi?id=749376
* Interfaces: https://bugzilla.mozilla.org/attachment.cgi?id=582210&action=edit (first 2 for privileged code, latter 2 for content)
*The bug requesting the feature under review: https://bugzilla.mozilla.org/show_bug.cgi?id=697132 (warning, long)
* some code: https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=697132&attachment=603628
From what I can gather from the above:
Introduce Feature (5-10 minutes) 
Resource lock API is an API for exposing power management functionality to Gaia and content.
- Goal of Feature, what is trying to be achieved (problem solved, use cases, etc)
1) Expose power management features to privileged gaia apps
2) Allow content to request a wake lock for a resource - for each resource, content can hold a lock of state of locked, locked but not visible and unlocked.
- What solutions/approaches were considered other than the proposed solution?
- Why was this solution chosen?
- Any security threats already considered in the design and why?
Battery flattening (by carelessly developed / malicious content) are discussed in bug #697132
There is some discussion of overriding lock state by privileged applications (e.g. to force airplaine mode, etc)
* Threat Brainstorming (30-40 minutes)
* Conclusions / Action Items (10-20 minutes)
Blocks: 754730
Depends on: 764131
(Assignee)

Comment 15

5 years ago
https://wiki.mozilla.org/Security/Reviews/ScreenSaverAPI
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [pending secreview 2012.05.04] → [completed secreview]
Keywords: sec-review-complete
Whiteboard: [completed secreview]
Keywords: sec-review-complete
You need to log in before you can comment on or make changes to this bug.