bordaigorl / rmview

A live viewer for reMarkable written in PyQt5
GNU General Public License v3.0
742 stars 61 forks source link

fix the screenshare authentication, for 2.10 #100

Closed ddvk closed 2 years ago

ddvk commented 2 years ago

it works, but it's a mess

bordaigorl commented 2 years ago

Wow this is amazing work thanks so much. I am still on 2.9 and I confirm this can be made to work after handling the issues below.

  1. Which version of jwt are you using? It seems the one I get from pip has a very different API. Using 1.2.0 I had to use d = jwt.JWT().decode(token, do_verify=False)

  2. On 2.9, ip = socket.inet_ntoa(reader.read(4)) throws an exception. So maybe 2.9 encoded different info at this point? It looks as you are not making use of this information; by just ignoring it the connection is established just fine. [EDIT: It could be an issue of delimiters, I get a meaningful list of 5 ip:port before it throws packed IP wrong length for inet_ntoa]

bordaigorl commented 2 years ago

Ah you pushed while I was writing my last comment.

ea11cc5 indeed solves issue 1 (I used a different jwt package).

By catching and ignoring exceptions when populating addresses I get a meaningful list of 5 ip:port before it throws packed IP wrong length for inet_ntoa

Let me add a third issue:

  1. it does not seem to work on USB, looks like it's not getting any UDP packet.
ddvk commented 2 years ago

By catching and ignoring exceptions when populating addresses I get a meaningful list of 5 ip:port before it throws packed IP wrong length for inet_ntoa

could you post the datagram? I thought it was 0 delimited pairs...

  1. it does not seem to work on USB, looks like it's not getting any UDP packet.

seems to be a bug in 2.9, in 2.10 it sends the broadcast over usb as well :(

bordaigorl commented 2 years ago

could you post the datagram? I thought it was 0 delimited pairs...

I get 5 entries and then an exception. This is what I get as delimiters:

b'\x00'
b'\x00'
b'\x01'
b'\x18'
b'\x22'
b'\x17'

After which I get b'\x0c' from the read(4) (which is the one that cannot get converted by inet_ntoa) and an empty string from the following read (for the port).

seems to be a bug in 2.9, in 2.10 it sends the broadcast over usb as well :(

Oh snap. Why all these complications. I guess I could just skip authentication if I detect <2.10... I am really hoping @pl-semiotics can provide a VNC server for 2.9+ so we are not locked into their quirky design decisions...

bordaigorl commented 2 years ago

@ddvk Actually: of the 5 entries, only the first 2 look real, the others might be interpretable as ip just by chance. Which would suggest having the guard while reader.read(1) == b'\x00':.

ddvk commented 2 years ago

any other way to synchronize the udp listening and starting vnc? it works, but it looks hackish to me and I had no idea how to do it with signals/slots

bordaigorl commented 2 years ago

The dependencies between ChallengeReaderProtocol and ScreenShareStream do look a bit hackish, but only in terms of clarity of code, I think. In terms of control-flow they do the right thing. One could try to decouple the two things more; a simple (but marginally better) way would be to add a callback parameter to ChallengeReaderProtocol, which is called when a new challenge is received. The other would be to have ChallengeReaderProtocol emit a Qt signal when receiving a new challenge (using a QObject signals field of ChallengeReaderProtocol as done in ScreenStreamSignals), but I don't see an immediate benefit. Maybe you have a more precise feeling for why the current solution does not feel completely right?

bordaigorl commented 2 years ago

Just updated to 2.10. Same issue as above, if I enable REMARKABLE_ENCODING the frame updates stop as soon as I pause writing for a little while. Just before they stop I get an unhandled message with id 100, sometimes preceded by a flurry of other ones, for example: 100, 4, 14, 5, 74, 100, 4, 12, 5, 73, 100, 4, 9, 5, 72, 100, 4, 6, 5, 70, 100, 4, 5, 68, 100 and then no other message from the server is received and no updates will be sent from then on.

ddvk commented 2 years ago

oh, we can just remove it then

bordaigorl commented 2 years ago

Yes just reporting for future reference, in case we decode the messages in the future. I'll push some minor refactoring tomorrow and merge, since 2.10 is being rolled out...

rosmo commented 2 years ago

Thanks for the code here! Since I prefer using standard VNC viewers, I ported the code to work with vncproxy: https://github.com/rosmo/vncproxy/tree/remarkable-2.10

Since you've been working with the built in VNC server, are you aware if it supports other encodings than ZRLE? (sadly it's not supported by NoVNC and RealVNC seems to have a problem with the data RM is sending, however TightVNC works fine - in general it looks like if you send any SetEncodings message, the RM VNC server stops sending you any updates even if the supported set of encodings had ZRLE in it)

ddvk commented 2 years ago

iirc only raw and zrle

rosmo commented 2 years ago

Unfortunately looks like raw isn't supported either (I added an override so I can specify exactly which encodings are sent to the RM). I guess I need to add ZRLE support to noVNC....

rosmo commented 2 years ago

Just leaving this here, in case someone looks: