pmbootstrap-meow/pmb/build/other.py
Oliver Smith 3666388619
Properly escape commands in pmb.chroot.user() (#1316)
## Introduction
In #1302 we noticed that `pmb.chroot.user()` does not escape commands
properly: When passing one string with spaces, it would pass them as
two strings to the chroot. The use case is passing a description with
a space inside to `newapkbuild` with `pmboostrap newapkbuild`.

This is not a security issue, as we don't pass strings from untrusted
input to this function.

## Functions for running commands in pmbootstrap
To put the rest of the description in context: We have four high level
functions that run commands:
* `pmb.helpers.run.user()`
* `pmb.helpers.run.root()`
* `pmb.chroot.root()`
* `pmb.chroot.user()`

In addition, one low level function that the others invoke:
* `pmb.helpers.run.core()`

## Flawed test case
The issue described above did not get detected for so long, because we
have a test case in place since day one, which verifies that all of the
functions above escape everything properly:
* `test/test_shell_escape.py`

So the test case ran a given command through all these functions, and
compared the result each time. However, `pmb.chroot.root()`
modified the command variable (passed by reference) and did the
escaping already, which means `pmb.chroot.user()` running directly
afterwards only returns the right output when *not* doing any escaping.

Without questioning the accuracy of the test case, I've escaped
commands and environment variables with `shlex.quote()` *before*
passing them to `pmb.chroot.user()`. In retrospective this does not
make sense at all and is reverted with this commit.

## Environment variables
By coincidence, we have only passed custom environment variables to
`pmb.chroot.user()`, never to the other high level functions. This only
worked, because we did not do any escaping and the passed line gets
executed as shell command:
```
$ MYENV=test echo test2
test 2
```
If it was properly escaped as one shell command:
```
$ 'MYENV=test echo test2'
sh: MYENV=test echo test2: not found
```
So doing that clearly doesn't work anymore. I have added a new `env`
parameter to `pmb.chroot.user()` (and to all other high level functions
for consistency), where environment variables can be passed as a
dictionary. Then the function knows what to do and we end up with
properly escaped commands and environment variables.

## Details
* Add new `env` parameter to all high level command execution functions
* New `pmb.helpers.run.flat_cmd()` function, that takes a command as
  list and environment variables as dict, and creates a properly escaped
  flat string from the input.
* Use that function for proper escaping in all high level exec funcs
* Don't escape commands *before* passing them to `pmb.chroot.user()`
* Describe parameters of the command execution functions
* `pmbootstrap -v` writes the exact command to the log that was
  executed (in addition to the simplified form we always write down for
  readability)
* `test_shell_escape.py`: verify that the command passed by reference
  has not been modified, add a new test for strings with spaces, add
  tests for new function `pmb.helpers.run.flat_cmd()`
* Remove obsolete commend in `pmb.chroot.distccd` about environment
  variables, because we don't use any there anymore
* Add `TERM=xterm` to default environment variables in the chroot,
  so running ncurses applications like `menuconfig` and `nano` works out of
  the box
2018-03-10 22:58:39 +00:00

218 lines
8 KiB
Python

"""
Copyright 2018 Oliver Smith
This file is part of pmbootstrap.
pmbootstrap is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
pmbootstrap is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with pmbootstrap. If not, see <http://www.gnu.org/licenses/>.
"""
import glob
import logging
import os
import shlex
import pmb.build.other
import pmb.chroot
import pmb.helpers.file
import pmb.helpers.git
import pmb.helpers.run
import pmb.parse.apkindex
import pmb.parse.version
def find_aport(args, package, must_exist=True):
"""
Find the aport, that provides a certain subpackage.
:param must_exist: Raise an exception, when not found
:returns: the full path to the aport folder
"""
# Try to get a cached result first (we assume, that the aports don't change
# in one pmbootstrap call)
ret = None
if package in args.cache["find_aport"]:
ret = args.cache["find_aport"][package]
else:
# Sanity check
if "*" in package:
raise RuntimeError("Invalid pkgname: " + package)
# Search in packages
paths = glob.glob(args.aports + "/*/" + package)
if len(paths) > 2:
raise RuntimeError("Package " + package + " found in multiple"
" aports subfolders. Please put it only in one"
" folder.")
elif len(paths) == 1:
ret = paths[0]
else:
# Search in subpackages
for path_current in glob.glob(args.aports + "/*/*/APKBUILD"):
apkbuild = pmb.parse.apkbuild(args, path_current)
if (package in apkbuild["subpackages"] or
package in apkbuild["provides"]):
ret = os.path.dirname(path_current)
break
# Crash when necessary
if ret is None and must_exist:
raise RuntimeError("Could not find aport for package: " +
package)
# Save result in cache
args.cache["find_aport"][package] = ret
return ret
def copy_to_buildpath(args, package, suffix="native"):
# Sanity check
aport = find_aport(args, package)
if not os.path.exists(aport + "/APKBUILD"):
raise ValueError("Path does not contain an APKBUILD file:" +
aport)
# Clean up folder
build = args.work + "/chroot_" + suffix + "/home/pmos/build"
if os.path.exists(build):
pmb.chroot.root(args, ["rm", "-rf", "/home/pmos/build"],
suffix=suffix)
# Copy aport contents
pmb.helpers.run.root(args, ["cp", "-r", aport + "/", build])
pmb.chroot.root(args, ["chown", "-R", "pmos:pmos",
"/home/pmos/build"], suffix=suffix)
def is_necessary(args, arch, apkbuild, indexes=None):
"""
Check if the package has already been built. Compared to abuild's check,
this check also works for different architectures, and it recognizes
changed files in an aport folder, even if the pkgver and pkgrel did not
change.
:param arch: package target architecture
:param apkbuild: from pmb.parse.apkbuild()
:param indexes: list of APKINDEX.tar.gz paths
:returns: boolean
"""
# Get package name, version, define start of debug message
package = apkbuild["pkgname"]
version_new = apkbuild["pkgver"] + "-r" + apkbuild["pkgrel"]
msg = "Build is necessary for package '" + package + "': "
# Get old version from APKINDEX
index_data = pmb.parse.apkindex.package(args, package, arch, False,
indexes)
if not index_data:
logging.debug(msg + "No binary package available")
return True
# a) Binary repo has a newer version
version_old = index_data["version"]
if pmb.parse.version.compare(version_old, version_new) == 1:
logging.warning("WARNING: Package '" + package + "' in your aports folder"
" has version " + version_new + ", but the binary package"
" repositories already have version " + version_old + "!"
" See also: <https://postmarketos.org/warning-repo2>")
return False
# b) Aports folder has a newer version
if version_new != version_old:
logging.debug(msg + "Binary package out of date (binary: " + version_old +
", aport: " + version_new + ")")
return True
# Aports and binary repo have the same version.
return False
def index_repo(args, arch=None):
"""
Recreate the APKINDEX.tar.gz for a specific repo, and clear the parsing
cache for that file for the current pmbootstrap session (to prevent
rebuilding packages twice, in case the rebuild takes less than a second).
:param arch: when not defined, re-index all repos
"""
pmb.build.init(args)
if arch:
paths = [args.work + "/packages/" + arch]
else:
paths = glob.glob(args.work + "/packages/*")
for path in paths:
if os.path.isdir(path):
path_arch = os.path.basename(path)
path_repo_chroot = "/home/pmos/packages/pmos/" + path_arch
logging.debug("(native) index " + path_arch + " repository")
commands = [
# Wrap the index command with sh so we can use '*.apk'
["sh", "-c", "apk -q index --output APKINDEX.tar.gz_"
" --rewrite-arch " + shlex.quote(path_arch) + " *.apk"],
["abuild-sign", "APKINDEX.tar.gz_"],
["mv", "APKINDEX.tar.gz_", "APKINDEX.tar.gz"]
]
for command in commands:
pmb.chroot.user(args, command, working_dir=path_repo_chroot)
else:
logging.debug("NOTE: Can't build index for: " + path)
pmb.parse.apkindex.clear_cache(args, path + "/APKINDEX.tar.gz")
def configure_abuild(args, suffix, verify=False):
"""
Set the correct JOBS count in abuild.conf
:param verify: internally used to test if changing the config has worked.
"""
path = args.work + "/chroot_" + suffix + "/etc/abuild.conf"
prefix = "export JOBS="
with open(path, encoding="utf-8") as handle:
for line in handle:
if not line.startswith(prefix):
continue
if line != (prefix + args.jobs + "\n"):
if verify:
raise RuntimeError("Failed to configure abuild: " + path +
"\nTry to delete the file (or zap the chroot).")
pmb.chroot.root(args, ["sed", "-i", "-e",
"s/^" + prefix + ".*/" + prefix + args.jobs + "/",
"/etc/abuild.conf"], suffix)
configure_abuild(args, suffix, True)
return
raise RuntimeError("Could not find " + prefix + " line in " + path)
def configure_ccache(args, suffix="native", verify=False):
"""
Set the maximum ccache size
:param verify: internally used to test if changing the config has worked.
"""
# Check if the settings have been set already
arch = pmb.parse.arch.from_chroot_suffix(args, suffix)
path = args.work + "/cache_ccache_" + arch + "/ccache.conf"
if os.path.exists(path):
with open(path, encoding="utf-8") as handle:
for line in handle:
if line == ("max_size = " + args.ccache_size + "\n"):
return
if verify:
raise RuntimeError("Failed to configure ccache: " + path + "\nTry to"
" delete the file (or zap the chroot).")
# Set the size and verify
pmb.chroot.user(args, ["ccache", "--max-size", args.ccache_size],
suffix)
configure_ccache(args, suffix, True)