feat: enable secure-boot for ttgo tdisplay #13
Conversation
| python << EOF | ||
| from jadepy import JadeAPI | ||
| import _cbor2 | ||
| import serial | ||
|
|
||
| create_jade_fn = JadeAPI.create_serial | ||
| kwargs = {'device': None, 'timeout': 5} | ||
|
|
||
| try: | ||
| with create_jade_fn(**kwargs) as jade: | ||
| verinfo = jade.get_version_info() | ||
|
|
||
| if 'SB' in verinfo['JADE_FEATURES']: | ||
| print('enabled') | ||
| else: | ||
| print('disabled') | ||
| except _cbor2.CBORDecodeEOF: | ||
| print('uninitialized') | ||
| except serial.serialutil.SerialException: | ||
| print('devicenotfound') | ||
| EOF |
There was a problem hiding this comment.
This is a cool approach to running python inside the script. Should we put this script in a separate .py file since it will be shared code?
There was a problem hiding this comment.
I like to keep redundancies small but in this case i see problems if we move this to a separate file. We would then need to pull it in every script with curl from a fixed url. This has major disadvantages:
- You execute code unknown code from an http endpoint
- Pulling stuff from online resources prevents offline usage
- You cant test different python code in separate git branches because every time you run the script it will be pulled from master (i see this problem currently with the dependency pulling)
| esptool.py --chip esp32 --before=default_reset --after=no_reset write_flash --flash_mode dio --flash_freq 40m --flash_size keep 0x1000 build/bootloader/bootloader.bin | ||
| echo "Flashing app..." | ||
| sleep 2 | ||
| esptool.py --before default_reset --after no_reset --chip esp32 write_flash --flash_mode dio --flash_size keep --flash_freq 40m 0x9000 build/partition_table/partition-table.bin 0xe000 build/ota_data_initial.bin 0x10000 build/jade.bin |
There was a problem hiding this comment.
Did you pull these from https://github.com/Blockstream/Jade/blob/1e5f9d163adfada1a83fb9b34e8435236d0e8a16/main/qemu/make-flash-img.sh#L13?
These memory addresses should be the same for all four devices, right?
There was a problem hiding this comment.
I took these from the output of the esptool.py after building the bootloader as suggested in the youtube tutorial here https://youtu.be/PeqP6oVnlIs?t=1231 (and removed port and baut rate as suggested).
Actually i am not sure if the addresses are the same for all devices, i would except that but i cant test it.
|
|
||
| if [[ "$enable_secure_boot" == "y" ]]; then | ||
| clear | ||
| echo -e "\nWARNING: You are about to enable Secure Boot on your device. This process is irreversible. Once Secure Boot is enabled, all future installations, updates, or firmware modifications will require the signing key created during this process. It is crucial to store the signing key (~/jade-diy.pem) in a secure, offline location and remove it from your computer to prevent unauthorized access.\n\n" |
There was a problem hiding this comment.
Can you please make ~/jade-diy.pem a constant at the top?
Something like something like signing_key_path or whatever you think is appropriate.
There was a problem hiding this comment.
Yes sure, good point
| cp configs/sdkconfig_display_ttgo_tdisplay.defaults sdkconfig.defaults | ||
| sed -i.bak '/CONFIG_DEBUG_MODE/d' ./sdkconfig.defaults |
There was a problem hiding this comment.
Just curious - was this causing errors or a style preference?
There was a problem hiding this comment.
Just a style preference to shorten the code
| idf.py build | ||
|
|
||
| [ "${CI:-false}" = true ] && echo "Exiting the script for CI runners." && exit 0 | ||
|
|
||
| while [ ! -c "${tty_device}" ]; do |
There was a problem hiding this comment.
Why did you remove these 8 lines?
There was a problem hiding this comment.
I think i did not remove them but moved them more to the top L64-74 (but skipped them within ci runners) to make sure that all flashing requirements are met before the build process starts
|
Thanks for your review and for inviting me to this project. I will come back to this soon but our workshop session starts at the end of this week so i dont want to introduce new changes now. So my plan is to collect users feedback for both versions (with and without secure-boot) and improve the code based on the users feedback |
|
Note: Some devices only support secure boot version 1. There should be a variable added to optionally set to sb version 1. Additional infos for how to enable secure boot: https://hideyourkeys.io/cheap-hardware-wallet-below-diy-guide/ |
… error on update and bump versions
|
@1-21gigasats thanks for proposing this change. Can you please rebase the PR for review and testing? |
I am currently refraining from adapting secure boot version 1, as the only hardware that is unable to do v2 is the M5Stack M5StickC PLUS according to https://github.com/Blockstream/Jade/tree/master/diy . If there are specific versions per device in the future it is maybe the best to adapt v1 for this version only as it needs different build config and openssl key. If you try to flash SB v2 on an v1 only device, boot loader flash should fail with an appropriate message. |
Yes sure but actually i am unfamiliar with the rebasing process. Could you explain me what to do or do it for me one time and show me the commands you used afterwards? But if the only goal is to keep a clean commit history, is a squash and merge after review not more appropriate? In my understanding rebasing requires to force push which feels unsafe |
1-21gigasats
changed the title
Here is my draft based on the discussions in epiccurious/jade-diy#76 for a ttgo to enable secure-boot via user menu including the mentioned python script to detect the device status.
The python script can detect the status:
I tested them all but did not test a full secure boot installation as this would waste another device reserved for the workshops. maybe you can test one full installation with enabling secure boot