Bug#789401: Proposed patch to solve the issue.

2024-04-27 Thread Georgios Zarkadas
Hi Thorsten,

Limiting access to the expanded chroot is something that can be done.

I currently use a `build' group and have {mode 750, ug root:build}  the
build directory,
were the base tgzs are unpacked as subdirectories, and {mode 2775, ug
root:build}
the result directory, so that pdebuild-internal can copy back resulting
debs.

`build' group members are the developers that can use pbuilder. This is a
separate group
than the one that has sudo rights to run pbuilder (though both have the
same members);
its sole role is to allow write access to the result directory and read
access to the build
directory.

I made this setup recently and maybe it needs some tuning (probably mode 775
for the result directory is enough), but so far packages build successfully
with it.

However, this solution requires the creation of a system group during
pbuilder's installation
and setting permissions to the associated directories. Also whoever uses a
custom directory
setup should afterwards set the above permissions accordingly.

/tmp/buildd is already preserved by the compatibility symlink code inside
pbuilder, in the sense
that it is there after the base tgz unpacking. Even if the compatibility
code is removed, one can
set BUILDDIR=/tmp/buildd in pbuilderrc and have it there, as long as
BUILDDIR is always created
during unpacking. That's why I have shipped the patch with the modified
comments at that point.

Even if a choice is made for pbuilder to support limiting access to the
expanded chroot,
I believe that chroot's temporary directories should be cleaned before
creating the base tgz.
Other things also go there (hookdir, pbuildersatisfydepends package, etc.)
and, since by design
these directories are for temporary stuff, persistence should not be
anticipated.

Cheers,
Georgios

Στις Σάβ 27 Απρ 2024 στις 5:10 μ.μ., ο/η Thorsten Glaser 
έγραψε:

> Hi Georgios,
>
> why not just ensure the parent directory of the chroot is not
> traversable for just any normal user?
>
> That would allow preserving /tmp/buildd as build place as well
> as retaining stuff under /run which packages create and which
> is, in practice, often needed for chroots where initscripts are
> not run.
>
> In addition, I often do use the access to the /tmp in the chroot
> for debugging and bootstrapping, so maybe create a new system
> group, chown 0:_pbuilder /var/cache/pbuilder/build; chmod 0750
> that directory, and good is? (Untested.)
>
> Then, I could add my user to that group and continue doing so.
>
> bye,
> //mirabilos
> --
> „Cool, /usr/share/doc/mksh/examples/uhr.gz ist ja ein Grund,
> mksh auf jedem System zu installieren.“
> -- XTaran auf der OpenRheinRuhr, ganz begeistert
> (EN: “[…]uhr.gz is a reason to install mksh on every system.”)
>


Bug#789401: Proposed patch to solve the issue.

2024-04-27 Thread Thorsten Glaser
Hi Georgios,

why not just ensure the parent directory of the chroot is not
traversable for just any normal user?

That would allow preserving /tmp/buildd as build place as well
as retaining stuff under /run which packages create and which
is, in practice, often needed for chroots where initscripts are
not run.

In addition, I often do use the access to the /tmp in the chroot
for debugging and bootstrapping, so maybe create a new system
group, chown 0:_pbuilder /var/cache/pbuilder/build; chmod 0750
that directory, and good is? (Untested.)

Then, I could add my user to that group and continue doing so.

bye,
//mirabilos
-- 
„Cool, /usr/share/doc/mksh/examples/uhr.gz ist ja ein Grund,
mksh auf jedem System zu installieren.“
-- XTaran auf der OpenRheinRuhr, ganz begeistert
(EN: “[…]uhr.gz is a reason to install mksh on every system.”)



Bug#789401: Proposed patch to solve the issue.

2024-04-27 Thread Georgios Zarkadas
The attached patch removes, during the recreation of base tgz,
all files from /tmp and /var/tmp (which is also world-writable).

It is made for the git version at salsa.debian.org but can also be applied
to the current (0.231) version as-is.

I have also modified a comment during the creation of BUILDDIR to alert for
the possibility of a user who keeps (still) in his/her configuration
/tmp/buildd
as the build directory.

It is not essential to the issue (only the tar command is), but I thought
it would be nice to have also. I can send a modified version of the patch,
if deemed necessary.

Cheers,
Georgios
diff --git a/pbuilder-modules b/pbuilder-modules
index aca876de..8d8a0c59 100644
--- a/pbuilder-modules
+++ b/pbuilder-modules
@@ -730,8 +730,9 @@ function extractbuildplace () {
 fi
 
 mountproc
-# FIXME maybe add more checks here? - actually it's not even really needed,
-# since it's created at chroot creation time too.
+# FIXME maybe add more checks here? - Always create it, since it may be set
+# in the configuration to be inside one of the excluded (at 'create_basetgz')
+# directories of the chroot (for example: '/tmp/buildd').
 mkdir -p "${BUILDPLACE}${BUILDDIR}"
 # XXX added in 0.216, to be deprecated in the future
 # Add a compatibility symlink from the old BUILDDIR (/tmp/buildd) to the new
@@ -834,7 +835,7 @@ function create_basetgz() {
 if [ -h "$BUILDPLACE/tmp/buildd" ] && [ "$(readlink -f "$BUILDPLACE/tmp/buildd")" = "${BUILDPLACE}$BUILDDIR" ]; then
 rm "$BUILDPLACE/tmp/buildd"
 fi
-if ! tar -c --use-compress-program "$COMPRESSPROG" -f "${BASETGZ}.tmp" --exclude ./sys/* --exclude ./proc/* ./* ; then
+if ! tar -c --use-compress-program "$COMPRESSPROG" -f "${BASETGZ}.tmp" --exclude "./sys/*" --exclude "./proc/*" --exclude "./tmp/*" --exclude "./tmp/.*" --exclude "./var/tmp/*" --exclude "./var/tmp/.*" ./* ; then
 log.e "failed building base tarball"
 rm -f "${BASETGZ}.tmp"
 exit 1;