From 5d72248bb526948dcaf72a306dd24160b11e6063 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 29 Jul 2026 16:23:29 -0400 Subject: [PATCH] Move PinValidityCheck to :core:util and kotlinize it. --- .../lock/v2/CreateSvrPinViewModel.java | 2 +- .../v2/PinValidityChecker_validity_Test.java | 38 --------- .../v2/testdata/PinValidityVector.java | 27 ------- .../pincreation/PinCreationViewModel.kt | 2 +- .../signalservice/api/kbs/PinString.java | 6 +- .../api/kbs/PinValidityChecker.java | 78 ------------------- .../signal/network/pin/PinValidityChecker.kt | 40 ++++++++++ .../network/pin/PinValidityCheckerTest.kt | 46 +++++++++++ .../data/kbs_pin_validity_vectors.json | 0 9 files changed, 91 insertions(+), 148 deletions(-) delete mode 100644 app/src/test/java/org/thoughtcrime/securesms/registration/v2/PinValidityChecker_validity_Test.java delete mode 100644 app/src/test/java/org/thoughtcrime/securesms/registration/v2/testdata/PinValidityVector.java delete mode 100644 lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/kbs/PinValidityChecker.java create mode 100644 lib/network/src/main/java/org/signal/network/pin/PinValidityChecker.kt create mode 100644 lib/network/src/test/java/org/signal/network/pin/PinValidityCheckerTest.kt rename {app => lib/network}/src/test/resources/data/kbs_pin_validity_vectors.json (100%) diff --git a/app/src/main/java/org/thoughtcrime/securesms/lock/v2/CreateSvrPinViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/lock/v2/CreateSvrPinViewModel.java index bbd02c357d..eb50cac0b3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/lock/v2/CreateSvrPinViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/lock/v2/CreateSvrPinViewModel.java @@ -7,7 +7,7 @@ import androidx.lifecycle.MutableLiveData; import androidx.lifecycle.ViewModel; import org.thoughtcrime.securesms.util.SingleLiveEvent; -import org.whispersystems.signalservice.api.kbs.PinValidityChecker; +import org.signal.network.pin.PinValidityChecker; import org.signal.network.util.Preconditions; public final class CreateSvrPinViewModel extends ViewModel implements BaseSvrPinViewModel { diff --git a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/PinValidityChecker_validity_Test.java b/app/src/test/java/org/thoughtcrime/securesms/registration/v2/PinValidityChecker_validity_Test.java deleted file mode 100644 index e39ed3d976..0000000000 --- a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/PinValidityChecker_validity_Test.java +++ /dev/null @@ -1,38 +0,0 @@ -package org.thoughtcrime.securesms.registration.v2; - -import org.junit.Test; -import org.signal.core.util.StreamUtil; -import org.thoughtcrime.securesms.registration.testdata.PinValidityVector; -import org.whispersystems.signalservice.api.kbs.PinValidityChecker; -import org.signal.network.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(StreamUtil.readFullyAsString(resourceAsStream), PinValidityVector[].class); - - assertTrue(data.length > 0); - - return data; - } - } -} diff --git a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/testdata/PinValidityVector.java b/app/src/test/java/org/thoughtcrime/securesms/registration/v2/testdata/PinValidityVector.java deleted file mode 100644 index 7688e9ed31..0000000000 --- a/app/src/test/java/org/thoughtcrime/securesms/registration/v2/testdata/PinValidityVector.java +++ /dev/null @@ -1,27 +0,0 @@ -package org.thoughtcrime.securesms.registration.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; - } -} \ No newline at end of file diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt index 01781a4124..12429b92d1 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt @@ -17,12 +17,12 @@ import kotlinx.coroutines.flow.onEach import org.signal.core.ui.compose.EventDrivenViewModel import org.signal.core.util.logging.Log import org.signal.libsignal.net.RequestResult +import org.signal.network.pin.PinValidityChecker import org.signal.registration.NetworkController import org.signal.registration.RegistrationFlowEvent import org.signal.registration.RegistrationFlowState import org.signal.registration.RegistrationRepository import org.signal.registration.RestoreDecision -import org.whispersystems.signalservice.api.kbs.PinValidityChecker import kotlin.time.toKotlinDuration /** diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/kbs/PinString.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/kbs/PinString.java index f4afcfceb6..c3a2d566b5 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/kbs/PinString.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/kbs/PinString.java @@ -5,9 +5,9 @@ package org.whispersystems.signalservice.api.kbs; -final class PinString { +public final class PinString { - static boolean allNumeric(CharSequence pin) { + public static boolean allNumeric(CharSequence pin) { for (int i = 0; i < pin.length(); i++) { if (!Character.isDigit(pin.charAt(i))) return false; } @@ -17,7 +17,7 @@ final class PinString { /** * Converts a string of not necessarily Arabic numerals to Arabic 0..9 characters. */ - static String toArabic(CharSequence numerals) { + public static String toArabic(CharSequence numerals) { int length = numerals.length(); char[] arabic = new char[length]; diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/kbs/PinValidityChecker.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/kbs/PinValidityChecker.java deleted file mode 100644 index eb88aa7025..0000000000 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/kbs/PinValidityChecker.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright 2023 Signal Messenger, LLC - * SPDX-License-Identifier: AGPL-3.0-only - */ - -package org.whispersystems.signalservice.api.kbs; - -public final class PinValidityChecker { - - public static boolean valid(String pin) { - pin = pin.trim(); - - if (pin.isEmpty()) { - return false; - } - - if (PinString.allNumeric(pin)) { - pin = PinString.toArabic(pin); - - return !sequential(pin) && - !sequential(reverse(pin)) && - !allTheSame(pin); - } else { - return true; - } - } - - private static String reverse(String string) { - char[] chars = string.toCharArray(); - - for (int i = 0; i < chars.length / 2; i++) { - char temp = chars[i]; - chars[i] = chars[chars.length - i - 1]; - chars[chars.length - i - 1] = temp; - } - - return new String(chars); - } - - private static boolean sequential(String pin) { - int length = pin.length(); - - if (length == 0) { - return false; - } - - char c = pin.charAt(0); - - for (int i = 1; i < length; i++) { - char n = pin.charAt(i); - if (n != c + 1) { - return false; - } - c = n; - } - - return true; - } - - private static boolean allTheSame(String pin) { - int length = pin.length(); - - if (length == 0) { - return false; - } - - char c = pin.charAt(0); - - for (int i = 1; i < length; i++) { - char n = pin.charAt(i); - if (n != c) { - return false; - } - } - - return true; - } -} diff --git a/lib/network/src/main/java/org/signal/network/pin/PinValidityChecker.kt b/lib/network/src/main/java/org/signal/network/pin/PinValidityChecker.kt new file mode 100644 index 0000000000..a5e94558a9 --- /dev/null +++ b/lib/network/src/main/java/org/signal/network/pin/PinValidityChecker.kt @@ -0,0 +1,40 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.signal.network.pin + +import org.whispersystems.signalservice.api.kbs.PinString + +/** + * Rejects PINs that are trivially guessable. A numeric PIN must not be empty, sequential in either + * direction, or a single repeated digit. Non-numeric PINs are only checked for emptiness. + */ +object PinValidityChecker { + + @JvmStatic + fun valid(pin: String): Boolean { + val trimmed = pin.trim() + + if (trimmed.isEmpty()) { + return false + } + + if (!PinString.allNumeric(trimmed)) { + return true + } + + val arabic = PinString.toArabic(trimmed) + + return !arabic.isSequential() && !arabic.reversed().isSequential() && !arabic.isSingleRepeatedChar() + } + + private fun String.isSequential(): Boolean { + return zipWithNext().all { (previous, next) -> next == previous + 1 } + } + + private fun String.isSingleRepeatedChar(): Boolean { + return all { it == this[0] } + } +} diff --git a/lib/network/src/test/java/org/signal/network/pin/PinValidityCheckerTest.kt b/lib/network/src/test/java/org/signal/network/pin/PinValidityCheckerTest.kt new file mode 100644 index 0000000000..8f6cf8d01f --- /dev/null +++ b/lib/network/src/test/java/org/signal/network/pin/PinValidityCheckerTest.kt @@ -0,0 +1,46 @@ +/* + * Copyright 2026 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.signal.network.pin + +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isNotEmpty +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.Json +import org.junit.Test + +class PinValidityCheckerTest { + + @Test + fun `validity matches the shared test vectors`() { + val vectors = loadVectors() + + assertThat(vectors).isNotEmpty() + + for (vector in vectors) { + assertThat(PinValidityChecker.valid(vector.pin), "${vector.name} [${vector.pin}]").isEqualTo(vector.valid) + } + } + + private fun loadVectors(): List { + val json = checkNotNull(javaClass.classLoader.getResourceAsStream(VECTOR_RESOURCE)) { "Missing $VECTOR_RESOURCE" } + .bufferedReader() + .use { it.readText() } + + return Json.decodeFromString(json) + } + + @Serializable + private data class PinValidityVector( + val name: String, + val pin: String, + val valid: Boolean + ) + + companion object { + private const val VECTOR_RESOURCE = "data/kbs_pin_validity_vectors.json" + } +} diff --git a/app/src/test/resources/data/kbs_pin_validity_vectors.json b/lib/network/src/test/resources/data/kbs_pin_validity_vectors.json similarity index 100% rename from app/src/test/resources/data/kbs_pin_validity_vectors.json rename to lib/network/src/test/resources/data/kbs_pin_validity_vectors.json