Skip to content
This repository has been archived by the owner on Nov 2, 2023. It is now read-only.

Investigate Rack protection settings #256

Open
florianm opened this issue Dec 8, 2021 · 1 comment
Open

Investigate Rack protection settings #256

florianm opened this issue Dec 8, 2021 · 1 comment

Comments

@florianm
Copy link
Contributor

florianm commented Dec 8, 2021

Problem

v0.3.7 works using the development server, but logs the user out unexpectedly when using the production server.
This indicates a broken setting on the server side.

Fix

@smoyth found the following fix:

There is a part of Sinatra called Rack::Protection that protects against various sorts of attacks. One of those protections getting tripped which was causing the session to be cleared which was causing the logout.This Rack::Protection got added to the Gemfile due to Yaw's "upgrading dependencies" commit. It is generally a good thing to have so I tried disabling a few of the usual suspects (e.g. authenticity_token , json_csrf but that didn't fix it.Total disabling of all protections works: disable :protection [...]
Considering that none of these protections were in place prior to this release, I think we can safely disable them.

Status quo

  • ODK Build 0.3.8 works and passes my test scenarios. Be aware these test the happy path and do not touch all possible edge cases. My tests do not prove the absence of any bugs.
  • Security is not decreased, as the patch only disables a newly added setting.
  • @issa-tseng has approved Tom's patch disabling protection in Disable Rack::Protection #255 (comment)
  • Older releases didn't have Rack::Protection - do we need it now? Edit: It's a new feature that was introduced between Build releases, OK without for now.

Discussion

Help wanted: someone experienced in Ruby Rack / Sinatra could advise on which protections should be added.

@florianm florianm added infrastructure needs discussion dependencies Pull requests that update a dependency file labels Dec 8, 2021
@florianm florianm modified the milestones: 0.3.5, 0.5.0 Dec 8, 2021
@yanokwa
Copy link
Member

yanokwa commented Dec 8, 2021

According to @smoyth...

I'm pretty sure the previous level was zero. This is a newer addition to Sinatra. The rack-protection gem did not exist in the project prior to this. I suppose the functionality could have gotten moved into that gem from the main Sinatra gem though. But I doubt it.

Given that, I'm happy with the disabled projections. It would be nice if a contributor wants to try to find the protections that would work, but I think it should be a low priority.

@florianm florianm added help wanted and removed infrastructure needs discussion dependencies Pull requests that update a dependency file labels Dec 8, 2021
@florianm florianm closed this as completed Dec 9, 2021
@florianm florianm reopened this Dec 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants