Redact private keys and expose them only via Unredacted (#115)

Author: leighmcculloch

fuzz/fuzz_targets/fuzz_compare_v13.rs

@@ -2,11 +2,18 @@
 
 use libfuzzer_sys::{fuzz_target, Corpus};
 
-use stellar_strkey::Strkey as StrkeyNew;
+use stellar_strkey::ed25519::PrivateKey as PrivateKeyNew;
+use stellar_strkey::{Strkey as StrkeyNew, Unredacted};
 use stellar_strkey_v13::Strkey as StrkeyOld;
 
 // Compare parsing and encoding between the current library and v0.13.
 fuzz_target!(|s: &str| -> Corpus {
+    // `S…` strkeys are parsed via ed25519::PrivateKey on the new side and
+    // Strkey::PrivateKeyEd25519 on the old side.
+    if s.starts_with('S') {
+        return compare_private_key(s);
+    }
+
     // Try parsing with both versions.
     let old_result: Result<StrkeyOld, _> = s.parse();
     let new_result: Result<StrkeyNew, _> = s.parse();
@@ -57,9 +64,6 @@ fn compare_internals(new: &StrkeyNew, old: &StrkeyOld) {
         (StrkeyNew::PublicKeyEd25519(n), StrkeyOld::PublicKeyEd25519(o)) => {
             assert_eq!(n.0, o.0, "PublicKeyEd25519 data mismatch");
         }
-        (StrkeyNew::PrivateKeyEd25519(n), StrkeyOld::PrivateKeyEd25519(o)) => {
-            assert_eq!(n.0, o.0, "PrivateKeyEd25519 data mismatch");
-        }
         (StrkeyNew::PreAuthTx(n), StrkeyOld::PreAuthTx(o)) => {
             assert_eq!(n.0, o.0, "PreAuthTx data mismatch");
         }
@@ -101,3 +105,19 @@ fn compare_internals(new: &StrkeyNew, old: &StrkeyOld) {
         }
     }
 }
+
+/// Compare new's `ed25519::PrivateKey` parse against old's
+/// `Strkey::PrivateKeyEd25519` for `S…` inputs.
+fn compare_private_key(s: &str) -> Corpus {
+    let new: Result<PrivateKeyNew, _> = s.parse();
+    let old: Result<StrkeyOld, _> = s.parse();
+    match (new, old) {
+        (Ok(new), Ok(StrkeyOld::PrivateKeyEd25519(old))) => {
+            assert_eq!(new.0, old.0, "PrivateKeyEd25519 data mismatch");
+            assert_eq!(Unredacted(&new).to_string().as_str(), s, "roundtrip failed");
+            Corpus::Keep
+        }
+        (Err(_), Err(_)) => Corpus::Keep,
+        (new, old) => panic!("private-key parse mismatch\nInput: {s}\nNew: {new:?}\nOld: {old:?}"),
+    }
+}

fuzz/fuzz_targets/fuzz_compare_v16.rs

@@ -2,11 +2,18 @@
 
 use libfuzzer_sys::{fuzz_target, Corpus};
 
-use stellar_strkey::Strkey as StrkeyNew;
+use stellar_strkey::ed25519::PrivateKey as PrivateKeyNew;
+use stellar_strkey::{Strkey as StrkeyNew, Unredacted};
 use stellar_strkey_v16::Strkey as StrkeyOld;
 
 // Compare parsing and encoding between the current library and v0.0.16.
 fuzz_target!(|s: &str| -> Corpus {
+    // `S…` strkeys are parsed via ed25519::PrivateKey on the new side and
+    // Strkey::PrivateKeyEd25519 on the old side.
+    if s.starts_with('S') {
+        return compare_private_key(s);
+    }
+
     // Try parsing with both versions.
     let old_result: Result<StrkeyOld, _> = s.parse();
     let new_result: Result<StrkeyNew, _> = s.parse();
@@ -57,9 +64,6 @@ fn compare_internals(new: &StrkeyNew, old: &StrkeyOld) {
         (StrkeyNew::PublicKeyEd25519(n), StrkeyOld::PublicKeyEd25519(o)) => {
             assert_eq!(n.0, o.0, "PublicKeyEd25519 data mismatch");
         }
-        (StrkeyNew::PrivateKeyEd25519(n), StrkeyOld::PrivateKeyEd25519(o)) => {
-            assert_eq!(n.0, o.0, "PrivateKeyEd25519 data mismatch");
-        }
         (StrkeyNew::PreAuthTx(n), StrkeyOld::PreAuthTx(o)) => {
             assert_eq!(n.0, o.0, "PreAuthTx data mismatch");
         }
@@ -101,3 +105,19 @@ fn compare_internals(new: &StrkeyNew, old: &StrkeyOld) {
         }
     }
 }
+
+/// Compare new's `ed25519::PrivateKey` parse against old's
+/// `Strkey::PrivateKeyEd25519` for `S…` inputs.
+fn compare_private_key(s: &str) -> Corpus {
+    let new: Result<PrivateKeyNew, _> = s.parse();
+    let old: Result<StrkeyOld, _> = s.parse();
+    match (new, old) {
+        (Ok(new), Ok(StrkeyOld::PrivateKeyEd25519(old))) => {
+            assert_eq!(new.0, old.0, "PrivateKeyEd25519 data mismatch");
+            assert_eq!(Unredacted(&new).to_string().as_str(), s, "roundtrip failed");
+            Corpus::Keep
+        }
+        (Err(_), Err(_)) => Corpus::Keep,
+        (new, old) => panic!("private-key parse mismatch\nInput: {s}\nNew: {new:?}\nOld: {old:?}"),
+    }
+}

fuzz/fuzz_targets/fuzz_roundtrip.rs

@@ -2,7 +2,7 @@
 
 use libfuzzer_sys::{arbitrary::Result, fuzz_target, Corpus};
 
-use stellar_strkey::Strkey;
+use stellar_strkey::{ed25519::PrivateKey, Strkey, Unredacted};
 
 const BASE32_ALPHABET: &str = "ABCDEFGHIJKLMNOPQRSTUVWXYZ234567=";
 
@@ -12,6 +12,19 @@ fuzz_target!(|s: &str| -> Corpus {
         return Corpus::Reject;
     }
 
+    // `S…` strkeys are parsed via ed25519::PrivateKey rather than the Strkey
+    // enum; handle that path separately so private-key roundtrips remain
+    // covered.
+    if s.starts_with('S') {
+        let Ok(pk): Result<PrivateKey, _> = s.parse() else {
+            return Corpus::Keep;
+        };
+        let roundtrip_s = Unredacted(&pk).to_string();
+        assert_eq!(roundtrip_s.as_str(), s);
+        assert_eq!(s.len(), 56);
+        return Corpus::Keep;
+    }
+
     // Parse the input as a strkey. Ignore invalid strkeys.
     let Ok(r): Result<Strkey, _> = s.parse() else {
         return Corpus::Keep;
@@ -27,7 +40,6 @@ fuzz_target!(|s: &str| -> Corpus {
         first_char,
         match r {
             Strkey::PublicKeyEd25519(_) => 'G',
-            Strkey::PrivateKeyEd25519(_) => 'S',
             Strkey::MuxedAccountEd25519(_) => 'M',
             Strkey::PreAuthTx(_) => 'T',
             Strkey::HashX(_) => 'X',
@@ -42,7 +54,6 @@ fuzz_target!(|s: &str| -> Corpus {
     let len = s.len();
     match &r {
         Strkey::PublicKeyEd25519(_) => assert_eq!(len, 56),
-        Strkey::PrivateKeyEd25519(_) => assert_eq!(len, 56),
         Strkey::PreAuthTx(_) => assert_eq!(len, 56),
         Strkey::HashX(_) => assert_eq!(len, 56),
         Strkey::MuxedAccountEd25519(_) => assert_eq!(len, 69),

src/cli/decode.rs

@@ -1,7 +1,7 @@
 use std::io::{IsTerminal, Read};
 use std::str::FromStr;
 
-use crate::{DecodeError, Decoded, Strkey};
+use crate::{ed25519, DecodeError, Decoded, Strkey, Unredacted};
 use clap::Args;
 
 #[derive(Debug)]
@@ -55,11 +55,21 @@ impl Cmd {
             });
         }
         let input = buf.trim();
-        let strkey = Strkey::from_str(input).map_err(|e| Error::Decode(input.to_string(), e))?;
-        if !opts.quiet {
-            super::warn_if_private(&strkey);
-        }
-        let json = serde_json::to_string_pretty(&Decoded(&strkey)).unwrap();
+        // `S…` strkeys are decoded via `ed25519::PrivateKey` directly; the
+        // Strkey enum intentionally excludes that variant.
+        let json = if let Ok(k) = Strkey::from_str(input) {
+            serde_json::to_string_pretty(&Decoded(&k)).unwrap()
+        } else {
+            let pk = ed25519::PrivateKey::from_str(input)
+                .map_err(|e| Error::Decode(input.to_string(), e))?;
+            if !opts.quiet {
+                super::warn_private_key();
+            }
+            serde_json::to_string_pretty(&serde_json::json!({
+                "private_key_ed25519": Decoded(Unredacted(&pk)),
+            }))
+            .unwrap()
+        };
         println!("{json}");
         Ok(())
     }

src/cli/encode.rs

@@ -2,7 +2,7 @@ use std::io::{IsTerminal, Read};
 
 use clap::Args;
 
-use crate::{Decoded, Strkey};
+use crate::{ed25519, Decoded, Strkey, Unredacted};
 
 // Bound on the JSON input size. The largest legitimate Decoded<Strkey> JSON
 // (a pretty-printed signed_payload_ed25519 with a max 64-byte payload) is
@@ -56,11 +56,26 @@ impl Cmd {
                 max: MAX_JSON_LEN,
             });
         }
-        let Decoded(strkey): Decoded<Strkey> = serde_json::from_str(&input).map_err(Error::Json)?;
-        if !opts.quiet {
-            super::warn_if_private(&strkey);
+        // Peek at the variant key: `private_key_ed25519` is handled outside
+        // the Strkey enum and routed through `ed25519::PrivateKey`.
+        let value: serde_json::Value = serde_json::from_str(&input).map_err(Error::Json)?;
+        let pk_value = value
+            .as_object()
+            .filter(|m| m.len() == 1)
+            .and_then(|m| m.get("private_key_ed25519"))
+            .cloned();
+        if let Some(pk_value) = pk_value {
+            let Decoded(Unredacted(pk)): Decoded<Unredacted<ed25519::PrivateKey>> =
+                serde_json::from_value(pk_value).map_err(Error::Json)?;
+            if !opts.quiet {
+                super::warn_private_key();
+            }
+            println!("{}", Unredacted(&pk));
+        } else {
+            let Decoded(strkey): Decoded<Strkey> =
+                serde_json::from_value(value).map_err(Error::Json)?;
+            println!("{strkey}");
         }
-        println!("{strkey}");
         Ok(())
     }
 }

src/cli/mod.rs

@@ -6,8 +6,6 @@ pub mod zero;
 use clap::{Parser, Subcommand};
 use std::{ffi::OsString, fmt::Debug};
 
-use crate::Strkey;
-
 #[derive(Parser, Debug, Clone)]
 #[command(
     author,
@@ -92,13 +90,8 @@ where
     root.run()
 }
 
-/// Emit a stderr warning when a `Strkey` bound for stdout contains secret
-/// material. Centralizes the invariant that every CLI path producing a
-/// `Strkey` for the user must screen it first.
-pub(crate) fn warn_if_private(strkey: &Strkey) {
-    if matches!(strkey, Strkey::PrivateKeyEd25519(_)) {
-        eprintln!(
-            "⚠️  Warning: output contains a private key with secret material. Handle with care."
-        );
-    }
+/// Emit a stderr warning that the output bound for stdout contains secret
+/// material. Called from CLI paths that handle private-key strkeys.
+pub(crate) fn warn_private_key() {
+    eprintln!("⚠️  Warning: output contains a private key with secret material. Handle with care.");
 }

src/decoded_json_format.rs

@@ -30,4 +30,9 @@
 /// If the input is untrusted, or this type is used in an
 /// allocation-sensitive application, callers should validate the input
 /// length prior to deserializing to avoid unexpected or unbounded allocations.
+///
+/// `Decoded` is not implemented directly for
+/// [`ed25519::PrivateKey`](crate::ed25519::PrivateKey); private-key bytes are
+/// serialized only through `Decoded<Unredacted<&PrivateKey>>`, requiring an
+/// explicit [`Unredacted`](crate::Unredacted) wrap to opt in.
 pub struct Decoded<T>(pub T);