diff --git a/Cargo.lock b/Cargo.lock index 3c2c047a7a..93cceff89d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -435,6 +435,15 @@ dependencies = [ "typenum", ] +[[package]] +name = "crypto-derive" +version = "0.1.0" +dependencies = [ + "proc-macro2 1.0.43", + "quote 1.0.21", + "syn 1.0.99", +] + [[package]] name = "crypto-mac" version = "0.8.0" @@ -635,6 +644,7 @@ dependencies = [ "blst", "bulletproofs", "criterion", + "crypto-derive", "curve25519-dalek-ng", "digest 0.10.3", "ed25519-consensus", diff --git a/Cargo.toml b/Cargo.toml index a66a0941ea..b94b16739f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,8 @@ digest = "0.10.3" once_cell = "1.13.1" readonly = "0.2.2" +crypto-derive = { path = "./crypto-derive", version = "0.1.0" } + [[bench]] name = "crypto" harness = false @@ -55,4 +57,4 @@ proptest-derive = "0.3.0" serde_json = "1.0.83" sha3 = "0.10.2" serde-reflection = "0.3.6" -wycheproof = "0.4.0" \ No newline at end of file +wycheproof = "0.4.0" diff --git a/crypto-derive/Cargo.lock b/crypto-derive/Cargo.lock new file mode 100644 index 0000000000..f56c7af6f7 --- /dev/null +++ b/crypto-derive/Cargo.lock @@ -0,0 +1,47 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "crypto-derive" +version = "0.1.0" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "proc-macro2" +version = "1.0.43" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a2ca2c61bc9f3d74d2886294ab7b9853abd9c1ad903a3ac7815c58989bb7bab" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbe448f377a7d6961e30f5955f9b8d106c3f5e449d493ee1b125c1d43c2b5179" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "syn" +version = "1.0.99" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58dbef6ec655055e20b86b15a8cc6d439cca19b667537ac6a1369572d151ab13" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "unicode-ident" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4f5b37a154999a8f3f98cc23a628d850e154479cd94decf3414696e12e31aaf" diff --git a/crypto-derive/Cargo.toml b/crypto-derive/Cargo.toml new file mode 100644 index 0000000000..ab291e36ae --- /dev/null +++ b/crypto-derive/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "crypto-derive" +version = "0.1.0" +edition = "2021" +license = "Apache-2.0" +authors = ["Mysten Labs "] +description = "Collection of useful cryptographic macros" +repository = "https://github.com/MystenLabs/fastcrypto" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[lib] +proc-macro = true + +[dependencies] +syn = { version = "1.0.64", features = ["derive"] } +proc-macro2 = "1.0.24" +quote = "1.0.9" diff --git a/crypto-derive/src/lib.rs b/crypto-derive/src/lib.rs new file mode 100644 index 0000000000..6caf0793a6 --- /dev/null +++ b/crypto-derive/src/lib.rs @@ -0,0 +1,36 @@ +use proc_macro::TokenStream; +use quote::quote; +use syn::DeriveInput; + +// Imported from diem-crypto-derive@0.0.3 +// https://github.com/diem/diem/blob/release-1.4.3/crypto/crypto-derive/src/lib.rs#L113 + +#[proc_macro_derive(SilentDisplay)] +pub fn silent_display(source: TokenStream) -> TokenStream { + let ast: DeriveInput = syn::parse(source).expect("Incorrect macro input"); + let name = &ast.ident; + let gen = quote! { + // In order to ensure that secrets are never leaked, Display is elided + impl ::std::fmt::Display for #name { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { + write!(f, "", stringify!(#name)) + } + } + }; + gen.into() +} + +#[proc_macro_derive(SilentDebug)] +pub fn silent_debug(source: TokenStream) -> TokenStream { + let ast: DeriveInput = syn::parse(source).expect("Incorrect macro input"); + let name = &ast.ident; + let gen = quote! { + // In order to ensure that secrets are never leaked, Debug is elided + impl ::std::fmt::Debug for #name { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { + write!(f, "", stringify!(#name)) + } + } + }; + gen.into() +} diff --git a/src/bls12381.rs b/src/bls12381.rs index c1a908c988..f1665dc14f 100644 --- a/src/bls12381.rs +++ b/src/bls12381.rs @@ -1,8 +1,7 @@ // Copyright (c) 2022, Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 - use std::{ - fmt::{self, Display}, + fmt::{self, Debug, Display}, mem::MaybeUninit, str::FromStr, }; @@ -15,6 +14,8 @@ use once_cell::sync::OnceCell; use rand::{rngs::OsRng, RngCore}; use zeroize::Zeroize; +use crypto_derive::{SilentDebug, SilentDisplay}; + use crate::{ pubkey_bytes::PublicKeyBytes, serde_helpers::{keypair_decode_base64, BlsSignature}, @@ -52,7 +53,7 @@ pub struct BLS12381PublicKey { pub type BLS12381PublicKeyBytes = PublicKeyBytes; #[readonly::make] -#[derive(Default, Debug)] +#[derive(SilentDebug, SilentDisplay, Default)] pub struct BLS12381PrivateKey { pub privkey: blst::SecretKey, pub bytes: OnceCell<[u8; BLS_PRIVATE_KEY_LENGTH]>, diff --git a/src/ed25519.rs b/src/ed25519.rs index ecc7d035d3..e64d9f06df 100644 --- a/src/ed25519.rs +++ b/src/ed25519.rs @@ -1,6 +1,7 @@ // Copyright (c) 2022, Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 use base64ct::{Base64, Encoding}; +use crypto_derive::{SilentDebug, SilentDisplay}; use ed25519_consensus::{batch, VerificationKeyBytes}; use eyre::eyre; use once_cell::sync::OnceCell; @@ -13,7 +14,7 @@ use serde_bytes::{ByteBuf, Bytes}; use serde_with::serde_as; use signature::{rand_core::OsRng, Signature, Signer, Verifier}; use std::{ - fmt::{self, Display}, + fmt::{self, Debug, Display}, str::FromStr, }; use zeroize::{Zeroize, ZeroizeOnDrop}; @@ -43,7 +44,7 @@ pub struct Ed25519PublicKey(pub ed25519_consensus::VerificationKey); pub type Ed25519PublicKeyBytes = PublicKeyBytes; -#[derive(Debug, Zeroize, ZeroizeOnDrop)] +#[derive(SilentDebug, SilentDisplay, Zeroize, ZeroizeOnDrop)] pub struct Ed25519PrivateKey(pub ed25519_consensus::SigningKey); // There is a strong requirement for this specific impl. in Fab benchmarks diff --git a/src/lib.rs b/src/lib.rs index 74d0bb3192..928bff4c00 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,6 @@ rust_2018_idioms, rust_2021_compatibility )] - use base64ct::{Base64, Encoding}; use blake2::{ digest::{Update, VariableOutput}, diff --git a/src/secp256k1.rs b/src/secp256k1.rs index 536f3294f0..37bf6c0a62 100644 --- a/src/secp256k1.rs +++ b/src/secp256k1.rs @@ -1,11 +1,13 @@ // Copyright (c) 2022, Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 + use crate::{ pubkey_bytes::PublicKeyBytes, serde_helpers::keypair_decode_base64, traits::{Authenticator, EncodeDecodeBase64, KeyPair, SigningKey, ToFromBytes, VerifyingKey}, }; use base64ct::{Base64, Encoding}; +use crypto_derive::{SilentDebug, SilentDisplay}; use once_cell::sync::{Lazy, OnceCell}; use rust_secp256k1::{ constants, @@ -34,7 +36,7 @@ pub type Secp256k1PublicKeyBytes = PublicKeyBytes; #[readonly::make] -#[derive(Debug)] +#[derive(SilentDebug, SilentDisplay)] pub struct Secp256k1PrivateKey { pub privkey: SecretKey, pub bytes: OnceCell<[u8; constants::SECRET_KEY_SIZE]>, diff --git a/src/tests/bls12381_tests.rs b/src/tests/bls12381_tests.rs index 7d17a04772..085e03d05b 100644 --- a/src/tests/bls12381_tests.rs +++ b/src/tests/bls12381_tests.rs @@ -436,3 +436,16 @@ fn test_sk_zeroization_on_drop() { unsafe { ::std::slice::from_raw_parts(bytes_ptr, BLS12381PrivateKey::LENGTH) }; assert_ne!(sk_memory, &sk_bytes[..]); } + +#[test] +fn dont_display_secrets() { + let keypairs = keys(); + keypairs.into_iter().for_each(|keypair| { + let sk = keypair.private(); + assert_eq!(format!("{}", sk), ""); + assert_eq!( + format!("{:?}", sk), + "" + ); + }); +} diff --git a/src/tests/ed25519_tests.rs b/src/tests/ed25519_tests.rs index 4ecc805339..e61304fb5b 100644 --- a/src/tests/ed25519_tests.rs +++ b/src/tests/ed25519_tests.rs @@ -475,6 +475,7 @@ fn test_public_key_bytes_conversion() { } #[test] +#[cfg(feature = "copy_key")] fn test_copy_key_pair() { let kp = keys().pop().unwrap(); let kp_copied = kp.copy(); @@ -571,3 +572,13 @@ fn wycheproof_test() { } } } + +#[test] +fn dont_display_secrets() { + let keypairs = keys(); + keypairs.into_iter().for_each(|keypair| { + let sk = keypair.private(); + assert_eq!(format!("{}", sk), ""); + assert_eq!(format!("{:?}", sk), ""); + }); +} diff --git a/src/tests/secp256k1_tests.rs b/src/tests/secp256k1_tests.rs index cd154747e9..31e54ef8ea 100644 --- a/src/tests/secp256k1_tests.rs +++ b/src/tests/secp256k1_tests.rs @@ -110,6 +110,7 @@ fn import_export_secret_key() { } #[test] +#[cfg(feature = "copy_key")] fn test_copy_key_pair() { let kp = keys().pop().unwrap(); let kp_copied = kp.copy(); @@ -312,6 +313,7 @@ use wycheproof::TestResult; proptest::proptest! { #[test] + #[cfg(feature = "copy_key")] fn test_k256_against_secp256k1_lib_with_recovery( r in <[u8; 32]>::arbitrary() ) { @@ -417,3 +419,16 @@ fn map_result(t: TestResult) -> TestResult { _ => TestResult::Invalid, // Treat Acceptable as Invalid } } + +#[test] +fn dont_display_secrets() { + let keypairs = keys(); + keypairs.into_iter().for_each(|keypair| { + let sk = keypair.private(); + assert_eq!(format!("{}", sk), ""); + assert_eq!( + format!("{:?}", sk), + "" + ); + }); +}