blob: ba45ae2ea4d5d2a5544ec86b04caae22aaae25e3 [file] [log] [blame] [view]
Daniel Chengd21b551f2017-10-30 08:43:231# Mojo "Style" Guide
2
3Mojo is Chrome's new IPC system and provides lots of useful abstractions. These
4abstractions can make it easier to write code that makes interprocess calls,
5but can also add significant complexity. Below are some recommendation from
6Mojo and IPC reviewers for best practices.
7
Michael Giuffrida68208212019-02-11 09:15:218For questions, concerns, or suggestions, reach out to
9[[email protected]](https://groups.google.com/a/chromium.org/forum/#!forum/chromium-mojo).
Daniel Chengd21b551f2017-10-30 08:43:2310
11> For legacy IPC, please see [security tips for IPC][security-tips-for-ipc].
12
13[TOC]
14
15
16## Simplicity
17
18Strive to write simple interfaces. Minimize the amount of cross-process state
19that needs to be maintained in sync.
20
21**_Good_**
22
23```c++
Daniel Chengaa8a2672017-11-02 08:05:0124interface TeleporterFactory {
Oksana Zhuravlova3fde5ed2020-04-11 00:40:5525 Create(Location start, Location end) => (pending_remote<Teleporter>);
Daniel Chengd21b551f2017-10-30 08:43:2326};
Daniel Chengaa8a2672017-11-02 08:05:0127
28interface Teleporter {
29 TeleportGoat(Goat) = ();
Daniel Chengd21b551f2017-10-30 08:43:2330};
31```
32
33**_Bad_**
34
35```c++
Daniel Chengaa8a2672017-11-02 08:05:0136interface Teleporter {
37 // Bad: comments will need to explicitly call out that both locations need to
38 // be bound before calling TeleportGoat()!
Daniel Chengd21b551f2017-10-30 08:43:2339 //
Daniel Chengaa8a2672017-11-02 08:05:0140 // In addition, if untrustworthy processes can talk to trustworthy processes,
41 // the Teleporter implementation will need to also handle the case where the
42 // Location objects are not yet bound.
43 SetStart(Location);
44 SetEnd(Location);
45 TeleportGoat(Goat) = ();
Daniel Chengd21b551f2017-10-30 08:43:2346};
47```
48
49Similarly, strive to make methods focused. Do not overuse optional types.
50
51**_Good_**
52
53```c++
Daniel Chengaa8a2672017-11-02 08:05:0154struct TeleporterStats {
55 AnimalStats animal_stats;
56 FungiStats fungi_stats;
57 GoatStats goat_stats;
58 PlantStats plant_stats;
59};
60
61interface Teleporter {
62 TeleportAnimal(Animal) => ();
63 TeleportFungi(Fungi) => ();
64 TeleportGoat(Goat) = ();
65 TeleportPlant(Plant) => ();
66
Roland Bock3afe8cc2020-11-06 23:14:5367 // TeleporterStats will be have a value if and only if the call was
68 // successful.
69 GetStats() => (TeleporterStats?);
Daniel Chengd21b551f2017-10-30 08:43:2370};
71```
72
73**_Bad_**
74
75```c++
Daniel Chengaa8a2672017-11-02 08:05:0176interface Teleporter {
77 // The intent of four optional arguments is unclear: can this call teleport
78 // multiple objects of different types at once, or is the caller only
79 // supposed to only pass one non-null argument per call?
80 Teleport(Animal?, Fungi?, Goat?, Plant?) => ();
81
Roland Bock3afe8cc2020-11-06 23:14:5382 // Does this return all stats if success is true? Or just the categories that
Daniel Chengaa8a2672017-11-02 08:05:0183 // the teleporter already has stats for? The intent is uncertain, so wrapping
84 // the disparate values into a result struct would be cleaner.
85 GetStats() =>
86 (bool success, AnimalStats?, FungiStats?, PlantStats?, FungiStats?);
Daniel Chengd21b551f2017-10-30 08:43:2387};
88```
89
90
91## Documentation
92
93Mojo structs, interfaces, and methods should all have comments. Make sure the
94comments cover the "how" and the "why" of using an interface and its methods,
95and not just the "what". Document preconditions, postconditions, and trust: if
96an interface is implemented in the browser process and handles requests from
97the renderer process, this should be mentioned in the comments. Complex features
98should also have an external `README.md` that covers the high-level flow of
99information through interfaces and how they interact to implement the feature.
100
101**_Good_**
102
103```c++
Daniel Chengaa8a2672017-11-02 08:05:01104// Interface for controlling a teleporter. Lives in the browser process, and
105// used to implement the Teleportation over Mojo IPC RFC.
106interface Teleporter {
107 // Teleportation helpers for different taxonomic kingdoms. Teleportation is
108 // not complete until the reply callback is invoked. The caller must NOT
109 // release the sender side resources until the reply callback runs; releasing
110 // the resources early will cause splinching.
111 TeleportAnimal(Animal) => ();
112 TeleportFungi(Fungi) => ();
113 // Goats require a specialized teleportation channel distinct from
114 // TeleportAnimal to ensure goatiness isolation.
115 TeleportGoat(Goat) => ();
116 TeleportPlant(Plant) => ();
Daniel Chengd21b551f2017-10-30 08:43:23117
Roland Bock3afe8cc2020-11-06 23:14:53118 // Returns current teleporter stats. On failure (e.g. a teleportation
119 // operation is currently in progress) a null stats object will be returned.
120 GetStats() => (TeleporterStats?);
Daniel Chengd21b551f2017-10-30 08:43:23121};
122```
123
124
125## Security
126
127Policy should be controlled solely by the browser process. "Policy" can mean
128any number of things, such as sizes, addresses, permissions, URLs, origins,
129etc. In an ideal world:
130
1311. Unprivileged process asks for a capability from the privileged process that
132 owns the resource.
1331. Privileged process applies policy to find an implementation for the
134 capability.
1351. Unprivileged process performs operations on the capability, constrained in
136 scope.
137
138The privileged process must own the capability lifecycle.
139
140
141### Do not trust less privileged processes
142
143This is the overriding principle for all guidelines in this section. When
144receiving data from a less trusted process, treat the data as if it were
145generated by a malicious adversary. Message handlers cannot assume that offsets
146are valid, calculations won't overflow, et cetera.
147
148In general:
149
150* the browser process is the most privileged process type and therefore, must
151 be maximally suspicious of its IPC inputs
152* the renderer and the ARC++ processes are the least privileged and least
153 trustworthy process types
154* other process types, such as GPU and plugin, fall in between
155
156When passing objects up a privilege gradient (from less → more privileged), the
157callee must validate the inputs before acting on them. When passing objects
158down a privilege gradient, such as from browser → renderer, it is OK for the
159callee to trust the caller.
160
161> See also: [Do not Handle Impossible Situations](#Do-not-handle-impossible-situations)
162
163
164### Do not send unnecessary or privilege-presuming data
165
Oksana Zhuravlovaee1afd12020-02-15 00:47:27166> Each `BrowserInterfaceBroker` for frames and workers is strongly associated with an
Darwin Huangb4bd2452019-10-08 22:56:04167> origin. Where possible, prefer to use this associated origin rather than
168> sending it over IPC. (See <https://crbug.com/734210> and
169> <https://crbug.com/775792/>).
Daniel Chengd21b551f2017-10-30 08:43:23170
171For example, the browser process must not (fully) trust the renderer's claims
172about origins. The browser process should already know what origin the renderer
173is evaluating, and thus should already have this data (for example, see
174`RenderFrameHost::GetLastCommittedOrigin()`). Thus, a method that requires
175passing an origin from the renderer to the browser process has a conceptual
176error, and quite possibly, a vulnerability.
177
178> Note: there are currently subtle races when using `GetLastCommittedOrigin()`
179> that will be resolved by fixing <https://crbug.com/729021>.
180
181Similarly, the browser process must not trust the renderer's claims about file
182pathnames. It would be unsafe for the browser process to save a downloaded file
183to `~/.bashrc` just because the renderer asked. Instead, it would be better for
184the browser process to:
185
1861. Kill the renderer if `basename(pathname) != pathname`, since the renderer is
187 obviously compromised if it makes this mistake.
1881. Defang the basename, by removing leading dots, et cetera. Note that the
189 definition of proper defanging varies per platform.
1901. Prepend its own parent directory to the basename, e.g. ~/Downloads.
191
192> TODO(https://crbug.com/779196): Even better would be to implement a C++ type
193> performs the appropriate sanitizations and recommend its usage directly here.
194
195
Lukasz Anforowiczbb99a8562019-01-25 19:19:38196### Validate privilege-presuming data received over IPC
197
198If it is not possible to avoid sending privilege-presuming data over IPC (see
199the previous section), then such data should be verified before being used.
200
201* Browser process:
202 - Use `ChildProcessSecurityPolicy`'s methods like
203 `CanAccessDataForOrigin` or `CanReadFile` to verify IPC messages
204 received from less privileged processes.
205 - When verification fails, ignore the IPC and terminate the renderer process
206 using `mojo::ReportBadMessage` (or using `mojo::GetBadMessageCallback` for
207 messages handled asynchronously). For legacy IPC, the renderer process
208 may be terminated by calling the `ReceivedBadMessage` function (separate
209 implementations exist for `//content`, `//chrome` and other layers).
210
Lukasz Anforowiczbb99a8562019-01-25 19:19:38211
Daniel Cheng0a7a4ec2018-02-13 23:30:27212### Do not define unused or unimplemented things
213
214Mojo interfaces often cross privilege boundaries. Having well-defined interfaces
215that don't contain stubbed out methods or unused parameters makes it easier to
Daniel Chengcda1df5b2018-03-30 21:30:16216understand and evaluate the implications of crossing these boundaries. Several
217common areas to watch out for:
Daniel Cheng0a7a4ec2018-02-13 23:30:27218
219
220#### Do use EnableIf to guard platform-specific constructs
Daniel Chengd21b551f2017-10-30 08:43:23221
222Platform-specific functionality should only be defined on the platforms where
Daniel Cheng0a7a4ec2018-02-13 23:30:27223it is implemented. Use the Mojo `EnableIf` annotation to guard definitions that
224should only be visible in certain build configurations.
Daniel Chengd21b551f2017-10-30 08:43:23225
Daniel Cheng0a7a4ec2018-02-13 23:30:27226**_Good_**
227```c++
228// GN file:
229mojom("view_bindings") {
230 // ...
Daniel Chengd21b551f2017-10-30 08:43:23231
Daniel Cheng0a7a4ec2018-02-13 23:30:27232 enabled_features = []
233 if (is_android) {
Bill Orrdc01ca32019-01-30 00:22:11234 enabled_features += [ "is_android" ]
Daniel Cheng0a7a4ec2018-02-13 23:30:27235 }
236}
237
238// mojom definition:
239interface View {
240 // ...
241
242 [EnableIf=is_android]
243 UpdateBrowserControlsState(bool enable_hiding, bool enable_showing,
244 bool animate);
245};
246
247// C++ implementation:
248class View : public mojom::View {
249 public:
250 // ...
251
Xiaohan Wang0727d882022-01-21 03:03:43252#if BUILDFLAG(IS_ANDROID)
Daniel Cheng0a7a4ec2018-02-13 23:30:27253 void UpdateBrowserControlsState(bool enable_hiding, bool enable_showing,
254 bool animate);
255#endif
256};
257```
258
259**_Bad_**
260```c++
261// mojom definition:
262interface View {
263 // ...
264
265 UpdateBrowserControlsState(bool enable_hiding, bool enable_showing,
266 bool animate);
267};
268
269// C++ implementation:
270class View : public mojom::View {
271 public:
272 // ...
273
Xiaohan Wang0727d882022-01-21 03:03:43274#if BUILDFLAG(IS_ANDROID)
Daniel Cheng0a7a4ec2018-02-13 23:30:27275 void UpdateBrowserControlsState(bool enable_hiding, bool enable_showing,
276 bool animate) override;
277#else
278 void UpdateBrowserControlsState(bool enable_hiding, bool enable_showing,
279 bool animate) override {
280 NOTREACHED();
281 }
282#endif
283};
284```
285
Xiaohan Wang688f49d2018-02-26 19:39:53286The `EnableIf` annotation can be applied to almost anything: imports,
287interfaces, methods, arguments, constants, structs, struct members, enums,
288enumerator values, et cetera.
Daniel Cheng0a7a4ec2018-02-13 23:30:27289
290
291#### Do not define unimplemented methods
292
293Reviewing IPC requires reviewing a concrete implementation of the Mojo
294interface, to evaluate how the (possibly untrustworthy) inputs are used, what
295outputs are produced, et cetera. If a method is not yet implemented, do not
296define it in the interface.
297
298**_Bad_**
299```c++
300// mojom definition:
301interface Spaceship {
302 EnterHyperspace();
303 ExitHyperspace();
304};
305
306// C++ implementation:
307class SpaceshipPrototype : public mojom::Spaceship {
308 void EnterHyperspace() { /* TODO(dcheng): Implement. */ }
309 void ExitHyperspace() { /* TODO(dcheng): Implement. */ }
310};
311```
312
313
Daniel Chengcda1df5b2018-03-30 21:30:16314#### Do not define placeholder enumerator values
Daniel Cheng0a7a4ec2018-02-13 23:30:27315
Daniel Chengcda1df5b2018-03-30 21:30:16316Do not define placeholder enumerator values like `kLast`, `kMax`, `kCount`, et
317cetera. Instead, rely on the autogenerated `kMaxValue` enumerator emitted for
318Mojo C++ bindings.
Daniel Chengd21b551f2017-10-30 08:43:23319
Daniel Chengcda1df5b2018-03-30 21:30:16320For UMA histograms, logging a Mojo enum is simple: simply use the two argument
321version of `UMA_HISTOGRAM_ENUMERATION`:
Daniel Chengd21b551f2017-10-30 08:43:23322
Daniel Chengcda1df5b2018-03-30 21:30:16323**_Good_**
324```c++
325// mojom definition:
326enum GoatStatus {
327 kHappy,
328 kSad,
329 kHungry,
330 kGoaty,
331};
332
333// C++:
334UMA_HISTOGRAM_ENUMERATION("Goat.Status", status);
335```
336
337Using a `kCount` sentinel complicates `switch` statements and makes it harder to
338enforce invariants: code needs to actively enforce that the otherwise invalid
339`kCount` sentinel value is not incorrectly passed around.
340
341**_Bad_**
342```c++
343// mojom definition:
344enum CatStatus {
345 kAloof,
346 kCount,
347};
348
349// C++
350switch (cat_status) {
351 case CatStatus::kAloof:
352 IgnoreHuman();
353 break;
354 case CatStatus::kCount:
355 // this should never happen
356}
357```
358
359Defining `kLast` manually results in ugly casts to perform arithmetic:
360
361**_Bad_**
362```c++
363// mojom definition:
364enum WhaleStatus {
365 kFail,
366 kNotFail,
367 kLast = kNotFail,
368};
369
370// C++:
371UMA_HISTOGRAM_ENUMERATION("Whale.Status", status,
372 static_cast<int>(WhaleStatus::kLast) + 1);
373```
374
375For interoperation with legacy IPC, also use `kMaxValue` rather than defining a
376custom `kLast`:
377
378**_Good_**
379```c++
380IPC_ENUM_TRAITS_MAX_VALUE(GoatStatus, GoatStatus::kMaxValue);
381```
Daniel Chengd21b551f2017-10-30 08:43:23382
383
384### Use structured types
385
386Where possible, use structured types: this allows the type system to help
387enforce that the input data is valid. Common ones to watch out for:
388
Oksana Zhuravlovac70ec7ef2018-03-13 21:08:42389* Files: use `mojo_base.mojom.File`, not raw descriptor types like `HANDLE`
Daniel Chengd21b551f2017-10-30 08:43:23390 and `int`.
Oksana Zhuravlovac70ec7ef2018-03-13 21:08:42391* File paths: use `mojo_base.mojom.FilePath`, not `string`.
Oksana Zhuravlova38321d082018-04-27 23:59:01392* JSON: use `mojo_base.mojom.Value`, not `string`.
Daniel Chengd21b551f2017-10-30 08:43:23393* Mojo interfaces: use `Interface` or `Interface&`, not `handle` or
394 `handle<message_pipe>`.
Oksana Zhuravlova34579e912018-03-23 00:18:49395* Nonces: use `mojo_base.mojom.UnguessableToken`, not `string`.
Daniel Chengd21b551f2017-10-30 08:43:23396* Origins: use `url.mojom.Origin`, not `url.mojom.Url` and certainly not
397 `string`.
Oksana Zhuravlova57e8dca2018-03-20 03:21:34398* Time types: use `mojo_base.mojom.TimeDelta` /
399 `mojo_base.mojom.TimeTicks` / `mojo_base.mojom.Time`, not `int64` /
Daniel Chengd21b551f2017-10-30 08:43:23400 `uint64` / `double` / et cetera.
401* URLs: use `url.mojom.Url`, not `string`.
Daniel Cheng876c66322018-08-29 06:42:31402* `array<uint8>` or `string` and `memcpy()`: use a Mojo struct and statically
403 define the serialized fields. While `memcpy()` may be tempting for its
404 simplicity, it can leak info in padding. Even worse, `memcpy()` can easily
405 copy [undocumented fields][serialize-struct-tm-safely] or newly introduced
406 fields that were never evaluated for safety by the developer or reviewer.
Daniel Chengd21b551f2017-10-30 08:43:23407
408**_Good_**
409
410```c++
411interface ReportingService {
Oksana Zhuravlova57e8dca2018-03-20 03:21:34412 ReportDeprecation(mojo_base.mojom.TimeTicks time,
Daniel Chengd21b551f2017-10-30 08:43:23413 url.mojom.Url resource,
414 uint32 line_number);
415};
416```
417
418**_Bad_**
419
420```c++
421interface ReportingService {
422 // Bad: unclear what units |time| is or what |data| contains.
Oksana Zhuravlova38321d082018-04-27 23:59:01423 ReportDeprecation(double time, mojo_base.mojom.Value data);
Daniel Chengd21b551f2017-10-30 08:43:23424};
425```
426
Daniel Cheng876c66322018-08-29 06:42:31427Avoid parallel arrays of data that require the receiver to validate that the
428arrays have matching lengths. Instead, bundle the data together in a struct so
429it is impossible to have a mismatch:
Daniel Chengd21b551f2017-10-30 08:43:23430
431**_Good_**
432
433```c++
434struct Pixel {
435 int8 reds;
436 int8 greens;
437 int8 blues;
438 int8 alphas;
439};
440
441struct Bitmap {
442 // Good: it is impossible for there to be mismatched data.
443 array<Pixel> pixels;
444};
445```
446
447**_Bad_**
448
449```c++
450// Bad: code using this struct will need to validate that all the arrays have
451// matching sizes.
452struct Bitmap {
453 array<int8> reds;
454 array<int8> greens;
455 array<int8> blues;
456 array<int8> alphas;
457};
458```
459
460
461### Beware of arithmetic overflow
462
463> TODO(dcheng): Import the guidance from the legacy IPC doc.
464
465Signed overflow is undefined in C++. If unsure about whether or not something
466will overflow, use the safe numeric helpers from `//base/numerics`!
467
468**_Good_**
469
470```c++
471base::CheckedNumeric<int32_t> size = mojo_rect->width();
472size *= mojo_rect.height();
473if (!size.IsValid()) {
474 mojo::ReportBadMessage("Bad size from renderer!");
475}
476```
477
478**_Bad_**
479
480```c++
481// Bad: Signed overflow is undefined in C++!
482int32_t size = mojo_rect->width() * mojo_rect.height();
483```
484
485Note that even if the types have defined overflow semantics, it is almost always
486a good idea to check for overflow.
487
488**_Good_**
489
490```c++
491uint32_t alloc_size;
492if (!CheckMul(request->elements(), request->element_size())
493 .AssignIfValid(&alloc_size)) {
494 // Safe: avoids allocating with a bogus size that overflowed to a smaller than
495 // expected value.
Rayan Kanso81aeee32019-07-11 18:50:44496 mojo::ReportBadMessage("Invalid allocation size");
Daniel Chengd21b551f2017-10-30 08:43:23497}
498
499Element* array = CreateArray(alloc_size);
500for (size_t i = 0; i < request->element_size(); ++i) {
501 array[i] = PopulateArray(i);
502}
503```
504
505**_Bad_**
506
507```c++
508uint32_t alloc_size = request->elements() * request->element_size();
509
510// Dangerous: alloc_size can overflow so that CreateArray allocates too little
511// memory. Subsequent assignments will turn into an out-of-bound write!
512Element* array = CreateArray(alloc_size);
513for (size_t i = 0; i < request->element_size(); ++i) {
514 array[i] = PopulateArray(i);
515}
516```
517
518
Erik Chen14a4bad2021-04-21 17:53:13519### All possible message values are semantically valid
520
521When possible, messages should be defined in such a way that all possible values
522are semantically valid. As a corollary, avoid having the value of one field
523dictate the validity of other fields.
524
525**_Good_**
526
527```c++
528union CreateTokenResult {
529 // Implies success.
530 string token;
531
532 // Implies failure.
533 string error_message;
534};
535
536struct TokenManager {
537 CreateToken() => (CreateTokenResult result);
538};
539```
540
541**_Bad_**
542```c++
543struct TokenManager {
544 // Requires caller to handle edge case where |success| is set to true, but
545 // |token| is null.
546 CreateToken() => (bool success, string? token, string? error_message);
547
548 // Requires caller to handle edge case where both |token| and |error_message|
549 // are set, or both are null.
550 CreateToken() => (string? token, string? error_message);
551};
552```
553
554There are some known exceptions to this rule because mojo does not handle
555optional primitives.
556
557**_Allowed because mojo has no support for optional primitives_**
558```c++
559 struct Foo {
560 int32 x;
561 bool has_x; // does the value of `x` have meaning?
562 int32 y;
563 bool has_y; // does the value of `y` have meaning?
564 };
565```
566
567Another common case where we tolerate imperfect message semantics is
568with weakly typed integer [bitfields](#handling-bitfields).
569
570### Handling bitfields
571
572Mojom has no native support for bitfields. There are two common approaches: a
573type-safe struct of bools which is a bit clunky (preferred) and an integer-based
574approach (allowed but not preferred).
575
576**_Type-safe bitfields_**
577```c++
578struct VehicleBits {
579 bool has_car;
580 bool has_bicycle;
581 bool has_boat;
582};
583
584struct Person {
585 VehicleBits bits;
586};
587```
588
589**_Integer based approach_**
590```c++
591struct Person {
592 const uint64 kHasCar = 1;
593 const uint64 kHasBicycle = 2;
594 const uint64 kHasGoat= 4;
595
596 uint32 vehicle_bitfield;
597};
598```
599
Daniel Chengd21b551f2017-10-30 08:43:23600## C++ Best Practices
601
602
Daniel Chengd8df82002018-02-09 20:52:15603### Use mojo::WrapCallbackWithDefaultInvokeIfNotRun And mojo::WrapCallbackWithDropHandler sparingly
Daniel Cheng9ae28e02018-01-17 18:44:10604
605Mojo provides several convenience helpers to automatically invoke a callback if
606the callback has not already been invoked in some other way when the callback is
607destroyed, e.g.:
608
609```c++
610 {
Dan Sandersce2b98e2021-02-03 06:46:10611 base::OnceCallback<int> cb = mojo::WrapCallbackWithDefaultInvokeIfNotRun(
612 base::BindOnce([](int) { ... }), -1);
Daniel Cheng9ae28e02018-01-17 18:44:10613 } // |cb| is automatically invoked with an argument of -1.
614```
615
Daniel Chengd8df82002018-02-09 20:52:15616This can be useful for detecting interface errors:
Daniel Cheng9ae28e02018-01-17 18:44:10617
Daniel Chengd8df82002018-02-09 20:52:15618```c++
619 process->GetMemoryStatistics(
620 mojo::WrapCallbackWithDefaultInvokeIfNotRun(
Dan Sandersce2b98e2021-02-03 06:46:10621 base::BindOnce(&MemoryProfiler::OnReplyFromRenderer), <failure args>));
Daniel Chengd8df82002018-02-09 20:52:15622 // If the remote process dies, &MemoryProfiler::OnReplyFromRenderer will be
623 // invoked with <failure args> when Mojo drops outstanding callbacks due to
624 // a connection error on |process|.
625```
626
627However, due to limitations of the current implementation, it's difficult to
628tell if a callback object has invoke-on-destroy behavior. In general:
629
6301. Prefer error connection handlers where possible.
6311. Only use the callback helpers for detecting interface errors. These
632 callbacks may be invoked during destruction and must carefully consider
633 receiver object lifetime. For more information, please see the
634 [Mojo documentation][mojo-doc-process-crashes].
635
636> Note that using the callback wrappers in the renderer is often unnecessary.
637> Message pipes are typically closed as part of a Document shutting down; since
638> many Blink objects already inherit `blink::ContextLifecycleObserver`, it is
639> usually more idiomatic to use this signal to perform any needed cleanup work.
Daniel Cheng9ae28e02018-01-17 18:44:10640
Daniel Chengd21b551f2017-10-30 08:43:23641### Use StructTraits
642
643Creating a typemap and defining a `StructTraits` specialization moves the
644complexity of serialization, deserialization, and validation into a central
645location. We universally recommend this over defining `TypeConverter`
646specializations: when a value fails deserialization, the receiver method will
647never even be invoked. As a bonus, it also reduces the number of copies during
648serialization and deserialization. 😄
649
650**_Good_**
651
652```c++
Daniel Cheng1f386932018-01-29 19:56:47653// In url_gurl_mojom_traits.h:
Daniel Chengd21b551f2017-10-30 08:43:23654template <>
655struct StructTraits<url::mojom::UrlDataView, GURL> {
656 static base::StringPiece url(const GURL& r);
657
658 // If Read() returns false, Mojo will discard the message.
659 static bool Read(url::mojom::UrlDataView data, GURL* out);
660};
661
Daniel Cheng1f386932018-01-29 19:56:47662// In url_gurl_mojom_traits.cc:
Daniel Chengd21b551f2017-10-30 08:43:23663// Note that methods that aren't simple getters should be defined
664// out-of-line to avoid code bloat.
665base::StringPiece StructTraits<url::mojom::UrlDataView, GURL>::url(
666 const GURL& r) {
667 if (r.possibly_invalid_spec().length() > url::kMaxURLChars ||
668 !r.is_valid()) {
669 return base::StringPiece();
670 }
671 return base::StringPiece(r.possibly_invalid_spec().c_str(),
672 r.possibly_invalid_spec().length());
673}
674
675bool StructTraits<url::mojom::UrlDataView, GURL>::Read(
676 url::mojom::UrlDataView data, GURL* out) {
677 base::StringPiece url_string;
678 if (!data.ReadUrl(&url_string))
679 return false;
680 if (url_string.length() > url::kMaxURLChars)
681 return false;
682 *out = GURL(url_string);
Dominic Farolino1f92f2a2021-05-27 21:22:41683 if (!url_string.empty() && !out->is_valid())
684 return false;
685 return true;
Daniel Chengd21b551f2017-10-30 08:43:23686}
687```
688
689**_Bad_**
690
691```c++
692template <>
693struct TypeConverter<url::mojom::UrlPtr, GURL> {
694 // Inefficient: this copies data once off the wire to create a
695 // url.mojom.Url object, then copies it again to create a GURL.
696 static GURL Convert(const url::mojom::UrlPtr url) {
697 if (url.url.is_empty()) return GURL();
698 // Not good: no way to signal errors, so any code that converts the
699 // Mojo struct to a GURL will somehow need to check for errors…
700 // but it can't even be distinguished from the empty URL case!
701 if (url.url.size() > url::kMaxURLChars) return GURL();
702 return GURL(url.url);
703 }
704};
705```
706
707There are also corresponding `EnumTraits` and `UnionTraits` specializations for
708mojo enums and unions respectively.
709
710
711### StructTraits getters should be simple
712
713Where possible, `StructTraits` should be returning const references or simple
714read-only views of the data. Having to create temporary data structures during
715serialization should be rare, and it should be even rarer to mutate the input
716argument.
717
718
719### Out-of-line complex serialization/deserialization logic
720
721A `StructTraits` specialization is almost always fully specialized. Only define
722`StructTraits` methods inline in the header if the method is a simple getter
723that returns a reference, pointer, or other simple POD. Define all other
724methods out-of-line to avoid code bloat.
725
726
727### Do not write one-off functions to convert to/from Mojo types
728
729There are some instances where it is simply not possible to define a
730`StructTraits` for type mapping: this commonly occurs with Blink IDL and Oilpan
731types. In these instances, add a `TypeConverter` specialization rather than
732defining a one-off conversion function. This makes it easier to search for and
733audit code that does potentially risky type conversions.
734
735> The use of `TypeConverter` should be limited as much as possible: ideally,
736> only use it in renderers.
737
738**_Good_**
739
740```c++
741template <>
742struct TypeConverter<IDLDictionary, mojom::blink::DictionaryPtr> {
743 static IDLDictionary* Convert(const mojom::blink::DictionaryPtr& in) {
744 // Note that unlike StructTraits, there is no out-of-band way to signal
745 // failure.
746 IDLDictionary* out = new IDLDictionary;
747 out->int_value = in->int_value;
748 out->str_value = in->str_value;
749 return out;
750 }
751};
752```
753
754**_Bad_**
755
756```c++
757// Using a custom one-off function makes this hard to discover in security
758// audits.
759IDLDictionary* FromMojo(const mojom::blink::DictionaryPtr& in) {
760 IDLDictionary* out = new IDLDictionary;
761 out->int_value = in->int_value;
762 out->str_value = in->str_value;
763 return out;
764}
765```
766
767
768### Use the proper abstractions
769
Oksana Zhuravlova9f3b8ef2019-08-26 20:27:40770`mojo::ReceiverSet` implies multiple clients may connect. If this actually isn't
Daniel Chengd21b551f2017-10-30 08:43:23771the case, please do not use it. For example, if an interface can be rebound,
Oksana Zhuravlova9f3b8ef2019-08-26 20:27:40772then use the singular `mojo::Receiver` and simply `reset()` the existing receiver
Daniel Chengd21b551f2017-10-30 08:43:23773before reusing it.
774
775
Daniel Chengf48fd972017-11-21 19:15:53776### Explicitly reject bad input
777
778While validation should be done inside `StructTraits` specializations when
779possible, there are situations where additional checks, e.g. overflow checks,
780are needed outside of `StructTraits` specializations. Use
781`mojo::ReportBadMessage()` or `mojo::GetBadMessageCallback()` to reject bad
782input in these situations. Under the hood, this may record UMAs, kill the
783process sending bad input, et cetera.
784
785* `mojo::ReportBadMessage()`: use to report bad IPC input while a message is
786 being dispatched on the stack.
787* `mojo::GetBadMessageCallback()`: use to generate a callback to report bad
788 IPC input. The callback must be generated while a message is being
789 dispatched on the stack; however, the returned callback may be invoked be
790 freely invoked in asynchronously posted callbacks.
791
792
Daniel Chengd21b551f2017-10-30 08:43:23793## Java Best Practices
794
795Unfortunately, there are no strongly established conventions here. Most code
796tends to write manual conversion helpers and throw an exception on conversion
797failure. See [NfcTypeConverter.java] as one example of how to write conversion
798code.
799
800
801## General Code Health
802
803
804### Naming Conventions
805
806Place mojo types in `<top-level namespace>.mojom`. Directories for Mojo traits
807should be named `mojom` (preferable) or `ipc`. Legacy names that are also
808encountered are `public/interfaces`, `interfaces`, or just `mojo`.
809
810`mojom` is preferred for consistency between the directory name and the nested
811namespace name.
812
813
814### Do not handle impossible situations
815
816Do not clutter the code by handling impossible situations. Omitting it makes
817the invariants clear. This takes two different forms:
818
819* A less trustworthy process can be compromised by an adversary and send
820 arbitrary data. When processing data from a less trustworthy process, do
821 not attempt to handle this invalid data: just call
822 `mojo::ReportBadMessage()`. When invoked in the context of processing an
823 IPC from the renderer, this will kill the renderer process.
824* A more trustworthy process must be trusted, by definition. Do not write
825 code to handle impossible situations "just in case" there's a bug. For
826 example, the renderer class `content::RenderFrameImpl` must always be
827 connected to certain control interfaces in the browser. It does not makes
828 sense to handle a Mojo connection error and try to reconnect: a connection
829 error signals that the browser process is in the process of deleting the
830 frame, and any attempt at reconnecting will never succeed.
831
832
833### Using mojo enums directly when possible
834
835`EnumTraits` generally do not add much value: incoming Mojo enum values are
836already validated before typemapping, so it is guaranteed that the input value
837to `EnumTraits<T>::FromMojom()` is already a valid enum value, so the method
838itself is just a bunch of boilerplate to map between two very similarly named,
839yet slightly different, enums.
840
841To avoid this, prefer to use the Mojo enum directly when possible.
842
843
844### Consider the cost of setting up message pipes
845
846Message pipes are fairly inexpensive, but they are not free either: it takes 6
847control messages to establish a message pipe. Keep this in mind: if the
848interface is used relatively frequently, connecting once and reusing the
849interface pointer is probably a good idea.
850
Alex Gough303f5822022-01-27 05:35:16851## Copy data out of BigBuffer before parsing
852
853[BigBuffer](mojo/public/mojom/base/big_buffer.mojom) uses shared memory to make
854passing large messages fast. When shmem is backing the message, it may be
855writable in the sending process while being read in the receiving process. If a
856BigBuffer is received from an untrustworthy process, you should make a copy of
857the data before processing it to avoid time-of-check time-of-use (TOCTOU) bugs.
858The |size()| of the data cannot be manipulated.
859
Daniel Chengd21b551f2017-10-30 08:43:23860
Chris Palmer76482e62017-12-04 21:05:06861## Ensure An Explicit Grant For WebUI Bindings
862
863WebUI renderers sometimes need to call special, powerful IPC endpoints in a
864privileged process. It is important to enforce the constraint that the
865privileged callee previously created and blessed the calling process as a WebUI
866process, and not as a (potentially compromised) web renderer or other
867low-privilege process.
868
869* Use the standard pattern for instantiating `MojoWebUIController`. WebUI
Chris Palmerd40cda92017-12-04 21:12:46870Mojo interfaces must only be exposed through a `MojoWebUIController` subclass.
Chris Palmer76482e62017-12-04 21:05:06871* If there is external functionality that the WebUI needs, make sure to route
872it through the Mojo interfaces implemented by the `MojoWebUIController`, to
873avoid circumventing access checks.
874
875
876## Not-Yet-Shipped Features Should Be Feature-Checked On The Privileged Side
877
878Sometimes, there will be powerful new features that are not yet turned on by
879default, such as behind a flag, Finch trial, or [origin
880trial](https://www.chromium.org/blink/origin-trials). It is not safe to check
881for the feature's availability on the renderer side (or in another low-privilege
882process type). Instead, ensure that the check is done in the process that has
883power to actually enact the feature. Otherwise, a compromised renderer could opt
884itself in to the feature! If the feature might not yet be fully developed and
885safe, vulnerabilities could arise.
886
887
Daniel Chengd21b551f2017-10-30 08:43:23888[security-tips-for-ipc]: https://www.chromium.org/Home/chromium-security/education/security-tips-for-ipc
889[NfcTypeConverter.java]: https://chromium.googlesource.com/chromium/src/+/e97442ee6e8c4cf6bcf7f5623c6fb2cc8cce92ac/services/device/nfc/android/java/src/org/chromium/device/nfc/NfcTypeConverter.java
Oksana Zhuravlovaa77a9a12021-05-07 18:02:10890[mojo-doc-process-crashes]: https://chromium.googlesource.com/chromium/src/+/main/mojo/public/cpp/bindings#Best-practices-for-dealing-with-process-crashes-and-callbacks
Lukasz Anforowiczbb99a8562019-01-25 19:19:38891[serialize-struct-tm-safely]: https://chromium-review.googlesource.com/c/chromium/src/+/679441