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

Ensure cpanm downloads modules over HTTPS #164

Closed
wants to merge 1 commit into from

Conversation

dgl
Copy link
Contributor

@dgl dgl commented Jul 2, 2024

Currently cpanminus defaults to using http, it doesn't know enough about the environment it is in for that to be possible to change (yet). Note that cpm does use HTTPS by default, so it's a potentially surprising difference depending which package manager is picked.

However, this image knows it has ca-certificates and a way to download over HTTPS, so it can default the mirror to the HTTPS version of the default URL. This is roughly following the suggestion from miyagawa/cpanminus#611 (comment) -- except using only --mirror and without the --verify part.

If Module::Signature was installed it would be possible to add --verify too, although there is a chain of trust issue there as well as needing more tools in the image (gpg), unless we also use the approach in #163 for Module::Signature and its deps.

tl;dr: This is the most minimal change that I think slightly raises the security bar.

@waterkip
Copy link
Contributor

waterkip commented Jul 2, 2024

We have reverted it in the past because of several issues. See #115 for a discussion on those. I don't think we can merge this, sorry.

@dgl
Copy link
Contributor Author

dgl commented Jul 2, 2024

@waterkip this is very carefully not setting --mirror-only or --from; it avoids the issue the revert was needed for. It uses https:// URLs for the CPAN meta DB search and and the CPAN download, it does not change anything about backpan (which will still use HTTP for now).

@dgl
Copy link
Contributor Author

dgl commented Jul 2, 2024

Basic sanity test with the issue from #116:

root@b94a07ba268d:~# export PERL_CPANM_OPT="-M https://www.cpan.org"
root@b94a07ba268d:~# cpanm App::cpm@0.997002
Found App::cpm 0.997017 which doesn't satisfy == 0.997002.
root@b94a07ba268d:~# export PERL_CPANM_OPT="--cpanmetadb https://cpanmetadb.plackperl.org/v1.0/ --mirror https://www.cpan.org"
root@b94a07ba268d:~# cpanm App::cpm@0.997002
--> Working on App::cpm
Fetching http://backpan.perl.org/authors/id/S/SK/SKAJI/App-cpm-0.997002.tar.gz ... OK

So backpan works, the download happens over HTTP, but there's not an option for that until cpanminus supports it properly.

@zakame
Copy link
Member

zakame commented Jul 3, 2024

Hi @dgl, thanks for the PR!

There's actually some upstream work being done with this, cc @stigtsp @garu miyagawa/cpanminus#674

To add with what @waterkip said, the core issue with using ENV PERL_CPANM_OPT is that it can violate the principle of least astonishment: someone or something downstream calling cpanm can break down because it is not expected for this environment variable to be set. A good example would be someone using their own DarkPAN first before the standard CPAN (regardless of HTTP/S) - as it stands in https://github.com/miyagawa/cpanminus/blob/96fd87ebbcc565ed9f367e7486af401b10182c38/Menlo-Legacy/lib/Menlo/CLI/Compat.pm#L152-L153, setting an HTTPS mirror first through the variable would break this.

At the moment, I'd rather propose using App::cpm for doing installs if HTTPS is strongly preferred, since that already uses HTTPS by default - I am considering amending the documentation here and in https://github.com/docker-library/official-images to use cpm install instead of cpanm.

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

Successfully merging this pull request may close these issues.

3 participants