Create tool(s) to assist mojom security reviews |
|||||||||||
Issue descriptionI hear the lamentations of security reviewers and they are reasonable: it's hard to look at a CL and understand where and how a new or existing mojom interface may be used, and it's therefore hard to know what level of trust should be assumed in either direction across said interface. Typically this has lead to suggestions of naming conventions (e.g. call "trusted" interfaces "FooHost", mirroring a legacy IPC pattern) or dubious documentation practices (e.g. explicitly claiming within a mojom file that some interface is exposed or consumed by the "browser process" even if it's in foundational service mojom.) These approaches imply that relative trust is a property of the interface definition, and this is something with which I fundamentally disagree. We have all the necessary data to reason precisely and accurately about which interfaces are exposed from one service to another. Direct interface exposure between any two services is declaratively encapsulated by the intersection of their manifest capability specs. Indirect interface access can be deduced by aggregating the transitive closure of all interfaces reachable from the directly-accessible ones.** The trouble is that this analysis is an arduous mental process to perform manually in the course of any given review. We should instead develop a tool (or tools) to aid in this process and ideally even integrate them into gerrit and/or codesearch if possible. ** Not strictly true since anyone can send raw message pipe handles and use them however they like; but legitimate uses of this behavior are rare enough that I believe it is OK for them to warrant special attention in security review. +CC some security and s13n folks
,
Jul 12 2017
,
Jul 21 2017
Assigning this to myself so I can keep my eye on it. For Guts team, we'd like to have this as a Q3 priority, if possible.
,
Jul 21 2017
,
Sep 20 2017
,
Nov 1 2017
,
Nov 3 2017
,
Dec 2 2017
,
Jan 12 2018
,
May 30 2018
,
Jan 2
We have a prototype at https://chromium-ipc.appspot.com. It works by parsing the JSON manifest files and constructing a node graph to visualize them, as well as parsing mojoms to obtain InterfaceRequests. However, with the change to move manifests away from JSON into C++ (see https://groups.google.com/a/chromium.org/d/topic/services-dev/NgZPNHXOZhg/discussion), this will stop working. The changes are definitely for the better, but when I mentioned that the change would break tooling (https://groups.google.com/a/chromium.org/d/msg/services-dev/HOQ_Itm9eUI/f6_WcYxPAwAJ) no solutions were suggested. I discussed briefly with dcheng@ about what a Clang Plugin could do here, but it seems like it would be rather complicated, especially when considering preprocessor defines and anything gated on runtime conditions. As a result, I don't really see a way forward, so I'm going to WontFix this.
,
Jan 2
Archived the source code here for posterity (google.com readable): https://source.cloud.google.com/chromium-ipc/visualizer
,
Jan 7
Hmm, I'm sorry I missed that discussion. I don't think we need to abandon the effort though, and I am not sure why we'd need a clang plugin. Can't we just build a dedicated pre-processing tool in C++ which spits out all the interesting metadata in an easily parsable format (for e.g. a Python tool), using for input the same std::vector<service_manager::Manifest> that that Chrome will end up using in production?
,
Jan 18
(4 days ago)
I did consider something like that too, but it has the same issues that I alluded to above: #ifdefs and any other conditional logic make it difficult to get the full universe of services available. The tool would have to run on all OSes, but that would still ignore things like BUILDFLAG. The other downside is that it requires basically a full build to produce the data, whereas before it only needed a checkout (but that's not insurmountable). For now I think we're okay tabling this because the tool didn't see strong adoption from members of the security team. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by roc...@chromium.org
, Jul 12 2017