New issue
Advanced search Search tips

Issue 916297 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Files app should ask for container username

Project Member Reported by smbar...@chromium.org, Dec 18

Issue description

Users can rename their container user. The only thing that matters for us is that the user is uid 1000. If you do this now, most features work except for the Files app, which I think is still using the email-derived username.

The Files app should ask cicerone for the container username before setting up sshfs: https://chromium.googlesource.com/chromiumos/platform2/+/master/system_api/dbus/vm_cicerone/cicerone_service.proto#652
 
Cc: nverne@chromium.org
I can eventually look at this, but if someone in nverne@ team wants to do it, all the changes are in CrostiniManager.

username is currently being set at
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/crostini/crostini_manager.cc?l=1149&rcl=d86b336f22131fdd8d65e57cd09178ae26ababa4

Then used for sshfs at https://cs.chromium.org/chromium/src/chrome/browser/chromeos/crostini/crostini_manager.cc?l=360&rcl=d86b336f22131fdd8d65e57cd09178ae26ababa4

Would it make sense to add the username to the StartLxdContainerResponse or in ContainerStartedSignal?
Labels: -Pri-3 M-73 Pri-2
Owner: nverne@chromium.org
Status: Assigned (was: Untriaged)
Status: Fixed (was: Assigned)
I've been looking at this this morning.  I think the best solution is for cicerone to return username in ContainerStartedSignal.  The username is already fetched in this code.

http://cs/chromeos_public/src/platform2/vm_tools/cicerone/service.cc?l=468&rcl=cf65e45c51a11ebd14f39daf0be997f5a0fd90ba

CrostiniManager can then store the username in running_containers_
https://cs.chromium.org/chromium/src/chrome/browser/chromeos/crostini/crostini_manager.cc?l=1401&rcl=6d12f848f28a9f28546410d6027087c413639e24
Owner: joelhockey@chromium.org
Status: Started (was: Fixed)
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2c6f3ee9680b60d4e224742245127dfcdccc98d3

commit 2c6f3ee9680b60d4e224742245127dfcdccc98d3
Author: Joel Hockey <joelhockey@chromium.org>
Date: Sun Dec 23 22:46:10 2018

vm_tools: return username in ContainerStartedSignal, SetUpLxdContainerUser

Return username in cicerone ContainerStartedSignal and
SetUpLxdContainerUser, and in tremplin SetUpUser.

This allows users to have a different username than
the default generated in chrome.  The username is used
by FilesApp when doing sshfs mount.

BUG= chromium:916297 
TEST=Install on device, test with FilesApp

Change-Id: If510e5063115fe85dd74869a14f8ba51764f9e66
Reviewed-on: https://chromium-review.googlesource.com/1388084
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/2c6f3ee9680b60d4e224742245127dfcdccc98d3/vm_tools/cicerone/service.cc
[modify] https://crrev.com/2c6f3ee9680b60d4e224742245127dfcdccc98d3/vm_tools/cicerone/virtual_machine.cc
[modify] https://crrev.com/2c6f3ee9680b60d4e224742245127dfcdccc98d3/vm_tools/proto/tremplin.proto
[modify] https://crrev.com/2c6f3ee9680b60d4e224742245127dfcdccc98d3/system_api/dbus/vm_cicerone/cicerone_service.proto
[modify] https://crrev.com/2c6f3ee9680b60d4e224742245127dfcdccc98d3/vm_tools/cicerone/virtual_machine.h

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tremplin/+/7c1768fec5f1cc64db4921b857634dc94b021491

commit 7c1768fec5f1cc64db4921b857634dc94b021491
Author: Joel Hockey <joelhockey@chromium.org>
Date: Mon Dec 24 07:52:05 2018

tremplin: Return username in SetUpUser and always enable-linger

When uid 1000 already exists, return username in SetUpUser.

Always set enable-linger for user whether exists or not.

BUG= chromium:916297 
TEST=Change username, start container, verify FilesApp sshfs
CQ-DEPEND=CL:1388084

Change-Id: Ia2defca433dcaedadef0713dadc7f621b16596b6
Reviewed-on: https://chromium-review.googlesource.com/1387886
Commit-Ready: Joel Hockey <joelhockey@chromium.org>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/7c1768fec5f1cc64db4921b857634dc94b021491/src/chromiumos/tremplin/tremplin.go

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 31

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

commit 7631a97395e88a9a4c2b880d540deb0626fe3078
Author: Joel Hockey <joelhockey@chromium.org>
Date: Mon Dec 31 22:06:38 2018

Return container_username in Fake Cicerone ContainerStarted

