From 13977f628759b7917a3da16382679c9deefb5d15 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Wed, 7 Dec 2022 17:20:20 -0500 Subject: [PATCH] Clean and document the DKG library's encryption Encryption used to be inlined into FROST. When writing the documentation, I realized it was decently hard to review. It also was antagonistic to other hosted DKG algorithms by not allowing code re-use. Encryption is now a standalone module, providing clear boundaries and reusability. Additionally, the DKG protocol itself used to use the ciphersuite's specified hash function (with an HKDF to prevent length extension attacks). Now, RecommendedTranscript is used to achieve much more robust transcripting and remove the HKDF dependency. This does add Blake2 into all consumers yet is preferred for its security properties and ease of review. --- Cargo.lock | 2 - crypto/dkg/Cargo.toml | 10 +- crypto/dkg/src/encryption.rs | 181 +++++++++++++++++ crypto/dkg/src/frost.rs | 186 +++++------------- crypto/dkg/src/lib.rs | 2 + crypto/dkg/src/promote.rs | 2 +- crypto/dkg/src/tests/frost.rs | 14 +- .../Distributed Key Generation.md | 15 ++ 8 files changed, 265 insertions(+), 147 deletions(-) create mode 100644 crypto/dkg/src/encryption.rs create mode 100644 docs/cryptography/Distributed Key Generation.md diff --git a/Cargo.lock b/Cargo.lock index 92ff5114..7b713f1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1669,12 +1669,10 @@ version = "0.2.0" dependencies = [ "chacha20 0.9.0", "ciphersuite", - "digest 0.10.6", "dleq", "flexible-transcript", "group", "hex", - "hkdf", "multiexp", "rand_core 0.6.4", "schnorr-signatures", diff --git a/crypto/dkg/Cargo.toml b/crypto/dkg/Cargo.toml index 0a1fcc3c..a63d5bb1 100644 --- a/crypto/dkg/Cargo.toml +++ b/crypto/dkg/Cargo.toml @@ -22,18 +22,12 @@ subtle = "2" hex = "0.4" -digest = "0.10" - -hkdf = "0.12" +transcript = { package = "flexible-transcript", path = "../transcript", version = "0.2", features = ["recommended"] } chacha20 = { version = "0.9", features = ["zeroize"] } group = "0.12" - -ciphersuite = { path = "../ciphersuite", version = "0.1", features = ["std"] } - -transcript = { package = "flexible-transcript", path = "../transcript", version = "0.2", features = ["recommended"] } - multiexp = { path = "../multiexp", version = "0.2", features = ["batch"] } +ciphersuite = { path = "../ciphersuite", version = "0.1", features = ["std"] } schnorr = { package = "schnorr-signatures", path = "../schnorr", version = "0.2" } dleq = { path = "../dleq", version = "0.2", features = ["serialize"] } diff --git a/crypto/dkg/src/encryption.rs b/crypto/dkg/src/encryption.rs new file mode 100644 index 00000000..652fbbea --- /dev/null +++ b/crypto/dkg/src/encryption.rs @@ -0,0 +1,181 @@ +use core::{hash::Hash, fmt::Debug}; +use std::{ + ops::Deref, + io::{self, Read, Write}, + collections::HashMap, +}; + +use zeroize::{Zeroize, Zeroizing}; +use rand_core::{RngCore, CryptoRng}; + +use chacha20::{ + cipher::{crypto_common::KeyIvInit, StreamCipher}, + Key as Cc20Key, Nonce as Cc20Iv, ChaCha20, +}; + +use group::GroupEncoding; + +use ciphersuite::Ciphersuite; + +use transcript::{Transcript, RecommendedTranscript}; + +use crate::ThresholdParams; + +pub trait ReadWrite: Sized { + fn read(reader: &mut R, params: ThresholdParams) -> io::Result; + fn write(&self, writer: &mut W) -> io::Result<()>; + + fn serialize(&self) -> Vec { + let mut buf = vec![]; + self.write(&mut buf).unwrap(); + buf + } +} + +pub trait Message: Clone + PartialEq + Eq + Debug + Zeroize + ReadWrite {} +impl Message for M {} + +#[derive(Clone, PartialEq, Eq, Debug, Zeroize)] +pub struct EncryptionKeyMessage { + msg: M, + enc_key: C::G, +} + +// Doesn't impl ReadWrite so that doesn't need to be imported +impl EncryptionKeyMessage { + pub fn read(reader: &mut R, params: ThresholdParams) -> io::Result { + Ok(Self { msg: M::read(reader, params)?, enc_key: C::read_G(reader)? }) + } + + pub fn write(&self, writer: &mut W) -> io::Result<()> { + self.msg.write(writer)?; + writer.write_all(self.enc_key.to_bytes().as_ref()) + } + + pub fn serialize(&self) -> Vec { + let mut buf = vec![]; + self.write(&mut buf).unwrap(); + buf + } +} + +pub trait Encryptable: Clone + AsMut<[u8]> + Zeroize + ReadWrite {} +impl + Zeroize + ReadWrite> Encryptable for E {} +#[derive(Clone, Zeroize)] +pub struct EncryptedMessage(Zeroizing); + +impl EncryptedMessage { + pub fn read(reader: &mut R, params: ThresholdParams) -> io::Result { + Ok(Self(Zeroizing::new(E::read(reader, params)?))) + } + + pub fn write(&self, writer: &mut W) -> io::Result<()> { + self.0.write(writer) + } + + pub fn serialize(&self) -> Vec { + let mut buf = vec![]; + self.write(&mut buf).unwrap(); + buf + } +} + +#[derive(Clone)] +pub(crate) struct Encryption { + dst: &'static [u8], + enc_key: Zeroizing, + enc_pub_key: C::G, + enc_keys: HashMap, +} + +impl Zeroize for Encryption { + fn zeroize(&mut self) { + self.enc_key.zeroize(); + self.enc_pub_key.zeroize(); + for (_, value) in self.enc_keys.drain() { + value.zeroize(); + } + } +} + +impl Encryption { + pub(crate) fn new(dst: &'static [u8], rng: &mut R) -> Self { + let enc_key = Zeroizing::new(C::random_nonzero_F(rng)); + Self { dst, enc_pub_key: C::generator() * enc_key.deref(), enc_key, enc_keys: HashMap::new() } + } + + pub(crate) fn registration(&self, msg: M) -> EncryptionKeyMessage { + EncryptionKeyMessage { msg, enc_key: self.enc_pub_key } + } + + pub(crate) fn register( + &mut self, + participant: Id, + msg: EncryptionKeyMessage, + ) -> M { + if self.enc_keys.contains_key(&participant) { + panic!("Re-registering encryption key for a participant"); + } + self.enc_keys.insert(participant, msg.enc_key); + msg.msg + } + + fn cipher(&self, participant: Id, encrypt: bool) -> ChaCha20 { + // Ideally, we'd box this transcript with ZAlloc, yet that's only possible on nightly + // TODO + let mut transcript = RecommendedTranscript::new(b"DKG Encryption v0"); + transcript.domain_separate(self.dst); + + let other = self.enc_keys[&participant]; + if encrypt { + transcript.append_message(b"sender", self.enc_pub_key.to_bytes()); + transcript.append_message(b"receiver", other.to_bytes()); + } else { + transcript.append_message(b"sender", other.to_bytes()); + transcript.append_message(b"receiver", self.enc_pub_key.to_bytes()); + } + + let mut shared = Zeroizing::new(other * self.enc_key.deref()).deref().to_bytes(); + transcript.append_message(b"shared_key", shared.as_ref()); + shared.as_mut().zeroize(); + + let zeroize = |buf: &mut [u8]| buf.zeroize(); + + let mut key = Cc20Key::default(); + let mut challenge = transcript.challenge(b"key"); + key.copy_from_slice(&challenge[.. 32]); + zeroize(challenge.as_mut()); + + // The RecommendedTranscript isn't vulnerable to length extension attacks, yet if it was, + // it'd make sense to clone it (and fork it) just to hedge against that + let mut iv = Cc20Iv::default(); + let mut challenge = transcript.challenge(b"iv"); + iv.copy_from_slice(&challenge[.. 12]); + zeroize(challenge.as_mut()); + + // Same commentary as the transcript regarding ZAlloc + // TODO + let res = ChaCha20::new(&key, &iv); + zeroize(key.as_mut()); + zeroize(iv.as_mut()); + res + } + + pub(crate) fn encrypt( + &self, + participant: Id, + mut msg: Zeroizing, + ) -> EncryptedMessage { + self.cipher(participant, true).apply_keystream(msg.as_mut().as_mut()); + EncryptedMessage(msg) + } + + pub(crate) fn decrypt( + &self, + participant: Id, + mut msg: EncryptedMessage, + ) -> Zeroizing { + self.cipher(participant, false).apply_keystream(msg.0.as_mut().as_mut()); + msg.0 + } +} diff --git a/crypto/dkg/src/frost.rs b/crypto/dkg/src/frost.rs index c65f69e6..9fd2dea4 100644 --- a/crypto/dkg/src/frost.rs +++ b/crypto/dkg/src/frost.rs @@ -9,49 +9,43 @@ use rand_core::{RngCore, CryptoRng}; use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing}; -use digest::Digest; -use hkdf::{Hkdf, hmac::SimpleHmac}; -use chacha20::{ - cipher::{crypto_common::KeyIvInit, StreamCipher}, - Key as Cc20Key, Nonce as Cc20Iv, ChaCha20, -}; +use transcript::{Transcript, RecommendedTranscript}; use group::{ ff::{Field, PrimeField}, GroupEncoding, }; - use ciphersuite::Ciphersuite; - use multiexp::{multiexp_vartime, BatchVerifier}; use schnorr::SchnorrSignature; -use crate::{DkgError, ThresholdParams, ThresholdCore, validate_map}; +use crate::{ + DkgError, ThresholdParams, ThresholdCore, validate_map, + encryption::{ReadWrite, EncryptionKeyMessage, EncryptedMessage, Encryption}, +}; #[allow(non_snake_case)] fn challenge(context: &str, l: u16, R: &[u8], Am: &[u8]) -> C::F { - const DST: &[u8] = b"FROST Schnorr Proof of Knowledge"; - - // Hashes the context to get a fixed size value out of it - let mut transcript = C::H::digest(context.as_bytes()).as_ref().to_vec(); - transcript.extend(l.to_be_bytes()); - transcript.extend(R); - transcript.extend(Am); - C::hash_to_F(DST, &transcript) + let mut transcript = RecommendedTranscript::new(b"DKG FROST v0"); + transcript.domain_separate(b"Schnorr Proof of Knowledge"); + transcript.append_message(b"context", context.as_bytes()); + transcript.append_message(b"participant", l.to_le_bytes()); + transcript.append_message(b"nonce", R); + transcript.append_message(b"commitments", Am); + C::hash_to_F(b"PoK 0", &transcript.challenge(b"challenge")) } /// Commitments message to be broadcast to all other parties. #[derive(Clone, PartialEq, Eq, Debug, Zeroize)] pub struct Commitments { commitments: Vec, - enc_key: C::G, cached_msg: Vec, sig: SchnorrSignature, } -impl Commitments { - pub fn read(reader: &mut R, params: ThresholdParams) -> io::Result { +impl ReadWrite for Commitments { + fn read(reader: &mut R, params: ThresholdParams) -> io::Result { let mut commitments = Vec::with_capacity(params.t().into()); let mut cached_msg = vec![]; @@ -67,21 +61,14 @@ impl Commitments { for _ in 0 .. params.t() { commitments.push(read_G()?); } - let enc_key = read_G()?; - Ok(Commitments { commitments, enc_key, cached_msg, sig: SchnorrSignature::read(reader)? }) + Ok(Commitments { commitments, cached_msg, sig: SchnorrSignature::read(reader)? }) } - pub fn write(&self, writer: &mut W) -> io::Result<()> { + fn write(&self, writer: &mut W) -> io::Result<()> { writer.write_all(&self.cached_msg)?; self.sig.write(writer) } - - pub fn serialize(&self) -> Vec { - let mut buf = vec![]; - self.write(&mut buf).unwrap(); - buf - } } /// State machine to begin the key generation protocol. @@ -104,7 +91,7 @@ impl KeyGenMachine { pub fn generate_coefficients( self, rng: &mut R, - ) -> (SecretShareMachine, Commitments) { + ) -> (SecretShareMachine, EncryptionKeyMessage>) { let t = usize::from(self.params.t); let mut coefficients = Vec::with_capacity(t); let mut commitments = Vec::with_capacity(t); @@ -118,14 +105,6 @@ impl KeyGenMachine { cached_msg.extend(commitments[i].to_bytes().as_ref()); } - // Generate an encryption key for transmitting the secret shares - // It would probably be perfectly fine to use one of our polynomial elements, yet doing so - // puts the integrity of FROST at risk. While there's almost no way it could, as it's used in - // an ECDH with validated group elemnents, better to avoid any questions on it - let enc_key = Zeroizing::new(C::random_nonzero_F(&mut *rng)); - let pub_enc_key = C::generator() * enc_key.deref(); - cached_msg.extend(pub_enc_key.to_bytes().as_ref()); - // Step 2: Provide a proof of knowledge let r = Zeroizing::new(C::random_nonzero_F(rng)); let nonce = C::generator() * r.deref(); @@ -139,17 +118,21 @@ impl KeyGenMachine { challenge::(&self.context, self.params.i(), nonce.to_bytes().as_ref(), &cached_msg), ); + // Additionally create an encryption mechanism to protect the secret shares + let encryption = Encryption::new(b"FROST", rng); + // Step 4: Broadcast + let msg = + encryption.registration(Commitments { commitments: commitments.clone(), cached_msg, sig }); ( SecretShareMachine { params: self.params, context: self.context, coefficients, - our_commitments: commitments.clone(), - enc_key, - pub_enc_key, + our_commitments: commitments, + encryption, }, - Commitments { commitments, enc_key: pub_enc_key, cached_msg, sig }, + msg, ) } } @@ -169,6 +152,11 @@ fn polynomial(coefficients: &[Zeroizing], l: u16) -> /// Secret share to be sent to the party it's intended for over an authenticated channel. #[derive(Clone, PartialEq, Eq, Debug)] pub struct SecretShare(F::Repr); +impl AsMut<[u8]> for SecretShare { + fn as_mut(&mut self) -> &mut [u8] { + self.0.as_mut() + } +} impl Zeroize for SecretShare { fn zeroize(&mut self) { self.0.as_mut().zeroize() @@ -181,59 +169,16 @@ impl Drop for SecretShare { } impl ZeroizeOnDrop for SecretShare {} -impl SecretShare { - pub fn read(reader: &mut R) -> io::Result { +impl ReadWrite for SecretShare { + fn read(reader: &mut R, _: ThresholdParams) -> io::Result { let mut repr = F::Repr::default(); reader.read_exact(repr.as_mut())?; Ok(SecretShare(repr)) } - pub fn write(&self, writer: &mut W) -> io::Result<()> { + fn write(&self, writer: &mut W) -> io::Result<()> { writer.write_all(self.0.as_ref()) } - - pub fn serialize(&self) -> Vec { - let mut buf = vec![]; - self.write(&mut buf).unwrap(); - buf - } -} - -fn create_ciphers( - mut sender: ::Repr, - receiver: &mut ::Repr, - ecdh: &mut ::Repr, -) -> (ChaCha20, ChaCha20) { - let directional = |sender: &mut ::Repr| { - let mut key = Cc20Key::default(); - key.copy_from_slice( - &Hkdf::>::extract( - Some(b"key"), - &[sender.as_ref(), ecdh.as_ref()].concat(), - ) - .0 - .as_ref()[.. 32], - ); - let mut iv = Cc20Iv::default(); - iv.copy_from_slice( - &Hkdf::>::extract( - Some(b"iv"), - &[sender.as_ref(), ecdh.as_ref()].concat(), - ) - .0 - .as_ref()[.. 12], - ); - sender.as_mut().zeroize(); - - let res = ChaCha20::new(&key, &iv); - >::as_mut(&mut key).zeroize(); - >::as_mut(&mut iv).zeroize(); - res - }; - - let res = (directional(&mut sender), directional(receiver)); - ecdh.as_mut().zeroize(); - res } /// Advancement of the key generation state machine. @@ -243,8 +188,7 @@ pub struct SecretShareMachine { context: String, coefficients: Vec>, our_commitments: Vec, - enc_key: Zeroizing, - pub_enc_key: C::G, + encryption: Encryption, } impl SecretShareMachine { @@ -253,16 +197,15 @@ impl SecretShareMachine { fn verify_r1( &mut self, rng: &mut R, - mut commitments: HashMap>, - ) -> Result<(HashMap>, HashMap), DkgError> { + mut commitments: HashMap>>, + ) -> Result>, DkgError> { validate_map(&commitments, &(1 ..= self.params.n()).collect::>(), self.params.i())?; - let mut enc_keys = HashMap::new(); let mut batch = BatchVerifier::::new(commitments.len()); let mut commitments = commitments .drain() - .map(|(l, mut msg)| { - enc_keys.insert(l, msg.enc_key); + .map(|(l, msg)| { + let mut msg = self.encryption.register(l, msg); // Step 5: Validate each proof of knowledge // This is solely the prep step for the latter batch verification @@ -281,7 +224,7 @@ impl SecretShareMachine { batch.verify_with_vartime_blame().map_err(DkgError::InvalidProofOfKnowledge)?; commitments.insert(self.params.i, self.our_commitments.drain(..).collect()); - Ok((commitments, enc_keys)) + Ok(commitments) } /// Continue generating a key. @@ -291,13 +234,11 @@ impl SecretShareMachine { pub fn generate_secret_shares( mut self, rng: &mut R, - commitments: HashMap>, - ) -> Result<(KeyMachine, HashMap>), DkgError> { - let (commitments, mut enc_keys) = self.verify_r1(&mut *rng, commitments)?; + commitments: HashMap>>, + ) -> Result<(KeyMachine, HashMap>>), DkgError> { + let commitments = self.verify_r1(&mut *rng, commitments)?; // Step 1: Generate secret shares for all other parties - let sender = self.pub_enc_key.to_bytes(); - let mut ciphers = HashMap::new(); let mut res = HashMap::new(); for l in 1 ..= self.params.n() { // Don't insert our own shares to the byte buffer which is meant to be sent around @@ -306,31 +247,20 @@ impl SecretShareMachine { continue; } - let (mut cipher_send, cipher_recv) = { - let receiver = enc_keys.get_mut(&l).unwrap(); - let mut ecdh = (*receiver * self.enc_key.deref()).to_bytes(); - - create_ciphers::(sender, &mut receiver.to_bytes(), &mut ecdh) - }; - let mut share = polynomial(&self.coefficients, l); - let mut share_bytes = share.to_repr(); + let share_bytes = Zeroizing::new(SecretShare::(share.to_repr())); share.zeroize(); - - cipher_send.apply_keystream(share_bytes.as_mut()); - drop(cipher_send); - - ciphers.insert(l, cipher_recv); - res.insert(l, SecretShare::(share_bytes)); - share_bytes.as_mut().zeroize(); + res.insert(l, self.encryption.encrypt(l, share_bytes)); } - self.enc_key.zeroize(); // Calculate our own share let share = polynomial(&self.coefficients, self.params.i()); self.coefficients.zeroize(); - Ok((KeyMachine { params: self.params, secret: share, commitments, ciphers }, res)) + Ok(( + KeyMachine { params: self.params, secret: share, commitments, encryption: self.encryption }, + res, + )) } } @@ -338,24 +268,17 @@ impl SecretShareMachine { pub struct KeyMachine { params: ThresholdParams, secret: Zeroizing, - ciphers: HashMap, commitments: HashMap>, + encryption: Encryption, } impl Zeroize for KeyMachine { fn zeroize(&mut self) { self.params.zeroize(); self.secret.zeroize(); - - // cipher implements ZeroizeOnDrop and zeroizes on drop, yet doesn't implement Zeroize - // The following is redundant, as Rust should automatically handle dropping it, yet it shows - // awareness of this quirk and at least attempts to be comprehensive - for (_, cipher) in self.ciphers.drain() { - drop(cipher); - } - for (_, commitments) in self.commitments.iter_mut() { commitments.zeroize(); } + self.encryption.zeroize(); } } impl Drop for KeyMachine { @@ -373,7 +296,7 @@ impl KeyMachine { pub fn complete( mut self, rng: &mut R, - mut shares: HashMap>, + mut shares: HashMap>>, ) -> Result, DkgError> { validate_map(&shares, &(1 ..= self.params.n()).collect::>(), self.params.i())?; @@ -391,11 +314,8 @@ impl KeyMachine { }; let mut batch = BatchVerifier::new(shares.len()); - for (l, mut share_bytes) in shares.drain() { - let mut cipher = self.ciphers.remove(&l).unwrap(); - cipher.apply_keystream(share_bytes.0.as_mut()); - drop(cipher); - + for (l, share_bytes) in shares.drain() { + let mut share_bytes = self.encryption.decrypt(l, share_bytes); let mut share = Zeroizing::new( Option::::from(C::F::from_repr(share_bytes.0)).ok_or(DkgError::InvalidShare(l))?, ); diff --git a/crypto/dkg/src/lib.rs b/crypto/dkg/src/lib.rs index d7239a09..48fb4c56 100644 --- a/crypto/dkg/src/lib.rs +++ b/crypto/dkg/src/lib.rs @@ -20,6 +20,8 @@ use group::{ use ciphersuite::Ciphersuite; +mod encryption; + /// The distributed key generation protocol described in the /// [FROST paper](https://eprint.iacr.org/2020/852). pub mod frost; diff --git a/crypto/dkg/src/promote.rs b/crypto/dkg/src/promote.rs index 32a410c0..b0fad364 100644 --- a/crypto/dkg/src/promote.rs +++ b/crypto/dkg/src/promote.rs @@ -28,7 +28,7 @@ pub trait CiphersuitePromote { } fn transcript(key: G, i: u16) -> RecommendedTranscript { - let mut transcript = RecommendedTranscript::new(b"FROST Generator Update"); + let mut transcript = RecommendedTranscript::new(b"DKG Generator Promotion v0"); transcript.append_message(b"group_key", key.to_bytes()); transcript.append_message(b"participant", i.to_be_bytes()); transcript diff --git a/crypto/dkg/src/tests/frost.rs b/crypto/dkg/src/tests/frost.rs index 3abba285..6bbbb3e2 100644 --- a/crypto/dkg/src/tests/frost.rs +++ b/crypto/dkg/src/tests/frost.rs @@ -4,7 +4,8 @@ use rand_core::{RngCore, CryptoRng}; use crate::{ Ciphersuite, ThresholdParams, ThresholdCore, - frost::{SecretShare, Commitments, KeyGenMachine}, + frost::KeyGenMachine, + encryption::{EncryptionKeyMessage, EncryptedMessage}, tests::{THRESHOLD, PARTICIPANTS, clone_without}, }; @@ -24,7 +25,7 @@ pub fn frost_gen( commitments.insert( i, - Commitments::read::<&[u8]>( + EncryptionKeyMessage::read::<&[u8]>( &mut these_commitments.serialize().as_ref(), ThresholdParams { t: THRESHOLD, n: PARTICIPANTS, i: 1 }, ) @@ -41,7 +42,14 @@ pub fn frost_gen( let shares = shares .drain() .map(|(l, share)| { - (l, SecretShare::::read::<&[u8]>(&mut share.serialize().as_ref()).unwrap()) + ( + l, + EncryptedMessage::read::<&[u8]>( + &mut share.serialize().as_ref(), + ThresholdParams { t: THRESHOLD, n: PARTICIPANTS, i: 1 }, + ) + .unwrap(), + ) }) .collect::>(); secret_shares.insert(l, shares); diff --git a/docs/cryptography/Distributed Key Generation.md b/docs/cryptography/Distributed Key Generation.md new file mode 100644 index 00000000..6d6818a4 --- /dev/null +++ b/docs/cryptography/Distributed Key Generation.md @@ -0,0 +1,15 @@ +# Distributed Key Generation + +Serai uses a modification of Pedersen's Distributed Key Generation, which is +actually Feldman's Verifiable Secret Sharing Scheme run by every participant, as +described in the FROST paper. The modification included in FROST was to include +a Schnorr Proof of Knowledge for coefficient zero, preventing rogue key attacks. +This results in a two-round protocol. + +### Encryption + +In order to protect the secret shares during communication, the `dkg` library +additionally sends an encryption key. These encryption keys are used in an ECDH +to derive a shared key. This key is then hashed to obtain two keys and IVs, one +for sending and one for receiving, with the given counterparty. Chacha20 is used +as the stream cipher.