search

cedarcode / cose-ruby

Ruby implementation of RFC 8152 CBOR Object Signing and Encryption (COSE)
https://rubygems.org/gems/cose
MIT License
16 stars 10 forks source link

Add EdDSA support #55

Closed bdewater closed 2 years ago

bdewater commented 4 years ago

This relies generic pkey support currently in OpenSSL gem master, slated to be released as 3.0 (https://github.com/ruby/openssl/pull/329). Closes https://github.com/cedarcode/cose-ruby/issues/48

EdDSA in COSE is 'pure' and takes no hash: https://tools.ietf.org/html/rfc8152#section-8.2

The ASN1 structure is described in https://tools.ietf.org/html/rfc5958 and some experimentation of returned values by the OpenSSL bindings using https://lapo.it/asn1js/

bdewater commented 4 years ago

Marked this as draft for now. While the fiddling with ASN1 works I think the OpenSSL gem would benefit from exposing some library functions. I posted a message upstream.

grzuy commented 4 years ago

Nice! :clap:

SamSaffron commented 2 years ago

@bdewater curious on what is blocking this, perhaps it can be merged as an "optional" protocol and users that want to consume it need to make sure they have the right version of openssl installed?

I am noticing some failures in the wild now https://github.com/solokeys/solo1 is -8 by default.

SamSaffron commented 2 years ago

I wonder if another option is leaning on rbnacl? would that make distribution easier?

bdewater commented 2 years ago

I forgot about this! I'll rebase it later today and see where things stand with OpenSSL gem 3.0 now that it's out.

I am noticing some failures in the wild now https://github.com/solokeys/solo1 is -8 by default.

🤔 doesn't it support -7 (ES256)? IIRC that's required for authenticators by the WebAuthn specification, you shouldn't see -8 unless you specify it in pubKeyCredParams?

SamSaffron commented 2 years ago

doesn't it support -7 (ES256)? IIRC that's required for authenticators by the WebAuthn specification, you shouldn't see -8 unless you specify it in pubKeyCredParams?

Great call @bdewater ...

@gschlager did some research and it turns out that dropping -258 and -259 from our list resolves the issue, so the issue we were experiencing was not even due to -8

bdewater commented 2 years ago

Rebased, I'll try this branch in the latest alpha of webauthn-ruby and with a modern Yubikey in the next few days to make sure it's all working as expected. I'm a little hazy on some details here after 2 years 😅

SamSaffron commented 2 years ago

Thanks so much @bdewater !

bdewater commented 2 years ago

@gschlager did some research and it turns out that dropping -258 and -259 from our list resolves the issue, so the issue we were experiencing was not even due to -8

If that's a bug in this gem, I'd appreciate a bug report (or confirmation it wasn't) 😄 . I don't know which authenticators support -258 and -259 at the moment.

bdewater commented 2 years ago

Thank you! The demo app was giving me some trouble to get working and I didn't have the bandwidth to fix it (or build a new one, it's been on my wish list 😁). I committed your suggestions :)

Worth noting that this same key works correctly – both for registration and authentication – if I remove the restriction for the relying party to only accept EdDSA.

You probably got an ES256 (-7) key in that case.

brauliomartinezlm commented 2 years ago

@bdewater @santiagorodriguez96 feel free to merge this at this point when you think it's ready. I'm happy to cut a release with this before EOW (going on vacation for 10 days on Saturday) after it's merged.

Thank you for doing this @bdewater and @santiagorodriguez96 for helping on the testing and tweaking! ❤️

bdewater commented 2 years ago

@brauliomartinezlm I can't merge in this repo - and have a good vacation! :)

brauliomartinezlm commented 2 years ago

I can't merge in this repo

@bdewater apologies. Just sent you an invite 🙂

and have a good vacation! :)

Thank you!

I'll do release tomorrow for this!