Use arrays instead of slices and Vecs #33

There are several instance of &[u8] and Vec<u8> being used as argument types
for data that is then fallibly converted into a fixed-size array inside the
function. Using &[u8; N] or [u8; N] for the argument types instead means that
many runtime length checks can be moved from the library's internals all the
way out to the API boundary, and eventually removed entirely when the
signatures of API functions are updated accordingly in a major update.

ChangeLog.md

@@ -1,6 +1,9 @@
 # Changelog
 
 ## Unreleased
+### Features
+* Added `utils::randombytes_array()` function to create fixed-sized arrays of random numbers
+
 ### Fixes
 * Loosened the argument types for `ItemMetadata::set_*()` methods to allow passing `String`s directly
 * Loosened the argument types for several methods to take any `IntoIterator`, not just `Iterator`
@@ -10,6 +13,7 @@
 * Renamed `SignedInvitation::from_*()` methods to `sender_*()` to avoid confusing them with constructors.
 * Renamed the `Account::is_etebase_server()` function to a method - `Client::is_server_valid()`.
 * `Error` now implements `Eq`.
+* Made some error messages more specific
 
 ## Version 0.5.3
 * Upgrade dependencies - ignore package lock. Upgrade to absolute latest.

TODO.md

@@ -0,0 +1,9 @@
+# TODO
+
+This file tracks planned changes to the project, especially breaking changes that have to wait
+until the next major release.
+
+## Planned changes for next major release
+
+- Change argument types in `src/service.rs` for public and private keys to use `[u8; 32]` instead
+  of `[u8]`. This should remove all the `ProgrammingError`s resulting from failed `try_into()` calls.

src/crypto.rs

@@ -8,6 +8,8 @@ use sodiumoxide::crypto::{
     sign,
 };
 
+use crate::utils::{SALT_SIZE, SYMMETRIC_KEY_SIZE};
+
 use super::error::{Error, Result};
 
 macro_rules! to_enc_error {
@@ -16,36 +18,36 @@ macro_rules! to_enc_error {
     };
 }
 
-fn generichash_quick(msg: &[u8], key: Option<&[u8]>) -> Result<Vec<u8>> {
+fn generichash_quick(msg: &[u8], key: Option<&[u8]>) -> Result<[u8; 32]> {
     let mut state = to_enc_error!(
         generichash::State::new(Some(32), key),
         "Failed to init hash"
     )?;
     to_enc_error!(state.update(msg), "Failed to update hash")?;
     Ok(to_enc_error!(state.finalize(), "Failed to finalize hash")?
         .as_ref()
-        .to_vec())
+        .try_into()
+        .expect("generichash returned result of wrong size"))
 }
 
 pub fn init() -> Result<()> {
     to_enc_error!(sodiumoxide::init(), "Failed initialising libsodium")
 }
 
-pub fn derive_key(salt: &[u8], password: &str) -> Result<Vec<u8>> {
-    let mut key = vec![0; 32];
-    let salt = &salt[..argon2id13::SALTBYTES];
-    let salt: &[u8; argon2id13::SALTBYTES] =
-        to_enc_error!(salt.try_into(), "Expect salt to be at least 16 bytes")?;
+pub fn derive_key(salt: &[u8; SALT_SIZE], password: &str) -> Result<[u8; SYMMETRIC_KEY_SIZE]> {
+    let mut key = [0; SYMMETRIC_KEY_SIZE];
     let password = password.as_bytes();
 
-    let ret = argon2id13::derive_key(
+    argon2id13::derive_key(
         &mut key,
         password,
         &argon2id13::Salt(*salt),
         argon2id13::OPSLIMIT_SENSITIVE,
         argon2id13::MEMLIMIT_MODERATE,
-    );
-    Ok(to_enc_error!(ret, "pwhash failed")?.as_ref().to_vec())
+    )
+    .map_err(|_| Error::Encryption("pwhash failed"))?;
+
+    Ok(key)
 }
 
 pub struct CryptoManager {
@@ -218,19 +220,19 @@ impl CryptoManager {
         )?)
     }
 
