HKDF and compare constant time in Rust

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
4
down vote

favorite












I have implemented HKDF in Rust, which passes all test vectors from RFC 5869, using HKDF with HMAC-SHA256. I also have a function to compare byte arrays in constant time, which I hope someone could confirm is correctly done. Any type of feedback is welcome.



EDIT: I mainly mean feedback on hkdf_compute(). HMAC in hkdf_extract() is defined in another module and not included here.



HKDF



pub struct Hkdf 
pub salt: Vec<u8>,
pub data: Vec<u8>,
pub info: Vec<u8>,
pub hmac: usize,
pub length: usize,


impl Drop for Hkdf
fn drop(&mut self)
//println!("DROPPING");
self.salt.clear();
self.data.clear();
self.info.clear()



impl Hkdf
/// Return HMAC matching argument passsed to Hkdf.
pub fn hkdf_extract(&self, data: &[u8], salt: &[u8]) -> Vec<u8>
let hmac_res = Hmac
secret_key: salt.to_vec(),
message: data.to_vec(),
sha2: self.hmac
;

hmac_res.hmac_compute()


/// The HKDF Expand step. Returns an HKDF.
pub fn hkdf_compute(&self) -> Vec<u8>
// Check that the selected key length is within the limit.
if self.length as f32 > 255_f32 * (self.hmac / 8) as f32
panic!("Derived key length above max. 255 * (HMAC OUTPUT LENGTH IN BYTES)");


let n_iter = (self.length as f32 / (self.hmac / 8) as f32).ceil() as usize;

let mut con_step: Vec<u8> = vec!;
let mut t_step: Vec<u8> = vec!;
let mut hkdf_final: Vec<u8> = vec!;

for x in 1..n_iter+1
con_step.append(&mut t_step);
con_step.extend_from_slice(&self.info);
con_step.push(x as u8);
t_step.extend_from_slice(&self.hkdf_extract(
&con_step,
&self.hkdf_extract(&self.data, &self.salt))
);
con_step.clear();

hkdf_final.extend_from_slice(&t_step);


hkdf_final.truncate(self.length);

hkdf_final




Compare constant time:



#[inline(never)]
/// Comparison in constant time.
pub fn compare_ct(x: &[u8], y: &[u8]) -> bool

let length = x.len();

if length != y.len()
false;


let mut result: u8 = 0;

for n in 0..length
result

result == 0







