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

Issue 670985 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: May 2017
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

arc-networkd security hardening

Project Member Reported by cernekee@chromium.org, Dec 3 2016

Issue description

Currently this daemon runs with full root privileges.  ArcIpConfig does require root-equivalent capabilities, but the rest of the program does not.  Use separate processes to enforce privilege separation.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/portage-stable/+/80295d90a050b4664d7888fbeb022189c817592a

commit 80295d90a050b4664d7888fbeb022189c817592a
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sun Dec 04 06:11:10 2016

libndp: upgraded package to upstream

Upgraded net-libs/libndp to version 1.6 on arm, x86

BUG= chromium:670985 
TEST=use arc-networkd to obtain an IPv6 address

Change-Id: I938938d610f8d666355e7fccdc8cccecac056c2b
Reviewed-on: https://chromium-review.googlesource.com/416498
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[rename] https://crrev.com/80295d90a050b4664d7888fbeb022189c817592a/net-libs/libndp/libndp-1.6.ebuild
[modify] https://crrev.com/80295d90a050b4664d7888fbeb022189c817592a/net-libs/libndp/Manifest

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/cc76f1fa5bacf73c410d27fe4995650a77d5407a

commit cc76f1fa5bacf73c410d27fe4995650a77d5407a
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sun Dec 04 01:40:18 2016

shill: Add dbus ACLs for arc-networkd user

Currently arc-networkd makes a couple of queries as root, but it is
being changed to run most of the daemon as an unprivileged user.
Allow it to make GetProperties queries.

BUG= chromium:670985 
TEST=run the updated arc-networkd with and without the change
CQ-DEPEND=CL:416377

Change-Id: Ic730943ed6ea9f220c6f709f10be60b7ad6ae379
Reviewed-on: https://chromium-review.googlesource.com/416256
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/cc76f1fa5bacf73c410d27fe4995650a77d5407a/shims/org.chromium.flimflam.conf

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/cc76f1fa5bacf73c410d27fe4995650a77d5407a

commit cc76f1fa5bacf73c410d27fe4995650a77d5407a
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sun Dec 04 01:40:18 2016

shill: Add dbus ACLs for arc-networkd user

Currently arc-networkd makes a couple of queries as root, but it is
being changed to run most of the daemon as an unprivileged user.
Allow it to make GetProperties queries.

BUG= chromium:670985 
TEST=run the updated arc-networkd with and without the change
CQ-DEPEND=CL:416377

Change-Id: Ic730943ed6ea9f220c6f709f10be60b7ad6ae379
Reviewed-on: https://chromium-review.googlesource.com/416256
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/cc76f1fa5bacf73c410d27fe4995650a77d5407a/shims/org.chromium.flimflam.conf

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 6 2016

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

commit a0d03d95c07777052dd1e47407c9c13126cabea8
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sun Dec 04 01:16:33 2016

chromeos-base/arc-networkd: Add protobuf dep and new user/group

These are needed to implement privilege separation in the daemon.

BUG= chromium:670985 
TEST=manually verify /etc/{passwd,group}
CQ-DEPEND=CL:416272,CL:416264

Change-Id: I9e6698365ab43f82b869c7afad7436ee55ae8af6
Reviewed-on: https://chromium-review.googlesource.com/416377
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/a0d03d95c07777052dd1e47407c9c13126cabea8/chromeos-base/arc-networkd/arc-networkd-9999.ebuild

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 6 2016

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

commit 87654a58d0804113693fd5a00cb44bbb008d3177
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sun Dec 04 17:03:40 2016

security_AccountsBaseline: add new arc-networkd account

This is required for arc-networkd privilege separation.

BUG= chromium:670985 
TEST=run test_that against veyron_minnie

Change-Id: I45a968b2389ce1858ca9617b93ac5ac67ccafe4d
Reviewed-on: https://chromium-review.googlesource.com/416264
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/87654a58d0804113693fd5a00cb44bbb008d3177/client/site_tests/security_AccountsBaseline/baseline.group
[modify] https://crrev.com/87654a58d0804113693fd5a00cb44bbb008d3177/client/site_tests/security_AccountsBaseline/baseline.passwd

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/firewalld/+/3d8e6ae763671ec2e7160e0cb93cf358865f7851

commit 3d8e6ae763671ec2e7160e0cb93cf358865f7851
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sun Dec 04 05:56:02 2016

Check the return value from DropRoot()

Unlike the other brillo::Minijail methods, this one can fail if the
username lookup does not succeed.

BUG= chromium:670985 
TEST=buildbots

Change-Id: Id1d5059fb155c1a227033af0eba837ab37e96fe9
Reviewed-on: https://chromium-review.googlesource.com/416302
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/3d8e6ae763671ec2e7160e0cb93cf358865f7851/iptables.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 6 2016

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

commit a783b6ab7c29b789fdada12a0547bcd5550ffd7e
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sun Dec 04 01:13:18 2016

Add arc-networkd user

