Fix RSA signatures

[dewyatt]

Hopefully closes https://github.com/riboseinc/rnp/issues/79

I think the main piece I'm not sure on here is the removal of the legacy code, but it was necessary to get rnp --verify to agree with the other tools.
rnp/rnpv/gpg now all agree that the signatures are valid (and invalid when tampered with).

I think the crux of the issue here is a mismatch in parameter names in botan_privkey_load_rsa vs RSA_PrivateKey, so I filed a PR there: randombit/botan#1045

[ronaldtse]

@randombit could you help check if the fix is appropriate? Thanks @dewyatt !

[randombit]

Re the check you had to remove: PKCS signature formatting is 0001FFFF.... but how Botan implements unpadded RSA signatures the leading 00 byte is removed. I think the correct check here is changing the test to if(hashbuf_from_sig[0] != 1) and modifying the following loop to start looking at index 1 instead of 2.

[codecov-io]

Codecov Report

Merging #82 into master will increase coverage by 1.22%.
The diff coverage is 41.66%.

@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
+ Coverage   21.29%   22.52%   +1.22%     
==========================================
  Files          24       24              
  Lines        8939     8994      +55     
  Branches       10       10              
==========================================
+ Hits         1904     2026     +122     
+ Misses       7031     6964      -67     
  Partials        4        4

Impacted Files	Coverage Δ
src/lib/rsa.c	62.04% <37.5%> (-1.93%)	⬇️
src/lib/signature.c	25.33% <50%> (+0.05%)	⬆️
src/lib/packet-parse.c	24.06% <0%> (-0.13%)	⬇️
src/lib/packet-print.c	6.94% <0%> (-0.06%)	⬇️
src/lib/dsa.c	0% <0%> (ø)	⬆️
src/lib/packet-show.c	1.19% <0%> (ø)	⬆️
src/rnp/rnp.c	20.8% <0%> (+0.46%)	⬆️
src/cmocka/rnp_tests.c	95.48% <0%> (+0.55%)	⬆️
src/lib/create.c	29.52% <0%> (+0.64%)	⬆️
src/lib/reader.c	7.46% <0%> (+0.78%)	⬆️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1e2e5a...9e56883. Read the comment docs.

[dewyatt]

@randombit Thanks! Ok I see that makes sense from the RFC (EM = 0x00 || 0x01 || PS || 0x00 || T.) so I added that check back in.

Anyways, it took me a while to finally realize it, but it looks like the issue was something different.
I think the main culprit here was that we were relying on uninitialized memory in the verification code.

$ src/rnp/rnp --verify inputfile.gpg 
sigbuf: 
9B F3 19 9C 4E 8C CE 4C AD 01 62 82 E0 F1 46 B8 A1 0F 0C 9D 6E 5E BD 24 6E EC 47 EE FB 6D 
E3 26 BE 94 CE 74 39 AC 58 FD 8B F0 31 EC 75 A4 52 A9 B7 31 33 90 0C D2 06 2B 72 85 9A 9A 
35 B4 E6 A0 74 AE AD B9 FB 4C DE 33 64 BD CB 9A EB 03 56 EF 66 A1 B8 FB 17 0F 70 36 D5 C0 
EE 63 EE 35 7B 44 1C 5E 2D 3E 72 4D 7F EB 3D 34 D8 C5 A2 1C EA 98 33 68 D1 8C C1 66 62 AB 
55 F1 0A FF 50 42 D7 36 

hashbuf_from_sig: 
20 01 FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 
FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 
FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF 00 30 31 30 0D 06 09 60 86 48 01 65 03 04 
02 01 05 00 04 20 62 9E 85 6F 0A DD 1F 4B 06 CE 4F 5E 86 38 51 B7 75 25 C6 0A F4 77 1E B5 
CE 23 DE 1C A4 C8 56 C3 

That first byte in hashbuf_from_sig is uninitialized memory (so it tended to fail that check I removed).
I added a commit to zero the buffer in pgp_rsa_public_decrypt so that we're not returning any random garbage in the buffer.

Can you give it another quick look and let me know if it looks good to you?

[randombit]

Argh yes uninitialized memory would do it, sorry about that - what you have here looks good to me.