share|improve this question





















  • The Hkdf isn't complete. It's missing the definition of `Hmac'.
    – Zeta
    Mar 22 at 12:58










  • I'm sorry, I don't know what you mean by 'definition'?
    – user159822
    Mar 22 at 13:00











  • In let hmac_res = Hmac … , you use Hmac, which is unknown to us, the readers and reviewers. I guess you're more interested in a review of hkdf_compute only?
    – Zeta
    Mar 22 at 13:02










  • You're right, post updated!
    – user159822
    Mar 22 at 13:06
















up vote
4
down vote

favorite












I have implemented HKDF in Rust, which passes all test vectors from RFC 5869, using HKDF with HMAC-SHA256. I also have a function to compare byte arrays in constant time, which I hope someone could confirm is correctly done. Any type of feedback is welcome.



EDIT: I mainly mean feedback on hkdf_compute(). HMAC in hkdf_extract() is defined in another module and not included here.



HKDF



pub struct Hkdf 
pub salt: Vec<u8>,
pub data: Vec<u8>,
pub info: Vec<u8>,
pub hmac: usize,
pub length: usize,


impl Drop for Hkdf
fn drop(&mut self)
//println!("DROPPING");
self.salt.clear();
self.data.clear();
self.info.clear()



impl Hkdf
/// Return HMAC matching argument passsed to Hkdf.
pub fn hkdf_extract(&self, data: &[u8], salt: &[u8]) -> Vec<u8>
let hmac_res = Hmac
secret_key: salt.to_vec(),
message: data.to_vec(),
sha2: self.hmac
;

hmac_res.hmac_compute()


/// The HKDF Expand step. Returns an HKDF.
pub fn hkdf_compute(&self) -> Vec<u8>
// Check that the selected key length is within the limit.
if self.length as f32 > 255_f32 * (self.hmac / 8) as f32
panic!("Derived key length above max. 255 * (HMAC OUTPUT LENGTH IN BYTES)");


let n_iter = (self.length as f32 / (self.hmac / 8) as f32).ceil() as usize;

let mut con_step: Vec<u8> = vec!;
let mut t_step: Vec<u8> = vec!;
let mut hkdf_final: Vec<u8> = vec!;

for x in 1..n_iter+1
con_step.append(&mut t_step);
con_step.extend_from_slice(&self.info);
con_step.push(x as u8);
t_step.extend_from_slice(&self.hkdf_extract(
&con_step,
&self.hkdf_extract(&self.data, &self.salt))
);
con_step.clear();

hkdf_final.extend_from_slice(&t_step);


hkdf_final.truncate(self.length);

hkdf_final




Compare constant time:



#[inline(never)]
/// Comparison in constant time.
pub fn compare_ct(x: &[u8], y: &[u8]) -> bool

let length = x.len();

if length != y.len()
false;


let mut result: u8 = 0;

for n in 0..length
result

result == 0







share|improve this question





















  • The Hkdf isn't complete. It's missing the definition of `Hmac'.
    – Zeta
    Mar 22 at 12:58










  • I'm sorry, I don't know what you mean by 'definition'?
    – user159822
    Mar 22 at 13:00











  • In let hmac_res = Hmac … , you use Hmac, which is unknown to us, the readers and reviewers. I guess you're more interested in a review of hkdf_compute only?
    – Zeta
    Mar 22 at 13:02










  • You're right, post updated!
    – user159822
    Mar 22 at 13:06












up vote
4
down vote

favorite









up vote
4
down vote

favorite











I have implemented HKDF in Rust, which passes all test vectors from RFC 5869, using HKDF with HMAC-SHA256. I also have a function to compare byte arrays in constant time, which I hope someone could confirm is correctly done. Any type of feedback is welcome.



EDIT: I mainly mean feedback on hkdf_compute(). HMAC in hkdf_extract() is defined in another module and not included here.



HKDF



pub struct Hkdf 
pub salt: Vec<u8>,
pub data: Vec<u8>,
pub info: Vec<u8>,
pub hmac: usize,
pub length: usize,


impl Drop for Hkdf
fn drop(&mut self)
//println!("DROPPING");
self.salt.clear();
self.data.clear();
self.info.clear()



impl Hkdf
/// Return HMAC matching argument passsed to Hkdf.
pub fn hkdf_extract(&self, data: &[u8], salt: &[u8]) -> Vec<u8>
let hmac_res = Hmac
secret_key: salt.to_vec(),
message: data.to_vec(),
sha2: self.hmac
;

hmac_res.hmac_compute()


/// The HKDF Expand step. Returns an HKDF.
pub fn hkdf_compute(&self) -> Vec<u8>
// Check that the selected key length is within the limit.
if self.length as f32 > 255_f32 * (self.hmac / 8) as f32
panic!("Derived key length above max. 255 * (HMAC OUTPUT LENGTH IN BYTES)");


let n_iter = (self.length as f32 / (self.hmac / 8) as f32).ceil() as usize;

let mut con_step: Vec<u8> = vec!;
let mut t_step: Vec<u8> = vec!;
let mut hkdf_final: Vec<u8> = vec!;

for x in 1..n_iter+1
con_step.append(&mut t_step);
con_step.extend_from_slice(&self.info);
con_step.push(x as u8);
t_step.extend_from_slice(&self.hkdf_extract(
&con_step,
&self.hkdf_extract(&self.data, &self.salt))
);
con_step.clear();

hkdf_final.extend_from_slice(&t_step);


hkdf_final.truncate(self.length);

