Skip to content

Commit

Permalink
Upgrade k256/p256/p384/ed25519-dalek; remove ring/signatory/secp256k1 (
Browse files Browse the repository at this point in the history
…#91)

This commit removes Signatory and its associated wrapper crates (for
*ring* and the `secp256k1` crates, which use native code bindings).

They can now be completely replaced by the `k256`, `p256`, and `p384`
crates, along with `ed25519-dalek`, which natively implement the traits
from the `signature` crate that Signatory originally provided.

The result is fewer overall dependencies and faster compile times (owing
to the elimination of C/ASM dependencies)

Additionally, this PR pulls in the `ccm` crate and begins to properly
implement key/object wrapping (i.e. encryption) using the same
algorithms as the YubiHSM2 itself.
  • Loading branch information
tony-iqlusion authored Sep 23, 2020
1 parent d78e28e commit 45bcbd6
Show file tree
Hide file tree
Showing 31 changed files with 890 additions and 629 deletions.
463 changes: 282 additions & 181 deletions Cargo.lock

Large diffs are not rendered by default.

24 changes: 15 additions & 9 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,26 @@ aes = "0.5"
anomaly = "0.2"
bitflags = "1"
block-modes = "0.6"
ccm = { version = "0.2", optional = true, features = ["std"] }
chrono = { version = "0.4", features = ["serde"], optional = true }
cmac = "0.4"
getrandom = "0.1"
digest = { version = "0.9", optional = true, default-features = false }
ecdsa = { version = "0.8", default-features = false }
ed25519 = "1"
ed25519-dalek = { version = "1", optional = true }
harp = { version = "0.1", optional = true }
hmac = { version = "0.9", optional = true }
k256 = { version = "0.5", optional = true, features = ["ecdsa", "keccak256", "sha256"] }
log = "0.4"
p256 = { version = "0.5", default-features = false, features = ["ecdsa-core"] }
p384 = { version = "0.4", default-features = false, features = ["ecdsa"] }
pbkdf2 = { version = "0.5", optional = true, default-features = false }
serde = { version = "1", features = ["serde_derive"] }
serde_json = { version = "1", optional = true }
ring = { version = "0.16", optional = true, default-features = false }
rand_core = { version = "0.5", features = ["std"] }
rusb = { version = "0.6", optional = true }
secp256k1 = { version = "0.17", optional = true }
sha2 = { version = "0.9", optional = true }
signatory = { version = "0.20", features = ["digest", "ecdsa", "ed25519", "k256", "p256", "p384"] }
signature = { version = "1.1.0", features = ["derive-preview"] }
signature = { version = "1", features = ["derive-preview"] }
subtle = "2"
thiserror = "1"
tiny_http = { version = "0.7", optional = true }
Expand All @@ -43,23 +48,24 @@ zeroize = { version = "1", features = ["zeroize_derive"] }

[dev-dependencies]
criterion = "0.3"
ed25519-dalek = "1"
lazy_static = "1"
ring = { version = "0.16", default-features = false }
signatory-ring = "0.20"
signatory-secp256k1 = "0.20"
p256 = { version = "0.5", features = ["ecdsa"] }

[features]
default = ["http", "passwords", "setup"]
http-server = ["tiny_http"]
http = ["harp"]
mockhsm = ["passwords", "ring"]
mockhsm = ["ccm", "digest", "ed25519-dalek", "p256/ecdsa", "secp256k1"]
passwords = ["hmac", "pbkdf2", "sha2"]
secp256k1 = ["k256"]
setup = ["chrono", "passwords", "serde_json", "uuid/serde"]
untested = ["sha2"]
usb = ["rusb"]

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]