This allows arc-networkd to drop root privileges.

BUG= chromium:670985 
TEST=`emerge-veyron_minnie arc-networkd` with updated ebuild

Change-Id: I0b3fb3d1f607ca20ab7270173f341b77f33749e0
Reviewed-on: https://chromium-review.googlesource.com/416272
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[add] https://crrev.com/a783b6ab7c29b789fdada12a0547bcd5550ffd7e/profiles/base/accounts/group/arc-networkd
[add] https://crrev.com/a783b6ab7c29b789fdada12a0547bcd5550ffd7e/profiles/base/accounts/user/arc-networkd

Project Member

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

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

commit 70398d4d66401f7dab4d7acad0ba87f1944eeec1
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sun Dec 04 05:49:07 2016

arc-networkd: Check the return value from DropRoot()

Surprise!  Unlike the other methods, this one can fail if the username
lookup does not succeed.

(I copied this code from firewalld, which also needs to be fixed.)

BUG= chromium:670985 
TEST=run arc-networkd, make sure it passes the assertion

Change-Id: I96c8c6c1aa43ceb1dbc747e8536b76767db52cd6
Reviewed-on: https://chromium-review.googlesource.com/416263
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/70398d4d66401f7dab4d7acad0ba87f1944eeec1/arc-networkd/arc_ip_config.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 6 2016

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

commit 4e62cc191d197258caaa62ebbac4acac3f2601fa
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Dec 03 19:50:53 2016

arc-networkd: Move Manager::Options into a separate class

These will be shared with other modules, and keeping them in manager.h
causes recursive header inclusion.

BUG= chromium:670985 
TEST=compile-tested only

Change-Id: I11998dfd888a7374e16814ab51ee9cdeb8d8a38d
Reviewed-on: https://chromium-review.googlesource.com/416360
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Abhishek Bhardwaj <abhishekbh@google.com>

[add] https://crrev.com/4e62cc191d197258caaa62ebbac4acac3f2601fa/arc-networkd/options.h
[modify] https://crrev.com/4e62cc191d197258caaa62ebbac4acac3f2601fa/arc-networkd/main.cc
[modify] https://crrev.com/4e62cc191d197258caaa62ebbac4acac3f2601fa/arc-networkd/manager.cc
[modify] https://crrev.com/4e62cc191d197258caaa62ebbac4acac3f2601fa/arc-networkd/manager.h

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 9 2016

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

commit 27bcaa63852cca67adf953e7ef783c6d497692c1
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Sat Dec 03 19:16:26 2016

arc-networkd: Implement privilege separation for ArcIpConfig

ArcIpConfig requires CAP_SYS_ADMIN, CAP_NET_ADMIN, and
CAP_DAC_READ_SEARCH, the most dangerous capabilities used by the daemon.
Fork a helper subprocess to handle changing the IP configuration and
manipulating the namespace, and then let Manager drop privileges
before it starts handling dbus/ndp/multicast duties.

Manager will still run with CAP_NET_RAW, as an unprivileged user.  Future
work may involve splitting NDP handling into a less-privileged subprocess
to limit the extent of damage if it is subverted.  The new HelperProcess
class can be used for additional subprocesses.

BUG= chromium:670985 
TEST=manually verify IPv6/multicast functionality
TEST=manually verify creds via /proc/<pid>/status
TEST=`kill -11` the main/sub processes and ensure that both die
CQ-DEPEND=CL:416377,CL:416256

Change-Id: I2ddec4ffbf516b583762b0405909a14088552fe1
Reviewed-on: https://chromium-review.googlesource.com/416361
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/arc-networkd.gyp
[modify] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/manager.cc
[modify] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/manager.h
[add] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/ipc.proto
[add] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/helper_process.h
[modify] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/main.cc
[add] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/ip_helper.cc
[add] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/ip_helper.h
[add] https://crrev.com/27bcaa63852cca67adf953e7ef783c6d497692c1/arc-networkd/helper_process.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 9 2016

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

commit b2c0c834e6dc56ac12f125f244c18c25b5c187ab
Author: Kevin Cernekee <cernekee@chromium.org>
Date: Tue Dec 06 19:47:57 2016

arc-networkd: Add missing <utility> header file

This fixes a cpplint error.

BUG= chromium:670985 
TEST=compile tested

Change-Id: Ifefbfca289dd3340b20502063993ceea8293cf83
Reviewed-on: https://chromium-review.googlesource.com/417135
Commit-Ready: Kevin Cernekee <cernekee@chromium.org>
Tested-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Abhishek Bhardwaj <abhishekbh@google.com>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/b2c0c834e6dc56ac12f125f244c18c25b5c187ab/arc-networkd/multicast_socket.cc
[modify] https://crrev.com/b2c0c834e6dc56ac12f125f244c18c25b5c187ab/arc-networkd/multicast_forwarder.cc

Status: Fixed (was: Untriaged)
Labels: VerifyIn-61

Comment 14 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment