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

Stricter HTTPS #267

Merged
merged 6 commits into from Feb 3, 2020
Merged

Stricter HTTPS #267

merged 6 commits into from Feb 3, 2020

Conversation

mavit
Copy link
Contributor

@mavit mavit commented Jun 25, 2019

For an explanation of the motivation of these changes, see the individual commits.

I expect that, nowadays, lax handing of TLS is no-longer needed; so many more things in the world require it, so any problems are likely to have been fixed.

I think it’s reasonable to merge this with a view to reverting it if it turns out to cause serious issues for nightly users.

@michaelherger
Copy link
Member

Unfortunately we're not there yet. Many systems still are using outdated SSL libraries, often failing https connections.

@mavit
Copy link
Contributor Author

mavit commented Jun 26, 2019

Users of such systems must be having an increasingly poor experience, as more and more content becomes inaccessible to them. I wonder if we're really doing them a favour by helping them keep their old insecure systems running.

In any case, perhaps insecure HTTPS should be made optional?

@michaelherger
Copy link
Member

A huge number of LMS instances is running on NAS devices where the user has little to no control over the update status of the libraries.

Optional sounds like a reasonable plan (though I hate to have one more pref...). Maybe a setting in Advanced/Security? Would you mind working on this?

@mavit
Copy link
Contributor Author

mavit commented Jul 10, 2019

I have added the setting.

@mherger mherger self-assigned this Jul 10, 2019
@mavit
Copy link
Contributor Author

mavit commented Jan 27, 2020

@mherger: Did you have any remaining concerns about merging this?

@michaelherger
Copy link
Member

Now that I've started work on LMS 8 it's probably a good moment to give this a try. Please change PR to merge with public/8.0 instead of 7.9 (and update the changelogs accordingly - sorry!)

Before this commit, we end up trying any failed connection twice, which takes twice as long.  Since most failures have nothing to do with SNI, this seems excessinve.
- Many repository hosts (such as SourceForge or GitHub) will redirect to HTTPS if you try HTTP, and this will only become more common over time.
- If the failure was nothing to do with HTTPS, we take twice as long with no benfit.
- Falling back to HTTP undermines the purpose of HTTPS.
Setting VERIFY_NONE makes us vulnerable to man-in-the-middle attacks, and undermines the purpose of HTTPS.
@mavit mavit changed the base branch from public/7.9 to public/8.0 January 28, 2020 15:12
@mavit
Copy link
Contributor Author

mavit commented Jan 28, 2020

I've rebased against public/8.0.

@mherger mherger merged commit c076108 into LMS-Community:public/8.0 Feb 3, 2020
@mherger
Copy link
Contributor

mherger commented Feb 3, 2020

Let's give this a try - thanks!

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.

None yet

3 participants