Implement 'destroyable' rather than 'auto closable' for the SecretBox…

….Nonce class to be more consistent with the rest of cava.
ConsenSysMesh · Apr 6, 2019 · 6f92b79 · 6f92b79
1 parent 4d4fd15
commit 6f92b79
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 31 deletions.
diff --git a/crypto/src/main/java/net/consensys/cava/crypto/sodium/SecretBox.java b/crypto/src/main/java/net/consensys/cava/crypto/sodium/SecretBox.java
@@ -193,7 +193,7 @@ public byte[] bytesArray() {
   /**
    * A SecretBox nonce.
    */
-  public static final class Nonce implements AutoCloseable {
+  public static final class Nonce implements Destroyable {
     final Allocated value;
 
     private Nonce(Pointer ptr, int length) {
@@ -230,6 +230,7 @@ public static Nonce fromBytes(byte[] bytes) {
       return Sodium.dup(bytes, Nonce::new);
     }
 
+    @Override
     public void destroy() {
       this.value.destroy();
     }
@@ -316,11 +317,6 @@ public Bytes bytes() {
     public byte[] bytesArray() {
       return value.bytesArray();
     }
-
-    @Override
-    public void close() {
-      destroy();
-    }
   }
 
   /**

diff --git a/...shake/src/main/java/net/consensys/cava/scuttlebutt/handshake/SecureScuttlebuttStream.java b/...shake/src/main/java/net/consensys/cava/scuttlebutt/handshake/SecureScuttlebuttStream.java
@@ -101,37 +101,49 @@ private Bytes decrypt(Bytes message, SecretBox.Key key, MutableBytes nonce, bool
   }
 
   private Bytes decryptMessage(Bytes message, SecretBox.Key key, MutableBytes nonce) {
-    MutableBytes snapshotNonce = nonce.mutableCopy();
+
+    SecretBox.Nonce headerNonce = null;
+    SecretBox.Nonce bodyNonce = null;
 
     try {
-      try (SecretBox.Nonce headerNonce = SecretBox.Nonce.fromBytes(snapshotNonce);
-          SecretBox.Nonce bodyNonce = SecretBox.Nonce.fromBytes(snapshotNonce.increment());) {
-        if (message.size() < 34) {
-          return null;
-        }
-
-        Bytes decryptedHeader = SecretBox.decrypt(message.slice(0, 34), key, headerNonce);
-
-        if (decryptedHeader == null) {
-          throw new StreamException("Failed to decrypt message header");
-        }
-
-        int bodySize = ((decryptedHeader.get(0) & 0xFF) << 8) + (decryptedHeader.get(1) & 0xFF);
-        if (message.size() < bodySize + 34) {
-          return null;
-        }
-        Bytes body = message.slice(34, bodySize);
-        Bytes decryptedBody = SecretBox.decrypt(Bytes.concatenate(decryptedHeader.slice(2), body), key, bodyNonce);
-        if (decryptedBody == null) {
-          throw new StreamException("Failed to decrypt message");
-        }
-        nonce.increment().increment();
-
-        return decryptedBody;
+      MutableBytes snapshotNonce = nonce.mutableCopy();
+      headerNonce = SecretBox.Nonce.fromBytes(snapshotNonce);
+      bodyNonce = SecretBox.Nonce.fromBytes(snapshotNonce.increment());
+
+      if (message.size() < 34) {
+        return null;
+      }
+
+      Bytes decryptedHeader = SecretBox.decrypt(message.slice(0, 34), key, headerNonce);
+
+      if (decryptedHeader == null) {
+        throw new StreamException("Failed to decrypt message header");
+      }
 
+      int bodySize = ((decryptedHeader.get(0) & 0xFF) << 8) + (decryptedHeader.get(1) & 0xFF);
+      if (message.size() < bodySize + 34) {
+        return null;
       }
+      Bytes body = message.slice(34, bodySize);
+      Bytes decryptedBody = SecretBox.decrypt(Bytes.concatenate(decryptedHeader.slice(2), body), key, bodyNonce);
+      if (decryptedBody == null) {
+        throw new StreamException("Failed to decrypt message");
+      }
+      nonce.increment().increment();
+
+      return decryptedBody;
     } catch (SodiumException | OutOfMemoryError ex) {
       throw new StreamException(ex);
+    } finally {
+      // These may be null if there was an error while trying to construct them
+      destroyIfNonNull(headerNonce);
+      destroyIfNonNull(bodyNonce);
+    }
+  }
+
+  private void destroyIfNonNull(SecretBox.Nonce nonce) {
+    if (nonce != null) {
+      nonce.destroy();
     }
   }