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

Don't open /dev/urandom on startup #6077

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ocaml/forkexecd/test/fe_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ let slave = function
(*
Printf.fprintf stderr "%s %d\n" total_fds (List.length present - 1)
*)
if total_fds + 1 (* Uuid.dev_urandom *) <> List.length filtered then
if total_fds <> List.length filtered then
fail "Expected %d fds; /proc/self/fd has %d: %s" total_fds
(List.length filtered) ls

Expand Down
3 changes: 3 additions & 0 deletions ocaml/libs/uuid/dune
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
(public_name uuid)
(modules uuidx)
(libraries
cstruct
mirage-crypto-rng
mirage-crypto-rng.unix
mtime
mtime.clock.os
ptime
Expand Down
68 changes: 34 additions & 34 deletions ocaml/libs/uuid/uuidx.ml
Original file line number Diff line number Diff line change
Expand Up @@ -116,39 +116,38 @@ let is_uuid str = match of_string str with None -> false | Some _ -> true

let dev_urandom = "/dev/urandom"

let dev_urandom_fd = Unix.openfile dev_urandom [Unix.O_RDONLY] 0o640
(* we can't close this in at_exit, because Crowbar runs at_exit, and
it'll fail because this FD will then be closed
*)

let read_bytes dev n =
let buf = Bytes.create n in
let read = Unix.read dev buf 0 n in
if read <> n then
raise End_of_file
else
Bytes.to_string buf

let make_uuid_urnd () = of_bytes (read_bytes dev_urandom_fd 16) |> Option.get

(* State for random number generation. Random.State.t isn't thread safe, so
only use this via with_non_csprng_state, which takes care of this.
*)
let rstate = Random.State.make_self_init ()

let rstate_m = Mutex.create ()

let with_non_csprng_state =
(* On OCaml 5 we could use Random.State.split instead,
and on OCaml 4 the mutex may not be strictly needed
*)
let finally () = Mutex.unlock rstate_m in
fun f ->
Mutex.lock rstate_m ;
Fun.protect ~finally (f rstate)

(** Use non-CSPRNG by default, for CSPRNG see {!val:make_uuid_urnd} *)
let make_uuid_fast () = with_non_csprng_state Uuidm.v4_gen
let fd = Unix.openfile dev [Unix.O_RDONLY] 0o640 in
let finally body_f clean_f =
try
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't take finally from a library or use Fun.protect?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a reformat.

let ret = body_f () in
clean_f () ; ret
with e -> clean_f () ; raise e
in
finally
(fun () ->
let buf = Bytes.create n in
let read = Unix.read fd buf 0 n in
if read <> n then
raise End_of_file
else
Bytes.to_string buf
)
(fun () -> Unix.close fd)

let () = Mirage_crypto_rng_unix.initialize (module Mirage_crypto_rng.Fortuna)

let csprng_urandom = read_bytes dev_urandom

let csprng_fortuna n = Mirage_crypto_rng.generate n |> Cstruct.to_string

let make_uuid csprng = of_bytes (csprng 16) |> Option.get

let make_uuid_urnd () = make_uuid csprng_urandom

let make_uuid_fortuna () = make_uuid csprng_fortuna

let make_uuid_fast = make_uuid_fortuna

let make_default = ref make_uuid_urnd

Expand All @@ -157,7 +156,8 @@ let make () = !make_default ()
let make_v7_uuid_from_parts time_ns rand_b = Uuidm.v7_ns ~time_ns ~rand_b

let rand64 () =
with_non_csprng_state (fun rstate () -> Random.State.bits64 rstate)
let b = csprng_fortuna 8 in
String.get_int64_ne b 0

let now_ns =
let start = Mtime_clock.counter () in
Expand All @@ -174,7 +174,7 @@ let make_v7_uuid () = make_v7_uuid_from_parts (now_ns ()) (rand64 ())
type cookie = string

let make_cookie () =
read_bytes dev_urandom_fd 64
read_bytes dev_urandom 64
|> String.to_seq
|> Seq.map (fun c -> Printf.sprintf "%1x" (int_of_char c))
|> List.of_seq
Expand Down
6 changes: 3 additions & 3 deletions ocaml/xapi/xapi_globs.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1612,11 +1612,11 @@ let other_options =
, (fun () -> string_of_bool !disable_webserver)
, "Disable the host webserver"
)
; ( "use-prng-uuid-gen"
(* eventually this'll be the default, except for Sessions *)
; ( "use-fortuna-uuid-gen"
(* eventually this'll be the default *)
, Arg.Unit (fun () -> Uuidx.make_default := Uuidx.make_uuid_fast)
, (fun () -> !Uuidx.make_default == Uuidx.make_uuid_fast |> string_of_bool)
, "Use PRNG based UUID generator instead of CSPRNG"
, "Use Fortuna based UUID generator instead of /dev/urandom"
)
]

Expand Down
Loading