git-build-recipe

Code review comment for ~tintou/git-build-recipe:master

Revision history for this message

Colin Watson (cjwatson) wrote on 2018-08-29:

Hm. There are going to be some more serious problems too.

URL filtering
-------------

In general, when we have user-supplied URLs, we normally filter them so that people can't e.g. pass in file:/// URLs and use those to construct some kind of exploit against the local system. For URLs in the recipe itself, we handle those by parsing the recipe in Launchpad and only allowing references to repositories hosted on Launchpad. But for submodules, that typically doesn't work; we'll need to expect that repositories have references to external URLs, and it's hard to filter those in advance (especially for the recursive strategy).

Now, it's possible we can get away without caring about this, since the builders on which we run git-build-recipe are throwaway VMs that don't have any secrets. But it needs to be documented for the benefit of other users of git-build-recipe, and it would be good if you could do some research to find if there's any reasonable way to filter submodule URLs before cloning them.

Recipe parsing in Launchpad
---------------------------

Launchpad currently takes the shortcut of parsing the supplied recipe using bzr-builder, and then mangling it slightly to account for the minor differences in git-build-recipe's format. We'll need to check whether that process needs any changes to account for this extension to the format. I think we ought to check that before landing this, because perhaps there will turn out to be some way to extend the format that's more compatible than others.

External access from Launchpad builders
---------------------------------------

The bad news is that this is the hard bit. The good news is that it doesn't need to block landing this code; it may well make the feature less useful until something's done about it, but the fixes for this won't be in git-build-recipe.

In general, Launchpad builders don't have access to the external internet. So this will for submodules that are hosted on Launchpad (using lp: or https://git.launchpad.net/ in submodule URL configuration), but it won't work for external URLs.

We'll need to have a policy discussion about whether we want to allow these on Launchpad. I'm leaning towards yes, because git submodules are pinned by their commit ID which is pretty good protection against the usual problems with external access for things like build reproducibility, but we still need to have the discussion in the Launchpad team and reach an agreement.

After that, there would be technical work to be done to implement this. We could reuse the proxy that we use to allow snap builders to have external internet access, perhaps dispatching it only to recipes that have a submodule-strategy. That would require changes in lib/lp/code/model/recipebuilder.py (similar to lib/lp/snappy/model/snapbuildbehaviour.py) in lp:launchpad, and to the corresponding bits of lp:launchpad-buildd.

Hm.  There are going to be some more serious problems too.

URL filtering
-------------

In general, when we have user-supplied URLs, we normally filter them so that people can't e.g. pass in file:/// URLs and use those to construct some kind of exploit against the local system.  For URLs in the recipe itself, we handle those by parsing the recipe in Launchpad and only allowing references to repositories hosted on Launchpad.  But for submodules, that typically doesn't work; we'll need to expect that repositories have references to external URLs, and it's hard to filter those in advance (especially for the recursive strategy).

Now, it's possible we can get away without caring about this, since the builders on which we run git-build-recipe are throwaway VMs that don't have any secrets.  But it needs to be documented for the benefit of other users of git-build-recipe, and it would be good if you could do some research to find if there's any reasonable way to filter submodule URLs before cloning them.

Recipe parsing in Launchpad
---------------------------

Launchpad currently takes the shortcut of parsing the supplied recipe using bzr-builder, and then mangling it slightly to account for the minor differences in git-build-recipe's format.  We'll need to check whether that process needs any changes to account for this extension to the format.  I think we ought to check that before landing this, because perhaps there will turn out to be some way to extend the format that's more compatible than others.

External access from Launchpad builders
---------------------------------------

The bad news is that this is the hard bit.  The good news is that it doesn't need to block landing this code; it may well make the feature less useful until something's done about it, but the fixes for this won't be in git-build-recipe.

In general, Launchpad builders don't have access to the external internet.  So this will for submodules that are hosted on Launchpad (using lp: or https://git.launchpad.net/ in submodule URL configuration), but it won't work for external URLs.

We'll need to have a policy discussion about whether we want to allow these on Launchpad.  I'm leaning towards yes, because git submodules are pinned by their commit ID which is pretty good protection against the usual problems with external access for things like build reproducibility, but we still need to have the discussion in the Launchpad team and reach an agreement.

After that, there would be technical work to be done to implement this.  We could reuse the proxy that we use to allow snap builders to have external internet access, perhaps dispatching it only to recipes that have a submodule-strategy.  That would require changes in lib/lp/code/model/recipebuilder.py (similar to lib/lp/snappy/model/snapbuildbehaviour.py) in lp:launchpad, and to the corresponding bits of lp:launchpad-buildd.

« Back to merge proposal