From 79dbf85c1e2c78212796c4dc118bd5cc53eae30b Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 29 May 2020 18:16:38 -0400 Subject: [PATCH] Improve local encrypted PIN storage. --- .../securesms/jobs/JobManagerFactories.java | 2 + .../securesms/keyvalue/KbsValues.java | 16 ++++-- .../securesms/keyvalue/PinValues.java | 16 +++++- .../lock/SignalPinReminderDialog.java | 12 ++--- .../securesms/megaphone/Megaphones.java | 6 +-- .../migrations/ApplicationMigrations.java | 7 ++- .../migrations/PinReminderMigrationJob.java | 50 +++++++++++++++++++ .../thoughtcrime/securesms/pin/PinState.java | 12 ++--- 8 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/migrations/PinReminderMigrationJob.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java index 5c7a68df1c..19dd7ae9f0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java @@ -29,6 +29,7 @@ import org.thoughtcrime.securesms.migrations.DatabaseMigrationJob; import org.thoughtcrime.securesms.migrations.LegacyMigrationJob; import org.thoughtcrime.securesms.migrations.MigrationCompleteJob; import org.thoughtcrime.securesms.migrations.PassingMigrationJob; +import org.thoughtcrime.securesms.migrations.PinReminderMigrationJob; import org.thoughtcrime.securesms.migrations.RecipientSearchMigrationJob; import org.thoughtcrime.securesms.migrations.RegistrationPinV2MigrationJob; import org.thoughtcrime.securesms.migrations.StickerAdditionMigrationJob; @@ -124,6 +125,7 @@ public final class JobManagerFactories { put(DatabaseMigrationJob.KEY, new DatabaseMigrationJob.Factory()); put(LegacyMigrationJob.KEY, new LegacyMigrationJob.Factory()); put(MigrationCompleteJob.KEY, new MigrationCompleteJob.Factory()); + put(PinReminderMigrationJob.KEY, new PinReminderMigrationJob.Factory()); put(RecipientSearchMigrationJob.KEY, new RecipientSearchMigrationJob.Factory()); put(RegistrationPinV2MigrationJob.KEY, new RegistrationPinV2MigrationJob.Factory()); put(StickerLaunchMigrationJob.KEY, new StickerLaunchMigrationJob.Factory()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/KbsValues.java b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/KbsValues.java index a6a13ba10c..75c8953a56 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/KbsValues.java +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/KbsValues.java @@ -3,8 +3,7 @@ package org.thoughtcrime.securesms.keyvalue; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import org.thoughtcrime.securesms.logging.Log; -import org.thoughtcrime.securesms.util.Base64; +import org.thoughtcrime.securesms.lock.PinHashing; import org.thoughtcrime.securesms.util.JsonUtils; import org.whispersystems.signalservice.api.KbsPinData; import org.whispersystems.signalservice.api.kbs.MasterKey; @@ -18,6 +17,7 @@ public final class KbsValues { public static final String V2_LOCK_ENABLED = "kbs.v2_lock_enabled"; private static final String MASTER_KEY = "kbs.registration_lock_master_key"; private static final String TOKEN_RESPONSE = "kbs.token_response"; + private static final String PIN = "kbs.pin"; private static final String LOCK_LOCAL_PIN_HASH = "kbs.registration_lock_local_pin_hash"; private static final String LAST_CREATE_FAILED_TIMESTAMP = "kbs.last_create_failed_timestamp"; @@ -37,12 +37,13 @@ public final class KbsValues { .remove(V2_LOCK_ENABLED) .remove(TOKEN_RESPONSE) .remove(LOCK_LOCAL_PIN_HASH) + .remove(PIN) .remove(LAST_CREATE_FAILED_TIMESTAMP) .commit(); } /** Should only be set by {@link org.thoughtcrime.securesms.pin.PinState}. */ - public synchronized void setKbsMasterKey(@NonNull KbsPinData pinData, @NonNull String localPinHash) { + public synchronized void setKbsMasterKey(@NonNull KbsPinData pinData, @NonNull String pin) { MasterKey masterKey = pinData.getMasterKey(); String tokenResponse; try { @@ -54,11 +55,18 @@ public final class KbsValues { store.beginWrite() .putString(TOKEN_RESPONSE, tokenResponse) .putBlob(MASTER_KEY, masterKey.serialize()) - .putString(LOCK_LOCAL_PIN_HASH, localPinHash) + .putString(LOCK_LOCAL_PIN_HASH, PinHashing.localPinHash(pin)) + .putString(PIN, pin) .putLong(LAST_CREATE_FAILED_TIMESTAMP, -1) .commit(); } + synchronized void setPinIfNotPresent(@NonNull String pin) { + if (store.getString(PIN, null) == null) { + store.beginWrite().putString(PIN, pin).commit(); + } + } + /** Should only be set by {@link org.thoughtcrime.securesms.pin.PinState}. */ public synchronized void setV2RegistrationLockEnabled(boolean enabled) { store.beginWrite().putBoolean(V2_LOCK_ENABLED, enabled).apply(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PinValues.java b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PinValues.java index 2b5821e787..c3932aa644 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PinValues.java +++ b/app/src/main/java/org/thoughtcrime/securesms/keyvalue/PinValues.java @@ -28,7 +28,7 @@ public final class PinValues { this.store = store; } - public void onEntrySuccess() { + public void onEntrySuccess(@NonNull String pin) { long nextInterval = SignalPinReminders.getNextInterval(getCurrentInterval()); Log.i(TAG, "onEntrySuccess() nextInterval: " + nextInterval); @@ -36,9 +36,11 @@ public final class PinValues { .putLong(LAST_SUCCESSFUL_ENTRY, System.currentTimeMillis()) .putLong(NEXT_INTERVAL, nextInterval) .apply(); + + SignalStore.kbsValues().setPinIfNotPresent(pin); } - public void onEntrySuccessWithWrongGuess() { + public void onEntrySuccessWithWrongGuess(@NonNull String pin) { long nextInterval = SignalPinReminders.getPreviousInterval(getCurrentInterval()); Log.i(TAG, "onEntrySuccessWithWrongGuess() nextInterval: " + nextInterval); @@ -46,6 +48,8 @@ public final class PinValues { .putLong(LAST_SUCCESSFUL_ENTRY, System.currentTimeMillis()) .putLong(NEXT_INTERVAL, nextInterval) .apply(); + + SignalStore.kbsValues().setPinIfNotPresent(pin); } public void onEntrySkipWithWrongGuess() { @@ -93,6 +97,14 @@ public final class PinValues { return PinKeyboardType.fromCode(store.getString(KEYBOARD_TYPE, null)); } + public void setNextReminderIntervalToAtMost(long maxInterval) { + if (store.getLong(NEXT_INTERVAL, 0) > maxInterval) { + store.beginWrite() + .putLong(NEXT_INTERVAL, maxInterval) + .apply(); + } + } + /** Should only be set by {@link org.thoughtcrime.securesms.pin.PinState} */ public void setPinState(@NonNull String pinState) { store.beginWrite().putString(PIN_STATE, pinState).commit(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/lock/SignalPinReminderDialog.java b/app/src/main/java/org/thoughtcrime/securesms/lock/SignalPinReminderDialog.java index cdde2b3912..30706d8ccf 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/lock/SignalPinReminderDialog.java +++ b/app/src/main/java/org/thoughtcrime/securesms/lock/SignalPinReminderDialog.java @@ -130,7 +130,7 @@ public final class SignalPinReminderDialog { if (PinHashing.verifyLocalPinHash(localHash, text)) { dialog.dismiss(); - mainCallback.onReminderCompleted(callback.hadWrongGuess()); + mainCallback.onReminderCompleted(text, callback.hadWrongGuess()); } } else { submit.setEnabled(false); @@ -149,10 +149,10 @@ public final class SignalPinReminderDialog { boolean hadWrongGuess = false; @Override - public void onPinCorrect() { + public void onPinCorrect(@NonNull String pin) { Log.i(TAG, "Correct PIN entry."); dialog.dismiss(); - mainCallback.onReminderCompleted(hadWrongGuess); + mainCallback.onReminderCompleted(pin, hadWrongGuess); } @Override @@ -188,7 +188,7 @@ public final class SignalPinReminderDialog { if (pin.length() < KbsConstants.MINIMUM_PIN_LENGTH) return; if (PinHashing.verifyLocalPinHash(localPinHash, pin)) { - callback.onPinCorrect(); + callback.onPinCorrect(pin); } else { callback.onPinWrong(); } @@ -200,7 +200,7 @@ public final class SignalPinReminderDialog { void verifyPin(@Nullable String pin, @NonNull PinVerifier.Callback callback); interface Callback { - void onPinCorrect(); + void onPinCorrect(@NonNull String pin); void onPinWrong(); boolean hadWrongGuess(); } @@ -212,6 +212,6 @@ public final class SignalPinReminderDialog { public interface Callback { void onReminderDismissed(boolean includedFailure); - void onReminderCompleted(boolean includedFailure); + void onReminderCompleted(@NonNull String pin, boolean includedFailure); } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/megaphone/Megaphones.java b/app/src/main/java/org/thoughtcrime/securesms/megaphone/Megaphones.java index 932761fb3f..d92cf18bb0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/megaphone/Megaphones.java +++ b/app/src/main/java/org/thoughtcrime/securesms/megaphone/Megaphones.java @@ -164,12 +164,12 @@ public final class Megaphones { } @Override - public void onReminderCompleted(boolean includedFailure) { + public void onReminderCompleted(@NonNull String pin, boolean includedFailure) { Log.i(TAG, "[PinReminder] onReminderCompleted(" + includedFailure + ")"); if (includedFailure) { - SignalStore.pinValues().onEntrySuccessWithWrongGuess(); + SignalStore.pinValues().onEntrySuccessWithWrongGuess(pin); } else { - SignalStore.pinValues().onEntrySuccess(); + SignalStore.pinValues().onEntrySuccess(pin); } controller.onMegaphoneSnooze(Event.PIN_REMINDER); diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java index ee1b5e3f49..b25df7692c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java @@ -39,7 +39,7 @@ public class ApplicationMigrations { private static final int LEGACY_CANONICAL_VERSION = 455; - public static final int CURRENT_VERSION = 14; + public static final int CURRENT_VERSION = 15; private static final class Version { static final int LEGACY = 1; @@ -56,6 +56,7 @@ public class ApplicationMigrations { static final int STORAGE_KEY_ROTATE = 12; static final int REMOVE_AVATAR_ID = 13; static final int STORAGE_CAPABILITY = 14; + static final int PIN_REMINDER = 15; } /** @@ -226,6 +227,10 @@ public class ApplicationMigrations { jobs.put(Version.STORAGE_CAPABILITY, new StorageCapabilityMigrationJob()); } + if (lastSeenVersion < Version.PIN_REMINDER) { + jobs.put(Version.PIN_REMINDER, new PinReminderMigrationJob()); + } + return jobs; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/PinReminderMigrationJob.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/PinReminderMigrationJob.java new file mode 100644 index 0000000000..449ebd28e1 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/PinReminderMigrationJob.java @@ -0,0 +1,50 @@ +package org.thoughtcrime.securesms.migrations; + +import androidx.annotation.NonNull; + +import org.thoughtcrime.securesms.jobmanager.Data; +import org.thoughtcrime.securesms.jobmanager.Job; +import org.thoughtcrime.securesms.keyvalue.SignalStore; + +import java.util.concurrent.TimeUnit; + +public class PinReminderMigrationJob extends MigrationJob { + + public static final String KEY = "PinReminderMigrationJob"; + + PinReminderMigrationJob() { + this(new Job.Parameters.Builder().build()); + } + + private PinReminderMigrationJob(@NonNull Parameters parameters) { + super(parameters); + } + + @Override + boolean isUiBlocking() { + return false; + } + + @Override + public @NonNull String getFactoryKey() { + return KEY; + } + + @Override + void performMigration() { + SignalStore.pinValues().setNextReminderIntervalToAtMost(TimeUnit.DAYS.toMillis(3)); + } + + @Override + boolean shouldRetry(@NonNull Exception e) { + return false; + } + + public static class Factory implements Job.Factory { + + @Override + public @NonNull PinReminderMigrationJob create(@NonNull Parameters parameters, @NonNull Data data) { + return new PinReminderMigrationJob(parameters); + } + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/pin/PinState.java b/app/src/main/java/org/thoughtcrime/securesms/pin/PinState.java index ca6ee1141f..a5bc26b245 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/pin/PinState.java +++ b/app/src/main/java/org/thoughtcrime/securesms/pin/PinState.java @@ -103,7 +103,7 @@ public final class PinState { Log.i(TAG, "Registration Lock V2"); TextSecurePreferences.setV1RegistrationLockEnabled(context, false); SignalStore.kbsValues().setV2RegistrationLockEnabled(true); - SignalStore.kbsValues().setKbsMasterKey(kbsData, PinHashing.localPinHash(pin)); + SignalStore.kbsValues().setKbsMasterKey(kbsData, pin); SignalStore.pinValues().resetPinReminders(); resetPinRetryCount(context, pin, kbsData); } else if (hasPinToRestore) { @@ -124,7 +124,7 @@ public final class PinState { * Invoked when the user is going through the PIN restoration flow (which is separate from reglock). */ public static synchronized void onSignalPinRestore(@NonNull Context context, @NonNull KbsPinData kbsData, @NonNull String pin) { - SignalStore.kbsValues().setKbsMasterKey(kbsData, PinHashing.localPinHash(pin)); + SignalStore.kbsValues().setKbsMasterKey(kbsData, pin); SignalStore.kbsValues().setV2RegistrationLockEnabled(false); SignalStore.pinValues().resetPinReminders(); SignalStore.storageServiceValues().setNeedsAccountRestore(false); @@ -160,7 +160,7 @@ public final class PinState { HashedPin hashedPin = PinHashing.hashPin(pin, pinChangeSession); KbsPinData kbsData = pinChangeSession.setPin(hashedPin, masterKey); - kbsValues.setKbsMasterKey(kbsData, PinHashing.localPinHash(pin)); + kbsValues.setKbsMasterKey(kbsData, pin); TextSecurePreferences.clearRegistrationLockV1(context); SignalStore.pinValues().setKeyboardType(keyboard); SignalStore.pinValues().resetPinReminders(); @@ -269,7 +269,7 @@ public final class PinState { pinChangeSession.enableRegistrationLock(masterKey); - kbsValues.setKbsMasterKey(kbsData, PinHashing.localPinHash(pin)); + kbsValues.setKbsMasterKey(kbsData, pin); kbsValues.setV2RegistrationLockEnabled(true); TextSecurePreferences.clearRegistrationLockV1(context); TextSecurePreferences.setRegistrationLockLastReminderTime(context, System.currentTimeMillis()); @@ -294,7 +294,7 @@ public final class PinState { pinChangeSession.enableRegistrationLock(masterKey); - kbsValues.setKbsMasterKey(kbsData, PinHashing.localPinHash(pin)); + kbsValues.setKbsMasterKey(kbsData, pin); TextSecurePreferences.clearRegistrationLockV1(context); updateState(buildInferredStateFromOtherFields()); @@ -333,7 +333,7 @@ public final class PinState { HashedPin hashedPin = PinHashing.hashPin(pin, pinChangeSession); KbsPinData newData = pinChangeSession.setPin(hashedPin, masterKey); - kbsValues.setKbsMasterKey(newData, PinHashing.localPinHash(pin)); + kbsValues.setKbsMasterKey(newData, pin); TextSecurePreferences.clearRegistrationLockV1(context); Log.i(TAG, "Pin set/attempts reset on KBS");