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

crypto: Provide OWASP session management security recommendations #15

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

Conversation

jas-
Copy link

@jas- jas- commented Jan 26, 2020

This will enable OWASP sessions management recommendations for implementations that may push sensitive information to a server side session.

@jas- jas- requested a review from roccomuso January 26, 2020 15:30
@jas-
Copy link
Author

jas- commented Apr 6, 2020

@roccomuso I hope all is well. I was looking over the travis-ci build logs. Looks like this pr fails due to node v0.10 not supporting es6 class constructors. Is there a reason to support such a legacy version? I was looking over the latest LTS, it isn't even a blip on the radar and the last security patch was four years ago, unless I over looked something.

@roccomuso
Copy link
Owner

I don't know if it's worth introducing this complexity to the code. And if this is wanted at all.
The sessions are stored in-memory. Inside the process. If someone manage to get access to the server probably you'll have bigger problems than this.

@jas-
Copy link
Author

jas- commented Apr 28, 2020

Afternoon, while memory is typically considered more difficult to access than reading a file from disk or even a database store, today's malware includes memory scraping functionality that is trivial to implement.

Couple this with the ability of an attacker to use row hammer attack techniques against memory as a non privileged user or process does indeed put the security of the session at risk.

However, it is your module and if you wish to reject it that is your prerogative. Have a good one.

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

Successfully merging this pull request may close these issues.

2 participants