diff --git a/client/src/main/java/org/asynchttpclient/util/HttpUtils.java b/client/src/main/java/org/asynchttpclient/util/HttpUtils.java index 3cca41e616..e67b699a53 100644 --- a/client/src/main/java/org/asynchttpclient/util/HttpUtils.java +++ b/client/src/main/java/org/asynchttpclient/util/HttpUtils.java @@ -24,8 +24,8 @@ import java.net.URLEncoder; import java.nio.ByteBuffer; import java.nio.charset.Charset; +import java.security.SecureRandom; import java.util.List; -import java.util.concurrent.ThreadLocalRandom; import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; @@ -114,9 +114,13 @@ public static String originHeader(Uri uri) { // The pool of ASCII chars to be used for generating a multipart boundary. private static final byte[] MULTIPART_CHARS = "-_1234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ".getBytes(US_ASCII); + // The boundary is what separates parts whose content is not escaped, so it must be unpredictable, + // like the cnonce in Realm and the SCRAM nonce. + private static final ThreadLocal BOUNDARY_RANDOM = ThreadLocal.withInitial(SecureRandom::new); + // a random size from 30 to 40 public static byte[] computeMultipartBoundary() { - ThreadLocalRandom random = ThreadLocalRandom.current(); + SecureRandom random = BOUNDARY_RANDOM.get(); byte[] bytes = new byte[random.nextInt(11) + 30]; for (int i = 0; i < bytes.length; i++) { bytes[i] = MULTIPART_CHARS[random.nextInt(MULTIPART_CHARS.length)]; diff --git a/client/src/main/java/org/asynchttpclient/ws/WebSocketUtils.java b/client/src/main/java/org/asynchttpclient/ws/WebSocketUtils.java index 628cc1d7df..3a20b1691a 100644 --- a/client/src/main/java/org/asynchttpclient/ws/WebSocketUtils.java +++ b/client/src/main/java/org/asynchttpclient/ws/WebSocketUtils.java @@ -15,8 +15,7 @@ */ package org.asynchttpclient.ws; -import io.netty.util.internal.ThreadLocalRandom; - +import java.security.SecureRandom; import java.util.Base64; import static java.nio.charset.StandardCharsets.US_ASCII; @@ -25,16 +24,16 @@ public final class WebSocketUtils { private static final String MAGIC_GUID = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"; + // RFC 6455 section 10.3 requires the handshake nonce to come from a strong source of entropy. + private static final ThreadLocal KEY_RANDOM = ThreadLocal.withInitial(SecureRandom::new); + private WebSocketUtils() { // Prevent outside initialization } public static String getWebSocketKey() { byte[] nonce = new byte[16]; - ThreadLocalRandom random = ThreadLocalRandom.current(); - for (int i = 0; i < nonce.length; i++) { - nonce[i] = (byte) random.nextInt(256); - } + KEY_RANDOM.get().nextBytes(nonce); return Base64.getEncoder().encodeToString(nonce); } diff --git a/client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java b/client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java index 57d031498d..eda97ffa4c 100644 --- a/client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java +++ b/client/src/test/java/org/asynchttpclient/util/HttpUtilsTest.java @@ -28,7 +28,9 @@ import java.nio.ByteBuffer; import java.nio.charset.Charset; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import static io.netty.handler.codec.http.HttpHeaderValues.APPLICATION_JSON; import static java.nio.charset.StandardCharsets.ISO_8859_1; @@ -119,6 +121,20 @@ public void testGetFollowRedirectPriorityGivenToRequest() { assertFalse(followRedirect, "Follow redirect value set in request should be given priority"); } + @RepeatedIfExceptionsTest(repeats = 5) + public void testComputeMultipartBoundary() { + String allowed = "-_1234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; + Set boundaries = new HashSet<>(); + for (int i = 0; i < 1000; i++) { + String boundary = new String(HttpUtils.computeMultipartBoundary(), US_ASCII); + assertTrue(boundary.length() >= 30 && boundary.length() <= 40, "Unexpected boundary length: " + boundary.length()); + for (int j = 0; j < boundary.length(); j++) { + assertTrue(allowed.indexOf(boundary.charAt(j)) != -1, "Illegal boundary char: " + boundary.charAt(j)); + } + assertTrue(boundaries.add(boundary), "Boundary was generated twice: " + boundary); + } + } + private static void formUrlEncoding(Charset charset) throws Exception { String key = "key"; String value = "中文";