Merge pull request #23 from chillyvee/cv_mymacsad_zondax

Remove suprious OSX USB Reads

Author: ftheirs

.gitignore

@@ -16,6 +16,9 @@
 *.dylib
 *.dll
 
+# Fortran module files - Why? For now, Allow go.mod
+#*.mod
+
 # Fortran module files
 *.smod
 

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 "[APDU_CODE_APP_NOT_OPEN] Ledger Connected but Chain Specific 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,32 @@ 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)
 		if err != nil {
 			return nil, err
 		}
-		if sequenceIdx == 0 {
+
+		// Recover from a known error condition:
+		// * Discard messages left over from previous exchange until isSequenceZero == true
+		if foundZeroSequence == false && isSequenceZero == false {
+			continue
+		}
+		foundZeroSequence = true
+
+		// 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
 		}
 

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")
 }
 

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 Other Program/Web Browser 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 Other Program/Web Browser 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 < len(buffer); 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")
 	}