New issue
Advanced search Search tips

Issue 640401 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature

Blocking:
issue 644347



Sign in to add a comment

crosh: rework loading of additional files

Project Member Reported by vapier@chromium.org, Aug 23 2016

Issue description

we want to make it easier to extend crosh, especially for non-traditional devices (so on a per-board basis).  simple design:

- add a func that will load all [0-9][0-9]-*.sh files in a given directory
- whenever a new file is found & loaded, log it to stdout for visibility
- run func against /usr/share/cros/crosh/extra.d/
- run func against /usr/share/cros/crosh/dev.d/ if device is in dev mode
- run func against /usr/share/cros/crosh/removable.d/ if rootfs is removable
- move crosh-dev to dev.d/50-crosh.sh
- move crosh-usb to removable.d/50-usb.sh
- move crosh-cups to dev.d/60-cups.sh

now boards can install their own fragments into xxx.d/ and crosh will DTRT.

we are concerned about security and people being able to extend crosh w/out proper review.  we can also add an autotest that has a whitelist that would apply to all official Chrome OS devices that people would need to update.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 2 2016

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

commit b60131f760a4c08254763a29073d71668face795
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Aug 30 20:17:50 2016

crosh: hoist cli parsing up to the main func

This keeps the option parsing logic in one place and adds standard
--help support.  It also means we get to reject unknown options.

BUG= chromium:640401 
TEST=crosh on device still works
TEST=running `./crosh` with --dev/--usb/--help/--asdf DTRT

Change-Id: I96aca8733b58141d2a349497ed99b7c35e7a0079
Reviewed-on: https://chromium-review.googlesource.com/378775
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/b60131f760a4c08254763a29073d71668face795/crosh/crosh

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 2 2016

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

commit 4fc4a85e83e85900db721a2a98792d02be063a7e
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Aug 30 21:17:17 2016

crosh: add support for module dirs

We want to let boards add their own sets of functions to crosh w/out
having to modify crosh itself.  Add a set of directories that we will
load blindly.

We don't want this to become a generic mechanism for CrOS for adding
code to crosh due to security/review concerns.  We'll enforce that
via a new autotest.

BUG= chromium:640401 
TEST=running crosh locally loads local fragments dynamically

Change-Id: I77393f2ff5cbd32cb746a0ab8c302d85d01f202a
Reviewed-on: https://chromium-review.googlesource.com/378776
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/4fc4a85e83e85900db721a2a98792d02be063a7e/crosh/crosh

Blocking: 644347
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/e237d974798368f0c41d1ed44ad0f871986586b1

commit e237d974798368f0c41d1ed44ad0f871986586b1
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Aug 30 21:39:43 2016

autotest-tests-security: enable new crosh security test

BUG= chromium:640401 
TEST=precq passes
CQ-DEPEND=CL:378815

Change-Id: Ie77f9b1d43a98e436b25942c7bce5a927e120a6e
Reviewed-on: https://chromium-review.googlesource.com/378835
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/e237d974798368f0c41d1ed44ad0f871986586b1/chromeos-base/autotest-tests-security/autotest-tests-security-9999.ebuild

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/306007f29cf53ee912f12bcb951852b10254a4a7

commit 306007f29cf53ee912f12bcb951852b10254a4a7
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Aug 30 21:40:32 2016

security_CroshModules: new security test

Make sure we keep control of crosh modules that get installed.

BUG= chromium:640401 
TEST=precq passes
TEST=running test normally locally passes
TEST=running test locally w/bad files fails the test & lists the bad files
CQ-DEPEND=CL:378835

Change-Id: I7578ad67746081c744b477c69be8b5dac517a959
Reviewed-on: https://chromium-review.googlesource.com/378815
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/306007f29cf53ee912f12bcb951852b10254a4a7/client/site_tests/security_CroshModules/control
[add] https://crrev.com/306007f29cf53ee912f12bcb951852b10254a4a7/client/site_tests/security_CroshModules/whitelist
[add] https://crrev.com/306007f29cf53ee912f12bcb951852b10254a4a7/client/site_tests/security_CroshModules/security_CroshModules.py

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/4ad2694f8ad4f0a97a303acb2a0598f5ebcf4061

commit 4ad2694f8ad4f0a97a303acb2a0598f5ebcf4061
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Aug 26 19:54:38 2016

crosh: relocate addons to new modules layout

BUG= chromium:640401 
TEST=precq passes
CQ-DEPEND=CL:380755

Change-Id: I1bc755e7d8bf6912fd8cdf8ba6f88bb430da361b
Reviewed-on: https://chromium-review.googlesource.com/380775
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/4ad2694f8ad4f0a97a303acb2a0598f5ebcf4061/chromeos-base/crosh/crosh-9999.ebuild

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 8 2016

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

commit 9249c6b4603ed9b6fcce3820cf37b0d1c6e99cb3
Author: Mike Frysinger <vapier@chromium.org>
Date: Tue Aug 30 21:42:52 2016

crosh: relocate addons to new modules layout

This makes it easier to extend crosh w/out modifying crosh itself.

BUG= chromium:640401 
TEST=precq passes
CQ-DEPEND=CL:380775

Change-Id: Ice88b5308185c453f92e8a412b0ae451e3e20cbe
Reviewed-on: https://chromium-review.googlesource.com/380755
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/9249c6b4603ed9b6fcce3820cf37b0d1c6e99cb3/crosh/run_tests.sh
[rename] https://crrev.com/9249c6b4603ed9b6fcce3820cf37b0d1c6e99cb3/crosh/dev.d/50-crosh.sh
[rename] https://crrev.com/9249c6b4603ed9b6fcce3820cf37b0d1c6e99cb3/crosh/extra.d/30-cups.sh
[modify] https://crrev.com/9249c6b4603ed9b6fcce3820cf37b0d1c6e99cb3/crosh/crosh
[rename] https://crrev.com/9249c6b4603ed9b6fcce3820cf37b0d1c6e99cb3/crosh/removable.d/50-crosh.sh

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 8 2016

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

commit f36a54508065ad93de749f9dd261c2cdba182629
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Aug 31 04:30:53 2016

crosh: document it!

BUG= chromium:640401 
TEST=reviewed output in gitiles

Change-Id: I12bb15fb6c8d6bfc459cca557a18b0a8bdde9d5b
Reviewed-on: https://chromium-review.googlesource.com/378796
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Owain Davies <otjdavies@chromium.org>

[add] https://crrev.com/f36a54508065ad93de749f9dd261c2cdba182629/crosh/README.md

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 8 2016

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

commit 35bc4ddd26a922ed29a01f0660d006243817fbfd
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Sep 08 18:36:19 2016

crosh: fix +x bits on README.md

Not sure how these got set, but def wrong, and messes up rendering.

BUG= chromium:640401 
TEST=reviewed output in gitiles

Change-Id: I82c7fde836fa3dce5dc1b7c414ee989380d97cce
Reviewed-on: https://chromium-review.googlesource.com/383191
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/35bc4ddd26a922ed29a01f0660d006243817fbfd/crosh/README.md

Labels: VerifyIn-55

Comment 12 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56

Comment 13 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 14 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 15 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 16 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 18 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)
Status: Fixed (was: Archived)

Sign in to add a comment