CRUX

Welcome to CRUX bug tracking.
Tasklist

FS#1851 - pkgmk: Pkgfile gets sourced before its signature is verified

Attached to Project: CRUX
Opened by Oleg Strikov (xj8z) - Saturday, 20 February 2021, 18:37 GMT
Last edited by Oleg Strikov (xj8z) - Sunday, 21 February 2021, 10:59 GMT
Task Type Bug Report
Category tools → pkgutils
Status New
Assigned To Thomas Penteker (teK)
Operating System CRUX
Severity High
Priority Normal
Reported Version 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

Security model of the ports system relies on the fact that only properly signed ports can be built by pkgmk. However, pkgmk sources not yet verified Pkgfile at the very beginning of its operation (see line 813 for details). This allows malicious Pkgfile to alter the behavior of pkgmk making all subsequent security measures unreliable. For example, malicious Pkgfile can define signify() { return 0; } which will be called by pkgmk instead of /usr/bin/signify (local symbols have precedence over calls to external programs) and allow modified files to pass signature/checksum verification by returning 0 (check passed) all the time. This is especially worrisome because rsync-based delivery of the ports tree is not protected against mitm attacks and any successful attack of this sort can lead to full control over the system by modifying the source code of one or more ports. Please note that using fakeroot to build ports doesn't protect against this attack because modified source code will be built, packaged and installed to the system as usual. Please find below a patch to /usr/ports/core/which/Pkgfile which makes changes to which.c and therefore /usr/bin/which but doesn't require .signature to be updated.
This task depends upon

Comment by Juergen Daubert (jue) - Monday, 22 February 2021, 14:36 GMT
a simple and obvious fix is to replace the call to signify with the full path /usr/bin/signify, dunno if this is enough or other measures are needed.
A general problem is that a Pkgfile can currently overwrite all functions of pkgmk, maybe we should not allow that.
Comment by Oleg Strikov (xj8z) - Tuesday, 23 February 2021, 11:39 GMT
We can try to fix all potential consequences of untrusted file being injected into the execution enviroment but I don't think that we can win this game. It looks like a very time-consuming project to learn all the dark corners of bash programming. My proposal is to follow one of the basic principles of defensive programming which states that program shouldn't do any meaningful processing of a file until its origin has been determined (this can be done by checking signature of a file against well-known public key). Basically we just need to check Pkgbuild's signature before sourcing it not after. This can be achieved by either (a) adding one more call to signify before sourcing Pkgbuild to check signature/checksum of this exact file or by (b) doing partial redesign of pkgmk to do signature/checksum verification of all static resources (Pkgfile, .footprint, patches and so on) early on, before any processing takes place.
Comment by Fun (fun) - Tuesday, 23 February 2021, 22:18 GMT
Btw, if you sync ports with the rsync driver, the signature too can be changed by a man-in-the-middle. So, it won't matter that you run signify before sourcing the port.

Moving all remote URLs in a different file will allow pkgmk to run signify before sourcing the port. See how K1ss linux keeps its sources: https://github.com/kisslinux/repo/blob/master/extra/sqlite/sources

Another method will be to extract the URLs with grep/perl/ruby/python, but you'll have to enforce some rules, because the maintainers are very creative when it comes to save some keystrokes on version bumps :). You can check this perl script to see what BASH variable substitutions may be needed to extract URLs: https://gitlab.com/therealfun/crux-ports/-/blob/master/upkg/upkg-dl-list

When you source the port, you can avoid overriding functions and variables by using a sub-shell: "( source Pkgfile; printf '%s\n' "{$sources[@]}" )", but that port can still "read your mail" from within that sub-shell :) You can see some almost-useless-tricks with the sub-shell construct here: https://gitlab.com/therealfun/crux-ports/-/blob/master/upkg/upkg-mk#L283

But, there are ports that depend on the possibility to override functions from pkgmk (search for "unpack_source" for example), so you can't block the function override "support" right now.

Anyway, reading the CRUX mail archives a couple years ago, I remember that pkgmk changed a lot from Per Liden. The first "wave" was the shared-directory thing, for sources and packages. All changes were "patches". Nobody liked the pkgmk source :)

There are a couple of issues that has to be changed first:
- move the remote URLs in another file, listed line by line, without depending on variable expansions
- find a clean solution for URLs having the same "filename" from the same port (java, from what I remember)
- find a clean solution for the unpack_source() need
After these, pkgmk can be rewritten even in C/C++, even split. The current version does too much. You can see how simple that source can be, even with all that it does, if we drop the shared-directory thing and don't support fakeroot builds (chmod in Pkgfile). Less than 100 lines: https://gitlab.com/therealfun/oprt/-/blob/master/src/upkgmk

But, I doubt that this will happen. The maintainers trust each other. They rarely use ports from external repositories. This is not a big issue. And I don't blame them :)
Comment by Oleg Strikov (xj8z) - Wednesday, 24 February 2021, 10:23 GMT
> Btw, if you sync ports with the rsync driver, the signature too can be changed by a man-in-the-middle. So, it won't matter that you run signify before sourcing the port.

Sorry, but I don't get your point. File .signature can indeed be changed by mitm but it needs to be signed by trusted key from /etc/ports/*.pub to pass signify. It's not just a list of SHA256 hashes, .signature also contains... well, signature :)

> https://gitlab.com/therealfun/{oprt,crux-ports}

I *really* like how you managed to make your own infrastructure over existing ports tree. It's very impressive. I'll definitely take a look. Many thanks for sharing.

> The first "wave" was the shared-directory thing, for sources and packages.

What do you mean by "shared-directory thing"? It's probably related to PKGMK_{SOURCE,PACKAGE}_DIR but I don't fully understand the matter.

> The maintainers trust each other. They rarely use ports from external repositories. This is not a big issue. And I don't blame them :)

My point is that we should either (a) completely remove signature verification or (b) make it reliable. Security theater is a bad thing.
Comment by Fun (fun) - Friday, 26 February 2021, 10:14 GMT
You're right about mitm. My mistake.

initially, pkgmk was made to build packages as root, from what I've read. The sources were downloaded near the Pkgfile file. It was simple. But, even now, the non-root setup is an exercise for all users :)

upkg (not oprt) is the last of my experiements to replace pkgmk/prt-get/ports in bash. I always finished with a much more complicated thing :). I'm still using it because: it runs signify before sourcing a Pkgfile, shows a diff against the previous version I've built before rebuilding a port, shows/asks/remembers post-install scripts, picks the port from the "right" repository.

I'm not sure signify was added to pkgmk for the mitm case on ports, but to catch if one source from 1000 locations we use to download sources from didn't changed after the maintainer signed the port.

For the mitm case on ports, we could use a signed manifest (.httpup.sig).

Loading...