Skip to content

Commit 7cb01ba

Browse files
authored
Merge commit from fork
Operator session tokens were stored verbatim in operator_sessions.token (the primary key) — the same 32-byte hex value carried in the session cookie. Anyone with read access to the database (backup, snapshot, future SQL-injection sink) could lift every active session token and hijack operator sessions with no further authentication. Persist only SHA-256(token), mirroring the enrollment-token fix (GHSA-ghmh-jhmj-wcmf) and operator_api_keys: - models.HashSessionToken: at-rest representation of a raw token. - Migration 019 renames operator_sessions.token -> token_hash. Existing rows hold raw values that can no longer match a hashed lookup, so they become unreachable and the operator re-authenticates — acceptable for a 24h-TTL session. - The store hashes the token on insert and on every lookup (Get/GetPendingTwoFactor/Promote/Delete), so callers keep passing the raw cookie value; the raw token never touches disk. Closes GHSA-q4vm-pq3q-8wgq.
1 parent 1f1ab9a commit 7cb01ba

8 files changed

Lines changed: 177 additions & 9 deletions

File tree

internal/models/operator.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ const (
5858
// OperatorSession represents a UI session. A session in `pending_totp` state
5959
// is awaiting a second-factor verification and is not yet authenticated.
6060
type OperatorSession struct {
61+
// Token is the raw session token carried in the operator's cookie. It is
62+
// transient: the store persists only HashSessionToken(Token), never the
63+
// raw value at rest (GHSA-q4vm-pq3q-8wgq).
6164
Token string
6265
OperatorID string
6366
State SessionState

internal/models/token.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,16 @@ func HashEnrollmentToken(raw string) string {
2828
sum := sha256.Sum256([]byte(raw))
2929
return hex.EncodeToString(sum[:])
3030
}
31+
32+
// HashSessionToken produces the at-rest representation of a raw operator
33+
// session token. Closes GHSA-q4vm-pq3q-8wgq: previously the raw 32-byte hex
34+
// token was stored verbatim in operator_sessions.token and sent in the
35+
// session cookie, so anyone with read access to the DB (backup, snapshot,
36+
// future SQL-injection sink) could hijack every active operator session.
37+
//
38+
// Symmetric: same input → same hex → constant-time DB lookup. Mirrors the
39+
// enrollment-token and operator_api_keys hashing already done elsewhere.
40+
func HashSessionToken(raw string) string {
41+
sum := sha256.Sum256([]byte(raw))
42+
return hex.EncodeToString(sum[:])
43+
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package store
2+
3+
import (
4+
"context"
5+
"database/sql"
6+
"testing"
7+
8+
_ "modernc.org/sqlite"
9+
)
10+
11+
// migrations018 is the ordered up-migration set preceding 019.
12+
var migrations018 = append(append([]string{}, migrations017...),
13+
"018_host_address_network_uniqueness.up.sql",
14+
)
15+
16+
// TestMigration019_RenamesTokenToHash_AndDownRoundTrip verifies the
17+
// operator_sessions.token → token_hash rename (GHSA-q4vm-pq3q-8wgq) and the
18+
// down migration's reverse rename.
19+
func TestMigration019_RenamesTokenToHash_AndDownRoundTrip(t *testing.T) {
20+
db, err := sql.Open("sqlite", ":memory:")
21+
if err != nil {
22+
t.Fatalf("open sqlite: %v", err)
23+
}
24+
defer db.Close()
25+
26+
applyMigrationFiles(t, db, migrations018)
27+
28+
if !columnExistsInDB(t, db, "operator_sessions", "token") {
29+
t.Fatal("token column should exist before 019")
30+
}
31+
if columnExistsInDB(t, db, "operator_sessions", "token_hash") {
32+
t.Fatal("token_hash should not exist before 019")
33+
}
34+
35+
if err := execMigrationFile(db, "019_session_token_hash.up.sql"); err != nil {
36+
t.Fatalf("apply 019: %v", err)
37+
}
38+
39+
if columnExistsInDB(t, db, "operator_sessions", "token") {
40+
t.Error("token column should be gone after 019")
41+
}
42+
if !columnExistsInDB(t, db, "operator_sessions", "token_hash") {
43+
t.Error("token_hash should exist after 019")
44+
}
45+
46+
// token_hash must remain the primary key: a duplicate value is rejected.
47+
mustExec(t, db, `INSERT INTO operators (id, username, display_name, password_hash) VALUES ('op1','u1','U1','h')`)
48+
mustExec(t, db, `INSERT INTO operator_sessions (token_hash, operator_id, expires_at, created_at, state) VALUES ('h1','op1','2999-01-01','2000-01-01','authenticated')`)
49+
if _, err := db.ExecContext(context.Background(),
50+
`INSERT INTO operator_sessions (token_hash, operator_id, expires_at, created_at, state) VALUES ('h1','op1','2999-01-01','2000-01-01','authenticated')`); err == nil {
51+
t.Error("token_hash should still be the primary key after rename (duplicate accepted)")
52+
}
53+
54+
// Down round-trip: token_hash → token.
55+
if err := execMigrationFile(db, "019_session_token_hash.down.sql"); err != nil {
56+
t.Fatalf("apply 019 down: %v", err)
57+
}
58+
if !columnExistsInDB(t, db, "operator_sessions", "token") {
59+
t.Error("token column should be back after 019 down")
60+
}
61+
if columnExistsInDB(t, db, "operator_sessions", "token_hash") {
62+
t.Error("token_hash should be gone after 019 down")
63+
}
64+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-- Down migration: revert column name. The application-layer application of
2+
-- SHA-256 cannot be reversed; the column will simply contain hex hashes that
3+
-- the (old) lookup-by-raw-token logic cannot match. Operators must
4+
-- re-authenticate after downgrade.
5+
ALTER TABLE operator_sessions RENAME COLUMN token_hash TO token;
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-- GHSA-q4vm-pq3q-8wgq: store SHA-256 of operator session tokens at rest.
2+
--
3+
-- The old column held the raw 32-byte hex token verbatim — the same value
4+
-- sent in the session cookie — so anyone with read access to the DB (backup,
5+
-- snapshot, future SQL-injection sink) could hijack every active session. The
6+
-- application layer now writes SHA-256 hex and looks sessions up by that hash,
7+
-- mirroring the enrollment-token fix (016) and operator_api_keys.
8+
--
9+
-- Existing rows are kept in place: their column now nominally holds a "hash"
10+
-- but actually contains a raw token, so it can never match the hash computed
11+
-- from an incoming cookie and the session becomes unreachable. Operators simply
12+
-- re-authenticate — the same outcome as a brief restart, acceptable for a
13+
-- 24h-TTL session token.
14+
ALTER TABLE operator_sessions RENAME COLUMN token TO token_hash;

internal/store/sqlite.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ func (s *SQLiteStore) Migrate(ctx context.Context) error {
126126
"016_enrollment_token_hash.up.sql",
127127
"017_pop_nonces.up.sql",
128128
"018_host_address_network_uniqueness.up.sql",
129+
"019_session_token_hash.up.sql",
129130
}
130131

131132
// Tracking table. Created once; idempotent on subsequent starts.

internal/store/sqlite_operators.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,11 @@ func (s *SQLiteStore) CreateOperatorSession(ctx context.Context, sess *models.Op
585585
if state == "" {
586586
state = models.SessionStateAuthenticated
587587
}
588+
// GHSA-q4vm-pq3q-8wgq: persist only the SHA-256 of the token. The raw
589+
// value lives solely in the operator's cookie, never at rest.
588590
_, err := s.db.ExecContext(ctx,
589-
`INSERT INTO operator_sessions (token, operator_id, expires_at, created_at, state) VALUES (?, ?, ?, ?, ?)`,
590-
sess.Token, sess.OperatorID, sess.ExpiresAt, sess.CreatedAt, state,
591+
`INSERT INTO operator_sessions (token_hash, operator_id, expires_at, created_at, state) VALUES (?, ?, ?, ?, ?)`,
592+
models.HashSessionToken(sess.Token), sess.OperatorID, sess.ExpiresAt, sess.CreatedAt, state,
591593
)
592594
if err != nil {
593595
return fmt.Errorf("insert session: %w", err)
@@ -600,8 +602,8 @@ func (s *SQLiteStore) CreateOperatorSession(ctx context.Context, sess *models.Op
600602
// pending_totp state are NOT returned here.
601603
func (s *SQLiteStore) GetOperatorBySession(ctx context.Context, token string) (*models.Operator, error) {
602604
row := s.db.QueryRowContext(ctx,
603-
`SELECT operator_id, expires_at, state FROM operator_sessions WHERE token=?`,
604-
token,
605+
`SELECT operator_id, expires_at, state FROM operator_sessions WHERE token_hash=?`,
606+
models.HashSessionToken(token),
605607
)
606608
var (
607609
operatorID string
@@ -639,8 +641,8 @@ func (s *SQLiteStore) GetOperatorBySession(ctx context.Context, token string) (*
639641
// be active. Sessions already authenticated are not returned.
640642
func (s *SQLiteStore) GetPendingTwoFactorOperator(ctx context.Context, token string) (*models.Operator, error) {
641643
row := s.db.QueryRowContext(ctx,
642-
`SELECT operator_id, expires_at, state FROM operator_sessions WHERE token=?`,
643-
token,
644+
`SELECT operator_id, expires_at, state FROM operator_sessions WHERE token_hash=?`,
645+
models.HashSessionToken(token),
644646
)
645647
var (
646648
operatorID string
@@ -678,8 +680,8 @@ func (s *SQLiteStore) GetPendingTwoFactorOperator(ctx context.Context, token str
678680
// does not exist or is not pending.
679681
func (s *SQLiteStore) PromoteOperatorSession(ctx context.Context, token string, newExpiry time.Time) error {
680682
result, err := s.db.ExecContext(ctx,
681-
`UPDATE operator_sessions SET state=?, expires_at=? WHERE token=? AND state=?`,
682-
models.SessionStateAuthenticated, newExpiry, token, models.SessionStatePendingTOTP,
683+
`UPDATE operator_sessions SET state=?, expires_at=? WHERE token_hash=? AND state=?`,
684+
models.SessionStateAuthenticated, newExpiry, models.HashSessionToken(token), models.SessionStatePendingTOTP,
683685
)
684686
if err != nil {
685687
return fmt.Errorf("promote session: %w", err)
@@ -695,7 +697,7 @@ func (s *SQLiteStore) PromoteOperatorSession(ctx context.Context, token string,
695697
}
696698

697699
func (s *SQLiteStore) DeleteOperatorSession(ctx context.Context, token string) error {
698-
_, err := s.db.ExecContext(ctx, `DELETE FROM operator_sessions WHERE token=?`, token)
700+
_, err := s.db.ExecContext(ctx, `DELETE FROM operator_sessions WHERE token_hash=?`, models.HashSessionToken(token))
699701
if err != nil {
700702
return fmt.Errorf("delete session: %w", err)
701703
}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package store
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
"time"
8+
9+
"github.com/forgekeep/nebula-mesh/internal/models"
10+
)
11+
12+
// TestCreateOperatorSession_StoresHashNotRaw covers GHSA-q4vm-pq3q-8wgq: the
13+
// session token must be persisted as its SHA-256 hash, never as the raw value
14+
// that travels in the cookie. A DB read (backup/snapshot) must not yield a
15+
// usable session token.
16+
func TestCreateOperatorSession_StoresHashNotRaw(t *testing.T) {
17+
s := newTestStore(t)
18+
ctx := context.Background()
19+
op := newTestOperator(t, s, "dave")
20+
21+
const raw = "raw-session-token-deadbeef"
22+
if err := s.CreateOperatorSession(ctx, &models.OperatorSession{
23+
Token: raw,
24+
OperatorID: op.ID,
25+
ExpiresAt: time.Now().Add(time.Hour),
26+
}); err != nil {
27+
t.Fatalf("create session: %v", err)
28+
}
29+
30+
// At rest the column holds the hash, not the raw token.
31+
var stored string
32+
if err := s.db.QueryRowContext(ctx,
33+
`SELECT token_hash FROM operator_sessions WHERE operator_id=?`, op.ID).Scan(&stored); err != nil {
34+
t.Fatalf("read token_hash: %v", err)
35+
}
36+
if want := models.HashSessionToken(raw); stored != want {
37+
t.Errorf("stored token_hash = %q, want %q", stored, want)
38+
}
39+
if stored == raw {
40+
t.Error("raw session token persisted verbatim at rest")
41+
}
42+
43+
// No row anywhere holds the raw token value.
44+
var rawRows int
45+
if err := s.db.QueryRowContext(ctx,
46+
`SELECT COUNT(*) FROM operator_sessions WHERE token_hash=?`, raw).Scan(&rawRows); err != nil {
47+
t.Fatalf("count raw rows: %v", err)
48+
}
49+
if rawRows != 0 {
50+
t.Errorf("found %d rows storing the raw token", rawRows)
51+
}
52+
53+
// Lookup by the raw token (as carried in the cookie) still works.
54+
got, err := s.GetOperatorBySession(ctx, raw)
55+
if err != nil {
56+
t.Fatalf("lookup by raw token: %v", err)
57+
}
58+
if got.ID != op.ID {
59+
t.Errorf("session resolved to %q, want %q", got.ID, op.ID)
60+
}
61+
62+
// A wrong token does not resolve.
63+
if _, err := s.GetOperatorBySession(ctx, "not-the-token"); !errors.Is(err, ErrNotFound) {
64+
t.Errorf("wrong token err = %v, want ErrNotFound", err)
65+
}
66+
}

0 commit comments

Comments
 (0)