Skip to content

Commit

Permalink
Add EDDSA support (kanidm#408)
Browse files Browse the repository at this point in the history
Cherry-picking notes:
- Introduces the following crates:
  - `ed25519-dalek`: to deal with ed25519 public key
  - `ed448-verifier`: to deal with ed448 public key
- Introduces new variants to `PublicKey`: `ED25519` and `ED448`
- Implements conversions to `PublicKey::ED25519` and `PublicKey::ED448`:
  - from `COSEAlgorithm` and `Certificate`
  - from `COSEKey`
- Appends assertions to the test cases for ed25519, and ed448 to make
  sure `PublicKey` can be created from `COSEKey`.

- I am not sure if the signature verification works
  • Loading branch information
Firstyear authored and kikuomax committed Nov 23, 2024
1 parent 9467b7d commit fd40fe8
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 29 deletions.
2 changes: 1 addition & 1 deletion fido-key-manager/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ async fn main() {
match &public_key.key {
COSEKeyType::EC_OKP(okp) => {
println!(" Octet key pair, curve {:?}", okp.curve);
println!(" X-coordinate: {}", hex::encode(okp.x));
println!(" X-coordinate: {}", hex::encode(&okp.x.0));
}
COSEKeyType::EC_EC2(ec) => {
println!(" Elliptic curve key, curve {:?}", ec.curve);
Expand Down
2 changes: 1 addition & 1 deletion webauthn-authenticator-rs/examples/authenticate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use webauthn_authenticator_rs::ui::{Cli, UiCallback};
use webauthn_authenticator_rs::AuthenticatorBackend;
use webauthn_rs_core::proto::RequestAuthenticationExtensions;
use webauthn_rs_core::WebauthnCore as Webauthn;
use webauthn_rs_proto::{AttestationConveyancePreference, COSEAlgorithm, UserVerificationPolicy};
use webauthn_rs_proto::{AttestationConveyancePreference, UserVerificationPolicy};

#[derive(Debug, clap::Parser)]
#[clap(about = "Register and authenticate test")]
Expand Down
2 changes: 2 additions & 0 deletions webauthn-rs-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ x509-path-finder = { git = "https://github.com/codemonger-io/x509-path-finder.gi
rustls-webpki = { git = "https://github.com/codemonger-io/webpki.git", tag = "v0.101.7-ext.1" }
const-oid = { version = "0.9", features = ["db"] }
rsa = "0.9"
ed25519-dalek = "2.1"
ed448-verifier = { git = "https://github.com/codemonger-io/ed448-verifier.git", tag = "v0.0.1" }

[dev-dependencies]
hex-literal = "0.3"
Expand Down
10 changes: 5 additions & 5 deletions webauthn-rs-core/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,11 +722,13 @@ impl WebauthnCore {
// For us, we return the credential for the caller to persist.
// If trust failed, we have already returned an Err before this point.

// TODO: Associate the credentialId with the transport hints returned by calling
// Associate the credentialId with the transport hints returned by calling
// credential.response.getTransports(). This value SHOULD NOT be modified before or after
// storing it. It is RECOMMENDED to use this value to populate the transports of the
// allowCredentials option in future get() calls to help the client know how to find a
// suitable authenticator.
//
// Done as part of credential construction if the transports are valid/trusted.

Ok(credential)
}
Expand Down Expand Up @@ -3102,8 +3104,7 @@ mod tests {
false,
);
debug!("{:?}", result);
// Currently UNSUPPORTED as openssl doesn't have eddsa management utils that we need.
assert!(result.is_err());
assert!(result.is_ok());
}

#[test]
Expand Down Expand Up @@ -3146,8 +3147,7 @@ mod tests {
false,
);
debug!("{:?}", result);
// Currently UNSUPPORTED as openssl doesn't have eddsa management utils that we need.
assert!(result.is_err());
assert!(result.is_ok());
}

// ⚠️ Currently IGNORED as it appears that pixel 3a send INVALID attestation requests.
Expand Down
182 changes: 163 additions & 19 deletions webauthn-rs-core/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#![allow(non_camel_case_types)]

use core::convert::TryFrom;
use ed25519_dalek::{Verifier as _, VerifyingKey as Ed25519VerifyingKey};
use ed448_verifier::VerifyingKey as Ed448VerifyingKey;
use p256::{
EncodedPoint,
ecdsa::{
Expand Down Expand Up @@ -44,9 +46,14 @@ use x509_parser::prelude::{X509Error, X509Name};
enum PublicKey {
ES256(P256VerifyingKey),
RS256(RsaVerifyingKey<Sha256>),
ED25519(Ed25519VerifyingKey),
ED448(Ed448VerifyingKey),
INSECURE_RS1(&'static str),
}

pub(crate) const PKEY_ALGORITHM_ID_ED25519_OID: &[u8] = &der_parser::oid!(raw 1.3.101 .112);
pub(crate) const PKEY_ALGORITHM_ID_ED448_OID: &[u8] = &der_parser::oid!(raw 1.3.101 .113);

impl TryFrom<(COSEAlgorithm, &Certificate)> for PublicKey {
type Error = WebauthnError;

Expand All @@ -72,13 +79,46 @@ impl TryFrom<(COSEAlgorithm, &Certificate)> for PublicKey {
})
.map(PublicKey::RS256)
}
COSEAlgorithm::EDDSA => {
// determines the actual curve from the algorithm identifier
// in the certificate. reference: RFC 8410, section 3
// https://datatracker.ietf.org/doc/html/rfc8410#section-3
//
// TODO: according to the COSEAlgorithmIdentifier definition
// at https://w3c.github.io/webauthn/#sctn-alg-identifier,
// the algorithm must be ed25519 in this context. do we really
// have to check the actual curve here?
let key_octets = pkey_info.subject_public_key.raw_bytes();
match pkey_info.algorithm.oid.as_bytes() {
PKEY_ALGORITHM_ID_ED25519_OID => {
Ed25519VerifyingKey::try_from(key_octets)
.map_err(|e| {
error!(?e, "ed25519 public key from certificate");
WebauthnError::TransientError("ed25519 public key from certificate")
})
.map(PublicKey::ED25519)
}
PKEY_ALGORITHM_ID_ED448_OID => {
Ed448VerifyingKey::try_from(key_octets)
.map_err(|e| {
error!(?e, "ed448 public key from certificate");
WebauthnError::TransientError("ed448 public key from certificate")
})
.map(PublicKey::ED448)
}
_ => {
Err(WebauthnError::TransientError("unsupported EDDSA algorithm"))
}
}
}
COSEAlgorithm::INSECURE_RS1 => {
Ok(PublicKey::INSECURE_RS1("INSECURE SHA1"))
},
_ => {
error!(
"unsupported signature algorithm: {:?}",
cert.signature_algorithm.oid,
// FIXME: should this be pkey_info.algorithm.oid?
);
Err(WebauthnError::TransientError("unsupported signature algorithm"))
}
Expand All @@ -104,6 +144,26 @@ impl TryFrom<&COSEKey> for PublicKey {
})?;
Ok(PublicKey::ES256(pkey))
}
COSEKeyType::EC_OKP(edk) => {
match edk.curve {
EDDSACurve::ED25519 => {
Ed25519VerifyingKey::try_from(edk.x.0.as_slice())
.map_err(|e| {
error!(?e, "ed25519 public key from COSEKey");
WebauthnError::TransientError("ed25519 public key from COSEKey")
})
.map(PublicKey::ED25519)
}
EDDSACurve::ED448 => {
Ed448VerifyingKey::try_from(edk.x.0.as_slice())
.map_err(|e| {
error!(?e, "ed448 public key from COSEKey");
WebauthnError::TransientError("ed448 public key from COSEKey")
})
.map(PublicKey::ED448)
}
}
}
COSEKeyType::RSA(rsak) => {
let n = BigUint::from_bytes_be(rsak.n.as_ref());
let e = BigUint::from_bytes_be(rsak.e.as_ref());
Expand Down Expand Up @@ -135,9 +195,6 @@ impl TryFrom<&COSEKey> for PublicKey {
}
}
}
_ => {
Err(WebauthnError::COSEKeyInvalidType)
}
}
}
}
Expand Down Expand Up @@ -170,6 +227,24 @@ fn pkey_verify_signature(
let result = pkey.verify_digest(digest, &signature);
Ok(result.is_ok())
}
PublicKey::ED25519(pkey) => {
let signature = ed25519_dalek::Signature::try_from(signature)
.map_err(|e| {
error!(?e, "ed25519 signature decoding");
WebauthnError::TransientError("ed25519 signature decoding")
})?;
let result = pkey.verify(verification_data, &signature);
Ok(result.is_ok())
}
PublicKey::ED448(pkey) => {
let signature = ed448_verifier::Signature::try_from(signature)
.map_err(|e| {
error!(?e, "ed448 signature decoding");
WebauthnError::TransientError("ed448 signature decoding")
})?;
let result = pkey.verify(verification_data, &signature);
Ok(result.is_ok())
}
PublicKey::INSECURE_RS1(_pkey) => {
error!("INSECURE SHA1 USAGE DETECTED");
Err(WebauthnError::CredentialInsecureCryptography)
Expand Down Expand Up @@ -453,13 +528,13 @@ impl TryFrom<&serde_cbor_2::Value> for COSEKey {
// return it
Ok(cose_key)
} else if key_type == (COSEKeyTypeId::EC_OKP as i128) && (type_ == COSEAlgorithm::EDDSA) {
debug!(?d, "WebauthnError::COSEKeyInvalidType - EC_OKP");
// https://datatracker.ietf.org/doc/html/rfc8152#section-13.2

let curve_type_value = m
.get(&serde_cbor_2::Value::Integer(-1))
.ok_or(WebauthnError::COSEKeyInvalidCBORValue)?;
let curve_type = cbor_try_i128!(curve_type_value)?;
let curve = cbor_try_i128!(curve_type_value).and_then(EDDSACurve::try_from)?;

/*
// Some keys may return this as a string rather than int.
// The only key so far is the solokey and it's ed25519 support
Expand All @@ -482,26 +557,22 @@ impl TryFrom<&serde_cbor_2::Value> for COSEKey {
.ok_or(WebauthnError::COSEKeyInvalidCBORValue)?;
let x = cbor_try_bytes!(x_value)?;

if x.len() != 32 {
if x.len() != curve.coordinate_size() {
return Err(WebauthnError::COSEKeyEDDSAXInvalid);
}

let mut x_temp = [0; 32];
x_temp.copy_from_slice(x);

let cose_key = COSEKey {
type_,
key: COSEKeyType::EC_OKP(COSEOKPKey {
curve: EDDSACurve::try_from(curve_type)?,
x: x_temp,
curve,
x: x.to_vec().into(),
}),
};

// The rfc additionally states:
// " Applications MUST check that the curve and the key type are
// consistent and reject a key if they are not."
// this means feeding the values to openssl to validate them for us!

cose_key.validate()?;
// return it
Ok(cose_key)
Expand Down Expand Up @@ -635,6 +706,26 @@ impl COSEKey {
ECDSACurve::SECP521R1 => validate_ec_key!(p521),
}
}
COSEKeyType::EC_OKP(edk) => {
match edk.curve {
EDDSACurve::ED25519 => {
Ed25519VerifyingKey::try_from(edk.x.0.as_slice())
.map_err(|e| {
error!(?e, "validating ed25519 public key");
WebauthnError::TransientError("validating ed25519 public key")
})?;
Ok(())
}
EDDSACurve::ED448 => {
Ed448VerifyingKey::try_from(edk.x.0.as_slice())
.map_err(|e| {
error!(?e, "validating ed448 public key");
WebauthnError::TransientError("validating ed448 public key")
})?;
Ok(())
}
}
}
COSEKeyType::RSA(rsak) => {
let n = BigUint::from_bytes_be(rsak.n.as_ref());
let e = BigUint::from_bytes_be(rsak.e.as_ref());
Expand All @@ -645,10 +736,6 @@ impl COSEKey {
})?;
Ok(())
}
COSEKeyType::EC_OKP(_edk) => {
warn!("ED25519 or ED448 keys are not currently supported");
Err(WebauthnError::COSEKeyEDUnsupported)
}
}
}

Expand Down Expand Up @@ -692,9 +779,14 @@ impl COSEKey {
let p = pkey::PKey::from_rsa(rsa_key).map_err(WebauthnError::OpenSSLError)?;
Ok(p)
}
_ => {
debug!("get_openssl_pkey");
Err(WebauthnError::COSEKeyInvalidType)
COSEKeyType::EC_OKP(edk) => {
let id = match &edk.curve {
EDDSACurve::ED25519 => pkey::Id::ED25519,
EDDSACurve::ED448 => pkey::Id::ED448,
};
pkey::PKey::public_key_from_raw_bytes(&edk.x.0, id)
.map_err(WebauthnError::OpenSSLError)
}
}
}
Expand Down Expand Up @@ -837,4 +929,56 @@ mod tests {
_ => panic!("Key should be parsed EC2 key"),
}
}

#[test]
fn cbor_ed25519() {
let hex_data = hex!(
"
A4 // Map - 4 elements
01 01 // 1: 1, ; kty: OKP key type
03 27 // 3: -8, ; alg: EDDSA signature algorithm
20 06 // -1: 6, ; crv: Ed25519 curve
21 58 20 43565027f918beb00257d112b903d15b93f5cbc7562dfc8458fbefd714546e3c // -2: x, ; Y-coordinate");
let val: Value = serde_cbor_2::from_slice(&hex_data).unwrap();
let key = COSEKey::try_from(&val).unwrap();
assert_eq!(key.type_, COSEAlgorithm::EDDSA);
match &key.key {
COSEKeyType::EC_OKP(pkey) => {
assert_eq!(
pkey.x.as_ref(),
hex!("43565027f918beb00257d112b903d15b93f5cbc7562dfc8458fbefd714546e3c")
);
assert_eq!(pkey.curve, EDDSACurve::ED25519);
}
_ => panic!("Key should be parsed OKP key"),
}

assert!(PublicKey::try_from(&key).is_ok());
}

#[test]
fn cbor_ed448() {
let hex_data = hex!(
"
A4 // Map - 4 elements
01 01 // 1: 1, ; kty: OKP key type
03 27 // 3: -8, ; alg: EDDSA signature algorithm
20 07 // -1: 7, ; crv: Ed448 curve
21 58 39 0c04658f79c3fd86c4b3d676057b76353126e9b905a7e204c07846c1a2ab3791b02fc5e9c6930345ea7bf8524b944220d4bd711c010c9b2a80 // -2: x, ; Y-coordinate");
let val: Value = serde_cbor_2::from_slice(&hex_data).unwrap();
let key = COSEKey::try_from(&val).unwrap();
assert_eq!(key.type_, COSEAlgorithm::EDDSA);
match &key.key {
COSEKeyType::EC_OKP(pkey) => {
assert_eq!(
pkey.x.as_ref(),
hex!("0c04658f79c3fd86c4b3d676057b76353126e9b905a7e204c07846c1a2ab3791b02fc5e9c6930345ea7bf8524b944220d4bd711c010c9b2a80")
);
assert_eq!(pkey.curve, EDDSACurve::ED448);
}
_ => panic!("Key should be parsed OKP key"),
}

assert!(PublicKey::try_from(&key).is_ok());
}
}
12 changes: 11 additions & 1 deletion webauthn-rs-core/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ pub enum EDDSACurve {
ED448 = 7,
}

impl EDDSACurve {
/// Returns the size in bytes of the coordinate for the specified curve
pub(crate) fn coordinate_size(&self) -> usize {
match self {
Self::ED25519 => 32,
Self::ED448 => 57,
}
}
}

/// An ECDSACurve identifier. You probably will never need to alter
/// or use this value, as it is set inside the Credential for you.
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
Expand Down Expand Up @@ -140,7 +150,7 @@ pub struct COSEOKPKey {
/// The curve that this key references.
pub curve: EDDSACurve,
/// The key's public X coordinate.
pub x: [u8; 32],
pub x: Base64UrlSafeData,
}

/// A COSE RSA PublicKey. This is a provided credential from a registered
Expand Down
2 changes: 0 additions & 2 deletions webauthn-rs-proto/src/attest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,6 @@ impl From<web_sys::PublicKeyCredential> for RegisterPublicKeyCredential {
fn from(data: web_sys::PublicKeyCredential) -> RegisterPublicKeyCredential {
use js_sys::Uint8Array;

// is_user_verifying_platform_authenticator_available

// AuthenticatorAttestationResponse has getTransports but web_sys isn't exposing it?
let transports = None;

Expand Down
2 changes: 2 additions & 0 deletions webauthn-rs-proto/src/cose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ impl COSEAlgorithm {
COSEAlgorithm::RS256,
// COSEAlgorithm::RS384,
// COSEAlgorithm::RS512
// -- Testing required
// COSEAlgorithm::EDDSA,
]
}

Expand Down

0 comments on commit fd40fe8

Please sign in to comment.