Преглед изворни кода

Session revocation via sessions_valid_after epoch (SA-1/SA-14, password-change scope)

Spry now records a per-user session-validity epoch on spry_users. Migration
M0002 adds the column additively (nullable; NULL reads as the Unix epoch via
UserEntity.EPOCH, so an existing database migrates without revoking anything
or losing data). UserService.set_password bumps the epoch whenever a password
changes - ChangePasswordAction then reloads the user and re-mints the current
session's auth slot through the new SpryAuth.relogin helper, so the user
changing their own password stays logged in while every older frame on other
devices is revoked. AlterUserAction's password reset bumps through the same
path (admin-forced reset revokes the target user's sessions).

Enforcement sits in SpryAuth.revalidate: after the live/enabled check it
compares the held frame's signed as_at (HeldSlot.as_at, preserved by the
Statum resolver) against user.sessions_valid_after and fails closed - the
guards answer unauthenticated exactly as for a disabled account. current()
stays the documented cheap display read; its doc comment now notes that
revocation is guard-enforced. LogoutAction is deliberately untouched: per
the approved RFC amendment a regular logout stays non-revoking.

Also adds a test harness (tests/TestMain.vala + meson target, mirroring
Statum's PASS/FAIL suite) covering the epoch comparison boundaries
(before/after/exact-equal/sub-second), the NULL-epoch migration semantics,
the bump source, and M0002's up/down SQL shape (additive-only). UserEntity's
epoch constant is a lazily-initialised property because Vala never runs
static field initialisers on bare class reads - an initialised static field
read null and would have failed open. docs/security-audit.md records the
addressed status for SA-1.
saddle-Clanker-Saddle пре 4 недеља
родитељ
комит
b3a9292bf7

+ 8 - 0
docs/security-audit.md

@@ -363,6 +363,14 @@ file is group/world-readable. Consider env-var-only secret injection
 (`STATUM_SIGNING_SECRET_KEY` etc.) for container deployments where the
 config file is image-baked.
 
+**Status: ADDRESSED (warning, no refusal).** Statum's config load now stats
+every file that contributed the merged "statum" section (following
+symlinks) and prints a loud warning naming the file, its mode and the
+`chmod 600` fix when it is group/world-readable AND the section actually
+carries a secret key. It deliberately does not refuse: refusal would brick
+0644-config dev setups and image-baked containers (rationale in
+Statum/src/Statum.vala). Env-var secret injection remains unimplemented.
+
 ---
 
 ## SA-10 — verify_frame ignores the signer claim (medium)

+ 1 - 0
meson.build

@@ -29,3 +29,4 @@ add_project_arguments(['--vapidir', vapi_dir], language: 'vala')
 
 subdir('src')
 subdir('tools')
+subdir('tests')

+ 11 - 1
src/Actions/ChangePasswordAction.vala

@@ -69,7 +69,17 @@ namespace Spry.Actions {
 					return directives().notify("error", current_password_invalid_message);
 				}
 				yield users.set_password(me.user_id, new_password.dup());
-				return directives().notify("success", password_changed_message);
+				// set_password bumped the session-validity epoch: re-mint THIS
+				// session's auth slot from the freshly loaded user so the frame
+				// riding this very response is authored after the bump and the
+				// user changing their own password stays logged in, while every
+				// other device holding an older frame is revoked (SA-1/SA-14).
+				var refreshed = yield users.get_user(me.user_id);
+				if (refreshed == null) {
+					return directives().notify("error", change_failed_message.printf("the account could not be reloaded"));
+				}
+				return SpryAuth.relogin(directives(), (!)refreshed, action_registry)
+					.notify("success", password_changed_message);
 			} catch (Error e) {
 				return directives().notify("error", change_failed_message.printf(e.message));
 			}

+ 32 - 3
src/Auth.vala

