Added 'rejectUnauthorized' to support self-signed TLS certificates

[cuongvo]

Per #566

[am11]

🎉

[nsams]

really? blindly adding that option should be no solution...

if @cuongvo is using a self-signed certificate he should import that so https is working correcetly.

[am11]

There is not much leeway for users behind corporate firewalls anyway. From our part, we are opting for most lenient settings, so our install scrip can grab libsass binary.

[nsams]

Well, the point of https and server certificates is that you can trust the server and not a man-in-the-middle. And this change disabled this. At least it could be optional (eg. use an env var like for proxy settings).

But I have to say that I don't know how corporate firewalls usually work with https.

[phosphore]

Bumping this after 6 years:

@nsams At least it could be optional (eg. use an env var like for proxy settings). #567 (comment)

Applying this insecure flag by default only to make optional binary configuration parameters work is not ideal. Since the getBinaryUrl function was designed to fallback to https://github.com/sass/node-sass/releases/download, the certificate validation will be disabled when requesting the binaries from github.com even if the user is not specifying an alternative download path.

node-sass/lib/extensions.js

Lines 242 to 250 in 5a4a48a

	function getBinaryUrl() {
	var site = getArgument('--sass-binary-site') ||
	process.env.SASS_BINARY_SITE ||
	process.env.npm_config_sass_binary_site ||
	(pkg.nodeSassConfig && pkg.nodeSassConfig.binarySite) ||
	'https://github.com/sass/node-sass/releases/download';
	return [site, 'v' + pkg.version, getBinaryName()].join('/');
	}

@am11 we are opting for most lenient settings, so our install scrip can grab libsass binary. #567 (comment)

I understand this, but you should at least consider letting node-sass users know the risk in the README section.

This should be treated as a security bug.

[saper]

Shall we revert this?

[cuongvo]

Agreed this should be optional and clearly documented. This is a path for folks with issues with the install and are knowingly bypassing certificate verification. Normal users should have the secure path by default.

6 year ago me shouldn't have sent in this PR 😅

I don't have the context on the impact of reverting this change. It may be worth considering this more thoroughly. I can see this breaking users who depend on this being insecure for their build servers and releases in internal environments that will need to modify code.

[phosphore]

I don't have the context on the impact of reverting this change. It may be worth considering this more thoroughly. I can see this breaking users who depend on this being insecure for their build servers and releases in internal environments that will need to modify code.

Wouldn't it be enough to check if the user has set either:

node-sass/lib/extensions.js

Lines 243 to 246 in 5a4a48a

	var site = getArgument('--sass-binary-site') ||
	process.env.SASS_BINARY_SITE ||
	process.env.npm_config_sass_binary_site ||
	(pkg.nodeSassConfig && pkg.nodeSassConfig.binarySite) ||

If that's the case apply the rejectUnauthorized, else don't?

[OS-WS]

Hi, is there a fix for CVE-2020-24025?
if so, in what commit?

thanks in advance!

[LiuJinghao]

Version 5.0.0 was released in August, but through reading the source code of 5.0.0, we found that this issue is still unresolved.

Is there a plan to fix this issue?