Hardware wallet wire protocol

[slush]

I would like to propose the convention that if the wallet sees a field in a protocol buffer that it does not recognise, if that field is 0, false or "" (depending on the type of the field), the wallet can safely ignore the field.

I'm affraid that there's no safe solution how to handle forward compatibility by the wallet. I think that if there will be some incompatible messages in the future, clients should ask for wallet version (part of Features message) and then pick appropriate set of messages. This may turn into the mess when wallet developers won't be sane enough, but I'm also affraid of wallet which will try to recover from some messages which it doesn't fully understand...

As it stands, nanopb currently doesn't allow this (it skips all fields it doesn't recognise). But I'm sure it's possible to modify the runtime decoder to follow this convention.

Forking protocol buffer standard? No way, at least not for me :-).

I see the approach slush has taken is to include a "Features" message which enumerates a wallet's features. I think a features list is somewhat open to interpretation.

I'm affraid of "partial support" of some features. Once you support OTP/PIN messages or not. I'm writing test suite which should cover 100 % of device use cases, so I believe this test suite will help developers to be sure all "features" are well implemented.

For example, what happens if a wallet only supports a subset of feature X? Does it report that it supports feature X or not?

It should not report support for some feature which is not fully implemented. As I said, we're handling with valuable information and we should not try to recover from unexpected state by heuristic or some guessing.

[someone42]

Forking protocol buffer standard? No way, at least not for me :-).

I'm not aware of any standard saying that protobuf parsers must skip unrecognised fields. In fact, modifying a parser to fail on unrecognised fields would make things more secure: it would protect against client software accidentally using an unsupported feature and then misleadingly reporting success to a user.

I'm affraid of "partial support" of some features. Once you support OTP/PIN messages or not. I'm writing test suite which should cover 100 % of device use cases, so I believe this test suite will help developers to be sure all "features" are well implemented.

It should not report support for some feature which is not fully implemented. As I said, we're handling with valuable information and we should not try to recover from unexpected state by heuristic or some guessing.

I agree that supporting OTP and PIN is a yes or no thing. But sometimes it is not so clear, and we can't anticipate what advanced features might be added to wallets in the future.

The ignore-on-0 convention would not encourage "unexpected state", as long as 0/false/"" is always defined to mean that a feature is disabled. For example, say an "is_hidden" field is added to the LoadDevice message. Wallets which do not support hidden volumes can safely ignore is_hidden = false. If such a wallet ever gets a message with is_hidden = true, it fails with a "feature unsupported" failure message.

Such a convention is safely used in microcontrollers, where reserved bits in peripheral hardware registers are often supposed to be set to 0. Then, if features are added to those registers, the designers will take 0 to mean disabled and 1 to mean enabled.

Edit: Here's a compromise: if a wallet sees an unrecognised field, it always fails with "feature unsupported", no matter what the value of the field. All new fields must be optional. If client software doesn't want to use a new feature, it must not include the corresponding fields.

[2112]

In my experience the response to the Ping message should contain a non-optional sequence number. This is particularly important in case of the device like yours that may require slight physical rotation to be able to read the LCD and/or may be jostled while pressing the button.

Theoretically the OS should issue USB disconnect/connect events in case of the brief interruption sensed on the USB port, Windows even plays a warning chime. But in practice this doesn't work 100% reliably and the USB device may get reset while being handled by the user to complete the required interaction.

There could be also an opposite situation where the USB device is plugged to the powered USB hub. The user interaction could cause hub disconnection, but the device's internal state would not be affected, despite the application receiving and handling the disconnect/connect events. If the application can detect that the device hadn't lost the internal state it may save the user some aggravation caused by a repeated initialization and retry.

[2112]

Unrelated to the above, there is one more design decision that you may need to make.

It seems that as it stands right now the protocol may be training the user to blindly press the button without actually looking at the LCD screen.

In the medical device field there is an ongoing discussion: what is the correct level of interaction required to dismiss a dialog about potentially dangerous and irreversible action.

1) pressing always the same button to approve after hearing a beep or always another button to cancel and hear another acknowledgement tone;

2) pressing exactly one of 4-10 buttons after reading the message, and where the message tells which button to press and the number of the button needs to change with repeated presentation of the same message. Pressing any other button than indicated in the message is interpreted as cancellation.

In case of the Piglet the button on the device would be still the same, single one, but the user would be required to look at the screen and the screen would tell the user which key to press on the computer's keyboard.

The medical field apparently still cannot agree which way to go on this.

[jim618]

