Skip to content

Commit 0e3e47b

Browse files
committed
Use a SecureRandom instead of Math.random for user cookie.
To get proper entropy in user authentication cookie creation, make use of `SecureRandom` instead of using `Math.random()`, or `Random`. 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 gitblit-org#1063 and extends and closes PR gitblit-org#1116
1 parent cc311f5 commit 0e3e47b

File tree

3 files changed

+121
-2
lines changed

3 files changed

+121
-2
lines changed

src/main/java/com/gitblit/models/UserModel.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.gitblit.Constants.RegistrantType;
3737
import com.gitblit.utils.ArrayUtils;
3838
import com.gitblit.utils.ModelUtils;
39+
import com.gitblit.utils.SecureRandom;
3940
import com.gitblit.utils.StringUtils;
4041

4142
/**
@@ -52,6 +53,8 @@ public class UserModel implements Principal, Serializable, Comparable<UserModel>
5253

5354
public static final UserModel ANONYMOUS = new UserModel();
5455

56+
private static final SecureRandom RANDOM = new SecureRandom();
57+
5558
// field names are reflectively mapped in EditUser page
5659
public String username;
5760
public String password;
@@ -660,8 +663,8 @@ public boolean isMyPersonalRepository(String repository) {
660663
String projectPath = StringUtils.getFirstPathElement(repository);
661664
return !StringUtils.isEmpty(projectPath) && projectPath.equalsIgnoreCase(getPersonalPath());
662665
}
663-
666+
664667
public String createCookie() {
665-
return StringUtils.getSHA1(String.valueOf(Math.random()));
668+
return StringUtils.getSHA1(RANDOM.randomBytes(32));
666669
}
667670
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright 2016 gitblit.com
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.gitblit.utils;
17+
18+
/**
19+
* Wrapper class for java.security.SecureRandom, which will periodically reseed
20+
* the PRNG in case an instance of the class has been running for a long time.
21+
*
22+
* @author Florian Zschocke
23+
*/
24+
public class SecureRandom {
25+
26+
/** Period (in ms) after which a new SecureRandom will be created in order to get a fresh random seed. */
27+
private static final long RESEED_PERIOD = 24 * 60 * 60 * 1000; /* 24 hours */
28+
29+
30+
private long last;
31+
private java.security.SecureRandom random;
32+
33+
34+
35+
public SecureRandom() {
36+
// Make sure the SecureRandom is seeded right from the start.
37+
// This also lets any blocks during seeding occur at creation
38+
// and prevents it from happening when getting next random bytes.
39+
seed();
40+
}
41+
42+
43+
44+
public byte[] randomBytes(int num) {
45+
byte[] bytes = new byte[num];
46+
nextBytes(bytes);
47+
return bytes;
48+
}
49+
50+
51+
public void nextBytes(byte[] bytes) {
52+
random.nextBytes(bytes);
53+
reseed(false);
54+
}
55+
56+
57+
void reseed(boolean forced) {
58+
long ts = System.currentTimeMillis();
59+
if (forced || (ts - last) > RESEED_PERIOD) {
60+
last = ts;
61+
runReseed();
62+
}
63+
}
64+
65+
66+
67+
private void seed() {
68+
random = new java.security.SecureRandom();
69+
random.nextBytes(new byte[0]);
70+
last = System.currentTimeMillis();
71+
}
72+
73+
74+
private void runReseed() {
75+
// Have some other thread hit the penalty potentially incurred by reseeding,
76+
// so that we can immediately return and not block the operation in progress.
77+
new Thread() {
78+
public void run() {
79+
seed();
80+
}
81+
}.start();
82+
}
83+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package com.gitblit.utils;
2+
3+
import static org.junit.Assert.*;
4+
5+
import java.util.Arrays;
6+
7+
import org.junit.Test;
8+
9+
public class SecureRandomTest {
10+
11+
@Test
12+
public void testRandomBytes() {
13+
SecureRandom sr = new SecureRandom();
14+
byte[] bytes1 = sr.randomBytes(10);
15+
assertEquals(10, bytes1.length);
16+
byte[] bytes2 = sr.randomBytes(10);
17+
assertEquals(10, bytes2.length);
18+
assertFalse(Arrays.equals(bytes1, bytes2));
19+
20+
assertEquals(0, sr.randomBytes(0).length);
21+
assertEquals(200, sr.randomBytes(200).length);
22+
}
23+
24+
@Test
25+
public void testNextBytes() {
26+
SecureRandom sr = new SecureRandom();
27+
byte[] bytes1 = new byte[32];
28+
sr.nextBytes(bytes1);
29+
byte[] bytes2 = new byte[32];
30+
sr.nextBytes(bytes2);
31+
assertFalse(Arrays.equals(bytes1, bytes2));
32+
}
33+
}

0 commit comments

Comments
 (0)