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

multi: allow persistent encryption key #946

Closed
buck54321 opened this issue Jan 27, 2021 · 8 comments · Fixed by #1119
Closed

multi: allow persistent encryption key #946

buck54321 opened this issue Jan 27, 2021 · 8 comments · Fixed by #1119

Comments

@buck54321
Copy link
Member

When the user logs in, they could be presented with a checkbox similar to a "remember me" option that will cause the app password to be effectively cached for the duration of their session. That said, the app password is all powerful, so I don't think we should cache the actual password or even the decoded Crypter. Instead, I think we should generate some kind of "session key" that enables generating the password for the duration of the user's session, but does not itself encode the encryption key.

type SessionAuth interface {
	PW() ([]byte, error)
}

func (c *Core) Session(pw []byte) (SessionAuth, error)

The SessionAuth itself would encrypt the password with some kind of temporary key and only reveal it when prompted. This approach has the benefit that Core is still in control of destroying the session key since Core controls the implementation of the concrete type.

When the user selects the "remember my password" checkbox, the WebServer knows to generate and cache a core.SessionAuth. Then, whenever WebServer has a password, they can do something like

if len(form.PW) == 0 && s.sessionAuth != nil {
	form.PW, err := s.sesssionAuth.PW()
	// handle error
}

The front end would need to know somehow whether there is an active "session" so we can hide the password fields. An empty or null password also takes on a special significance with this scheme, signaling from to the webserver to check for a SessionAuth.

On logout, the WebServer should forget the cached session, but Core should also disable the SessionAuth somehow so it doesn't really matter.

Anyway, this is just one suggestion, and there are probably better ways to do it. I've been revisiting this for over and over though, and haven't come up with anything super clean.

@martonp
Copy link
Contributor

martonp commented Jun 8, 2021

I'll work on this one.

@martonp
Copy link
Contributor

martonp commented Jun 21, 2021

@buck54321 What's the benefit of caching both an encrypted password and the encryption key vs. just caching the password itself?

@buck54321
Copy link
Member Author

I'm just a little weary about keeping the raw password in memory. It really just amounts to a layer of obfuscation, but maybe better than nothing? I'd love to hear others' opinions.

@martonp
Copy link
Contributor

martonp commented Jun 21, 2021

The key could be cached in the UI so that all the information is not in the same place.

@buck54321
Copy link
Member Author

What key? What do you mean by "in the UI"?

@martonp
Copy link
Contributor

martonp commented Jun 21, 2021

The encrypted password would be stored in the ‘SessionAuth’ object, and the key used to encrypt it would be cached in the JavaScript code. When the user is logged into a session, instead of sending the password, the cached key would be sent in the request. In this way, not only is there a layer of obfuscation, but also the key is separated from the encrypted password.

@buck54321
Copy link
Member Author

I could see something like that. You'll have to consider how to handle refreshes and new tabs. Sounds like the session key would need to be stored in Window.localStorage too. You'll also need to consider how to handle expired keys. On load, the Application will need to check localStorage and check any stored session key with the server to see if it's still valid, otherwise we won't know whether to hide password fields.

@martonp
Copy link
Contributor

martonp commented Jun 21, 2021

Agreed.

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 a pull request may close this issue.

2 participants