Add a best-effort sudo safety check
As motivation for this; we have had two breakouts of dib in recent memory. One was a failure to unmount through symlinks in the core code (I335316019ef948758392b03e91f9869102a472b9) and the other was removing host keys on the build-system (Ib01d71ff9415a0ae04d963f6e380aab9ac2260ce). For the most part, dib runs unprivileged. Bits of the core code are hopefully well tested (modulo bugs like the first one!). We give free reign inside the chroot (although there is still some potential there for adverse external affects via bind mounts). Where we could be a bit safer (and could have prevented at least the second of these breakouts) is with some better checking that the "sudo" calls *outside* the chroot at least looked sane. This adds a basic check that we're using chroot or image paths when calling sudo in those parts of elements that run *outside* the chroot. Various files are updated to accomodate this check; mostly by just ignoring it for existing code (I have not audited these calls). Nobody is pretending this type of checking makes dib magically safe, or removes the issues with it needing to do things as root during the build. But this can help find egregious errors like the key removal. Change-Id: I161a5aea1d29dcdc7236f70d372c53246ec73749
This commit is contained in:
parent
6c57795056
commit
672705831f
28
bin/dib-lint
28
bin/dib-lint
@ -54,7 +54,7 @@ excluded() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
error() {
|
error() {
|
||||||
echo "ERROR: $1"
|
echo -e "ERROR: $1"
|
||||||
rc=1
|
rc=1
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -146,6 +146,32 @@ for i in $(find elements -type f \
|
|||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|
||||||
|
# check that sudo calls in phases run outside the chroot look
|
||||||
|
# "safe"; meaning that they seem to operate within the chroot
|
||||||
|
# somehow. This is not fool-proof, but catches egregious errors,
|
||||||
|
# and makes you think about it if you're doing something outside
|
||||||
|
# the box.
|
||||||
|
if ! excluded safe_sudo; then
|
||||||
|
if [[ $(dirname $i) =~ (root.d|extra-data.d|block-device.d|finalise.d|cleanup.d) ]]; then
|
||||||
|
while read LINE
|
||||||
|
do
|
||||||
|
if [[ $LINE =~ "sudo " ]]; then
|
||||||
|
# messy regex ahead! Don't match:
|
||||||
|
# - explicitly ignored
|
||||||
|
# - basic comments
|
||||||
|
# - install-packages ... sudo ...
|
||||||
|
# - any of the paths passed into the out-of-chroot elements
|
||||||
|
if [[ $LINE =~ (dib-lint: safe_sudo|^#|install-packages|TARGET_ROOT|IMAGE_BLOCK_DEVICE|TMP_MOUNT_PATH|TMP_IMAGE_PATH) ]]; then
|
||||||
|
continue
|
||||||
|
fi
|
||||||
|
error "$i : potentially unsafe sudo\n -- $LINE"
|
||||||
|
fi
|
||||||
|
done < $i
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
done
|
done
|
||||||
|
|
||||||
for i in $(find elements -type f -and -name '*.rst' -or -type f -executable); do
|
for i in $(find elements -type f -and -name '*.rst' -or -type f -executable); do
|
||||||
|
@ -425,3 +425,30 @@ example if one were building tripleo-images, the variable would be set like:
|
|||||||
|
|
||||||
export ELEMENTS_PATH=tripleo-image-elements/elements
|
export ELEMENTS_PATH=tripleo-image-elements/elements
|
||||||
disk-image-create rhel7 cinder-api
|
disk-image-create rhel7 cinder-api
|
||||||
|
|
||||||
|
Linting
|
||||||
|
-------
|
||||||
|
|
||||||
|
You should always run ``bin/dib-lint`` over your elements. It will
|
||||||
|
warn you of common issues.
|
||||||
|
|
||||||
|
sudo
|
||||||
|
""""
|
||||||
|
|
||||||
|
Using ``sudo`` outside the chroot environment can cause breakout
|
||||||
|
issues where you accidentally modify parts of the host
|
||||||
|
system. ``dib-lint`` will warn if it sees ``sudo`` calls that do not
|
||||||
|
use the path arguments given to elements running outside the chroot.
|
||||||
|
|
||||||
|
To disable the error for a call you know is safe, add
|
||||||
|
|
||||||
|
::
|
||||||
|
|
||||||
|
# dib-lint: safe_sudo
|
||||||
|
|
||||||
|
to the end of the ``sudo`` command line. To disable the check for an
|
||||||
|
entire file, add
|
||||||
|
|
||||||
|
::
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
@ -21,5 +21,5 @@ DIB_APT_SOURCES=`readlink -f $DIB_APT_SOURCES`
|
|||||||
|
|
||||||
# copy the sources.list to cloudimg
|
# copy the sources.list to cloudimg
|
||||||
pushd $TMP_MOUNT_PATH/etc/apt/
|
pushd $TMP_MOUNT_PATH/etc/apt/
|
||||||
sudo cp -f $DIB_APT_SOURCES sources.list
|
sudo cp -f $DIB_APT_SOURCES sources.list # dib-lint: safe_sudo
|
||||||
popd
|
popd
|
||||||
|
@ -14,6 +14,8 @@
|
|||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -15,6 +15,8 @@
|
|||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ ${DIB_DEBUG_TRACE:-1} -gt 0 ]; then
|
if [ ${DIB_DEBUG_TRACE:-1} -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -31,9 +31,9 @@ if [ -e ${DIR} ]; then
|
|||||||
echo "${DIR} already exists!"
|
echo "${DIR} already exists!"
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
sudo mkdir -p ${DIR}
|
sudo mkdir -p ${DIR} # dib-lint: safe_sudo
|
||||||
|
|
||||||
# Copy to DIR
|
# Copy to DIR
|
||||||
for KEY in $(find ${DIB_ADD_APT_KEYS} -type f); do
|
for KEY in $(find ${DIB_ADD_APT_KEYS} -type f); do
|
||||||
sudo cp -L ${KEY} ${DIR}
|
sudo cp -L ${KEY} ${DIR} # dib-lint: safe_sudo
|
||||||
done
|
done
|
||||||
|
@ -1,5 +1,7 @@
|
|||||||
#!/bin/bash
|
#!/bin/bash
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ "${DIB_DEBUG_TRACE:-0}" -gt 0 ]; then
|
if [ "${DIB_DEBUG_TRACE:-0}" -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -14,6 +14,8 @@
|
|||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ ${DIB_DEBUG_TRACE:-1} -gt 0 ]; then
|
if [ ${DIB_DEBUG_TRACE:-1} -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -1,5 +1,7 @@
|
|||||||
#!/bin/bash
|
#!/bin/bash
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -10,6 +10,6 @@ MIRROR_SOURCE=$DIB_IMAGE_CACHE/pypi/mirror/
|
|||||||
if [ -d "$MIRROR_SOURCE" ]; then
|
if [ -d "$MIRROR_SOURCE" ]; then
|
||||||
MIRROR_TARGET=$TMP_MOUNT_PATH/tmp/pypi
|
MIRROR_TARGET=$TMP_MOUNT_PATH/tmp/pypi
|
||||||
|
|
||||||
sudo mkdir -p $MIRROR_SOURCE $MIRROR_TARGET
|
sudo mkdir -p $MIRROR_SOURCE $MIRROR_TARGET # dib-lint: safe_sudo
|
||||||
sudo mount --bind $MIRROR_SOURCE $MIRROR_TARGET
|
sudo mount --bind $MIRROR_SOURCE $MIRROR_TARGET # dib-lint: safe_sudo
|
||||||
fi
|
fi
|
||||||
|
@ -1,5 +1,7 @@
|
|||||||
#!/bin/bash
|
#!/bin/bash
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -1,5 +1,6 @@
|
|||||||
#!/bin/bash
|
#!/bin/bash
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
if [ ${DIB_DEBUG_TRACE:-0} -gt 1 ]; then
|
if [ ${DIB_DEBUG_TRACE:-0} -gt 1 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -1,6 +1,8 @@
|
|||||||
#!/bin/bash
|
#!/bin/bash
|
||||||
# These are useful, or at worst not harmful, for all images we build.
|
# These are useful, or at worst not harmful, for all images we build.
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -1,5 +1,7 @@
|
|||||||
#!/bin/bash
|
#!/bin/bash
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
@ -14,6 +14,9 @@
|
|||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
#
|
#
|
||||||
|
|
||||||
|
# dib-lint: disable=safe_sudo
|
||||||
|
|
||||||
if [ "${DIB_DEBUG_TRACE:-0}" -gt 0 ]; then
|
if [ "${DIB_DEBUG_TRACE:-0}" -gt 0 ]; then
|
||||||
set -x
|
set -x
|
||||||
fi
|
fi
|
||||||
|
Loading…
Reference in New Issue
Block a user