search

Nitrokey / heads

A minimal Linux that runs as a coreboot or LinuxBoot ROM payload to provide a secure, flexible boot environment for laptops and servers.
http://osresearch.net/
GNU General Public License v2.0
15 stars 1 forks source link

Firmware file consistency verification #3

Closed szszszsz closed 4 years ago

szszszsz commented 4 years ago

At the moment firmware file consistency check is not made before the flashing. This could result in hard to debug issues, or complete "bricking" of the hardware on the user side, shall the data transfer to the USB drive be interrupted.

This should be automated. The firmware binary and metadata (hash,signature) should be packed to a single .tar archive, which would be then checked by the Heads during the update execution.

For now we instruct users to do it by hand:

alex-nitrokey commented 4 years ago

I am not sure if this is best adresses in osresearch/heads instead.

I am not sure yet if this is necessary, though.

szszszsz commented 4 years ago

Let's discuss it here first, and it can later be moved to upstream.

Why do you think its not needed? Is there any firmware file verification implemented already?

alex-nitrokey commented 4 years ago

Okay, at first I was wondering if flashrom itself would complain about a incomplete file, but even then it would not detect bitwise errors. I am not sure how severe this issue is, but in my opinion it is a valid point.

To keep it simple I would recommend a solution in which the checksum is only checked if a checksum file is present and ignored otherwise so that users could flash without the check if they want.

Furthermore, I think this is something that we might be able to provide ourselves and creating a PR upstream afterwards. Only finding some time would be vital :grimacing:

szszszsz commented 4 years ago

Exactly. PR possible, but time is scarce.

I would not agree with the hash sum check being optional - support side this will be a nightmare to troubleshoot, if any of that will happen. And 1 in 1000 will probably do. Why not building own firmware format, like suggested? I do not see any disadvantages. tar is already available and supported in the Heads.

alex-nitrokey commented 4 years ago

I see, I missed the tar part. Sound like a valid idea, just wanted to avoid letting the user store multiple files. Actually, now that I think about it I pretty much like it :+1: We just need to look which (if any) compression/archive tools are supported in heads.

szszszsz commented 4 years ago

Tar and zip are AFAIR :-)

alex-nitrokey commented 4 years ago

I added a simple file integrity check here

What do you think about including yours and mine PGP key in the built and adding signing checks as well? Otherwise, I think this can be closed by next release.

szszszsz commented 4 years ago
  1. Please rename extension to something unique and recognizable as Nitropad firmware package, like e.g. .npf (from NitroPadFirmware). See docx and xlsx as analogy. The reason is, that it will get mixed with regular user archives during file selection, and perhaps confuse user to unpack archive after downloading and before flashing.
  2. UI: potential improvement - on using ROM file there should be a confirmation dialog asking, whether it was checked with sha256sum tool by hand earlier on (could be safely done later).
  3. Do you mean public keys for import and verifying the package against? I think that's a good idea in general, I am worried though about possibly of easily done phishing attacks on the UI, if provided with the firmware pack file. How would you like to implement it? It would be better probably to have this integrated with the firmware image in the keychain.
alex-nitrokey commented 4 years ago

Please rename extension to something unique and recognizable as Nitropad firmware package, like e.g. .npf (from NitroPadFirmware).

Are you sure? Actually, I like the fact that it is a plain .zip archive :thinking:

UI: potential improvement - on using ROM file there should be a confirmation dialog asking, whether it was checked with sha256sum tool by hand earlier on.

I will add a warning.

Do you mean public keys for import and verifying the package against? I think that's a good idea in general, I am worried though about possibly easily done phishing attacks on the UI, if provided with the firmware pack file. How would you like to implement it? It would be better probably to have this integrated with the firmware image in the keychain.

I mean having our key pairs already included in the trustdb of the gnupg folder in the rom an checking the signatures of a sha256sum.txt.gpg in the archive. On the other hand this would mean that we would be two people able to manipulate the machine. On the other hand this would make it much more difficult to flash roms of the bad guys... And some trust in Nitrokey needs to be taken for granted anyway.

alex-nitrokey commented 4 years ago

Please rename extension to something unique and recognizable as Nitropad firmware package, like e.g. .npf (from NitroPadFirmware). See docx and xlsx as analogy. The reason is, that it will get mixed with regular user archives during file selection, and perhaps confuse user to unpack archive after downloading and before flashing.

What about using .tar instead? Less likely confusion. But I am fine with arbitrary file endings as well :shrug:

szszszsz commented 4 years ago

What about using .tar instead? Less likely confusion. But I am fine with arbitrary file endings as well

I do not see how that would help :-) Let's not be afraid of the custom extension. We have a format with a structure we expect in the software, hence we are fully entitled to have one. From the UX perspective I think it would be better.

szszszsz commented 4 years ago

I mean having our key pairs already included in the trustdb of the gnupg folder in the rom an checking the signatures of a sha256sum.txt.gpg in the archive. On the other hand this would mean that we would be two people able to manipulate the machine. On the other hand this would make it much more difficult to flash roms of the bad guys... And some trust in Nitrokey needs to be taken for granted anyway.

It would be nice to have this but for informative purposes only, as long as the firmware is not planned to be locked. If time will come, the proper key will be established and handled with care. Now the decision to execute firmware update is on the user, and obligation to confirm the authenticy.

alex-nitrokey commented 4 years ago

It would be nice to have this but for informative purposes only, as long as the firmware is not planned to be locked. If time will come, the proper key will be established and handled with care. Now the decision to execute firmware update is on the user, and obligation to confirm the authenticy.

Of course, the user shall always have the possibility to shoot herself in the foot :) No, seriously, I am pretty much against any kind of restriction of flashing just some random file! There should be a fat warning though.

alex-nitrokey commented 4 years ago

change .zip to .npf and added warning for *.rom users. https://github.com/Nitrokey/heads/commits/verified_firmware

szszszsz commented 4 years ago

Looks good!

alex-nitrokey commented 4 years ago

Fixed in v1.2