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

First pass at adding HTTPS support to CPAN. #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

First pass at adding HTTPS support to CPAN. #119

wants to merge 1 commit into from

Conversation

dweekly
Copy link

@dweekly dweekly commented May 24, 2018

To address #118

Adds HTTPS to MIRRORED.BY and enforces cert checks with LWP using Mozilla::CA.

@MartinMcGrath
Copy link

https support, and a having a default urllist pointing to an https site seems like a great thing to have.

References:

https://perlmonks.org/?node_id=11108980

https://rt.cpan.org/Public/Bug/Display.html?id=130819

Copy link
Contributor

@briandfoy briandfoy left a comment

Choose a reason for hiding this comment

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

Looks good to me on the parts I know about.

@dweekly
Copy link
Author

dweekly commented May 18, 2020

May want to consider adding LWP::Protocol::https as a new preferred-but-optional dependency? Wasn't sure of the best place to add that.

@andk
Copy link
Owner

andk commented May 19, 2020

Oh, sorry, I haven't been paying enough attention, not sure how it happened. Now I looked and I'm not happy when I see such a line in a patch:

+               map { $->can( 'https' ) ? $_->https : $_->http } @mirrors

Probably not tested?
I'm also not happy, when http is simply replaced with https, it should be a seamless fallback when people have broken https support or do not want to use https. I'm not sure how to do this right either or I would have taken care of this sooner.

@dweekly
Copy link
Author

dweekly commented May 19, 2020

Hi, @andk! Thanks for the feedback and review.

Let me:
A) Fix the typo.
B) Add more tests.
C) Handle if HTTPS is not available gracefully.

Do you think there should be a config setting to disable HTTPS?

@andk
Copy link
Owner

andk commented May 19, 2020 via email

@dweekly
Copy link
Author

dweekly commented May 23, 2020

@andk Before proceeding much further, I thought it would be helpful to articulate the vision and plan here to get your input on the direction: https://docs.google.com/document/d/1DRkiCJhJu4RDI0u_JppBpFa0djouskxEyNHax912U_w/edit?usp=sharing

@andk
Copy link
Owner

andk commented May 24, 2020

Thanks a lot. I think I'm through with commenting now. I have added 5 comment boxes

@Grinnz
Copy link
Contributor

Grinnz commented Mar 11, 2021

@dweekly FYI, some sections of that document are now resolved by nature of the CPAN mirror network now being redundant - see https://log.perl.org/2021/02/cpan-mirror-list-changes.html

@dweekly
Copy link
Author

dweekly commented Mar 11, 2021

@dweekly FYI, some sections of that document are now resolved by nature of the CPAN mirror network now being redundant - see https://log.perl.org/2021/02/cpan-mirror-list-changes.html

@Grinnz - thanks for flagging. I think it's fair that dealing with a diversity of CPAN endpoints will now be moot with the deprecation of the MIRRORED.BY list, but CPAN should still ensure that the connection to www.cpan.org is secure and authenticated (namely, using >=TLS 1.2 with hostname verification to ensure you're talking to the real CPAN.org) in order to close off a range of MITM attacks on Perl users and services.

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.

5 participants