-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On Tue, Aug 16, 2016 at 04:14:00AM +0000, HW42 wrote:
> Andrew David Wong:
> > A new announcement, "Security challenges for the Qubes build process,"
> > has just been posted on the Qubes website:
> >
> >
https://www.qubes-os.org/news/2016/05/30/build-security/
>
> Hi,
>
> As suggested in this post I implemented a append-only log for
> qubes-builder.
Thanks!
> Instead of one log file as the 'qubes.AppendLog' name in the post
> suggest I decided to create a separate log file per make invocation to
> keep it a bit simpler.
>
> The attached patch consists of three parts:
>
> 1) A little wrapper GNUmakefile which calls the logging script. This is
> a bit ugly but has the advantage that 'make' calls work as before.
Is it possible to somehow integrate it into main Makefile?
> 2) The logging script logs some information about make invocation and
> the current state of the git repos and then calls the real qubes-builder
> Makefile. It is also responsible to copy the output to the remote
> domain.
>
> 3) The rpc service which runs in the log vm.
>
> To enable logging set QUBES_BUILD_LOG_CMD in the build vm to
> 'qrexec-client-vm logs qubesbuilder.BuildLog' (probably in
> .bash_profile).
>
> When you installed the rpc service in the log vm and added a matching
> policy you now get a log file for every make invocation in
> ~/QubesIncomingBuildLog/build-vm/.
>
> For the official releases I think we should have a git repo where the
> logs for each published packages is kept (Maybe using a separate key to
> sign this is a good idea).
Yes, this is good idea.
In such a case I think the service should handle this automatically -
commit newly added log, tag and push. Maybe simply add a way to call
some post-script from there? Or configure it with some env variable?
Additionally it would be nice to somehow ease correlation of package
with its build log. Doing that only based on timestamp may not be so
simple in some cases. For example `make qubes` can take some time and
will produce a single log for a whole set of packages.
But that's a minor issue, one can always use grep ;)
> The attached patch work in principal but might need some fine tuning.
>
> a) Do you prefer the GNUmakefile trick or calling the script directly?
If integrating into main makefile would be even uglier, better call it
directly - I can always add a shell alias :)
> b) What additional info should be logged? Currently the following is
> logged:
>
> - make arguments
> - hash of each git HEAD
> - status and hashes of untracked or modified files except for the top
> level repo (this includes downloaded sources)
Take a look at `make build-id`. It does two things:
- refuse to continue in case of any unclean git repo
- prints version tag (if present) or hash of HEAD for each repo
It may make sense to add a mode to always print hash instead of version
tag. Currently it doesn't handle untracked files (downloaded sources
etc), but IMO it's better to extend `make build-id` instead of
duplicating it.
> - make build-info
> - make is called with --trace
> - VERBOSE is set to 2 if unset
What `make --trace` gives more than VERBOSE=2?
> Do you think more or less should be logged (for example the untracked
> files are a bit too verbose. logging builder.conf might be a good idea)?
As for builder.conf - I think make build-info is verbose enough.
Surely I don't want to log the whole builder.conf verbatim, as I have
there github API token (for qubes-builder-github). Ok, I can
move it elsewhere (.bash_profile, another file included from
builder.conf etc), but that would make things even more complex.
As for untracked files - in ideal world it shouldn't be needed, but in
practice IMO it is good idea. In fact that blog post was exactly because
of an issue with source verification...
Some minor comments inline below.
>
> HW42
>
> From a1f08948ad7a146ac28358cc1858ace9ce74c350 Mon Sep 17 00:00:00 2001
> From: HW42 <
hw...@ipsumj.de>
> Date: Tue, 16 Aug 2016 00:04:16 +0200
> Subject: [PATCH] add tools for append-only build logs
>
> see also
https://www.qubes-os.org/news/2016/05/30/build-security/
> and #2023
> ---
> GNUmakefile | 9 ++++
> rpc-services/qubesbuilder.BuildLog | 87 ++++++++++++++++++++++++++++++++++++++
> scripts/log-git-status | 36 ++++++++++++++++
> scripts/make-with-log | 37 ++++++++++++++++
> 4 files changed, 169 insertions(+)
> create mode 100644 GNUmakefile
> create mode 100755 rpc-services/qubesbuilder.BuildLog
> create mode 100755 scripts/log-git-status
> create mode 100755 scripts/make-with-log
>
> diff --git a/GNUmakefile b/GNUmakefile
> new file mode 100644
> index 0000000..44ca4ca
> --- /dev/null
> +++ b/GNUmakefile
> @@ -0,0 +1,9 @@
> +.PHONY: $(MAKECMDGOALS) __default
> +
> +.NOTPARALLEL:
> +
> +__default $(firstword $(MAKECMDGOALS)):
> + @scripts/make-with-log $(MAKECMDGOALS)
> +
> +$(wordlist 2, $(words $(MAKECMDGOALS)), $(MAKECMDGOALS)):
> + @# ignore
Does it preserve variables set on make command line (`make
COMPONENTS=core-admin qubes`)? I'd expect so, but want to make sure.
> diff --git a/rpc-services/qubesbuilder.BuildLog b/rpc-services/qubesbuilder.BuildLog
> new file mode 100755
> index 0000000..291c8f7
> --- /dev/null
> +++ b/rpc-services/qubesbuilder.BuildLog
> @@ -0,0 +1,87 @@
> +#!/usr/bin/env python
> +
> +from __future__ import print_function
> +
> +import tempfile
> +import io
> +import sys
> +import os
> +import errno
> +import shutil
> +from datetime import datetime
> +
> +def sanitize_line(untrusted_line):
> + line = bytearray(untrusted_line)
> + for i, c in enumerate(line):
> + if c >= 0x20 and c <= 0x7e:
> + pass
> + else:
> + line[i] = 0x2e
> + return bytearray(line).decode('ascii')
> +
> +sys.stdin = io.open(0, 'rb')
Why?
> +
> +start = datetime.utcnow()
> +
> +tmp_log = tempfile.NamedTemporaryFile(prefix="qubes-build-log_", delete=False)
> +
> +qrexec_remote = os.getenv('QREXEC_REMOTE_DOMAIN')
> +if not qrexec_remote:
> + print('ERROR: QREXEC_REMOTE_DOMAIN not set', file=sys.stderr)
> + sys.exit(1)
> +
> +
> +def log(msg, remote=True, now=datetime.utcnow()):
> + if remote:
> + remote_str = '{}:'.format(qrexec_remote)
> + else:
> + remote_str = '>'
> +
> + line = '{:%Y-%m-%d %H:%M:%S.%f} {} {}\n'.format(now, remote_str, msg)
> +
> + tmp_log.write(line.encode('utf-8'))
> +
> +log('starting log', now=start, remote=False)
> +
> +while True:
> + untrusted_line = sys.stdin.readline()
> + if untrusted_line == b'':
> + break
Can be folded into:
for line in iter(sys.stdin.readline, b''):
> +
> + log(sanitize_line(untrusted_line.rstrip(b'\n')))
> +
> +
> +log('closing log', remote=False)
> +tmp_log.close()
> +
> +file_name_base = ('{home}/QubesIncomingBuildLog/{remote}/'
> + '{remote}_{time:%Y-%m-%d_%H-%M-%S}').format(
> + home=os.getenv('HOME', '/'),
> + remote=qrexec_remote,
> + time=start)
os.path.expanduser('~/QubesIncomingBuildLog/...'..)
> +
> +try:
> + os.makedirs(os.path.dirname(file_name_base))
> +except OSError as err:
> + if err.errno != errno.EEXIST:
> + raise
> +
> +try_no = 0
> +while True:
> + if try_no == 0:
> + file_name = file_name_base
> + else:
> + file_name = '{}.{}'.format(file_name_base, try_no)
> +
> + try:
> + fd = os.open(file_name, os.O_CREAT | os.O_EXCL, 0o664)
> + except OSError as err:
> + if err.errno == errno.EEXIST:
> + try_no += 1
> + continue
> + raise
> +
> + os.close(fd)
> + break
> +
> +shutil.move(
tmp_log.name, file_name)
> diff --git a/scripts/log-git-status b/scripts/log-git-status
> new file mode 100755
> index 0000000..a5f16ea
> --- /dev/null
> +++ b/scripts/log-git-status
> @@ -0,0 +1,36 @@
> +#!/bin/bash
> +
> +set -e
> +set -o pipefail
> +shopt -s nullglob
> +
> +script_path="$(readlink -f "$0")"
> +base_dir="$(readlink -f "$(dirname "$script_path")/..")"
> +
> +repo_abs=.
> +if [ $# -gt 0 ]; then
> + repo_abs="$1"
> +fi
> +cd "$repo_abs"
> +repo_abs="$(readlink -f $(git rev-parse --show-toplevel))"
> +
> +
> +repo="${repo_abs#$base_dir}"
> +
> +echo ">> qubes-builder$repo $(git rev-parse 'HEAD^{}')"
> +
> +ignore_args=""
> +if [ -n "$repo" ]; then
> + ignore_args="--ignored"
> +fi
> +
> +git status --porcelain -uall $ignore_args --ignore-submodules=all
> +git status -z -uall $ignore_args --ignore-submodules=all | sed -zn 'h; /^R/n; g; s/^...//; p' | xargs -r0 sha512sum
> +
> +if [ -z "$repo" ]; then
> + for d in qubes-src/*/; do
> + $script_path $d
> + done
> +else
> + git submodule -q foreach $script_path
> +fi
> diff --git a/scripts/make-with-log b/scripts/make-with-log
> new file mode 100755
> index 0000000..4f9c791
> --- /dev/null
> +++ b/scripts/make-with-log
> @@ -0,0 +1,37 @@
> +#!/bin/bash
> +
> +set -e
> +set -o pipefail
> +
> +cd $(dirname "$0")/..
> +
> +# hide wrapper GNUmakefile
> +if [ -n "$MAKELEVEL" ]; then
> + (( MAKELEVEL-=1 )) || true
> +fi
> +
> +if [ -z "$QUBES_BUILD_LOG_CMD" ]; then
> + make -f Makefile "$@"
> + exit $?
> +fi
> +
> +exec {real_stdout_fd}>&1
> +
> +(
> +export LC_ALL=C.UTF-8
> +echo "> starting build with log"
> +echo ">> make args: $*"
> +for v in MAKEFLAGS; do
> + echo ">> $v=${!v}"
> +done
> +$(dirname $0)/log-git-status
> +make -f Makefile build-info NO_COLOR=1
> +
> +if [ -z "$VERBOSE"]; then
> + export VERBOSE=2
> +fi
> +
> +echo "> running make"
> +make -f Makefile --trace "$@"
> +echo "> done"
> +) 2>&1 | tee /dev/fd/$real_stdout_fd | eval "$QUBES_BUILD_LOG_CMD"
Is eval really needed here? I think you can use just
$QUBES_BUILD_LOG_CMD here (without quotes).
- --
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAEBCAAGBQJXs02bAAoJENuP0xzK19csLYsH/00hg+papkAKfjlsiaJHwqA1
tM/TOIObb35XLWm+i6ATxhaTA0UD+nYwRBunFewIdI2vrXY99leNuYwZ6Rgf0nm5
HvOBmXU+1HUtJix/oGNcmYRq6pkLtt79lGrV1MaStCrP+UeVn1FrLR1SrCxigFID
yrgufBSFY4Ew5EmZmQJx8w/aDkzv05N51AmQWDkAxJ8n+Nd8N/CxPvryT+yn9Ih+
bq8BKO139guAaQT/pZN7AGfvoZwMiltKUqq5FRAQuwcEtCNDmP24xCenswPtq2EV
nJ+xKVja8xrDVTCjW+GB9soGWeB2tL8fpDmcPqLn5kRsV8O+lEUOxLyGDlAgYXQ=
=ivfw
-----END PGP SIGNATURE-----