lawliet89/biscuit

Fill in for `ring`'s deficiencies

lawliet89 opened this issue ยท 12 comments

openssl or others.

Maybe gate these behind a feature switch.

Current ring deficiencies that will be addressed:

Not in ring, but "nice to have" for biscuit:

  • RSA Key handling (construct JWK to and from DER/PEM)

Not in ring, but I am not sure biscuit needs it:

  • ECDSA using P-521 and SHA-512
  • RSA1_5
  • RSA_OAEP
  • A128KW, A192KW, A256KW
  • A192GCMKW
  • ECDH-ES+A128KW, ECDH-ES+A192KW, ECDH-ES+A256KW
  • PBES2_HS256_A128KW
  • PBES2_HS384_A192KW
  • PBES2_HS512_A256KW
  • A128CBC_HS256
  • A192CBC_HS384
  • A256CBC_HS512

Hi @lawliet89,

I started playing with porting the crypto of biscuit to openssl (you can see the result here: https://gitlab.mjdsystems.ca/MJDSys/biscuit-openssl ). I did that as I wanted to tap some crypto currently available in openssl not yet available in Ring. So far I'm just playing with what I need for my projects, but I'd be happy to help bring these changes back into biscuit (behind a feature gate is fine!) if you would like. Do you have any idea of what you would like this support to look like? I was thinking maybe putting all the crypto behind some set of traits, so a user could substitute in their own preferred crypto library in the future, though I fear that might make simple use of the library hard.

One easy thing I could submit as a PR here is removing the constant time equality checks in the unit tests. As they are tests, I don't think we need that to be constant time as they are just tests. I wouldn't change the "real" code, as that would have security implications.

I think gating it behind a feature gate would be fine. The traits idea might be too complicated, you're right. Eventually, I'd like to not use OpenSSL at all and just use everything that ring has to offer.

Sure, a PR would be great if you're up for it.

@lawliet89 Sorry, somehow Github never poked me about this comment. I'll see what I can do.

Thanks @MJDSys. Maybe I have to @ ping you to notify you.

Is there an ugly way to get an rsa pubkey as Vec<u8> from n and e out of a jwk right now without pulling in openssl? I'm going to need to generate rsa keys from JWKs in my work, so if nobody else has a solution I'll bite the bullet.

Is there an ugly way to get an rsa pubkey as Vec from n and e out of a jwk right now without pulling in openssl? I'm going to need to generate rsa keys from JWKs in my work, so if nobody else has a solution I'll bite the bullet.

If you have n and e from a JWK you can use ring::signature::primitive::verify_rsa to verify the signature directly from them.

If you have n and e and you need to convert them into SPKI form, ring doesn't have any support for encoding the SPKI form now. Similarly, if you have the SPKI form and you need to extract n and e then there's an open PR to add an API for that to ring but I've just not had time to review, update, and merge it. If you know of a potential sponsor for that work then email me at brian@briansmith.org. Similarly for the other features mentioned in the first message above.

Current ring deficiencies that will be addressed:

ECDSA signing (briansmith/ring#207 and briansmith/ring#209)

ECDSA signing is in ring 0.13.0-alpha4.

Hey!
I was recently looking for ES512 and I noticed that ring is not planning to support it.
Would there still be an interest to support it via openssl behind a feature gate or does it add too much complexity to biscuit?

Or could we move from ring to RustCrypto, they have rsa and elliptic-curve. If acceptable, I'll try to do so.

ring seems to have many issues and mostly appears to be unmaintained, I think the 3 main other options are the "openssl", "native-tls", or RustCrypto crates so I think it would make sense to look at moving to one of those given that the last stable release of ring was over 2 years ago which means it has 2+ years worth of BoringSSL CVEs unfixed.

the last stable release of ring was over 2 years ago which means it has 2+ years worth of BoringSSL CVEs unfixed.

AFAIK, there were no bugs inherited from BoringSSL that required fixes to be backported to the stable release. That's a good thing.