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

Avoid cross origin forgery by using same-site cookie #2948

Merged
merged 2 commits into from
Jun 3, 2021
Merged

Avoid cross origin forgery by using same-site cookie #2948

merged 2 commits into from
Jun 3, 2021

Conversation

bjoernricks
Copy link
Contributor

@bjoernricks bjoernricks commented May 27, 2021

What:

Only allow access to the session cookie from the same site and not for
third parties.

Why:

This avoids CSRF attacks like like e.g.
BREACH.

For more details please take a look at
http://www.sjoerdlangkemper.nl/2016/04/14/preventing-csrf-with-samesite-cookie-attribute/

How:

Build new gsad and did take a look if the samesite cookie settings are present in the response of the login request. Additionally I've clicked though several pages to test if something did break.

Checklist:

  • Tests
  • CHANGELOG Entry
  • Labels for ports to other branches

Only allow access to the session cookie from the same site and not for
third parties. This avoids CSRF attacks like like e.g.
[BREACH](http://breachattack.com/).

For more details please take a look at
http://www.sjoerdlangkemper.nl/2016/04/14/preventing-csrf-with-samesite-cookie-attribute/
@bjoernricks bjoernricks added port-to-main Use mergifiy to port PR to master port-to-stable Use mergifiy to port PR to stable port-to-21.10 labels May 27, 2021
@bjoernricks bjoernricks requested a review from a team as a code owner May 27, 2021 18:44
@bjoernricks bjoernricks enabled auto-merge May 27, 2021 18:46
@codecov
Copy link

codecov bot commented May 27, 2021

Codecov Report

Merging #2948 (d0b8867) into gsa-20.08 (2f34292) will increase coverage by 0.67%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##           gsa-20.08    #2948      +/-   ##
=============================================
+ Coverage      53.06%   53.74%   +0.67%     
=============================================
  Files           1072     1072              
  Lines          25900    25908       +8     
  Branches        7372     7372              
=============================================
+ Hits           13744    13923     +179     
+ Misses         11034    10881     -153     
+ Partials        1122     1104      -18     
Impacted Files Coverage Δ
gsa/src/web/pages/extras/cvsscalculatorpage.js 94.73% <ø> (ø)
gsa/src/web/pages/extras/feedstatuspage.js 97.67% <ø> (ø)
gsa/src/web/pages/extras/trashcanpage.js 3.27% <ø> (ø)
gsa/src/web/pages/permissions/multipledialog.js 15.78% <ø> (ø)
gsa/src/web/pages/radius/dialog.js 66.66% <ø> (ø)
gsa/src/web/pages/results/details.js 47.12% <ø> (+40.22%) ⬆️
gsa/src/web/pages/start/page.js 51.23% <ø> (ø)
gsa/src/web/pages/users/detailspage.js 33.33% <ø> (ø)
gsa/src/web/pages/users/row.js 26.66% <ø> (ø)
gsa/src/gmp/models/user.js 100.00% <100.00%> (ø)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b97bb0...d0b8867. Read the comment docs.

@bjoernricks bjoernricks merged commit a6c8b7b into greenbone:gsa-20.08 Jun 3, 2021
@bjoernricks bjoernricks deleted the same-site-session-cookie branch June 3, 2021 13:18
bjoernricks added a commit that referenced this pull request Jun 3, 2021
Avoid cross origin forgery by using same-site cookie (backport #2948)
bjoernricks added a commit that referenced this pull request Jun 3, 2021
Avoid cross origin forgery by using same-site cookie (backport #2948)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port-to-main Use mergifiy to port PR to master port-to-stable Use mergifiy to port PR to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants