1353040 - Switch from using string interpolation with sandbox_init to sandbox_init_with_parameters

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement)

Description

[Alex Gaynor [:Alex_Gaynor]]

Using sandbox_init_with_parameters with save us from some of the slightly messy interpolation.

It does not appear to be documented (but then, basically none of the sandbox infrastructure is!), however both Chrome and WebKit appear to use it, so I think it's safe.

Comment 1

[Alex Gaynor [:Alex_Gaynor]]

Attachment: Bug 1353040 - Switch from sandbox_init to sandbox_init_with_parameters

This API produces much more readable code (though slightly more verbose). While this is not a publicly documented API on macOS, it is used by both WebKit and Chrome.

There are two parameters which I have not ported to this API, because I am not sure how to handle integers best yet.

Review commit: https://reviewboard.mozilla.org/r/126060/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/126060/

Comment 2

[Haik Aftandilian [:haik]]

Comment on attachment 8854077 [details]
Bug 1353040 - Switch from sandbox_init to sandbox_init_with_parameters

https://reviewboard.mozilla.org/r/126060/#review128668

r+ For the approach because named parameters are less error prone. Please add some support to make printing the policy for debugging straightforward. I think printing the params vector followed by the policy would be sufficient. No need to do string interpolation. And then re-request review. Thanks.

Once concern I mentioned on IRC was that with sandbox_init_with_parameters, we don't get to see the final policy string before passing it of to the OS. i.e., we don't get to see exactly how the argument strings are interpolated into the policy.

Comment 3

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8854077 [details]
Bug 1353040 - Switch from sandbox_init to sandbox_init_with_parameters

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/126060/diff/1-2/

Comment 4

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8854077 [details]
Bug 1353040 - Switch from sandbox_init to sandbox_init_with_parameters

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/126060/diff/2-3/

Comment 5

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8854077 [details]
Bug 1353040 - Switch from sandbox_init to sandbox_init_with_parameters

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/126060/diff/3-4/

Comment 6

[Haik Aftandilian [:haik]]

Comment on attachment 8854077 [details]
Bug 1353040 - Switch from sandbox_init to sandbox_init_with_parameters

https://reviewboard.mozilla.org/r/126060/#review128996

Comment 7

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8854077 [details]
Bug 1353040 - Switch from sandbox_init to sandbox_init_with_parameters

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/126060/diff/4-5/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/07cf34197c32
Switch from sandbox_init to sandbox_init_with_parameters. r=haik

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/07cf34197c32