From 0179c436714d3b4abe63081438883ffc5037556c Mon Sep 17 00:00:00 2001 From: Jens Bannmann Date: Mon, 20 Jul 2015 09:49:41 +0200 Subject: [PATCH 1/3] SCryptUtil now accepts char[] passwords in addition to Strings - issue #31 --- .../com/lambdaworks/crypto/SCryptUtil.java | 71 ++++++++++++++++--- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/lambdaworks/crypto/SCryptUtil.java b/src/main/java/com/lambdaworks/crypto/SCryptUtil.java index ca29a00..a11bc78 100644 --- a/src/main/java/com/lambdaworks/crypto/SCryptUtil.java +++ b/src/main/java/com/lambdaworks/crypto/SCryptUtil.java @@ -2,9 +2,12 @@ package com.lambdaworks.crypto; -import java.io.UnsupportedEncodingException; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.security.SecureRandom; +import java.util.Arrays; import static com.lambdaworks.codec.Base64.*; @@ -40,11 +43,35 @@ public class SCryptUtil { * @return The hashed password. */ public static String scrypt(String passwd, int N, int r, int p) { + byte[] bytes = passwd.getBytes(StandardCharsets.UTF_8); + String result = scryptInternal(bytes, N, r, p); + wipeArray(bytes); + return result; + } + + /** + * Hash the supplied plaintext password and generate output in the format described + * in {@link SCryptUtil}. + * + * @param passwd Password. + * @param N CPU cost parameter. + * @param r Memory cost parameter. + * @param p Parallelization parameter. + * @return The hashed password. + */ + public static String scrypt(char[] passwd, int N, int r, int p) { + byte[] bytes = toBytes(passwd); + String result = scryptInternal(bytes, N, r, p); + wipeArray(bytes); + return result; + } + + private static String scryptInternal(byte[] passwordBytes, int N, int r, int p) { try { byte[] salt = new byte[16]; SecureRandom.getInstance("SHA1PRNG").nextBytes(salt); - byte[] derived = SCrypt.scrypt(passwd.getBytes("UTF-8"), salt, N, r, p, 32); + byte[] derived = SCrypt.scrypt(passwordBytes, salt, N, r, p, 32); String params = Long.toString(log2(N) << 16L | r << 8 | p, 16); @@ -54,22 +81,48 @@ public static String scrypt(String passwd, int N, int r, int p) { sb.append(encode(derived)); return sb.toString(); - } catch (UnsupportedEncodingException e) { - throw new IllegalStateException("JVM doesn't support UTF-8?"); } catch (GeneralSecurityException e) { throw new IllegalStateException("JVM doesn't support SHA1PRNG or HMAC_SHA256?"); } } + private static byte[] toBytes(char[] chars) { + CharBuffer charBuffer = CharBuffer.wrap(chars); + ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(charBuffer); + byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); + wipeArray(byteBuffer.array()); + return bytes; + } + /** * Compare the supplied plaintext password to a hashed password. * * @param passwd Plaintext password. * @param hashed scrypt hashed password. - * * @return true if passwd matches hashed value. */ public static boolean check(String passwd, String hashed) { + byte[] bytes = passwd.getBytes(StandardCharsets.UTF_8); + boolean result = checkInternal(bytes, hashed); + wipeArray(bytes); + return result; + } + + /** + * Compare the supplied plaintext password to a hashed password. + * + * @param passwd Plaintext password. + * @param hashed scrypt hashed password. + * @return true if passwd matches hashed value. + */ + public static boolean check(char[] passwd, String hashed) { + byte[] bytes = toBytes(passwd); + boolean result = checkInternal(bytes, hashed); + wipeArray(bytes); + return result; + } + + private static boolean checkInternal(byte[] passwordBytes, String hashed) { try { String[] parts = hashed.split("\\$"); @@ -85,7 +138,7 @@ public static boolean check(String passwd, String hashed) { int r = (int) params >> 8 & 0xff; int p = (int) params & 0xff; - byte[] derived1 = SCrypt.scrypt(passwd.getBytes("UTF-8"), salt, N, r, p, 32); + byte[] derived1 = SCrypt.scrypt(passwordBytes, salt, N, r, p, 32); if (derived0.length != derived1.length) return false; @@ -94,8 +147,6 @@ public static boolean check(String passwd, String hashed) { result |= derived0[i] ^ derived1[i]; } return result == 0; - } catch (UnsupportedEncodingException e) { - throw new IllegalStateException("JVM doesn't support UTF-8?"); } catch (GeneralSecurityException e) { throw new IllegalStateException("JVM doesn't support SHA1PRNG or HMAC_SHA256?"); } @@ -109,4 +160,8 @@ private static int log2(int n) { if (n >= 4 ) { n >>>= 2; log += 2; } return log + (n >>> 1); } + + private static void wipeArray(byte[] array) { + Arrays.fill(array, (byte) 0); + } } From 12d73a38b2ed3c0c38cde98a84056b5b4d50f539 Mon Sep 17 00:00:00 2001 From: Jens Bannmann Date: Mon, 20 Jul 2015 10:04:39 +0200 Subject: [PATCH 2/3] Ensure arrays are wiped in any case --- .../com/lambdaworks/crypto/SCryptUtil.java | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/lambdaworks/crypto/SCryptUtil.java b/src/main/java/com/lambdaworks/crypto/SCryptUtil.java index a11bc78..101653e 100644 --- a/src/main/java/com/lambdaworks/crypto/SCryptUtil.java +++ b/src/main/java/com/lambdaworks/crypto/SCryptUtil.java @@ -44,9 +44,11 @@ public class SCryptUtil { */ public static String scrypt(String passwd, int N, int r, int p) { byte[] bytes = passwd.getBytes(StandardCharsets.UTF_8); - String result = scryptInternal(bytes, N, r, p); - wipeArray(bytes); - return result; + try { + return scryptInternal(bytes, N, r, p); + } finally { + wipeArray(bytes); + } } /** @@ -61,9 +63,11 @@ public static String scrypt(String passwd, int N, int r, int p) { */ public static String scrypt(char[] passwd, int N, int r, int p) { byte[] bytes = toBytes(passwd); - String result = scryptInternal(bytes, N, r, p); - wipeArray(bytes); - return result; + try { + return scryptInternal(bytes, N, r, p); + } finally { + wipeArray(bytes); + } } private static String scryptInternal(byte[] passwordBytes, int N, int r, int p) { @@ -89,9 +93,11 @@ private static String scryptInternal(byte[] passwordBytes, int N, int r, int p) private static byte[] toBytes(char[] chars) { CharBuffer charBuffer = CharBuffer.wrap(chars); ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(charBuffer); - byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); - wipeArray(byteBuffer.array()); - return bytes; + try { + return Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); + } finally { + wipeArray(byteBuffer.array()); + } } /** @@ -103,9 +109,11 @@ private static byte[] toBytes(char[] chars) { */ public static boolean check(String passwd, String hashed) { byte[] bytes = passwd.getBytes(StandardCharsets.UTF_8); - boolean result = checkInternal(bytes, hashed); - wipeArray(bytes); - return result; + try { + return checkInternal(bytes, hashed); + } finally { + wipeArray(bytes); + } } /** @@ -117,9 +125,11 @@ public static boolean check(String passwd, String hashed) { */ public static boolean check(char[] passwd, String hashed) { byte[] bytes = toBytes(passwd); - boolean result = checkInternal(bytes, hashed); - wipeArray(bytes); - return result; + try { + return checkInternal(bytes, hashed); + } finally { + wipeArray(bytes); + } } private static boolean checkInternal(byte[] passwordBytes, String hashed) { From dd815a2e9080c6a9c10fe2f0592f491926f27d06 Mon Sep 17 00:00:00 2001 From: cleberz Date: Sun, 7 Oct 2018 05:20:20 -0500 Subject: [PATCH 3/3] Ensured no conversions to String take place and wiped out intermediate char arrays --- .../com/lambdaworks/crypto/SCryptUtil.java | 168 ++++++++++++------ 1 file changed, 118 insertions(+), 50 deletions(-) diff --git a/src/main/java/com/lambdaworks/crypto/SCryptUtil.java b/src/main/java/com/lambdaworks/crypto/SCryptUtil.java index 101653e..9fb85db 100644 --- a/src/main/java/com/lambdaworks/crypto/SCryptUtil.java +++ b/src/main/java/com/lambdaworks/crypto/SCryptUtil.java @@ -7,7 +7,9 @@ import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.security.SecureRandom; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import static com.lambdaworks.codec.Base64.*; @@ -31,9 +33,11 @@ * @author Will Glozer */ public class SCryptUtil { + /** * Hash the supplied plaintext password and generate output in the format described - * in {@link SCryptUtil}. + * in {@link SCryptUtil}. This method maybe unsafe as it uses Strings, which are not guaranteed to be + * freed immediately. * * @param passwd Password. * @param N CPU cost parameter. @@ -45,46 +49,69 @@ public class SCryptUtil { public static String scrypt(String passwd, int N, int r, int p) { byte[] bytes = passwd.getBytes(StandardCharsets.UTF_8); try { - return scryptInternal(bytes, N, r, p); + return new String(scrypt(bytes, N, r, p), StandardCharsets.UTF_8); } finally { wipeArray(bytes); } } - + /** * Hash the supplied plaintext password and generate output in the format described - * in {@link SCryptUtil}. + * in {@link SCryptUtil}. This call will aggressively clean up password data in memory. + * + * @param passwd Password in UTF-8 encoding. + * @param N CPU cost parameter. + * @param r Memory cost parameter. + * @param p Parallelization parameter. * - * @param passwd Password. - * @param N CPU cost parameter. - * @param r Memory cost parameter. - * @param p Parallelization parameter. * @return The hashed password. */ - public static String scrypt(char[] passwd, int N, int r, int p) { - byte[] bytes = toBytes(passwd); - try { - return scryptInternal(bytes, N, r, p); - } finally { - wipeArray(bytes); - } - } - - private static String scryptInternal(byte[] passwordBytes, int N, int r, int p) { + public static byte[] scrypt(byte[] passwordBytes, int N, int r, int p) { try { byte[] salt = new byte[16]; SecureRandom.getInstance("SHA1PRNG").nextBytes(salt); byte[] derived = SCrypt.scrypt(passwordBytes, salt, N, r, p, 32); - String params = Long.toString(log2(N) << 16L | r << 8 | p, 16); - - StringBuilder sb = new StringBuilder((salt.length + derived.length) * 2); - sb.append("$s0$").append(params).append('$'); - sb.append(encode(salt)).append('$'); - sb.append(encode(derived)); - - return sb.toString(); + byte[] params = Long.toString(log2(N) << 16L | r << 8 | p, 16).getBytes(StandardCharsets.UTF_8); + + byte[] prefix = "$s0$".getBytes(StandardCharsets.UTF_8); + byte[] dollar = "$".getBytes(StandardCharsets.UTF_8); + final char[] charEncodedSalt = encode(salt); + byte[] byteEncodedSalt = toBytes(charEncodedSalt); + wipeArray(charEncodedSalt); + final char[] charEncodedDerived = encode(derived); + byte[] byteEncodedDerived = toBytes(charEncodedDerived); + wipeArray(charEncodedDerived); + + byte[] result = new byte[prefix.length + + params.length + + dollar.length + + byteEncodedSalt.length + + dollar.length + + byteEncodedDerived.length]; + System.arraycopy(prefix, 0, result, 0, prefix.length); + System.arraycopy(params, 0, result, prefix.length, params.length); + System.arraycopy(dollar, 0, result, prefix.length + + params.length, dollar.length); + System.arraycopy(byteEncodedSalt, 0, result, prefix.length + + params.length + + dollar.length, byteEncodedSalt.length); + System.arraycopy(dollar, 0, result, prefix.length + + params.length + + dollar.length + + byteEncodedSalt.length, dollar.length); + System.arraycopy(byteEncodedDerived, 0, result, prefix.length + + params.length + + dollar.length + + byteEncodedSalt.length + + dollar.length, byteEncodedDerived.length); + wipeArray(salt); + wipeArray(derived); + wipeArray(params); + wipeArray(byteEncodedSalt); + wipeArray(byteEncodedDerived); + return result; } catch (GeneralSecurityException e) { throw new IllegalStateException("JVM doesn't support SHA1PRNG or HMAC_SHA256?"); } @@ -102,47 +129,42 @@ private static byte[] toBytes(char[] chars) { /** * Compare the supplied plaintext password to a hashed password. + * This method maybe unsafe as it uses Strings, which are not guaranteed to be + * freed immediatelly. * * @param passwd Plaintext password. * @param hashed scrypt hashed password. + * * @return true if passwd matches hashed value. */ public static boolean check(String passwd, String hashed) { - byte[] bytes = passwd.getBytes(StandardCharsets.UTF_8); - try { - return checkInternal(bytes, hashed); - } finally { - wipeArray(bytes); - } + return check(passwd.getBytes(StandardCharsets.UTF_8), hashed.getBytes(StandardCharsets.UTF_8)); } - + /** * Compare the supplied plaintext password to a hashed password. + * This call will aggressively clean up password data in memory. + * + * @param passwd Plaintext password encoded in UTF-8. + * @param hashed scrypt hashed password encoded in UTF-8. * - * @param passwd Plaintext password. - * @param hashed scrypt hashed password. * @return true if passwd matches hashed value. */ - public static boolean check(char[] passwd, String hashed) { - byte[] bytes = toBytes(passwd); - try { - return checkInternal(bytes, hashed); - } finally { - wipeArray(bytes); - } - } - - private static boolean checkInternal(byte[] passwordBytes, String hashed) { + public static boolean check(byte[] passwordBytes, byte[] hashed) { try { - String[] parts = hashed.split("\\$"); + byte[][] parts = split(hashed, "$".getBytes(StandardCharsets.UTF_8)); - if (parts.length != 5 || !parts[1].equals("s0")) { + if (parts.length != 5 || !Arrays.equals(parts[1], "s0".getBytes(StandardCharsets.UTF_8))) { throw new IllegalArgumentException("Invalid hashed value"); } - long params = Long.parseLong(parts[2], 16); - byte[] salt = decode(parts[3].toCharArray()); - byte[] derived0 = decode(parts[4].toCharArray()); + long params = Long.parseLong(new String(parts[2], StandardCharsets.UTF_8), 16); + final char[] charEncodedSalt = toCharArray(parts[3]); + byte[] salt = decode(charEncodedSalt); + wipeArray(charEncodedSalt); + final char[] charEncodedDerived = toCharArray(parts[4]); + byte[] derived0 = decode(charEncodedDerived); + wipeArray(charEncodedDerived); int N = (int) Math.pow(2, params >> 16 & 0xffff); int r = (int) params >> 8 & 0xff; @@ -156,12 +178,53 @@ private static boolean checkInternal(byte[] passwordBytes, String hashed) { for (int i = 0; i < derived0.length; i++) { result |= derived0[i] ^ derived1[i]; } + wipeArray(derived0); + wipeArray(derived1); return result == 0; } catch (GeneralSecurityException e) { throw new IllegalStateException("JVM doesn't support SHA1PRNG or HMAC_SHA256?"); } } + private static char[] toCharArray(byte[] bs) { + char[] result = new char[bs.length]; + for(int i = 0; i < bs.length; i++) { + result[i] = (char) bs[i]; + } + return result; + } + + /** + * Splites a byte array into chunks delimited by separatorBytes, similar + * to {@link String#split}. + */ + private static byte[][] split(byte[] array, byte[] separatorBytes) { + List result = new ArrayList(); + int lastSplitIndex = 0; + for(int i = 0; i < array.length - separatorBytes.length; i++) + { + boolean found = false; + int j = 0; + while (j < separatorBytes.length && array[i + j] == separatorBytes[j]) + j++; + found = (j == separatorBytes.length); + if(found) { + byte[] substring = new byte[i - lastSplitIndex]; + System.arraycopy(array, lastSplitIndex, substring, 0, i - lastSplitIndex); + result.add(substring); + lastSplitIndex = i + j; + } else if(array.length == i + separatorBytes.length + 1) { + byte[] substring = new byte[i + separatorBytes.length + 1 - lastSplitIndex]; + System.arraycopy(array, lastSplitIndex, substring, 0, i + separatorBytes.length + 1 - lastSplitIndex); + result.add(substring); + } + } + if(result.isEmpty()) { + result.add(array); + } + return result.toArray(new byte[0][]); + } + private static int log2(int n) { int log = 0; if ((n & 0xffff0000 ) != 0) { n >>>= 16; log = 16; } @@ -174,4 +237,9 @@ private static int log2(int n) { private static void wipeArray(byte[] array) { Arrays.fill(array, (byte) 0); } + + private static void wipeArray(char[] array) { + Arrays.fill(array, (char) 0); + } + }