Subject: bug#73166: shell-autorized-directories

From: Saku Laesvuori <[email-masked]>
Date: Mon, 11 Nov 2024 09:54:22 +0200

> > I do agree that it seems more convenient to run `guix shell --allow`
> > than copy a rather long line from the hint and run it to append a line
> > to shell-authorized-directories.
> >
> > Authorizing files instead of directories does not seem that great of an
> > idea to me. I doubt it really improves security that much. For example,
> > all my projects have a .guix/modules/xxx-package.scm file that contains
> > the package definition and guix.scm just loads it from that file.
> > Malicious code could be added here without touching the guix.scm file at
> > all, so the file-based authorization would not notice it.
> >
> > So this would only increase security when guix.scm does not refer to any
> > other files in the untrusted directory. Here it might get quite annoying
> > to re-authorize the directory every time every time someone changes the
> > version number.
> 
> Thanks for your feedback Saku.
> 
> Indeed, it only increases security for revisions of guix.scm and
> manifest.scm, not the repository as a whole.  But I think it's the exact
> same problematic for tools like direnv (same approach as here) or even
> emacs .dir-locals.el (which checks the last modified time of this file
> IIRC).  They can't vouch for the whole repository, but they can
> guarantee that the user explicitely accepted to run a guix.scm or
> manifest.scm (respectively a .envrc or .dir-locals.el) that depends on
> other files in the repo (that was not a guarantee previously, you could
> accept to run a manifest.scm before it depends on files in the repo).

Is it common to source other files from direnv or do people normally
just set environment variables and run programs from system PATH? If
sourcing other files is very rare with direnv and very common with guix
shell, comparing the security models is not as useful. I have never used
direnv, so I don't know. Maybe it is also often used to source
semitrusted files.

> I guess there are two use-cases :
> 1) scheme development with guix.scm loading local changes: Indeed this
> change is not really improving security, but neither is it harmful.

This case is a bit broader than just scheme but yes, the change does not
really have an impact here. The projects I refered to are mostly written
in Haskell. I load the package definitions from other files to
guix.scm/manifest.scm just to make the repositories work cleanly as Guix
channels.

> 2) custom manifest.scm in non-scheme projects (my use-case): Often in
> this case you would only change your manifest.scm, and it indeed
> increases security (the alternative would have been to automatically add
> the -m manifest.scm option but I'm not feeling secure with this
> alternative).
> More on my use-case: https://lists.sr.ht/~abcdw/rde-devel/patches/54944

Yes, but only slightly, I think. Because loading code from other files
is normal with guix manifests (see above), an attacker would first
refactor the repository into a guix channel to introduce loading from
another file in a non-suspicious way and only after that include the
malicious code.

> > Thus it seems that file-based authorization will only catch
> > false-positives. At least I would refactor my repository to a guix
> > channel and load the packaged from there with guix.scm to bypass this
> > security mechanism before adding any malicious code.
> >
> > Hashing the entire untrusted directory could work, but I'm not sure
> > would that have acceptable performance in larger cases.
> 
> Another option could be to add the expected output path of the guix
> shell invocation in the hash?  This could be simpler than hashing the
> whole directory.

That would only secure the shell environment, but the manifest could
still contain something like 

```scheme
(system* "rm -rf $HOME")
(specifications->manifest (list "hello"))
```

where the environment is safe but loading it causes bad side
effects.

> Although I'm not sure this is convenient for neither use-cases.
> Validation with guix shell --allow for every code change is not
> convenient.

That too.

Anyway, I am not opposed to this change. The only effects for my use
cases are positive (nicer UI with the --allow flag). I just want to
point out that I don't think this makes any attacks significantly
harder.

- Saku