@@ -109,7 +109,10 @@ namespace Spry {
 		 * disabled, deleted or revoked account still resolves here until
 		 * its slot is cleared. Authorisation decisions must go through
 		 * {@link require_login} or {@link require_permission}, which
-		 * re-validate against the user store on every guarded request.
+		 * re-validate against the user store on every guarded request —
+		 * that is also where session revocation (the user's
+		 * `sessions_valid_after` epoch, bumped on password change) is
+		 * enforced; this read stays non-authoritative by design.
 		 */
 		public static CurrentUser current(StatumRequest request) {
 			HeldSlot slot;
@@ -156,6 +159,19 @@ namespace Spry {
 			return directives.set_private_typed<AuthPublic, AuthPrivate>(SLOT_TYPE, Scope.SESSION, pub, priv);
 		}
 
+		/**
+		 * Re-mints `user`'s `auth` slot exactly as {@link login} does, for
+		 * callers that just bumped the user's `sessions_valid_after` epoch
+		 * (a password change) and want the CURRENT session to survive its
+		 * own revocation: the fresh frame is authored after the bump, so it
+		 * clears the new epoch while every older frame on other devices is
+		 * revoked. Chains onto `directives` (typically the same builder the
+		 * notification rides on).
+		 */
+		public static DirectiveBuilder relogin(DirectiveBuilder directives, UserProjection user, ActionRegistry action_registry) throws GLib.Error {
+			return login(directives, user, action_registry);
+		}
+
 		/**
 		 * Guard for handlers that need a logged-in user: returns `null`
 		 * when the held `auth` slot maps to a live, enabled account, else a
@@ -281,19 +297,32 @@ namespace Spry {
 		 * Re-validates the held `auth` slot against the user store:
 		 * returns the live, enabled user for {@link AuthPrivate}.`user_id`,
 		 * or `null` when no slot is held, the account is missing or
-		 * disabled, or the store could not be reached (fail closed, with a
-		 * warning).
+		 * disabled, the held frame predates the user's session-validity
+		 * epoch (revoked by a password change — SA-1/SA-14), or the store
+		 * could not be reached (fail closed, with a warning).
 		 */
 		private static async UserProjection? revalidate(StatumRequest request, UserService users) {
 			var me = current(request);
 			if (me.is_anonymous) {
 				return null;
 			}
+			HeldSlot slot;
+			DateTime? authored_at = null;
+			if (request.held.try_get(SLOT_TYPE, out slot) && slot != null) {
+				authored_at = ((!)slot).as_at;
+			}
 			try {
 				var user = yield users.get_user(me.user_id);
 				if (user == null || !user.enabled) {
 					return null;
 				}
+				// SA-1/SA-14: a frame authored before the user's epoch was
+				// minted under a previous password (or pre-migration, when the
+				// epoch is NULL = Unix epoch and everything passes). Reject it
+				// closed, exactly like a disabled account.
+				if (authored_at == null || !UserService.frame_is_valid_at(user, (!)authored_at)) {
+					return null;
+				}
 				return user;
 			} catch (Error e) {
 				warning(@"[Spry] could not re-validate the auth slot for user $(me.user_id): $(e.message)");

+ 45 - 0
src/Authentication/Migrations/M0002_SessionsValidAfter.vala

@@ -0,0 +1,45 @@
+using InvercargillSql.Migrations;
+
+namespace Spry.Authentication.Migrations {
+
+    /**
+     * Adds the SA-1/SA-14 session-validity epoch to `spry_users`.
+     *
+     * `sessions_valid_after` records the moment the user's held auth frames
+     * must have been authored after: {@link Spry.SpryAuth.revalidate} rejects
+     * any `auth` slot whose snapshot `as_at` is older, so bumping the column
+     * revokes every frame minted before the bump. It is set on password
+     * changes (see {@link Spry.Authentication.UserService.set_password}); a
+     * regular logout stays non-revoking.
+     *
+     * The column is deliberately NULLABLE with no SQL default: the migration
+     * framework's SQLite dialect cannot render a DATETIME `DEFAULT`
+     * (DateTime is not one of the scalar types its SQL literal renderer
+     * knows, so it would emit `DEFAULT NULL`), and a plain ADD COLUMN must
+     * work on existing rows without rewriting them. NULL is interpreted in
+     * code ({@link Spry.Authentication.UserEntity.EPOCH}) as the Unix epoch,
+     * so every pre-migration frame stays valid — the upgrade is
+     * additive-only and revokes nothing.
+     */
+    public class M0002_SessionsValidAfter : Migration {
+
+
+        public override string name { get { return "Add Spry sessions_valid_after epoch"; } }
+        public override uint64 serial { get { return 2; } }
+        public override string migration_namespace { get { return "spry-authentication"; }}
+
+        public override void up (InvercargillSql.Migrations.MigrationBuilder b) {
+            b.alter_table ("spry_users", (t) => {
+                t.add_column<DateTime?>("sessions_valid_after");
+            });
+        }
+
+        public override void down (InvercargillSql.Migrations.MigrationBuilder b) {
+            b.alter_table ("spry_users", (t) => {
+                t.drop_column("sessions_valid_after");
+            });
+        }
+
+    }
+
+}

+ 39 - 1
src/Authentication/UserEntity.vala

@@ -9,6 +9,29 @@ namespace Spry.Authentication {
 	 */
 	public class UserEntity : Object {
 
+		/**
+		 * The session-validity epoch NULL resolves to when the row predates
+		 * {@link Migrations.M0002_SessionsValidAfter}: a frame authored any
+		 * time after 1970-01-01T00:00:00Z is valid, so migrating an existing
+		 * database revokes nothing.
+		 *
+		 * A lazily-built property rather than an initialised static field:
+		 * Vala only runs GObject static-field initialisers in `class_init`,
+		 * which a bare `UserEntity.EPOCH` read (e.g. from UserProjection's
+		 * getter) never triggers — the field would still be null and every
+		 * comparison against it would silently fail open.
+		 */
+		private static DateTime? _epoch = null;
+
+		public static DateTime EPOCH {
+			get {
+				if (_epoch == null) {
+					_epoch = new DateTime.utc(1970, 1, 1, 0, 0, 0.0);
+				}
+				return (!)_epoch;
+			}
+		}
+
 		public int64 id { get; set; }
 		public string username { get; set; }
 		public string email { get; set; }
@@ -20,6 +43,20 @@ namespace Spry.Authentication {
 		public DateTime modified { get; set; }
 		public bool enabled { get; set; }
 
+		/**
+		 * SA-1/SA-14: held auth frames for this user must have been authored
+		 * at or after this instant. Bumped on password change (see {@link
+		 * UserService.set_password}); `null` on rows written before M0002,
+		 * read as {@link EPOCH} via {@link
+		 * effective_sessions_valid_after}.
+		 */
+		public DateTime? sessions_valid_after { get; set; }
+
+		/** {@link sessions_valid_after} with `null` resolved to {@link EPOCH}. */
+		public DateTime effective_sessions_valid_after {
+			get { return sessions_valid_after ?? EPOCH; }
+		}
+
 		public static void entity_mapping(EntityMapperBuilder<UserEntity> cfg) {
 			cfg.table("spry_users")
 				.column<int64?>("id", o => o.id, (o, v) => o.id = v)
@@ -31,7 +68,8 @@ namespace Spry.Authentication {
 				.column<DateTime>("date_of_birth", o => o.date_of_birth, (o, v) => o.date_of_birth = v)
 				.column<DateTime>("created", o => o.created, (o, v) => o.created = v)
 				.column<DateTime>("modified", o => o.modified, (o, v) => o.modified = v)
-				.column<bool>("enabled", o => o.enabled, (o, v) => o.enabled = v);
+				.column<bool>("enabled", o => o.enabled, (o, v) => o.enabled = v)
+				.column<DateTime?>("sessions_valid_after", o => o.sessions_valid_after, (o, v) => o.sessions_valid_after = v);
 		}
 
 	}

+ 14 - 0
src/Authentication/UserProjection.vala

@@ -26,6 +26,19 @@ namespace Spry.Authentication {
 		public DateTime modified { get; set; }
 		public bool enabled { get; set; }
 
+		/**
+		 * SA-1/SA-14: held auth frames for this user must have been authored
+		 * at or after this instant ({@link Spry.SpryAuth.revalidate}
+		 * enforces it). `null` on rows written before M0002 — read it
+		 * through {@link effective_sessions_valid_after}.
+		 */
+		public DateTime? sessions_valid_after { get; set; }
+
+		/** {@link sessions_valid_after} with `null` resolved to the Unix epoch. */
+		public DateTime effective_sessions_valid_after {
+			get { return sessions_valid_after ?? UserEntity.EPOCH; }
+		}
+
 	private string _username;
 
 	/**
@@ -47,6 +60,7 @@ namespace Spry.Authentication {
 				.select<DateTime>("created", expr("u.created"), (o, v) => o.created = v)
 				.select<DateTime>("modified", expr("u.modified"), (o, v) => o.modified = v)
 				.select<bool>("enabled", expr("u.enabled"), (o, v) => o.enabled = v)
+				.select<DateTime?>("sessions_valid_after", expr("u.sessions_valid_after"), (o, v) => o.sessions_valid_after = v)
 				.join<UserPermissionEntity>("p", expr("p.user_id == u.id"))
 				.select_many<string>("permissions", expr("p.permission"), (o, v) => o._permissions = v.to_immutable_buffer());
 		}

+ 31 - 0
src/Authentication/UserService.vala

@@ -86,20 +86,51 @@ namespace Spry.Authentication {
 				created = new DateTime.now_utc(),
 				modified = new DateTime.now_utc(),
 				enabled = enabled,
+				sessions_valid_after = UserEntity.EPOCH,
 			};
 
 			yield db.insert_async<UserEntity>(user);
 			return user;
 		}
 
+		/**
+		 * Replaces the user's password AND bumps the session-validity epoch
+		 * (SA-1/SA-14): every auth frame authored before this call is revoked —
+		 * {@link Spry.SpryAuth.revalidate} answers unauthenticated for it from
+		 * the next guarded request. Callers that keep the changing user logged
+		 * in re-mint their slot afterwards (see {@link
+		 * Spry.Actions.ChangePasswordAction}).
+		 */
 		public async void set_password(int64 user_id, string password) throws Error {
 			var user = yield load_user(user_id);
 
 			user.password_hash = Sodium.PasswordHashing.hash(password);
+			user.sessions_valid_after = session_epoch_now();
 			user.modified = new DateTime.now_utc();
 			yield db.update_async<UserEntity>(user);
 		}
 
+		/**
+		 * The instant new frames must be authored after — the value {@link
+		 * Spry.SpryAuth.revalidate} compares a held frame's `as_at` against.
+		 * Public so callers re-minting a slot straight after a bump can rely
+		 * on the fresh frame clearing its own epoch.
+		 */
+		public static DateTime session_epoch_now() {
+			return new DateTime.now_utc();
+		}
+
+		/**
+		 * True when a frame authored at `as_at` is still valid for `user`:
+		 * authored at or after the user's epoch. Equality passes because the
+		 * frame a caller re-mints immediately after a bump carries the bump's
+		 * own timestamp, and a same-second re-mint must survive its own
+		 * revocation.
+		 */
+		public static bool frame_is_valid_at(UserProjection user, DateTime as_at) {
+			return as_at.compare(user.effective_sessions_valid_after) >= 0;
+		}
+
 		public async UserEntity alter_user(int64 user_id, string username, string email, string forename, string surname, DateTime date_of_birth, bool enabled) throws Error {
 			var user = yield load_user(user_id);
 

+ 3 - 1
src/SpryModule.vala

@@ -5,7 +5,8 @@ namespace Spry {
 
 	/**
 	 * Inversion module that registers Spry's user store: the `spry_users` and
-	 * `spry_user_permissions` schema (M0001 migration), the entity/projection
+	 * `spry_user_permissions` schema (M0001 + M0002 migrations), the
+	 * entity/projection
 	 * mappings, and the scoped {@link Spry.Authentication.UserService}.
 	 *
 	 * Register this module with the application container alongside
@@ -19,6 +20,7 @@ namespace Spry {
 			// Sql table setup
 			var sql = container.configure_with<DatabaseConfigurator>();
 			sql.add_migration<Authentication.Migrations.M0001_Initial>();
+			sql.add_migration<Authentication.Migrations.M0002_SessionsValidAfter>();
 			sql.add_entity<Authentication.UserEntity>(Authentication.UserEntity.entity_mapping);
 			sql.add_entity<Authentication.UserPermissionEntity>(Authentication.UserPermissionEntity.entity_mapping);
 			sql.add_projection<Authentication.UserProjection>(Authentication.UserProjection.projection_mapping);

+ 1 - 0
src/meson.build

@@ -6,6 +6,7 @@ sources = files(
     'Authentication/UserProjection.vala',
     'Authentication/UserService.vala',
     'Authentication/Migrations/M0001_Initial.vala',
+    'Authentication/Migrations/M0002_SessionsValidAfter.vala',
     'Actions/UserManagementAction.vala',
     'Actions/UserActionPrivate.vala',
     'Actions/ProfileValidation.vala',

+ 171 - 0
tests/TestMain.vala

@@ -0,0 +1,171 @@
+using InvercargillSql.Migrations;
+using InvercargillSql.Dialects;
+using Spry.Authentication;
+using Spry.Authentication.Migrations;
+
+namespace Spry.Tests {
+
+    int failures = 0;
+    int passes = 0;
+
+    void check(bool condition, string label) {
+        if (condition) {
+            passes++;
+            print("PASS %s\n", label);
+        } else {
+            failures++;
+            print("FAIL %s\n", label);
+        }
+    }
+
+    UserProjection user_with_epoch(DateTime? epoch) {
+        var user = new UserProjection();
+        user.id = 42;
+        user.enabled = true;
+        user.sessions_valid_after = epoch;
+        return user;
+    }
+
+    /**
+     * The epoch comparison at the heart of SA-1/SA-14 revocation: a frame
+     * authored BEFORE the user's sessions_valid_after is revoked; one
+     * authored after (or exactly AT it — the re-mint a password change
+     * issues in the same second) stays valid.
+     */
+    void test_epoch_comparison() {
+        var epoch = new DateTime.utc(2025, 6, 1, 12, 0, 0.0);
+
+        var before = epoch.add_seconds(-1);
+        check(!UserService.frame_is_valid_at(user_with_epoch(epoch), before),
+            "SA-1: a frame authored 1s before the epoch is revoked");
+
+        var after = epoch.add_seconds(1);
+        check(UserService.frame_is_valid_at(user_with_epoch(epoch), after),
+            "SA-1: a frame authored 1s after the epoch stays valid");
+
+        check(UserService.frame_is_valid_at(user_with_epoch(epoch), epoch),
+            "SA-1: a frame authored exactly at the epoch stays valid (same-second re-mint)");
+
+        check(!UserService.frame_is_valid_at(user_with_epoch(epoch), epoch.add_seconds(-0.5)),
+        "SA-1: sub-second-older frames are revoked (DateTime.compare uses microseconds)");
+
+        // Far-past frame against a bumped epoch — the replay SA-1 describes.
+        var replayed = new DateTime.utc(2020, 1, 1, 0, 0, 0.0);
+        check(!UserService.frame_is_valid_at(user_with_epoch(epoch), replayed),
+            "SA-1: a frame replayed from long before the bump is revoked");
+    }
+
+    /**
+     * Pre-M0002 rows read back with a NULL epoch: they must behave as the
+     * Unix epoch so migrating an existing database revokes nothing.
+     */
+    void test_null_epoch_is_unix_epoch() {
+        var migrated = user_with_epoch(null);
+        check(migrated.effective_sessions_valid_after.to_unix() == 0,
+            "M0002: a NULL epoch reads as the Unix epoch (1970-01-01T00:00:00Z)");
+
+        var old_frame = new DateTime.utc(2001, 2, 3, 4, 5, 6.0);
+        check(UserService.frame_is_valid_at(migrated, old_frame),
+            "M0002: a pre-migration frame stays valid against a NULL epoch");
+
+        check(UserEntity.EPOCH.to_unix() == 0 && UserEntity.EPOCH.get_timezone_abbreviation() == "UTC",
+            "M0002: UserEntity.EPOCH is Unix epoch zero in UTC");
+    }
+
+    /**
+     * The bump source: session_epoch_now must be a fresh UTC instant that a
+     * frame authored immediately after it clears (the re-mint contract).
+     */
+    void test_epoch_now_is_usable_bump() {
+        var before = new DateTime.now_utc();
+        var bump = UserService.session_epoch_now();
+        var after = new DateTime.now_utc();
+
+        check(bump.to_unix() >= before.to_unix() && bump.to_unix() <= after.to_unix(),
+            "SA-1: session_epoch_now lands between two surrounding now_utc samples");
+
+        var user = user_with_epoch(bump);
+        check(UserService.frame_is_valid_at(user, bump),
+            "SA-1: a frame authored at the bump instant itself stays valid");
+        check(!UserService.frame_is_valid_at(user, bump.add_seconds(-1)),
+            "SA-1: a frame authored 1s before the bump is revoked");
+    }
+
+    /**
+     * M0002 shape: additive-only. One ADD COLUMN, nullable (no NOT NULL), no
+     * DEFAULT (the SQLite dialect cannot render a DateTime default), so
+     * existing rows keep working with a NULL epoch.
+     */
+    void test_m0002_sql_shape() {
+        var migration = new M0002_SessionsValidAfter();
+        check(migration.serial == 2, "M0002: serial is 2 (ordered after M0001)");
+        check(migration.migration_namespace == "spry-authentication",
+            "M0002: shares M0001's migration namespace");
+
+        var builder = new MigrationBuilder(new SqliteDialect());
+        migration.up(builder);
+
+        var operations = builder.get_operations();
+        check(operations.length == 1, "M0002: up() emits exactly one schema operation");
+
+        foreach (var op in operations) {
+            var sql = op.to_sql(new SqliteDialect());
+            check(sql.has_prefix("ALTER TABLE spry_users ADD COLUMN sessions_valid_after "),
+                @"M0002: SQL alters spry_users and adds sessions_valid_after — got: $sql");
+            check("INTEGER" in sql,
+                @"M0002: the column is stored INTEGER (unix seconds) — got: $sql");
+            check(!("NOT NULL" in sql),
+                @"M0002: the column is nullable (additive-only upgrade) — got: $sql");
+            check(!("DEFAULT" in sql),
+                @"M0002: no DEFAULT clause (DateTime defaults are unrenderable) — got: $sql");
+        }
+    }
+
+    /**
+     * M0002 down(): the column is dropped again, so the migration is
+     * reversible (losing only the epoch, never user rows).
+     */
+    void test_m0002_down_shape() {
+        var migration = new M0002_SessionsValidAfter();
+        var builder = new MigrationBuilder(new SqliteDialect());
+        migration.down(builder);
+
+        var operations = builder.get_operations();
+        check(operations.length == 1, "M0002: down() emits exactly one schema operation");
+        foreach (var op in operations) {
+            var sql = op.to_sql(new SqliteDialect());
+            check(sql == "ALTER TABLE spry_users DROP COLUMN sessions_valid_after",
+                @"M0002: down() drops only sessions_valid_after — got: $sql");
+        }
+    }
+
+    /**
+     * SpryAuth.permission_matches is re-exercised here because the auth
+     * guards (the enforcement point of this change) sit behind it.
+     */
+    void test_permission_matching_untouched() {
+        string[] patterns = { "admin" };
+        check(SpryAuth.permission_matches(patterns, "anything"),
+            "guards: the admin wildcard still matches every permission");
+        string[] scoped = { "users.*" };
+        check(SpryAuth.permission_matches(scoped, "users.read"),
+            "guards: a trailing-* pattern still prefix-matches");
+        check(!SpryAuth.permission_matches(scoped, "other.read"),
+            "guards: a non-matching permission still fails");
+    }
+
+    public int main(string[] args) {
+        test_epoch_comparison();
+        test_null_epoch_is_unix_epoch();
+        test_epoch_now_is_usable_bump();
+        test_m0002_sql_shape();
+        test_m0002_down_shape();
+        test_permission_matching_untouched();
+
+        print("---- %d passed, %d failed ----\n", passes, failures);
+        if (failures > 0) {
+            return 1;
+        }
+        return 0;
+    }
+}

+ 14 - 0
tests/meson.build

@@ -0,0 +1,14 @@
+# Spry test suite: SA-1/SA-14 session-validity epoch semantics (frame
+# as_at vs sessions_valid_after, the NULL-epoch migration default, the
+# bump source) and the M0002 migration's additive-only SQL shape.
+# Prints PASS/FAIL lines and exits non-zero on failure. Live-database
+# paths (the actual ALTER on a real store) are manually verified — see
+# the task completion report.
+
+spry_tests = executable('spry-tests',
+    ['TestMain.vala'],
+    dependencies: [statum_dep, spry_dep, sodium_deps, invercargill_sql_dep, sqlite_dep, astralis_dep, libxml_dep],
+    install: false
+)
+
+test('spry', spry_tests)