Merge pull request #57 from corretto/develop

Merge version 1.1.1 into Master
corretto · Sep 12, 2019 · 57bae8c · 57bae8c
2 parents 70e17cf + bef5f12
commit 57bae8c
Show file tree

Hide file tree

Showing 12 changed files with 426 additions and 308 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,12 +1,14 @@
 # Changelog
 
-## 1.1.1 (Unreleased)
+## 1.1.1
 
 ### Patches
 * `amazon-corretto-crypto-provider.security` updated to work on both JDK8 and JDK9+
+* Improve performance of single-byte handling in message digests.
 
 ### Maintenance
 * Support using a different JDK for testing via the `TEST_JAVA_HOME` JVM property
+* Clarify licensing
 
 ## 1.1.0
 

diff --git a/LICENSE b/LICENSE
@@ -1,3 +1,4 @@
+Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 
                                  Apache License
                            Version 2.0, January 2004

diff --git a/README.md b/README.md
@@ -161,4 +161,8 @@ AmazonCorrettoCryptoProvider.INSTANCE.assertHealthy();
 ```
 
 # License
-This library is licensed under the Apache 2.0 License. 
+This library is licensed under the Apache 2.0 license although portions of this product include software licensed under the
+[dual OpenSSL and SSLeay license](https://www.openssl.org/source/license.html).
+Specifically, this product includes software developed by the OpenSSL Project for use in the OpenSSL Toolkit ([http://www.openssl.org](http://www.openssl.org/))
+and this product also includes cryptographic software written by Eric Young ([email protected]).
+
diff --git a/THIRD-PARTY b/THIRD-PARTY
diff --git a/src/com/amazon/corretto/crypto/provider/InputBuffer.java b/src/com/amazon/corretto/crypto/provider/InputBuffer.java
@@ -193,7 +193,7 @@ public static interface StateSupplier<S> extends Supplier<S> {
     //@ public invariant bufferStateConsistent(bufferState, firstData);
 
     //@ normal_behavior
-    //@   requires 0 <= capacity;
+    //@   requires 0 < capacity;
     //@   ensures bytesReceived == 0;
     //@   ensures bytesProcessed == 0;
     //@   ensures bufferState == BufferState.Uninitialized;
@@ -203,8 +203,8 @@ public static interface StateSupplier<S> extends Supplier<S> {
     //@   signals_only IllegalArgumentException;
     //@ pure
     InputBuffer(final int capacity) {
-        if (capacity < 0) {
-            throw new IllegalArgumentException("Capacity must be non-negative");
+        if (capacity <= 0) {
+            throw new IllegalArgumentException("Capacity must be positive");
         }
         //@ set bufferState = BufferState.Uninitialized;
         buff = new AccessibleByteArrayOutputStream(0, capacity);
@@ -379,6 +379,26 @@ private boolean fillBuffer(final byte[] arr, final int offset, final int length)
         return true;
     }
 
+    /**
+     * Copies {@code val} into {@link #buff} if an only if there is
+     * sufficient space. Returns {@code true} if the data was copied.
+     * @return {@code true} if there was sufficient space in the buffer and data was copied.
+     */
+    private boolean fillBuffer(final byte val) {
+        // Overflow safe comparison.
+        if (buffSize - buff.size() < 1) {
+            return false;
+        }
+        try {
+            buff.write(val);
+        } catch (IndexOutOfBoundsException ex) {
+            throw new ArrayIndexOutOfBoundsException(ex.toString());
+        }
+        //@ set bytesReceived = bytesReceived + 1;
+        //@ set bufferState = (bufferState == BufferState.Ready) ? BufferState.DataIn : bufferState;
+        return true;
+    }
+
     /*@ private normal_behavior
       @   old int length = src.remaining();
       @   requires canTakeData(bufferState);
@@ -564,6 +584,19 @@ public void update(final byte[] src, final int offset, final int length) {
         //@ set bytesReceived = bytesReceived + length;
     }
 
+    public void update(final byte val) {
+        if (fillBuffer(val)) {
+            return;
+        }
+        processBuffer(false);
+        if (fillBuffer(val)) {
+            return;
+        }
+
+        // We explicitly do not support capacities of zero where we cannot even append a single byte.
+        throw new AssertionError("Unreachable code. Cannot buffer even a single byte");
+    }
+
     //@ public normal_behavior
     //@   requires canTakeData(bufferState);
     //@   requires arrayUpdater != null && finalHandler != null;

diff --git a/template-src/com/amazon/corretto/crypto/provider/TemplateHashSpi.java b/template-src/com/amazon/corretto/crypto/provider/TemplateHashSpi.java
@@ -28,6 +28,7 @@ public final class TemplateHashSpi extends MessageDigestSpi implements Cloneable
     private static final int HASH_SIZE;
     private static final byte[] INITIAL_CONTEXT;
 
+    private byte[] oneByteArray = null;
     private InputBuffer<byte[], byte[]> buffer;
 
     static {
@@ -44,6 +45,7 @@ public final class TemplateHashSpi extends MessageDigestSpi implements Cloneable
      * @param digest Output buffer - must have at least getHashSize() bytes
      * @param buf Input buffer
      */
+    // NOTE: This method trusts that all of the array lengths and bufLen are sane.
     static native void fastDigest(byte[] digest, byte[] buf, int bufLen);
 
     /**
@@ -72,6 +74,11 @@ public final class TemplateHashSpi extends MessageDigestSpi implements Cloneable
      * @param length Length within buf
      */
     private static native void updateContextByteArray(byte[] context, byte[] buf, int offset, int length);
+    private static void synchronizedUpdateContextByteArray(byte[] context, byte[] buf, int offset, int length) {
+        synchronized (context) {
+            updateContextByteArray(context, buf, offset, length);
+        }
+    }
 
     /**
      * Updates a native context array with some bytes from a native byte buffer. Note that the native-side code does not
@@ -81,6 +88,11 @@ public final class TemplateHashSpi extends MessageDigestSpi implements Cloneable
      * @param buf Buffer to update from
      */
     private static native void updateNativeByteBuffer(byte[] context, ByteBuffer buf);
+    private static void synchronizedUpdateNativeByteBuffer(byte[] context, ByteBuffer buf) {
+        synchronized (context) {
+            updateNativeByteBuffer(context, buf);
+        }
+    }
 
     /**
      * Finishes the digest operation. The native context is left in an undefined state.
@@ -90,17 +102,22 @@ public final class TemplateHashSpi extends MessageDigestSpi implements Cloneable
      * @param offset Offset within output buffer
      */
     private static native void finish(byte[] context, byte[] digest, int offset);
+    private static void synchronizedFinish(byte[] context, byte[] digest, int offset) {
+        synchronized (context) {
+            finish(context, digest, offset);
+        }
+    }
 
     public TemplateHashSpi() {
         Loader.checkNativeLibraryAvailability();
 
         this.buffer = new InputBuffer<byte[], byte[]>(1024)
             .withInitialStateSupplier(INITIAL_CONTEXT::clone)
-            .withUpdater(TemplateHashSpi::updateContextByteArray)
-            .withUpdater(TemplateHashSpi::updateNativeByteBuffer)
+            .withUpdater(TemplateHashSpi::synchronizedUpdateContextByteArray)
+            .withUpdater(TemplateHashSpi::synchronizedUpdateNativeByteBuffer)
             .withDoFinal((context) -> {
                 final byte[] result = new byte[HASH_SIZE];
-                finish(context, result, 0);
+                synchronizedFinish(context, result, 0);
                 return result;
             })
             .withSinglePass((src, offset, length) -> {
@@ -117,27 +134,16 @@ public TemplateHashSpi() {
 
     @Override
     protected void engineUpdate(byte input) {
-        engineUpdate(new byte[] { input }, 0, 1);
+        buffer.update(input);
     }
 
-    // Note that routines that interact with the native buffer need to be synchronized, to ensure that we don't cause
-    // heap corruption or other such fun shenanigans when multiple C threads try to manipulate native offsets at the
-    // same time. For routines that don't interact with the native buffer directly, we don't synchronize them as this
-    // class is documented to be non-thread-safe.
-
-    // In practice, the synchronization overhead is small enough to be negligible, as the monitor lock should be
-    // uncontended as long as the caller abides by the MessageDigest contract.
-
-    // Note that we could probably still do better than this in native code by adding a simple atomic field to mark the
-    // buffer as being busy.
-
     @Override
-    protected synchronized void engineUpdate(byte[] input, int offset, int length) {
+    protected void engineUpdate(byte[] input, int offset, int length) {
         buffer.update(input, offset, length);
     }
 
     @Override
-    protected synchronized void engineUpdate(ByteBuffer buf) {
+    protected void engineUpdate(ByteBuffer buf) {
         buffer.update(buf);
     }
 
@@ -148,7 +154,7 @@ protected int engineGetDigestLength() {
 
     @SuppressWarnings("unchecked")
     @Override
-    public synchronized Object clone() {
+    public Object clone() {
         try {
             TemplateHashSpi clonedObject = (TemplateHashSpi)super.clone();
 
@@ -161,7 +167,7 @@ public synchronized Object clone() {
     }
 
     @Override
-    protected synchronized byte[] engineDigest() {
+    protected byte[] engineDigest() {
         try {
             return buffer.doFinal();
         } finally {
@@ -170,7 +176,7 @@ protected synchronized byte[] engineDigest() {
     }
 
     @Override
-    protected synchronized int engineDigest(byte[] buf, int offset, int len) throws DigestException {
+    protected int engineDigest(byte[] buf, int offset, int len) throws DigestException {
         if (len < HASH_SIZE) throw new IllegalArgumentException("Buffer length too small");
         final byte[] digest = engineDigest();
         try {
@@ -182,7 +188,7 @@ protected synchronized int engineDigest(byte[] buf, int offset, int len) throws
     }
 
     @Override
-    protected synchronized void engineReset() {
+    protected void engineReset() {
         buffer.reset();
     }
 }
diff --git a/template-src/com/amazon/corretto/crypto/provider/TemplateHmacSpi.java b/template-src/com/amazon/corretto/crypto/provider/TemplateHmacSpi.java
@@ -57,6 +57,8 @@ public class TemplateHmacSpi extends MacSpi {
      */
     private static native void initContext(byte[] ctx);
 
+    // Note: All following native methods assume that normalKey has the correctLength of {@link #BLOCK_SIZE}.
+
     /**
      * Updates the provided context with the data specifried by {@code src}. The {@code key} is
      * optional and must be provided <em>only</em> for the initial {@code update*} call (whether
@@ -78,6 +80,11 @@ public class TemplateHmacSpi extends MacSpi {
      */
     private static native void updateCtxArray(byte[] ctx, byte[] normalKey, byte[] src, int offset,
             int length);
+    private static void synchronizedUpdateCtxArray(byte[] ctx, byte[] normalKey, byte[] src, int offset, int length) {
+        synchronized (ctx) {
+            updateCtxArray(ctx, normalKey, src, offset, length);
+        }
+    }
 
     /**
      * Updates the provided context with the data specifried by {@code src}. The {@code key} is
@@ -95,6 +102,11 @@ private static native void updateCtxArray(byte[] ctx, byte[] normalKey, byte[] s
      *            data the be included in the HMAC
      */
     private static native void updateCtxBuffer(byte[] ctx, byte[] normalKey, ByteBuffer src);
+    private static void synchronizedUpdateCtxBuffer(byte[] ctx, byte[] normalKey, ByteBuffer src) {
+        synchronized (ctx) {
+            updateCtxBuffer(ctx, normalKey, src);
+        }
+    }
 
     /**
      * Finishes calculating and returns the HMAC in {@code result}.
@@ -107,6 +119,11 @@ private static native void updateCtxArray(byte[] ctx, byte[] normalKey, byte[] s
      *            an array of length {@link #getContextSize()} to receive the HMAC result
      */
     private static native void doFinal(byte[] ctx, byte[] normalKey, byte[] result);
+    private static void synchronizedDoFinal(byte[] ctx, byte[] normalKey, byte[] result) {
+        synchronized (ctx) {
+            doFinal(ctx, normalKey, result);
+        }
+    }
 
     private static native void fastHmac(byte[] normalKey, byte[] message, int offset, int length,
             byte[] result);
@@ -216,26 +233,26 @@ private TemplateHmacSpi(boolean inSelfTest) {
         buffer = new InputBuffer<byte[], Void>(1024)
                 .withInitialUpdater((src, offset, length) -> {
                     assertInitialized();
-                    updateCtxArray(baseState.ctx, baseState.normalKey, src, offset, length);
+                    synchronizedUpdateCtxArray(baseState.ctx, baseState.normalKey, src, offset, length);
                     return null;
                     })
                 .withInitialUpdater((src) -> {
                     assertInitialized();
-                    updateCtxBuffer(baseState.ctx, baseState.normalKey, src);
+                    synchronizedUpdateCtxBuffer(baseState.ctx, baseState.normalKey, src);
                     return null;
                     })
                 .withUpdater((ignored, src, offset, length) -> {
                     assertInitialized();
-                    updateCtxArray(baseState.ctx, null, src, offset, length);
+                    synchronizedUpdateCtxArray(baseState.ctx, null, src, offset, length);
                     })
                 .withUpdater((ignored, src) -> {
                     assertInitialized();
-                    updateCtxBuffer(baseState.ctx, null, src);
+                    synchronizedUpdateCtxBuffer(baseState.ctx, null, src);
                     })
                 .withDoFinal((ignored) -> {
                     assertInitialized();
                     final byte[] result = new byte[HASH_SIZE];
-                    doFinal(baseState.ctx, baseState.normalKey, result);
+                    synchronizedDoFinal(baseState.ctx, baseState.normalKey, result);
                     baseState.reset();
                     return result;
                 })
@@ -255,7 +272,7 @@ private void assertInitialized() {
     }
 
     @Override
-    protected synchronized byte[] engineDoFinal() {
+    protected byte[] engineDoFinal() {
         try {
             return buffer.doFinal();
         } finally {
@@ -264,12 +281,12 @@ protected synchronized byte[] engineDoFinal() {
     }
 
     @Override
-    protected synchronized int engineGetMacLength() {
+    protected int engineGetMacLength() {
         return HASH_SIZE;
     }
 
     @Override
-    protected synchronized void engineInit(Key key, AlgorithmParameterSpec params)
+    protected void engineInit(Key key, AlgorithmParameterSpec params)
             throws InvalidKeyException, InvalidAlgorithmParameterException {
         if (params != null) {
             throw new InvalidAlgorithmParameterException("Params must be null");
@@ -291,26 +308,22 @@ protected synchronized void engineInit(Key key, AlgorithmParameterSpec params)
     }
 
     @Override
-    protected synchronized void engineReset() {
+    protected void engineReset() {
         buffer.reset();
     }
 
     @Override
-    protected synchronized void engineUpdate(byte val) {
-        if (oneByteArray == null) {
-            oneByteArray = new byte[1];
-        }
-        oneByteArray[0] = val;
-        engineUpdate(oneByteArray, 0, 1);
+    protected void engineUpdate(byte val) {
+        buffer.update(val);
     }
 
     @Override
-    protected synchronized void engineUpdate(byte[] src, int offset, int length) {
+    protected void engineUpdate(byte[] src, int offset, int length) {
         buffer.update(src, offset, length);
     }
 
     @Override
-    protected synchronized void engineUpdate(ByteBuffer input) {
+    protected void engineUpdate(ByteBuffer input) {
         buffer.update(input);
     }
 }
diff --git a/tst/com/amazon/corretto/crypto/provider/test/AesCtrDrbgTest.java b/tst/com/amazon/corretto/crypto/provider/test/AesCtrDrbgTest.java
@@ -28,6 +28,7 @@
 import com.amazon.corretto.crypto.provider.AmazonCorrettoCryptoProvider;
 import org.apache.commons.codec.DecoderException;
 import org.apache.commons.codec.binary.Hex;
+import org.junit.After;
 import org.junit.Assume;
 import org.junit.AssumptionViolatedException;
 import org.junit.Before;
@@ -47,6 +48,11 @@ public void setup() {
         rnd = new AesCtrDrbg();
     }
 
+    @After
+    public void teardown() {
+        rnd = null;
+    }
+
     @Test
     public void whenAlgorithmParametersPassed_constructionFails() throws Exception {
         // Java 9 introduces support for parameters objects passed to SecureRandom instances. Currently we reject those