For the upgrade path of the protobuf format there are a few things we do in the bitcoinj wallet:
1) there is a wallet major and minor version. The major version gets updated on non backwards compatible changes. That way something like MultiBit can look at the version number and say: this is a wallet version 6 - I only know about wallets up to 5 so sorry I cannot load it.

2) also you can put in a new mandatory field to effectively jam a spanner in the machine. A protobuf parser cannot 'legally' load a mandatory field that it does not know about. You can use this to really really stop messages from the future being loaded.

3) in the bitcoinj wallet protobuf there is an extension message with a name/value pair. You can use it to add in additional information - it is designed so that, say, I want to put in some org.multibit fields I can add them in. Other clients will just read them and faithfully write them back (to avoid data loss) but not do anything with them.

[marcus_of_augustus]

Watching.

[J-Norm]

It should speak RS232. Almost all point of sale systems speak it and the usb bridges are ubiquitous.

[stick]

It should speak RS232. Almost all point of sale systems speak it and the usb bridges are ubiquitous.

I think what we try to establish in this thread is an application protocol. Physical transport is not that important (RS232, PS/2, USB HID, Infrared, ...)

[slush]

My only request is that, for all the non-signing messages, there is a maximum message size specified. A reasonable value would be something like 256 bytes. This allows buffers to be statically allocated (i.e. without the use of malloc). The limit cannot apply to signing messages, because transactions can be very large.

Hard-encoding maximum message size into the protocol sounds problematic. It already requires hacks for message signing, so you still need some workaround in the code. Why to limit other messages then?

PB message can be decoded in smaller chunks, i.e. you don't need to allocate receiving buffer to fit whole message into it.

[slush]

...There could be also an opposite situation where the USB device is plugged to the powered USB hub. The user interaction could cause hub disconnection, but the device's internal state would not be affected...

I just added "session_id" variable into Initialize/Features messages, so the computer can easily restart the device's internal state and check that all buffers have been cleared, because computer received the same session id back.

Handling buffers and delayed responses may be a bit tricky as I already see in my serial transport prototype. However session_id/ping messages are quite effective way how to detect possible issues on the wire.

[2112]

I just added "session_id" variable into Initialize/Features messages, so the computer can easily restart the device's internal state and check that all buffers have been cleared, because computer received the same session id back.

Handling buffers and delayed responses may be a bit tricky as I already see in my serial transport prototype. However session_id/ping messages are quite effective way how to detect possible issues on the wire.

I really don't want to interfere with your design, but my feeling is that a rarely-changing session_id will not be sufficient.

Fundamentally, you have a problem of synchronizing two state machines over a channel with erasures.

I'm not sure that the above problem has any simpler solution that the "sequence number" patterned after TCP over IP (or X-MODEM or Kermit protocol.)

The session_id would be then equivalent to seqence numbers starting with a value different than zero and different for each session.

Again, those are my feelings. I don't have a concrete proof, neither for nor against.

Edit: On the other hand I still remember the lesson you taught me in the Stratum thread: the protocol has to be conceptually simple enough to be reasonably implemented by the majority of the Bitcoin application programmers. If the price is an occasional, rare failure then it is worth to pay it.

[beekeeper]

Just a suggestion (I found default TI code quite bugged, especially under linux).
Since you want to use Stellaris Launchpad. The best way would be, ofc, to use a different chip for USB com which also be in charge to reset the arm chip by toggling its reset pin. E.g. if you use a USB<->SER converter, like one made by FTDI, you can use its RTS and DTR pins to perform reset.
If you want to have the ARM chip running wallet to do the USB device part too (which is bad for security imho), you can do something I tested already with TI code. In my case, the ARM chip runs a CDC, not HID, but it can easily converted to some HID command.
In usb_dev_serial, there is ControlHandler() which calls SetControlLineState() on USBD_CDC_EVENT_SET_CONTROL_LINE_STATE event (this happens when /dev/ttyACM is opened and closed).
In SetControlLineState() I added:

Code:

        //when /dev/ttyACM is closed, RTS ->0 is emulated. When this happens, we will reset the entire chip

	//old_can_send, can_send some boolean/char vars initialized to 0

	can_send = 0;
	if((usState & 2) != 0) //D1  Carrier control for half duplex modems. This signal corresponds to V.24 signal 105 and RS-232 signal RTS.
		can_send  = 1;
	if(old_can_send != can_send ){
		if(old_can_send == 1){
			MAP_SysCtlPeripheralReset(SYSCTL_PERIPH_USB0); //explicit peri reset, system reset wont reset peripherials too
			SysCtlReset(); //system reset
		}
		//other blah blah
	
	}
	old_can_send = can_send;

   
   
In main() I added:

