saymrwulf/curve25519-dalek-source
1
0
Fork 0
mirror of https://github.com/saymrwulf/curve25519-dalek-source.git synced 2026-09-04 20:24:10 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Made ExpandedSecretKey private to avoid signing key oracle (#205)

Browse source 
This fix eliminates a scenario where a user misuses the `ExpandedSecretKey` API
in a way that leaks the user's secret key. In short, if a user sends
`ExpandedSecretKey::sign(sk, msg, pk1)` followed by
`ExpandedSecretKey::sign(sk, msg, pk2)`, where `pk1 != pk2`, a passive
adversary [can easily][0] derive `sk`. To mitigate this, we remove the API
entirely.

[0]: https://github.com/MystenLabs/ed25519-unsafe-libs
This commit is contained in:
Alex Xiong 2022-10-16 03:04:03 +08:00 committed by GitHub
parent ad461f4f0f
commit 9638ab40a5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 99 additions and 129 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
benches/ed25519_benchmarks.rs
View file

@ -16,7 +16,7 @@ use criterion::Criterion;
mod ed25519_benches {
use super::*;
use ed25519_dalek::ExpandedSecretKey;
use ed25519_dalek::verify_batch;
use ed25519_dalek::Keypair;
use ed25519_dalek::PublicKey;
use ed25519_dalek::Signature;
@ -30,20 +30,7 @@ mod ed25519_benches {
let keypair: Keypair = Keypair::generate(&mut csprng);
let msg: &[u8] = b"";
c.bench_function("Ed25519 signing", move |b| {
b.iter(| | keypair.sign(msg))
});
}
fn sign_expanded_key(c: &mut Criterion) {
let mut csprng: ThreadRng = thread_rng();
let keypair: Keypair = Keypair::generate(&mut csprng);
let expanded: ExpandedSecretKey = (&keypair.secret).into();
let msg: &[u8] = b"";
c.bench_function("Ed25519 signing with an expanded secret key", move |b| {
b.iter(| | expanded.sign(msg, &keypair.public))
});
c.bench_function("Ed25519 signing", move |b| b.iter(|| keypair.sign(msg)));
}
fn verify(c: &mut Criterion) {
@ -78,8 +65,10 @@ mod ed25519_benches {
let keypairs: Vec<Keypair> = (0..size).map(|_| Keypair::generate(&mut csprng)).collect();
let msg: &[u8] = b"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
let messages: Vec<&[u8]> = (0..size).map(|_| msg).collect();
let signatures: Vec<Signature> = keypairs.iter().map(|key| key.sign(&msg)).collect();
let public_keys: Vec<PublicKey> = keypairs.iter().map(|key| key.public).collect();
let signatures: Vec<Signature> =
keypairs.iter().map(|key| key.sign(&msg)).collect();
let public_keys: Vec<PublicKey> =
keypairs.iter().map(|key| key.public_key()).collect();
b.iter(|| verify_batch(&messages[..], &signatures[..], &public_keys[..]));
},
@ -100,7 +89,6 @@ mod ed25519_benches {
config = Criterion::default();
targets =
sign,
sign_expanded_key,
verify,
verify_strict,
verify_batch_signatures,

2
src/batch.rs
View file

@ -209,7 +209,7 @@ fn zero_rng() -> ZeroRng {
/// let msg: &[u8] = b"They're good dogs Brant";
/// let messages: Vec<&[u8]> = (0..64).map(|_| msg).collect();
/// let signatures: Vec<Signature> = keypairs.iter().map(|key| key.sign(&msg)).collect();
/// let public_keys: Vec<PublicKey> = keypairs.iter().map(|key| key.public).collect();
/// let public_keys: Vec<PublicKey> = keypairs.iter().map(|key| key.public_key()).collect();
///
/// let result = verify_batch(&messages[..], &signatures[..], &public_keys[..]);
/// assert!(result.is_ok());

3
src/errors.rs
View file

@ -43,6 +43,8 @@ pub(crate) enum InternalError {
name_c: &'static str, length_c: usize, },
/// An ed25519ph signature can only take up to 255 octets of context.
PrehashedContextLengthError,
/// A mismatched (public, secret) key pair.
MismatchedKeypairError,
}
impl Display for InternalError {
@ -63,6 +65,7 @@ impl Display for InternalError {
{} has length {}, {} has length {}.", na, la, nb, lb, nc, lc),
InternalError::PrehashedContextLengthError
=> write!(f, "An ed25519ph signature can only take up to 255 octets of context"),
InternalError::MismatchedKeypairError => write!(f, "Mismatched Keypair detected"),
}
}
}

75
src/keypair.rs
View file

@ -17,7 +17,7 @@ use serde::de::Error as SerdeError;
#[cfg(feature = "serde")]
use serde::{Deserialize, Deserializer, Serialize, Serializer};
#[cfg(feature = "serde")]
use serde_bytes::{Bytes as SerdeBytes, ByteBuf as SerdeByteBuf};
use serde_bytes::{ByteBuf as SerdeByteBuf, Bytes as SerdeBytes};
pub use sha2::Sha512;
@ -32,15 +32,34 @@ use crate::public::*;
use crate::secret::*;
/// An ed25519 keypair.
// Invariant: `public` is always the public key of `secret`. This prevents the signing function
// oracle attack described in https://github.com/MystenLabs/ed25519-unsafe-libs
#[derive(Debug)]
pub struct Keypair {
/// The secret half of this keypair.
pub secret: SecretKey,
pub(crate) secret: SecretKey,
/// The public half of this keypair.
pub public: PublicKey,
pub(crate) public: PublicKey,
}
impl From<SecretKey> for Keypair {
fn from(secret: SecretKey) -> Self {
let public = PublicKey::from(&secret);
Self { secret, public }
}
}
impl Keypair {
/// Get the secret key of this keypair.
pub fn secret_key(&self) -> SecretKey {
SecretKey(self.secret.0)
}
/// Get the public key of this keypair.
pub fn public_key(&self) -> PublicKey {
self.public
}
/// Convert this keypair to bytes.
///
/// # Returns
@ -49,7 +68,8 @@ impl Keypair {
/// `SECRET_KEY_LENGTH` of bytes is the `SecretKey`, and the next
/// `PUBLIC_KEY_LENGTH` bytes is the `PublicKey` (the same as other
/// libraries, such as [Adam Langley's ed25519 Golang
/// implementation](https://github.com/agl/ed25519/)).
/// implementation](https://github.com/agl/ed25519/)). It is guaranteed that
/// the encoded public key is the one derived from the encoded secret key.
pub fn to_bytes(&self) -> [u8; KEYPAIR_LENGTH] {
let mut bytes: [u8; KEYPAIR_LENGTH] = [0u8; KEYPAIR_LENGTH];
@ -62,32 +82,31 @@ impl Keypair {
///
/// # Inputs
///
/// * `bytes`: an `&[u8]` representing the scalar for the secret key, and a
/// compressed Edwards-Y coordinate of a point on curve25519, both as bytes.
/// (As obtained from `Keypair::to_bytes()`.)
///
/// # Warning
///
/// Absolutely no validation is done on the key. If you give this function
/// bytes which do not represent a valid point, or which do not represent
/// corresponding parts of the key, then your `Keypair` will be broken and
/// it will be your fault.
/// * `bytes`: an `&[u8]` of length [`KEYPAIR_LENGTH`], representing the
/// scalar for the secret key, and a compressed Edwards-Y coordinate of a
/// point on curve25519, both as bytes. (As obtained from
/// [`Keypair::to_bytes`].)
///
/// # Returns
///
/// A `Result` whose okay value is an EdDSA `Keypair` or whose error value
/// is an `SignatureError` describing the error that occurred.
pub fn from_bytes<'a>(bytes: &'a [u8]) -> Result<Keypair, SignatureError> {
pub fn from_bytes(bytes: &[u8]) -> Result<Keypair, SignatureError> {
if bytes.len() != KEYPAIR_LENGTH {
return Err(InternalError::BytesLengthError {
name: "Keypair",
length: KEYPAIR_LENGTH,
}.into());
}
.into());
}
let secret = SecretKey::from_bytes(&bytes[..SECRET_KEY_LENGTH])?;
let public = PublicKey::from_bytes(&bytes[SECRET_KEY_LENGTH..])?;
Ok(Keypair{ secret: secret, public: public })
if public != (&secret).into() {
return Err(InternalError::MismatchedKeypairError.into());
}
Ok(Keypair { secret, public })
}
/// Generate an ed25519 keypair.
@ -131,7 +150,10 @@ impl Keypair {
let sk: SecretKey = SecretKey::generate(csprng);
let pk: PublicKey = (&sk).into();
Keypair{ public: pk, secret: sk }
Keypair {
public: pk,
secret: sk,
}
}
/// Sign a `prehashed_message` with this `Keypair` using the
@ -244,16 +266,17 @@ impl Keypair {
{
let expanded: ExpandedSecretKey = (&self.secret).into(); // xxx thanks i hate this
expanded.sign_prehashed(prehashed_message, &self.public, context).into()
expanded
.sign_prehashed(prehashed_message, &self.public, context)
.into()
}
/// Verify a signature on a message with this keypair's public key.
pub fn verify(
&self,
message: &[u8],
signature: &ed25519::Signature
) -> Result<(), SignatureError>
{
signature: &ed25519::Signature,
) -> Result<(), SignatureError> {
self.public.verify(message, signature)
}
@ -303,7 +326,7 @@ impl Keypair {
/// let mut prehashed_again: Sha512 = Sha512::default();
/// prehashed_again.update(message);
///
/// let verified = keypair.public.verify_prehashed(prehashed_again, Some(context), &sig);
/// let verified = keypair.public_key().verify_prehashed(prehashed_again, Some(context), &sig);
///
/// assert!(verified.is_ok());
///
@ -329,7 +352,8 @@ impl Keypair {
where
D: Digest<OutputSize = U64>,
{
self.public.verify_prehashed(prehashed_message, context, signature)
self.public
.verify_prehashed(prehashed_message, context, signature)
}
/// Strictly verify a signature on a message with this keypair's public key.
@ -399,8 +423,7 @@ impl Keypair {
&self,
message: &[u8],
signature: &ed25519::Signature,
) -> Result<(), SignatureError>
{
) -> Result<(), SignatureError> {
self.public.verify_strict(message, signature)
}
}

17
src/lib.rs
View file

@ -87,7 +87,7 @@
//! # let message: &[u8] = b"This is a test of the tsunami alert system.";
//! # let signature: Signature = keypair.sign(message);
//!
//! let public_key: PublicKey = keypair.public;
//! let public_key: PublicKey = keypair.public_key();
//! assert!(public_key.verify(message, &signature).is_ok());
//! # }
//! ```
@ -111,10 +111,9 @@
//! # let keypair: Keypair = Keypair::generate(&mut csprng);
//! # let message: &[u8] = b"This is a test of the tsunami alert system.";
//! # let signature: Signature = keypair.sign(message);
//! # let public_key: PublicKey = keypair.public;
//!
//! let public_key_bytes: [u8; PUBLIC_KEY_LENGTH] = public_key.to_bytes();
//! let secret_key_bytes: [u8; SECRET_KEY_LENGTH] = keypair.secret.to_bytes();
//! let public_key_bytes: [u8; PUBLIC_KEY_LENGTH] = keypair.public_key().to_bytes();
//! let secret_key_bytes: [u8; SECRET_KEY_LENGTH] = keypair.secret_key().to_bytes();
//! let keypair_bytes: [u8; KEYPAIR_LENGTH] = keypair.to_bytes();
//! let signature_bytes: [u8; SIGNATURE_LENGTH] = signature.to_bytes();
//! # }
@ -127,6 +126,7 @@
//! # extern crate ed25519_dalek;
//! # use std::convert::TryFrom;
//! # use rand::rngs::OsRng;
//! # use std::convert::TryInto;
//! # use ed25519_dalek::{Keypair, Signature, Signer, PublicKey, SecretKey, SignatureError};
//! # use ed25519_dalek::{PUBLIC_KEY_LENGTH, SECRET_KEY_LENGTH, KEYPAIR_LENGTH, SIGNATURE_LENGTH};
//! # fn do_test() -> Result<(SecretKey, PublicKey, Keypair, Signature), SignatureError> {
@ -134,8 +134,8 @@
//! # let keypair_orig: Keypair = Keypair::generate(&mut csprng);
//! # let message: &[u8] = b"This is a test of the tsunami alert system.";
//! # let signature_orig: Signature = keypair_orig.sign(message);
//! # let public_key_bytes: [u8; PUBLIC_KEY_LENGTH] = keypair_orig.public.to_bytes();
//! # let secret_key_bytes: [u8; SECRET_KEY_LENGTH] = keypair_orig.secret.to_bytes();
//! # let public_key_bytes: [u8; PUBLIC_KEY_LENGTH] = keypair_orig.public_key().to_bytes();
//! # let secret_key_bytes: [u8; SECRET_KEY_LENGTH] = keypair_orig.secret_key().to_bytes();
//! # let keypair_bytes: [u8; KEYPAIR_LENGTH] = keypair_orig.to_bytes();
//! # let signature_bytes: [u8; SIGNATURE_LENGTH] = signature_orig.to_bytes();
//! #
@ -181,7 +181,7 @@
//! # let keypair: Keypair = Keypair::generate(&mut csprng);
//! # let message: &[u8] = b"This is a test of the tsunami alert system.";
//! # let signature: Signature = keypair.sign(message);
//! # let public_key: PublicKey = keypair.public;
//! # let public_key: PublicKey = keypair.public_key();
//! # let verified: bool = public_key.verify(message, &signature).is_ok();
//!
//! let encoded_public_key: Vec<u8> = serialize(&public_key).unwrap();
@ -213,7 +213,7 @@
//! # let keypair: Keypair = Keypair::generate(&mut csprng);
//! let message: &[u8] = b"This is a test of the tsunami alert system.";
//! # let signature: Signature = keypair.sign(message);
//! # let public_key: PublicKey = keypair.public;
//! # let public_key: PublicKey = keypair.public_key();
//! # let verified: bool = public_key.verify(message, &signature).is_ok();
//! # let encoded_public_key: Vec<u8> = serialize(&public_key).unwrap();
//! # let encoded_signature: Vec<u8> = serialize(&signature).unwrap();
@ -234,7 +234,6 @@
#![no_std]
#![warn(future_incompatible)]
#![deny(missing_docs)] // refuse to compile if documentation is missing
#![cfg_attr(not(test), forbid(unsafe_code))]
#[cfg(any(feature = "std", test))]

50
src/secret.rs
View file

@ -239,7 +239,7 @@ impl<'d> Deserialize<'d> for SecretKey {
// same signature scheme, and which both fail in exactly the same way. For a
// better-designed, Schnorr-based signature scheme, see Trevor Perrin's work on
// "generalised EdDSA" and "VXEdDSA".
pub struct ExpandedSecretKey {
pub(crate) struct ExpandedSecretKey {
pub(crate) key: Scalar,
pub(crate) nonce: [u8; 32],
}
@ -256,7 +256,7 @@ impl<'a> From<&'a SecretKey> for ExpandedSecretKey {
///
/// # Examples
///
/// ```
/// ```ignore
/// # extern crate rand;
/// # extern crate sha2;
/// # extern crate ed25519_dalek;
@ -302,7 +302,7 @@ impl ExpandedSecretKey {
///
/// # Examples
///
/// ```
/// ```ignore
/// # extern crate rand;
/// # extern crate sha2;
/// # extern crate ed25519_dalek;
@ -342,7 +342,7 @@ impl ExpandedSecretKey {
///
/// # Examples
///
/// ```
/// ```ignore
/// # extern crate rand;
/// # extern crate sha2;
/// # extern crate ed25519_dalek;
@ -375,12 +375,13 @@ impl ExpandedSecretKey {
/// # fn main() { }
/// ```
#[inline]
pub fn from_bytes(bytes: &[u8]) -> Result<ExpandedSecretKey, SignatureError> {
pub(crate) fn from_bytes(bytes: &[u8]) -> Result<ExpandedSecretKey, SignatureError> {
if bytes.len() != EXPANDED_SECRET_KEY_LENGTH {
return Err(InternalError::BytesLengthError {
name: "ExpandedSecretKey",
length: EXPANDED_SECRET_KEY_LENGTH,
}.into());
}
.into());
}
let mut lower: [u8; 32] = [0u8; 32];
let mut upper: [u8; 32] = [0u8; 32];
@ -396,7 +397,7 @@ impl ExpandedSecretKey {
/// Sign a message with this `ExpandedSecretKey`.
#[allow(non_snake_case)]
pub fn sign(&self, message: &[u8], public_key: &PublicKey) -> ed25519::Signature {
pub(crate) fn sign(&self, message: &[u8], public_key: &PublicKey) -> ed25519::Signature {
let mut h: Sha512 = Sha512::new();
let R: CompressedEdwardsY;
let r: Scalar;
@ -441,7 +442,7 @@ impl ExpandedSecretKey {
///
/// [rfc8032]: https://tools.ietf.org/html/rfc8032#section-5.1
#[allow(non_snake_case)]
pub fn sign_prehashed<'a, D>(
pub(crate) fn sign_prehashed<'a, D>(
&self,
prehashed_message: D,
public_key: &PublicKey,
@ -507,28 +508,6 @@ impl ExpandedSecretKey {
}
}
#[cfg(feature = "serde")]
impl Serialize for ExpandedSecretKey {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let bytes = &self.to_bytes()[..];
SerdeBytes::new(bytes).serialize(serializer)
}
}
#[cfg(feature = "serde")]
impl<'d> Deserialize<'d> for ExpandedSecretKey {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'d>,
{
let bytes = <SerdeByteBuf>::deserialize(deserializer)?;
ExpandedSecretKey::from_bytes(bytes.as_ref()).map_err(SerdeError::custom)
}
}
#[cfg(test)]
mod test {
use super::*;
@ -547,4 +526,15 @@ mod test {
assert!(!memory.contains(&0x15));
}
#[test]
fn pubkey_from_secret_and_expanded_secret() {
let mut csprng = rand::rngs::OsRng {};
let secret: SecretKey = SecretKey::generate(&mut csprng);
let expanded_secret: ExpandedSecretKey = (&secret).into();
let public_from_secret: PublicKey = (&secret).into(); // XXX eww
let public_from_expanded_secret: PublicKey = (&expanded_secret).into(); // XXX eww
assert!(public_from_secret == public_from_expanded_secret);
}
}

57
tests/ed25519.rs
View file

@ -29,7 +29,6 @@ use sha2::Sha512;
#[cfg(test)]
mod vectors {
use curve25519_dalek::{edwards::EdwardsPoint, scalar::Scalar};
use ed25519::signature::Signature as _;
use sha2::{digest::Digest, Sha512};
use std::convert::TryFrom;
@ -69,8 +68,10 @@ mod vectors {
let sig_bytes: Vec<u8> = FromHex::from_hex(&parts[3]).unwrap();
let secret: SecretKey = SecretKey::from_bytes(&sec_bytes[..SECRET_KEY_LENGTH]).unwrap();
let public: PublicKey = PublicKey::from_bytes(&pub_bytes[..PUBLIC_KEY_LENGTH]).unwrap();
let keypair: Keypair = Keypair{ secret: secret, public: public };
let expected_public: PublicKey =
PublicKey::from_bytes(&pub_bytes[..PUBLIC_KEY_LENGTH]).unwrap();
let keypair: Keypair = Keypair::from(secret);
assert_eq!(expected_public, keypair.public_key());
// The signatures in the test vectors also include the message
// at the end, but we just want R and S.
@ -97,8 +98,10 @@ mod vectors {
let sig_bytes: Vec<u8> = FromHex::from_hex(signature).unwrap();
let secret: SecretKey = SecretKey::from_bytes(&sec_bytes[..SECRET_KEY_LENGTH]).unwrap();
let public: PublicKey = PublicKey::from_bytes(&pub_bytes[..PUBLIC_KEY_LENGTH]).unwrap();
let keypair: Keypair = Keypair{ secret: secret, public: public };
let expected_public: PublicKey =
PublicKey::from_bytes(&pub_bytes[..PUBLIC_KEY_LENGTH]).unwrap();
let keypair: Keypair = Keypair::from(secret);
assert_eq!(expected_public, keypair.public_key());
let sig1: Signature = Signature::from_bytes(&sig_bytes[..]).unwrap();
let mut prehash_for_signing: Sha512 = Sha512::default();
@ -280,17 +283,6 @@ mod integrations {
assert!(result.is_ok());
}
#[test]
fn pubkey_from_secret_and_expanded_secret() {
let mut csprng = OsRng{};
let secret: SecretKey = SecretKey::generate(&mut csprng);
let expanded_secret: ExpandedSecretKey = (&secret).into();
let public_from_secret: PublicKey = (&secret).into(); // XXX eww
let public_from_expanded_secret: PublicKey = (&expanded_secret).into(); // XXX eww
assert!(public_from_secret == public_from_expanded_secret);
}
}
#[serde(crate = "serde_crate")]
@ -401,28 +393,6 @@ mod serialisation {
}
}
#[test]
fn serialize_deserialize_expanded_secret_key_bincode() {
let expanded_secret_key = ExpandedSecretKey::from(&SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap());
let encoded_expanded_secret_key: Vec<u8> = bincode::serialize(&expanded_secret_key).unwrap();
let decoded_expanded_secret_key: ExpandedSecretKey = bincode::deserialize(&encoded_expanded_secret_key).unwrap();
for i in 0..EXPANDED_SECRET_KEY_LENGTH {
assert_eq!(expanded_secret_key.to_bytes()[i], decoded_expanded_secret_key.to_bytes()[i]);
}
}
#[test]
fn serialize_deserialize_expanded_secret_key_json() {
let expanded_secret_key = ExpandedSecretKey::from(&SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap());
let encoded_expanded_secret_key = serde_json::to_string(&expanded_secret_key).unwrap();
let decoded_expanded_secret_key: ExpandedSecretKey = serde_json::from_str(&encoded_expanded_secret_key).unwrap();
for i in 0..EXPANDED_SECRET_KEY_LENGTH {
assert_eq!(expanded_secret_key.to_bytes()[i], decoded_expanded_secret_key.to_bytes()[i]);
}
}
#[test]
fn serialize_deserialize_keypair_bincode() {
let keypair = Keypair::from_bytes(&KEYPAIR_BYTES).unwrap();
@ -471,13 +441,10 @@ mod serialisation {
#[test]
fn serialize_secret_key_size() {
let secret_key: SecretKey = SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap();
assert_eq!(bincode::serialized_size(&secret_key).unwrap() as usize, BINCODE_INT_LENGTH + SECRET_KEY_LENGTH);
}
#[test]
fn serialize_expanded_secret_key_size() {
let expanded_secret_key = ExpandedSecretKey::from(&SecretKey::from_bytes(&SECRET_KEY_BYTES).unwrap());
assert_eq!(bincode::serialized_size(&expanded_secret_key).unwrap() as usize, BINCODE_INT_LENGTH + EXPANDED_SECRET_KEY_LENGTH);
assert_eq!(
bincode::serialized_size(&secret_key).unwrap() as usize,
BINCODE_INT_LENGTH + SECRET_KEY_LENGTH
);
}
#[test]