Skip to content

Commit

Permalink
Impl ZeroizeOnDrop for RsaPrivateKey+newtypes (#311)
Browse files Browse the repository at this point in the history
`RsaPrivateKey` self-zeroizes on drop, so add the `ZeroizeOnDrop` marker
trait to `RsaPrivateKey` and all newtypes thereof, i.e. `DecryptingKey`
and `SigningKey` for the various padding modes.

This also removes the `Zeroize` impl on `RsaPrivateKey`, since it
self-zeroizes on `Drop`, and allowing `Zeroize` might accidentally
permit use-after-zeroize vulnerabilities.
  • Loading branch information
tarcieri authored Apr 26, 2023
1 parent b1151df commit 78ea9cb
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 19 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ digest = { version = "0.10.5", default-features = false, features = ["alloc", "o
pkcs1 = { version = "0.7.5", default-features = false, features = ["alloc", "pkcs8"] }
pkcs8 = { version = "0.10.2", default-features = false, features = ["alloc"] }
signature = { version = "2", default-features = false , features = ["digest", "rand_core"] }
zeroize = { version = "1", features = ["alloc"] }
zeroize = { version = "1.5", features = ["alloc"] }

# optional dependencies
serde = { version = "1.0.103", optional = true, default-features = false, features = ["derive"] }
Expand Down
21 changes: 6 additions & 15 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use num_traits::{FromPrimitive, One, ToPrimitive};
use rand_core::CryptoRngCore;
#[cfg(feature = "serde")]
use serde::{Deserialize, Serialize};
use zeroize::Zeroize;
use zeroize::{Zeroize, ZeroizeOnDrop};

use crate::algorithms::generate::generate_multi_prime_key_with_exp;
use crate::dummy_rng::DummyRng;
Expand Down Expand Up @@ -61,22 +61,11 @@ impl Hash for RsaPrivateKey {
}
}

impl Zeroize for RsaPrivateKey {
fn zeroize(&mut self) {
self.d.zeroize();
for prime in self.primes.iter_mut() {
prime.zeroize();
}
self.primes.clear();
if self.precomputed.is_some() {
self.precomputed.take().unwrap().zeroize();
}
}
}

impl Drop for RsaPrivateKey {
fn drop(&mut self) {
self.zeroize();
self.d.zeroize();
self.primes.zeroize();
self.precomputed.zeroize();
}
}

Expand All @@ -87,6 +76,8 @@ impl Deref for RsaPrivateKey {
}
}

impl ZeroizeOnDrop for RsaPrivateKey {}

#[derive(Debug, Clone)]
pub(crate) struct PrecomputedValues {
/// D mod (P-1)
Expand Down
12 changes: 10 additions & 2 deletions src/oaep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@
//! # Usage
//!
//! See [code example in the toplevel rustdoc](../index.html#oaep-encryption).
use alloc::boxed::Box;
use alloc::string::{String, ToString};
use alloc::vec::Vec;
use core::fmt;
use core::marker::PhantomData;
use rand_core::CryptoRngCore;

use digest::{Digest, DynDigest, FixedOutputReset};
use num_bigint::BigUint;
use zeroize::Zeroizing;
use rand_core::CryptoRngCore;
use zeroize::{ZeroizeOnDrop, Zeroizing};

use crate::algorithms::oaep::*;
use crate::algorithms::pad::{uint_to_be_pad, uint_to_zeroizing_be_pad};
Expand Down Expand Up @@ -411,6 +412,13 @@ where
}
}

impl<D, MGD> ZeroizeOnDrop for DecryptingKey<D, MGD>
where
D: Digest,
MGD: Digest + FixedOutputReset,
{
}

#[cfg(test)]
mod tests {
use crate::key::{RsaPrivateKey, RsaPublicKey};
Expand Down
6 changes: 5 additions & 1 deletion src/pkcs1v15.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use signature::{
DigestSigner, DigestVerifier, Keypair, RandomizedDigestSigner, RandomizedSigner,
SignatureEncoding, Signer, Verifier,
};
use zeroize::Zeroizing;
use zeroize::{ZeroizeOnDrop, Zeroizing};

use crate::algorithms::pad::{uint_to_be_pad, uint_to_zeroizing_be_pad};
use crate::algorithms::pkcs1v15::*;
Expand Down Expand Up @@ -418,6 +418,8 @@ where
}
}

impl<D> ZeroizeOnDrop for SigningKey<D> where D: Digest {}

impl<D> Signer<Signature> for SigningKey<D>
where
D: Digest,
Expand Down Expand Up @@ -731,6 +733,8 @@ impl EncryptingKeypair for DecryptingKey {
}
}

impl ZeroizeOnDrop for DecryptingKey {}

mod oid {
use const_oid::ObjectIdentifier;

Expand Down
5 changes: 5 additions & 0 deletions src/pss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use signature::{
hazmat::{PrehashVerifier, RandomizedPrehashSigner},
DigestVerifier, Keypair, RandomizedDigestSigner, RandomizedSigner, SignatureEncoding, Verifier,
};
use zeroize::ZeroizeOnDrop;

use crate::algorithms::pad::{uint_to_be_pad, uint_to_zeroizing_be_pad};
use crate::algorithms::pss::*;
Expand Down Expand Up @@ -483,6 +484,8 @@ where
}
}

impl<D> ZeroizeOnDrop for SigningKey<D> where D: Digest {}

/// Signing key for producing "blinded" RSASSA-PSS signatures as described in
/// [draft-irtf-cfrg-rsa-blind-signatures](https://datatracker.ietf.org/doc/draft-irtf-cfrg-rsa-blind-signatures/).
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -656,6 +659,8 @@ where
}
}

impl<D> ZeroizeOnDrop for BlindedSigningKey<D> where D: Digest {}

/// Verifying key for checking the validity of RSASSA-PSS signatures as
/// described in [RFC8017 § 8.1].
///
Expand Down

0 comments on commit 78ea9cb

Please sign in to comment.