authpolicy: Parse process outputs in sandboxed environment |
|||||
Issue description- Create exe that parses ProcessExecutor output - Call that exe, wrapped in minijail - Convert output to protobuf - Parse protobuf in authpolicy
,
Nov 22 2016
Mattias, on https://chromium-review.googlesource.com/#/c/405427/ you write: > There's a lot of parsing going on in samba_interface.cc. Ideally, that parsing > would also take place in the more limited sandbox. Although we'd still have to > communicate results back to authpolicyd. Writing results in proto format to > stdout might be a reasonable approach? I don't get how that would significantly reduce attack surface. Afaics, the danger of parsing in authpolicyd would be exposing the D-Bus interface to session manager to an attacker. Yet passing a proto does exactly the same thing since authpolicyd pushes that proto through the D-Bus interface to session manager. Am I missing something?
,
Nov 23 2016
The point here is that parsing untrusted input is a particularly risky thing to do. Experience shows that parsing code is a popular and worthwhile attack target. Hence, my desire to move any ad-hoc parsing (including "split-that-string-into-lines-then-look-for-a-token-named-such-and-such-split-the-value-to-extract-fields") that you need to perform out of the privileged authpolicyd process. Your observation is correct that we still need to parse any data we consume. However, we can do that using parsers that are less ad-hoc and have been hardened over the years to the point where we consider them practically bullet-proof (hence the proto suggestion). So an attacker that manages to exploit the samba side would get the ability push fake policy, but wouldn't get the ability to talk to dbus directly, or mess with authpolicyd's on-disk state.
,
Nov 23 2016
Also, please do let me know if/when my paranoia level reaches an unbearable point :-D I'm basically going by the principle of building as tight a sandbox around samba / krb as possible.
,
Dec 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/dccfe73232e0b2d4658b228449b5e7c4539c812f commit dccfe73232e0b2d4658b228449b5e7c4539c812f Author: Lutz Justen <ljusten@chromium.org> Date: Tue Dec 06 21:08:15 2016 authpolicy: Add libminijail as dependency Authpolicy is switched to libminijail in CL:417299. This CL adds the ebuild dependency on libminijail. BUG= chromium:666693 TEST=Emerges. Change-Id: I81befd2db0f72169c0d142ccfc1dcfce923d95b6 Reviewed-on: https://chromium-review.googlesource.com/417260 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/dccfe73232e0b2d4658b228449b5e7c4539c812f/chromeos-base/authpolicy/authpolicy-9999.ebuild
,
Dec 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/6d7b900e4f26657ce1b55c66b4381c65ee8832ed commit 6d7b900e4f26657ce1b55c66b4381c65ee8832ed Author: Lutz Justen <ljusten@chromium.org> Date: Tue Dec 06 20:52:57 2016 authpolicy: Switched ProcessExecutor over to libminijail In preparation to proper sandboxing, switches ProcessExecutor over to use libminijail under the hood and exposes a few methods for minijail sandboxing. Fixes unit tests. Some unit tests had slightly different assumptions, for instance libminijail uses environment variables internally, so the list of environment variables is not empty by default as in the old code. BUG= chromium:666693 TEST=Compiles, tested with custom test code, ran unit test. Change-Id: I9fb5fba4989d0461dfa588f7e7b9454664106b15 Reviewed-on: https://chromium-review.googlesource.com/417299 Commit-Ready: Roman Sorokin <rsorokin@chromium.org> Tested-by: Roman Sorokin <rsorokin@chromium.org> Reviewed-by: Roman Sorokin <rsorokin@chromium.org> [add] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/pipe_helper.h [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/process_executor.cc [add] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/pipe_helper.cc [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/authpolicy.gyp [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/process_executor.h [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/process_executor_unittest.cc [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/samba_interface.cc
,
Dec 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/6d7b900e4f26657ce1b55c66b4381c65ee8832ed commit 6d7b900e4f26657ce1b55c66b4381c65ee8832ed Author: Lutz Justen <ljusten@chromium.org> Date: Tue Dec 06 20:52:57 2016 authpolicy: Switched ProcessExecutor over to libminijail In preparation to proper sandboxing, switches ProcessExecutor over to use libminijail under the hood and exposes a few methods for minijail sandboxing. Fixes unit tests. Some unit tests had slightly different assumptions, for instance libminijail uses environment variables internally, so the list of environment variables is not empty by default as in the old code. BUG= chromium:666693 TEST=Compiles, tested with custom test code, ran unit test. Change-Id: I9fb5fba4989d0461dfa588f7e7b9454664106b15 Reviewed-on: https://chromium-review.googlesource.com/417299 Commit-Ready: Roman Sorokin <rsorokin@chromium.org> Tested-by: Roman Sorokin <rsorokin@chromium.org> Reviewed-by: Roman Sorokin <rsorokin@chromium.org> [add] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/pipe_helper.h [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/process_executor.cc [add] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/pipe_helper.cc [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/authpolicy.gyp [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/process_executor.h [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/process_executor_unittest.cc [modify] https://crrev.com/6d7b900e4f26657ce1b55c66b4381c65ee8832ed/authpolicy/samba_interface.cc
,
Dec 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/2958ba4378cae0a9bb21f28c9f4b65822aa55d1c commit 2958ba4378cae0a9bb21f28c9f4b65822aa55d1c Author: Lutz Justen <ljusten@chromium.org> Date: Mon Dec 12 09:27:01 2016 authpolicy: Add authpolicyd-exec user and group The authpolicyd-exec user will be used to execute sandboxed processes from the authpolicy daemon. This is done so that the processes cannot write files that the authpolicyd user can write, e.g. authpolicy configuration files. BUG= chromium:666693 TEST=Emerges, ran security_AccountsBaseline test. CQ-DEPEND=CL:418758 Change-Id: Id3e2387ac41598c9175ebd25bacc53513942a110 Reviewed-on: https://chromium-review.googlesource.com/418717 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Tested-by: Roman Sorokin <rsorokin@chromium.org> Reviewed-by: Roman Sorokin <rsorokin@chromium.org> [modify] https://crrev.com/2958ba4378cae0a9bb21f28c9f4b65822aa55d1c/client/site_tests/security_AccountsBaseline/baseline.group [modify] https://crrev.com/2958ba4378cae0a9bb21f28c9f4b65822aa55d1c/client/site_tests/security_AccountsBaseline/baseline.passwd
,
Dec 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/eclass-overlay/+/16a835cb7c4310e87262d4b0d0d54a7fc464ef25 commit 16a835cb7c4310e87262d4b0d0d54a7fc464ef25 Author: Lutz Justen <ljusten@chromium.org> Date: Mon Dec 12 09:34:03 2016 authpolicy: Add authpolicyd-exec user and group The authpolicyd-exec user will be used to execute sandboxed processes from the authpolicy daemon. This is done so that the processes cannot write files that the authpolicyd user can write, e.g. authpolicy configuration files. BUG= chromium:666693 TEST=Emerges, ran security_AccountsBaseline test. CQ-DEPEND=CL:418717 Change-Id: I139b018ddbf3838b4a50a3047cce406ba3e9b232 Reviewed-on: https://chromium-review.googlesource.com/418758 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Roman Sorokin <rsorokin@chromium.org> [modify] https://crrev.com/16a835cb7c4310e87262d4b0d0d54a7fc464ef25/profiles/base/accounts/group/authpolicyd [add] https://crrev.com/16a835cb7c4310e87262d4b0d0d54a7fc464ef25/profiles/base/accounts/user/authpolicyd-exec [add] https://crrev.com/16a835cb7c4310e87262d4b0d0d54a7fc464ef25/profiles/base/accounts/group/authpolicyd-exec
,
Dec 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/791c69603fc7505059229f01055b526506699f6b commit 791c69603fc7505059229f01055b526506699f6b Author: Lutz Justen <ljusten@chromium.org> Date: Mon Dec 12 10:28:10 2016 authpolicy: Use protobuf configuration file Uses a dedicated configuration file in protobuf format instead of parsing the smb.conf file to retrieve configuration. Writes smb.conf to a temp directory every time it is used. This is more secure (protobuf parsing is battle-tested) and it allows us to update the smb.conf, which was impossible before since it was written once at domain join. BUG= chromium:666693 TEST=Compiles, tested with custom test code. Change-Id: I268e916293d49ff0faf31435a0e39c991790f383 Reviewed-on: https://chromium-review.googlesource.com/418995 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Roman Sorokin <rsorokin@chromium.org> [modify] https://crrev.com/791c69603fc7505059229f01055b526506699f6b/authpolicy/authpolicy.gyp [modify] https://crrev.com/791c69603fc7505059229f01055b526506699f6b/authpolicy/authpolicy.h [modify] https://crrev.com/791c69603fc7505059229f01055b526506699f6b/authpolicy/samba_interface.cc [add] https://crrev.com/791c69603fc7505059229f01055b526506699f6b/authpolicy/proto/authpolicy_containers.proto [modify] https://crrev.com/791c69603fc7505059229f01055b526506699f6b/authpolicy/samba_interface.h [modify] https://crrev.com/791c69603fc7505059229f01055b526506699f6b/authpolicy/errors.h [modify] https://crrev.com/791c69603fc7505059229f01055b526506699f6b/authpolicy/authpolicy_main.cc [modify] https://crrev.com/791c69603fc7505059229f01055b526506699f6b/authpolicy/errors.cc
,
Dec 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/eb012f76da2f7a7b4305b3646d1ab97462e407c7 commit eb012f76da2f7a7b4305b3646d1ab97462e407c7 Author: Lutz Justen <ljusten@chromium.org> Date: Tue Dec 13 14:53:02 2016 authpolicy: Install authpolicy_parser executable The executable is going to be called from the authpolicy daemon in a sandboxed environment. BUG= chromium:666693 TEST=Compiles, tested with custom test code. CQ-DEPEND=CL:419455 Change-Id: I34dd7b1f400b03376aedb63c8ef6afb4e8f5b4f0 Reviewed-on: https://chromium-review.googlesource.com/419435 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Roman Sorokin <rsorokin@chromium.org> [modify] https://crrev.com/eb012f76da2f7a7b4305b3646d1ab97462e407c7/chromeos-base/authpolicy/authpolicy-9999.ebuild
,
Dec 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/16fe63be8711ebf9559d7fcc56d9fa473adaa45b commit 16fe63be8711ebf9559d7fcc56d9fa473adaa45b Author: Lutz Justen <ljusten@chromium.org> Date: Tue Dec 13 14:50:22 2016 authpolicy: Move parsing to a separate process Moves all parsing code to separate processes. Uses protobufs to safely exchange data between the trusted authpolicy process and the untrusted parser process. BUG= chromium:666693 TEST=Compiles, tested with custom test code. CQ-DEPEND=CL:419435 Change-Id: Idf99d7c5868e3751f5720fda48bafcb389e1ed0f Reviewed-on: https://chromium-review.googlesource.com/419455 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Roman Sorokin <rsorokin@chromium.org> [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/policy/device_policy_encoder.cc [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/authpolicy.gyp [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/samba_interface_internal.cc [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/samba_interface.h [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/samba_interface.cc [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/authpolicy.h [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/policy/user_policy_encoder_gen.cc [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/samba_interface_internal_unittest.cc [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/samba_interface_internal.h [add] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/authpolicy_parser_main.cc [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/policy/user_policy_encoder.cc [add] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/constants.h [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/authpolicy.cc [modify] https://crrev.com/16fe63be8711ebf9559d7fcc56d9fa473adaa45b/authpolicy/proto/authpolicy_containers.proto
,
Dec 19 2016
,
Dec 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/6cb9690fe13b75744275509912b8c0ea9e6ff04e commit 6cb9690fe13b75744275509912b8c0ea9e6ff04e Author: Lutz Justen <ljusten@chromium.org> Date: Tue Dec 13 14:50:22 2016 authpolicy: Add seccomp filters and use authpolicy-exec user Adds custom seccomp filters for all executed processes. Executes as authpolicyd-exec. Makes sure that all permissions are set up properly. Mount only /var read-write and the rest read-only. BUG= chromium:666693 TEST=Compiles, tested with custom test code. Change-Id: Ie9fe8957313d3dcbff974018f82dc02746e39de4 Reviewed-on: https://chromium-review.googlesource.com/420664 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org> Reviewed-by: Roman Sorokin <rsorokin@chromium.org> [modify] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/process_executor.cc [add] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/seccomp_filters/kinit-seccomp.policy [add] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/seccomp_filters/net_ads_gpo_list-seccomp.policy [add] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/seccomp_filters/net_ads_info-seccomp.policy [modify] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/process_executor.h [modify] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/samba_interface.cc [modify] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/samba_interface_internal_unittest.cc [modify] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/authpolicy_parser_main.cc [add] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/seccomp_filters/net_ads_search-seccomp.policy [add] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/seccomp_filters/net_ads_join-seccomp.policy [add] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/seccomp_filters/smbclient-seccomp.policy [add] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/seccomp_filters/authpolicy_parser-seccomp.policy [modify] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/errors.h [modify] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/errors.cc [modify] https://crrev.com/6cb9690fe13b75744275509912b8c0ea9e6ff04e/authpolicy/etc/init/authpolicyd.conf
,
Jan 9 2017
,
Jul 6 2017
bulk Verify of Chromad V1 bugs |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by tnagel@chromium.org
, Nov 21 2016