hkdf_final




Compare constant time:



#[inline(never)]
/// Comparison in constant time.
pub fn compare_ct(x: &[u8], y: &[u8]) -> bool

let length = x.len();

if length != y.len()
false;


let mut result: u8 = 0;

for n in 0..length
result

result == 0







share|improve this question













I have implemented HKDF in Rust, which passes all test vectors from RFC 5869, using HKDF with HMAC-SHA256. I also have a function to compare byte arrays in constant time, which I hope someone could confirm is correctly done. Any type of feedback is welcome.



EDIT: I mainly mean feedback on hkdf_compute(). HMAC in hkdf_extract() is defined in another module and not included here.



HKDF



pub struct Hkdf 
pub salt: Vec<u8>,
pub data: Vec<u8>,
pub info: Vec<u8>,
pub hmac: usize,
pub length: usize,


impl Drop for Hkdf
fn drop(&mut self)
//println!("DROPPING");
self.salt.clear();
self.data.clear();
self.info.clear()



impl Hkdf
/// Return HMAC matching argument passsed to Hkdf.
pub fn hkdf_extract(&self, data: &[u8], salt: &[u8]) -> Vec<u8>
let hmac_res = Hmac
secret_key: salt.to_vec(),
message: data.to_vec(),
sha2: self.hmac
;

hmac_res.hmac_compute()


/// The HKDF Expand step. Returns an HKDF.
pub fn hkdf_compute(&self) -> Vec<u8>
// Check that the selected key length is within the limit.
if self.length as f32 > 255_f32 * (self.hmac / 8) as f32
panic!("Derived key length above max. 255 * (HMAC OUTPUT LENGTH IN BYTES)");


let n_iter = (self.length as f32 / (self.hmac / 8) as f32).ceil() as usize;

let mut con_step: Vec<u8> = vec!;
let mut t_step: Vec<u8> = vec!;
let mut hkdf_final: Vec<u8> = vec!;

for x in 1..n_iter+1
con_step.append(&mut t_step);
con_step.extend_from_slice(&self.info);
con_step.push(x as u8);
t_step.extend_from_slice(&self.hkdf_extract(
&con_step,
&self.hkdf_extract(&self.data, &self.salt))
);
con_step.clear();

hkdf_final.extend_from_slice(&t_step);


hkdf_final.truncate(self.length);

hkdf_final




Compare constant time:



#[inline(never)]
/// Comparison in constant time.
pub fn compare_ct(x: &[u8], y: &[u8]) -> bool

let length = x.len();

if length != y.len()
false;


let mut result: u8 = 0;

for n in 0..length
result

result == 0









share|improve this question












share|improve this question




share|improve this question








edited May 27 at 10:57
























asked Mar 22 at 12:55







user159822


















  • The Hkdf isn't complete. It's missing the definition of `Hmac'.
    – Zeta
    Mar 22 at 12:58










  • I'm sorry, I don't know what you mean by 'definition'?
    – user159822
    Mar 22 at 13:00











  • In let hmac_res = Hmac … , you use Hmac, which is unknown to us, the readers and reviewers. I guess you're more interested in a review of hkdf_compute only?
    – Zeta
    Mar 22 at 13:02










  • You're right, post updated!
    – user159822
    Mar 22 at 13:06
















  • The Hkdf isn't complete. It's missing the definition of `Hmac'.
    – Zeta
    Mar 22 at 12:58










  • I'm sorry, I don't know what you mean by 'definition'?
    – user159822
    Mar 22 at 13:00











  • In let hmac_res = Hmac … , you use Hmac, which is unknown to us, the readers and reviewers. I guess you're more interested in a review of hkdf_compute only?
    – Zeta
    Mar 22 at 13:02










  • You're right, post updated!
    – user159822
    Mar 22 at 13:06















The Hkdf isn't complete. It's missing the definition of `Hmac'.
– Zeta
Mar 22 at 12:58




The Hkdf isn't complete. It's missing the definition of `Hmac'.
– Zeta
Mar 22 at 12:58












