From 444bbc68c4f99c07bf1442a7074cd8a79fa1caae Mon Sep 17 00:00:00 2001 From: Chill Validation Date: Thu, 21 Apr 2022 17:05:40 +0900 Subject: [PATCH 1/4] CV Port Remove suprious OSX USB Reads --- .gitignore | 3 +++ apduWrapper.go | 50 ++++++++++++++++++++++++++++++++++++--------- apduWrapper_test.go | 6 ++++-- ledger_hid.go | 42 ++++++++++++++++++++++++++++++++++--- 4 files changed, 86 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index d61e513..ee4c829 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,9 @@ *.dylib *.dll +# Fortran module files - Why? For now, Allow go.mod +#*.mod + # Fortran module files *.smod diff --git a/apduWrapper.go b/apduWrapper.go index 5900645..38c16bb 100644 --- a/apduWrapper.go +++ b/apduWrapper.go @@ -19,6 +19,7 @@ package ledger_go import ( "encoding/binary" "fmt" + "github.com/pkg/errors" ) @@ -42,7 +43,7 @@ func ErrorMessage(errorCode uint16) string { case 0x6985: return "[APDU_CODE_CONDITIONS_NOT_SATISFIED] Conditions of use not satisfied" case 0x6986: - return "[APDU_CODE_COMMAND_NOT_ALLOWED] Command not allowed (no current EF)" + return "[APDU_CODE_COMMAND_NOT_ALLOWED] Command not allowed / User Rejected (no current EF)" case 0x6A80: return "[APDU_CODE_BAD_KEY_HANDLE] The parameters in the data field are incorrect" case 0x6B00: @@ -51,6 +52,8 @@ func ErrorMessage(errorCode uint16) string { return "[APDU_CODE_INS_NOT_SUPPORTED] Instruction code not supported or invalid" case 0x6E00: return "[APDU_CODE_CLA_NOT_SUPPORTED] CLA not supported" + case 0x6E01: + return "[0x6E01] Ledger Connected but Cosmos App Not Open" case 0x6F00: return "APDU_CODE_UNKNOWN" case 0x6F01: @@ -105,26 +108,35 @@ func SerializePacket( func DeserializePacket( channel uint16, buffer []byte, - sequenceIdx uint16) (result []byte, totalResponseLength uint16, err error) { + sequenceIdx uint16) (result []byte, totalResponseLength uint16, isSequenceZero bool, err error) { + + isSequenceZero = false if (sequenceIdx == 0 && len(buffer) < 7) || (sequenceIdx > 0 && len(buffer) < 5) { - return nil, 0, errors.New("Cannot deserialize the packet. Header information is missing.") + return nil, 0, isSequenceZero, errors.New("Cannot deserialize the packet. Header information is missing.") } var headerOffset uint8 if codec.Uint16(buffer) != channel { - return nil, 0, errors.New("Invalid channel") + return nil, 0, isSequenceZero, errors.New(fmt.Sprintf("Invalid channel. Expected %d, Got: %d", channel, codec.Uint16(buffer))) } headerOffset += 2 if buffer[headerOffset] != 0x05 { - return nil, 0, errors.New("Invalid tag") + return nil, 0, isSequenceZero, errors.New(fmt.Sprintf("Invalid tag. Expected %d, Got: %d", 0x05, buffer[headerOffset])) } headerOffset++ - if codec.Uint16(buffer[headerOffset:]) != sequenceIdx { - return nil, 0, errors.New("Wrong sequenceIdx") + foundSequenceIdx := codec.Uint16(buffer[headerOffset:]) + if foundSequenceIdx == 0 { + isSequenceZero = true + } else { + isSequenceZero = false + } + + if foundSequenceIdx != sequenceIdx { + return nil, 0, isSequenceZero, errors.New(fmt.Sprintf("Wrong sequenceIdx. Expected %d, Got: %d", sequenceIdx, foundSequenceIdx)) } headerOffset += 2 @@ -136,7 +148,7 @@ func DeserializePacket( result = make([]byte, len(buffer)-int(headerOffset)) copy(result, buffer[headerOffset:]) - return result, totalResponseLength, nil + return result, totalResponseLength, isSequenceZero, nil } // WrapCommandAPDU turns the command into a sequence of 64 byte packets @@ -170,15 +182,33 @@ func UnwrapResponseAPDU(channel uint16, pipe <-chan []byte, packetSize int) ([]b var totalSize uint16 var done = false + // return values from DeserializePacket + var result []byte + var responseSize uint16 + var err error + + foundZeroSequence := false + isSequenceZero := false + for !done { // Read next packet from the channel buffer := <-pipe - result, responseSize, err := DeserializePacket(channel, buffer, sequenceIdx) + result, responseSize, isSequenceZero, err = DeserializePacket(channel, buffer, sequenceIdx) // this may fail if the wrong sequence arrives (espeically if left over all 0000 was in the buffer from the last tx) + + // Recover from a known error condition: + // * Discard messages left over from previous exchange until isSequenceZero == true + if foundZeroSequence == false && isSequenceZero == false { + continue + } + foundZeroSequence = true + if err != nil { return nil, err } - if sequenceIdx == 0 { + + // Initialize totalSize (previously we did this if sequenceIdx == 0, but sometimes Nano X can provide the first sequenceIdx == 0 packet with all zeros, then a useful packet with sequenceIdx == 1 + if totalSize == 0 { totalSize = responseSize } diff --git a/apduWrapper_test.go b/apduWrapper_test.go index ca8d5a2..377a89e 100644 --- a/apduWrapper_test.go +++ b/apduWrapper_test.go @@ -211,11 +211,12 @@ func Test_DeserializePacket_FirstPacket(t *testing.T) { var firstPacketHeaderSize = 7 packet, _, _ := SerializePacket(0x0101, sampleCommand, packetSize, 0) - output, totalSize, err := DeserializePacket(0x0101, packet, 0) + output, totalSize, isSequenceZero, err := DeserializePacket(0x0101, packet, 0) assert.Nil(t, err, "Simple deserialize should not have errors") assert.Equal(t, len(sampleCommand), int(totalSize), "TotalSize is incorrect") assert.Equal(t, packetSize-firstPacketHeaderSize, len(output), "Size of the deserialized packet is wrong") + assert.Equal(t, true, isSequenceZero, "Test Case Should Find Sequence == 0") assert.True(t, bytes.Compare(output[:len(sampleCommand)], sampleCommand) == 0, "Deserialized message does not match the original") } @@ -226,11 +227,12 @@ func Test_DeserializePacket_SecondMessage(t *testing.T) { var firstPacketHeaderSize = 5 // second packet does not have responseLength (uint16) in the header packet, _, _ := SerializePacket(0x0101, sampleCommand, packetSize, 1) - output, totalSize, err := DeserializePacket(0x0101, packet, 1) + output, totalSize, isSequenceZero, err := DeserializePacket(0x0101, packet, 1) assert.Nil(t, err, "Simple deserialize should not have errors") assert.Equal(t, 0, int(totalSize), "TotalSize should not be returned from deserialization of non-first packet") assert.Equal(t, packetSize-firstPacketHeaderSize, len(output), "Size of the deserialized packet is wrong") + assert.Equal(t, false, isSequenceZero, "Test Case Should Find Sequence == 1") assert.True(t, bytes.Equal(output[:len(sampleCommand)], sampleCommand), "Deserialized message does not match the original") } diff --git a/ledger_hid.go b/ledger_hid.go index c7e44f1..554b847 100644 --- a/ledger_hid.go +++ b/ledger_hid.go @@ -23,6 +23,7 @@ import ( "errors" "fmt" "sync" + "time" "github.com/zondax/hid" ) @@ -59,7 +60,7 @@ func (admin *LedgerAdminHID) ListDevices() ([]string, error) { devices := hid.Enumerate(0, 0) if len(devices) == 0 { - fmt.Printf("No devices") + fmt.Printf("No devices. Ledger LOCKED OR Keplr/Ledger Live may have control of device.") } for _, d := range devices { @@ -130,7 +131,7 @@ func (admin *LedgerAdminHID) Connect(requiredIndex int) (LedgerDevice, error) { } } - return nil, fmt.Errorf("LedgerHID device (idx %d) not found", requiredIndex) + return nil, fmt.Errorf("LedgerHID device (idx %d) not found. Ledger LOCKED OR Keplr/Ledger Live may have control of device", requiredIndex) } func (ledger *LedgerDeviceHID) write(buffer []byte) (int, error) { @@ -165,17 +166,52 @@ func (ledger *LedgerDeviceHID) readThread() { buffer := make([]byte, PacketSize) readBytes, err := ledger.device.Read(buffer) + // Check for HID Read Error (May occur even during normal runtime) if err != nil { - return + continue + } + + // Discard all zero packets from Ledger Nano X on macOS + allZeros := true + for i := 0; i < 64; i++ { + if buffer[i] != 0 { + allZeros = false + break + } } + + // Discard all zero packet + if allZeros { + // HID Returned Empty Packet - Retry Read + continue + } + select { case ledger.readChannel <- buffer[:readBytes]: + // Send data to UnwrapResponseAPDU default: + // Possible source of bugs + // Drop a buffer if ledger.readChannel is busy + } + } +} + +func (ledger *LedgerDeviceHID) drainRead() { + // Allow time for late packet arrivals (When main program doesn't read enough packets) + <-time.After(50 * time.Millisecond) + for { + select { + case <-ledger.readChannel: + default: + return } } } func (ledger *LedgerDeviceHID) Exchange(command []byte) ([]byte, error) { + // Purge messages that arrived after previous exchange completed + ledger.drainRead() + if len(command) < 5 { return nil, fmt.Errorf("APDU commands should not be smaller than 5") } From 5a73254a36017469e148601f5fa773555dd96c55 Mon Sep 17 00:00:00 2001 From: Chill Validation Date: Sat, 3 Dec 2022 07:43:24 +0900 Subject: [PATCH 2/4] CV Remove project specifc names --- apduWrapper.go | 2 +- ledger_hid.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apduWrapper.go b/apduWrapper.go index 38c16bb..1c3b6a0 100644 --- a/apduWrapper.go +++ b/apduWrapper.go @@ -53,7 +53,7 @@ func ErrorMessage(errorCode uint16) string { case 0x6E00: return "[APDU_CODE_CLA_NOT_SUPPORTED] CLA not supported" case 0x6E01: - return "[0x6E01] Ledger Connected but Cosmos App Not Open" + return "[0x6E01] Ledger Connected but Chain Specific App Not Open" case 0x6F00: return "APDU_CODE_UNKNOWN" case 0x6F01: diff --git a/ledger_hid.go b/ledger_hid.go index 554b847..70f8d3e 100644 --- a/ledger_hid.go +++ b/ledger_hid.go @@ -60,7 +60,7 @@ func (admin *LedgerAdminHID) ListDevices() ([]string, error) { devices := hid.Enumerate(0, 0) if len(devices) == 0 { - fmt.Printf("No devices. Ledger LOCKED OR Keplr/Ledger Live may have control of device.") + fmt.Printf("No devices. Ledger LOCKED OR Other Program/Web Browser may have control of device.") } for _, d := range devices { @@ -131,7 +131,7 @@ func (admin *LedgerAdminHID) Connect(requiredIndex int) (LedgerDevice, error) { } } - return nil, fmt.Errorf("LedgerHID device (idx %d) not found. Ledger LOCKED OR Keplr/Ledger Live may have control of device", requiredIndex) + return nil, fmt.Errorf("LedgerHID device (idx %d) not found. Ledger LOCKED OR Other Program/Web Browser may have control of device.", requiredIndex) } func (ledger *LedgerDeviceHID) write(buffer []byte) (int, error) { From 375a16fc58c3f84efcfabe93685fdc5c26628840 Mon Sep 17 00:00:00 2001 From: Chill Validation Date: Wed, 7 Dec 2022 17:58:53 +0900 Subject: [PATCH 3/4] CV move error check, check full buffer --- apduWrapper.go | 7 +++---- ledger_hid.go | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apduWrapper.go b/apduWrapper.go index 1c3b6a0..9738100 100644 --- a/apduWrapper.go +++ b/apduWrapper.go @@ -195,6 +195,9 @@ func UnwrapResponseAPDU(channel uint16, pipe <-chan []byte, packetSize int) ([]b buffer := <-pipe result, responseSize, isSequenceZero, err = DeserializePacket(channel, buffer, sequenceIdx) // this may fail if the wrong sequence arrives (espeically if left over all 0000 was in the buffer from the last tx) + if err != nil { + return nil, err + } // Recover from a known error condition: // * Discard messages left over from previous exchange until isSequenceZero == true @@ -203,10 +206,6 @@ func UnwrapResponseAPDU(channel uint16, pipe <-chan []byte, packetSize int) ([]b } foundZeroSequence = true - if err != nil { - return nil, err - } - // Initialize totalSize (previously we did this if sequenceIdx == 0, but sometimes Nano X can provide the first sequenceIdx == 0 packet with all zeros, then a useful packet with sequenceIdx == 1 if totalSize == 0 { totalSize = responseSize diff --git a/ledger_hid.go b/ledger_hid.go index 70f8d3e..b9ba067 100644 --- a/ledger_hid.go +++ b/ledger_hid.go @@ -173,7 +173,7 @@ func (ledger *LedgerDeviceHID) readThread() { // Discard all zero packets from Ledger Nano X on macOS allZeros := true - for i := 0; i < 64; i++ { + for i := 0; i < len(buffer); i++ { if buffer[i] != 0 { allZeros = false break From 4f033d2e9f6d1752245258d645d6fb1709e56441 Mon Sep 17 00:00:00 2001 From: Chill Validation Date: Wed, 7 Dec 2022 23:34:29 +0900 Subject: [PATCH 4/4] CV APDU_CODE_APP_NOT_OPEN message --- apduWrapper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apduWrapper.go b/apduWrapper.go index 9738100..daafa52 100644 --- a/apduWrapper.go +++ b/apduWrapper.go @@ -53,7 +53,7 @@ func ErrorMessage(errorCode uint16) string { case 0x6E00: return "[APDU_CODE_CLA_NOT_SUPPORTED] CLA not supported" case 0x6E01: - return "[0x6E01] Ledger Connected but Chain Specific App Not Open" + return "[APDU_CODE_APP_NOT_OPEN] Ledger Connected but Chain Specific App Not Open" case 0x6F00: return "APDU_CODE_UNKNOWN" case 0x6F01: