From 4bc865794f02bc458dff7adfc7c126be0dac064e Mon Sep 17 00:00:00 2001
From: matejcik <ja@matejcik.cz>
Date: Thu, 28 May 2020 11:57:04 +0200
Subject: [PATCH] core: only unlock storage if it is locked (solves determinism
 issue in tests)

---
 core/src/apps/base.py                  | 11 ++++++--
 core/src/apps/homescreen/lockscreen.py |  5 +++-
 tests/device_tests/test_debuglink.py   | 37 +++++++++++++++++++++++++-
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/core/src/apps/base.py b/core/src/apps/base.py
index acfb50521..1bbcaaeb2 100644
--- a/core/src/apps/base.py
+++ b/core/src/apps/base.py
@@ -151,8 +151,15 @@ def lock_device() -> None:
 
 
 async def unlock_device(ctx: wire.GenericContext = wire.DUMMY_CONTEXT) -> None:
-    await verify_user_pin(ctx)
-    # verify_user_pin will raise if the PIN was invalid
+    """Ensure the device is in unlocked state.
+
+    If the storage is locked, attempt to unlock it. Reset the homescreen and the wire
+    handler.
+    """
+    if not config.is_unlocked():
+        # verify_user_pin will raise if the PIN was invalid
+        await verify_user_pin(ctx)
+
     set_homescreen()
     wire.find_handler = wire.find_registered_workflow_handler
 
diff --git a/core/src/apps/homescreen/lockscreen.py b/core/src/apps/homescreen/lockscreen.py
index b2487e385..3a923872a 100644
--- a/core/src/apps/homescreen/lockscreen.py
+++ b/core/src/apps/homescreen/lockscreen.py
@@ -7,9 +7,12 @@ async def lockscreen() -> None:
     from apps.common.request_pin import can_lock_device
     from apps.base import unlock_device
 
+    # Only show the lockscreen UI if the device can in fact be locked.
     if can_lock_device():
         await Lockscreen()
-
+    # Otherwise proceed directly to unlock() call. If the device is already unlocked,
+    # it should be a no-op storage-wise, but it resets the internal configuration
+    # to an unlocked state.
     try:
         await unlock_device()
     except wire.PinCancelled:
diff --git a/tests/device_tests/test_debuglink.py b/tests/device_tests/test_debuglink.py
index 62fb434c0..9a7814c1f 100644
--- a/tests/device_tests/test_debuglink.py
+++ b/tests/device_tests/test_debuglink.py
@@ -16,7 +16,7 @@
 
 import pytest
 
-from trezorlib import messages
+from trezorlib import debuglink, device, messages, misc
 
 from ..common import MNEMONIC12
 
@@ -47,3 +47,38 @@ class TestDebuglink:
 
         resp = client.call_raw(messages.PassphraseAck(passphrase=""))
         assert isinstance(resp, messages.Address)
+
+
+@pytest.mark.skip_ui
+@pytest.mark.skip_t1
+def test_softlock_instability(client):
+    def load_device():
+        debuglink.load_device(
+            client,
+            mnemonic=MNEMONIC12,
+            pin="1234",
+            passphrase_protection=False,
+            label="test",
+        )
+
+    # start from a clean slate:
+    client.debug.reseed(0)
+    device.wipe(client)
+    entropy_after_wipe = misc.get_entropy(client, 16)
+
+    # configure and wipe the device
+    load_device()
+    client.debug.reseed(0)
+    device.wipe(client)
+    assert misc.get_entropy(client, 16) == entropy_after_wipe
+
+    load_device()
+    # the device has PIN -> lock it
+    client.call(messages.LockDevice())
+    client.debug.reseed(0)
+    # wipe_device should succeed with no need to unlock
+    device.wipe(client)
+    # the device is now trying to run the lockscreen, which attempts to unlock.
+    # If the device actually called config.unlock(), it would use additional randomness.
+    # That is undesirable. Assert that the returned entropy is still the same.
+    assert misc.get_entropy(client, 16) == entropy_after_wipe