Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Prevent the creation of 'weak' PINs.
Simple checks to prevent the same number, or sequentially increasing/decreasing PINs. e.g. 1111, 1234, 54321, etc.
- Loading branch information
1 parent
b7296a4
commit 87eab27
Showing
9 changed files
with
287 additions
and
28 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 38 additions & 0 deletions
38
...est/java/org/thoughtcrime/securesms/registration/v2/PinValidityChecker_validity_Test.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package org.thoughtcrime.securesms.registration.v2; | ||
|
||
import org.junit.Test; | ||
import org.thoughtcrime.securesms.registration.v2.testdata.PinValidityVector; | ||
import org.thoughtcrime.securesms.util.Util; | ||
import org.whispersystems.signalservice.internal.registrationpin.PinValidityChecker; | ||
import org.whispersystems.signalservice.internal.util.JsonUtil; | ||
|
||
import java.io.IOException; | ||
import java.io.InputStream; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertTrue; | ||
|
||
public final class PinValidityChecker_validity_Test { | ||
|
||
@Test | ||
public void vectors_valid() throws IOException { | ||
for (PinValidityVector vector : getKbsPinValidityTestVectorList()) { | ||
boolean valid = PinValidityChecker.valid(vector.getPin()); | ||
|
||
assertEquals(String.format("%s [%s]", vector.getName(), vector.getPin()), | ||
vector.isValid(), | ||
valid); | ||
} | ||
} | ||
|
||
private static PinValidityVector[] getKbsPinValidityTestVectorList() throws IOException { | ||
try (InputStream resourceAsStream = ClassLoader.getSystemClassLoader().getResourceAsStream("data/kbs_pin_validity_vectors.json")) { | ||
|
||
PinValidityVector[] data = JsonUtil.fromJson(Util.readFullyAsString(resourceAsStream), PinValidityVector[].class); | ||
|
||
assertTrue(data.length > 0); | ||
|
||
return data; | ||
} | ||
} | ||
} |
27 changes: 27 additions & 0 deletions
27
app/src/test/java/org/thoughtcrime/securesms/registration/v2/testdata/PinValidityVector.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package org.thoughtcrime.securesms.registration.v2.testdata; | ||
|
||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
||
public class PinValidityVector { | ||
|
||
@JsonProperty("name") | ||
private String name; | ||
|
||
@JsonProperty("pin") | ||
private String pin; | ||
|
||
@JsonProperty("valid") | ||
private boolean valid; | ||
|
||
public String getName() { | ||
return name; | ||
} | ||
|
||
public String getPin() { | ||
return pin; | ||
} | ||
|
||
public boolean isValid() { | ||
return valid; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
[ | ||
{ | ||
"name": "Empty", | ||
"pin": "", | ||
"valid": false | ||
}, | ||
{ | ||
"name": "Alpha", | ||
"pin": "abcd", | ||
"valid": true | ||
}, | ||
{ | ||
"name": "Sequential", | ||
"pin": "1234", | ||
"valid": false | ||
}, | ||
{ | ||
"name": "Non-sequential", | ||
"pin": "6485", | ||
"valid": true | ||
}, | ||
{ | ||
"name": "Sequential descending", | ||
"pin": "43210", | ||
"valid": false | ||
}, | ||
{ | ||
"name": "Sequential with space", | ||
"pin": "1234 ", | ||
"valid": false | ||
}, | ||
{ | ||
"name": "Non-sequential with space", | ||
"pin": "1236 ", | ||
"valid": true | ||
}, | ||
{ | ||
"name": "Sequential Non-arabic digits", | ||
"pin": "١٢٣٤٥", | ||
"valid": false | ||
}, | ||
{ | ||
"name": "Sequential descending Non-arabic digits", | ||
"pin": "٥٤٣٢١", | ||
"valid": false | ||
}, | ||
{ | ||
"name": "Non-sequential Non-arabic digits", | ||
"pin": "١٢٣٥٤", | ||
"valid": true | ||
}, | ||
{ | ||
"name": "All digits the same", | ||
"pin": "9999", | ||
"valid": false | ||
}, | ||
{ | ||
"name": "All Non-arabic digits the same", | ||
"pin": "٢٢٢٢", | ||
"valid": false | ||
} | ||
] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
32 changes: 32 additions & 0 deletions
32
...ce/src/main/java/org/whispersystems/signalservice/internal/registrationpin/PinString.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package org.whispersystems.signalservice.internal.registrationpin; | ||
|
||
import org.whispersystems.signalservice.api.kbs.HashedPin; | ||
|
||
import java.nio.charset.StandardCharsets; | ||
import java.text.Normalizer; | ||
|
||
final class PinString { | ||
|
||
static boolean allNumeric(CharSequence pin) { | ||
for (int i = 0; i < pin.length(); i++) { | ||
if (!Character.isDigit(pin.charAt(i))) return false; | ||
} | ||
return true; | ||
} | ||
|
||
/** | ||
* Converts a string of not necessarily Arabic numerals to Arabic 0..9 characters. | ||
*/ | ||
static String toArabic(CharSequence numerals) { | ||
int length = numerals.length(); | ||
char[] arabic = new char[length]; | ||
|
||
for (int i = 0; i < length; i++) { | ||
int digit = Character.digit(numerals.charAt(i), 10); | ||
|
||
arabic[i] = (char) ('0' + digit); | ||
} | ||
|
||
return new String(arabic); | ||
} | ||
} |
Oops, something went wrong.
87eab27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. This address most of the concerns I had with short PINs. But it does not yet help against social engineering, an attacker coupd still try a personalized educated guess on the PIN of his target. If the target had set a 4 difit PIN chances are he used the same PIN as he has set to unlock his SIM with, or that he uses a family members year of birth as his PIN. It would imo be better to raise the minimum digits to 5, because years come in 4 digits, and SIM card PIN are also often 4 digits.
87eab27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's please also ban 1984. the nature of Signal makes it extremely likely that that will be a commonly used PIN.