Convert DER formatted signature to raw before exporting and raw forma…#2
Convert DER formatted signature to raw before exporting and raw forma…#2BenjaminWareham wants to merge 4 commits intomasterfrom
Conversation
…tted signature to der before verifying
| }; | ||
|
|
||
| sign(data, key, stp) | ||
| let mut signer = Signer::new(stp, &key)?; |
There was a problem hiding this comment.
You should add a comment here explaining what you are doing and why (signatures are not compatible with other JWT implementations).
Link to the issue you found, and the Python implementation.
johnpbatty
changed the title
johnpbatty
left a comment
johnpbatty
left a comment
There was a problem hiding this comment.
My main concern is the "magic" encode/decode functions using hard-coded offsets into vectors. Would be cleaner if you could use a DER lib to do the decoding/encoding.
src/lib.rs
Outdated
| res == 0 | ||
| } | ||
|
|
||
| fn der_to_raw_signature(der_sig: Vec<u8>) -> Vec<u8>{ |
There was a problem hiding this comment.
You are taking ownership of the der_sig parameter here, which you don't really need to do. Would be better to take a reference (&Vec<u8>), or even better to take a reference to a slice (&[u8])
src/lib.rs
Outdated
|
|
||
| fn der_to_raw_signature(der_sig: Vec<u8>) -> Vec<u8>{ | ||
| let len_r = der_sig[3] as usize; | ||
| let len_s = der_sig[len_r + 5] as usize; |
There was a problem hiding this comment.
There's a lot of magic going on in here, with hard-coded constants. At a minimum you need to add some comments to explain what is going on. Have you looked to see whether there are any library functions to extract raw signatures from DER (in OpenSSL, perhaps?).
src/lib.rs
Outdated
| let mut raw_sig: Vec<u8> =vec![]; | ||
| if der_sig[4] == 0 { | ||
| let mut r = der_sig[5..len_r + 4].to_vec(); | ||
| raw_sig.append(&mut r); |
There was a problem hiding this comment.
Here and below, better to avoid the mut variable, and use:
raw_sig.extend_from_slice(der_sig[5..len_r + 4])
src/lib.rs
Outdated
| return raw_sig | ||
| } | ||
|
|
||
| fn raw_to_der_signature(raw_sig: Vec<u8>) -> Vec<u8>{ |
There was a problem hiding this comment.
Again, make the parameter a slice reference: &[u8]
src/lib.rs
Outdated
|
|
||
| fn raw_to_der_signature(raw_sig: Vec<u8>) -> Vec<u8>{ | ||
| let mut r: Vec<u8> = raw_sig[0..raw_sig.len()/2].to_vec(); | ||
| let mut s: Vec<u8> = raw_sig[raw_sig.len()/2..].to_vec(); |
There was a problem hiding this comment.
Again, the magic needs explanation, or replacing with a library function.
src/lib.rs
Outdated
| b2 = r.len()+1; | ||
| let mut r2 = vec![0]; | ||
| r2.append(&mut r); | ||
| r = r2; |
There was a problem hiding this comment.
Look like you are prepending a zero? Replace all of this with r.insert(0, 0). Rather than have a mut b2 that you update, would be better to just assign it after this point with r.len()
src/lib.rs
Outdated
| r = r2; | ||
| } | ||
|
|
||
| if s[0]>127{ |
There was a problem hiding this comment.
Formatting - add spaces as before
src/lib.rs
Outdated
| b3 = s.len() + 1; | ||
| let mut s2 = vec! [0]; | ||
| s2.append(&mut s); | ||
| s = s2; |
There was a problem hiding this comment.
Use s.insert(...) as before.
src/lib.rs
Outdated
| let mut der_sig: Vec<u8> = vec![48, b1 as u8, 2, b2 as u8]; | ||
| der_sig.append(&mut r); | ||
| der_sig.append(&mut vec![2, b3 as u8]); | ||
| der_sig.append(&mut s); |
There was a problem hiding this comment.
I think you should use Vec extend() rather than append() for all of the above - as the parameter does not need to be mutable. In general, avoid mutability unless you really need it!
src/lib.rs
Outdated
|
|
||
| fn der_to_raw_signature(der_sig: Vec<u8>) -> Vec<u8>{ | ||
| let len_r = der_sig[3] as usize; | ||
| let len_s = der_sig[len_r + 5] as usize; |
There was a problem hiding this comment.
Some DER parser/encoder crates:
2 participants
…tted signature to der before verifying