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

[idf.py] 'flash' command does not support Flash Encryption (Release) Mode, just soft bricks device (IDFGH-8923) #10340

Closed
3 tasks done
chipweinberger opened this issue Dec 9, 2022 · 8 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@chipweinberger
Copy link
Contributor

chipweinberger commented Dec 9, 2022

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v4.4.3-215-g9f3f82f54b

Operating System used.

macOS

How did you build your project?

Command line with idf.py

Related Issues

esptool.py: espressif/esptool#809
VSC Extension: espressif/vscode-esp-idf-extension#853

What is the expected behavior?

idf.py flash should detect when your chip in Secure Download Mode and help the user flash an encrypted binary.

What is the actual behavior?

idf.py flash flashes the binary as plaintext, causing the device to not boot.

My Suggested Behavior

Goal: idf.py flash should be a 1 line command that can flash a device despite the state of Flash Encryption.

idf.py flash should detect when your chip has Flash Encryption enabled using esptool.py get_security_info and force you to specify a flag --host-generated-key-file /path/to/key.bin, or --force-flash-raw otherwise return an error.

Note: --host-generated-key-file flag would do nothing unless Flash Encryption was detected. This way, when specified, we'll have a 1 line command that can alway flash a device despite the state of Flash Encryption.

Note: idf.py encrypted-flash does not support release mode and returns an error when tried. IMO, encryption should be supported in the normal idf.py flash command too. We need a single command that aways works.

To automate further, we could have a KConfig for specifying the location of the keyfile, similar to custom partition tables. A keyfile_path.txt will then be written into the build folder.idf.py flash would then consult this file, when it detects it needs to apply encryption. This would make idf.py flash work seamlessly despite the mode of the device.

Steps to reproduce.

  1. Enable Flash Encryption (Release)
  2. burn all relevant device fuses
  3. use 'idf.py flash'
@chipweinberger chipweinberger added the Type: Bug bugs in IDF label Dec 9, 2022
@chipweinberger chipweinberger changed the title [idf.py] 'flash' command does not support Flash Encryption (Release) Mode, soft bricks device [idf.py] 'flash' command does not support Flash Encryption (Release) Mode, just soft bricks device Dec 9, 2022
@espressif-bot espressif-bot added the Status: Opened Issue is new label Dec 9, 2022
@github-actions github-actions bot changed the title [idf.py] 'flash' command does not support Flash Encryption (Release) Mode, just soft bricks device [idf.py] 'flash' command does not support Flash Encryption (Release) Mode, just soft bricks device (IDFGH-8923) Dec 9, 2022
@mahavirj
Copy link
Member

@chipweinberger

There are 2 workflows to enable the flash encryption on the device. Device generated keys and host generated keys. We recommend the 1st one, as it provides highest level of the security, keys are generated on the device and then immediately gets read protected. Of-course, in the host generated keys approach, encryption could provide speed advantage.

For the 1st workflow, re-flashing with pre-encrypted artifacts on host is really not an option and OTA updates becomes the recommended option (with secure DL mode enabled). This is already highlighted in IDF docs too.

For the 2nd workflow, we have steps documented for re-flashing at https://docs.espressif.com/projects/esp-idf/en/latest/esp32s3/security/flash-encryption.html#using-host-generated-key

idf.py flash should detect when your chip has Flash Encryption enabled using esptool.py get_security_info and force you to specify a flag --host-generated-key-file /path/to/key.bin, or --force-flash-raw otherwise return an error.

IMO, this would involve lot of the complexity in the tooling level. Additionally it will be difficult to understand the workflow type purely based on the EFuse summary of the device.

Probably you may consider adding a idf.py project specific extension here, we don't have detailed documentation for it but here is pointer to relevant code

esp-idf/tools/idf.py

Lines 625 to 672 in 454aeb3

