Description of problem: When adding a package to a Copr repo the default selected build method is 'rpkg'. In contrast to rpmbuild, rpkg automatically downloads missing sources. This is an insecure default, since sources are often only available over an insecure connection (i.e. HTTP instead of HTTPS). (And .spec files usually don't contain any cryptographic checksum checking logic.) Thus, this default enables man-in-the-middle attacks. Version-Release number of selected component (if applicable): How reproducible: always Steps to Reproduce: 1. create a new copr repository with default settings (specifiy a https git-pull url where the .spec file is located) 2. create a new package with default settings 3. Create a new build with default settings Actual results: any HTTP source URLs in the securely obtained .spec file are automatically downloaded without any verification, by default. Expected results: no auto-downloading, unless it's a HTTPS URL (and the server has a valid certificate) Additional info: With rpmbuild, the auto-downloading is disabled, by default. And to enable it, one has to explicitly define `_disable_source_fetch` to 0. This is a good default, because RPM source packages don't have a standard source file verification mechanism built in.
> With rpmbuild, the auto-downloading is disabled, by default. And to enable it, one has to explicitly define `_disable_source_fetch` to 0. This is a good default, because RPM source packages don't have a standard source file verification mechanism built in. That's what we do. We set `_disable_source_fetch` to 0. And yes, the URL should be https to avoid any kind of attacks. But the question is if it is not a responsibility of a packager to put https there and avoid http altogether. rpm obviously doesn't check it atm. We could add the check into rpkg but I think it would be better to check it on rpm level if anywhere.
Counterexample (where the _disable_source_fetch setting doesn't help): The build log of a package in my repository: https://copr-be.cloud.fedoraproject.org/results/gsauthof/epel/srpm-builds/00703678/builder-live.log Copr happily auto-downloads over HTTP: Downloading http://releases.nixos.org/patchelf/patchelf-0.9//patchelf-0.9.tar.bz2 to /tmp/tmpipnu79bm/copr-epel/patchelf/patchelf-0.9.tar.bz2 The package has a HTTPS git URL configured in COPR. The .spec file includes a HTTP source location: https://github.com/gsauthof/copr-epel/blob/master/patchelf/patchelf.spec#L9 Source0: http://releases.nixos.org/patchelf/patchelf-%{version}//%{name}-%{version}.tar.bz2 Note (as mentioned in my initial report) that I created the patchelf package in my Copr repository with the default build method, i.e. with 'rpkg'. I think the objective is to have secure defaults. If 'rpkg' is the default then 'rpkg' should not perform insecure downloads, by default. I mean, this is something simple to catch at a global level with a central service like Copr. We don't have to rely exclusively on the author of a .spec file to always catch this issue - for each .spec file. Consider this as defense-in-depth, if you like. Similar to - say - compiling your software with stack-protector enabled, even when the software is in good shape.
> Counterexample (where the _disable_source_fetch setting doesn't help): Sorry but i don't quite understand what you mean here..."Counterexample" to what? We use _disable_source_fetch 0 to _implement_ the downloading of the Source. rpkg sets that macro off and then calls rpmbuild -bs to do the job. rpmbuild afterwards will download the sources (if they are not already present) and because this line: https://github.com/gsauthof/copr-epel/blob/master/patchelf/patchelf.spec#L9 contains http:// protocol for Source0 and because rpmbuild doesn't mind that, the Source0 is normally downloaded. Now, I think, the line at https://github.com/gsauthof/copr-epel/blob/master/patchelf/patchelf.spec#L9 should be definitely changed to use https://. I also think if any extra-protection is needed, then it should be implemented in the piece of code that downloads the sources (that is in rpmbuild). Simply put, I think this issue should be opened against rpm(build). rpkg could implement that check as well but it would be troublesome (if _disable_source_fetch is false and if the source is not present and hence should be downloaded and if the url begins with https://, then throw error).
Okay, I can now see how exactly rpm implements the downloading and we actually can tweak the following two rpm macros to forbid http:// %__urlhelpercmd /usr/bin/curl %__urlhelperopts --silent --show-error --fail --globoff --location -o so changing %__urlhelperopts to: %__urlhelperopts --proto -http --silent --show-error --fail --globoff --location -o would do the job. Now the question is if this shouldn't be a default. For that reason, the issue against rpm would be good. But we have a way to do this in COPR on our own. Thanks.
Just to clarify: I misread your 'Comment 1'. My counterexample was to address your 'That's what we do' part. I overlooked your 'We set `_disable_source_fetch` to 0' part. Consider my 'counterexample' then as real-world example how easily things can go wrong with the current defaults Copr uses.
(In reply to Georg Sauthoff from comment #5) > Just to clarify: I misread your 'Comment 1'. My counterexample was to > address your 'That's what we do' part. I overlooked your 'We set > `_disable_source_fetch` to 0' part. > > Consider my 'counterexample' then as real-world example how easily things > can go wrong with the current defaults Copr uses. Ok, cool. Did you open an issue against rpm to have this as default?
No, I didn't open any issue against rpm. Regarding possible changes to improve the situation: What about just putting the checksum beside the source URL, i.e. inside the .spec file? Whatever software that evaluates Source lines would then have to be changed to verify the downloaded file against the checksum. For example something like this: Source0: http://releases.nixos.org/patchelf/patchelf-%{version}//%{name}-%{version}.tar.bz2 Source0-sha256: a0f65c1ba148890e9f2f7823f4bedf7ecad5417772f64f994004f59a39014f83 This scheme protects HTTP and HTTPS downloads. I imagine not every project cares to upgrade from HTTP to HTTPS. Also, just using HTTPS isn't sufficient in general - with complex mirror networks a lot can happen before a file gets to the HTTPS endpoint that delivers the source archive. At the moment, curl doesn't provide a no-downgrade switch. Thus, it would be optimal to specify `--proto =https` if and only if the URL starts with https://. FWIW, my current work-around for my Copr packages is to switch from 'rpkg' to the 'makefile' build method. It works like this: 1. trivial makefile calls a package specific build script: https://github.com/gsauthof/copr-epel/blob/master/.copr/Makefile 2. build script defines URL, filename and checksum, e.g.: https://github.com/gsauthof/copr-epel/blob/master/patchelf/build.sh 3. a generic script that does the actual download/verification is sourced: https://github.com/gsauthof/copr-epel/blob/master/build.sh Advantage: downloads are reliably verified [1] Disadvantage: you have to update source file versions in 2 places [1]: using that scheme I even discovered a checksum mismatch in the nemiver source, i.e. Fedora 27 SRPM .tar.xz vs. current gnome download - compared the archive contents and there are no differences, thus just something harmless like an accidental recreation of the archive likely caused this.
(In reply to Georg Sauthoff from comment #7) > No, I didn't open any issue against rpm. > > Regarding possible changes to improve the situation: > > What about just putting the checksum beside the source URL, i.e. inside the > .spec file? Whatever software that evaluates Source lines would then have to > be changed to verify the downloaded file against the checksum. > > For example something like this: > > Source0: > http://releases.nixos.org/patchelf/patchelf-%{version}//%{name}-%{version}. > tar.bz2 > Source0-sha256: > a0f65c1ba148890e9f2f7823f4bedf7ecad5417772f64f994004f59a39014f83 > > This scheme protects HTTP and HTTPS downloads. I imagine not every project > cares to upgrade from HTTP to HTTPS. Also, just using HTTPS isn't sufficient > in general - with complex mirror networks a lot can happen before a file > gets to the HTTPS endpoint that delivers the source archive. > > At the moment, curl doesn't provide a no-downgrade switch. Thus, it would be > optimal to specify `--proto =https` if and only if the URL starts with > https://. > > FWIW, my current work-around for my Copr packages is to switch from 'rpkg' > to the 'makefile' build method. It works like this: > > 1. trivial makefile calls a package specific build script: > https://github.com/gsauthof/copr-epel/blob/master/.copr/Makefile > 2. build script defines URL, filename and checksum, e.g.: > https://github.com/gsauthof/copr-epel/blob/master/patchelf/build.sh > 3. a generic script that does the actual download/verification is sourced: > https://github.com/gsauthof/copr-epel/blob/master/build.sh > > Advantage: downloads are reliably verified [1] > Disadvantage: you have to update source file versions in 2 places > > [1]: using that scheme I even discovered a checksum mismatch in the nemiver > source, i.e. Fedora 27 SRPM .tar.xz vs. current gnome download - compared > the archive contents and there are no differences, thus just something > harmless like an accidental recreation of the archive likely caused this. Interesting solution. By the way, I opened https://pagure.io/copr/copr/pull-request/211. You can say your word there.
Fixed by https://pagure.io/copr/copr/c/11253003dffb3c8ce860ce9f57e7d56c414dc586
Hello, new COPR version has been released.