Introduce SecureRandom wrapper for properly seeded static instances

Introduce our own wrapper `SecureRandom` around `java.security.SecureRandom`.
This a) makes sure that the PRNG is seeded on creation and not when
random bytes are retrieved, and
b) uses a static instance in the `UserModel` so that lags do not occur
during operation due to potentially seeding getting blocked on Unix
when reading from the system's entropy pool. To keep the random data
still secure, the static instance will reseed all 24 hours, also a
functionality of the wrapper class.

This fixes #1063 and extends and closes PR #1116

diff --git a/src/main/java/com/gitblit/models/UserModel.java b/src/main/java/com/gitblit/models/UserModel.java
index edbdf028bf7e533589c028f13f9e68db855c6208..f8f7ed6dcf32d463ddd18e7234003c6e05fa231d 100644 (file)
--- a/src/main/java/com/gitblit/models/UserModel.java
+++ b/src/main/java/com/gitblit/models/UserModel.java
@@ -37,6 +37,7 @@ import com.gitblit.Constants.PermissionType;
 import com.gitblit.Constants.RegistrantType;\r
 import com.gitblit.utils.ArrayUtils;\r
 import com.gitblit.utils.ModelUtils;\r
+import com.gitblit.utils.SecureRandom;\r
 import com.gitblit.utils.StringUtils;\r
 \r
 /**\r
@@ -53,6 +54,8 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
 \r
        public static final UserModel ANONYMOUS = new UserModel();\r
 \r
+       private static final SecureRandom RANDOM = new SecureRandom();\r
+\r
        // field names are reflectively mapped in EditUser page\r
        public String username;\r
        public String password;\r
@@ -661,11 +664,8 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
                String projectPath = StringUtils.getFirstPathElement(repository);\r
                return !StringUtils.isEmpty(projectPath) && projectPath.equalsIgnoreCase(getPersonalPath());\r
        }\r
-       \r
+\r
        public String createCookie() {\r
-               SecureRandom random = new SecureRandom();\r
-               byte[] values = new byte[20];\r
-               random.nextBytes(values);\r
-               return StringUtils.getSHA1(String.valueOf(values));\r
+               return StringUtils.getSHA1(RANDOM.randomBytes(32));\r
        }\r
 }\r

diff --git a/src/main/java/com/gitblit/utils/SecureRandom.java b/src/main/java/com/gitblit/utils/SecureRandom.java
new file mode 100644 (file)
index 0000000..119533d
--- /dev/null
+++ b/src/main/java/com/gitblit/utils/SecureRandom.java
@@ -0,0 +1,83 @@
+/*
+ * Copyright 2016 gitblit.com
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.gitblit.utils;
+
+/**
+ * Wrapper class for java.security.SecureRandom, which will periodically reseed
+ * the PRNG in case an instance of the class has been running for a long time.
+ *
+ * @author Florian Zschocke
+ */
+public class SecureRandom {
+
+       /** Period (in ms) after which a new SecureRandom will be created in order to get a fresh random seed. */
+       private static final long RESEED_PERIOD = 24 * 60 * 60 * 1000; /* 24 hours */
+
+
+       private long last;
+       private java.security.SecureRandom random;
+
+
+
+       public SecureRandom() {
+               // Make sure the SecureRandom is seeded right from the start.
+               // This also lets any blocks during seeding occur at creation
+               // and prevents it from happening when getting next random bytes.
+               seed();
+       }
+
+
+
+       public byte[] randomBytes(int num) {
+               byte[] bytes = new byte[num];
+               nextBytes(bytes);
+               return bytes;
+       }
+
+
+       public void nextBytes(byte[] bytes) {
+               random.nextBytes(bytes);
+               reseed(false);
+       }
+
+
+       void reseed(boolean forced) {
+               long ts = System.currentTimeMillis();
+               if (forced || (ts - last) > RESEED_PERIOD) {
+                       last = ts;
+                       runReseed();
+               }
+       }
+
+
+
+       private void seed() {
+               random = new java.security.SecureRandom();
+               random.nextBytes(new byte[0]);
+               last = System.currentTimeMillis();
+       }
+
+
+       private void runReseed() {
+               // Have some other thread hit the penalty potentially incurred by reseeding,
+               // so that we can immediately return and not block the operation in progress.
+               new Thread() {
+                       public void run() {
+                               seed();
+                       }
+               }.start();
+       }
+}

diff --git a/src/test/java/com/gitblit/utils/SecureRandomTest.java b/src/test/java/com/gitblit/utils/SecureRandomTest.java
new file mode 100644 (file)
index 0000000..c4098c2
--- /dev/null
+++ b/src/test/java/com/gitblit/utils/SecureRandomTest.java
@@ -0,0 +1,33 @@
+package com.gitblit.utils;
+
+import static org.junit.Assert.*;
+
+import java.util.Arrays;
+
+import org.junit.Test;
+
+public class SecureRandomTest {
+
+       @Test
+       public void testRandomBytes() {
+               SecureRandom sr = new SecureRandom();
+               byte[] bytes1 = sr.randomBytes(10);
+               assertEquals(10, bytes1.length);
+               byte[] bytes2 = sr.randomBytes(10);
+               assertEquals(10, bytes2.length);
+               assertFalse(Arrays.equals(bytes1, bytes2));
+
+               assertEquals(0, sr.randomBytes(0).length);
+               assertEquals(200, sr.randomBytes(200).length);
+       }
+
+       @Test
+       public void testNextBytes() {
+               SecureRandom sr = new SecureRandom();
+               byte[] bytes1 = new byte[32];
+               sr.nextBytes(bytes1);
+               byte[] bytes2 = new byte[32];
+               sr.nextBytes(bytes2);
+               assertFalse(Arrays.equals(bytes1, bytes2));
+       }
+}