Re: [BuildStream] Protect against plugin modifications of artifacts

[Tristan Van Berkom <tristan vanberkom codethink co uk>]

Hi again,

> On Jun 22, 2020, at 10:50 PM, William Salmon <will salmon codethink co uk> wrote:
>
> 
>
> > On 22/06/2020 14:26, Tristan Van Berkom wrote:
> > Hi,
> > > > On Jun 22, 2020, at 9:37 PM, William Salmon <will salmon codethink co uk> wrote:
> > >
> > > 
> > >
> > > > On 22/06/2020 12:24, Tristan Van Berkom wrote:
> > > > Hi William,
> > > > Thanks for engaging in this.
> > > > > On Mon, 2020-06-22 at 11:02 +0100, William Salmon wrote:
> > > > > Reply in line.
> > > > [...]
> > > [..]
> > > > The main differences here are:
> > > >   A.) The guarantees we can provide about running safe sandboxed code
> > > >       compared with untrusted local python interpreter code.
> > > >       This would lead us down a path of providing ever more carefully
> > > >       crafted APIs in the core in order to produce reproducible output
> > > >       in the artifact, all the while not being certain what python APIs
> > > >       a plugin might use to produce output.
> > >
> > > Else were you mention that you don't want to get in to a debate about if we can trust python. So lets 
> > > not, lets decide were things should live and then decide if we need to change our plugin language 
> > > separately.
> > >
> > > If a plugin creates a config file with logic dependent on config options and variables etc and then the 
> > > plugin adds a configure command that calls cat to add this text to a file. This seems just as susceptible 
> > > to plugin author or plugin language issues as if the plugin directly adds a file to the cas (via the nice 
> > > api we have now) so it can be included in the sandbox.
> > >
> > > These seem equivalent except that using cat is more complex and requires cat be a build dependency. There 
> > > are many alternatives to cat but they still require some extra bin be in the sandbox and adding new 
> > > configure commands.
> > It is not equivalent.
> > `cat`, or whatever you use inside the sandbox, is part of a controlled environment, the python code your 
> > plugin is running, is not.
>
> Yes but sometimes the logic to generate the conf is non trivial so in those cases were the plugin needs to 
> effect the text, the danger is in creating the PLUGIN_GENERATED_CONTENT and is a separate issue.
>
> Once PLUGIN_GENERATED_CONTENT is created we can ether
>
> configuration_commands.append("""
> cat > /path/to/build/file.conf << EOF
> """ + PLUGIN_GENERATED_CONTENT + """
> EOF
> """)
>
> or
>
> file.write(PLUGIN_GENERATED_CONTENT)

Definitely agree.

The thing is, both are highly problematic.

The fact that generating data in python is problematic is not a reason to avoid fixing illegal writing to the 
sandbox (which probably mostly happens *due* to the latter problem), its a reason to ensure that we *also* 
ensure that doesn’t happen.

Maybe the key here is to ensure a safe and stable way for a core maintained function to reliably export 
public data to element assemble methods. In doing so, we can provide any context which elements need to 
generate output based on this public data safely, and this would be done by a single, well tested codepath 
ensuring that public data is never exported differently with incoming future python versions.

Cheers,
    -Tristan

PS: I think this whole thread also calls mutability of public data by plugin code into question, if I’m not 
mistaken this only happens for the sake of Debian build plugins, which could otherwise be transporting this 
context directly through artifact payload data.