tillitis / tkey-ssh-agent

SSH Agent for TKey, the flexible open hardware/software USB security key 🔑
https://www.tillitis.se
GNU General Public License v2.0
127 stars 14 forks source link

Attempt to queue multiple LoadAppData cmds fails #25

Closed quite closed 10 months ago

quite commented 1 year ago

I've tried rewriting the LoadApp function to queue -- keep in flight -- multiple LoadAppData cmds, while reading the responses only when they come in. This should be possible since we have an RX FIFO of 512 bytes in hardware.

The idea is to keep the app continuously fed with data to process, instead of waiting for its responses (acks) for every frame of data. The goal being faster loading of the app.

There are two branches. loadappdata-queue is a first attempt, which I think is flawed because (when the queue is full) it waits for and reads a response after each sending of cmd.

The second attempt loadappdata-queue-take-2 always makes an attempt to read a response, but times out after some milliseconds if there is none. But both attempts fails in the same way. After a few cmds/responses, we get a response which has the wrong cmd code. Running towards QEMU works though. And setting the queueLen to 1 works for running towards hw.

Output from running loadappdata-queue-take-2:

~/t/tillitis-key1-apps ./tk-ssh-agent -k
Auto-detected serial port /dev/ttyACM0
Connecting to device on serial port /dev/ttyACM0 ...
GetAppNameVersion tx (frame len: 1+1):
00000000  58 09                                             |X.|
GetAppNameVersion: ReadFrame: Read timeout
GetNameVersion tx (frame len: 1+1):
00000000  50 01                                             |P.|
Device is in firmware mode.
Loading app...
LoadUSS tx len:129 contents omitted
app size: 27656, 0x6c08, 0b110110000001000
SetAppSize tx (frame len: 1+32):
00000000  52 03 08 6c 00 00 00 00  00 00 00 00 00 00 00 00  |R..l............|
00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000020  00                                                |.|
LoadAppData tx (frame len: 1+128):
00000000  13 05 81 40 01 41 81 41  01 42 81 42 01 43 81 43  |...@.A.A.B.B.C.C|
00000010  01 44 81 44 01 45 81 45  01 46 81 46 01 47 81 47  |.D.D.E.E.F.F.G.G|
00000020  01 48 81 48 01 49 81 49  01 4a 81 4a 01 4b 81 4b  |.H.H.I.I.J.J.K.K|
00000030  01 4c 81 4c 01 4d 81 4d  01 4e 81 4e 01 4f 81 4f  |.L.L.M.M.N.N.O.O|
00000040  37 71 00 40 41 11 17 75  00 00 13 05 45 bc 97 75  |7q.@A..u....E..u|
00000050  00 00 93 85 c5 bb 63 57  b5 00 23 20 05 00 11 05  |......cW..# ....|
00000060  e3 4d b5 fe 97 00 00 00  e7 80 e0 00 00 00 00 00  |.M..............|
00000070  00 00 13 01 01 81 23 26  11 7e 23 24 81 7e 23 22  |......#&.~#$.~#"|
00000080  91                                                |.|
QUEUE after send ID:0: [0] (nsent:127 offset:0 binlen:27656)
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0]
LoadAppData tx (frame len: 1+128):
00000000  33 05 7e 23 20 21 7f 23  2e 31 7d 23 2c 41 7d 23  |3.~# !.#.1}#,A}#|
00000010  2a 51 7d 23 28 61 7d 23  26 71 7d 23 24 81 7d 23  |*Q}#(a}#&q}#$.}#|
00000020  22 91 7d 23 20 a1 7d 23  2e b1 7b 05 65 13 05 05  |".}# .}#..{.e...|
00000030  cb 33 01 a1 40 37 05 00  ff 83 25 05 08 ae d8 83  |.3..@7....%.....|
00000040  25 45 08 ae da 83 25 85  08 ae dc 83 25 c5 08 ae  |%E....%.....%...|
00000050  de 83 25 05 09 2e c1 83  25 45 09 2e c3 83 25 85  |..%.....%E....%.|
00000060  09 2e c5 03 25 c5 09 2a  c7 37 e5 09 6a 13 05 75  |....%..*.7..j..u|
00000070  66 aa cb 37 d5 bc f3 13  05 85 90 aa c9 37 b5 67  |f..7.........7.g|
00000080  bb                                                |.|
QUEUE after send ID:1: [0 1] (nsent:127 offset:127 binlen:27656)
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1]
LoadAppData tx (frame len: 1+128):
00000000  53 05 13 05 55 e8 aa cf  37 a5 ca 84 13 05 b5 73  |S...U...7......s|
00000010  aa cd 37 f5 6e 3c 13 05  25 37 aa d3 37 05 95 fe  |..7.n<..%7..7...|
00000020  13 05 b5 82 aa d1 37 f5  4f a5 13 05 a5 53 aa d7  |......7.O....S..|
00000030  37 35 1d 5f 13 05 15 6f  aa d5 37 55 0e 51 13 05  |75._...o..7U.Q..|
00000040  f5 27 aa db 37 85 e6 ad  13 05 15 2d aa d9 37 75  |.'..7......-..7u|
00000050  05 9b 13 05 c5 88 aa df  37 75 3e 2b 13 05 f5 c1  |........7u>+....|
00000060  aa dd 37 e5 83 1f 13 05  b5 9a 23 22 a1 10 37 c5  |..7.......#"..7.|
00000070  41 fb 13 05 b5 d6 23 20  a1 10 37 d5 e0 5b 13 05  |A.....# ..7..[..|
00000080  95                                                |.|
QUEUE after send ID:2: [0 1 2] (nsent:127 offset:254 binlen:27656)
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0) (frame len: 1+4):
00000000  11 06 00 00 00                                    |.....|
QUEUE after recv: [1 2]
LoadAppData tx (frame len: 1+128):
00000000  73 05 08 6d f2 fd 15 e5  fd 13 45 15 00 e5 b7 13  |s..m......E.....|
00000010  fc 34 00 13 15 2c 00 4e  95 00 41 05 65 13 05 05  |.4...,.N..A.e...|
00000020  15 0a 95 13 06 00 08 81  45 97 60 00 00 e7 80 a0  |........E.`.....|
00000030  d5 01 45 83 a5 0d 08 f5  dd 83 a5 4d 08 33 06 ad  |..E........M.3..|
00000040  00 05 05 23 00 b6 00 e3  16 85 fe 93 f5 84 01 01  |...#............|
00000050  45 e3 92 75 fb 13 d5 54  00 93 7c 35 00 05 65 13  |E..u...T..|5..e.|
00000060  05 05 0d 0a 95 13 06 00  08 81 45 97 60 00 00 e7  |..........E.`...|
00000070  80 80 d1 05 65 13 05 05  15 0a 95 03 45 05 00 7d  |....e.......E..}|
00000080  15                                                |.|
QUEUE after send ID:3: [1 2 3] (nsent:127 offset:889 binlen:27656)
LoadAppData rx (expectedID:1): no data
no frame to read yet
QUEUE: [1 2 3]
LoadAppData rx (expectedID:1): no data
no frame to read yet
QUEUE: [1 2 3]
LoadAppData rx (expectedID:1): no data
no frame to read yet
QUEUE: [1 2 3]
LoadAppData rx (expectedID:1): no data
no frame to read yet
QUEUE: [1 2 3]
LoadAppData rx (expectedID:1) (frame len: 1+4):
00000000  31 06 00 00 00                                    |1....|
QUEUE after recv: [2 3]
LoadAppData tx (frame len: 1+128):
00000000  13 05 86 00 d5 8d c2 05  4d 8d 85 65 93 85 45 0d  |........M..e..E.|
00000010  8a 95 88 c1 05 65 13 05  15 1d 0a 95 03 45 05 00  |.....e.......E..|
00000020  85 65 93 85 05 1d 8a 95  83 c5 05 00 05 66 13 06  |.e...........f..|
00000030  36 1d 0a 96 03 46 06 00  85 66 93 86 26 1d 8a 96  |6....F...f..&...|
00000040  83 c6 06 00 22 05 4d 8d  93 15 86 00 d5 8d c2 05  |....".M.........|
00000050  4d 8d 85 65 93 85 05 0d  8a 95 88 c1 13 95 5c 00  |M..e..........\.|
00000060  83 a5 0d 10 f5 dd 13 65  b5 01 93 85 4d 10 88 c1  |.......e....M...|
00000070  93 06 f0 07 03 a5 0d 10  75 dd 01 45 89 45 23 a2  |........u..E.E#.|
00000080  bd                                                |.|
QUEUE after send ID:0: [2 3 0] (nsent:127 offset:1524 binlen:27656)
LoadAppData rx (expectedID:2): no data
no frame to read yet
QUEUE: [2 3 0]
LoadAppData rx (expectedID:2): no data
no frame to read yet
QUEUE: [2 3 0]
LoadAppData rx (expectedID:2): no data
no frame to read yet
QUEUE: [2 3 0]
LoadAppData rx (expectedID:2) (frame len: 1+4):
00000000  51 06 00 00 00                                    |Q....|
QUEUE after recv: [3 0]
LoadAppData tx (frame len: 1+128):
00000000  33 05 0d 10 75 dd 81 45  01 45 31 46 23 a2 cd 10  |3...u..E.E1F#...|
00000010  03 a6 0d 10 75 de 93 f5  f5 0f 05 05 23 a2 bd 10  |....u.......#...|
00000020  63 07 25 01 b3 85 aa 00  83 c5 05 00 d5 b7 81 4c  |c.%............L|
00000030  2d be 13 95 5c 00 83 a5  0d 10 f5 dd 13 65 85 01  |-...\........e..|
00000040  93 85 4d 10 88 c1 03 a5  0d 10 75 dd 81 4c 13 05  |..M.......u..L..|
00000050  f0 0f 23 a2 ad 10 11 be  09 45 63 1e ac 18 05 65  |..#......Ec....e|
00000060  13 05 25 15 0a 95 03 45  05 00 85 65 93 85 15 15  |..%....E...e....|
00000070  8a 95 83 c5 05 00 05 66  13 06 36 15 0a 96 03 46  |.......f..6....F|
00000080  06                                                |.|
QUEUE after send ID:1: [3 0 1] (nsent:127 offset:2032 binlen:27656)
LoadAppData rx (expectedID:3): no data
no frame to read yet
QUEUE: [3 0 1]
LoadAppData rx (expectedID:3): no data
no frame to read yet
QUEUE: [3 0 1]
LoadAppData rx (expectedID:3): no data
no frame to read yet
QUEUE: [3 0 1]
LoadAppData rx (expectedID:3) (frame len: 1+4):
00000000  71 06 00 00 00                                    |q....|
QUEUE after recv: [0 1]
LoadAppData tx (frame len: 1+128):
00000000  53 05 8a 95 23 80 a5 00  13 95 5c 00 83 a5 0d 10  |S...#.....\.....|
00000010  f5 dd 93 65 95 01 13 85  4d 10 0c c1 83 a5 0d 10  |...e....M.......|
00000020  f5 dd 91 45 0c c1 03 a5  0d 10 75 dd 05 45 23 a2  |...E......u..E#.|
00000030  ad 10 05 65 13 05 15 0d  0a 95 03 45 05 00 83 a5  |...e.......E....|
00000040  0d 10 f5 dd 23 a2 ad 10  05 65 13 05 25 0d 0a 95  |....#....e..%...|
00000050  03 45 05 00 83 a5 0d 10  f5 dd 81 4c 23 a2 ad 10  |.E.........L#...|
00000060  39 b2 13 05 f0 07 46 44  22 8c 63 64 a4 00 13 0c  |9.....FD".cd....|
00000070  f0 07 88 09 d6 45 2e 95  85 65 93 85 15 15 8a 95  |.....E...e......|
00000080  62                                                |b|
QUEUE after send ID:2: [0 1 2] (nsent:127 offset:2540 binlen:27656)
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0) (frame len: 1+4):
00000000  11 11 00 00 00                                    |.....|
LoadApp: receiveRspLoadAppData: ReadFrame: Expected cmd code 0x6 (rspLoadAppData), got 0x11
dehanj commented 10 months ago

