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

docs: Update Skip SSL to mention it is insecure (no-changelog) #11935

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Ten0
Copy link

@Ten0 Ten0 commented Nov 27, 2024

Fixes part 1 of this issue.

Looking up the forum, it looks like most people in the n8n ecosystem are unaware that checking the "Ignore SSL Issues" is a security hazard. The team suggests using it to unaware users, but most users just won't even know what SSL is.

This PR helps at fixing this security issue by letting people who might care about security know that checking this box is in fact a security issue.

(Specifically, checking "Ignore SSL issues" for example allows anyone on the same wifi as a running instance to steal any such credentials it may hold by doing ARP spoofing, and some people on the internet to steal them by doing IP spoofing or DNS spoofing.)

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

Most people in the n8n ecosystem seem unaware that checking the "Ignore SSL Issues" is a security hazard.
[The team suggests using it to unaware users](https://community.n8n.io/t/self-signed-certificate-in-certificate-chain/20709/2), but most users just won't even know what SSL is.
They have no idea of the security hazards of checking this box.

This PR helps at fixing this security issue by letting people who might care about security know that checking this box *is* a security issue.

(Specifically, checking this box for example allows anyone on the same wifi as a running instance to steal any such credentials it may hold by doing ARP spoofing.)
@CLAassistant
Copy link

CLAassistant commented Nov 27, 2024

CLA assistant check
All committers have signed the CLA.

Joffcom
Joffcom previously approved these changes Nov 27, 2024
Copy link
Member

@Joffcom Joffcom left a comment

Choose a reason for hiding this comment

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

Thanks for this

@n8n-assistant n8n-assistant bot added community Authored by a community member core Enhancement outside /nodes-base and /editor-ui node/improvement New feature or request ui Enhancement in /editor-ui or /design-system in linear Issue or PR has been created in Linear for internal review labels Nov 27, 2024
@Joffcom
Copy link
Member

Joffcom commented Nov 27, 2024

Hey @Ten0,

Thanks for the PR, We have created "GHC-513" as the internal reference to get this reviewed.

One of us will be in touch if there are any changes needed, in most cases this is normally within a couple of weeks but it depends on the current workload of the team.

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...components/SettingsLogStreaming/descriptions.ee.ts 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Ten0
Copy link
Author

Ten0 commented Nov 27, 2024

Test fails seem unrelated.

@Joffcom Joffcom changed the title (security) Let users know that skipping SSL validation is insecure docs: Update Skip SSL to mention it is insecure (no-changelog) Nov 28, 2024
@Joffcom
Copy link
Member

Joffcom commented Nov 28, 2024

@Ten0 the lint failed tests are related to your changes, Are you able to run the linter to resolve them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Authored by a community member core Enhancement outside /nodes-base and /editor-ui in linear Issue or PR has been created in Linear for internal review node/improvement New feature or request ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants