From 13928e9c10ef85b89410d2fd9a4a3fb3cd5f9c49 Mon Sep 17 00:00:00 2001
From: John Hood <cgull@glup.org>
Date: Fri, 25 Dec 2015 17:38:26 -0500
Subject: [PATCH] Use a secure counter for OCB's nonce.  Protect nonce in
 Network::Packet.

---
 src/crypto/crypto.cc   | 10 ++++++++++
 src/crypto/crypto.h    |  7 +++++++
 src/network/network.cc | 19 ++++++-------------
 src/network/network.h  |  9 ++++-----
 4 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/src/crypto/crypto.cc b/src/crypto/crypto.cc
index a8c035a..d6c27aa 100644
--- a/src/crypto/crypto.cc
+++ b/src/crypto/crypto.cc
@@ -61,6 +61,16 @@ long int myatoi( const char *str )
   return ret;
 }
 
+uint64_t Crypto::unique( void )
+{
+  static uint64_t counter = 0;
+  uint64_t rv = counter++;
+  if ( counter == 0 ) {
+    throw CryptoException( "Counter wrapped", true );
+  }
+  return rv;
+}
+
 AlignedBuffer::AlignedBuffer( size_t len, const char *data )
   : m_len( len ), m_allocated( NULL ), m_data( NULL )
 {
diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h
index da3b49b..472850a 100644
--- a/src/crypto/crypto.h
+++ b/src/crypto/crypto.h
@@ -58,6 +58,13 @@ namespace Crypto {
     ~CryptoException() throw () {}
   };
 
+  /*
+   * OCB (and other algorithms) require a source of nonce/sequence
+   * numbers that never repeats its output.  Enforce that with this
+   * function.
+   */
+  uint64_t unique( void );
+
   /* 16-byte-aligned buffer, with length. */
   class AlignedBuffer {
   private:
diff --git a/src/network/network.cc b/src/network/network.cc
index cb23d48..e920a5d 100644
--- a/src/network/network.cc
+++ b/src/network/network.cc
@@ -67,21 +67,16 @@ const uint64_t DIRECTION_MASK = uint64_t(1) << 63;
 const uint64_t SEQUENCE_MASK = uint64_t(-1) ^ DIRECTION_MASK;
 
 /* Read in packet from coded string */
-Packet::Packet( string coded_packet, Session *session )
-  : seq( -1 ),
-    direction( TO_SERVER ),
+Packet::Packet( const Message & message )
+  : seq( message.nonce.val() & SEQUENCE_MASK ),
+    direction( (message.nonce.val() & DIRECTION_MASK) ? TO_CLIENT : TO_SERVER ),
     timestamp( -1 ),
     timestamp_reply( -1 ),
     payload()
 {
-  Message message = session->decrypt( coded_packet );
-
-  direction = (message.nonce.val() & DIRECTION_MASK) ? TO_CLIENT : TO_SERVER;
-  seq = message.nonce.val() & SEQUENCE_MASK;
-
   dos_assert( message.text.size() >= 2 * sizeof( uint16_t ) );
 
-  uint16_t *data = (uint16_t *)message.text.data();
+  const uint16_t *data = (uint16_t *)message.text.data();
   timestamp = be16toh( data[ 0 ] );
   timestamp_reply = be16toh( data[ 1 ] );
 
@@ -114,7 +109,7 @@ Packet Connection::new_packet( string &s_payload )
     saved_timestamp_received_at = 0;
   }
 
-  Packet p( next_seq++, direction, timestamp16(), outgoing_timestamp_reply, s_payload );
+  Packet p( direction, timestamp16(), outgoing_timestamp_reply, s_payload );
 
   return p;
 }
@@ -244,7 +239,6 @@ Connection::Connection( const char *desired_ip, const char *desired_port ) /* se
     key(),
     session( key ),
     direction( TO_CLIENT ),
-    next_seq( 0 ),
     saved_timestamp( -1 ),
     saved_timestamp_received_at( 0 ),
     expected_receiver_seq( 0 ),
@@ -365,7 +359,6 @@ Connection::Connection( const char *key_str, const char *ip, const char *port )
     key( key_str ),
     session( key ),
     direction( TO_SERVER ),
-    next_seq( 0 ),
     saved_timestamp( -1 ),
     saved_timestamp_received_at( 0 ),
     expected_receiver_seq( 0 ),
@@ -520,7 +513,7 @@ string Connection::recv_one( int sock_to_recv, bool nonblocking )
     }
   }
 
-  Packet p( string( msg_payload, received_len ), &session );
+  Packet p( session.decrypt( string( msg_payload, received_len ) ) );
 
   dos_assert( p.direction == (server ? TO_SERVER : TO_CLIENT) ); /* prevent malicious playback to sender */
 
diff --git a/src/network/network.h b/src/network/network.h
index 23022ac..677d0d0 100644
--- a/src/network/network.h
+++ b/src/network/network.h
@@ -76,18 +76,18 @@ namespace Network {
 
   class Packet {
   public:
-    uint64_t seq;
+    const uint64_t seq;
     Direction direction;
     uint16_t timestamp, timestamp_reply;
     string payload;
     
-    Packet( uint64_t s_seq, Direction s_direction,
+    Packet( Direction s_direction,
 	    uint16_t s_timestamp, uint16_t s_timestamp_reply, string s_payload )
-      : seq( s_seq ), direction( s_direction ),
+      : seq( Crypto::unique() ), direction( s_direction ),
 	timestamp( s_timestamp ), timestamp_reply( s_timestamp_reply ), payload( s_payload )
     {}
     
-    Packet( string coded_packet, Session *session );
+    Packet( const Message & message );
     
     string tostring( Session *session );
   };
@@ -173,7 +173,6 @@ namespace Network {
     void setup( void );
 
     Direction direction;
-    uint64_t next_seq;
     uint16_t saved_timestamp;
     uint64_t saved_timestamp_received_at;
     uint64_t expected_receiver_seq;