Improve and test various key type Clone/Debug implementations.

Cargo.toml

@@ -241,6 +241,9 @@ include = [
     "tests/digest_tests.txt",
     "tests/ecdsa_from_pkcs8_tests.txt",
     "tests/ecdsa_tests.rs",
+    "tests/ecdsa_test_private_key_p256.p8",
+    "tests/ecdsa_test_public_key_p256.der",
+    "tests/ecdsa_test_public_key_p256_debug.txt",
     "tests/ecdsa_sign_asn1_tests.txt",
     "tests/ecdsa_sign_fixed_tests.txt",
     "tests/ecdsa_verify_asn1_tests.txt",
@@ -250,7 +253,9 @@ include = [
     "tests/ed25519_tests.rs",
     "tests/ed25519_tests.txt",
     "tests/ed25519_test_private_key.bin",
+    "tests/ed25519_test_private_key.p8",
     "tests/ed25519_test_public_key.bin",
+    "tests/ed25519_test_public_key.der",
     "tests/hkdf_tests.rs",
     "tests/hkdf_tests.txt",
     "tests/hmac_tests.rs",
@@ -268,6 +273,9 @@ include = [
     "tests/rsa_pss_sign_tests.txt",
     "tests/rsa_pss_verify_tests.txt",
     "tests/rsa_tests.rs",
+    "tests/rsa_test_private_key_2048.p8",
+    "tests/rsa_test_public_key_2048.der",
+    "tests/rsa_test_public_key_2048_debug.txt",
     "tests/signature_tests.rs",
     "third_party/fiat/curve25519.c",
     "third_party/fiat/curve25519_tables.h",

src/aead.rs

@@ -275,7 +275,7 @@ impl Algorithm {
     pub fn nonce_len(&self) -> usize { NONCE_LEN }
 }
 
-derive_debug_via_self!(Algorithm, self.id);
+derive_debug_via_id!(Algorithm);
 
 #[derive(Debug, Eq, PartialEq)]
 enum AlgorithmID {

src/aead/quic.rs

@@ -86,7 +86,7 @@ impl Algorithm {
     pub fn sample_len(&self) -> usize { SAMPLE_LEN }
 }
 
-derive_debug_via_self!(Algorithm, self.id);
+derive_debug_via_id!(Algorithm);
 
 #[derive(Debug, Eq, PartialEq)]
 enum AlgorithmID {

src/agreement.rs

@@ -84,7 +84,7 @@ pub struct Algorithm {
     ) -> Result<(), error::Unspecified>,
 }
 
-derive_debug_via_self!(Algorithm, self.curve);
+derive_debug_via_field!(Algorithm, stringify!(Algorithm), curve);
 
 impl Eq for Algorithm {}
 impl PartialEq for Algorithm {
@@ -128,13 +128,15 @@ impl<'a> EphemeralPrivateKey {
 }
 
 /// A public key for key agreement.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct PublicKey(ec::PublicKey);
 
 impl AsRef<[u8]> for PublicKey {
     fn as_ref(&self) -> &[u8] { self.0.as_ref() }
 }
 
+derive_debug_self_as_ref_hex_bytes!(PublicKey);
+
 /// Performs a key agreement with an ephemeral private key and the given public
 /// key.
 ///

src/debug.rs

@@ -14,12 +14,64 @@
 
 // Generates an implementation of the Debug trait for a type that defers to the
 // Debug implementation for a given field.
-macro_rules! derive_debug_via_self {
-    ($typename:ty, self.$($tt:tt)+) => {
+macro_rules! derive_debug_via_id {
+    ($typename:ident) => {
         impl ::core::fmt::Debug for $typename {
             fn fmt(&self, f: &mut ::core::fmt::Formatter) -> Result<(), ::core::fmt::Error> {
-                ::core::fmt::Debug::fmt(&self.$($tt)+, f)
+                ::core::fmt::Debug::fmt(&self.id, f)
             }
         }
     };
 }
+
+macro_rules! derive_debug_via_field {
+    ($type:ty, $typename:expr, $field:ident) => {
+        impl ::core::fmt::Debug for $type {
+            fn fmt(&self, f: &mut ::core::fmt::Formatter) -> Result<(), ::core::fmt::Error> {
+                f.debug_struct($typename)
+                    .field(stringify!($field), &self.$field)
+                    .finish()
+            }
+        }
+    };
+}
+
+// Generates an implementation of the Debug trait for a type that outputs the
+// hex encoding of the byte slice representation of the value.
+macro_rules! derive_debug_self_as_ref_hex_bytes {
+    ($typename:ident) => {
+        impl ::core::fmt::Debug for $typename {
+            fn fmt(&self, f: &mut ::core::fmt::Formatter) -> Result<(), ::core::fmt::Error> {
+                crate::debug::write_hex_tuple(f, stringify!($typename), self)
+            }
+        }
+    };
+}
+
+pub(crate) fn write_hex_tuple(
+    fmt: &mut core::fmt::Formatter, type_name: &str, value: &AsRef<[u8]>,
+) -> Result<(), ::core::fmt::Error> {
+    fmt.debug_tuple(type_name)
+        .field(&HexStr(value.as_ref()))
+        .finish()
+}
+
+struct HexStr<'a>(pub &'a [u8]);
+
+impl<'a> core::fmt::Debug for HexStr<'a> {
+    fn fmt(&self, fmt: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
+        fmt.write_str("\"")?;
+        write_hex_bytes(fmt, self.0)?;
+        fmt.write_str("\"")?;
+        Ok(())
+    }
+}
+
+pub(crate) fn write_hex_bytes(
+    fmt: &mut core::fmt::Formatter, bytes: &[u8],
+) -> Result<(), ::core::fmt::Error> {
+    for byte in bytes {
+        write!(fmt, "{:02x}", byte)?;
+    }
+    Ok(())
+}

src/digest.rs

@@ -24,7 +24,7 @@
 // The goal for this implementation is to drive the overhead as close to zero
 // as possible.
 
-use crate::{c, cpu, endian::*, polyfill};
+use crate::{c, cpu, debug, endian::*, polyfill};
 use core::{self, num::Wrapping};
 
 mod sha1;
@@ -223,10 +223,7 @@ impl AsRef<[u8]> for Digest {
 impl core::fmt::Debug for Digest {
     fn fmt(&self, fmt: &mut core::fmt::Formatter) -> core::fmt::Result {
         write!(fmt, "{:?}:", self.algorithm)?;
-        for byte in self.as_ref() {
-            write!(fmt, "{:02x}", byte)?;
-        }
-        Ok(())
+        debug::write_hex_bytes(fmt, self.as_ref())
     }
 }
 
@@ -272,7 +269,7 @@ impl PartialEq for Algorithm {
 
 impl Eq for Algorithm {}
 
-derive_debug_via_self!(Algorithm, self.id);
+derive_debug_via_id!(Algorithm);
 
 /// SHA-1 as specified in [FIPS 180-4]. Deprecated.
 ///

src/ec.rs

@@ -31,7 +31,7 @@ pub struct Curve {
         fn(public_out: &mut [u8], private_key: &Seed) -> Result<(), error::Unspecified>,
 }
 
-derive_debug_via_self!(Curve, self.id);
+derive_debug_via_id!(Curve);
 
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum CurveID {

src/ec/curve25519/ed25519/signing.rs

@@ -40,7 +40,7 @@ pub struct KeyPair {
     public_key: PublicKey,
 }
 
-derive_debug_via_self!(KeyPair, self.public_key);
+derive_debug_via_field!(KeyPair, stringify!(Ed25519KeyPair), public_key);
 
 impl<'a> KeyPair {
     /// Generates a new key pair and returns the key pair serialized as a
@@ -204,13 +204,15 @@ impl signature::KeyPair for KeyPair {
     fn public_key(&self) -> &Self::PublicKey { &self.public_key }
 }
 
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy)]
 pub struct PublicKey([u8; PUBLIC_KEY_LEN]);
 
 impl AsRef<[u8]> for PublicKey {
     fn as_ref(&self) -> &[u8] { self.0.as_ref() }
 }
 
+derive_debug_self_as_ref_hex_bytes!(PublicKey);
+
 fn unwrap_pkcs8(
     version: pkcs8::Version, input: untrusted::Input,
 ) -> Result<(untrusted::Input, Option<untrusted::Input>), error::KeyRejected> {

src/ec/keys.rs

@@ -73,11 +73,5 @@ impl AsRef<[u8]> for PublicKey {
     fn as_ref(&self) -> &[u8] { &self.bytes[..self.len] }
 }
 
-impl core::fmt::Debug for PublicKey {
-    fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
-        f.debug_tuple("PublicKey").field(&self.as_ref()).finish()
-    }
-}
-
 /// The maximum length, in bytes, of an encoded public key.
 pub const PUBLIC_KEY_MAX_LEN: usize = 1 + (2 * ELEM_MAX_BYTES);

src/ec/suite_b/ecdsa/signing.rs

@@ -49,7 +49,7 @@ enum AlgorithmID {
     ECDSA_P384_SHA384_ASN1_SIGNING,
 }
 
-derive_debug_via_self!(Algorithm, self.id);
+derive_debug_via_id!(Algorithm);
 
 impl PartialEq for Algorithm {
     fn eq(&self, other: &Self) -> bool { self.id == other.id }
@@ -66,7 +66,7 @@ pub struct KeyPair {
     public_key: PublicKey,
 }
 
-derive_debug_via_self!(KeyPair, self.public_key);
+derive_debug_via_field!(KeyPair, stringify!(EcdsaKeyPair), public_key);
 
 impl KeyPair {
     /// Generates a new key pair and returns the key pair serialized as a
@@ -231,9 +231,11 @@ impl signature::KeyPair for KeyPair {
     fn public_key(&self) -> &Self::PublicKey { &self.public_key }
 }
 
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy)]
 pub struct PublicKey(ec::PublicKey);
 
+derive_debug_self_as_ref_hex_bytes!(PublicKey);
+
 impl AsRef<[u8]> for PublicKey {
     fn as_ref(&self) -> &[u8] { self.0.as_ref() }
 }

src/ec/suite_b/ecdsa/verification.rs

@@ -48,7 +48,7 @@ enum AlgorithmID {
     ECDSA_P384_SHA384_FIXED,
 }
 
-derive_debug_via_self!(Algorithm, self.id);
+derive_debug_via_id!(Algorithm);
 
 impl signature::VerificationAlgorithm for Algorithm {
     fn verify(

src/hmac.rs

@@ -168,7 +168,13 @@ pub struct SigningKey {
     ctx_prototype: SigningContext,
 }
 
-derive_debug_via_self!(SigningKey, self.ctx_prototype.inner.algorithm());
+impl core::fmt::Debug for SigningKey {
+    fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
+        f.debug_struct("SigningKey")
+            .field("algorithm", self.digest_algorithm())
+            .finish()
+    }
+}
 
 impl AsRef<[u8]> for Signature {
     #[inline]
@@ -277,7 +283,13 @@ pub struct SigningContext {
     outer: digest::Context,
 }
 
-derive_debug_via_self!(SigningContext, self.inner.algorithm());
+impl core::fmt::Debug for SigningContext {
+    fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
+        f.debug_struct("SigningContext")
+            .field("algorithm", self.inner.algorithm())
+            .finish()
+    }
+}
 
 impl SigningContext {
     /// Constructs a new HMAC signing context using the given digest algorithm
@@ -325,6 +337,14 @@ pub struct VerificationKey {
     wrapped: SigningKey,
 }
 
+impl core::fmt::Debug for VerificationKey {
+    fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
+        f.debug_struct("VerificationKey")
+            .field("algorithm", self.digest_algorithm())
+            .finish()
+    }
+}
+
 impl VerificationKey {
     /// Construct an HMAC verification key using the given digest algorithm and
     /// key value.

src/rsa/bigint.rs

@@ -1251,7 +1251,6 @@ mod tests {
     // "src/rsa/bigint_elem_exp_vartime_tests.txt". See that file for details.
     // In the meantime, the function is tested indirectly via the RSA
     // verification and signing tests.
-
     #[test]
     fn test_elem_mul() {
         test::from_file("src/rsa/bigint_elem_mul_tests.txt", |section, test_case| {

src/rsa/signing.rs

@@ -33,11 +33,11 @@ pub struct KeyPair {
     qInv: bigint::Elem<P, R>,
     qq: bigint::Modulus<QQ>,
     q_mod_n: bigint::Elem<N, R>,
-    public_key: verification::Key,
-    public_key_serialized: PublicKey,
+    public: verification::Key,
+    public_key: PublicKey,
 }
 
-derive_debug_via_self!(KeyPair, self.public_key);
+derive_debug_via_field!(KeyPair, stringify!(RsaKeyPair), public_key);
 
 impl KeyPair {
     /// Parses an unencrypted PKCS#8-encoded RSA private key.
@@ -364,8 +364,8 @@ impl KeyPair {
             qInv,
             q_mod_n,
             qq,
-            public_key,
-            public_key_serialized,
+            public: public_key,
+            public_key: public_key_serialized,
         })
     }
 
@@ -373,7 +373,7 @@ impl KeyPair {
     ///
     /// A signature has the same length as the public modulus.
     pub fn public_modulus_len(&self) -> usize {
-        self.public_key_serialized
+        self.public_key
             .modulus()
             .big_endian_without_leading_zero()
             .as_slice_less_safe()
@@ -384,17 +384,19 @@ impl KeyPair {
 impl signature::KeyPair for KeyPair {
     type PublicKey = PublicKey;
 
-    fn public_key(&self) -> &Self::PublicKey { &self.public_key_serialized }
+    fn public_key(&self) -> &Self::PublicKey { &self.public_key }
 }
 
 /// A serialized RSA public key.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct PublicKey(Box<[u8]>);
 
 impl AsRef<[u8]> for PublicKey {
     fn as_ref(&self) -> &[u8] { self.0.as_ref() }
 }
 
+derive_debug_self_as_ref_hex_bytes!(PublicKey);
+
 impl PublicKey {
     fn from_n_and_e(n: io::Positive, e: io::Positive) -> Self {
         let bytes = der_writer::write_all(der::Tag::Sequence, &|output| {
@@ -524,7 +526,7 @@ impl KeyPair {
         &self, padding_alg: &'static Encoding, rng: &rand::SecureRandom, msg: &[u8],
         signature: &mut [u8],
     ) -> Result<(), error::Unspecified> {
-        let mod_bits = self.public_key.n_bits;
+        let mod_bits = self.public.n_bits;
         if signature.len() != mod_bits.as_usize_bytes_rounded_up() {
             return Err(error::Unspecified);
         }
@@ -535,7 +537,7 @@ impl KeyPair {
         // RFC 8017 Section 5.1.2: RSADP, using the Chinese Remainder Theorem
         // with Garner's algorithm.
 
-        let n = &self.public_key.n;
+        let n = &self.public.n;
 
         // Step 1. The value zero is also rejected.
         let base = bigint::Elem::from_be_bytes_padded(untrusted::Input::from(signature), n)?;
@@ -576,7 +578,7 @@ impl KeyPair {
         // minimum value, since the relationship of `e` to `d`, `p`, and `q` is
         // not verified during `KeyPair` construction.
         {
-            let verify = bigint::elem_exp_vartime(m.clone(), self.public_key.e, n);
+            let verify = bigint::elem_exp_vartime(m.clone(), self.public.e, n);
             let verify = verify.into_unencoded(n);
             bigint::elem_verify_equal_consttime(&verify, &c)?;
         }

tests/ecdsa_test_public_key_p256.der

@@ -0,0 +1 @@
+üf˜£ã#ePÄÉ墳M`*eÒ“‘P«3èM¼ƒø¦¦¹“?5«Y$^[Zzõܧk3Ëç®îY<C3AE>³Ê5ëõ.Í

tests/ecdsa_test_public_key_p256_debug.txt

@@ -0,0 +1 @@
+PublicKey("04fc116698a3e3236550c4c9efa9bd4d0619602a65d2930e9150ab33e84dbc83f8a6a6b9933f35ab59245e5b5a7af5dca76b33cbe7aeee5981b3ca350bebf52ecd")

tests/ecdsa_tests.rs

@@ -31,7 +31,11 @@
     warnings
 )]
 
-use ring::{rand, signature, test};
+use ring::{
+    rand,
+    signature::{self, KeyPair},
+    test,
+};
 
 // ECDSA *signing* tests are in src/ec/ecdsa/signing.rs.
 
@@ -197,3 +201,31 @@ fn signature_ecdsa_verify_fixed_test() {
         },
     );
 }
+
+#[test]
+fn ecdsa_test_public_key_coverage() {
+    const PRIVATE_KEY: &'static [u8] = include_bytes!("ecdsa_test_private_key_p256.p8");
+    const PUBLIC_KEY: &'static [u8] = include_bytes!("ecdsa_test_public_key_p256.der");
+    const PUBLIC_KEY_DEBUG: &'static str = include_str!("ecdsa_test_public_key_p256_debug.txt");
+
+    let key_pair = signature::EcdsaKeyPair::from_pkcs8(
+        &signature::ECDSA_P256_SHA256_FIXED_SIGNING,
+        untrusted::Input::from(PRIVATE_KEY),
+    )
+    .unwrap();
+
+    // Test `AsRef<[u8]>`
+    assert_eq!(key_pair.public_key().as_ref(), PUBLIC_KEY);
+
+    // Test `Clone`.
+    {
+        let _ = key_pair.public_key().clone();
+    }
+
+    // Test `Debug`.
+    assert_eq!(PUBLIC_KEY_DEBUG, format!("{:?}", key_pair.public_key()));
+    assert_eq!(
+        format!("EcdsaKeyPair {{ public_key: {:?} }}", key_pair.public_key()),
+        format!("{:?}", key_pair)
+    );
+}

tests/ed25519_test_public_key.der

@@ -0,0 +1 @@
+X	<09><><EFBFBD><EFBFBD>羃ﾖ~<7E>｡W焜ｬ鏐5ﾃｶﾈ贒k}

tests/ed25519_tests.rs

@@ -165,3 +165,30 @@ fn test_ed25519_from_pkcs8() {
         },
     );
 }
+
+#[test]
+fn ed25519_test_public_key_coverage() {
+    const PRIVATE_KEY: &'static [u8] = include_bytes!("ed25519_test_private_key.p8");
+    const PUBLIC_KEY: &'static [u8] = include_bytes!("ed25519_test_public_key.der");
+    const PUBLIC_KEY_DEBUG: &'static str =
+        "PublicKey(\"5809e9fef6dcec58f0f2e3b0d67e9880a11957e083ace85835c3b6c8fbaf6b7d\")";
+
+    let key_pair =
+        signature::Ed25519KeyPair::from_pkcs8(untrusted::Input::from(PRIVATE_KEY)).unwrap();
+
+    // Test `AsRef<[u8]>`
+    assert_eq!(key_pair.public_key().as_ref(), PUBLIC_KEY);
+
+    // Test `Clone`.
+    let _ = key_pair.public_key().clone();
+
+    // Test `Debug`.
+    assert_eq!(PUBLIC_KEY_DEBUG, format!("{:?}", key_pair.public_key()));
+    assert_eq!(
+        format!(
+            "Ed25519KeyPair {{ public_key: {:?} }}",
+            key_pair.public_key()
+        ),
+        format!("{:?}", key_pair)
+    );
+}

tests/hmac_tests.rs

@@ -96,3 +96,15 @@ fn hmac_test_case_inner(
 
     Ok(())
 }
+
+#[test]
+fn hmac_debug() {
+    let key = hmac::SigningKey::new(&digest::SHA256, &[0; 32]);
+    assert_eq!("SigningKey { algorithm: SHA256 }", format!("{:?}", &key));
+
+    let ctx = hmac::SigningContext::with_key(&key);
+    assert_eq!("SigningContext { algorithm: SHA256 }", format!("{:?}", &ctx));
+
+    let key = hmac::VerificationKey::new(&digest::SHA384, &[0; 32]);
+    assert_eq!("VerificationKey { algorithm: SHA384 }", format!("{:?}", &key));
+}

tests/rsa_test_public_key_2048_debug.txt

@@ -0,0 +1 @@
+PublicKey("3082010a0282010100c8a78500a5a250db8ed36c85b8dcf83c4be1953114faaac7616e0ea24922fa6b7ab01f85582c815cc3bdeb5ed46762bc536accaa8b72705b00cef316b2ec508fb9697241b9e34238419cccf7339eeb8b062147af4f5932f613d9bc0ae70bf6d56d4432e83e13767587531bfa9dd56531741244be75e8bc9226b9fa44b4b8a101358d7e8bb75d0c724a4f11ece77776263faefe79612eb1d71646e77e8982866be1400eafc3580d3139b41aaa7380187372f22e35bd55b288496165c881ed154d5811245c52d56cc09d4916d4f2a50bcf5ae0a2637f4cfa6bf9daafc113dba8383b6dd7da6dd8db22d8510a8d3115983308909a1a0332517aa55e896e154249b30203010001")

tests/rsa_tests.rs

@@ -32,7 +32,13 @@
 )]
 
 #[cfg(feature = "use_heap")]
-use ring::{error, io::der, rand, signature, test};
+use ring::{
+    error,
+    io::der,
+    rand,
+    signature::{self, KeyPair},
+    test,
+};
 
 #[cfg(feature = "use_heap")]
 #[test]
@@ -255,3 +261,36 @@ fn test_signature_rsa_primitive_verification() {
         },
     )
 }
+
+#[cfg(feature = "use_heap")]
+#[test]
+fn rsa_test_public_key_coverage() {
+    const PRIVATE_KEY: &'static [u8] = include_bytes!("rsa_test_private_key_2048.p8");
+    const PUBLIC_KEY: &'static [u8] = include_bytes!("rsa_test_public_key_2048.der");
+    const PUBLIC_KEY_DEBUG: &'static str = include_str!("rsa_test_public_key_2048_debug.txt");
+
+    let key_pair = signature::RsaKeyPair::from_pkcs8(untrusted::Input::from(PRIVATE_KEY)).unwrap();
+
+    // Test `AsRef<[u8]>`
+    assert_eq!(key_pair.public_key().as_ref(), PUBLIC_KEY);
+
+    // Test `Clone`.
+    let _ = key_pair.public_key().clone();
+
+    // Test `exponent()`.
+    assert_eq!(
+        &[0x01, 0x00, 0x01],
+        key_pair
+            .public_key()
+            .exponent()
+            .big_endian_without_leading_zero()
+            .as_slice_less_safe()
+    );
+
+    // Test `Debug`
+    assert_eq!(PUBLIC_KEY_DEBUG, format!("{:?}", key_pair.public_key()));
+    assert_eq!(
+        format!("RsaKeyPair {{ public_key: {:?} }}", key_pair.public_key()),
+        format!("{:?}", key_pair)
+    );
+}