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

Security concerns with with webhooks #380

Closed
jani123 opened this issue Mar 15, 2020 · 3 comments
Closed

Security concerns with with webhooks #380

jani123 opened this issue Mar 15, 2020 · 3 comments

Comments

@jani123
Copy link

jani123 commented Mar 15, 2020

Describe the bug

This could be feature request as well, but I make it as bug report since this feels vulnerability. Currently webhooks works so that you have address like http://<somewhere>/webhook/<workload id>/webhook/<node name>. But the problem is that webhook endpoint needs to be public. With this kind address structure it is pretty easy to make an attack with data and pollute potentially many other systems. Eg. attacker can loop through index numbers(even uuid would make a difference) and make dictionary based node names(ppl tend to use words) and it would not get too many tries to find some working combination(very limited set).

Expected behavior

Webhooks should be harder to send data to without proper link. Eg. ActiveCampaign trigger does not have any additionaly security measures like validating requests with their credentials.

More secure addresses can be achieved by adding eg. some security get parameter along side with the address which only can be verified by n8n. If backwards compatibility is wanted, it could be option if it is required or not.

CORS header support would prevent sending data from wrong sites.

Examples what others suggests:

https://developer.bigcommerce.com/api-docs/getting-started/webhooks/about-webhooks#security suggest there should be header

https://developer.github.com/webhooks/securing/
github suggests to verify requests with signature header per received event(Btw. GithubTrigger does not seem to be checking this). This is the style I'm currently developing custom trigger with and hopefully there will be merge request for the node some day. Sadly this can be only made per service basis if they provide such powerful way.

https://developer.paypal.com/docs/integration/direct/webhooks/notification-messages/
Paypal uses signatures made with certifications. And it is used in PaypalTrigger-node

https://developers.activecampaign.com/page/webhooks
ActiveCampaign does not provide any additinal security measures so it would be important to have cryptographically better addresses.

Environment (please complete the following information):

  • n8n Version: Cloned from master few days ago

Additional context

So three things would make these better

  • More complex urls for webhooks
  • CORS Header support
  • Always use data validation in webhook triggers when service provider provides way to do so.
@janober
Copy link
Member

janober commented May 22, 2020

Thank you very much for putting this together!

I have good news. We currently make changes to the webhooks and it will the by default use a uuid. It will so be then very hard to guess them.

I hope we will also find soon some time to add proper CORS support in the future.

About the data-validation. Most Trigger-Nodes should implement it already for the other ones we also try to add it in the future. Any help there is however very appreciated.

@janober
Copy link
Member

janober commented Jun 23, 2020

We just released a beta version of n8n with static webhooks. For existing webhook-nodes it will keep on working like before. Meaning the webhook-URL is a combination of workflow-id and webhook-name. For newly created nodes it will by default create a UUID. The value can simply get changed. It is however important to mention that webhook-URLs are now unique so not two different workflows could define the path GET /webhook only one can. If two define the same and both workflows gets set to active at the same time, it will error and will not allow the second one to get activated.

The beta image: n8nio/n8n:0.70.0-static-webhooks-beta

Please inform us if everything new and existing works fine that we can release it asap. We did already test but the more people the better obviously.Thanks!

@janober
Copy link
Member

janober commented Jul 15, 2020

Got fixed in the newly released [email protected]

@janober janober closed this as completed Jul 15, 2020
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

2 participants