Wrong message type is passed from device

[vladimirvolek]

How to reproduce:

1. enable a passphrase on your device
2. go to https://szymonlesisz.github.io/trezor-connect-explorer/#/getaddress
3. click on 'GET ADDRESS' button (popup will show up on a screen + device will show confirmation "Where to enter your passphrase" Device / Host)
4. close the popup and wait for a second
5. click on 'Host' on the device
6. click 'GET ADDRESS' button again

You should see

The current message from the device should be 'Features' not 'PassphraseRequest'. Maybe is cached?

console log:

[prusnak]

@vladimirvolek is this still a thing?

[vladimirvolek]

@prusnak @tsusanka Yes. I just reproduced this issue.

[matejcik]

what I'm somewhat sure is happening here is this:

1. connect calls the bridge and waits for answer
2. by closing the pop-up, you cancel the wait, but Trezor is still in state "ready to send reply"
3. when you select something on the device, the reply is sent out
4. bridge's next attempt to read USB will return that reply - even though it's happened after a different write.

I'm not sure what's the appropriate place to resolve this. We might want bridge to clear the queue when reconnecting to the device. Or we might want Connect to send a Cancel when the pop-up is closed (i don't know if that's possible though). Or maybe bridge should send the Cancel when a wait is interrupted? Or it could poll the USB descriptor even when nobody is reading, to catch stray messages like this one.

Not much we can do on the firmware side though. As far as the device knows, nothing is wrong. You can't take back the USB message that you already sent.

[jpochyla]

Or it could poll the USB descriptor even when nobody is reading, to catch stray messages like this one.

This is an interesting idea! cc @karel-3d

[karelbilek]

That would probably break the "post" and "read" messages that's implemented for debug link

…

On Thu, Mar 28, 2019, 1:26 AM Jan Pochyla ***@***.***> wrote: Or it could poll the USB descriptor even when nobody is reading, to catch stray messages like this one. This is an interesting idea! cc @karel-3d <https://github.com/karel-3d> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/trezor/trezor-core/issues/295#issuecomment-477292950>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGZ8aSXRNXc1BmkZ6EqBNXipeA45Z8-ks5va7frgaJpZM4Vtx6b> .

[karelbilek]

bridge can maybe wait for the stray messages in the case "call" call (not separate "post" and "read" call) is interrupted between the input and output part but this waiting has to be cancelled when another call then posts something to the device (I don't currently have any free time for playing with bridge. :/ )

…

On Thu, Mar 28, 2019, 1:26 AM Jan Pochyla ***@***.***> wrote: Or it could poll the USB descriptor even when nobody is reading, to catch stray messages like this one. This is an interesting idea! cc @karel-3d <https://github.com/karel-3d> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/trezor/trezor-core/issues/295#issuecomment-477292950>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGZ8aSXRNXc1BmkZ6EqBNXipeA45Z8-ks5va7frgaJpZM4Vtx6b> .

[tsusanka]

I dare to say this is not a firmware issue, I'm suggesting to move this to trezord-go then. @prusnak could you move it? I don't think I have permissions for that.

[prusnak]

Transferred

[karelbilek]

Does the explorer work with UDP emulator? I can't see anything there.

....oh that's because it's on github.io and trezord has whitelisted origins for trezor.io.

Maybe it would be a good idea to add an option for whitelisting more origins? (#155)

[karelbilek]

...well adding github.io to the allowed websites didn't help. I still see "no connected devices" in the explorer.

[karelbilek]

And clicking on "get address" gets me this (forever loading)

in the github.io site, I see the error

"postMessage: popupMessagePort not found"

[karelbilek]

I got the same issue with the device itself. Tested FFX and Chrome, wallet works (through bridge), not sure where is the issue.

[karelbilek]

Oh, I needed to manually remove #loading from the popup URL and it started working

[karelbilek]

When I played with it, it is not easy to implement, because. On cancelling of the call, we automatically "release" the device, which automatically closes the device on USB level.

So now, we will need to periodically, when cancelled during waiting for call read, do

• open the device (on USB level)
• read from the device, with a short timeout
• if nothing is read, keep this in a set of "devices, that we periodically check"
• if something is read, just read all
• when device is disconnected or another call is done on the device, remove it from the set of "devices that we periodically check" - and be sure it is done immediately, so the periodic check that throws away the message is not eating message for someone else
• and work around all the mutexes that we have everywhere to prevent all the weird bugs

[karelbilek]

Fixed in #156 but I am not sure it's worth the added complexity (and probably some new bugs in the new code).

[tsusanka]

Note that Connect (therefore Suite) now has a workaround for this (see trezor/connect#561 as linked).