Use salt from the SD card for encrypted storage

[prusnak]

Depends on merging https://github.com/trezor/trezor-firmware/pull/264

If the file named /trezor/entropy is found on the SD card during the boot, open the file, hash it and use the result as an extra entropy when initializing the encrypted storage.

We need to discuss the following: 1) how do we transition from regular storage to storage+sd_entropy storage (i.e. do we only allow this via the process on Trezor or can a user create their own entropy file manually on PC?) 2) do we need to store a special flag in the storage? (bool use_sd_card_entropy) - this would be very helpful, but we might need to initialize the storage twice?

[andrewkozlik]

Terminology I propose using the term "SD salt" or "external salt" rather than "entropy". The reason is that this data will be combined with the PIN using PBKDF2 and it will be appended to the salt parameter which currently consists of "random salt" stored in the NV RAM and "hardware salt".

Requirements

• The SD card may be inserted and removed while Trezor is in operation.
• The SD card must be inserted during unlock and during PIN change.
• Multiple Trezor devices may store their salt on one SD card.
• Trezor must determine whether it has been configured to work with a salt and ask the user to insert the SD card when it is required.
• Trezor must detect and alert the user if the salt on the SD card is invalid, old or forged so that the user can identify a DOS attack.
• The following operations will be supported via trezorctl:
  • Generate salt.
  • Regenerate salt.
  • Remove salt.

Implementation details

• Trezor internal storage will contain a random SD salt authentication key as a plaintext entry. The regenerate salt operation will regenerate this authentication key.
• The salt will be stored in one of the following files (to be decided):
  • /trezor/device_/salt
  • /trezor/salt_
• The file will contain 32 bytes of random data followed by a 16 byte HMAC-SHA256 authentication tag.

Possible future features to consider

• Automatically regenerate the salt during every unlock operation. This would increase flash degradation, so it should be an optional setting.
• Allow the user to make a backup of the salt onto another SD card directly on Trezor. Note that backing-up the salt is in contradiction with the previous feature.
• Allow paranoid users to provide their own salt.

[prusnak]

This would increase flash degradation, so it should be an optional setting.

Maybe we can do the following trick to avoid flash degradation when replacing the salt:

1. keep the original salt file
2. store the new salt in salt.new
3. erase salt file
4. rename salt.new to salt

This might force FAT to use new blocks every time for a new entropy file, but we should test it. Also it is a non atomic operation.

[matejcik]

1. keep the original salt file
2. store the new salt in salt.new
3. erase salt file
4. rename salt.new to salt

this makes old salts recoverable. is that a problem? it doesn't seem to be, but?

[andrewkozlik]

1. keep the original salt file
2. store the new salt in salt.new
3. erase salt file
4. rename salt.new to salt

this makes old salts recoverable. is that a problem? it doesn't seem to be, but?

Making old salts recoverable would be a bit problematic. Consider a scenario where an attacker tampers with the Trezor and manages to read out the internal NVRAM, then at some later time the attacker gains access to the SD card. If the user suspects that somebody tampered with their Trezor, then they could regenerate the salt and think they are safe. But if old salts are recoverable then the attacker could still decrypt the state of the storage as it was at the moment of tampering and gain access to the seed. On the other hand if the attacker gained access to the Trezor, then they could have done worse things. For example replaced the user's Trezor with a fake one, which would send all data from the SD card to the attacker as soon as it is plugged in.

In any case, I assumed that in step 3 above we would overwrite the original salt file with random data. To be more precise:

1. keep the original salt file
2. store the new salt in salt.new
3. reencrypt the DEK and SAK using the new salt
4. overwrite salt file with random data
5. rename salt.new to salt

[matejcik]

In any case, I assumed that in step 3 above we would overwrite the original salt file with random data. To be more precise:

That makes the exercise pointless from the flash wear standpoint.

But in any case we are still wearing out the FAT segment. Isn't this a moot point though? How susceptible are today's SD cards to flash wear?

[andrewkozlik]

I started work on this feature: https://github.com/trezor/trezor-firmware/tree/andrewkozlik/sd-salt. The implementation seems to be working well on the emulator, but I haven't had time to test it all that much. I didn't do any testing whatsoever on the device. @stick please have a look to check that I am using the SD card and FATFS correctly.

TODOs

• Possibly some minor refactoring or renaming of functions.
• Write core unit tests.
• Write device tests.
• Write storage tests.
• Create a separate function to manage the entire PIN checking (unlock) process, see below.

Notes In the core code I refer to the salt as "SD card salt", but in the storage code I refer to it as "external salt". Right now it's the same thing, but later on the external salt will be a combination of different salts from different sources.

Note that when this is merged with the FIDO2 branch, the check_pin() function in core/src/apps/webauthn/init.py will need to take retrieve the salt. This is similar to the change made in recovery_device/init.py.

It would be good to delegate the whole PIN checking procedure to a separate function, because it is getting complicated - get PIN from user, check if SD salt is in use, get salt from SD card, check PIN. Right now this code is duplicated in various forms in several places.

Testing Some test scenarios that come to mind:

• Removal of SD card at different stages, e.g. during PIN entry in unlock, change PIN and SD salt operations
• Switching SD card for a different one without salt.
• Switching SD card for one with an old salt previously generated on the same device.
• Entering invalid PIN during unlock, change PIN and SD salt operations.
• Check change_pin works with salt enabled and disabled.
• Check unlock, change PIN and SD salt operations work with PIN enabled and disabled.
• Check load_device works as before.
• Check recovery_device works as before.
• Check reset_device works as before.

[andrewkozlik]

The implementation is finished, but I think we still need to make a final decision on the naming of this feature. In case "SD card salt" is not suitable, it would be best to change the message name (SdSalt) and trezorctl command name (currently sd-salt) accordingly before merging. I prepared a short description of this feature for the wiki, which might help us identify some key words:

This feature serves as additional protection against physical attacks. When it is enabled, a randomly generated secret is stored on the SD card. During every PIN checking and unlocking operation this secret is combined with the entered PIN value to decrypt data stored on the device. Simply put, the device gets bound to the SD card and cannot be unlocked without it. However, if the SD card is lost or damaged the device can still be factory-reset or recovered using the backup seed.

Some names that come to mind (with trezorctl command and example usage):

1. SD card protect: sd-protect, "secure your Trezor with SD card protection"
2. SD card salt: sd-salt, "secure your Trezor with with SD card salt"
3. SD card lock: sd-lock, "lock your Trezor with an SD card"
4. SD card encryption: sd-encrypt, "encrypt the storage with SD card salt"
5. SD card binding: sd-bind, "bind your Trezor with an SD card"
6. SD card key: sd-key, "secure your Trezor with SD card key"
7. SD card armor: sd-armor, "secure your Trezor with SD card armor"

[prusnak]

Let's have a vote on Monday :-)

[andrewkozlik]

The results of the vote, "+" indicates a vote for, "-" indicates a vote against:

(-1, +4) sd-protect: +@tsusanka , -@ciny , +@hiviah , +@prusnak , +@bentonoliver (-2, +1) sd-salt: -@tsusanka , +@hiviah , -@prusnak (-1, +0) sd-lock: -@prusnak (-3, +0) sd-encrypt: -@andrewkozlik , -@onvej , -@prusnak (-0, +2) sd-bind: +@tsusanka , +@bentonoliver (-2, +0) sd-key: -@onvej , -@prusnak

(I am not sure I captured all votes, so feel free to edit the above.)

Some opinions that people expressed:

• The word "protect" is too generic and vague. It's not immediately clear what is being protected and how.
• The word "salt" will have no meaning to many users.
• "sd-lock" could be mistaken to mean locking the SD card against writing.
• The association of the SD card with the word "encrypt" should be reserved for encryption of data on the SD card, which is a different feature.
• The word "bind" is not very well known among non-native speakers. The word "pair" might be a better alternative in this respect, but it's more vague.
• The word "key" is very generic.

@bentonoliver, @hiviah any opinions?

[hiviah]

I think actually "salt" means something to many users who heard at least something about cryptography. "sd-protect" would be second, but it's too generic.

So

• sd-salt
• sd-protect

[bentonoliver]

++ sd-bind ++ sd-protect Because both describe the intended result (as opposed to "salt" or "key").

[prusnak]

Closed via #526

[andrewkozlik]

Testing @sorooris

You will need two FAT32-formatted SD cards. You will also need to be using the current version of trezorctl. Clone the trezor-firmware repo if you don't have it yet and execute the following:

cd trezor-firmware
git checkout master
git pull
pipenv shell
# Type any trezorctl commands into this shell.

There is one new command for this feature in trezorctl: trezorctl sd-protect --help trezorctl sd-protect enable trezorctl sd-protect refresh trezorctl sd-protect disable

Basic flow:

1. Insert SD card when Trezor is powered down.
2. Power up.
3. Run trezorctl sd-protect enable. Verify that the device asks for confirmation. Note: We are still experiencing random failures when writing to the SD card, so if this step fails, then try repeating it several times. Verify that it fails gracefully.
4. Remove SD card and restart. Verify that the device cannot be unlocked without the SD card and user is informed.
5. Power down, insert the same SD card, power up. Verify that the device can be unlocked successfully.
6. Run trezorctl sd-protect disable. Verify that the device asks for confirmation.
7. Remove SD card and restart. Verify that the device can be unlocked without the SD card.

Scenarios (some copied from above for convenience):

• Perform the basic flow on a device which has a PIN set and on a device which has PIN disabled.
• Between steps 5 and 6 of the basic flow verify that the PIN can be enabled, disabled or changed and the device works as expected.
• Between steps 5 and 6 of the basic flow run trezorctl sd-protect refresh.
• Test removal of SD card at different stages, e.g. during PIN entry in unlock, during change PIN or during one of the sd-protect operations. Verify that the device fails gracefully.
• When SD protection is enabled, switch the SD card for a clean one. Verify that the clean one cannot be used to unlock the device and user is informed.
• Enable SD protection on two Trezors each with its own SD card. Verify that when the SD cards are switched, then one cannot be used to unlock the other device and the user is informed.
• Enable SD protection on one Trezor and then on another Trezor using the same SD card for both. Verify that the SD card can be used to unlock both Trezors. Disable SD protection on one of the Trezors and verify that the SD card can still be used to unlock the other one.
• Enter invalid PIN during unlock, change PIN and SD protection operations. Verify that the device fails gracefully.
• Verify that seed check works as expected when SD protection is enabled or disabled.
• Verify that FIDO2 operations which require PIN entry work as expected when PIN and SD protection are enabled.