fix rotate the session on sign-in

Invalidate the session before binding it to the trainee, so a session identifier that existed before authentication never survives into the authenticated session. Without this, an attacker who planted a session cookie before the victim signed in would inherit the login (session fixation). Sign-out already invalidated; sign-in now does too.

Commit
352e9ca2443afa52180d71bbaed5a0c674a044b3
Author
Marius Peter <dev@marius-peter.com>
Author date
Committer
Marius Peter <dev@marius-peter.com>
Committer date
Changed files
lib/web/handlers_auth.ml
index 2ae80049..ad4d8b4b 100644..100644
@@ -20,7 +20,12 @@
20 20 | Some _ -> redirect_to request Routes.home
21 21 | None -> html (Pages.register request ())
22 22
23 Added: (* Rotate the session before binding it to the trainee. A session identifier
24 Added: that existed before authentication must not survive into the authenticated
25 Added: session, or an attacker who planted that identifier would inherit the
26 Added: login (session fixation). Invalidating first issues a fresh identifier. *)
23 27 let establish request (trainee : Trainee.t) =
28 Added: Dream.invalidate_session request >>= fun () ->
24 29 Dream.set_session_field request session_key
25 30 (Trainee.id_to_string trainee.id)
26 31 >>= fun () -> redirect_to request Routes.home
test/web/auth.ml
index 21fdda3a..e489b786 100644..100644
@@ -17,6 +17,47 @@
17 17 fun () ->
18 18 let c = client () in
19 19 Alcotest.(check int) "GET /login" 200 (status (get c "/login")) );
20 Added: ( "signing in rotates the session identifier",
21 Added: `Quick,
22 Added: fun () ->
23 Added: (* A session identifier that existed before authentication must not
24 Added: survive into the authenticated session. Otherwise an attacker
25 Added: who planted the identifier would inherit the login (session
26 Added: fixation). Read the pre-login cookie, sign in, and confirm the
27 Added: cookie the server now sets is different. *)
28 Added: let session_cookie c =
29 Added: List.find_opt
30 Added: (fun cookie ->
31 Added: String.length cookie >= 13
32 Added: && String.sub cookie 0 13 = "dream.session")
33 Added: c.jar
34 Added: in
35 Added: let c = client () in
36 Added: let _ = register c ~username:"lifter" ~password:"heavyduty1" in
37 Added: (* Sign out, then obtain a fresh anonymous session cookie. *)
38 Added: let token = Option.get (csrf_token (body (get c "/"))) in
39 Added: let _ = post c "/logout" [ ("dream.csrf", token) ] in
40 Added: let _ = get c "/login" in
41 Added: let before = session_cookie c in
42 Added: Alcotest.(check bool)
43 Added: "an anonymous session cookie exists before sign-in" true
44 Added: (Option.is_some before);
45 Added: let token = Option.get (csrf_token (body (get c "/login"))) in
46 Added: let _ =
47 Added: post c "/login"
48 Added: [
49 Added: ("dream.csrf", token);
50 Added: ("username", "lifter");
51 Added: ("password", "heavyduty1");
52 Added: ]
53 Added: in
54 Added: let after = session_cookie c in
55 Added: Alcotest.(check bool)
56 Added: "a session cookie exists after sign-in" true
57 Added: (Option.is_some after);
58 Added: Alcotest.(check bool)
59 Added: "the session identifier changed on sign-in" true (before <> after)
60 Added: );
20 61 ( "every page carries the copyright and deployment version banner",
21 62 `Quick,
22 63 fun () ->