trjhtr

I've tried as best I can, to implement HMAC as specified in the RFC 2104. This is also my first Rust project, so the chances that rookie mistakes have been made are high. Since I'm fairly new to the language, I'm looking for feedback in general.

use functions;
use ring::digest, test;

// Set blocksizes
pub const BLOCKSIZE_256: usize = 64;
pub const BLOCKSIZE_512: usize = 128;

/// Return either a SHA256 or SHA512 digest of byte vector
fn hash(variant: i32, mut data: Vec<u8>) -> Vec<u8> 
 if variant == 256 
 data = (digest::digest(&digest::SHA256, &data).as_ref()).to_vec();
 else if variant == 512 
 data = (digest::digest(&digest::SHA512, &data).as_ref()).to_vec();
 else 
 panic!("Invalid variant. Valid variants are 256 and 512.");
 
 return data;


/// Return a key k that has been padded to fit the selected blocksize
fn key_deriv(variant: i32, mut k: Vec<u8>) -> Vec<u8> 
 if variant == 256 
 // If key k is bigger than blocksize, it should be hashed and then padded with zeroes
 // to fit blocksize
 if k.len() > BLOCKSIZE_256 
 k = hash(variant, k);
 
 while k.len() < BLOCKSIZE_256 
 k.push(0x00);
 
 return k;
 else if variant == 512 
 if k.len() > BLOCKSIZE_512 
 k = hash(variant, k);
 
 while k.len() < BLOCKSIZE_512 
 k.push(0x00);
 
 return k;
 else 
 panic!("Invalid variant. Valid variants are 256 and 512.");
 


/// Returns an HMAC from message m and key k
pub fn hmac(variant: i32, mut k: Vec<u8>, mut m: Vec<u8>) -> Vec<u8> 
 // Initialize vectors that will hold the ipad and opad
 let mut ipad = vec!;
 let mut opad = vec!;
 // Pad the key
 k = key_deriv(variant, k);

 for count in 0..k.len() 
 ipad.push(k[count] ^ 0x36);
 opad.push(k[count] ^ 0x5C);
 

 ipad.append(&mut m);
 ipad = hash(variant, ipad);
 opad.append(&mut ipad);
 opad = hash(variant, opad);

 return opad;

A possible improvement I'm considering is to change the zero padding of k in key_deriv(). Right now it operates on a while loop, but I noticed that an approach like below, was slightly faster. Though I suspected it might be a little more memory intensive because of the vector allocation and I'm not sure which of the two to choose.

let len = k.len();
if len < BLOCKSIZE_256 
 k.append(&mut vec![0x00; (BLOCKSIZE_256-len)]);

1. A comment stating that you are declaring values isn't useful.




2. Don't use return at the end of blocks; just let the block evaluate to the last expression.




3. This extends to conditionals like if/else: just let the condition evaluate, there's no reason to set a variable. Note that this allows you to avoid unneeded mutability.




4. Any time you find yourself writing parallel conditions in multiple places, that's an opportunity to add abstraction. In this case, an enum would be sufficient. Note that this removes the panics; we've statically removed an entire failure case!




5. You don't need to add parenthesis for chained functions.




6. Pay attention to your duplication between your conditionals. You can extract most of the duplicated code to DRY up the logic. Note how you even have comments on only one of your two conditional branches even though it should apply to both.




7. This even makes it so that we no longer need constants because the match prevents duplication.




8. Instead of looping and pushing values, use Vec:resize




9. Instead of calling the method key_deriv and having comments explaining that it pads it, just rename the method.




10. Don't needlessly make variable bindings mutable; it's completely fine to re-bind the same name with let. A good example of this is the key in hmac.




11. Take &[u8] if you aren't making use of the memory allocation.




12. Use a Cow to avoid allocating anything if the key is already correctly sized.




13. Creating a Vec and then manually pushing bytes is probably the slowest path. Instead, either allocate a Vec with enough space or use collect which knows how many elements there are. You could also duplicate the key twice and then mutate it in place.




14. Avoid indexing into a slice when feasible. Iterators tend to have less overhead.




15. Don't use variable names like k. Instead, type out very long words like "key".




16. There's no real reason to use Vec::append as you don't use the source vector until it's destroyed a few lines later. Might as well use the boring extend_from_slice.




17. Because you've forced hash to return a Vec, you are forcing one extra allocation (that of hash(inner_pad)) You can avoid that by returning the result of the digest and only converting to a Vec when needed.

extern crate ring;

use ring::digest;
use std::borrow::Cow;

enum Variant 
 Small,
 Big,


impl Variant 
 /// Return either a SHA256 or SHA512 digest of byte vector
 fn hash(&self, data: &[u8]) -> Vec<u8> 
 let method = match *self 
 Variant::Small => &digest::SHA256,
 Variant::Big => &digest::SHA512,
 ;
 digest::digest(method, data).as_ref().to_vec()
 

 fn blocksize(&self) -> usize 
 match *self 
 Variant::Small => 64,
 Variant::Big => 128,
 
 

 fn pad_key<'a>(&self, k: &'a [u8]) -> Cow<'a, [u8]> 
 let mut k = Cow::from(k);

 // If key k is bigger than blocksize, it should be hashed...
 if k.len() > self.blocksize() 
 k = self.hash(&k).into();
 

 // ... and then padded with zeroes to fit the blocksize
 if k.len() < self.blocksize() 
 let mut resized_key = k.into_owned();
 resized_key.resize(self.blocksize(), 0x00);
 k = resized_key.into();
 

 k
 

 /// Returns an HMAC from message m and key k
 pub fn hmac(&self, key: &[u8], message: &[u8]) -> Vec<u8> 
 let mut pad = key.to_vec();
 for i in &mut pad *i ^= byte ;
 pad
 ;

 let mut inner_pad = make_padded_key(0x36);
 let mut outer_pad = make_padded_key(0x5C);

 inner_pad.extend_from_slice(message);
 let inner_pad = self.hash(&inner_pad);

 outer_pad.extend_from_slice(&inner_pad);
 self.hash(&outer_pad)
 


fn main()