From ec98581520eb9f39d937b449710f3e8d2e394821 Mon Sep 17 00:00:00 2001
From: ale <ale@incal.net>
Date: Mon, 19 Dec 2022 13:52:22 +0000
Subject: [PATCH] Isolate session namespaces between users
Also provide backwards compatibility for API clients that do not set
the session_id attribute, in which case the server reverts to the old
behavior (no support for multiple concurrent sessions).
---
server/keystore.go | 20 +++++++++++++++-----
server/keystore_test.go | 12 +++++++++---
server/server.go | 2 +-
3 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/server/keystore.go b/server/keystore.go
index 88abc7c8..8a94197d 100644
--- a/server/keystore.go
+++ b/server/keystore.go
@@ -127,7 +127,6 @@ func NewKeyStore(config *Config) (*KeyStore, error) {
func (s *KeyStore) expire(t time.Time) {
s.mx.Lock()
s.userKeys.expire(t)
- //s.updateKeyspaceSize()
s.mx.Unlock()
}
@@ -177,8 +176,11 @@ func (s *KeyStore) Open(ctx context.Context, username, password, sessionID strin
}
s.mx.Lock()
- s.userKeys.addSessionWithKey(sessionID, username, &userKey{pkey: pem}, ttlSeconds)
- //s.updateKeyspaceSize()
+ s.userKeys.addSessionWithKey(
+ nsSessionID(username, sessionID),
+ username,
+ &userKey{pkey: pem},
+ ttlSeconds)
s.mx.Unlock()
return nil
}
@@ -209,10 +211,10 @@ func (s *KeyStore) Get(username, ssoTicket string) ([]byte, error) {
// Close the user's key store and wipe the associated unencrypted key
// from memory. Returns true if a key was actually discarded.
-func (s *KeyStore) Close(sessionID string) bool {
+func (s *KeyStore) Close(username, sessionID string) bool {
s.mx.Lock()
defer s.mx.Unlock()
- return s.userKeys.deleteSession(sessionID)
+ return s.userKeys.deleteSession(nsSessionID(username, sessionID))
}
func wipeBytes(b []byte) {
@@ -220,3 +222,11 @@ func wipeBytes(b []byte) {
b[i] = 0
}
}
+
+// Combine usernames and session IDs so that every user gets its own
+// session namespace. This allows for backwards compatibility for
+// clients that do not set a session_id (in which case the service
+// automatically reverts to the old behavior).
+func nsSessionID(username, sessionID string) string {
+ return username + "\000" + sessionID
+}
diff --git a/server/keystore_test.go b/server/keystore_test.go
index cb020831..9b4ed223 100644
--- a/server/keystore_test.go
+++ b/server/keystore_test.go
@@ -131,8 +131,14 @@ func TestKeystore_OpenAndGet(t *testing.T) {
t.Fatalf("keystore.Get() returned bad key: got %v, expected %v", result, expectedPEM)
}
+ // Verify user namespace isolation
+ keystore.Close("otheruser", "session")
+ if _, err := keystore.Get("testuser", ssoTicket); err != nil {
+ t.Fatalf("keystore.Get() returned error after Close(otheruser): %v", err)
+ }
+
// Call Close() and forget the key.
- keystore.Close("session")
+ keystore.Close("testuser", "session")
if _, err := keystore.Get("testuser", ssoTicket); err == nil {
t.Fatal("keystore.Get() returned no error after Close()")
}
@@ -168,12 +174,12 @@ func TestKeystore_OpenAndGet_MultipleSessions(t *testing.T) {
}
// Call Close() on the first session, key should still be around.
- keystore.Close("session1")
+ keystore.Close("testuser", "session1")
if _, err := keystore.Get("testuser", ssoTicket); err != nil {
t.Fatalf("keystore.Get() after Close(session1): %v", err)
}
// Closing the second session should wipe the key.
- keystore.Close("session2")
+ keystore.Close("testuser", "session2")
if _, err := keystore.Get("testuser", ssoTicket); err == nil {
t.Fatal("keystore.Get() returned no error after Close(session2)")
}
diff --git a/server/server.go b/server/server.go
index 149324bc..74c4439b 100644
--- a/server/server.go
+++ b/server/server.go
@@ -76,7 +76,7 @@ func (s *keyStoreServer) handleClose(w http.ResponseWriter, r *http.Request) {
return
}
- if s.KeyStore.Close(req.SessionID) {
+ if s.KeyStore.Close(req.Username, req.SessionID) {
log.Printf("Close(%s): discarded key", req.Username)
}
--
GitLab