# Load extensions from components dir
idf_py_extensions_path = os.path.join(os.environ['IDF_PATH'], 'tools', 'idf_py_actions')
extension_dirs = [realpath(idf_py_extensions_path)]
extra_paths = os.environ.get('IDF_EXTRA_ACTIONS_PATH')
if extra_paths is not None:
for path in extra_paths.split(';'):
path = realpath(path)
if path not in extension_dirs:
extension_dirs.append(path)
extensions = []
for directory in extension_dirs:
if directory and not os.path.exists(directory):
print_warning('WARNING: Directory with idf.py extensions doesn\'t exist:\n %s' % directory)
continue
sys.path.append(directory)
for _finder, name, _ispkg in sorted(iter_modules([directory])):
if name.endswith('_ext'):
extensions.append((name, import_module(name)))
# Load component manager idf.py extensions if not explicitly disabled
if os.getenv('IDF_COMPONENT_MANAGER') != '0':
extensions.append(('component_manager_ext', idf_extensions))
# Optional load `pyclang` for additional clang-tidy related functionalities
try:
from pyclang import idf_extension
extensions.append(('idf_clang_tidy_ext', idf_extension))
except ImportError:
pass
for name, extension in extensions:
try:
all_actions = merge_action_lists(all_actions, extension.action_extensions(all_actions, project_dir))
except AttributeError:
print_warning('WARNING: Cannot load idf.py extension "%s"' % name)
# Load extensions from project dir
if os.path.exists(os.path.join(project_dir, 'idf_ext.py')):
sys.path.append(project_dir)
try:
from idf_ext import action_extensions
except ImportError:
print_warning('Error importing extension file idf_ext.py. Skipping.')
print_warning(
"Please make sure that it contains implementation (even if it's empty) of add_action_extensions")

@chipweinberger
Copy link
Contributor Author

chipweinberger commented Dec 12, 2022

Thanks for your consideration and thoughts.

I've noticed that esptool.py will add --no-stub automatically if Secure Download Mode is detected on the device.

My thought was that esptool.py must already be getting the Security Info on every call. If that was true, it should be simple to detect when flash encryption is enabled, and return an error asking to specify a keyfile.

Furthermore, it seems simple and ergonomic to support passing a keyfile in general, manually, to idf.py flash. I think users would appreciate the simplified workflow.

These are just ideas, of course.

@mahavirj
Copy link
Member

If that was true, it should be simple to detect when flash encryption is enabled, and return an error asking to specify a keyfile.

That won't help in the 1st workflow I described in my earlier comment. So idea here is that, once "release" mode of the flash enc is enabled (and it happens on the manufacturing line), only way remains to update the firmware is over the OTA update channel. If we are really need a way to update the firmware over UART DL mode then I think that can always happen using "Development" mode of the flash enc feature.

@chipweinberger
Copy link
Contributor Author

chipweinberger commented Dec 13, 2022

Right, for the 1st workflow it would still report an error, telling you to specify a keyfile (which you do not have). or a --force-flash command, to flash anyway (which would hard brick the device!).

This still seems like a big improvement. Especially for the 1st workflow, where you do not have the key and could ruin your device.

Use Case: Of course "Development" Mode (3rd workflow...), is the most ideal for development. But I want to produce beta devices for users that use Release Mode, which is secure, and want to retain the ability to fully update them over UART (bootloader, partition table, etc, if I need to.). For this, the current idf.py tools are not ergonomic.

Use Case 2: Not hard bricking device in 1st workflow. Am I wrong in saying that for the 1st workflow, accidentally calling idf.py flash will ruin your device without warning? I have never tried...

@mahavirj
Copy link
Member

mahavirj commented Feb 8, 2023

@chipweinberger

Use Case 2: Not hard bricking device in 1st workflow. Am I wrong in saying that for the 1st workflow, accidentally calling idf.py flash will ruin your device without warning? I have never tried...

Yes. We have now added a fix for this in the esptool through espressif/esptool@0be5fcd

Please check once, and if that looks good then probably we can close this ticket (implementing the host key based encryption can be handled in the project specific IDF extension, as I had commented earlier)

@chipweinberger
Copy link
Contributor Author

chipweinberger commented Feb 8, 2023

That PR looks wonderful!

Thank you for circling back and fixing this. That's a great improvement.

"WARNING: Detected flash encryption and "
"secure download mode enabled."
"Flashing plaintext binary may brick your device! "
"Use --force to override the warning."

I would say:

"Use --force to override this warning only if the binary and esp32 are encrypted with the same host generated key"

question - why does secure download mode matter? I would expect we just check for flash encryption release mode. Perhaps that is how we check for release vs development mode?

@mahavirj
Copy link
Member

mahavirj commented Feb 8, 2023

I will put this feedback internally. Interim, docs have bit more information with dedicated section for this: https://docs.espressif.com/projects/esptool/en/latest/esp32/esptool/basic-commands.html#encrypted-flash-protection

Thank you for your feedback and suggestions 👍

CC @Harshal5

@mahavirj
Copy link
Member

mahavirj commented Feb 8, 2023

question - why does secure download mode matter? I would expect we just check for flash encryption release mode. Perhaps that is how we check for release vs development mode?

Yes, the change primarily detects if encrypted writes are still supported through UART DL mode or not. In case, they are supported then potentially device could be recovered, irrespective of the FLASH_CRYPT_CNT state.

@espressif-bot espressif-bot added Resolution: Done Issue is done internally Status: Done Issue is done internally and removed Status: Opened Issue is new labels Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

3 participants