I'm sorry, I don't know what you mean by 'definition'?
– user159822
Mar 22 at 13:00





I'm sorry, I don't know what you mean by 'definition'?
– user159822
Mar 22 at 13:00













In let hmac_res = Hmac … , you use Hmac, which is unknown to us, the readers and reviewers. I guess you're more interested in a review of hkdf_compute only?
– Zeta
Mar 22 at 13:02




In let hmac_res = Hmac … , you use Hmac, which is unknown to us, the readers and reviewers. I guess you're more interested in a review of hkdf_compute only?
– Zeta
Mar 22 at 13:02












You're right, post updated!
– user159822
Mar 22 at 13:06




You're right, post updated!
– user159822
Mar 22 at 13:06










1 Answer
1






active

oldest

votes

















up vote
2
down vote



accepted










In general you should try and avoid floating point operations for any cryptographically specific code. Floating points have issues with regards to precision, and even I don't see any issue for this specific code, they always require special attention, e.g. during reviewing of the code.



Similarly, using recursive code is strongly discouraged for cryptographic algorithms. If your code can be coded using loops and only local variables you should really do so. I imagine that goes against the idea of Rust (and is thus up for debate), but crypto code generally requires special attention. If you keep to recursive calls make sure you document them as such, both inline and in the function description.



Although you nicely implemented the Drop destructor it makes more sense to create either a class with just a keyInputMaterial (now called data I presume) or simply keep all the data in parameters / local variables. The idea of just keeping the keyInputMaterial is that you may reuse the object to derive other keys using the function but with different salt / info. In that case you obviously need to retrain the Drop to clear the input key material.



Note that the internal key handling (padding, initial hashing) of HMAC is always the same. So a sneaky speed up is to perform pre-calculation on the salt which acts as the HMAC key (mainly useful if there is a lot of output expected, this speedup matters more for e.g. PBKDF2).




Your constant time function looks very much run-of-the-mill constant time, which in this case is great :) I don't think HKDF warrants a function where HMAC(k, x) and HMAC(k, y) are compared using a random key - but now you know about that neat little trick.



Note that you may want to make sure that you document that the comparison needs to be done in constant time or include it in a verify function (beware of truncated values - you did this in your code, but you need to consider it in the calling code as well!).




Note that I'm not a Rust expert, so I won't comment on the vector handling / slicing etc.






share|improve this answer























  • Excuse my overly compressed comment, I have a tough time with character limits. Hope you can see to formatting correctly.
    – user159822
    May 13 at 15:22










  • This is amazing feedback, really. I still have some follow-up questions I hope you would be willing to answer. * Do you have an alternative to CEIL func with floats? * For the recursive code I assume you mean where the extract function is called instead of passing a PRK in expand? * You want me to let the IKM be passed as a function paramter, so that it can be reused instead of Dropped? * The sneaky speed up, I'm not 100% sure what you mean here. You don't happen to have an example? * Am I not taking truncated values into account by comparing equal length vectors?
    – user159822
    May 13 at 15:32










  • You can perform + 1 before division instead of ceil For recursive, I see a call to hkdf_compute from hkdf_expand and vice versa. Yes, that or keep using it as a field, but pass the other fields as arguments. Please have a look at HMAC where the hash is used twice and the intermediate states of HMAC after hashing ipad and opad can be kept (you need to be able to store intermediate hash results though!). Yes, your compare code is fine, but was talking about using it. If you provide a verify function then the user is less likely to use another compare function.
    – Maarten Bodewes
    May 13 at 16:07










  • 1. Fantastic! 2. Understood. 3. Got it. 4. I think I see what you're getting at. I tried pre-padding the password in my PBKDF2 impl and immediately got a ~5 sec performance boost. 5. Right, I have taken care of this then. Thank you!
    – user159822
    May 13 at 17:40











Your Answer




StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);








 

draft saved


