security bug: multiple access to .pem file passed to --key-file option

[sirio]

nrfutill accesses (opens and closes) the .pem file passed to --key-file multiple times.

This forces the .pem file to be written to disk, which is a security breach when running on an untrusted machine (e.g. a CI platform).

For example, generating a DFU settings page with this command:

nrfutil settings generate \
    --family NRF52 \
    --application my_app_nrf52832_xxAA_s132_7.0.1.hex \
    --application-version 1 --bootloader-version 1 --bl-settings-version 2 \
    --app-boot-validation VALIDATE_ECDSA_P256_SHA256 \
    --sd-boot-validation VALIDATE_ECDSA_P256_SHA256 \
    --softdevice nRF5_SDK_16.0.0_98a08e2/components/softdevice/s132/hex/s132_nrf52_7.0.1_softdevice.hex \
    --key-file key.pem \
    settings.hex

... requires reading from 'key.pem' which must be written to disk.

We can use 'cat' to simulate a process which will perform some authentication and output the private key itself (without writing it to disk): `--key-file <(cat key.pem)`

Unfortunately, this breaks:

  File "/Users/siriobalmelli/.nix-profile/lib/python3.7/site-packages/nordicsemi/__main__.py", line 396, in generate
    sd_boot_validation_type=sd_boot_validation, sd_file=softdevice, key_file=key_file)
  File "/Users/siriobalmelli/.nix-profile/lib/python3.7/site-packages/nordicsemi/dfu/bl_dfu_sett.py", line 260, in generate
    self.sd_boot_validation_bytes = Package.sign_firmware(key_file, self.sd_bin)
  File "/Users/siriobalmelli/.nix-profile/lib/python3.7/site-packages/nordicsemi/dfu/package.py", line 581, in sign_firmware
    signer.load_key(key)
  File "/Users/siriobalmelli/.nix-profile/lib/python3.7/site-packages/nordicsemi/dfu/signing.py", line 88, in load_key
    self.sk = SigningKey.from_pem(sk_pem)
  File "/Users/siriobalmelli/.nix-profile/lib/python3.7/site-packages/ecdsa/keys.py", line 791, in from_pem
    privkey_pem = string[string.index(b("-----BEGIN EC PRIVATE KEY-----")):]
ValueError: subsection not found

This is because the file is accessed twice, first here:

            elif app_boot_validation_type == 'VALIDATE_ECDSA_P256_SHA256':
                self.app_boot_validation_type = 3 & 0xffffffff
                self.app_boot_validation_bytes = Package.sign_firmware(key_file, self.app_bin)

... and then also here:

            elif sd_boot_validation_type == 'VALIDATE_ECDSA_P256_SHA256':
                self.sd_boot_validation_type = 3 & 0xffffffff
                self.sd_boot_validation_bytes = Package.sign_firmware(key_file, self.sd_bin)

Removing softdevice validation with `--sd-boot-validation VALIDATE_GENERATED_CRC`, the problem goes away because the "keyfile" (in this case the `<(cat key.pem)` subshell) is only accessed once.

This forces the caller to write the private key file to disk. Creating a temp file is still insecure: it cannot be unlinked from the filesystem because it will be accessed twice.

Opening a file multiple times like this is also inefficient and bad form.

The private key file should be opened and read _once_, and then the contents passed to functions that require it.

I am willing to file a pull request if there is interest in getting this fixed.

[Hung Bui]

Hi Sirio, 

The developer will continue the discussion on the github case. 
I'm just curious on why having the private key file on hard disk would be an issue ? I assume your plan is to use some kind of authorized external storage instead of the internal hard drive to store the key file ? 
Our assumption is that the process to generate DFU .zip file or bootloader setting file should be done on a secure PC, trusted by the manufacturer. 
Could you explain what you meant by CI platform ? 

[sirio]

Thank you, I appreciate the answer.

One example of a workflow were writing to disk is a problem is where the private key is queried from an external system, with some authentication and logging done each time.

If the private key obtained this way is passed as the output of a subprocess, it exists only in memory while the DFU page is generated, and is never visible to other processes by being written to disk.

This does not make the key irrecoverable, but is significantly raises the bar. Writing a temp file to disk means an attacker can get the private key trivially, for example on macOS:

`while true; do find /var/folders -type f -name "tmp*" -exec cat '{}' 2>/dev/null \; ; done`

CI is Continuous Integration (see https://en.wikipedia.org/wiki/Continuous_integration) ... which is one example of a type of system where avoiding writing secrets to disk is preferred.

I am closing the issue here and will continue on Github.