From 00274735c33f1e24e4de1103231fcd0b8a8dac97 Mon Sep 17 00:00:00 2001
From: Didactic Drunk <1479616+didactic-drunk@users.noreply.github.com>
Date: Tue, 15 Jun 2021 12:06:33 -0700
Subject: [PATCH] Add Secret.move_from & Secret.copy_from

---
 README.md                          | 25 ++++++++++++++++++++-----
 spec/not_spec.cr                   | 13 +++++++------
 src/crypto-secret/class_methods.cr | 18 ++++++++++++++++++
 src/crypto-secret/not.cr           |  8 +++++---
 4 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/README.md b/README.md
index 4454b96..f8b0883 100644
--- a/README.md
+++ b/README.md
@@ -33,8 +33,9 @@ Secret providers may implement additional protections via:
 ## Usage
 
 #### Rules:
-1. Secrets are only available within a readonly or readwrite block
-2. Secrets are not thread safe except for the provided `Bytes` (only when reading) within a single readonly or readwrite block 
+1. Secrets should be erased (wiped) ASAP
+2. Secrets are only available within a `readonly` or `readwrite` block
+3. Secrets are not thread safe except for the provided `Slice` (only when reading) within a single `readonly` or `readwrite` block 
 
 
 ```crystal
@@ -58,11 +59,23 @@ end # secret is erased
 If you need thread safety :
 1. Switch to a Stateless Secret
 2. Or switch the Secret's state to readonly or readwrite after construction and never switch it again.  [sodium.cr]() makes use of this technique to provide thread safe encryption/decryption
-3. Or wrap all access in a Mutex
+3. Or wrap all access in a Mutex (compatible with all Secret classes)
 
 If you need more better performance:
 * Consider 1. or 2.
 
+If you need compatibility with any `Secret`:
+* Always use a `Mutex`
+* Never rely on 1. or 2.
+
+#### Converting `Bytes` to a `Secret`
+```crystal
+slice = method_that_return_bytes()
+secret = Crypto::Secret::Bidet.move_from slice # erases slice
+# or
+secret = Crypto::Secret::Bidet.copy_from slice
+```
+
 ## What is a Secret?
 
 <strike>Secrets are Keys</strike>
@@ -152,10 +165,12 @@ TODO: describe implementations
 
 ```
 class MySecret
-  include Crypto::Secret
+  # Choose one
+  include Crypto::Secret::Stateless
+  include Crypto::Secret::Stateful
 
   def initialize(size)
-    # allocate storage
+    # allocate or reference storage
     # optionally mlock
   end
 
diff --git a/spec/not_spec.cr b/spec/not_spec.cr
index 70cc569..d94f888 100644
--- a/spec/not_spec.cr
+++ b/spec/not_spec.cr
@@ -7,14 +7,14 @@ describe Crypto::Secret::Not do
     key = Bytes.new ksize
     key[1] = 1_u8
 
-    secret1 = Crypto::Secret::Not.new key.dup
-    secret1.to_slice { |s| s.should eq key }
+    secret1 = Crypto::Secret::Not.copy_from key
+    secret1.readonly { |s| s.should eq key }
 
-    secret2 = Crypto::Secret::Not.new key.dup
+    secret2 = Crypto::Secret::Not.copy_from key
 
     (secret1 == secret2).should be_true
-    secret1.to_slice do |s1|
-      secret2.to_slice do |s2|
+    secret1.readonly do |s1|
+      secret2.readonly do |s2|
         (s1 == s2).should be_true
       end
     end
@@ -23,9 +23,10 @@ describe Crypto::Secret::Not do
   it "bytesize" do
     secret = Crypto::Secret::Not.new 5
     secret.bytesize.should eq 5
+    secret.readonly { |s| s.bytesize.should eq 5 }
   end
 
-  it "doesn't leak key material" do
+  it "doesn't leak key material when inspecting" do
     secret = Crypto::Secret::Not.new 5
     secret.to_s.should match /\(\*\*\*SECRET\*\*\*\)$/
     secret.inspect.should match /\(\*\*\*SECRET\*\*\*\)$/
diff --git a/src/crypto-secret/class_methods.cr b/src/crypto-secret/class_methods.cr
index 6ba45b6..4eb723e 100644
--- a/src/crypto-secret/class_methods.cr
+++ b/src/crypto-secret/class_methods.cr
@@ -1,4 +1,22 @@
 module Crypto::Secret::ClassMethods
+  # Copies `data` to the new Secret and **erases data**
+  # Returns a **readonly** Secret
+  def move_from(data : Bytes)
+    copy_from data
+  ensure
+    data.wipe
+  end
+
+  # Copies `data` to the new Secret
+  # Returns a **readonly** Secret
+  def copy_from(data : Bytes)
+    new(data.bytesize).tap do |obj|
+      obj.readwrite do |slice|
+        data.copy_to slice
+      end
+    end
+  end
+
   # Returns a **readonly** random Secret
   def random(size)
     buf = new(size)
diff --git a/src/crypto-secret/not.cr b/src/crypto-secret/not.cr
index e174b02..7d6a44c 100644
--- a/src/crypto-secret/not.cr
+++ b/src/crypto-secret/not.cr
@@ -11,11 +11,13 @@ module Crypto::Secret
   struct Not
     include Stateless
 
-    def self.new(size)
-      new Bytes.new(size)
+    def self.new(size : Int32)
+      bytes = Bytes.new size
+      new(references: bytes)
     end
 
-    def initialize(@bytes : Bytes)
+    def initialize(*, references : Bytes)
+      @bytes = references
     end
 
     delegate_to_slice @bytes