Re: [gentoo-portage-dev] [PATCH v3 1/2] bin/install-qa-check.d: add new 90bad-bin-owner QA check.

2018-08-07 Thread Zac Medico
On 08/07/2018 10:44 AM, Michael Orlitzky wrote:
> On 08/07/2018 01:34 PM, Zac Medico wrote:
>>
>> Why not use ${ED%/} instead of ${D%/} here, so that the output is the
>> same regardless of ${EPREFIX}?
>>
> 
> We want to show where the executable was actually installed, and
> generally that includes EPREFIX. For example, I'd want to see
> 
>   /var/tmp/whatever/.../root/prefix/usr/bin/foo
> 
> reported as
> 
>   /root/prefix/usr/bin/foo
> 
> rather than
> 
>   /usr/bin/foo
> 
> Of course, these checks are now skipped on prefix systems anyway, so
> it's a bit of a moot point. But I think it's more future-proof to strip
> only the $D.

Sounds good. Thanks! Merged:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=807ac3d9d6eecead73f59d399b30559e5c731587
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature


Re: [gentoo-portage-dev] [PATCH v3 1/2] bin/install-qa-check.d: add new 90bad-bin-owner QA check.

2018-08-07 Thread Michael Orlitzky
On 08/07/2018 01:34 PM, Zac Medico wrote:
> 
> Why not use ${ED%/} instead of ${D%/} here, so that the output is the
> same regardless of ${EPREFIX}?
> 

We want to show where the executable was actually installed, and
generally that includes EPREFIX. For example, I'd want to see

  /var/tmp/whatever/.../root/prefix/usr/bin/foo

reported as

  /root/prefix/usr/bin/foo

rather than

  /usr/bin/foo

Of course, these checks are now skipped on prefix systems anyway, so
it's a bit of a moot point. But I think it's more future-proof to strip
only the $D.



Re: [gentoo-portage-dev] [PATCH v3 1/2] bin/install-qa-check.d: add new 90bad-bin-owner QA check.

2018-08-07 Thread Zac Medico
On 08/07/2018 09:46 AM, Michael Orlitzky wrote:
> + if [[ ${found[@]} ]]; then
> + eqawarn "system executables owned by nonzero uid:"
> + for f in "${found[@]}"; do
> + # Strip off the leading destdir before outputting the 
> path.
> + eqawarn "  ${f#${D%/}}"

Why not use ${ED%/} instead of ${D%/} here, so that the output is the
same regardless of ${EPREFIX}?
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature


[gentoo-portage-dev] [PATCH v3 1/2] bin/install-qa-check.d: add new 90bad-bin-owner QA check.

2018-08-07 Thread Michael Orlitzky
System executables that are not owned by root pose a security
risk. The owner of the executable is free to modify it at any time;
so, for example, he can change a daemon's behavior to make it
malicious before the next time the service is started (usually by
root).

On a "normal" system, the superuser should own every system executable
(even setuid ones, for security reasons). This commit adds a new
install-time check that reports any such binaries with a QA
warning. To avoid false positives, non-"normal" systems (like prefix)
are skipped at the moment.

Bug: https://bugs.gentoo.org/629398
---
 bin/install-qa-check.d/90bad-bin-owner | 48 ++
 1 file changed, 48 insertions(+)
 create mode 100644 bin/install-qa-check.d/90bad-bin-owner

diff --git a/bin/install-qa-check.d/90bad-bin-owner 
b/bin/install-qa-check.d/90bad-bin-owner
new file mode 100644
index 0..c3ee30746
--- /dev/null
+++ b/bin/install-qa-check.d/90bad-bin-owner
@@ -0,0 +1,48 @@
+# Copyright 1999-2018 Gentoo Foundation
+# Distributed under the terms of the GNU General Public License v2
+
+bad_bin_owner_check() {
+   # Warn about globally-installed executables (in /bin, /usr/bin, /sbin,
+   # /usr/sbin, or /opt/bin) that are owned by a nonzero UID.
+
+   # This check doesn't work on non-root prefix installations at
+   # the moment, because every executable therein is owned by a
+   # nonzero UID.
+   [[ "${EUID}" -ne "0" || "${PORTAGE_INST_UID}" -ne "0" ]] && return
+
+   local d f found=()
+
+   for d in "${ED%/}/opt/bin" "${ED%/}/bin"  "${ED%/}/usr/bin" \
+  "${ED%/}/sbin" 
"${ED%/}/usr/sbin"; do
+   [[ -d "${d}" ]] || continue
+
+   # Read the results of the "find" command into the "found" bash 
array.
+   #
+   # Use -L to catch symlinks whose targets are owned by a 
non-root user,
+   # even though it won't catch ABSOLUTE symlinks until the package
+   # is RE-installed (the first time around, the target won't 
exist).
+   #
+   # We do want to list non-superuser setuid executables, because
+   # they can be exploited. The owner can simply wipe the setuid
+   # bit, and then alter the contents of the file. The superuser
+   # will then have a time bomb in his $PATH.
+   while read -r -d '' f; do
+   found+=( "${f}" )
+   done < <(find -L "${d}"   \
+   -maxdepth 1   \
+   -type f   \
+   ! -uid 0  \
+   -print0)
+   done
+
+   if [[ ${found[@]} ]]; then
+   eqawarn "system executables owned by nonzero uid:"
+   for f in "${found[@]}"; do
+   # Strip off the leading destdir before outputting the 
path.
+   eqawarn "  ${f#${D%/}}"
+   done
+   fi
+}
+
+bad_bin_owner_check
+:
-- 
2.16.4