draft discarded


















StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190204%2fhkdf-and-compare-constant-time-in-rust%23new-answer', 'question_page');

);

Post as a guest





























1 Answer
1






active

oldest

votes








1 Answer
1






active

oldest

votes









active

oldest

votes






active

oldest

votes








up vote
2
down vote



accepted










In general you should try and avoid floating point operations for any cryptographically specific code. Floating points have issues with regards to precision, and even I don't see any issue for this specific code, they always require special attention, e.g. during reviewing of the code.



Similarly, using recursive code is strongly discouraged for cryptographic algorithms. If your code can be coded using loops and only local variables you should really do so. I imagine that goes against the idea of Rust (and is thus up for debate), but crypto code generally requires special attention. If you keep to recursive calls make sure you document them as such, both inline and in the function description.



Although you nicely implemented the Drop destructor it makes more sense to create either a class with just a keyInputMaterial (now called data I presume) or simply keep all the data in parameters / local variables. The idea of just keeping the keyInputMaterial is that you may reuse the object to derive other keys using the function but with different salt / info. In that case you obviously need to retrain the Drop to clear the input key material.



Note that the internal key handling (padding, initial hashing) of HMAC is always the same. So a sneaky speed up is to perform pre-calculation on the salt which acts as the HMAC key (mainly useful if there is a lot of output expected, this speedup matters more for e.g. PBKDF2).




Your constant time function looks very much run-of-the-mill constant time, which in this case is great :) I don't think HKDF warrants a function where HMAC(k, x) and HMAC(k, y) are compared using a random key - but now you know about that neat little trick.



Note that you may want to make sure that you document that the comparison needs to be done in constant time or include it in a verify function (beware of truncated values - you did this in your code, but you need to consider it in the calling code as well!).




Note that I'm not a Rust expert, so I won't comment on the vector handling / slicing etc.






share|improve this answer























  • Excuse my overly compressed comment, I have a tough time with character limits. Hope you can see to formatting correctly.
    – user159822
    May 13 at 15:22










  • This is amazing feedback, really. I still have some follow-up questions I hope you would be willing to answer. * Do you have an alternative to CEIL func with floats? * For the recursive code I assume you mean where the extract function is called instead of passing a PRK in expand? * You want me to let the IKM be passed as a function paramter, so that it can be reused instead of Dropped? * The sneaky speed up, I'm not 100% sure what you mean here. You don't happen to have an example? * Am I not taking truncated values into account by comparing equal length vectors?
    – user159822
    May 13 at 15:32










  • You can perform + 1 before division instead of ceil For recursive, I see a call to hkdf_compute from hkdf_expand and vice versa. Yes, that or keep using it as a field, but pass the other fields as arguments. Please have a look at HMAC where the hash is used twice and the intermediate states of HMAC after hashing ipad and opad can be kept (you need to be able to store intermediate hash results though!). Yes, your compare code is fine, but was talking about using it. If you provide a verify function then the user is less likely to use another compare function.
    – Maarten Bodewes
    May 13 at 16:07










  • 1. Fantastic! 2. Understood. 3. Got it. 4. I think I see what you're getting at. I tried pre-padding the password in my PBKDF2 impl and immediately got a ~5 sec performance boost. 5. Right, I have taken care of this then. Thank you!
    – user159822
    May 13 at 17:40















up vote
2
down vote



accepted










In general you should try and avoid floating point operations for any cryptographically specific code. Floating points have issues with regards to precision, and even I don't see any issue for this specific code, they always require special attention, e.g. during reviewing of the code.



Similarly, using recursive code is strongly discouraged for cryptographic algorithms. If your code can be coded using loops and only local variables you should really do so. I imagine that goes against the idea of Rust (and is thus up for debate), but crypto code generally requires special attention. If you keep to recursive calls make sure you document them as such, both inline and in the function description.



