[pacman-dev] Set "secure" $HOME

Message ID 20190812164540.5929-1-jonathon@manjaro.org
State Rejected, archived
Headers show
Series
  • [pacman-dev] Set "secure" $HOME
Related show

Commit Message

Jonathon Fernyhough Aug. 12, 2019, 4:45 p.m. UTC
By default, $HOME is that of the build user. This is usually not a
problem in ephemeral build containers/chroots but can allow some files
to escape into the filesystem where `makepkg` is run outside of a chroot.

There can also be instances of generated files (e.g. cache, precompiled
bytecode) being placed into these locations and which are then relied
upon by other parts of the software.

As a concrete example, Perl6 and nim have a precompilation cache (though
well-behaved in how it is used).

`export HOME=/nonexistent` is already used by Debian in their build
tooling and so does not represent a divergence from established
practice. It also allows for badly-behaved build processes to be more
easily spotted and an issue filed upstream where appropriate.

Signed-off-by: Jonathon Fernyhough <jonathon@manjaro.org>
---
 scripts/makepkg.sh.in | 2 ++
 1 file changed, 2 insertions(+)

Comments

Levente Polyak Aug. 12, 2019, 5:05 p.m. UTC | #1
On August 12, 2019 6:45:40 PM GMT+02:00, Jonathon Fernyhough <jonathon@manjaro.org> wrote:
>By default, $HOME is that of the build user. This is usually not a
>problem in ephemeral build containers/chroots but can allow some files
>to escape into the filesystem where `makepkg` is run outside of a
>chroot.
>
>There can also be instances of generated files (e.g. cache, precompiled
>bytecode) being placed into these locations and which are then relied
>upon by other parts of the software.
>
>As a concrete example, Perl6 and nim have a precompilation cache
>(though
>well-behaved in how it is used).
>
>`export HOME=/nonexistent` is already used by Debian in their build
>tooling and so does not represent a divergence from established
>practice. It also allows for badly-behaved build processes to be more
>easily spotted and an issue filed upstream where appropriate.
>
>Signed-off-by: Jonathon Fernyhough <jonathon@manjaro.org>
>---
> scripts/makepkg.sh.in | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>index 43484db3..04edc38a 100644
>--- a/scripts/makepkg.sh.in
>+++ b/scripts/makepkg.sh.in
>@@ -40,6 +40,8 @@ export COMMAND_MODE='legacy'
> unset CDPATH
> # Ensure GREP_OPTIONS doesn't screw with our grep calls
> unset GREP_OPTIONS
>+# Prevent build-user-specific files "escaping" into the filesystem
>+export HOME=/nonexistent
> 
> declare -r makepkg_version='@PACKAGE_VERSION@'
> declare -r confdir='@sysconfdir@'

Just a side note but Debian has a macro based system that sets vars and options as needed and doesn't require every pkgbuild to handle them case by case.
No judgment here, but its not always borken package upstream but this will also have effect on maven, gradle and other generic build systems that will be affected and would be a breaking change that downstream must then incooperate.
Eli Schwartz Aug. 12, 2019, 5:14 p.m. UTC | #2
On August 12, 2019 12:45:40 PM EDT, Jonathon Fernyhough <jonathon@manjaro.org> wrote:
> By default, $HOME is that of the build user. This is usually not a
> problem in ephemeral build containers/chroots but can allow some files
> to escape into the filesystem where `makepkg` is run outside of a
> chroot.
> 
> There can also be instances of generated files (e.g. cache,
> precompiled
> bytecode) being placed into these locations and which are then relied
> upon by other parts of the software.
> 
> As a concrete example, Perl6 and nim have a precompilation cache
> (though
> well-behaved in how it is used).
> 
> `export HOME=/nonexistent` is already used by Debian in their build
> tooling and so does not represent a divergence from established
> practice. It also allows for badly-behaved build processes to be more
> easily spotted and an issue filed upstream where appropriate.

I don't understand why you think it's a good idea to quarantine makepkg from knowing where HOME is, when makepkg is one of the things that needs to read a file from HOME.

That is to say, your patch will break reading $HOME/.makepkg.conf, and also break reading $HOME/.config/pacman/makepkg.conf if $XDG_CONFIG_HOME is unset. But that raises the point that your patch also doesn't actually quarantine makepkg from knowing where XDG-respecting programs store and write to their content.

More generally, your patch is trying to solve the problem for which we've always said "well, uh, that's why build chroots exist". What would you like to unset next?

/usr/local
PATH=$HOME/bin:$PATH
General variables defined in bashrc based on HOME, but now containing hardcodes paths? Maybe we should do full environment cleaning. Maybe we should just implement a systemd-nspawn wrapper directly in makepkg.

... Or we can continue as we do today. makepkg provides build orchestration, and makechrootpkg provides isolation.

Patch

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 43484db3..04edc38a 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -40,6 +40,8 @@  export COMMAND_MODE='legacy'
 unset CDPATH
 # Ensure GREP_OPTIONS doesn't screw with our grep calls
 unset GREP_OPTIONS
+# Prevent build-user-specific files "escaping" into the filesystem
+export HOME=/nonexistent
 
 declare -r makepkg_version='@PACKAGE_VERSION@'
 declare -r confdir='@sysconfdir@'