search

boltcard / bolt-nfc-android-app

Program your bolt card easily from your android mobile with this app
MIT License
36 stars 14 forks source link

Fix for boltcard bricking caused by false version assumptions #43

Closed johnheenan closed 2 months ago

johnheenan commented 2 months ago

This is a fix for boltcard bricking caused by false assumption that a setup is always version number 1 and a reset is aways version number 0.

johnheenan commented 2 months ago

Please see https://github.com/boltcard/bolt-nfc-android-app/issues/44 for issue

NicolasDorier commented 2 months ago

I don't understand why this would fix anything, though I don't see any problem into doing it that way.

Keeping track of the version in the ntag can be something useful on its own right. Asked more questions on the issue.

If this version is sent to boltcard service in the request asking for a Reset, then I can make sure I am grabbing the right keys for the user to reset. Doing this requires more code on both the app, btcpay side and the spec, but this is quite easy win. The downside would be that we would be limited to 255 setup/reset cycles... Also we don't want to break currently issued cards.

johnheenan commented 2 months ago

If this version is sent to boltcard service in the request asking for a Reset, then I can make sure I am grabbing the right keys for the user to reset. Doing this requires more code on both the app, btcpay side and the spec, but this is quite easy win. The downside would be that we would be limited to 255 setup/reset cycles... Also we don't want to break currently issued cards.

The version is not sent to the boltcard service (BTCPay Server for example). The version comes from the boltcard service.

The current master code DOES NOTHING with the version contained in the variable named json parsed from the boltcard service for seup in file SetupBoltcard.js const {K0, K1, K2, K3, K4, LNURLW: lnurlw_base} = json; and also DOES NOTHING with the version in the same variable named json for reset in file ResetBoltcard.js const {K0, K1, K2, K3, K4} = json;

To me this is completely black and white.

The source code is wrong. No argument. Case closed. A fix is provided. Move on.

johnheenan commented 2 months ago

The fix is centred around two lines:

For setup: const {K0, K1, K2, K3, K4, LNURLW: lnurlw_base, version=1} = json;

For reset: const {K0, K1, K2, K3, K4, version=0} = json;

For the situation where a version is not provided for setup or reset there is no change to code behaviour

For the situation where a version is provided then the version is respected.

If this fix breaks other apps then the other apps are buggy. Black and white

johnheenan commented 2 months ago

Currently there is evidence the app store apk behaves as if a fix is in place (see issue https://github.com/boltcard/bolt-nfc-android-app/issues/44) and so was compiled from different source code.

For the purpose of this Pull Request, this has no relevance.

As I indicated in the issue, such a practice, if it occurred, should not be considered aberrant.

johnheenan commented 2 months ago

This Pull Request won't fix bricking. See https://github.com/boltcard/bolt-nfc-android-app/issues/44#issuecomment-2316953426