Although you nicely implemented the Drop destructor it makes more sense to create either a class with just a keyInputMaterial (now called data I presume) or simply keep all the data in parameters / local variables. The idea of just keeping the keyInputMaterial is that you may reuse the object to derive other keys using the function but with different salt / info. In that case you obviously need to retrain the Drop to clear the input key material.



Note that the internal key handling (padding, initial hashing) of HMAC is always the same. So a sneaky speed up is to perform pre-calculation on the salt which acts as the HMAC key (mainly useful if there is a lot of output expected, this speedup matters more for e.g. PBKDF2).




Your constant time function looks very much run-of-the-mill constant time, which in this case is great :) I don't think HKDF warrants a function where HMAC(k, x) and HMAC(k, y) are compared using a random key - but now you know about that neat little trick.



Note that you may want to make sure that you document that the comparison needs to be done in constant time or include it in a verify function (beware of truncated values - you did this in your code, but you need to consider it in the calling code as well!).




Note that I'm not a Rust expert, so I won't comment on the vector handling / slicing etc.






share|improve this answer























  • Excuse my overly compressed comment, I have a tough time with character limits. Hope you can see to formatting correctly.
    – user159822
    May 13 at 15:22










  • This is amazing feedback, really. I still have some follow-up questions I hope you would be willing to answer. * Do you have an alternative to CEIL func with floats? * For the recursive code I assume you mean where the extract function is called instead of passing a PRK in expand? * You want me to let the IKM be passed as a function paramter, so that it can be reused instead of Dropped? * The sneaky speed up, I'm not 100% sure what you mean here. You don't happen to have an example? * Am I not taking truncated values into account by comparing equal length vectors?
    – user159822
    May 13 at 15:32










  • You can perform + 1 before division instead of ceil For recursive, I see a call to hkdf_compute from hkdf_expand and vice versa. Yes, that or keep using it as a field, but pass the other fields as arguments. Please have a look at HMAC where the hash is used twice and the intermediate states of HMAC after hashing ipad and opad can be kept (you need to be able to store intermediate hash results though!). Yes, your compare code is fine, but was talking about using it. If you provide a verify function then the user is less likely to use another compare function.
    – Maarten Bodewes
    May 13 at 16:07










  • 1. Fantastic! 2. Understood. 3. Got it. 4. I think I see what you're getting at. I tried pre-padding the password in my PBKDF2 impl and immediately got a ~5 sec performance boost. 5. Right, I have taken care of this then. Thank you!
    – user159822
    May 13 at 17:40













up vote
2
down vote



accepted







up vote
2
down vote



accepted






In general you should try and avoid floating point operations for any cryptographically specific code. Floating points have issues with regards to precision, and even I don't see any issue for this specific code, they always require special attention, e.g. during reviewing of the code.



Similarly, using recursive code is strongly discouraged for cryptographic algorithms. If your code can be coded using loops and only local variables you should really do so. I imagine that goes against the idea of Rust (and is thus up for debate), but crypto code generally requires special attention. If you keep to recursive calls make sure you document them as such, both inline and in the function description.



Although you nicely implemented the Drop destructor it makes more sense to create either a class with just a keyInputMaterial (now called data I presume) or simply keep all the data in parameters / local variables. The idea of just keeping the keyInputMaterial is that you may reuse the object to derive other keys using the function but with different salt / info. In that case you obviously need to retrain the Drop to clear the input key material.



Note that the internal key handling (padding, initial hashing) of HMAC is always the same. So a sneaky speed up is to perform pre-calculation on the salt which acts as the HMAC key (mainly useful if there is a lot of output expected, this speedup matters more for e.g. PBKDF2).




Your constant time function looks very much run-of-the-mill constant time, which in this case is great :) I don't think HKDF warrants a function where HMAC(k, x) and HMAC(k, y) are compared using a random key - but now you know about that neat little trick.



Note that you may want to make sure that you document that the comparison needs to be done in constant time or include it in a verify function (beware of truncated values - you did this in your code, but you need to consider it in the calling code as well!).




