From 14a4bad68c2aba934d2c3e4594e3fd0cf2b221ff Mon Sep 17 00:00:00 2001
From: Erik Chen <erikchen@chromium.org>
Date: Wed, 21 Apr 2021 17:53:13 +0000
Subject: [PATCH] Update mojo security documentation.

One of the general security principles is that syntactically correct
messages are semantically valid. Write that explicitly an include an
example for bitfields.

Change-Id: If17ffe152702eb6441a922dae70bb65063c7592f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2822919
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#874759}
---
 docs/security/mojo.md | 81 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/docs/security/mojo.md b/docs/security/mojo.md
index 9876d01758d983..2aededfdb0b81c 100644
--- a/docs/security/mojo.md
+++ b/docs/security/mojo.md
@@ -521,6 +521,87 @@ for (size_t i = 0; i < request->element_size(); ++i) {
 ```
 
 
+### All possible message values are semantically valid
+
+When possible, messages should be defined in such a way that all possible values
+are semantically valid. As a corollary, avoid having the value of one field
+dictate the validity of other fields.
+
+**_Good_**
+
+```c++
+union CreateTokenResult {
+  // Implies success.
+  string token;
+
+  // Implies failure.
+  string error_message;
+};
+
+struct TokenManager {
+  CreateToken() => (CreateTokenResult result);
+};
+```
+
+**_Bad_**
+```c++
+struct TokenManager {
+  // Requires caller to handle edge case where |success| is set to true, but
+  // |token| is null.
+  CreateToken() => (bool success, string? token, string? error_message);
+
+  // Requires caller to handle edge case where both |token| and |error_message|
+  // are set, or both are null.
+  CreateToken() => (string? token, string? error_message);
+};
+```
+
+There are some known exceptions to this rule because mojo does not handle
+optional primitives.
+
+**_Allowed because mojo has no support for optional primitives_**
+```c++
+  struct Foo {
+    int32 x;
+    bool has_x;  // does the value of `x` have meaning?
+    int32 y;
+    bool has_y;  // does the value of `y` have meaning?
+  };
+```
+
+Another common case where we tolerate imperfect message semantics is
+with weakly typed integer [bitfields](#handling-bitfields).
+
+### Handling bitfields
+
+Mojom has no native support for bitfields. There are two common approaches: a
+type-safe struct of bools which is a bit clunky (preferred) and an integer-based
+approach (allowed but not preferred).
+
+**_Type-safe bitfields_**
+```c++
+struct VehicleBits {
+  bool has_car;
+  bool has_bicycle;
+  bool has_boat;
+};
+
+struct Person {
+  VehicleBits bits;
+};
+```
+
+**_Integer based approach_**
+```c++
+struct Person {
+  const uint64 kHasCar = 1;
+  const uint64 kHasBicycle = 2;
+  const uint64 kHasGoat= 4;
+
+  uint32 vehicle_bitfield;
+};
+```
+
 ## C++ Best Practices