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

Use of rejectUnauthorized: false as default #17

Open
Frank683 opened this issue Jul 30, 2020 · 4 comments
Open

Use of rejectUnauthorized: false as default #17

Frank683 opened this issue Jul 30, 2020 · 4 comments

Comments

@Frank683
Copy link

Frank683 commented Jul 30, 2020

Hi,

It is a bad idea to use a default setting of rejectUnauthorized: false
If anyone uses a local, non public CA for their certificates they need to provide it via https.certificateAuthority to make the request work.
Disabling TLS certificate checks in general - without even mentioning it in the documentation - is a really bad idea.

Appreciate your comments and hopefully a change of this setting with the next release.

@s-KaiNet
Copy link
Owner

Hi, thank you for your valuable feedback!

I know, that in nodejs programming, rejectUnauthorized: false might be considered as harmful and bad practice. However, the target platform for this package is SharePoint. It's used by many other related projects as well. A lot of SharePoint installations used to be on-premisses. I saw a lot of issues when users complain that everything stopped working because something wrong was with their on-premises certificates. That is why from the beginning of the project that was the default value and I haven't heard any complaints about it. When you work with on-premises installations, you have a clear understanding, what is your target platform, what is the security configuration. Thus I don't see any big issues having this default. Now you see why I can't change it - it will immediately break a lot of projects, and what is more important, I don't see any good reason to do it.

I updated documentation to indicate the default parameters.

@Frank683
Copy link
Author

I see why you can't change it right now, that's clear so far. A major version bump (like 2.x to 3.x) would have been a chance to do it since it introduced breaking changes (got as replacement of request-promise) anyway.

For sure lots of SP installations were and are on-premise but I think more and more will also be on SP online what makes the topic quite relevant.

If you google for the usual SSL certificate error messages from the node SSL module you'll find like 90% of answers suggesting to just set rejectUnauthorized: false. This will surely get rid of the error messages but at the cost of security, so why use SSL anyway then?

To conclude, I understand that it can't be changed right now but I would appreciate if it could be done somehow in future versions. Thank you for adapting the documentation to point out the default behavior or the module.

@s-KaiNet
Copy link
Owner

The problem is not only about version change, but code change as well. I will have to change sp-request, node-sp-auth, and some other related modules. All modules, which depend on node-sp-auth and sp-request should also be adjusted accordingly. Also, I don't think that SharePoint Online certificates will be ever compromised.

Taking all this into account, at least for now, I don't want to make these changes. I'm really sorry if I made you feel frustrated, but sometimes we all have to make complicated decisions.

@patlachance
Copy link

Also, I don't think that SharePoint Online certificates will be ever compromised.

Data Leakage Prevention solutions used within major companies intercept TLS traffic and usually use internal root certificate authorities to dynamically generate custom TLS certificates presented to HTTP clients for them to establish a secured connection with the browsing appliance.

The problem is not only about version change, but code change as well. I will have to change sp-request, node-sp-auth, and some other related modules. All modules, which depend on node-sp-auth and sp-request should also be adjusted accordingly.

I know this is lot of work but would you mind:
1- reconsidering
2- explain what's the best way to change the various code so that others could propose pull requests?

Thanks for your help

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

No branches or pull requests

3 participants