-    pub fn derive_subkey(&self, salt: &[u8]) -> Result<Vec<u8>> {
+    pub fn derive_subkey(&self, salt: &[u8]) -> Result<[u8; 32]> {
         generichash_quick(&self.sub_derivation_key, Some(salt))
     }
 
     pub fn crypto_mac(&self) -> Result<CryptoMac> {
         CryptoMac::new(Some(&self.mac_key))
     }
 
-    pub fn calculate_mac(&self, msg: &[u8]) -> Result<Vec<u8>> {
+    pub fn calculate_mac(&self, msg: &[u8]) -> Result<[u8; 32]> {
         generichash_quick(msg, Some(&self.mac_key))
     }
 
-    pub fn calculate_hash(&self, msg: &[u8]) -> Result<Vec<u8>> {
+    pub fn calculate_hash(&self, msg: &[u8]) -> Result<[u8; 32]> {
         generichash_quick(msg, None)
     }
 }
@@ -299,14 +301,14 @@ impl BoxCryptoManager {
         let privkey_scalar = scalarmult::Scalar(*privkey);
         let privkey = box_::SecretKey(*privkey);
         let pubkey_scalar = scalarmult::scalarmult_base(&privkey_scalar);
-        let pubkey = box_::PublicKey(pubkey_scalar[..].try_into().unwrap());
+        let pubkey = box_::PublicKey(pubkey_scalar.0);
 
         Ok(BoxCryptoManager { privkey, pubkey })
     }
 
     pub fn encrypt(&self, msg: &[u8], pubkey: &[u8; box_::PUBLICKEYBYTES]) -> Result<Vec<u8>> {
-        let pubkey = box_::PublicKey(pubkey[..].try_into().unwrap());
-        let privkey = box_::SecretKey(self.privkey[..].try_into().unwrap());
+        let pubkey = box_::PublicKey(*pubkey);
+        let privkey = box_::SecretKey(self.privkey.0);
         let nonce = box_::gen_nonce();
         let encrypted = box_::seal(msg, &nonce, &pubkey, &privkey);
         let ret = [nonce.as_ref(), &encrypted].concat();
@@ -315,8 +317,8 @@ impl BoxCryptoManager {
     }
 
     pub fn decrypt(&self, cipher: &[u8], pubkey: &[u8; sign::PUBLICKEYBYTES]) -> Result<Vec<u8>> {
-        let pubkey = box_::PublicKey(pubkey[..].try_into().unwrap());
-        let privkey = box_::SecretKey(self.privkey[..].try_into().unwrap());
+        let pubkey = box_::PublicKey(*pubkey);
+        let privkey = box_::SecretKey(self.privkey.0);
         let nonce = &cipher[..box_::NONCEBYTES];
         let nonce: &[u8; box_::NONCEBYTES] =
             to_enc_error!(nonce.try_into(), "Got a nonce of a wrong size")?;

src/encrypted_models.rs

@@ -10,12 +10,12 @@ use std::collections::HashMap;
 
 use serde::{Deserialize, Serialize};
 use serde_repr::{Deserialize_repr, Serialize_repr};
+use sodiumoxide::crypto::sign;
 
 use super::{
     chunker::Rollsum,
     crypto::{BoxCryptoManager, CryptoMac, CryptoManager},
     error::{Error, Result},
-    try_into,
     utils::{
         buffer_pad, buffer_pad_fixed, buffer_pad_small, buffer_unpad, buffer_unpad_fixed,
         from_base64, memcmp, randombytes, shuffle, to_base64, MsgPackSerilization, StringBase64,
@@ -265,15 +265,17 @@ impl SignedInvitation {
         &self,
         identity_crypto_manager: &BoxCryptoManager,
     ) -> Result<Vec<u8>> {
-        let from_pubkey = match self.from_pubkey.as_deref() {
-            Some(from_pubkey) => from_pubkey,
-            None => {
-                return Err(Error::ProgrammingError(
-                    "Missing invitation encryption key.",
-                ))
-            }
-        };
-        identity_crypto_manager.decrypt(&self.signed_encryption_key, try_into!(from_pubkey)?)
+        let from_pubkey = self
+            .from_pubkey
+            .as_deref()
+            .ok_or(Error::ProgrammingError(
+                "Missing invitation encryption key.",
+            ))?
+            .try_into()
+            .map_err(|_| {
+                Error::Encryption("Received invitation encryption key has wrong length")
+            })?;
+        identity_crypto_manager.decrypt(&self.signed_encryption_key, from_pubkey)
     }
 }
 
@@ -475,7 +477,7 @@ impl EncryptedCollection {
         account_crypto_manager: &AccountCryptoManager,
         identity_crypto_manager: &BoxCryptoManager,
         username: &str,
-        pubkey: &[u8],
+        pubkey: &[u8; sign::PUBLICKEYBYTES],
         access_level: CollectionAccessLevel,
     ) -> Result<SignedInvitation> {
         let uid = to_base64(&randombytes(32))?;
@@ -486,8 +488,8 @@ impl EncryptedCollection {
             collection_type,
         };
         let raw_content = rmp_serde::to_vec_named(&content)?;
-        let signed_encryption_key = identity_crypto_manager
-            .encrypt(&buffer_pad_small(&raw_content)?, try_into!(pubkey)?)?;
+        let signed_encryption_key =
+            identity_crypto_manager.encrypt(&buffer_pad_small(&raw_content)?, pubkey)?;
         Ok(SignedInvitation {
             uid,
             version: CURRENT_VERSION,
@@ -526,9 +528,12 @@ impl EncryptedCollection {
         collection_type: Option<&[u8]>,
     ) -> Result<CollectionCryptoManager> {
         let encryption_key =
-            Self::collection_key_static(parent_crypto_manager, encryption_key, collection_type)?;
+            Self::collection_key_static(parent_crypto_manager, encryption_key, collection_type)?
+                .as_slice()
+                .try_into()
+                .map_err(|_| Error::Encryption("Collection encryption key has wrong length"))?;
 
-        CollectionCryptoManager::new(try_into!(&encryption_key[..])?, version)
+        CollectionCryptoManager::new(&encryption_key, version)
     }
 
     pub fn crypto_manager(
@@ -609,12 +614,13 @@ impl EncryptedRevision {
         crypto_manager: &ItemCryptoManager,
         additional_data: &[u8],
     ) -> Result<bool> {
-        let mac = from_base64(&self.uid)?;
+        let mac = from_base64(&self.uid)?
+            .as_slice()
+            .try_into()
+            .map_err(|_| Error::Encryption("Collection MAC has wrong length"))?;
         let ad_hash = self.calculate_hash(crypto_manager, additional_data)?;
 
-        crypto_manager
-            .0
-            .verify(&self.meta, try_into!(&mac[..])?, Some(&ad_hash))
+        crypto_manager.0.verify(&self.meta, &mac, Some(&ad_hash))
     }
 
     pub fn set_meta(
@@ -639,14 +645,17 @@ impl EncryptedRevision {
         crypto_manager: &ItemCryptoManager,
         additional_data: &[u8],
     ) -> Result<Vec<u8>> {
-        let mac = from_base64(&self.uid)?;
+        let mac = from_base64(&self.uid)?
+            .as_slice()
+            .try_into()
+            .map_err(|_| Error::Encryption("Collection MAC has wrong length"))?;
         let ad_hash = self.calculate_hash(crypto_manager, additional_data)?;
 
-        buffer_unpad(&crypto_manager.0.decrypt_detached(
-            &self.meta,
-            try_into!(&mac[..])?,
-            Some(&ad_hash),
-        )?)
+        buffer_unpad(
+            &crypto_manager
+                .0
+                .decrypt_detached(&self.meta, &mac, Some(&ad_hash))?,
+        )
     }
 
     pub fn set_content(
@@ -978,11 +987,17 @@ impl EncryptedItem {
         encryption_key: Option<&[u8]>,
     ) -> Result<ItemCryptoManager> {
         let encryption_key = match encryption_key {
-            Some(encryption_key) => parent_crypto_manager.0.decrypt(encryption_key, None)?,
+            Some(encryption_key) => parent_crypto_manager
+                .0
+                .decrypt(encryption_key, None)?
+                .try_into()
+                .map_err(|_| {
+                    Error::ProgrammingError("Decrypted encryption key has wrong length")
+                })?,
             None => parent_crypto_manager.0.derive_subkey(uid.as_bytes())?,
         };
 
-        ItemCryptoManager::new(try_into!(&encryption_key[..])?, version)
+        ItemCryptoManager::new(&encryption_key, version)
     }
 
     pub fn crypto_manager(