Code:

    USBDCDCInit(0, (tUSBDCDCDevice *)&g_sCDCDevice);
	
	//after initing USB stuff or it will segfault
    //system reset and peri reset dont signal usb host anything so we have to do something about
    //USB host will see this device disconnecting and another device connecting, so everything will be reinitialized host side
    MAP_USBDevDisconnect(USB0_BASE);
    for(ulLoop = 0; ulLoop < 100000000; ulLoop++){;}
    MAP_USBDevConnect(USB0_BASE);

   

[someone42]

My only request is that, for all the non-signing messages, there is a maximum message size specified. A reasonable value would be something like 256 bytes. This allows buffers to be statically allocated (i.e. without the use of malloc). The limit cannot apply to signing messages, because transactions can be very large.

Hard-encoding maximum message size into the protocol sounds problematic. It already requires hacks for message signing, so you still need some workaround in the code. Why to limit other messages then?

PB message can be decoded in smaller chunks, i.e. you don't need to allocate receiving buffer to fit whole message into it.

After having a closer look at nanopb's API, I withdraw any request for limited sizes of messages or fields. It is indeed possible to decode smaller chunks of messages. There is even a mechanism to decode smaller chunks of individual fields.

[jim618]

I have been starting to think about what UI changes will be needed for MultiBit for BIP32 wallets.

One of the limitations of MultiBit/ bitcoinj is that currently you do not have quick access to the complete set of unspent outputs for an arbitary key (or BIP32 sequence of keys).

MultiBit has to 'replay the blocks' i.e. get the blocks starting with a far-enough-back-in-time block and set what transactions are relevant. It is very useful to have a date "slightly before the first transaction or the birthdate of the wallet". You can then set the blockchain head to then and get the blocks from a bitcoind. Bloom filters will speed this up but it is conceptually the same.

Anyhow, is it possible to know the birthdate of a wallet ? By this I mean the date when the wallet seed was created.
Is there a real-time network-synced clock on the device that could be remember this and expose it? Or the host computer could give the device the current time perhaps.

Otherwise I will have to ask the user for the creation date of the wallet (which of course they will not know accurately) or get it from another channel other than bitcoind (possible but then I am leaking pretty important information to some server like blockchain.info or an electrum server and I am no longer strictly P2P).

[slush]

Anyhow, is it possible to know the birthdate of a wallet ?

Interesting ideas, Jim. Technically this is not a problem, however thanks to LoadDevice() method, device can contain seed used before the device was created. For example when user lost the device and will try to reload seed to new one. As this is not 100% bulletproof, we should look at another solution...

Otherwise I will have to ask the user for the creation date of the wallet (which of course they will not know accurately) or get it from another channel other than bitcoind (possible but then I am leaking pretty important information to some server like blockchain.info or an electrum server and I am no longer strictly P2P).

Looks like this is issue not directly related to Multibit, but to chainless clients generally. Maybe we should encode something into the mnemonic format itself? Full timestamp isn't possible because it requires too much bits, but maybe some lighter solution?

Edit: After quick brainstorming with Stick I realized that this is stupid idea, for many various reasons. So I think that asking user for some date is the best and the most transparent solution so far.

[jim618]

Yes I think I will have to ask the user or get it from a non P2P server.

I presume the device does not even have a battery and gets it's power from the USB so it wouldn't have a real time clock.

[slush]

I presume the device does not even have a battery and gets it's power from the USB so I wouldn't have a real time clock.

Correct.

[jgarzik]

Anyhow, is it possible to know the birthdate of a wallet ? By this I mean the date when the wallet seed was created.
Is there a real-time network-synced clock on the device that could be remember this and expose it? Or the host computer could give the device the current time perhaps.

Generally speaking, yes, you should store the key birthday (or seed birthday) somewhere.  When [re]scanning, this makes a huge difference in startup and network-sync times.

[stick]

Generally speaking, yes, you should store the key birthday (or seed birthday) somewhere.  When [re]scanning, this makes a huge difference in startup and network-sync times.

While it might make a huge difference now (because seeded private keys are relatively new concept), it won't make a big difference in a future. I agree it might help to avoid unnecessary rescanning, but relying on that feature is flawed ...

[jgarzik]

Generally speaking, yes, you should store the key birthday (or seed birthday) somewhere.  When [re]scanning, this makes a huge difference in startup and network-sync times.

While it might make a huge difference now (because seeded private keys are relatively new concept), it won't make a big difference in a future. I agree it might help to avoid unnecessary rescanning, but relying on that feature is flawed ...

Can you elaborate?

All the major client developers agree this is a good idea.  The benefits into the future increase, as the block chain gets larger.  It makes an even bigger difference in the future.