Logshake contains sensitive private information like IMEI

RESOLVED FIXED in Firefox OS master

Status

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: nhirata, Assigned: daleharvey)

Tracking

unspecified
FxOS-S1 (26Jun)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.5+, b2g-master fixed)

Details

(Whiteboard: [spark][systemsfe])

Attachments

(3 attachments)

1. turn on logshake from the dev menu
2. shake device
3. look at the logs from the logshake

expected: no sensitive private data
Actual: IMEI, serialno in proc-cmdline.log; ip address can be found in properties.log

Note:
1. reason why this bug is filed is because bugzilla lite sends all the data: bug 1008301
Whiteboard: [spark]
[Blocking Requested - why for this release]:
blocking-b2g: --- → spark?
Paul, FYI.  I think there might be some more logging information that might be sensitive.  Could you take a look and see what we could potentially do to mitigate it, please?
Flags: needinfo?(ptheriault)
I'm generally not sure that we should remove this info from logshake, but rather strip it during upload in BZLite. Let's wait for Paul's feedback, though.
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #4)
> I'm generally not sure that we should remove this info from logshake, but
> rather strip it during upload in BZLite. Let's wait for Paul's feedback,
> though.

I think we need to either be sure that we are making the data sanitized, or we should limit access somehow. I'm assuming this information is basically as sensitive as crash information - i.e. could have all sorts of sensitive information in it (allizom bugzilla is down right at the moment so I can actually look). 

Are you sure you can strip all private information? We should probably involve privacy team on this as well.

PS Ive unhid this bug- this is pre-production so I don't think it needs to be hidden.
Group: core-security
Alex, could you please have a look, or reject this bug if you don't have time for it?
Assignee: nobody → lissyx+mozillians
Status: NEW → ASSIGNED
I don't see what I can do: sensitive informations should not be dumped in the first place, I think. Filtering after will be a mess ...
Assignee: lissyx+mozillians → nobody
Status: ASSIGNED → NEW
I think this (comment 4) would mean that it would be up to Dale.  Perhaps getting a list of the ok logs from Paul might be good.  NI Dale as a heads up.
Flags: needinfo?(dale)
(Assignee)

Comment 9

3 years ago
https://bugzilla.mozilla.org/show_bug.cgi?id=1167639 is an example of what log information we get from logshake currently.

A lot of the logs are only useful in some situations, I propose for now, we limit what we get from the activity to only dev-log-main.log, properties.log, logshake-screenshot.png and  system-b2g-application.ini 

In https://bugzilla.mozilla.org/show_bug.cgi?id=1149012 we are going to look at alternate ways to pick up the version information which would make picking up these files by default less needed, and to follow up from that we start working on the ability for users to pick what logging they want to happen / get sent

Any information they send is going to revealing, I think this strikes a reasonable tradeoff
Flags: needinfo?(dale)

Comment 10

3 years ago
Paul, should we sanitize the data before we upload the data.  Need you input.
(In reply to Dale Harvey (:daleharvey) from comment #9)
> https://bugzilla.mozilla.org/show_bug.cgi?id=1167639 is an example of what
> log information we get from logshake currently.
> 
> A lot of the logs are only useful in some situations, I propose for now, we
> limit what we get from the activity to only dev-log-main.log,
> properties.log, logshake-screenshot.png and  system-b2g-application.ini 
> 
> In https://bugzilla.mozilla.org/show_bug.cgi?id=1149012 we are going to look
> at alternate ways to pick up the version information which would make
> picking up these files by default less needed, and to follow up from that we
> start working on the ability for users to pick what logging they want to
> happen / get sent
> 
> Any information they send is going to revealing, I think this strikes a
> reasonable tradeoff

That sounds like a reasonable tradeoff to me - from comment one, only the IMEI & serial number are of main concern to me since these are unique numbers which can't be changed. And this information would be publicly accessibly and tied to one of our users, which is not great. But it sounds like this information is in a specific file, which isn't in the list Dale suggests so that sounds like a reasonable approach to me.
Flags: needinfo?(ptheriault)

Comment 12

3 years ago
Naoki, Do we still need this bug? Based on Paul's comment #11, we don't need to worry about sensitive private data like IMEI and serial # in the log file generated by BzLite.
Flags: needinfo?(nhirata.bugzilla)
(In reply to Jean Gong from comment #12)
> Naoki, Do we still need this bug? Based on Paul's comment #11, we don't need
> to worry about sensitive private data like IMEI and serial # in the log file
> generated by BzLite.

We need to the change that Dale suggests - IE change it so it doesn't grab all the logs by default. 

I definitely didn't say that we don't need to worry about sensitive private data. If possible we should avoid capturing sensitive data in the logs on the phone. But the real risk here, I think, is that we then publish this private information on the web. If mitigate this risk by being selective about what files are actually included, then that risk is mitigated. Ideally logs wouldn't capture any sensitive information, and I would encourage developers to avoid outputing sensitive data to logs. 

But for the purposes of this bug, i think the change that is required is being selective about what files we publish to bugzilla by default.
(Assignee)

Comment 14

3 years ago
Looks like we agree on what we should do here
Assignee: nobody → dale
(Assignee)

Comment 15

3 years ago
Created attachment 8612177 [details] [diff] [review]
1159969.patch

Alexandre, so this reduces the private information the user has to send while reporting, I think we can / should work on having settings enabled logging so if someone is having problems with the email application they can enable logging for that (or wifi etc), but starting off by limiting the data we collect and then can add it on a per case basis, maybe via an optional setting.
Attachment #8612177 - Flags: review?(lissyx+mozillians)
Comment on attachment 8612177 [details] [diff] [review]
1159969.patch

Review of attachment 8612177 [details] [diff] [review]:
-----------------------------------------------------------------

This is totally killing the goal of the feature. If we do not want sensitive information to be sent, we should rather make sure they are not being sent by default. Here we are removing lots of valuable logging instead of fixing this.
Attachment #8612177 - Flags: review?(lissyx+mozillians) → review-
(In reply to Paul Theriault [:pauljt] from comment #13)
> (In reply to Jean Gong from comment #12)
> > Naoki, Do we still need this bug? Based on Paul's comment #11, we don't need
> > to worry about sensitive private data like IMEI and serial # in the log file
> > generated by BzLite.
> 
> We need to the change that Dale suggests - IE change it so it doesn't grab
> all the logs by default. 
> 
> I definitely didn't say that we don't need to worry about sensitive private
> data. If possible we should avoid capturing sensitive data in the logs on
> the phone. But the real risk here, I think, is that we then publish this
> private information on the web. If mitigate this risk by being selective
> about what files are actually included, then that risk is mitigated. Ideally
> logs wouldn't capture any sensitive information, and I would encourage
> developers to avoid outputing sensitive data to logs. 
> 
> But for the purposes of this bug, i think the change that is required is
> being selective about what files we publish to bugzilla by default.

I disagree: this list has been built upon time and experience. Yes, it's a wide scope; but it's what its takes when you fight intermittent/hard to reproduce issues. Which was the main purpose of having this kind of feature.
(Assignee)

Comment 18

3 years ago
Been discussing this on IRC with Alexandre, the issue with this is it makes it impossible to get some useful information for debugging hard to reproduce bugs.

So a proposal for the best balance is that we do no upload all files by default, instead we have a checkbox or such next to the files that are to be uploaded and by default the above list (properties / application.ini / screenshot / main log) are checked.

As a further enhancement, in v2 we will be able to go back to already filed bugs and attach previously created logs, we just need to make a file explorer for them.
(Assignee)

Comment 19

3 years ago
So apologies for more UX but does this sound reasonable Jacqueline / Amy? on the "Create bug" view we need a checkbox like UX so the user can see and choose which files are to be uploaded.
Flags: needinfo?(jsavory)
Flags: needinfo?(amlee)
Whiteboard: [spark] → [spark][systemsfe]
Note, this would affect bug 1159969 if we change things like this, wouldn't it?  We won't get the information of the build from the log, so it means that the bug is even more important...
(Assignee)

Comment 22

3 years ago
If we find a fix for 1149012 then application.ini and properties.log are less needed, thats the only effect on this currently

Comment 23

3 years ago
Spark triage team agree this is Spark blocker since we would be sharing private data.
blocking-b2g: spark? → spark+
Created attachment 8621724 [details]
Selected_Files_Bzlite.pdf

Hi Dale, I've attached a mini spec that shows how I think the log files should be attached to the bug. 

I did try changing the attached files from an 'x' to a checkbox as you suggested, but I think it just isn't clear to users what is happening. The way I've designed is another step, but I feel that it is overall easier to understand.

I wasn't sure which files should be default set to checked on, so feel free to set which ones are most important to have automatically checked.
Flags: needinfo?(jsavory)
(Assignee)

Comment 25

3 years ago
Ok yeh this seems pretty clear to the user, working on it now, thanks
Comment on attachment 8623880 [details] [review]
https://github.com/mozilla-b2g/bzlite/pull/23

Sorry for the delay. I left a few comments on the PR, but overall looks fine.
Attachment #8623880 - Flags: review?(drs) → review+
(Assignee)

Comment 28

3 years ago
No bother cheers for the review, addressed the comments and rebased

https://github.com/mozilla-b2g/bzlite/commit/ef6488ecd68b9a3e68b16cb62f407c135ee250be
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(nhirata.bugzilla)
Flags: needinfo?(amlee)
Resolution: --- → FIXED
blocking-b2g: spark+ → 2.5+
status-b2g-v2.5: --- → fixed
status-b2g-master: --- → fixed
Target Milestone: --- → FxOS-S1 (26Jun)
status-b2g-v2.5: fixed → ---
You need to log in before you can comment on or make changes to this bug.