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

fallback to HTTP-only when TLS is unavailable #1

Open
wants to merge 1 commit into
base: secure-by-default
Choose a base branch
from

Conversation

garu
Copy link

@garu garu commented May 17, 2024

This patch will make cpanm fall back to (non-TLS) HTTP only requests when all default options are used AND TLS support is unavailable, with the following message:

WARNING: TLS support not found. Falling back to insecure HTTP-only requests

Please note that, for security reasons, if TLS support is available but broken (e.g. invalid/expired certificates), requests will fail unless you explicitly use --insecure.

Also note that if you use --insecure but provide custom HTTPS URLs, they will still fail if TLS support is not available.

Similarly, if you provide custom HTTP-only URLs, they will go over plain HTTP and not require TLS support.

cpanm will fall back to (non-TLS) HTTP only requests when
all default options are used AND TLS support is unavailable, but
please note that, for security reasons, if TLS support is
available but broken (e.g. invalid/expired certificates), requests
will fail unless you explicitly use --insecure.

Also note that if you use --insecure but provide custom HTTPS
URLs, they will still fail if TLS support is not available.

Similarly, if you provide custom HTTP-only URLs, they will go
over plain HTTP and not require TLS support.
@garu garu requested review from stigtsp and atoomic May 17, 2024 06:15
@garu
Copy link
Author

garu commented May 17, 2024

@atoomic @stigtsp I believe this should cover miyagawa's requests and concerns. Please review! (@atoomic are you able to build and test the fatpacked version like we did on the PTS?)

Menlo-Legacy/lib/Menlo/CLI/Compat.pm Show resolved Hide resolved
Menlo-Legacy/lib/Menlo/CLI/Compat.pm Show resolved Hide resolved
@atoomic
Copy link

atoomic commented May 18, 2024

Note the main PR that matters is the one targeting master branch
I can build the cpanm fatpack version but throw away the vm we used a few days a

@garu
Copy link
Author

garu commented May 18, 2024

Note the main PR that matters is the one targeting master branch I can build the cpanm fatpack version but throw away the vm we used a few days a

I just wanted to make sure this can be built and fatpacked properly before merging. I know the one that matters is the one targetting master, I just figured by doing the fallback also on the devel branch we make things easier for miyagawa, as he can keep both in sync.

(if neither of you have trouble with this patch, once we merge I'll update the one targetting the master branch as well)

$self->diag_fail( join( ', ', @protocol )." not supported by available HTTP Clients." );
}

$backend->new(agent => "Menlo/$Menlo::VERSION", verify_SSL => 1);
Copy link

@stigtsp stigtsp May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe --insecure should set verify_SSL => 0, in case the mirror used is only available on https and the user doesn't care about checking the host certs.

Copy link

@stigtsp stigtsp May 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore that, just saw the # HTTPTinyish does not provide an option to disable certificates check, let's switch to http on demand comment above

@stigtsp
Copy link

stigtsp commented May 24, 2024

Btw: Are we aware of some actual examples of situations when an automatic fallback to HTTP is needed?

Since a fallback is imho not great from a security angle, should we find some example of what we're optimizing for, and why using --insecure is not acceptable in those corner cases?

This would be useful for testing as well.

During the previous effort to change verify_SSL to 1 by default in HTTP::Tiny, there were several discussions that mentioned to a hypothetical situation where you'd bootstrap a perl installation on environments without a working SSL setup - but didn't actually see an example of those, and I'd always felt that it is awkward to optimize for those cases.

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