Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readout protection should be enabled by default for added security #6

Merged
merged 2 commits into from Nov 20, 2018

Conversation

baldurmen
Copy link

When I first flashed my Tomu, I went over the instructions quickly and didn't see the part about the readout protection.

I think having it on by default makes for more secure defaults and will help others not to make the mistake I made by inadvertence.

@gl-sergei
Copy link
Member

Consider different case. You wanted to test your board and try to run various apps and examples and you accidentally locked SWD. Now you want to develop your own app and debug it with open source tools like OpenOCD or Black Magic Probe. You are going to have some trouble because these tools are not able to unlock SWD. You will need proprietary tools like J-Link for it.

@baldurmen
Copy link
Author

As far as I understand, it's easy to force the Tomu back into the bootloader by shorting the two contacts on the top. You can re-flash the chip from there if you accidentally locked SWD.

I understand that some people will want to play around with the Tomu board or use it for development, but I think the majority will use it as a cheap, open-source and open-hardware U2F token. I fear people will miss out on this step and never put the chip into readout protection.

@xobs
Copy link
Member

xobs commented Aug 8, 2018

I think there is some confusion.

The "Enable Readout Protection" is, perhaps, a misleadingly named variable. What it does is prevent the ability to enter Toboot and flash new firmware. It permenantly installs this u2f program.

With Readout Protection disabled, users can enter Toboot and flash new firmware by shorting the pins, as @gl-sergi mentioned.

Regardless of the value of Readout Protection, the block where the keys are stored is always erased when a new program is loaded. In that way, you can't pull out the keys by loading a smaller program and dumping flash.

Of course, regardless of the value you can extract it with SWD, which is one reason the case is clear.

@baldurmen
Copy link
Author

baldurmen commented Aug 8, 2018

@xobs Thanks, I understand now.

I'm not sure this really works then, as I've been flashing my Tomu with the Readout Protection flag on and I can still access Toboot by shorting the pins...

mkdir -p build

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/u2f.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/u2f.o.d -I. -I.. u2f.c -o build/u2f.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/usb-hid.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/usb-hid.o.d -I. -I.. usb-hid.c -o build/usb-hid.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/dbug.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/dbug.o.d -I. -I.. dbug.c -o build/dbug.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/u2f-hid.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/u2f-hid.o.d -I. -I.. u2f-hid.c -o build/u2f-hid.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/u2f-apdu.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/u2f-apdu.o.d -I. -I.. u2f-apdu.c -o build/u2f-apdu.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/sha256.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/sha256.o.d -I. -I.. sha256.c -o build/sha256.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/neug.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/neug.o.d -I. -I.. neug.c -o build/neug.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/random.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/random.o.d -I. -I.. random.c -o build/random.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/ec_p256r1.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/ec_p256r1.o.d -I. -I.. ec_p256r1.c -o build/ec_p256r1.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/bn.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/bn.o.d -I. -I.. bn.c -o build/bn.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/jpc_p256r1.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/jpc_p256r1.o.d -I. -I.. jpc_p256r1.c -o build/jpc_p256r1.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/call-ec_p256r1.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/call-ec_p256r1.o.d -I. -I.. call-ec_p256r1.c -o build/call-ec_p256r1.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/modp256r1.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/modp256r1.o.d -I. -I.. modp256r1.c -o build/modp256r1.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/mod.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/mod.o.d -I. -I.. mod.c -o build/mod.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/hmac.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/hmac.o.d -I. -I.. hmac.c -o build/hmac.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/csn.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/csn.o.d -I. -I.. csn.c -o build/csn.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/platform.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/platform.o.d -I. -I.. platform.c -o build/platform.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/entry.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/entry.o.d -I. -I.. ../entry.c -o build/entry.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/chopstx.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/chopstx.o.d -I. -I.. ../chopstx.c -o build/chopstx.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/eventflag.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/eventflag.o.d -I. -I.. ../eventflag.c -o build/eventflag.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/sys-efm32.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/sys-efm32.o.d -I. -I.. ../mcu/sys-efm32.c -o build/sys-efm32.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/usb-efm32.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/usb-efm32.o.d -I. -I.. ../mcu/usb-efm32.c -o build/usb-efm32.o

arm-none-eabi-gcc -c -mcpu=cortex-m0plus -O3 -Os -fstack-usage -ffunction-sections -fdata-sections -fno-common -Wall -Wextra -Wstrict-prototypes -Wa,-alms=build/adc-efm32.lst -DMAKE_ENTRY_PUBLIC -DUSE_SYS3 -DFREE_STANDING -DMHZ=21 -DENFORCE_DEBUG_LOCK -mthumb -mno-thumb-interwork -DTHUMB -MD -MP -MF .dep/adc-efm32.o.d -I. -I.. ../mcu/adc-efm32.c -o build/adc-efm32.o

arm-none-eabi-gcc build/u2f.o build/usb-hid.o build/dbug.o build/u2f-hid.o build/u2f-apdu.o build/sha256.o build/neug.o build/random.o build/ec_p256r1.o build/bn.o build/jpc_p256r1.o build/call-ec_p256r1.o build/modp256r1.o build/mod.o build/hmac.o build/csn.o build/platform.o build/entry.o build/chopstx.o build/eventflag.o build/sys-efm32.o build/usb-efm32.o build/adc-efm32.o  -mcpu=cortex-m0plus -nostartfiles -Tu2f.ld -Wl,-Map=build/u2f.map,--cref,--no-warn-mismatch,--gc-sections  -mthumb -mno-thumb-interwork  -o build/u2f.elf
arm-none-eabi-objcopy -O binary build/u2f.elf build/u2f.bin

@xobs
Copy link
Member

xobs commented Aug 8, 2018

That is correct. You can still access Toboot by shorting the pins, and you can flash new software.

You cannot read out the current firmware, only write new firmware.

And thanks to https://github.com/im-tomu/chopstx/blob/efm32/u2f/platform.c#L50 any firmware you flash won't have access to the keys, because they will be erased prior to your software getting written.

@xobs
Copy link
Member

xobs commented Aug 8, 2018

Ah, I believe I am the one who is confused.

Yes, this prevents attaching SWD, and doesn't prevent entering Toboot. That is a different flag.

Perhaps it is a good idea to disable SWD, then...

@lightonflux
Copy link

It would be pretty sweet if the readme provided info how to check if readout protection is enabled on the flashed firmware. To increase trust.

@gl-sergei
Copy link
Member

@baldurmen
Copy link
Author

Any more thoughts on this?

If you don't want to merge it, please close the PR. Otherwise, I'd really like this to be merged.

@xobs xobs merged commit d7c847b into im-tomu:efm32 Nov 20, 2018
@xobs
Copy link
Member

xobs commented Nov 20, 2018

It's a good change. Thanks for the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants