search

grml / grml-debootstrap

wrapper around debootstrap
59 stars 27 forks source link

Fix compatibility with libpam-tmpdir. #234

Closed adrelanos closed 9 months ago

adrelanos commented 10 months ago

By creating folder the required temporary folder.

fixes https://github.com/grml/grml-debootstrap/issues/232

zeha commented 10 months ago

I understand where you're coming from, but that seems to be the wrong fix. The host setup should (mostly) not affect the target. Maybe we should reset TMPDIR when acting in the chroot instead.

adrelanos commented 10 months ago

I understand where you're coming from, but that seems to be the wrong fix.

Why? Some tool at some stage needs to create the directory. I was also wondering which tool would be the most appropriate. Maybe libpam-tmpdir should do it (or provider a helper tool to do that) but then I thought it won't be running at that time.

Maybe we should reset TMPDIR when acting in the chroot instead.

Maybe. Then all invocations of chroot could be replaced with $chroot (or different variable name) which would prepend:

env --unset=TEMP --unset=TEMPDIR --unset=TMP --unset=TMPDIR

What however should not be done is on the host operating system (at the beginning of grml-debootstrap):

unset TEMP TEMPDIR TMP TMPDIR

Because that would disable libpam-tmpdir during all invocations of any tools running on the host operating system and not only inside the chroot.

On the other hand, wouldn't it be more secure to start using libpam-tmpdir as soon as possible even inside the chroot?

zeha commented 10 months ago

I understand where you're coming from, but that seems to be the wrong fix.

Why? Some tool at some stage needs to create the directory. I was also wondering which tool would be the most appropriate. Maybe libpam-tmpdir should do it (or provider a helper tool to do that) but then I thought it won't be running at that time.

libpam-tmpdir on the host is a host thing. Its existence on the host shall not affect the chroot, which can end up on a completely different system / VM / ...

I know there are some other things that today leak from the host, but I'd like to get rid of them too - at least for --vm mode.

Maybe we should reset TMPDIR when acting in the chroot instead.

Maybe. Then all invocations of chroot could be replaced with $chroot (or different variable name) which would prepend:

env --unset=TEMP --unset=TEMPDIR --unset=TMP --unset=TMPDIR

I was thinking more along the lines of env -i chroot ..., as a holistic approach to stop all ENV leakage.

On the other hand, wouldn't it be more secure to start using libpam-tmpdir as soon as possible even inside the chroot?

No, there is no concurrent access while the chroot is built. libpam-tmpdir does not add any value during this time.

Only after booting it might be useful, but this decision should be left to the user.

adrelanos commented 10 months ago

Ok, very good. Lets define $chroot being env -i chroot and use that?

I would attempt a PR but wait for it until https://github.com/grml/grml-debootstrap/pull/231 is merged (otherwise merge conflict).

mika commented 9 months ago

Ok, very good. Lets define $chroot being env -i chroot and use that? I would attempt a PR but wait for it until https://github.com/grml/grml-debootstrap/pull/231 is merged (otherwise merge conflict).

Sounds like worth a try, yes :)

zeha commented 9 months ago

Instead of $chroot, we could have sth like clean_chroot() { env -i chroot "$@" } (?)

adrelanos commented 9 months ago

Works for me either way. $chroot has the advantage that it could be user customized through an environment variable.

Instead of $chroot, we could have sth like clean_chroot() { env -i chroot "$@" } (?)

Since @mika liked the post, seems to agree with it (and I personally don't need the environment variable), I am happy to and will attempt to implement this at a later time when other PR is ready and merged to avoid merge conflicts.