Bug:  916297 
Change-Id: I9cebd2e2077b47ae2221b918a11f475b586a48b8
Reviewed-on: https://chromium-review.googlesource.com/c/1392690
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619319}
[modify] https://crrev.com/7631a97395e88a9a4c2b880d540deb0626fe3078/chromeos/dbus/fake_cicerone_client.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/tremplin/+/eb6f2ff57ffeca5b68b823dd64d3ce000319e67e

commit eb6f2ff57ffeca5b68b823dd64d3ce000319e67e
Author: Joel Hockey <joelhockey@chromium.org>
Date: Thu Jan 03 23:04:07 2019

tremplin: return homedir in GetContainerUsername

BUG= chromium:916297 
TEST=Change username homedir, verify FilesApp open linux files.
CQ-DEPEND=CL:1393139

Change-Id: Icfa0debfdd34f8737e4b169f99ceb63c57fbe6d8
Reviewed-on: https://chromium-review.googlesource.com/1392863
Commit-Ready: Joel Hockey <joelhockey@chromium.org>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/eb6f2ff57ffeca5b68b823dd64d3ce000319e67e/src/chromiumos/tremplin/tremplin.go

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/b3983997e350ee832af34053a8116abb24d82e70

commit b3983997e350ee832af34053a8116abb24d82e70
Author: Joel Hockey <joelhockey@chromium.org>
Date: Thu Jan 03 23:04:11 2019

vm_tools: return homedir in ContainerStartedSignal

BUG= chromium:916297 
TEST=Install on device, test with FilesApp

Change-Id: I5e4bc81d8e33d347e1e65481aaf8cf429df7cfdb
Reviewed-on: https://chromium-review.googlesource.com/1392579
Commit-Ready: Joel Hockey <joelhockey@chromium.org>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/b3983997e350ee832af34053a8116abb24d82e70/vm_tools/cicerone/service.cc
[modify] https://crrev.com/b3983997e350ee832af34053a8116abb24d82e70/vm_tools/cicerone/virtual_machine.cc
[modify] https://crrev.com/b3983997e350ee832af34053a8116abb24d82e70/vm_tools/cicerone/virtual_machine.h

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/59861fa32e0c2863b0be461926e1f3526167693c

commit 59861fa32e0c2863b0be461926e1f3526167693c
Author: Joel Hockey <joelhockey@chromium.org>
Date: Thu Jan 03 23:04:06 2019

system_api: Return homedir in ContainerStartedSignal

BUG= chromium:916297 
TEST=Install on device, test with FilesApp

Change-Id: Ibd6ef8ccabc9902c2c0a4b74af4a73f656575c01
Reviewed-on: https://chromium-review.googlesource.com/1393139
Commit-Ready: Joel Hockey <joelhockey@chromium.org>
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>

[modify] https://crrev.com/59861fa32e0c2863b0be461926e1f3526167693c/vm_tools/proto/tremplin.proto
[modify] https://crrev.com/59861fa32e0c2863b0be461926e1f3526167693c/system_api/dbus/vm_cicerone/cicerone_service.proto

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 4

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

commit 681e82b3abe371132b0314ea7524c74278db27bc
Author: Joel Hockey <joelhockey@chromium.org>
Date: Fri Jan 04 07:43:12 2019

Use username from cicerone when doing sshfs for crostini mount

Chrome creates the container with default username derived from
the email address of the current logged in profile.
E.g. test.user@gmail.com gets username 'testuser'.

It is possible for users to change the username inside the container.
As long as the user keeps uid=1000, this does not cause issues
except for the sshfs mount where FilesApp connects to the container
using 'sshfs://username@hostname'.

Cicerone is now updated to return the container username in
ContainerStartedSignal and in SetUpLxdContainerUser.  CrostiniManager
can now use this for the sshfs mount.

Removed crostini_util ContainerHomeDirectoryForProfile which was
using the default username.  The container homedir is now available
in CrostiniManager::GetContainerInfo.

Bug:  916297 
Change-Id: I27f41647c3b8809170521d741867a8ed430e5476
Reviewed-on: https://chromium-review.googlesource.com/c/1388124
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619897}
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/crostini/crostini_manager.cc
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/crostini/crostini_manager.h
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/crostini/crostini_manager_unittest.cc
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/crostini/crostini_share_path.cc
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/crostini/crostini_share_path_unittest.cc
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/crostini/crostini_util.cc
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/crostini/crostini_util.h
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/file_manager/path_util.cc
[modify] https://crrev.com/681e82b3abe371132b0314ea7524c74278db27bc/chrome/browser/chromeos/file_manager/path_util_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment