Bug 1536846

> 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.