Been investigating this and found a few issues. I have ported this code and have a working copy in tkeyclient/load_app_multi_frame so will close this issue.

What i found. CH552 drops bytes, and that is what you saw when the transfer failed. I have a fix for it in this branch. It was most likely more prone to happen when sending multiple frames in parallel, compared to "ordinary" usage.

Some minor issues I found was that the nsent variable was not set to zero if the queue is full, hence offset is updated even if we don't transfer anything. Se this snipped from above, offset goes from 254 to 889 with just one transfer.

LoadAppData tx (frame len: 1+128):
00000000  53 05 13 05 55 e8 aa cf  37 a5 ca 84 13 05 b5 73  |S...U...7......s|
00000010  aa cd 37 f5 6e 3c 13 05  25 37 aa d3 37 05 95 fe  |..7.n<..%7..7...|
00000020  13 05 b5 82 aa d1 37 f5  4f a5 13 05 a5 53 aa d7  |......7.O....S..|
00000030  37 35 1d 5f 13 05 15 6f  aa d5 37 55 0e 51 13 05  |75._...o..7U.Q..|
00000040  f5 27 aa db 37 85 e6 ad  13 05 15 2d aa d9 37 75  |.'..7......-..7u|
00000050  05 9b 13 05 c5 88 aa df  37 75 3e 2b 13 05 f5 c1  |........7u>+....|
00000060  aa dd 37 e5 83 1f 13 05  b5 9a 23 22 a1 10 37 c5  |..7.......#"..7.|
00000070  41 fb 13 05 b5 d6 23 20  a1 10 37 d5 e0 5b 13 05  |A.....# ..7..[..|
00000080  95                                                |.|
QUEUE after send ID:2: [0 1 2] (nsent:127 offset:254 binlen:27656)
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0): no data
no frame to read yet
QUEUE: [0 1 2]
LoadAppData rx (expectedID:0) (frame len: 1+4):
00000000  11 06 00 00 00                                    |.....|
QUEUE after recv: [1 2]
LoadAppData tx (frame len: 1+128):
00000000  73 05 08 6d f2 fd 15 e5  fd 13 45 15 00 e5 b7 13  |s..m......E.....|
00000010  fc 34 00 13 15 2c 00 4e  95 00 41 05 65 13 05 05  |.4...,.N..A.e...|
00000020  15 0a 95 13 06 00 08 81  45 97 60 00 00 e7 80 a0  |........E.`.....|
00000030  d5 01 45 83 a5 0d 08 f5  dd 83 a5 4d 08 33 06 ad  |..E........M.3..|
00000040  00 05 05 23 00 b6 00 e3  16 85 fe 93 f5 84 01 01  |...#............|
00000050  45 e3 92 75 fb 13 d5 54  00 93 7c 35 00 05 65 13  |E..u...T..|5..e.|
00000060  05 05 0d 0a 95 13 06 00  08 81 45 97 60 00 00 e7  |..........E.`...|
00000070  80 80 d1 05 65 13 05 05  15 0a 95 03 45 05 00 7d  |....e.......E..}|
00000080  15                                                |.|
QUEUE after send ID:3: [1 2 3] (nsent:127 offset:889 binlen:27656)