Idea for a solution:

When an affiliated session is entered, keep
- Alternative 1: the serial number
- Alternative 2: all machine-info contents
in /tmp/ until the session ends.

Delete the file if a non-affiliated session is entered (tbd: how to determine this from the startup script -- or move the responsibility to delete the file into chrome).
Make sure websites/extensions/apps can't access /tmp/ paths (they shouldn't be able to due to the URL filter for chromeos, but I'd double-check).

WDYT?

Mattias/Thiemo - do we still need to remove the machine-info file when we enter a user session? Seems like we've now established it's acceptable from a privacy standpoint to expose this info to affiliated user sessions.

Pavol: we only write machine-info on *device start*? What if I sign in to an unaffiliated user, then sign out, then sign in to an affiliated user - will the first signin/sign out remove the serial number which means the second signin won't have access to the serial number?

Sorry, this was not precise. The exact event is:

# This starts every time the UI (re)starts in order to restore
# /tmp/machine-info if needed.
start on starting ui

It is definitely written again when entering the sign-in screen after sign-out.

Got it. In any case, removing the file for an unaffiliated user (from within chrome) sgtm.

Now that we allow use of some parts of machine-info (serial number) within the session but not other parts (disk serial number) we need to reconsider the design to ensure protection for the other parts.

Maybe the easiest approach would be moving the serial number to another place (session_manager?) and reinstating the previous logic of deleting /tmp/machine-info at session start?

(Background: /tmp/machine-info was added in  issue 201608 . My interpretation of that bug is that a file-based approach to broker access to the serial number was chosen for simplicity and there's no a priori reason why another approach (such as a daemon) wouldn't be permissible.)

Note that the logic of deleting /tmp/machine-info still exists, and is the main reason why this bug is happening :-)

The serial number is usually available during the user session because chrome reads it in at startup (which is usually the sign-in screen), and keeps it in memory (in the StatisticsProvider singleton). So even after the file is deleted, if chrome doesn't restart, it can still be accessed through StatisticsProvider.

Thiemo, are you effectively proposing that
- chrome stops accessing /tmp/machine-info
- session_manager should read in /tmp/machine-info (or get the VPD data in some other way) and provide a DBus call for chrome to retrieve the key-value mappings
- session_manager can then filter these after user session start
?



By the way, I've checked who uses StatisticsProvider potentially after session start and for which values.

VALUE "hardware_class", "devsw_boot" - OK
These are determined by running /usr/bin/crossystem and parsing the output, so it's fine.

VALUES "ubind_attribute"/"gbind_attribute" - OK
This seems to come from /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt.
That file is explicitly created for this purpose by dump_vpd_log - see https://chromium-review.googlesource.com/c/chromiumos/platform/vpd/+/18921 .

VALUE "rlz_brand_code" (used by google_brand_chromeos.cc) - PROBABLY OK
This seems to come from /tmp/machine-info, but it seems to be only used on sign-in screen:
google_brand_chromeos.cc writes it into pref kRLZBrand which is used by UserSessionManager.

VALUE "region" - PROBABLY NOT OK
- about_handler.cc (FindRegulatoryLabelDir)
This seems to come from /tmp/machine-info, so it could be an issue.

VALUE "customization_id" - PROBABLY NOT OK
- info_private_api.cc (ChromeosInfoPrivateGetFunction::GetValue)
This seems to come from /tmp/machine-info, so it could be an issue.

VALUE "serial_number" - NOT OK
- enterprise_device_attributes_api.cc (EnterpriseDeviceAttributesGetDeviceSerialNumberFunction::Run)
- hostname_handler.cc (HostnameHandler::OnDeviceHostnamePropertyChanged)
This is in /tmp/machine-info and is an issue.


So the summary is:
- if we do the session_manager thing, we can get rid of the vpd_echo.txt mechanism.
  Or we can re-use the vpd_echo.txt mechanism for things that should be available after user session start.
- we could have additional issues after chrome restart with region/customization_id and other uses of serial_number like hostname_handler.cc.

Stephen/Roger are running into this problem for RLZ, too. Stephen has pointed out that there is a whitelist [1] which controls the contents of /mnt/stateful_partition/unencrypted/cache/vpd/filtered.txt.

rlz_brand_code, region, and customization_id are already part of that whitelist and therefore OK, but we should add 

Product_S/N
serial_number

for enterprise and

rlz_embargo_end_date
should_send_rlz_ping

for RLZ.

I'll prepare a CL.

[1] https://cs.corp.google.com/chromeos_public/src/platform/vpd/util/dump_vpd_log?rcl=0fe4d9c7c0515a0423844e8369b68183ae0755fb&l=162

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vpd/+/17fd16b23e36148d437014463c08d4356f81d55d

commit 17fd16b23e36148d437014463c08d4356f81d55d
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Tue May 08 11:58:01 2018

dump_vpd_log: Add to filter whitelist

Include more VPD keys in the filtered file to support  crbug.com/813015 
and crbug.com/820800 launches:
  Product_S/N
  serial_number
  should_send_rlz_ping
  rlz_embargo_end_date

BRANCH=none
BUG= chromium:823724 , chromium:813015 , chromium:839910 
TEST=none

Change-Id: Iccbcfaf971aa5b77b71d19c30fcd29fa2b4dd0be
Reviewed-on: https://chromium-review.googlesource.com/1041965
Commit-Ready: Thiemo Nagel <tnagel@chromium.org>
Tested-by: Thiemo Nagel <tnagel@chromium.org>
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>

[modify] https://crrev.com/17fd16b23e36148d437014463c08d4356f81d55d/util/dump_vpd_log

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0

commit 7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Thu May 10 15:55:25 2018

StatisticsProvider: Update comments, rename constant

Having already enrolled the device is no reason for failing to obtain
the serial number anymore (https://crrev.com/c/442464).

At that occasion, also fix inconsistencies in the description of the
serial number logic and append ForTest to the name of the
kSerialNumberKey constant.

TBR=emaxx

Bug:  823724 
Change-Id: Ic129f4e1141843b695e6f079d684bfd53bdb3810
Reviewed-on: https://chromium-review.googlesource.com/1051770
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557536}
[modify] https://crrev.com/7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0/chrome/browser/chromeos/login/wizard_controller_browsertest.cc
[modify] https://crrev.com/7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0/chrome/browser/chromeos/policy/device_cloud_policy_initializer_unittest.cc
[modify] https://crrev.com/7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0/chrome/browser/chromeos/policy/device_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc
[modify] https://crrev.com/7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc
[modify] https://crrev.com/7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0/chromeos/system/statistics_provider.cc
[modify] https://crrev.com/7d3d2d7f7e69014dd57ea13ca2ff50af3ac8aaf0/chromeos/system/statistics_provider.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vpd/+/7981ec789af5714b9bed80773f68d7879ba44ee8

commit 7981ec789af5714b9bed80773f68d7879ba44ee8
Author: Thiemo Nagel <tnagel@chromium.org>
Date: Thu May 10 20:58:02 2018

dump_vpd_log: Add to filter whitelist

Include more VPD keys in the filtered file to support  crbug.com/813015 
and crbug.com/820800 launches:
  Product_S/N
  serial_number
  should_send_rlz_ping
  rlz_embargo_end_date

BRANCH=none
BUG= chromium:823724 , chromium:813015 , chromium:839910 
TEST=none

Change-Id: Iccbcfaf971aa5b77b71d19c30fcd29fa2b4dd0be
Reviewed-on: https://chromium-review.googlesource.com/1041965
Commit-Ready: Thiemo Nagel <tnagel@chromium.org>
Tested-by: Thiemo Nagel <tnagel@chromium.org>
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
(cherry picked from commit 17fd16b23e36148d437014463c08d4356f81d55d)
Reviewed-on: https://chromium-review.googlesource.com/1054228
Commit-Queue: Thiemo Nagel <tnagel@chromium.org>

[modify] https://crrev.com/7981ec789af5714b9bed80773f68d7879ba44ee8/util/dump_vpd_log