[[bench]]
name = "ed25519"
Expand Down
46 changes: 27 additions & 19 deletions src/asymmetric/public_key.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
//! Public keys for use with asymmetric cryptography / signatures
use crate::{
asymmetric,
ecdsa::{self, algorithm::CurveAlgorithm},
ed25519,
use crate::{asymmetric, ecdsa::algorithm::CurveAlgorithm, ed25519};
use ::ecdsa::{
elliptic_curve::{
sec1::{self, UncompressedPointSize, UntaggedPointSize},
weierstrass::{point, Curve},
},
generic_array::{
typenum::{Unsigned, U1},
ArrayLength, GenericArray,
},
};
use serde::{Deserialize, Serialize};
use signatory::ecdsa::{
curve::{CompressedPointSize, UncompressedPointSize},
generic_array::{typenum::U1, ArrayLength, GenericArray},
Curve,
};
use std::ops::Add;

/// Response from `command::get_public_key`
Expand Down Expand Up @@ -52,19 +53,26 @@ impl PublicKey {
}

/// Return the ECDSA public key of the given curve type if applicable
pub fn ecdsa<C>(&self) -> Option<ecdsa::PublicKey<C>>
pub fn ecdsa<C>(&self) -> Option<sec1::EncodedPoint<C>>
where
C: Curve + CurveAlgorithm,
<C::ScalarSize as Add>::Output: Add<U1> + ArrayLength<u8>,
CompressedPointSize<C::ScalarSize>: ArrayLength<u8>,
UncompressedPointSize<C::ScalarSize>: ArrayLength<u8>,
C: Curve + CurveAlgorithm + point::Compression,
UntaggedPointSize<C>: Add<U1> + ArrayLength<u8>,
UncompressedPointSize<C>: ArrayLength<u8>,
{
if self.algorithm == C::asymmetric_algorithm() {
Some(ecdsa::PublicKey::from_untagged_point(
GenericArray::from_slice(&self.bytes),
))
if self.algorithm != C::asymmetric_algorithm()
|| self.bytes.len() != C::FieldSize::to_usize() * 2
{
return None;
}

let mut bytes = GenericArray::default();
bytes.copy_from_slice(&self.bytes);
let result = sec1::EncodedPoint::from_untagged_bytes(&bytes);

if C::COMPRESS_POINTS {
Some(result.compress())
} else {
None
Some(result)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/authentication/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
use super::{Error, ErrorKind};
use anomaly::ensure;
use getrandom::getrandom;
#[cfg(feature = "hmac")]
use hmac::Hmac;
#[cfg(feature = "pbkdf2")]
use pbkdf2::pbkdf2;
use rand_core::{OsRng, RngCore};
#[cfg(feature = "sha2")]
use sha2::Sha256;
use std::fmt::{self, Debug};
Expand Down Expand Up @@ -36,7 +36,7 @@ impl Key {
/// Generate a random `Key` using `OsRng`.
pub fn random() -> Self {
let mut challenge = [0u8; SIZE];
getrandom(&mut challenge).expect("RNG failure!");
OsRng.fill_bytes(&mut challenge);
Key(challenge)
}

Expand Down
29 changes: 12 additions & 17 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
connector::Connector,
device::{self, commands::*, StorageInfo},
domain::Domain,
ecdsa::{self, commands::*},
ecdsa::commands::*,
ed25519::{self, commands::*},
hmac::{self, commands::*},
object::{self, commands::*, generate},
Expand Down Expand Up @@ -903,27 +903,22 @@ impl Client {
///
/// <https://developers.yubico.com/YubiHSM2/Commands/Sign_Ecdsa.html>
///
/// # secp256k1 notes
/// # Security Warning
///
/// The YubiHSM 2 does not produce signatures in "low S" form, which is expected
/// for most cryptocurrency applications (the typical use case for secp256k1).
/// This is a low-level ECDSA API, and if used incorrectly could potentially
/// result in forgeable signatures.
///
/// If your application demands this (e.g. Bitcoin), you'll need to normalize
/// the signatures. One option for this is the `secp256k1` crate's
/// [Signature::normalize_s] function.
///
/// Normalization functionality is built into the `yubihsm::signatory` API
/// found in this crate (when the `secp256k1` feature is enabled).
pub fn sign_ecdsa<T>(&self, key_id: object::Id, digest: T) -> Result<ecdsa::Signature, Error>
/// We recommend using the [`ecdsa::Signer`] type instead, which provides a
/// high-level, well-typed, misuse resistant API.
pub fn sign_ecdsa_prehash_raw<T>(&self, key_id: object::Id, digest: T) -> Result<Vec<u8>, Error>
where
T: Into<Vec<u8>>,
{
Ok(self
.send_command(SignEcdsaCommand {
key_id,
digest: digest.into(),
})?
.into())
self.send_command(SignEcdsaCommand {
key_id,
digest: digest.into(),
})
.map(Into::into)
}

/// Compute an Ed25519 signature with the given key ID.
Expand Down
14 changes: 11 additions & 3 deletions src/ecdsa.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
//! Elliptic Curve Digital Signature Algorithm (ECDSA) support
pub mod algorithm;
pub mod nistp256;
pub mod nistp384;

#[cfg(feature = "secp256k1")]
pub mod secp256k1;

pub(crate) mod commands;
mod signature;
mod signer;

pub use self::{algorithm::Algorithm, signature::Signature, signer::Signer};
pub use signatory::ecdsa::{curve, PublicKey};
pub use self::{algorithm::Algorithm, nistp256::NistP256, nistp384::NistP384, signer::Signer};
pub use ::ecdsa::{asn1, elliptic_curve::sec1, signature, Signature};

#[cfg(feature = "secp256k1")]
pub use self::secp256k1::Secp256k1;
6 changes: 5 additions & 1 deletion src/ecdsa/algorithm.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
//! ECDSA algorithms (i.e. hash functions)
use super::{NistP256, NistP384};
use crate::{algorithm, asymmetric};
use anomaly::fail;
use signatory::ecdsa::curve::{NistP256, NistP384, Secp256k1};

#[cfg(feature = "secp256k1")]
use super::Secp256k1;

/// Valid algorithms for asymmetric keys
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -63,6 +66,7 @@ impl CurveAlgorithm for NistP384 {
}
}

#[cfg(feature = "secp256k1")]
impl CurveAlgorithm for Secp256k1 {
fn asymmetric_algorithm() -> asymmetric::Algorithm {
asymmetric::Algorithm::EcK256
Expand Down
7 changes: 3 additions & 4 deletions src/ecdsa/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
//!
//! <https://developers.yubico.com/YubiHSM2/Commands/Sign_Ecdsa.html>
use super::Signature;
use crate::{
command::{self, Command},
object,
Expand All @@ -26,14 +25,14 @@ impl Command for SignEcdsaCommand {

/// Response from ECDSA signing request
#[derive(Serialize, Deserialize, Debug)]
pub struct SignEcdsaResponse(pub(crate) Signature);
pub struct SignEcdsaResponse(pub Vec<u8>);

impl Response for SignEcdsaResponse {
const COMMAND_CODE: command::Code = command::Code::SignEcdsa;
}

impl From<SignEcdsaResponse> for Signature {
fn from(response: SignEcdsaResponse) -> Signature {
impl From<SignEcdsaResponse> for Vec<u8> {
fn from(response: SignEcdsaResponse) -> Vec<u8> {
response.0
}
}
20 changes: 20 additions & 0 deletions src/ecdsa/nistp256.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//! NIST P-256 elliptic curve (a.k.a. prime256v1, secp256r1)
//!
//! ## About
//!
//! NIST P-256 is a Weierstrass curve specified in FIPS 186-4: Digital Signature
//! Standard (DSS):
//!
//! <https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-4.pdf>
//!
//! Also known as prime256v1 (ANSI X9.62) and secp256r1 (SECG), it's included in
//! the US National Security Agency's "Suite B" and is widely used in protocols
//! like TLS and the associated X.509 PKI.
pub use p256::NistP256;

/// ECDSA/P-256 signature (fixed-size)
pub type Signature = super::Signature<NistP256>;

/// ECDSA/P-256 signer
pub type Signer = super::Signer<NistP256>;
16 changes: 16 additions & 0 deletions src/ecdsa/nistp384.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//! NIST P-384 elliptic curve.
//!
//! ## About
//!
//! NIST P-384 is a Weierstrass curve specified in FIPS 186-4: Digital Signature
//! Standard (DSS):
//!
//! <https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-4.pdf>
pub use p384::NistP384;

/// ECDSA/P-384 signature (fixed-size)
pub type Signature = super::Signature<NistP384>;

/// ECDSA/P-384 signer
pub type Signer = super::Signer<NistP384>;
21 changes: 21 additions & 0 deletions src/ecdsa/secp256k1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//! secp256k1 elliptic curve
//!
//! ## About
//!
//! The secp256k1 elliptic curve is specified by Certicom's SECG in
//! "SEC 2: Recommended Elliptic Curve Domain Parameters":
//!
//! <https://www.secg.org/sec2-v2.pdf>
//!
//! It's primarily notable for usage in Bitcoin and other cryptocurrencies.
pub use k256::{ecdsa::recoverable, Secp256k1};

/// ECDSA/secp256k1 signature (fixed-size)
pub type Signature = super::Signature<Secp256k1>;

/// ECDSA/secp256k1 signature with public key recovery support (ala Ethereum)
pub type RecoverableSignature = k256::ecdsa::recoverable::Signature;

/// ECDSA/secp256k1 signer
pub type Signer = super::Signer<Secp256k1>;
37 changes: 0 additions & 37 deletions src/ecdsa/signature.rs

This file was deleted.

Loading

0 comments on commit 45bcbd6

Please sign in to comment.