Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Non-EC keys for COSE signatures #627

Open
mmartinv opened this issue Feb 22, 2024 · 11 comments
Open

Support Non-EC keys for COSE signatures #627

mmartinv opened this issue Feb 22, 2024 · 11 comments
Labels
bug Something isn't working conformance-test Issues related to passing the FIDO Aliance conformance tests jira spec-nonconformity Bits where we aren't following the exact specification

Comments

@mmartinv
Copy link
Contributor

Currently the library used to handle COSE signatures supports EC keys only, we need to support also RSA keys to be conformant with the FIDO specification.

@mmartinv mmartinv added bug Something isn't working spec-nonconformity Bits where we aren't following the exact specification jira conformance-test Issues related to passing the FIDO Aliance conformance tests labels Feb 22, 2024
@7flying
Copy link
Contributor

7flying commented Mar 5, 2024

We should consider: https://docs.rs/cose-rust/latest/cose/

@nullr0ute
Copy link
Contributor

@puiterwijk do you remember if there were specific features we needed as to why we chose aws-nitro-enclaves-cose crate when we implemented this?

@puiterwijk
Copy link
Contributor

@nullr0ute because we needed a cose library that uses OpenSSL for the crypto. And this was basically the only one that was at least somewhat mature at the time that I could find.

@7flying
Copy link
Contributor

7flying commented Mar 5, 2024

Good point @puiterwijk , at this point in time cose-rust also uses rust-openssl, so we should be good FIPS-compliance wise too.

@nullr0ute
Copy link
Contributor

nullr0ute commented Mar 5, 2024

We should consider: https://docs.rs/cose-rust/latest/cose/

So digging into "cose-rust" a little there seems to be two projects by the same name.

The first is here and is the one that comes up in crates.io here, I have concerns about that as the last release was 6 years ago and it uses NSS (not openssl).

The second is here and appears to be the one referenced above as they are both at release 0.1.7 (although not tagged in github). It seems it's a thesis project so I am not sure how responsive they will be on bugs and even if it will continue to be developed post thesis.

In both cases I have concerns about support looking forward, are there other possibilities?

@7flying
Copy link
Contributor

7flying commented Mar 5, 2024

@nullr0ute I've found this one from Google: coset, it might be more stable, but we'll need to investigate further to check if it suits our needs. I've seen references to EC and RSA keys in the code-

@miabbott
Copy link
Contributor

miabbott commented Mar 5, 2024

@7flying @nullr0ute If we are concerned about which Rust crate to use re: OpenSSL support, could we reach out to either the crypto team or the Rust team within Red Hat about this?

@nullr0ute
Copy link
Contributor

@7flying @nullr0ute If we are concerned about which Rust crate to use re: OpenSSL support, could we reach out to either the crypto team or the Rust team within Red Hat about this?

So we know from that team we need something that supports OpenSSL for the underlying crypto. The problem here is actually finding a COSE crate that 1) supports the features we need 2) has an active upstream community maintaining it.

@miabbott
Copy link
Contributor

miabbott commented Mar 5, 2024

So we know from that team we need something that supports OpenSSL for the underlying crypto. The problem here is actually finding a COSE crate that 1) supports the features we need 2) has an active upstream community maintaining it.

Right, I'm suggesting they may already know of one and could save us some research.

@7flying
Copy link
Contributor

7flying commented Apr 2, 2024

List of functions to change/refine:

  • COSE SignInner::new
  • COSESignInner::new_with_protected
  • COSESIgninner::serialize_data
  • COSESIgninner::verify_signature(SigningPublicKey)
  • COSESIgninner::get_payload
  • COSESIgninner::get_protected_and_payload
  • COSESIgninner::get_unprotected

@7flying
Copy link
Contributor

7flying commented Apr 8, 2024

on top of that serialize_data needs to implement the low-level serde::Serialize, but coset internally uses ciborium, so rather than implementing that trait, we need to switch from serde_cbor to ciborium.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working conformance-test Issues related to passing the FIDO Aliance conformance tests jira spec-nonconformity Bits where we aren't following the exact specification
Projects
None yet
Development

No branches or pull requests

5 participants