Note that I'm not a Rust expert, so I won't comment on the vector handling / slicing etc.






share|improve this answer















In general you should try and avoid floating point operations for any cryptographically specific code. Floating points have issues with regards to precision, and even I don't see any issue for this specific code, they always require special attention, e.g. during reviewing of the code.



Similarly, using recursive code is strongly discouraged for cryptographic algorithms. If your code can be coded using loops and only local variables you should really do so. I imagine that goes against the idea of Rust (and is thus up for debate), but crypto code generally requires special attention. If you keep to recursive calls make sure you document them as such, both inline and in the function description.



Although you nicely implemented the Drop destructor it makes more sense to create either a class with just a keyInputMaterial (now called data I presume) or simply keep all the data in parameters / local variables. The idea of just keeping the keyInputMaterial is that you may reuse the object to derive other keys using the function but with different salt / info. In that case you obviously need to retrain the Drop to clear the input key material.



Note that the internal key handling (padding, initial hashing) of HMAC is always the same. So a sneaky speed up is to perform pre-calculation on the salt which acts as the HMAC key (mainly useful if there is a lot of output expected, this speedup matters more for e.g. PBKDF2).




Your constant time function looks very much run-of-the-mill constant time, which in this case is great :) I don't think HKDF warrants a function where HMAC(k, x) and HMAC(k, y) are compared using a random key - but now you know about that neat little trick.



Note that you may want to make sure that you document that the comparison needs to be done in constant time or include it in a verify function (beware of truncated values - you did this in your code, but you need to consider it in the calling code as well!).




Note that I'm not a Rust expert, so I won't comment on the vector handling / slicing etc.







share|improve this answer















share|improve this answer



share|improve this answer








edited May 9 at 21:15


























answered May 9 at 14:06









Maarten Bodewes

417211




417211











  • Excuse my overly compressed comment, I have a tough time with character limits. Hope you can see to formatting correctly.
    – user159822
    May 13 at 15:22










  • This is amazing feedback, really. I still have some follow-up questions I hope you would be willing to answer. * Do you have an alternative to CEIL func with floats? * For the recursive code I assume you mean where the extract function is called instead of passing a PRK in expand? * You want me to let the IKM be passed as a function paramter, so that it can be reused instead of Dropped? * The sneaky speed up, I'm not 100% sure what you mean here. You don't happen to have an example? * Am I not taking truncated values into account by comparing equal length vectors?
    – user159822
    May 13 at 15:32










  • You can perform + 1 before division instead of ceil For recursive, I see a call to hkdf_compute from hkdf_expand and vice versa. Yes, that or keep using it as a field, but pass the other fields as arguments. Please have a look at HMAC where the hash is used twice and the intermediate states of HMAC after hashing ipad and opad can be kept (you need to be able to store intermediate hash results though!). Yes, your compare code is fine, but was talking about using it. If you provide a verify function then the user is less likely to use another compare function.
    – Maarten Bodewes
    May 13 at 16:07










  • 1. Fantastic! 2. Understood. 3. Got it. 4. I think I see what you're getting at. I tried pre-padding the password in my PBKDF2 impl and immediately got a ~5 sec performance boost. 5. Right, I have taken care of this then. Thank you!
    – user159822
    May 13 at 17:40

















  • Excuse my overly compressed comment, I have a tough time with character limits. Hope you can see to formatting correctly.
    – user159822
    May 13 at 15:22










  • This is amazing feedback, really. I still have some follow-up questions I hope you would be willing to answer. * Do you have an alternative to CEIL func with floats? * For the recursive code I assume you mean where the extract function is called instead of passing a PRK in expand? * You want me to let the IKM be passed as a function paramter, so that it can be reused instead of Dropped? * The sneaky speed up, I'm not 100% sure what you mean here. You don't happen to have an example? * Am I not taking truncated values into account by comparing equal length vectors?
    – user159822
    May 13 at 15:32










  • You can perform + 1 before division instead of ceil For recursive, I see a call to hkdf_compute from hkdf_expand and vice versa. Yes, that or keep using it as a field, but pass the other fields as arguments. Please have a look at HMAC where the hash is used twice and the intermediate states of HMAC after hashing ipad and opad can be kept (you need to be able to store intermediate hash results though!). Yes, your compare code is fine, but was talking about using it. If you provide a verify function then the user is less likely to use another compare function.
    – Maarten Bodewes
    May 13 at 16:07










  • 1. Fantastic! 2. Understood. 3. Got it. 4. I think I see what you're getting at. I tried pre-padding the password in my PBKDF2 impl and immediately got a ~5 sec performance boost. 5. Right, I have taken care of this then. Thank you!
    – user159822
    May 13 at 17:40
















Excuse my overly compressed comment, I have a tough time with character limits. Hope you can see to formatting correctly.
– user159822
May 13 at 15:22




Excuse my overly compressed comment, I have a tough time with character limits. Hope you can see to formatting correctly.
– user159822
May 13 at 15:22












This is amazing feedback, really. I still have some follow-up questions I hope you would be willing to answer. * Do you have an alternative to CEIL func with floats? * For the recursive code I assume you mean where the extract function is called instead of passing a PRK in expand? * You want me to let the IKM be passed as a function paramter, so that it can be reused instead of Dropped? * The sneaky speed up, I'm not 100% sure what you mean here. You don't happen to have an example? * Am I not taking truncated values into account by comparing equal length vectors?
– user159822
May 13 at 15:32




This is amazing feedback, really. I still have some follow-up questions I hope you would be willing to answer. * Do you have an alternative to CEIL func with floats? * For the recursive code I assume you mean where the extract function is called instead of passing a PRK in expand? * You want me to let the IKM be passed as a function paramter, so that it can be reused instead of Dropped? * The sneaky speed up, I'm not 100% sure what you mean here. You don't happen to have an example? * Am I not taking truncated values into account by comparing equal length vectors?
– user159822
May 13 at 15:32












You can perform + 1 before division instead of ceil For recursive, I see a call to hkdf_compute from hkdf_expand and vice versa. Yes, that or keep using it as a field, but pass the other fields as arguments. Please have a look at HMAC where the hash is used twice and the intermediate states of HMAC after hashing ipad and opad can be kept (you need to be able to store intermediate hash results though!). Yes, your compare code is fine, but was talking about using it. If you provide a verify function then the user is less likely to use another compare function.
– Maarten Bodewes
May 13 at 16:07




You can perform + 1 before division instead of ceil For recursive, I see a call to hkdf_compute from hkdf_expand and vice versa. Yes, that or keep using it as a field, but pass the other fields as arguments. Please have a look at HMAC where the hash is used twice and the intermediate states of HMAC after hashing ipad and opad can be kept (you need to be able to store intermediate hash results though!). Yes, your compare code is fine, but was talking about using it. If you provide a verify function then the user is less likely to use another compare function.
– Maarten Bodewes
May 13 at 16:07












1. Fantastic! 2. Understood. 3. Got it. 4. I think I see what you're getting at. I tried pre-padding the password in my PBKDF2 impl and immediately got a ~5 sec performance boost. 5. Right, I have taken care of this then. Thank you!
– user159822
May 13 at 17:40





1. Fantastic! 2. Understood. 3. Got it. 4. I think I see what you're getting at. I tried pre-padding the password in my PBKDF2 impl and immediately got a ~5 sec performance boost. 5. Right, I have taken care of this then. Thank you!
– user159822
May 13 at 17:40













 

draft saved


draft discarded


























 


draft saved


draft discarded














StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190204%2fhkdf-and-compare-constant-time-in-rust%23new-answer', 'question_page');

);

Post as a guest













































































Popular posts from this blog

Greedy Best First Search implementation in Rust

Function to Return a JSON Like Objects Using VBA Collections and Arrays

C++11 CLH Lock Implementation