mirleft / ocaml-nocrypto

OCaml cryptographic library
ISC License
111 stars 53 forks source link

Implement RSASSA-PKCS1-V1_5-SIGN/VERIFY with EMSA-PKCS1-v1.5-ENCODE #121

Closed cfcs closed 7 years ago

cfcs commented 7 years ago

I had a go at this since I need it, open to suggestions or refactoring.

Addresses https://github.com/mirleft/ocaml-nocrypto/issues/119

cfcs commented 7 years ago

Have some questions:

1) The calculation of minimum key size is done here and here. The cdiv rounding-up means that we accept keys that are smaller than the minimum length (hence the -7). Example: For SHA512 I would expect the minimum key size to be 752 bits: 512 + len(asn1 stuff == 19*8 == 152) + (min_pad == 11*8 == 88), which is 94 bytes, but due to the rounding up we accept key sizes down to 745 bits (744 and below are rejected with an Insufficient_key exception). This seems counter-intuitive to me, but the self-verification works, so I kept it like this. Is it intentional? I couldn't find a third-party RSA+PKCS1 library where it was easy to generate keys with strange sizes, so I haven't tested interoperability with other implementations.

2) I'm not currently using constant-time operations for comparing the structs, I guess I should?

cfcs commented 7 years ago

The Travis check for 4.03 fails because of a missing dependency on ppx_deriving: https://travis-ci.org/mirleft/ocaml-nocrypto/jobs/233434937#L914

cfcs commented 7 years ago

I think I fixed the link between Hash.S.t and Rsa.PKCS1.S.t now.

Merging Hannes' dup PR first, and adding support for that here, would allow me to not destroy the Hash.S.t when signing (that would be awesome): https://github.com/mirleft/ocaml-nocrypto/pull/125

pqwy commented 7 years ago

Re: rounded-up key sizes: the first byte of PKCS paddings is 0x00 -- I presume for this reason exactly -- so the key can be up to 7 bits smaller than the padded message while the message still denotes a number smaller than the modulus.

It's total, delightful, pedantry because I doubt anyone ever uses keys with sizes not divisible by 8.

cfcs commented 7 years ago

Ah, that makes sense!

pqwy commented 7 years ago

👋