hww: _api_info write byte for initialized status #1229

NicolaLS · 2024-06-12T19:14:47Z

Write another byte for the initialized status in the _api_info function. This field can be used to check whether a device can be unlocked or not. The unlocked byte is not sufficient because unlocked = false could mean the device is locked or the device is uninitialized.

benma

Awesome, thanks. Please add a CHANGELOG entry

benma · 2024-06-12T19:43:40Z

src/hww.c

@@ -97,6 +99,10 @@ static size_t _api_info(uint8_t* buf)
    *current = keystore_is_locked() ? 0x00 : 0x01;
    current++;

+    // 1 byte initialized status
+    *current = memory_is_initialized() ? 0x00 : 0x01;


wouldn't 0x01 for "is_initialized" be more intuitive? If one interprets 0x00 as false and 0x01 as true.

True, I just copy and pasted the above without thinking much. I'll change it, and also change the switch in the go api then.

benma

Thanks! Please also, in a new commit, adjust the Py lib in the same repo, add a CHANGELOG entry to the Py lib and bump the pylib version (major version bump as it's an incompatible change)

https://github.com/BitBoxSwiss/bitbox02-firmware/blob/master/py/bitbox02/bitbox02/communication/bitbox_api_protocol.py#L683

NicolaLS · 2024-06-18T13:48:43Z

py/bitbox02/bitbox02/communication/bitbox_api_protocol.py

-        This is useful to get the version of the firmware when a usb descriptor is not available
-        (via BitBoxBridge, etc.).
+        Returns (version, platform, edition, unlocked, initialized).
+        This is useful to get the version of the firmware or the device unlocked/initialized status


I think the This is useful (...) part could be improved but I don't understand it well enough and also I'm bad at constructing concise sentences.

-> this is useful if there was no handshake yet too, otherwise we could just call the other info request that already has the initialized attribute in the response...how could this be phrased ? (when I tried to use the other request in api-go it would throw an error like this: handshake must come first

but I guess it is fine to leave it as is?

benma · 2024-06-19T07:39:04Z

py/bitbox02/bitbox02/communication/bitbox_api_protocol.py

+        initialized_byte = response[0]
+        initialized = {0x00: False, 0x01: True}[initialized_byte]


This will crash on firmware versions that don't support this yet. Maybe one way of dealing with this is to return Optional[bool] and document that it is None if not supported.

ah yes thanks, it would throw index out of range for initialized_byte = response[0] should I just try except it and set initialized to None in except or is there a better way of handling this (checking somehow if it is supported ?)

You can check the firmware version, there are other examples. For now let's expect this to go into v9.20.0, but we can adjust the number if it lands quicker.

benma

CI complains about code formatting. Apply black py/bitbox02/bitbox02/communication/bitbox_api_protocol.py to format the file.

benma · 2024-06-25T09:23:26Z

py/bitbox02/bitbox02/communication/bitbox_api_protocol.py

+        Returns (version, platform, edition, unlocked, initialized).
+        This is useful to get the version of the firmware or the device unlocked/initialized status
+        when a usb descriptor is not available (via BitBoxBridge, etc.). In case the firmware does not
+        respond with the initialized byte yet None is returned.


The docstring could be more explicit: "The initialized status is supported from firmware v9.20.0 (it is None if not supported)"

NicolaLS · 2024-06-25T14:29:37Z

@benma thanks! rebased and re-formatted/changed docstring as suggested.

benma

please also bump version strings to 9.20.0, which is now possible after we released 9.19.0

https://github.com/BitBoxSwiss/bitbox02-firmware/blob/master/CMakeLists.txt#L99-L100

benma · 2024-07-02T08:16:25Z

py/bitbox02/bitbox02/communication/bitbox_api_protocol.py

-        return (version_str, platform, edition, unlocked)
+
+        initialized = None
+        if version_str >= semver.VersionInfo(9, 20, 0):


Please test your changes - this check does not work, it crashes because version_str is not a valid version string with the v prefix.

Use parse_device_version(version_str) instead.

right, thanks.

Write another byte for the initialized status in the _api_info function. This field can be used to check whether a device can be unlocked or not. The unlocked byte is not sufficient because unlocked = false could mean the device is locked or the device is uninitialized.

The device now returns an additional byte from REQ_INFO: 0x00 if the device is not initialized and 0x01 if it is. Read this byte in get_info from the REQ_INFO response in case the firmware version supports it, and add it to the tuple that is returned, None if the firmware does not support it yet.

benma

tACK, thanks!

benma requested changes Jun 12, 2024

View reviewed changes

NicolaLS mentioned this pull request Jun 12, 2024

api/firmware/device: info use device status fields BitBoxSwiss/bitbox02-api-go#98

Open

NicolaLS force-pushed the hww-info-initialized branch 2 times, most recently from 6f3fa21 to 0884c0e Compare June 12, 2024 20:25

NicolaLS requested a review from benma June 12, 2024 20:26

NicolaLS force-pushed the hww-info-initialized branch from 0884c0e to a84f8c5 Compare June 17, 2024 16:14

benma requested changes Jun 18, 2024

View reviewed changes

NicolaLS commented Jun 18, 2024

View reviewed changes

NicolaLS requested a review from benma June 18, 2024 13:51

benma requested changes Jun 19, 2024

View reviewed changes

NicolaLS force-pushed the hww-info-initialized branch 2 times, most recently from 6fcdc15 to 5b74fa5 Compare June 21, 2024 01:56

NicolaLS requested a review from benma June 21, 2024 01:58

benma requested changes Jun 25, 2024

View reviewed changes

NicolaLS force-pushed the hww-info-initialized branch from 5b74fa5 to c915e94 Compare June 25, 2024 14:28

benma requested changes Jul 2, 2024

View reviewed changes

NicolaLS force-pushed the hww-info-initialized branch from c915e94 to 62b4f29 Compare July 2, 2024 14:52

NicolaLS force-pushed the hww-info-initialized branch from 62b4f29 to 349983a Compare July 2, 2024 15:01

NicolaLS requested a review from benma July 2, 2024 15:21

benma approved these changes Jul 3, 2024

View reviewed changes

benma merged commit 34b38f1 into BitBoxSwiss:master Jul 3, 2024
1 check passed

NicolaLS mentioned this pull request Jul 30, 2024

get_info read initialized byte from HWW_INFO req BitBoxSwiss/bitbox-api-rs#82

Merged

		initialized_byte = response[0]
		initialized = {0x00: False, 0x01: True}[initialized_byte]

hww: _api_info write byte for initialized status #1229

hww: _api_info write byte for initialized status #1229

Uh oh!

Conversation

NicolaLS commented Jun 12, 2024

Uh oh!

benma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolaLS commented Jun 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benma left a comment

Choose a reason for hiding this comment

Uh oh!

benma Jul 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

NicolaLS commented Jun 25, 2024 •

edited

Loading

benma Jul 2, 2024 •

edited

Loading