miyagawa / cpanminus

cpanminus - get, unpack, build and install modules from CPAN
http://cpanmin.us
746 stars 213 forks source link

make cpanm secure by default #674

Open garu opened 2 months ago

garu commented 2 months ago

Hi Miyagawa! Thank you so much for all the work you put to cpanm, it's an amazing tool that I use pretty much daily.

Right now, cpanm downloads from HTTP, not HTTPS, even on systems with a fully working TLS.

This patch enforces HTTPS on everything, though still allowing one to force HTTP via --from or --mirror.

We have built and tested the fatpacked cpanm with these changes and they all work really well. Please let me know if you have any concerns or if there is anything I can do to get this merged and released.

Cheers!

miyagawa commented 2 months ago

I think this is good for 99% of the users out there with a working TLS via curl out of the box. My concern is that it would suddenly break the remaining 1% on their CI systems.

I wonder if it makes sense to automatically fallback to HTTP for a while and then disable the fallback as a next step.

atoomic commented 2 months ago

Would not you face the same problem you are describing when disabling the fallback later? I would say, if someone really wants to use http, he can either set a custom mirror or use an older release of cpanm.

note: the 5.8 ci was recently removed.

atoomic commented 2 months ago

We could also consider only supporting SSL but provide a more user friendly message on failures if TLS is not available so users are in charge and can use an alternate solution if needed.

garu commented 2 months ago

@miyagawa we did a new commit to this PR patching configure_http so that it would let people know when an SSL backend is not available.

We also tweaked sub mirror to detect SSL certificate issues even when the client thinks it can handle SSL but the system's certificates don't exist or are invalid.

We believe this fully covers the edge cases you mentioned. Please let us know if you need any amends.

(if this update is acceptable, would you like us to also send the PR to another branch, like master?)

miyagawa commented 2 months ago

Would not you face the same problem you are describing when disabling the fallback later?

Sure, but that can be done much later. I would say that HTTPS by default but allowing automatic fallback is still better than HTTP by default. Nobody would disagree that HTTPs by default and disallowing fallback is the most secure, but I am not sure if we're ready to pull that trigger now.

That can definitely change if e.g. perl ships with IO::Socket::SSL as a core module (iiuc there's a discussion about it in PSC).

stigtsp commented 2 months ago

[...] allowing automatic fallback is still better than HTTP by default. Nobody would disagree that HTTPs by default and disallowing fallback is the most secure, but I am not sure if we're ready to pull that trigger now.

Automatic fallback from HTTPS to HTTP would imho be equivalent to not using HTTPS at all, as it could allow for a downgrade attack.

That can definitely change if e.g. perl ships with IO::Socket::SSL as a core module (iiuc there's a discussion about it in PSC).

Wouldn't HTTPS support in those cases be provided by for example curl, wget, via HTTP::Tinyish?

Allowing HTTP (or unverified HTTPS) with an explicit --insecure parameter or similar would make more sense, and should accommodate corner cases like an environment that is missing cacerts or TLS libraries.

garu commented 2 months ago

@miyagawa we have worked on an HTTP fallback, which is now #678, merging directly to master.

It contains an --insecure flag, like curl, that turns everything back to http-only, restoring current cpanm behavior when used. It also fallbacks to http-only whenever you deliberately use an http:// uri either directly or on --mirror.

If it pleases you, we can also do that same enforcement here.

Thanks!

miyagawa commented 2 months ago

@stigtsp

Automatic fallback from HTTPS to HTTP would imho be equivalent to not using HTTPS at all, as it could allow for a downgrade attack.

i'm aware I was being unclear. I was not promoting automatic downgrade from HTTPS to HTTP. It was only about choosing the default whether it should be HTTPs or HTTP. Once HTTPS is chosen, of course it should not automatically fall back to HTTP.

So the fallback to HTTP should only be allowed if:

The tricky bit here is that when the HTTP client gets a certificate error, there's no way to identify that if it's connected to an untrusted source, or if the user's TLS environment is not working (e.g. system certificates are too old).

atoomic commented 2 months ago

I think the behavior implemented here is providing what you describe:

but I would be encline to ignore that discussion for now, and focus on change from #678 which is the real fix for current customers.

Note that with the current released version of cpanm (which does not use HTTPTinyish) it's even harder to detect if the failure is coming from a certificate error. HTTPTinyish make it easier as it provides an object (aka HashRef) which contains all information including the error message.

But you are right when there is a certificate error, we do not know what's wrong. This is why we currently rely on the user and ask him to investigate the issue, and if needed use the --insecure option, which can be too strong if used blindly.

stigtsp commented 2 months ago

Hi @miyagawa

The tricky bit here is that when the HTTP client gets a certificate error, there's no way to identify that if it's connected to an untrusted source, or if the user's TLS environment is not working (e.g. system certificates are too old).

I agree with the situations you're describing: hypothetical determinations like "this CA store is too old, ignore it" or similar will be fragile, and are likely to introduce vulnerabilities. A fatal error is an important feature when we cannot verify that the connection is to a trusted host.

So, do we actually need to optimize for edge cases where users update their cpanminus, but have a broken TLS setup?

I'm hoping an explicit --insecure and/or some environment variable would be a sufficient break-glass option for users who don't need secure transport when downloading and executing code from the network.

Thanks again for looking at this