FS#76255 - Handling of git submodules in PKGBUILDs broken as of git 2.38.1 update

Attached to Project: Arch Linux
Opened by Paul Mulders (justinkb) - Thursday, 20 October 2022, 11:46 GMT
Last edited by Toolybird (Toolybird) - Tuesday, 08 November 2022, 21:37 GMT
Task Type Bug Report
Category Documentation
Status Closed
Assigned To No-one
Architecture All
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 2
Private No

Details

Description:

Because of security-related changes in git 2.38.1 (see https://lore.kernel.org/lkml/xmqq4jw1uku5.fsf@gitster.g/), the way makepkg interacts with git repositories with submodules (cloning method) is no longer functional.

Additional info:
Using pacman 6.0.1-8 and git 2.38.1-1

Steps to reproduce:

$ asp checkout libtool
$ makepkg -s

leads to:

```
==> Starting prepare()...
patching file m4/libtool.m4
Hunk #1 succeeded at 730 (offset 2 lines).
patching file tests/link-order2.at
Hunk #1 succeeded at 48 (offset 1 line).
Hunk #2 succeeded at 92 (offset 1 line).
Hunk #3 succeeded at 121 (offset 1 line).
Submodule 'bootstrap' (https://github.com/gnulib-modules/bootstrap.git) registered for path 'gl-mod/bootstrap'
Submodule 'gnulib' (git://git.savannah.gnu.org/gnulib.git) registered for path 'gnulib'
Cloning into '/home/paul/Developer/abs/libtool/trunk/src/libtool/gl-mod/bootstrap'...
Cloning into '/home/paul/Developer/abs/libtool/trunk/src/libtool/gnulib'...
fatal: transport 'file' not allowed
fatal: clone of '/home/paul/Developer/abs/libtool/trunk/src/gnulib' into submodule path '/home/paul/Developer/abs/libtool/trunk/src/libtool/gnulib' failed
Failed to clone 'gnulib'. Retry scheduled
Cloning into '/home/paul/Developer/abs/libtool/trunk/src/libtool/gnulib'...
fatal: transport 'file' not allowed
fatal: clone of '/home/paul/Developer/abs/libtool/trunk/src/gnulib' into submodule path '/home/paul/Developer/abs/libtool/trunk/src/libtool/gnulib' failed
Failed to clone 'gnulib' a second time, aborting
==> ERROR: A failure occurred in prepare().
Aborting...
```
This task depends upon

Closed by  Toolybird (Toolybird)
Tuesday, 08 November 2022, 21:37 GMT
Reason for closing:  Fixed
Additional comments about closing:  wiki has been updated
Comment by Morten Linderud (Foxboron) - Thursday, 20 October 2022, 11:50 GMT
This isn't really a makepkg bug. All PKGBUILDs using git-submodules do that induvidually without any help from makepkg. What you *really* want is documentation updates how Arch should be packaging stuff with git-submodules.
Comment by Paul Mulders (justinkb) - Thursday, 20 October 2022, 12:18 GMT
Isn't it the case that the issue only occurs because of some cloning method makepkg uses for the purpose of efficiency?
Comment by Morten Linderud (Foxboron) - Thursday, 20 October 2022, 12:31 GMT
What cloning method does makepkg use for efficiency? It just issues `git clone` on the source if it's a git repository.
Comment by Paul Mulders (justinkb) - Thursday, 20 October 2022, 12:39 GMT
Had a quick look at git.sh.in in libmakepkg sources. I think the issue is with the use of the -s (--shared) flag for the cloning operation in extract_git. Git man page lists its usage as potentially dangerous. I'm guessing because behind the scenes it uses the local cloning mechanism that makes the change of that git config default problematic. I'm not a git expert tho
Comment by Paul Mulders (justinkb) - Thursday, 20 October 2022, 16:48 GMT
I've done some tests, and it seems like the -s flag is unrelated. In any case, if the submodule idiom for PKGBUILDs simply has to change, how to do that? Simply passing a config flag to set the git config option to its previous default? Does that not reintroduce the security issue from the CVE? I get that maintainers of package have a responsibility of making sure their sources are legit, but I'm wondering about any issues in case a repository gets compromised after the fact.
Comment by loqs (loqs) - Thursday, 20 October 2022, 17:48 GMT
@justinkb the attack blocked by the protocol.file.allow is described in [1]. If the PKGBUILD has set all the submodules to local references in $srcdir the attack is not possible.
If the submodules are as set by upstream then you are relying on the commit being pinned in the PKGBUILD and the maintainer monitoring the submodules. The PKGBUILD would also need to allow the data which has been copied from outside $srcdir and is now available inside it to be exploited.
The PKGBUILD change suggested benjarobin [2] seems the best fix available to date.
Edit:
Also avoiding the local clone would make offline building impossible, requiring network connectivity at least in prepare().

[1] https://github.com/git/git/commit/a1d4f67c12ac172f835e6d5e4e0a197075e2146b
[2] https://bbs.archlinux.org/viewtopic.php?pid=2063104#p2063104
Comment by Paul Mulders (justinkb) - Thursday, 20 October 2022, 18:42 GMT
Guess I don't really understand the deal with that vulnerability anyway. Is the exfiltration accomplished via pushing to git repo by someone who hasn't realized the submodules were defined maliciously? If that is the issue, the whole thing isn't relevant to pacman anyway (other than the config change necessitating updating pkgbuilds), is it? Pacman only pulls from git repos, never pushes, does it?
Comment by Toolybird (Toolybird) - Saturday, 22 October 2022, 22:23 GMT
So this is definitely not a pacman/makepkg bug as noted by @Foxboron. The relevant wiki section [1] will need to be edited once an accepted solution has been agreed upon.

Are we happy with `git -c protocol.file.allow=always submodule update ...' as mentioned in the BBS thread? I'm guessing there'll be quite a few pkgs that are now FTBFS so maybe we need a TODO action list?

[1] https://wiki.archlinux.org/title/VCS_package_guidelines#Git_submodules

Loading...