lib: Validate metadata structure more consistently during pull
Previously we were doing e.g. `ot_util_filename_validate()` specifically inline in dirtree objects, but only *after* writing them into the staging directory (by default). In (non-default) cases such as not using a transaction, such an object could be written directly into the repo. A notable gap here is that `pull-local --untrusted` was *not* doing this verification, just checksums. We harden that (and also the static delta writing path, really *everything* that calls `ostree_repo_write_metadata()` to also do "structure" validation which includes path traversal checks. Basically, let's try hard to avoid having badly structured objects even in the repo. One thing that sucks in this patch is that we need to allocate a "bounce buffer" for metadata in the static delta path, because GVariant imposes alignment requirements, which I screwed up and didn't fulfill when designing deltas. It actually didn't matter before because we weren't parsing them, but now we are. In theory we could check alignment but ...eh, not worth it, at least not until we change the delta compiler to emit aligned metadata which actually may be quite tricky. (Big picture I doubt this really matters much right now but I'm not going to pull out a profiler yet for this) The pull test was extended to check we didn't even write a dirtree with path traversal into the staging directory. There's a bit of code motion in extracting `_ostree_validate_structureof_metadata()` from `fsck_metadata_object()`. Then `_ostree_verify_metadata_object()` builds on that to do checksum verification too. Closes: #1412 Approved by: jlebon
This commit is contained in:
1 parent
f3ae36ff43
commit
8e6e64a5ad
8 files changed
+138
-54
No files matched your search
+4
-1
@@ -227,7 +227,10 @@ ${CMD_PREFIX} ostree --repo=corruptrepo remote add --set=gpg-verify=false pathtr
|
||||
if ${CMD_PREFIX} ostree --repo=corruptrepo pull pathtraverse pathtraverse-test 2>err.txt; then
|
||||
fatal "Pulled a repo with path traversal in dirtree"
|
||||
fi
|
||||
assert_file_has_content_literal err.txt 'Invalid / in filename ../afile'
|
||||
assert_file_has_content_literal err.txt 'ae9a5d2701a02740aa2ee317ba53b13e3efb0f29609cd4896e1bafeee4caddb5.dirtree: Invalid / in filename ../afile'
|
||||
# And verify we didn't write the object into the staging directory even
|
||||
find corruptrepo/tmp -name '9a5d2701a02740aa2ee317ba53b13e3efb0f29609cd4896e1bafeee4caddb5.dirtree' >find.txt
|
||||
assert_not_file_has_content find.txt '9a5d2701a02740aa2ee317ba53b13e3efb0f29609cd4896e1bafeee4caddb5'
|
||||
rm corruptrepo -rf
|
||||
echo "ok path traversal checked on pull"
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
#!/bin/bash
|
||||
#
|
||||
# Copyright (C) 2014 Alexander Larsson <alexl@redhat.com>
|
||||
# Copyright (C) 2018 Red Hat, Inc.
|
||||
#
|
||||
# This library is free software; you can redistribute it and/or
|
||||
# modify it under the terms of the GNU Lesser General Public
|
||||
@@ -22,7 +23,7 @@ set -euo pipefail
|
||||
|
||||
. $(dirname $0)/libtest.sh
|
||||
|
||||
echo '1..3'
|
||||
echo '1..4'
|
||||
|
||||
setup_test_repository "bare"
|
||||
|
||||
@@ -60,10 +61,20 @@ else
|
||||
fi
|
||||
|
||||
rm -rf repo2
|
||||
mkdir repo2
|
||||
ostree_repo_init repo2 --mode="bare"
|
||||
if ${CMD_PREFIX} ostree --repo=repo2 pull-local --untrusted repo; then
|
||||
assert_not_reached "corrupted untrusted pull unexpectedly failed!"
|
||||
else
|
||||
echo "ok untrusted pull with corruption failed"
|
||||
fi
|
||||
|
||||
|
||||
cd ${test_tmpdir}
|
||||
tar xf ${test_srcdir}/ostree-path-traverse.tar.gz
|
||||
rm -rf repo2
|
||||
ostree_repo_init repo2 --mode=archive
|
||||
if ${CMD_PREFIX} ostree --repo=repo2 pull-local --untrusted ostree-path-traverse/repo pathtraverse-test 2>err.txt; then
|
||||
fatal "pull-local unexpectedly succeeded"
|
||||
fi
|
||||
assert_file_has_content_literal err.txt 'Invalid / in filename ../afile'
|
||||
echo "ok untrusted pull-local path traversal"
|
||||
Reference in new issue
Block a user