Adds a few additional bits of state to the MojoIpcz transport
implementation, in particular to track relative trust level across the
transport independent of broker/non-broker status.
Also introduces new Mojo invitation API flags to indicate when the
caller is inviting a process (or accepting an invitation as a process)
that is trustworthy, e.g. an elevated Windows process.
Combined these changes allow trusted non-broker nodes to transfer
pre-duplicated handles to brokers, rendering elevated processes
fully functional in a network of MojoIpcz processes. This turns out
to be necessary for existing elevated process usage in Chrome due
to how ipcz communication is bootstrapped.
TEST=browser_tests for ImageWriterUtilityClient* on Windows with MojoIpcz enabled
Bug: 1374611
Change-Id: Id9a6056e239c6ec0f258cd053489fd1771e3c850
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3963307
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/main@{#1061775}
With this change, TestService will have the same behavior as other
utility service like the Network process, where a disconnection cause
the utility process to terminate.
Bug: 977637
Change-Id: Iaa40598445b129d1737a663777b9f9f092c6fe04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3851127
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1038918}
This is a reland of 893230151e
The previous version of this CL mistakenly removed a default
case statement for an 'int' field, so an expected compile error
was not generated but instead an early return was skipped.
Original change's description:
> Add launch failure notifications to BrowserChildProcessObserver
>
> Also, fix a bug where the Windows sandbox would return SBOX_ALL_OK
> even if base::LaunchProcess failed, and launch_result was not
> being set for elevated processes.
>
> Add reporting of GetLastError on Windows in the
> ChildProcessTerminationInfo for failed launches.
>
> Also, clean up some switch statements to remove default cases.
>
> BUG=1280005
>
> Change-Id: I1001fc950b8456b78ef1a9a985ca07cf288e8a04
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3340072
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Will Harris <wfh@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#952182}
Bug: 1280005, 1280541
Change-Id: I81f3354e9870a455644e77144a40c4acbdf14ebe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3345720
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#952667}
This reverts commit 893230151e.
Reason for revert: "browser_tests" failing on builder "Linux ChromiumOS MSan Tests"
Original change's description:
> Add launch failure notifications to BrowserChildProcessObserver
>
> Also, fix a bug where the Windows sandbox would return SBOX_ALL_OK
> even if base::LaunchProcess failed, and launch_result was not
> being set for elevated processes.
>
> Add reporting of GetLastError on Windows in the
> ChildProcessTerminationInfo for failed launches.
>
> Also, clean up some switch statements to remove default cases.
>
> BUG=1280005
>
> Change-Id: I1001fc950b8456b78ef1a9a985ca07cf288e8a04
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3340072
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Will Harris <wfh@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#952182}
Bug: 1280005, 1280541
Change-Id: I368d0c89ca6911cb4145bcec13edf2ad8bd4d829
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3344787
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Aya Elsayed <ayaelattar@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Owners-Override: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/main@{#952303}
Also, fix a bug where the Windows sandbox would return SBOX_ALL_OK
even if base::LaunchProcess failed, and launch_result was not
being set for elevated processes.
Add reporting of GetLastError on Windows in the
ChildProcessTerminationInfo for failed launches.
Also, clean up some switch statements to remove default cases.
BUG=1280005
Change-Id: I1001fc950b8456b78ef1a9a985ca07cf288e8a04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3340072
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#952182}
This test should not be flaky or fail, so perhaps situation has changed
since it was disabled. It's important that this works.
So re-enable it to monitor for more flakes. Will Disable if they happen
again.
BUG=1268087,927298
Change-Id: I529e02daa690bc4255579a223520bb4a70f1cfe3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3313644
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#949721}
sandbox.mojom.Sandbox can now be used as the sandbox type so we
replace it everywhere.
The guts of the change are in //sandbox/policy/sandbox_type.h where
SandboxType is now deleted, and //sandbox/policy/mojom/sandbox.mojom
where sandbox types that are not already used in mojom ServiceSandbox
attributes are added.
Some cascading changes:-
- kService wasn't implemented on Mac (as it is equivalent to kUtility).
As we cannot alias enum fields in mojo like we can in C++ I have added
kService for Mac. The alternative is to define platform specific
ServiceSandbox attributes for all kService interfaces which seems to
put this complexity in the wrong place.
- sandbox_type.h included a number of buildflag headers that other files
then relied on. As sandbox_type.h is no longer needed in many places
and no longer needs these defines, they have been introduced where
required.
- sandbox::mojom::Sandbox is forward declared in a couple of headers
that are widely imported, hopefully reducing the number of times the
mojom.h is included but not used.
- some build deps must be modified.
- LibAssistantService needs a sandbox to be defined even when hosted
in process, so has kNoSandbox now when enable_cros_libassistant is
false.
Bug: 1210301
Change-Id: I13fa4fa8cbbb3090a38806fe5532787bbdf1e2fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3213677
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Sean Topping <seantopping@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Derek Schuff <dschuff@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: David Dorwin <ddorwin@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#934126}
The style guide encourages use of T{}-style casts under the assumption
that the compiler will warn about narrowing. This warning is currently
disabled; enabling it requires fixing up the cases that narrow.
Bug: 1216696
Change-Id: I096d3de7f3c23fe663d78cae9f25e7ebe1b23f69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2946380
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#890173}
This change removes calls to base::ASCIIToUTF16 in //content and //ui
with a single-line string literal and replaces them with a u"..."
literal instead. Files where this change would cause compilation errors
were not changed.
This is a mechanical change:
$ git grep -lw ASCIIToUTF16 content ui | xargs \
sed -i 's/\(base::\)\?ASCIIToUTF16(\("\(\\.\|[^\\"]\)*"\))/u\2/g'
$ git cl format
Bug: 1189439
Change-Id: I0d5601dc15324c43012b8d26260405f1efdca07e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2780265
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#865766}
Originally the sandbox policies lived in //content, but with
servicification this would create unwanted dependencies between
//services and //content. Instead, create a new //sandbox/policy
library to hold the sandbox integration code. This library can depend
on the low-level //sandbox routines, but not nice versa.
Tbr: ajgo@chromium.org (mechanical change rule)
Bug: 1097376
Change-Id: I1ca9ac0015a625197f2d3aae104e8f7aa78dcfd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2272609
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786385}
Note to QA: This CL is purely mechanical and shouldn't be blamed
for future regressions on touched files.
This is a follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/2211138
which already removed all usage using content::BrowserThread.
Hence this script now matches unqualified BrowserThread:: without
risking having "content::" be selected as "traits_before" by the regex
(ran on same revision as step #1).
content:: is now always added if outside namespace content {}
(deleting unused using content::BrowserThread; decls)
Script @ https://crbug.com/1026641#c92
(will TBR fdoray@ post-review for mechanical change)
TBR=fdoray@hchromium.org
AX-Relnotes: n/a.
Bug: 1026641
Change-Id: I51ae2f83eb17d19b54563fd9b4fc040d2aa0c948
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212469
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772458}
This is a step towards doing full IWYU of browser_test.h, which will
have other benefits.
Completely mechanical and already R+ed as part of r765923.
Tbr: sky
Bug: none
Change-Id: Icb7ab728098a6cf29c0920da4b524e96a7c024c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2186411
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766361}
Include this directly in relevant test files. This lets us convert the
HAS_OUT_OF_PROC_TEST_RUNNER checks in this file and
view_event_test_base.h into #errors, and force people to not even
include this file in files that can't use it.
Bug: none
Tbr: sky
Change-Id: I86626099eb047eb53e8b3611de38ba6bebc01a0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2136117
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#765923}
Removes unused kNaClLoader (was PROCESS_TYPE_NACL_LOADER)
from Windows as these are in fact Ppapi (PPAPI) sandbox types.
Removes unused values from SandboxType enum and replaces default cases.
SandboxType is not used for iteration so these boundary values are removed
from the SandboxType enum.
The kInvalid SandboxType is retained as it is used as an error case in
a couple of places. It might be possible to remove this in the future.
This removes default cases from switch statements where it might make
sense to have a notification in future when new sandbox types are added.
In these cases the default case is replaced with all otherwise
unchecked cases, so retaining the existing behavior.
Change-Id: I76ffc8ae617f3f8fa9aa68236551ebcfa4cce32f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1938076
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723425}
This CL converts TestServicePtr and TestServiceRequest
to new Mojo types.
It uses Remote, PendingReceiver and Receiver instead of
QueryableDataStorePtr, QueryableDataStoreRequest and Binding.
Bug: 955171
Change-Id: I61f3d6b66ac6f5faffe533800bbe482a2db772ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1803948
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697068}
*** Note: There is no behavior change from this patch. ***
The PostTask APIs will shortly be changed to require all tasks to explicitly
specify their thread affinity, i.e., whether the task should run on the thread
pool or a specific named thread such as a BrowserThread. This patch updates all
call sites with thread affinity annotation. We also remove the "WithTraits"
suffix to make the call sites more readable.
Before:
// Thread pool task.
base::PostTaskWithTraits(FROM_HERE, {...}, ...);
// UI thread task.
base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI, ...}, ...);
After:
// Thread pool task.
base::PostTask(FROM_HERE, {base::ThreadPool(), ...}, ...);
// UI thread task.
base::PostTask(FROM_HERE, {BrowserThread::UI, ...}, ...);
This patch was semi-automatically prepared with these steps:
1. Patch in https://chromium-review.googlesource.com/c/chromium/src/+/1635827
to make thread affinity a build-time requirement.
2. Run an initial pass with a clang rewriter:
https://chromium-review.googlesource.com/c/chromium/src/+/1635623
3. ninja -C out/Debug | grep 'requested here' | cut -d: -f1-3 | sort | \
uniq > errors.txt
4. while read line; do
f=$(echo $line | cut -d: -f 1)
r=$(echo $line | cut -d: -f 2)
c=$(echo $line | cut -d: -f 3)
sed -i "${r}s/./&base::ThreadPool(),/$c" $f
done < errors.txt
5. GOTO 3 until build succeeds.
6. Remove the "WithTraits" suffix from task API call sites:
$ tools/git/mffr.py -i <(cat <<EOF
[
["PostTaskWithTraits", "PostTask"],
["PostDelayedTaskWithTraits", "PostDelayedTask"],
["PostTaskWithTraitsAndReply", "PostTaskAndReply"],
["CreateTaskRunnerWithTraits", "CreateTaskRunner"],
["CreateSequencedTaskRunnerWithTraits", "CreateSequencedTaskRunner"],
["CreateUpdateableSequencedTaskRunnerWithTraits", "CreateUpdateableSequencedTaskRunner"],
["CreateSingleThreadTaskRunnerWithTraits", "CreateSingleThreadTaskRunner"],
["CreateCOMSTATaskRunnerWithTraits", "CreateCOMSTATaskRunner"]
]
EOF
)
This CL was uploaded by git cl split.
R=boliu@chromium.org, tsepez@chromium.org
Bug: 968047
Change-Id: I346372d16a3856186ea74d14e0dd8a12f7cacae5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729589
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683554}
This reverts commit ffe0185ea6.
Reason for revert: Causes test failures in official builds
Original change's description:
> Shorten TRAP_SEQUENCE() to one instruction on most platforms.
>
> Previously, TRAP_SEQUENCE() consisted of:
> - an instruction to trigger a debugger breakpoint
> - an instruction to ensure fatal termination (usually encoded as an
> illegal instruction)
>
> But all that's really needed is the latter, so eliminate the
> instruction to trigger the debugger breakpoint.
>
> Note: on Intel, the debugger breakpoint instruction (int3) is only one
> byte, while the fatal termination instruction (ud2) is two bytes.
> Unfortunately, crash reports seem to be indicating that int3 is
> non-fatal. Since it's important that TRAP_SEQUENCE() terminates, it
> cannot rely on int3.
>
> Bug: 958675
> Change-Id: I84b3123b07a9871dbd3b062fd73e79137b1ef6dd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1672227
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#678065}
TBR=dcheng@chromium.org,alexmos@chromium.org,rsesek@chromium.org,mark@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 958675, 985138
Change-Id: Ib9c3d09f6b2a5dc182cb125f90a7d9130c98e5be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1713904
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#679787}
Previously, TRAP_SEQUENCE() consisted of:
- an instruction to trigger a debugger breakpoint
- an instruction to ensure fatal termination (usually encoded as an
illegal instruction)
But all that's really needed is the latter, so eliminate the
instruction to trigger the debugger breakpoint.
Note: on Intel, the debugger breakpoint instruction (int3) is only one
byte, while the fatal termination instruction (ud2) is two bytes.
Unfortunately, crash reports seem to be indicating that int3 is
non-fatal. Since it's important that TRAP_SEQUENCE() terminates, it
cannot rely on int3.
Bug: 958675
Change-Id: I84b3123b07a9871dbd3b062fd73e79137b1ef6dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1672227
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678065}
As stated in BrowserChildProcessHostImpl::OnChildDisconnected:
"OnChildDisconnected may be called without OnChannelConnected, so stop the
early exit watcher so GetTerminationStatus can close the process handle"
This test previously did not take this into account and always expected a
connected notification before a disconnected notification, so the test can
be fixed by just removing this assumption.
BUG=879555
Change-Id: I68ce0d27765a6b5387d57bf7af10b84c8fb240ac
Reviewed-on: https://chromium-review.googlesource.com/c/1286910
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601294}
content_browsertests UtilityProcessHostBrowserTest.LaunchProcessAndCrash is
flaky on Windows. Disabling the test so that it doesn't cause build bots to
be red on non-breaking changes.
Bug: 879555
Change-Id: If2aff8e69d240adf09173b82472db6c86eb6aa91
Reviewed-on: https://chromium-review.googlesource.com/1227303
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592216}
Add new "metrics name" which is non-localized and will be used for
recording metrics for child processes. The name is taken from the
mojo service name for mojo processes, or can be custom set.
BUG=767906
Change-Id: Ibcfa314fe4c475256f76a4369d17e63e32050195
Reviewed-on: https://chromium-review.googlesource.com/1072789
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563777}
Removes UtilityProcessHost from content/public and renames
UtilityprocessHostImpl to UtilityProcessHost.
Now that all use cases of UtilityProcessHost have been replaced by
using services (but for the PowerMonitor browser tests), this will
prevent further forking of process bypassing the service manager.
Bug: 775677
Change-Id: I2107271a54e4ab3141102a5c4fbcce4ea21915dd
Reviewed-on: https://chromium-review.googlesource.com/961496
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543238}
