Closed gabi-250 closed 2 years ago
Good catch.
The Zig implementation doesn't have this issue since it defines a bn2binPadded()
function for this.
Instead of adding zero_left_pad()
calls, maybe it would be less error-prone to do something similar, and define a to_bytes_be_padded()
function instead. That can be implemented as a trait.
@jedisct1 Thank you for the review!
I addressed your comment in a separate commit, but I'm happy to squash it before merging.
Looking good!
Thank you so much for this, Gabi!
I noticed
pk.finalize()
occasionally (and for no apparent reason) returns anUnsupportedParameters
error, instead of theOk
result I would expect.I don't have a minimal reproducible example I can share yet, but here are the steps I took to reproduce the issue:
pk.blind()
to create a blinded message for a recipient whose public key ispk
BlindedSignature
pk.finalize(...)
<--- this will occasionally fail the verification:sig.len() != modulus_bytes
(in my case,sig.len() = 511
andmodulus_bytes = 512
): https://github.com/jedisct1/rust-blind-rsa-signatures/blob/622b6a5949544a35c9d5913ddb599a7fe6de5cfa/src/lib.rs#L357-L359This is what I think happens:
blind_sig.len() == modulus_bytes
blind_sig.len() == modulus_bytes
at the beginning offinalize()
!BigUint
, with some transformations in between (i.e.unblind
), doesn't guarantee the output vector has the same length as the input one, becauseBigUint
will strip away any leading zeroes from the representation of the number: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=80d02761702f676119fedbb9a5ddd845This patch fixes the problem for me (the solution seems to be to pad any vectors obtained using
to_bytes_be
to have a length ofmodulus_bytes
).