New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 916051 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Last visit 28 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

chapsd: move debugging state from files in /home/chronos/ to dbus callbacks

Project Member Reported by kroot@google.com, Dec 18

Issue description

Chrome Version: 73.0.3637.0
Chrome OS Version: 11390.0.2018_12_12_1426
Chrome OS Platform: samsung-caroline
Network info: N/A

In src/platform2/crosh/crosh the setup for chaps debugging is by touching files in /home/chronos specifically /home/chronos/.chaps_debug_1 and /home/chronos/.chaps_debug_2. /home/chronos is owned by chronos:chronos.

In src/platform2/chaps/init/chapsd.sh that runs as root it removes the files created by crosh so that the files act as one-shot flags to enable debug. Since /home/chronos is not writable by a group that root is in, it must use the DAC_OVERRIDE capability to remove those files in the current scheme.

SELinux requires that the "dac_override" capability is explicitly granted for this case. This lowers the overall security by granting excessively broad permissions to the domain. We need some kind of change to obviate the need to grant "dac_override" to chapsd.

This could be by creating a subdirectory somewhere which has group writability for chronos-access. Both "chronos" and "root" are members of this group thereby allowing root to rm files in that directory.
 
Cc: f...@chromium.org vapier@chromium.org
Cc: apronin@chromium.org dkrahn@chromium.org ejcaruso@chromium.org
Components: OS>Systems>Security
Summary: chapsd: move debugging state from files in /home/chronos/ to dbus callbacks (was: chaps debugging method requires DAC_OVERRIDE)
the logic needs to get rewritten in general, but we haven't prioritized it.  best case would be to delete it all.

if we think we still want it, we should add some dbus methods to chapsd itself that crosh can call to set state.  that way chapds owns it all.

how chapds wants to maintain that state is up to it.  prob easiest port would be to add flag files under /var/lib/chaps/ instead.

i wouldn't worry about migration.  can just update the init script to rm the old files in the pre-startup phase for a few releases.
Cc: -dkrahn@chromium.org louiscollard@chromium.org
Labels: Cros-Hwsec-Ready
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 10

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

commit a1e874b00575adeb2947af59a202a3495732c1da
Author: Qijiang Fan <fqj@chromium.org>
Date: Thu Jan 10 03:52:12 2019

chaps: store client set log level for next run.

chaps_client --set_log_level will ask chapsd to store log level to
/var/lib/chaps/.log_level for only the next time when chapsd starts.

BUG=chromium:916051
TEST=sudo -u chronos chaps_client --set_log_level=-2
TEST=cat /var/lib/chaps/.log_level; reboot;

Change-Id: I01fce5c7a60a821fb8740ac3459d247caa363ef2
Reviewed-on: https://chromium-review.googlesource.com/1400245
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Qijiang Fan <fqj@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a1e874b00575adeb2947af59a202a3495732c1da/chaps/chaps_adaptor.cc
[modify] https://crrev.com/a1e874b00575adeb2947af59a202a3495732c1da/chaps/chaps_adaptor.h
[modify] https://crrev.com/a1e874b00575adeb2947af59a202a3495732c1da/chaps/init/chapsd.sh
[modify] https://crrev.com/a1e874b00575adeb2947af59a202a3495732c1da/chaps/chapsd.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 10

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

commit f8bae61dc2eab4c8eadbb7f2b906c4ec45dd2b43
Author: Qijiang Fan <fqj@chromium.org>
Date: Thu Jan 10 21:58:44 2019

crosh: no need to save a .chaps_debug_* file by crosh

chapsd will save log level to /var/lib/chaps/.log_level

BUG=chromium:916051
TEST=None
CQ-DEPEND=CL:1400245

Change-Id: I97ec70228a41301c6b49f2c1475ee596442f4a05
Reviewed-on: https://chromium-review.googlesource.com/1400246
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Qijiang Fan <fqj@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

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

Owner: f...@chromium.org
Status: Started (was: Untriaged)

Sign in to add a comment