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

Add check for missing cookieKey, generate one if not specified #235

Closed
wants to merge 1 commit into from

Conversation

worldwise001
Copy link
Contributor

@mcpherrinm
Copy link
Contributor

Could we deterministically generate one, perhaps like we do the per-secret keys?

Randomly generating one is going to lead to confusion and sadness as your cookies are only valid on the server that generated it.

@worldwise001
Copy link
Contributor Author

Yeah we could. What would we use as a stable value to deterministically generate one reliably on multiple servers?

I guess the other consideration is that if you're going to configure more than one, maybe you will need to specify one manually.

@mcpherrinm
Copy link
Contributor

We have a master key that we derive keys per-secret -- we could derive a key for cookies too.
However, we'd have to be careful whatever we use for derivation doesn't collide with the name of a secret.

see https://github.com/square/keywhiz/blob/master/server/src/main/java/keywhiz/service/crypto/ContentCryptographer.java

@@ -75,6 +87,14 @@ public ServiceModule(KeywhizConfig config, Environment environment) {
bind(KeywhizConfig.class).toInstance(config);
}

private String generateCookieKey() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means the cookie key won't work across hosts, and it'll also get wiped on restart and everyone ends up being logged out?

@worldwise001
Copy link
Contributor Author

We can derive off the master key. I'll submit an update.

@worldwise001
Copy link
Contributor Author

Whoops I completely forgot about this. I'll see if I can get this updated today.

@mcpherrinm
Copy link
Contributor

Abandoned PR

@mcpherrinm mcpherrinm closed this Mar 21, 2017
@mbyczkowski mbyczkowski deleted the shh/keywhiz/cookie_key branch August 1, 2019 17:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Null Pointer Exception if you start without a config file
3 participants