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

Add Secret Function Support #21

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

Add Secret Function Support #21

wants to merge 3 commits into from

Conversation

DavidTPate
Copy link

This PR is being done in conjunction with expressjs/session#214. Currently, in order to vary the secret value for the cookie parser instantiations I have to memoize them. I'd much rather be able to set the secret based upon the request. This adds that functionality and tests around it.

…e request object to create a dynamic secret.
@dougwilson
Copy link
Contributor

Please describe the exact use-case of why a secret would be influenced by a request, and try to provide links to (at least similar) crypto patterns that this matches.

The tests need to reflect a real-world example in some way and constructing a server-only secret from user-controlled input is not acceptable, unfortunately.

@dougwilson
Copy link
Contributor

Hi @DavidTPate , and update on this?

@DavidTPate
Copy link
Author

@dougwilson Sorry for the delay my workload has been crazy as of late.

I'm having some trouble coming up with a good real-world example without making too many assumptions about the system that someone has implemented so I kept it very straightforward and used user-controlled input in the tests.

What I'm wanting is the ability to vary my encryption key for a multi-tenant system so that one tenant is not able to resolve anything about another tenant's session id. You'll find similar implementations when dealing with multi-tenant systems that are authenticated to on a per-tenant basis instead of a global basis (think of Slack for a per-tenant basis and GitHub for a global basis).

One such example would be Securing Session Tokens, specifically:

Using DPAPI for cookie encryption is not a workable solution for an application that has multiple role instances because each role instance will use a different encryption key, and the Windows Azure load balancer could route a request to any instance.

As I mentioned I wanted to not make many assumptions about how someone was going to use this so I kept it quite straightforward. Do you think it would be appropriate to add some more to it where it has a function that looks up keys or something along those lines?

@rektide
Copy link

rektide commented Aug 17, 2016

Hi. We could use this. We have a key rotation system in place.

Currently we're working around the lack of ability to change cookie-parser keys by writing wrapper middleware that builds a cookie-parser, and tears it down and replaces it with a new one whenever our keys change.

Please provide any means to update the keys. This PR would be a fine implementation strategy toward that end. We don't need per-request key-providing but the implementation strategy still seems sane, and is definitely flexible.

@DavidTPate
Copy link
Author

@dougwilson As I mentioned in my last comment awhile ago I didn't want to make too many assumptions about how someone has their system setup for an example with this. I could put together a pretty simple key rotation based on setInterval or something along those lines for an example. I'm just not sure of the usefulness there.

I'm open to any suggestions that people have for something that doesn't make too many assumptions about how one might use this.

…val`. While not the best or most complete example it shows one such method of using this functionality.
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

3 participants