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

Ignore broken certificates when using n over ssl proxy #426

Closed
wants to merge 1 commit into from
Closed

Ignore broken certificates when using n over ssl proxy #426

wants to merge 1 commit into from

Conversation

alqu
Copy link

@alqu alqu commented Apr 6, 2017

In wget there is already the "--no-check-certificate" option, but in curl it was missing. Added.

Pull Request Template:

Describe what you did

I added an additional param for curl that ignores invalid ssl certificates. Otherwise, the download fails and leaves a broken install behind.

How you did it

I tried to pull a newer version of node with n latest through an ssl proxy. Curl was complaining that the certificate was not valid - which is true for SSL proxies a.k.a. MITM.

How to verify it doesn't effect the functionality of n

Fetching updates with a "normal" environment works flawlessly.

In wget there is already the "--no-check-certificate" option, but in curl it was missing. Added.
@dskrvk
Copy link

dskrvk commented Jan 17, 2018

This shouldn't be enabled by default in either place (#475 removes the option for wget). If some users absolutely need this and are OK with sacrificing security, there can be an option to skip certificate checks.

@dskrvk
Copy link

dskrvk commented Jan 17, 2018

Actually, the same change has already been submitted and received a similar comment in #324.

@shadowspawn
Copy link
Collaborator

Proxy setup is a bit tricky @alqu, but as noted by @dskrvk the default for curl and wget should be secure.

@alqu
Copy link
Author

alqu commented Mar 26, 2019

Thanks!

Meanwhile, this feature request is not that urgent nor valid anymore, so I am fine with it.

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