Closed Bug 833991 Opened 7 years ago Closed 7 years ago

Security Review for Android App Not Responding (ANR) Reporting

Categories

(mozilla.org :: Security Assurance: Review Request, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jchen, Assigned: mgoodwin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [completed secreview][snippets][Fx])

Initial Questions:

Project/Feature Name: Android App Not Responding (ANR) Reporting
Tracking  ID:
Description:
App Not Responding (ANR) dialogs present a bad user experience for Fennec users, and they can be caused by a variety of bugs. In order to determine and fix the causes of ANRs, we need telemetric data about the origin of the ANRs.

The reporting process involves sending thread stacks and logs through telemetry when ANRs happen. This is the Fennec equivalent of the "chrome hang" telemetry already present on desktop Firefox Nightlies.

There will be a client side and a server side. The client side will gather the stacks and logs and use existing telemetry system to report. The server side will process the newly added telemetry fields and save the data for further analysis.

Security-wise, there should not be too much concern, because we reuse much of the existing telemetry code for reporting. The data-gathering part of the client side may need additional review for security.

Privacy-wise, ANR reporting is part of telemetry, and the user already has control over enabling telemetry. However, the prompt given to the user may need to change, or the user may need to be prompted separately because the stacks and logs contain more sensitive information than regular telemetry data.

Additional Information:

Urgency: a week
Key Initiative: Firefox Mobile
Release Date: 
Project Status: development
Mozilla Data: Yes
New or Change: New
Mozilla Project: none
Mozilla Related: 
Separate Party: No

Security Review Questions:

Affects Products: Yes
Review Due Date: 
Review Invitees: 
Extra Information:
assigning to mgoodwin to look into
unhiding as this does not need to be secret
Assignee: nobody → mgoodwin
Group: mozilla-corporation-confidential
Status: NEW → ASSIGNED
Do you have an estimate of when you can look at this, Mark?
Flags: needinfo?(mgoodwin)
Yeah, I'll take a look at this next week. If a group review is needed I'll schedule one for the week after.
Flags: needinfo?(mgoodwin)
Whiteboard: [in-progress secreview][start 2013-02-18][target 2013-02-28][snippets]
I see that if the dalvik stack trace file has fennec traces we send the whole thing; is this necessary? Can we get away with just the lines that have fennec entries?
Flags: needinfo?(nchen)
(In reply to Mark Goodwin [:mgoodwin] from comment #4)
> I see that if the dalvik stack trace file has fennec traces we send the
> whole thing; is this necessary? Can we get away with just the lines that
> have fennec entries?

Yes, technically we only need the Fennec lines. The reason I chose to send the file without further parsing/processing is because I wanted to avoid spending extra CPU time (another thread is hung/busy at this point, so our ANR reporting thread might not get much CPU time). Also, we wouldn't be dependent on the exact format of the stack trace file. But maybe these concerns are unfounded.

I've been thinking about setting a hard limit on the size of stack trace to send (e.g. 10kB max). This should crop out most of the unrelated stack traces and save network bandwidth. Also it wouldn't add any extra parsing/processing.
Flags: needinfo?(nchen)
(In reply to Jim Chen [:jchen :nchen] from comment #5)
> (In reply to Mark Goodwin [:mgoodwin] from comment #4)
> > I see that if the dalvik stack trace file has fennec traces we send the
> > whole thing; is this necessary? Can we get away with just the lines that
> > have fennec entries?
> 
> Yes, technically we only need the Fennec lines. The reason I chose to send
> the file without further parsing/processing is because I wanted to avoid
> spending extra CPU time (another thread is hung/busy at this point, so our
> ANR reporting thread might not get much CPU time). Also, we wouldn't be
> dependent on the exact format of the stack trace file. But maybe these
> concerns are unfounded.

We're already dependent on the format in that we require it to match the current assumed format for the isGeckoTraces check. It's a tough one to call on CPU time; I guess it depends how big these things get.

This is more a user perception / privacy than security concern, I guess (or, up to Tom). Tom, are you aware there's non-fennec stuff in those logs?
Flags: needinfo?(tom)
I don't think that a typical user (even a typical Aurora user) would expect Fennec to send crash reports which contain info about other apps. Would the 10kb heuristic effectively filter for just Fennec-related material?
Flags: needinfo?(tom)
(In reply to Tom Lowenthal [:StrangeCharm] from comment #7)
> I don't think that a typical user (even a typical Aurora user) would expect
> Fennec to send crash reports which contain info about other apps. Would the
> 10kb heuristic effectively filter for just Fennec-related material?

Not really; it'd (obviously) reduce the quantity of unrelated information though.
(In reply to Mark Goodwin [:mgoodwin] from comment #8)
> (In reply to Tom Lowenthal [:StrangeCharm] from comment #7)
> > I don't think that a typical user (even a typical Aurora user) would expect
> > Fennec to send crash reports which contain info about other apps. Would the
> > 10kb heuristic effectively filter for just Fennec-related material?
> 
> Not really; it'd (obviously) reduce the quantity of unrelated information
> though.

Using the sample stack trace from Bug 834086, the Fennec stacks are about 21kB. Obviously the exact size will be a bit random depending on device/build/etc, so scanning for the end of the Fennec stacks would be a more sure way.
Filed Bug 845416 to limit stacks to the Fennec process.
thanks jchen
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [in-progress secreview][start 2013-02-18][target 2013-02-28][snippets] → [completed secreview][snippets]
Whiteboard: [completed secreview][snippets] → [completed secreview][snippets][FxA]
Whiteboard: [completed secreview][snippets][FxA] → [completed secreview][snippets][Fx]
You need to log in before you can comment on or make changes to this bug.