Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--devel should not skip prepare and dependencies #245

Closed
Morganamilo opened this issue Aug 13, 2018 · 22 comments
Closed

--devel should not skip prepare and dependencies #245

Morganamilo opened this issue Aug 13, 2018 · 22 comments
Labels
enhancement New feature or request

Comments

@Morganamilo
Copy link
Contributor

Description

The --devel feature checks for updates by running pkgver and comparing the new version to the currently installed version. This is done via makepkg -odc --noprepare --skipinteg. Then parsing makepkg --printsrcinfo

The problem here is that pkgver expects that prepare has been run and all deps have been installed. You can't exactly run git describe without the git dependency installed.

The dependency issue is not that big of a deal as --devel works with already installed packages, although a new pkgbuild may gain new dependencies.

I also have problems with using --skipinteg. Checksums ensure the correct flles were downloaded and PGP keys are a security measure. When I read a pkgbuild and see these keys, it gives me trust that the sources it downloads truly come from the upstream developers. With these disabled it is possible, that if upstream were compromised, using --devel would lead to untrusted code being ran.

It is true that this attack vector is small and practically impossible to pull off (most devel packages use SKIP even). But I would prefer it if aurman did not pass --skipinteg unless I explicitly say so.

Expected Behavior

I think I explained above but I just noticed "behavior" is typo'd here ;)

Possible Solution

Use makepkg -oc

Although then you would have to handle any new dependencies that a pkgbuild gains. And I can see how doing this so early could be quite annoying.

Although if a pkgbuild changes then so should the pkgver. So perhaps you could exclude any packages that have a higher pkgver from the --devel check. This would actually speed things up too, since you're checking less packages. These skipped packages would then get pulled in by the normal update checker.

Version of aurman you are using

2.17.2.r0.gd857951-1

Steps to Reproduce

For an example of the --noprepare issue there is chromium-widevine. This is not a devel package but it is one of the few out there that will fail the pkgver is prepare has not been run. I added it to [devel_packages] as I can't think of any actual devel packages right now.

morganamilo@Octavia ~ % aurman -Su --do_everything --devel
~~ initializing aurman...
~~ the following packages are neither in known repos nor in the aur
:: saur-git-r81.8759745-1
~~ looking for new pkgbuilds of devel packages and fetching them...
>>>> makepkg -odc --noprepare --skipinteg
==> Making package: aurman-git 2.17.2.r0.gd857951-1 (Mon 13 Aug 2018 01:37:35 BST)
==> WARNING: Skipping dependency checks.
==> Retrieving sources...
  -> Updating aurman_sources git repo...
Fetching origin
==> WARNING: Skipping all source file integrity checks.
==> Extracting sources...
  -> Creating working copy of aurman_sources git repo...
Cloning into 'aurman_sources'...
done.
Switched to a new branch 'makepkg'
==> Starting pkgver()...
==> Sources are ready.
==> Cleaning up...
>>>> makepkg --printsrcinfo
>>>> makepkg -odc --noprepare --skipinteg
==> Making package: aurutils-git 1.5.3.r761.g8fe42a3-1 (Mon 13 Aug 2018 01:37:37 BST)
==> WARNING: Skipping dependency checks.
==> Retrieving sources...
  -> Updating aurutils git repo...
Fetching origin
==> WARNING: Skipping all source file integrity checks.
==> Extracting sources...
  -> Creating working copy of aurutils git repo...
Cloning into 'aurutils'...
done.
==> Starting pkgver()...
==> Sources are ready.
==> Cleaning up...
>>>> makepkg --printsrcinfo
>>>> makepkg -odc --noprepare --skipinteg
==> Making package: chromium-widevine 1:4.10.1146.0-3 (Mon 13 Aug 2018 01:37:38 BST)
==> WARNING: Skipping dependency checks.
==> Retrieving sources...
  -> Found chrome-eula_text-20180809.html
  -> Found google-chrome-stable_68.0.3440.106-1_amd64.deb
==> WARNING: Skipping all source file integrity checks.
==> Extracting sources...
  -> Extracting google-chrome-stable_68.0.3440.106-1_amd64.deb with bsdtar
==> Starting pkgver()...
awk: fatal: cannot open file `libwidevinecdm.so' for reading (No such file or directory)
==> ERROR: pkgver is not allowed to be empty.
==> ERROR: pkgver() generated an invalid version: 
2018-08-13 01:37:39,126 - wrappers - makepkg - ERROR - makepkg query ['makepkg', '-odc', '--noprepare', '--skipinteg'] failed in directory /home/morganamilo/.cache/aurman/chromium-widevine
2018-08-13 01:37:39,126 - main - main - ERROR - 
Traceback (most recent call last):
  File "/usr/lib/python3.7/site-packages/aurman/main.py", line 1245, in main
    process(argv[1:])
  File "/usr/lib/python3.7/site-packages/aurman/main.py", line 965, in process
    package.get_devel_version(ignore_arch)
  File "/usr/lib/python3.7/site-packages/aurman/classes.py", line 1179, in get_devel_version
    makepkg(["-odc", "--noprepare", "--skipinteg"], False, package_dir)
  File "/usr/lib/python3.7/site-packages/aurman/wrappers.py", line 137, in makepkg
    raise InvalidInput("makepkg query {} failed in directory {}".format(makepkg_query, dir_to_execute))
aurman.own_exceptions.InvalidInput: makepkg query ['makepkg', '-odc', '--noprepare', '--skipinteg'] failed in directory /home/morganamilo/.cache/aurman/chromium-widevine

Read the README

Yep

Running linux distribution

Arch

@eli-schwartz
Copy link
Collaborator

I also have problems with using --skipinteg. Checksums ensure the correct flles were downloaded and PGP keys are a security measure. When I read a pkgbuild and see these keys, it gives me trust that the sources it downloads truly come from the upstream developers. With these disabled it is possible, that if upstream were compromised, using --devel would lead to untrusted code being ran.

That shouldn't be a problem though, since --skipinteg is only used to generate a new devel version, but the package should still be built afresh later, and do full integrity checks. This is only used for the internal, hidden preprocessing which does not need to check integrity.

I'd say --noprepare should still be avoided though as it is possible -- and permitted -- for the prepare() function to actually change the pkgver value. Not sure whether that would necessitate running untrusted code in there though?? Also unsure in which cases failure to run prepare() would result in a pkgver that does not trigger a fresh build, but should have.

@Morganamilo
Copy link
Contributor Author

True that they will be verified during during the build proper. But prepare is just a bash function so in theory anything could be in there.

For example if the upstream code was compromised and ./configure was called, who knows what code will run. Sure ./configure should be in build, not prepare. Point being, prepare is arbitrary bash that may invoke code from $srcdir.

Better safe than sorry I think. The only downside to not having --skipinteg is a little time I believe.

@polygamma
Copy link
Owner

polygamma commented Aug 13, 2018

I guess makepkg -odc is the way to go. You are right about --skipinteg and --noprepare. Anyway the dependency thing is nothing which should be done automatically. Most of the dependencies of packages are not needed for prepare() and pkgver(), aurman could never detect which of them are actually needed, thus installing all dependencies to be safe is too bloated. Installing none of them and using -d is fine, if it fails, the user may manually install the missing deps and try again.

@polygamma polygamma added the enhancement New feature or request label Aug 13, 2018
@polygamma
Copy link
Owner

polygamma commented Aug 13, 2018

see: 2.17.2...5904421

@polygamma
Copy link
Owner

Also added a new error message for this case.

screenshot from 2018-08-13 15-30-16

@polygamma
Copy link
Owner

polygamma commented Aug 13, 2018

Although if a pkgbuild changes then so should the pkgver. So perhaps you could exclude any packages that have a higher pkgver from the --devel check. This would actually speed things up too, since you're checking less packages. These skipped packages would then get pulled in by the normal update checker.

Excluding those packages is not a good idea, it is possible that one still needs to know the exact current version, not only, that there is a newer version. Think about the following:
Package A is a devel package installed in version 1.0. AUR lists currently 2.0, but if executing pkgver() one gets 2.1. Package B depends on A>=2.1, means that the dependency solver of aurman cannot find a solution to install package B because it would assume version 2.0 for Package A.

@Morganamilo
Copy link
Contributor Author

Ah didn't realise aurman did that. I thought it was rpc only.

Anyway 2/3 points fixed ain't too bad, thanks.

@polygamma
Copy link
Owner

polygamma commented Aug 13, 2018

That's the only exception, because it really is needed to implement fetching of current versions for development packages, in order to provide a non-useless AUR helper.

@polygamma
Copy link
Owner

polygamma commented Aug 13, 2018

Can anyone think of a sensible solution for the not yet fulfilled point out of those three? The case, that one executes e.g. aurman -S --devel package_one whereas package_one needs a specific dependency installed in order to be able to run makepkg -odc without errors. As already mentioned: It surely is possible to say - "Well, install all dependencies" but that obviously is overkill in 95% of the cases. Besides being overkill: I do not want to install all dependencies of a package, before even being able to decide, if I really want to install that package itself.

@Morganamilo
Copy link
Contributor Author

morganamilo@Vinyl ~ % aurman -S --devel smplayer-svn
~~ initializing aurman...
~~ the following packages are neither in known repos nor in the aur
:: linux-ck-zzorra-4.15.12-1
:: diodon-bzr-r535-1
:: pcmciautils-018-8
:: linux-ck-zzorra-headers-4.15.12-1
:: gegl02-0.2.0-8
:: js-24.2.0-4
:: js38-38.8.0-5
:: TEST-1-1
:: aurweb-test-4.6.0-1
:: libcloudproviders-0.2.5+9+g93dc5ea-1
:: vte-0.28.2-8
:: Ignoring installed package linux-ck
:: Ignoring installed package linux-ck-headers
:: Ignoring installed package vim-youcompleteme-git
~~ looking for new pkgbuilds of devel packages and fetching them...
==> Making package: smplayer-svn .r-1 (Mon 13 Aug 2018 17:10:36 BST)
==> WARNING: Skipping dependency checks.
==> Retrieving sources...
  -> Updating smplayer-svn svn repo...
/usr/share/makepkg/source/svn.sh: line 74: svn: command not found
==> WARNING: Failure while updating smplayer-svn svn repo
==> Validating source files with sha256sums...
    smplayer-svn ... Skipped
==> Extracting sources...
  -> Creating working copy of  svn repo...
==> Starting pkgver()...
grep: smplayer.spec: No such file or directory
/home/morganamilo/.cache/aurman/smplayer-svn/PKGBUILD: line 26: svnversion: command not found
==> Sources are ready.
==> Cleaning up...

Here I try to instil an -svn package without snv installed. Pkgver fails but it continues on. This ends up generating a different pkgver to what it should.

@polygamma
Copy link
Owner

Interesting... But what could I do about this, if makepkg returns with 0?

@Morganamilo
Copy link
Contributor Author

Don't pass -d. Let makepkg bail out due to missing deps and have your new warning tell the user to install the deps themselves?

@polygamma
Copy link
Owner

In that case it comes down to installing all dependencies again :( Otherwise makepkg is going to complain. And if one is at that point, it may be aurman which installs all dependencies first, and then we are back at the beginning

@Morganamilo
Copy link
Contributor Author

Eh well. You could just wait for somebody to actually have this problem. I just wanted to show it's possible, but who knows how rare it is for this to happen.

@polygamma
Copy link
Owner

polygamma commented Aug 13, 2018

I have an idea, on how to implement this in a way I'd be fine with. You should also be fine with it, because it fulfills the point you criticized. Will let you know here, when I have done it

polygamma added a commit that referenced this issue Aug 13, 2018
@polygamma
Copy link
Owner

see: 2f2e93c

screenshot from 2018-08-13 19-26-56

@polygamma
Copy link
Owner

polygamma commented Aug 13, 2018

The all in all changes are as of now: 2.17.2...1a1b26d

What do you think, @Morganamilo ?

@AladW
Copy link

AladW commented Aug 13, 2018

If I were to handle this (which I wouldn't, since imo this problematic is the user's problem), I'd simply install VCS clients with --asdeps and potentially remove them again after. You can get the list from VCSAGENTS in libmakepkg, iirc.

@Morganamilo
Copy link
Contributor Author

I think it looks good, not that I've had a change to actually run it though.

@AladW VCS clients sre the most common case, but a pkgbuild does expect all its depends installed before prepare is ran. It is not just due to missing VCS clients that pkgver/prepare may fail.

@AladW
Copy link

AladW commented Aug 13, 2018

I guess the real issue is that makepkg considers errors in source/pkgver as non-fatal.

[vagrant@archlinux smplayer-svn]$ makepkg -od
==> Making package: smplayer-svn 18.6.0.r-1 (Mon 13 Aug 2018 09:11:08 PM UTC)
==> WARNING: Skipping dependency checks.
==> Retrieving sources...
  -> Updating smplayer-svn svn repo...
/usr/share/makepkg/source/svn.sh: line 74: svn: command not found
==> WARNING: Failure while updating smplayer-svn svn repo
==> Validating source files with sha256sums...
    smplayer-svn ... Skipped
==> Extracting sources...
  -> Creating working copy of  svn repo...
==> Starting pkgver()...
/home/vagrant/.cache/aurutils/sync/smplayer-svn/PKGBUILD: line 26: svnversion: command not found
==> Sources are ready.

@polygamma
Copy link
Owner

polygamma commented Aug 13, 2018

I guess the real issue is that makepkg considers errors in source/pkgver as non-fatal.

Yeah, that really is weird. Maybe @eli-schwartz knows more about this?
Anyway: As of now aurman does the following when using --devel:

If the user does not specify --devel_skip_deps, too, for all detected development packages is being checked, if all dependencies are fulfilled. If there are unfulfilled dependencies, the warning which can be seen in #245 (comment) is being shown and execution of aurman is aborted. If all dependencies are fulfilled, great, after fetching and showing of PKGBUILDs, makepkg -oc or makepkg -ocA is being executed, then the current version being fetched from makepkg --printsrcinfo. If the user specifies --devel_skip_deps, dependencies are not checked and makepkg -odc or makepkg -odcA is being executed, failing makepkg yields a normal stacktrace. If makepkg -odc does not fail but yields a wrong version like in our smplayer-svn example, well, users problem because he specified --devel_skip_deps. I guess this is all in all sensible, default of aurman is sane, warns about unfulfilled dependencies, but the user may ignore the warnings if he wants to.

@eli-schwartz
Copy link
Collaborator

Yeah, that really is weird. Maybe @eli-schwartz knows more about this?

Fixed: https://lists.archlinux.org/pipermail/pacman-dev/2018-August/022765.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants