dmitrystu / sboot_stm32

Secure USB DFU1.1 bootloader for STM32
Apache License 2.0
303 stars 63 forks source link

add checksum functionality #6

Closed sn00pster closed 5 years ago

sn00pster commented 5 years ago

Hi,

I have modified the code to allow checksum verification of the application, this prevents invalid application images from being booted, the bootloader will remain in dfu mode if it cannot verify that the image is valid.

There are 3 new config.h defines:

define DFU_VERIFY_CHECKSUM _ENABLE

define DFU_CHECKSUM_OFFSET 0x800

define DFU_ENCRYPT_CHECK _ENABLE

DFU_VERIFY_CHECKSUM :

Specifies whether the checksum verification is enabled, if it is disabled then the bootloader operates as it did before.

DFU_CHECKSUM_OFFSET:

This specifies the offset of the checksum in the application image, you must locate 2 uint32_t at this location in your firmware image using your compiler/linker settings.

uint32_t app_checksum[2] = {0x01020304, 0x0A0B0C0D}; // locate this variable at the specified offset in flash

The first entry is the firmware length and the second entry is the firmware crc, they are set to the values 0x01020304 and 0x0A0B0C0D respectively as this allows the fwcrypt program to verify that the given offset is valid when DFU_ENCRYPT_CHECK is enabled. (these values will be replaced with the correct values by fwcrypt)

DFU_ENCRYPT_CHECK:

If enabled, fwcrypt will check that the checksum locations contain 0x01020304, 0x0A0B0C0D, if not the fwcrypt program will exit with an error, if they do contain it then the checksum area was allocated correctly and the checksum verification can be performed.

fwcrypt has been updated to support the generation and insertion of the firmware crc and firmware length information into the binary image before encryption.

dmitrystu commented 5 years ago

Thank you for your contribution. I have an another idea to have a firmware verification without magic numbers and offsets. Using 2 or more sequental CRC.

bootloader user firmware CRC1 CRC2 CRC3 CRC4

Here is a algo in pseudocode:

uint32_t ptr = app_start;
uint32_t checksum = initial_value;
int count = 0;
do {
    checksum = crc32_turn(checksum, *ptr++);
    if (checksum == *ptr)  {
        if (++count == 4) goto application;
    } else {
        count = 0;
} while (in ROM)
goto bootloader;
sn00pster commented 5 years ago

The magic numbers are only there so that you can verify that you have indeed correctly placed the crc at the right location in memory, you can disable them if you like.

If you're placing the CRC after the firmware then I don't know how you will know where it is as the size of the firmware can change, this is why I put the "crc section" at a known point in the application because it will always be at that location, in my case it's straight after the vectors in the application image. You also need to know the length of the application to calculate the crc, hence why my changes to fwcrypt generate the correct crc and store this along with the length at the known location.

I'm actually just making a few changes to my branch at the moment, I'm adding a non lookup table crc option which is slower but decreases the amount of flash used by the crc.

sn00pster commented 5 years ago

I see what you're doing in your algorithm, I know the chance of false positive is low but it is a possibility with your algorithm, I prefer the known location method.

sn00pster commented 5 years ago

Maybe once I've finished my implementation we could add support for yours as well?

sn00pster commented 5 years ago

I've finished up my changes to the L4xx version, works with all the flavours, you can select FAST or SMALL checksum. If you don't have the definition in your config file then the checksum is disabled, the define needs to be in the config to turn it on.

When it's turned on, if you don't define the type of checksum it defaults to SMALL, you have to define another parameter to enable FAST checksum.

I need to apply these changes to all the other startup files and then I will push them, it will be monday now.

sn00pster commented 5 years ago

@dmitrystu I will add support for your algorithm on Monday, we only need one CRC, the second word after the CRC will be the XOR of the CRC, I think that’s good enough.

sn00pster commented 5 years ago

I’m happy to just keep the checksum at the end method combined with an XOR, I think this will work nicely. As I mentioned, I’ve also implemented the CRC without the lookup table.

I will push all my changes on Monday and I hope they will be satisfactory.

sn00pster commented 5 years ago

@dmitrystu Hi Dmitry, I have pushed my latest changes for the checksum validation.

I've tested this on the l475. The M0 variant needs to be checked, I don't have any M0 systems so cannot test the code myself, but there were obvious changes to the checksum routines to make it compatible with the M0 core, I think it's correct but needs double checking.

I implemented the variant of your suggestion and removed by previous version, fwcrpyt stores the crc and (crc ^ 0xFFFFFFFF) in the following two words directly after the firmware image.

The checksum is passed on boot when the word after the current byte matches the current checksum and the word after matches the XOR of it, if there's no match on the checksum then the crc continues until all of FLASH has been tested.

I modified the include "../config.h" and changed it to "config.h" in the assembler files, my reason for doing this is that it allows sboot_stm32 to be used as a submodule and the tree never needs to be modified by the user, they copy the config.h into their build tree and modify it there.

Thanks for your previous comments on this and the suggestion of storing the crc after the firmware image.

One other thing, we might want to add a weak definition to a routine to allow the clock to be configured early in the boot process, this would make the crc substantially faster if the processor is running from its main clock, obviously the user will have to supply the actual function in their code (probably c) but I think it would be a huge benefit.

dmitrystu commented 5 years ago

I can merge this commit into the separate branch.

sn00pster commented 5 years ago

Thanks for your comments.

I will fix these things tomorrow and update.

Apologies for leaving the table included in the slow case as well, it gets a little complex when you are flipping between different source files and trying to keep them the same, obviously moving the checksum calculation to a separate file solves this.

sn00pster commented 5 years ago

@dmitrystu Dimitry, I have (hopefully) made all the changes now.

I am unable to test the cortex-m0 checksum implementations as I do not have access to an M0 processor, if you have access could you please test it.

I have added copyright notes to the files I have added, I'm unsure as to whether it also needs to reference you as well, please advise me on this and whether I need to amend these.

I have added the files to the Makefile, but I'm unable to test as I do not use Makefiles or have a build system set up to use them, I simply added the checksum files to both the crypt utility and the main firmware.

The checksum method as per your request is now completely generic and alternative methods can be provided easily.

dmitrystu commented 5 years ago

Created a new branch 'feat/checksum'. Unfortunately can't add you as a collaborator for this branch. This is a restriction of the personal account. Will check later. I'm a bit drunk now :wine_glass:

sn00pster commented 5 years ago

Not a problem, probably best not to look at commits while drunk! I was talking to my colleague earlier and we have some other stuff which we may provide implementations for which may be of use to people, again, making them configurable so that if people dont need it then its not included.

Have fun, enjoy being drunk!

sn00pster commented 5 years ago

@dmitrystu When I am in work in the morning I will get the office to order me a discovery board with an M0 on it so that I can fix the M0 port.

sn00pster commented 5 years ago

@dmitrystu Changes related to M3/M4 ports done and pushed to the tree.

M0 changes still outstanding, I have a M0 discovery board arriving tomorrow so that I can do the M0 modifications and testing.

Hopefully we are close now.

sn00pster commented 5 years ago

Board with STM32L053C8T6 arrived today but unfortunately I was busy with other stuff, I will hopefully finish this up on Monday.

dmitrystu commented 5 years ago

Created WIP merge request for this feature. https://github.com/dmitrystu/sboot_stm32/pull/10 Let's continue there.