From 8986640a98fcf9995962f0bfc19bb3cfa7bdafbf Mon Sep 17 00:00:00 2001
From: Fabrice Le Fessant <fabrice.le_fessant@ocamlpro.com>
Date: Fri, 18 May 2018 16:57:43 +0200
Subject: [PATCH] P2p: make nonces unpredictable when connecting

Avoid replay-attacks by preventing a node from determining one of the
nonces used in the encryption of a channel between two nodes.
---
 src/lib_crypto/crypto_box.ml    | 17 +++++++++++++++++
 src/lib_crypto/crypto_box.mli   | 10 ++++++++++
 src/lib_p2p/p2p_socket.ml       | 34 ++++++++++++++++++---------------
 vendors/ocaml-hacl/src/hacl.ml  |  9 +++++++++
 vendors/ocaml-hacl/src/hacl.mli |  2 ++
 5 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/src/lib_crypto/crypto_box.ml b/src/lib_crypto/crypto_box.ml
index 221ba7910..8b7062996 100644
--- a/src/lib_crypto/crypto_box.ml
+++ b/src/lib_crypto/crypto_box.ml
@@ -64,6 +64,23 @@ let random_keypair () =
 let zero_nonce = MBytes.make Nonce.bytes '\x00'
 let random_nonce = Nonce.gen
 let increment_nonce = Nonce.increment
+let generate_nonce mbytes =
+  let hash = Blake2B.hash_bytes mbytes in
+  Nonce.of_bytes_exn @@ (Bigstring.sub (Blake2B.to_bytes hash) 0 Nonce.bytes)
+
+let init_to_resp_seed = MBytes.of_string "Init -> Resp"
+let resp_to_init_seed  = MBytes.of_string "Resp -> Init"
+let generate_nonces ~incoming ~sent_msg ~recv_msg =
+  let (init_msg, resp_msg, false)
+    | (resp_msg, init_msg, true) = (sent_msg, recv_msg, incoming) in
+  let nonce_init_to_resp =
+    generate_nonce [ init_msg ; resp_msg ; init_to_resp_seed ] in
+  let nonce_resp_to_init =
+    generate_nonce [ init_msg ; resp_msg ; resp_to_init_seed ] in
+  if incoming then
+    (nonce_init_to_resp, nonce_resp_to_init)
+  else
+    (nonce_resp_to_init, nonce_init_to_resp)
 
 let precompute sk pk = Box.dh pk sk
 
diff --git a/src/lib_crypto/crypto_box.mli b/src/lib_crypto/crypto_box.mli
index 0ae438194..506fe83c6 100644
--- a/src/lib_crypto/crypto_box.mli
+++ b/src/lib_crypto/crypto_box.mli
@@ -16,6 +16,16 @@ val zero_nonce : nonce
 val random_nonce : unit -> nonce
 val increment_nonce : ?step:int -> nonce -> nonce
 
+(** [generate_nonces ~incoming ~sent_msg ~recv_msg] generates two
+    nonces by hashing (Blake2B) the arguments. The nonces should be
+    used to initialize the encryption on the communication
+    channels. Because an attacker cannot control both messages,
+    it cannot determine the nonces that will be used to encrypt
+    the messages. The sent message should contains a random nonce,
+    and we should never send the exact same message twice. *)
+val generate_nonces :
+  incoming:bool -> sent_msg:MBytes.t -> recv_msg:MBytes.t -> nonce * nonce
+
 module Secretbox : sig
   type key
 
diff --git a/src/lib_p2p/p2p_socket.ml b/src/lib_p2p/p2p_socket.ml
index 6d40f8fc6..a03c11bd8 100644
--- a/src/lib_p2p/p2p_socket.ml
+++ b/src/lib_p2p/p2p_socket.ml
@@ -125,23 +125,28 @@ module Connection_message = struct
     | Some last ->
         fail_unless (last = len) P2p_errors.Encoding_error >>=? fun () ->
         MBytes.set_int16 buf 0 encoded_message_len ;
-        P2p_io_scheduler.write fd buf
+        P2p_io_scheduler.write fd buf >>=? fun () ->
+        (* We return the raw message as it is used later to compute
+           the nonces *)
+        return buf
 
   let read fd =
     let header_buf = MBytes.create Crypto.header_length in
     P2p_io_scheduler.read_full
       ~len:Crypto.header_length fd header_buf >>=? fun () ->
     let len = MBytes.get_uint16 header_buf 0 in
-    let buf = MBytes.create len in
-    P2p_io_scheduler.read_full ~len fd buf >>=? fun () ->
-    match Data_encoding.Binary.read encoding buf 0 len with
+    let pos = Crypto.header_length in
+    let buf = MBytes.create (pos + len) in
+    MBytes.set_int16 buf 0 len ;
+    P2p_io_scheduler.read_full ~len ~pos fd buf >>=? fun () ->
+    match Data_encoding.Binary.read encoding buf pos len with
     | None ->
         fail P2p_errors.Decoding_error
-    | Some (read_len, message) ->
-        if read_len <> len then
+    | Some (next_pos, message) ->
+        if next_pos <> pos+len then
           fail P2p_errors.Decoding_error
         else
-          return message
+          return (message, buf)
 
 end
 
@@ -176,15 +181,15 @@ let authenticate
     ~proof_of_work_target
     ~incoming fd (remote_addr, remote_socket_port as point)
     ?listening_port identity supported_versions =
-  let local_nonce = Crypto_box.random_nonce () in
+  let local_nonce_seed = Crypto_box.random_nonce () in
   lwt_debug "Sending authenfication to %a" P2p_point.Id.pp point >>= fun () ->
   Connection_message.write fd
     { public_key = identity.P2p_identity.public_key ;
       proof_of_work_stamp = identity.proof_of_work_stamp ;
-      message_nonce = local_nonce ;
+      message_nonce = local_nonce_seed ;
       port = listening_port ;
-      versions = supported_versions } >>=? fun () ->
-  Connection_message.read fd >>=? fun msg ->
+      versions = supported_versions } >>=? fun sent_msg ->
+  Connection_message.read fd >>=? fun (msg, recv_msg) ->
   let remote_listening_port =
     if incoming then msg.port else Some remote_socket_port in
   let id_point = remote_addr, remote_listening_port in
@@ -198,13 +203,13 @@ let authenticate
     (P2p_errors.Not_enough_proof_of_work remote_peer_id) >>=? fun () ->
   let channel_key =
     Crypto_box.precompute identity.P2p_identity.secret_key msg.public_key in
+  let (local_nonce, remote_nonce) =
+    Crypto_box.generate_nonces ~incoming ~sent_msg ~recv_msg in
+  let cryptobox_data = { Crypto.channel_key ; local_nonce ; remote_nonce } in
   let info =
     { P2p_connection.Info.peer_id = remote_peer_id ;
       versions = msg.versions ; incoming ;
       id_point ; remote_socket_port ;} in
-  let cryptobox_data =
-    { Crypto.channel_key ; local_nonce ;
-      remote_nonce = msg.message_nonce } in
   return (info, (fd, info, cryptobox_data))
 
 type connection = {
@@ -552,4 +557,3 @@ let close ?(wait = false) st =
   Writer.shutdown st.writer >>= fun () ->
   P2p_io_scheduler.close st.conn.fd >>= fun _ ->
   Lwt.return_unit
-
diff --git a/vendors/ocaml-hacl/src/hacl.ml b/vendors/ocaml-hacl/src/hacl.ml
index 628367a5f..66a84649c 100644
--- a/vendors/ocaml-hacl/src/hacl.ml
+++ b/vendors/ocaml-hacl/src/hacl.ml
@@ -166,6 +166,15 @@ module Nonce = struct
     Bigstring.blit nonce 0 new_nonce 0 24 ;
     incr_byte new_nonce step 22 ;
     new_nonce
+
+  let of_bytes buf =
+    if Bigstring.length buf <> bytes then None else Some buf
+
+  let of_bytes_exn buf =
+    match of_bytes buf with
+    | Some s -> s
+    | None -> invalid_arg "Hacl.Nonce.of_bytes_exn: invalid length"
+
 end
 
 module Secretbox = struct
diff --git a/vendors/ocaml-hacl/src/hacl.mli b/vendors/ocaml-hacl/src/hacl.mli
index 84f2f361c..cfb6d354c 100644
--- a/vendors/ocaml-hacl/src/hacl.mli
+++ b/vendors/ocaml-hacl/src/hacl.mli
@@ -71,6 +71,8 @@ module Nonce : sig
   val bytes : int
   val gen : unit -> t
   val increment : ?step:int -> t -> t
+  val of_bytes : Bigstring.t -> t option
+  val of_bytes_exn : Bigstring.t -> t
 end
 
 module Secretbox : sig