From 472ed1fc3736fc4c3cc1759eeca5fe1841ab98b1 Mon Sep 17 00:00:00 2001
From: Didactic Drunk <1479616+didactic-drunk@users.noreply.github.com>
Date: Mon, 14 Jun 2021 11:15:48 -0700
Subject: [PATCH] Secret: Avoid leaking keys in logs

---
 README.md                   | 7 ++++++-
 spec/not_spec.cr            | 8 ++++++++
 src/crypto-secret/secret.cr | 5 +++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index a349224..174a52a 100644
--- a/README.md
+++ b/README.md
@@ -9,7 +9,7 @@ Secrets hold sensitive information
 The Secret interface manages limited time access to a secret and securely erases the secret when no longer needed.
 
 Secret providers may implement additional protections via:
-* `#noaccess`, `#readonly` or `readwrite`.
+* `#noaccess`, `#readonly` or `#readwrite`
 * Using [mprotect]() to control access
 * Encrypting the data when not in use
 * Deriving keys on demand from a HSM
@@ -116,8 +116,13 @@ end
 
 ## What attacks does a Secret protect against?
 
+* Timing attacks when comparing secrets by overriding `==`
+* Leaking data in to logs by overriding `inspect`
+* Wiping memory when the secret is no longer in use
+
 TODO: describe implementations
 
+
 ## Other languages/libraries
 
 * rust: [secrets](https://github.com/stouset/secrets/)
diff --git a/spec/not_spec.cr b/spec/not_spec.cr
index 37e79b5..92faeed 100644
--- a/spec/not_spec.cr
+++ b/spec/not_spec.cr
@@ -19,4 +19,12 @@ describe Crypto::Secret::Not do
       end
     end
   end
+
+  it "doesn't leak key material" do
+    secret = Crypto::Secret::Not.new 5
+    secret.to_s.should match /\(\*\*\*SECRET\*\*\*\)$/
+    secret.inspect.should match /\(\*\*\*SECRET\*\*\*\)$/
+    secret.to_s.should_not match /Bytes|Slice/
+    secret.inspect.should_not match /Bytes|Slice/
+  end
 end
diff --git a/src/crypto-secret/secret.cr b/src/crypto-secret/secret.cr
index 4d0d2af..00e4f61 100644
--- a/src/crypto-secret/secret.cr
+++ b/src/crypto-secret/secret.cr
@@ -100,6 +100,11 @@ module Crypto::Secret
     end
   end
 
+  # Hide internal state to prevent leaking in to logs
+  def inspect(io : IO) : Nil
+    io << self.class.to_s << "(***SECRET***)"
+  end
+
   abstract def to_slice(& : Bytes -> Nil)
   abstract def bytesize : Int32