Complete rewrite of crypto part

Cargo.toml

@@ -10,11 +10,11 @@ docopt = "0.6"
 rustc-serialize = "0.3"
 log = "0.3"
 epoll = "0.2"
-sodiumoxide = {version = "0.0.9", optional = true}
+libsodium-sys = {version = "0.0.9", optional = true}
 
 [build-dependencies]
 gcc = "0.3"
 
 [features]
 default = []
-crypto = ["sodiumoxide"] #not default as it requires external library
+crypto = ["libsodium-sys"] #not default as it requires external library

src/crypto/dummy.rs

@@ -13,7 +13,7 @@ impl Crypto {
         0
     }
 
-    pub fn auth_bytes(&self) -> usize {
+    pub fn additional_bytes(&self) -> usize {
         0
     }
 
@@ -21,11 +21,11 @@ impl Crypto {
         panic!("This binary has no crypto support");
     }
 
-    pub fn decrypt(&self, mut _buf: &mut [u8], _nonce: &[u8], _hash: &[u8]) -> Result<(), Error> {
+    pub fn decrypt(&self, mut _buf: &mut [u8], _nonce: &[u8], _hash: &[u8]) -> Result<usize, Error> {
         unreachable!("This should never be called")
     }
 
-    pub fn encrypt(&mut self, mut _buf: &mut [u8]) -> (Vec<u8>, Vec<u8>) {
+    pub fn encrypt(&mut self, mut _buf: &mut [u8], _mlen: usize, _nonce_bytes: &mut [u8], _header: &[u8]) -> usize {
         unreachable!("This should never be called")
     }
 }

src/crypto/sodium.rs

@@ -1,19 +1,15 @@
-use std::mem;
+use std::{mem, ptr};
 
-use sodiumoxide::crypto::stream::chacha20::{Key as CryptoKey, Nonce, stream_xor_inplace, gen_nonce,
+use libsodium_sys::*;
-    KEYBYTES, NONCEBYTES};
-use sodiumoxide::crypto::auth::hmacsha512256::{Key as AuthKey, Tag, authenticate, verify, TAGBYTES};
-use sodiumoxide::crypto::pwhash::{derive_key, SALTBYTES, Salt, HASHEDPASSWORDBYTES,
-    OPSLIMIT_INTERACTIVE, MEMLIMIT_INTERACTIVE};
 
 use super::super::types::Error;
 
 pub enum Crypto {
     None,
-    ChaCha20HmacSha512256{crypto_key: CryptoKey, auth_key: AuthKey, nonce: Nonce}
+    ChaCha20Poly1305{key: [u8; 32], nonce: [u8; 8]}
 }
 
-fn inc_nonce(nonce: Nonce) -> Nonce {
+fn inc_nonce(nonce: [u8; 8]) -> [u8; 8] {
     unsafe {
         let mut num: u64 = mem::transmute(nonce);
         num = num.wrapping_add(1);
@@ -25,64 +21,101 @@ impl Crypto {
     pub fn method(&self) -> u8 {
         match self {
             &Crypto::None => 0,
-            &Crypto::ChaCha20HmacSha512256{crypto_key: _, auth_key: _, nonce: _} => 1
+            &Crypto::ChaCha20Poly1305{key: _, nonce: _} => 1
         }
     }
 
     pub fn nonce_bytes(&self) -> usize {
         match self {
             &Crypto::None => 0,
-            &Crypto::ChaCha20HmacSha512256{crypto_key: _, auth_key: _, nonce: _} => NONCEBYTES
+            &Crypto::ChaCha20Poly1305{key: _, ref nonce} => nonce.len()
         }
     }
 
-    pub fn auth_bytes(&self) -> usize {
+    pub fn additional_bytes(&self) -> usize {
         match self {
             &Crypto::None => 0,
-            &Crypto::ChaCha20HmacSha512256{crypto_key: _, auth_key: _, nonce: _} => TAGBYTES
+            &Crypto::ChaCha20Poly1305{key: _, nonce: _} => crypto_aead_chacha20poly1305_ABYTES
         }
     }
 
     pub fn from_shared_key(password: &str) -> Self {
-        let salt = "vpncloudVPNCLOUDvpncl0udVpnCloud";
+        let salt = "vpncloudVPNCLOUDvpncl0udVpnCloud".as_bytes();
-        assert_eq!(salt.len(), SALTBYTES);
+        assert_eq!(salt.len(), crypto_pwhash_scryptsalsa208sha256_SALTBYTES);
-        let mut key = [0; HASHEDPASSWORDBYTES];
+        let mut key = [0; crypto_pwhash_scryptsalsa208sha256_STRBYTES];
-        derive_key(&mut key, password.as_bytes(), &Salt::from_slice(salt.as_bytes()).unwrap(),
+        let res = unsafe { crypto_pwhash_scryptsalsa208sha256(
-            OPSLIMIT_INTERACTIVE, MEMLIMIT_INTERACTIVE).unwrap();
+            key.as_mut_ptr(),
-        let mut crypto_key = CryptoKey([0; KEYBYTES]);
+            key.len() as u64,
-        let mut auth_key = AuthKey([0; KEYBYTES]);
+            password.as_bytes().as_ptr(),
-        for i in 0..KEYBYTES {
+            password.as_bytes().len() as u64,
-            crypto_key.0[i] = key[i];
+            salt.as_ptr() as *const [u8; crypto_pwhash_scryptsalsa208sha256_SALTBYTES],
+            crypto_pwhash_scryptsalsa208sha256_OPSLIMIT_INTERACTIVE as u64,
+            crypto_pwhash_scryptsalsa208sha256_MEMLIMIT_INTERACTIVE
+        ) };
+        if res != 0 {
+            panic!("Key derivation failed");
         }
-        for i in 0..KEYBYTES {
+        let mut crypto_key = [0; 32];
-            auth_key.0[i] = key[KEYBYTES+i];
+        for i in 0..crypto_key.len() {
+            crypto_key[i] = key[i];
         }
-        Crypto::ChaCha20HmacSha512256{crypto_key: crypto_key, auth_key: auth_key, nonce: gen_nonce()}
+        let mut nonce = [0u8; 8];
+        unsafe { randombytes_buf(nonce.as_mut_ptr(), nonce.len()) };
+        Crypto::ChaCha20Poly1305{key: crypto_key, nonce: nonce}
     }
 
-    pub fn decrypt(&self, mut buf: &mut [u8], nonce: &[u8], hash: &[u8]) -> Result<(), Error> {
+    pub fn decrypt(&self, mut buf: &mut [u8], nonce: &[u8], header: &[u8]) -> Result<usize, Error> {
         match self {
-            &Crypto::None => unreachable!("This should never be called"),
+            &Crypto::None => Ok(buf.len()),
-            &Crypto::ChaCha20HmacSha512256{ref crypto_key, ref auth_key, nonce: _} => {
+            &Crypto::ChaCha20Poly1305{ref key, nonce: _} => {
-                let nonce = Nonce::from_slice(nonce).unwrap();
+                let mut mlen: u64 = buf.len() as u64;
-                let hash = Tag::from_slice(hash).unwrap();
+                let res = unsafe { crypto_aead_chacha20poly1305_decrypt(
-                stream_xor_inplace(&mut buf, &nonce, crypto_key);
+                    buf.as_mut_ptr(), // Base pointer to buffer
-                match verify(&hash, &buf, auth_key) {
+                    &mut mlen, // Mutable size of buffer (will be set to used size)
-                    true => Ok(()),
+                    ptr::null_mut::<[u8; 0]>(), // Mutable base pointer to secret nonce (always NULL)
-                    false => Err(Error::CryptoError("Decryption failed"))
+                    buf.as_ptr(), // Base pointer to message
+                    buf.len() as u64, // Size of message
+                    header.as_ptr(), // Base pointer to additional data
+                    header.len() as u64, // Size of additional data
+                    nonce.as_ptr() as *const [u8; 8], // Base pointer to public nonce
+                    key.as_ptr() as *const [u8; 32] // Base pointer to key
+                ) };
+                match res {
+                    0 => Ok(mlen as usize),
+                    _ => Err(Error::CryptoError("Failed to decrypt"))
                 }
             }
         }
     }
 
-    pub fn encrypt(&mut self, mut buf: &mut [u8]) -> (Vec<u8>, Vec<u8>) {
+    pub fn encrypt(&mut self, mut buf: &mut [u8], mlen: usize, nonce_bytes: &mut [u8], header: &[u8]) -> usize {
         match self {
-            &mut Crypto::None => unreachable!("This should never be called"),
+            &mut Crypto::None => mlen,
-            &mut Crypto::ChaCha20HmacSha512256{ref crypto_key, ref auth_key, ref mut nonce} => {
+            &mut Crypto::ChaCha20Poly1305{ref key, ref mut nonce} => {
                 *nonce = inc_nonce(*nonce);
-                let hash = authenticate(&buf, auth_key);
+                let mut clen: u64 = buf.len() as u64;
-                stream_xor_inplace(&mut buf, nonce, crypto_key);
+                assert_eq!(nonce_bytes.len(), nonce.len());
-                (nonce.0.iter().map(|v| *v).collect(), hash.0.iter().map(|v| *v).collect())
+                assert_eq!(nonce.len(), crypto_aead_chacha20poly1305_NPUBBYTES);
+                assert_eq!(key.len(), crypto_aead_chacha20poly1305_KEYBYTES);
+                assert_eq!(0, crypto_aead_chacha20poly1305_NSECBYTES);
+                assert!(clen as usize >= mlen + crypto_aead_chacha20poly1305_ABYTES);
+                let res = unsafe { crypto_aead_chacha20poly1305_encrypt(
+                    buf.as_mut_ptr(), // Base pointer to buffer
+                    &mut clen, // Mutable size of buffer (will be set to used size)
+                    buf.as_ptr(), // Base pointer to message
+                    mlen as u64, // Size of message
+                    header.as_ptr(), // Base pointer to additional data
+                    header.len() as u64, // Size of additional data
+                    ptr::null::<[u8; 0]>(), // Base pointer to secret nonce (always NULL)
+                    nonce.as_ptr() as *const [u8; 8], // Base pointer to public nonce
+                    key.as_ptr() as *const [u8; 32] // Base pointer to key
+                ) };
+                assert_eq!(res, 0);
+                assert_eq!(clen as usize, mlen + crypto_aead_chacha20poly1305_ABYTES);
+                unsafe {
+                    ptr::copy_nonoverlapping(nonce.as_ptr(), nonce_bytes.as_mut_ptr(), nonce.len());
+                }
+                clen as usize
             }
         }
     }
@@ -93,14 +126,21 @@ fn encrypt_decrypt() {
     let mut sender = Crypto::from_shared_key("test");
     let receiver = Crypto::from_shared_key("test");
     let msg = "HelloWorld0123456789";
-    let mut buffer: Vec<u8> = msg.bytes().collect();
+    let msg_bytes = msg.as_bytes();
-    let (nonce1, hash1) = sender.encrypt(&mut buffer);
+    let mut buffer = [0u8; 1024];
-    assert!(msg.as_bytes() != &buffer as &[u8]);
+    let header = [0u8; 8];
-    receiver.decrypt(&mut buffer, &nonce1, &hash1).unwrap();
+    for i in 0..msg_bytes.len() {
-    assert_eq!(msg.as_bytes(), &buffer as &[u8]);
+        buffer[i] = msg_bytes[i];
-    let (nonce2, hash2) = sender.encrypt(&mut buffer);
+    }
+    let mut nonce1 = [0u8; 8];
+    let size = sender.encrypt(&mut buffer, msg_bytes.len(), &mut nonce1, &header);
+    assert_eq!(size, msg_bytes.len() + sender.additional_bytes());
+    assert!(msg_bytes != &buffer[..msg_bytes.len()] as &[u8]);
+    receiver.decrypt(&mut buffer[..size], &nonce1, &header).unwrap();
+    assert_eq!(msg_bytes, &buffer[..msg_bytes.len()] as &[u8]);
+    let mut nonce2 = [0u8; 8];
+    let size = sender.encrypt(&mut buffer, msg_bytes.len(), &mut nonce2, &header);
     assert!(nonce1 != nonce2);
-    assert!(hash1 == hash2);
+    receiver.decrypt(&mut buffer[..size], &nonce2, &header).unwrap();
-    receiver.decrypt(&mut buffer, &nonce2, &hash2).unwrap();
+    assert_eq!(msg_bytes, &buffer[..msg_bytes.len()] as &[u8]);
-    assert_eq!(msg.as_bytes(), &buffer as &[u8]);
 }

src/main.rs

@@ -3,7 +3,7 @@ extern crate time;
 extern crate docopt;
 extern crate rustc_serialize;
 extern crate epoll;
-#[cfg(feature = "crypto")] extern crate sodiumoxide;
+#[cfg(feature = "crypto")] extern crate libsodium_sys;
 
 mod util;
 mod types;
@@ -31,9 +31,6 @@ use crypto::Crypto;
 
 //TODO: Implement IPv6
 //TODO: Call close
-//FIXME: The crypto method should be signaled as part of the protocol
-//FIXME: The HMAC should also include the header
-//FIXME: Encryption should also include all following additional headers
 
 struct SimpleLogger;
 

src/udpmessage.rs

@@ -65,7 +65,8 @@ impl<'a> fmt::Debug for Message<'a> {
 }
 
 pub fn decode<'a>(data: &'a mut [u8], crypto: &mut Crypto) -> Result<(Options, Message<'a>), Error> {
-    if data.len() < mem::size_of::<TopHeader>() {
+    let mut end = data.len();
+    if end < mem::size_of::<TopHeader>() {
         return Err(Error::ParseError("Empty message"));
     }
     let mut pos = 0;
@@ -81,18 +82,21 @@ pub fn decode<'a>(data: &'a mut [u8], crypto: &mut Crypto) -> Result<(Options, M
         return Err(Error::CryptoError("Wrong crypto method"));
     }
     if crypto.method() > 0 {
-        let len = crypto.nonce_bytes() + crypto.auth_bytes();
+        let len = crypto.nonce_bytes();
-        if data.len() < pos + len {
+        if end < pos + len {
             return Err(Error::ParseError("Truncated crypto header"));
         }
-        let (crypto_header, crypto_data) = data[pos..].split_at_mut(len);
+        {
+            let (before, after) = data.split_at_mut(pos);
+            let (nonce, crypto_data) = after.split_at_mut(len);
             pos += len;
-        let (nonce, hash) = crypto_header.split_at(crypto.nonce_bytes());
+            end = try!(crypto.decrypt(crypto_data, nonce, &before[..mem::size_of::<TopHeader>()])) + pos;
-        try!(crypto.decrypt(crypto_data, nonce, hash));
+        }
+        assert_eq!(end, data.len()-crypto.additional_bytes());
     }
     let mut options = Options::default();
     if header.flags & 0x01 > 0 {
-        if data.len() < pos + NETWORK_ID_BYTES {
+        if end < pos + NETWORK_ID_BYTES {
             return Err(Error::ParseError("Truncated options"));
         }
         let id = u64::from_be(*unsafe { as_obj::<u64>(&data[pos..pos+NETWORK_ID_BYTES]) });
@@ -100,15 +104,15 @@ pub fn decode<'a>(data: &'a mut [u8], crypto: &mut Crypto) -> Result<(Options, M
         pos += NETWORK_ID_BYTES;
     }
     let msg = match header.msgtype {
-        0 => Message::Data(&data[pos..]),
+        0 => Message::Data(&data[pos..end]),
         1 => {
-            if data.len() < pos + 1 {
+            if end < pos + 1 {
                 return Err(Error::ParseError("Empty peers"));
             }
             let count = data[pos];
             pos += 1;
             let len = count as usize * 6;
-            if data.len() < pos + len {
+            if end < pos + len {
                 return Err(Error::ParseError("Peer data too short"));
             }
             let mut peers = Vec::with_capacity(count as usize);
@@ -127,28 +131,24 @@ pub fn decode<'a>(data: &'a mut [u8], crypto: &mut Crypto) -> Result<(Options, M
             Message::Peers(peers)
         },
         2 => {
-            if data.len() < pos + 1 {
+            if end < pos + 1 {
                 return Err(Error::ParseError("Init data too short"));
             }
             let count = data[pos] as usize;
             pos += 1;
             let mut addrs = Vec::with_capacity(count);
             for _ in 0..count {
-                if data.len() < pos + 1 {
+                if end < pos + 1 {
                     return Err(Error::ParseError("Init data too short"));
                 }
                 let len = data[pos] as usize;
                 pos += 1;
-                if data.len() < pos + len {
+                if end < pos + len + 1 {
                     return Err(Error::ParseError("Init data too short"));
                 }
                 let base = Address(to_vec(&data[pos..pos+len]));
-                pos += len;
+                let prefix_len = data[pos+len];
-                if data.len() < pos + 1 {
+                pos += len + 1;
-                    return Err(Error::ParseError("Init data too short"));
-                }
-                let prefix_len = data[pos];
-                pos += 1;
                 addrs.push(Range{base: base, prefix_len: prefix_len});
             }
             Message::Init(addrs)
@@ -176,11 +176,7 @@ pub fn encode(options: &Options, msg: &Message, buf: &mut [u8], crypto: &mut Cry
     let header_dat = unsafe { as_bytes(&header) };
     unsafe { ptr::copy_nonoverlapping(header_dat.as_ptr(), buf[pos..].as_mut_ptr(), header_dat.len()) };
     pos += header_dat.len();
-    let nonce_pos = pos;
     pos += crypto.nonce_bytes();
-    let hash_pos = pos;
-    pos += crypto.auth_bytes();
-    let crypto_pos = pos;
     if let Some(id) = options.network_id {
         assert!(buf.len() >= pos + NETWORK_ID_BYTES);
         unsafe {
@@ -243,13 +239,11 @@ pub fn encode(options: &Options, msg: &Message, buf: &mut [u8], crypto: &mut Cry
         }
     }
     if crypto.method() > 0 {
-        let (nonce, hash) = crypto.encrypt(&mut buf[crypto_pos..pos]);
+        let (header, rest) = buf.split_at_mut(mem::size_of::<TopHeader>());
-        assert_eq!(nonce.len(), crypto.nonce_bytes());
+        let (nonce, rest) = rest.split_at_mut(crypto.nonce_bytes());
-        assert_eq!(hash.len(), crypto.auth_bytes());
+        let crypto_start = header.len() + nonce.len();
-        unsafe {
+        assert!(rest.len() >= pos - crypto_start + crypto.additional_bytes());
-            ptr::copy_nonoverlapping(nonce.as_ptr(), buf[nonce_pos..].as_mut_ptr(), crypto.nonce_bytes());
+        pos = crypto.encrypt(rest, pos-crypto_start, nonce, header) + crypto_start;
-            ptr::copy_nonoverlapping(hash.as_ptr(), buf[hash_pos..].as_mut_ptr(), crypto.auth_bytes());
-        }
     }
     pos
 }
@@ -279,7 +273,7 @@ fn encode_message_encrypted() {
     let msg = Message::Data(&payload);
     let mut buf = [0; 1024];
     let size = encode(&mut options, &msg, &mut buf[..], &mut crypto);
-    assert_eq!(size, 53);
+    assert_eq!(size, 37);
     assert_eq!(&buf[..8], &[118,112,110,1,1,0,0,0]);
     let (options2, msg2) = decode(&mut buf[..size], &mut crypto).unwrap();
     assert_eq!(options, options2);

vpncloud.md

@@ -92,10 +92,6 @@ vpncloud(1) -- Peer-to-peer VPN
 
     Display the help.
 
-  * `-V`, `--version`:
-
-    Print the version and exit.
-
 
 ## DESCRIPTION
 
@@ -198,11 +194,10 @@ vpncloud -t tun -c REMOTE_HOST:PORT --subnet 10.0.0.X/32 --ifup 'ifconfig $IFNAM
   primitives are expected to be very secure, their application has not been
   reviewed.
   The shared key is hashed using *ScryptSalsa208Sha256* to derive a key,
-  which is used to encrypt the payload of messages using *ChaCha20*. The
+  which is used to encrypt the payload of messages using *ChaCha20Poly1305*.
-  authenticity of messages is verified using *HmacSha512256* hashes.
+  The encryption includes an authentication that also protects the header and
-  This method only protects the contents of the message (payload, peer list,
+  all additional headers.
-  etc.) but not the header of each message.
+  This method does only protect against attacks on single messages but not
-  Also, this method does only protect against attacks on single messages but not
   on attacks that manipulate the message series itself (i.e. suppress messages,
   reorder them, and duplicate them).
 
@@ -224,7 +219,21 @@ Every packet sent over UDP contains the following header (in order):
     This field specifies the version and helps nodes to parse the rest of the
     header and the packet.
 
-  * 2 `reserved bytes` that are currently unused
+  * 1 byte `crypto method`
+
+    This field specifies the method that must be used to decrypt the rest of the
+    data (additional headers and message contents). The currently supported
+    methods are:
+
+    - Method `0`, **No encryption**: Rest of the data can be read without
+      decrypting it.
+
+    - Method `1`, **ChaCha20Poly1305**: The header is followed by a 8 byte
+      *nonce*. The rest of the data is encrypted with the
+      `libsodium::crypto_aead_chacha20poly1305` method, using the 8 byte header
+      as additional data.
+
+  * 1 `reserved byte` that is currently unused
 
   * 1 byte for `flags`
 
@@ -234,7 +243,6 @@ Every packet sent over UDP contains the following header (in order):
     order. Currently the following additional headers are supported:
 
     - Bit 1: Network ID
-    - Bit 2: Crypto information
 
   * 1 byte for the `message type`
 
@@ -246,23 +254,18 @@ Every packet sent over UDP contains the following header (in order):
     - Type 2: Initial message
     - Type 3: Closing message
 
-After this 8 byte header, the additional headers as specified in the `flags`
+After this 8 byte header, the rest of the message follows. It is encrypted using
-field will follow in the order of their respective flag bits.
+the method specified in the header.
+
+In the decrypted data, the additional headers as specified in the `flags` field
+will follow in the order of their respective flag bits.
 
   * **Network ID**:
 
     The network id is encoded as 8 bytes.
 
-  * **Crypto information**:
+After the additional headers, the message as specified in the `message type`
-
+field will follow:
-    If this header is present, the contents of the message are encrypted and
-    must have to decrypted before decoding.
-    This option contains 40 bytes. The first 8 bytes are the **nonce** for this
-    message and the later 32 bytes are the **authentication hash** of the
-    message.
-
-After the additional headers, message as specified in the `message type` field
-will follow:
 
   * **Data packet** (message type 0):
     This packet contains payload. The format of the data depends on the device