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: transparent encryption of session data #166

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jas-
Copy link

@jas- jas- commented Mar 10, 2020

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

@voxpelli
Copy link
Owner

I think I much rather add the possibility to add an encoding/decoding callback than to bring this into the module itself.

There are many ways it can be done and it would probably be better to have those solutions outside the core, but have the core be extendable enough to support it still.

What do you think?

@jas-
Copy link
Author

jas- commented Mar 10, 2020

It’s your party, however from an implementation perspective, if I can turn on or turn off a security feature without the fuss of trying to get it to play well with the storage engine the better.

@voxpelli
Copy link
Owner

My thinking would be that rather than add a bool flag it would ask for an encryption strategy, so sending a compatible such one, published and versioned separately, would be about as complex as just setting the book flag – only one additional dependency.

I do very much appreciate feedback and thoughts. It makes the project and my decisions better 👍

@jas-
Copy link
Author

jas- commented Mar 11, 2020

Morning. A bool flag? Maybe the typedef for the secret option is what you are referring to? Anyways, while I understand the simplicity for the internal structure of your existing module (which is a very well thought out project), I still believe that this is a simpler method for the store to provide the requisite security as compared to trying to monkey patch it between the session module and the data storage engine.

An important question as someone using your module would be “how can I best protect my sessions in an at rest data store?” pseudo code

var session = require('express-session');
var kruptein = require('kruptein')( {use_scrypt: true} );

app.use(session({
  store: new (require('connect-pg-simple')(session))(),
  secret: process.env.FOO_COOKIE_SECRET,
  resave: false,
  cookie: { maxAge: 30 * 24 * 60 * 60 * 1000 } // 30 days
}));

app.get('/', function(req, res, next) {
  kruptein.get('squirrel', req.session, function(err, pt) {
   if (err) return fn(err);
   req.session = pt;
  });
  if (req.session.views) {
    req.session.views++
    res.setHeader('Content-Type', 'text/html')
    res.write('<p>views: ' + req.session.views + '</p>')
    res.write('<p>expires in: ' + (req.session.cookie.maxAge / 1000) + 's</p>')
    res.end()
  } else {
    req.session.views = 1
    kruptein.set('squirrel', req.session, function(err, ct) {
      if (err) return fn(err);
    });
    res.end('welcome to the session demo. refresh!')
  }
})

or as the provided pr implements it?


app.use(session({
  store: new (require('connect-pg-simple')(session))( {secret: 'squirrel' } ),
  secret: process.env.FOO_COOKIE_SECRET,
  resave: false,
  cookie: { maxAge: 30 * 24 * 60 * 60 * 1000 } // 30 days
}));

Base automatically changed from master to main January 25, 2021 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants