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

esp32s3, multiple signatures required for new firmwares (both at boot and during ota) (IDFGH-13711) #14583

Closed
greenaddress opened this issue Sep 15, 2024 · 15 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@greenaddress
Copy link

Is your feature request related to a problem?

esp32s3 supports up to 3 signatures blocks being setup during secureboot bootloader flashing however it seems that by default it only requires 1 at any given time during OTA, the other signatures block for future updates can be set to 0xFF. If they are not set to 0xFF then they are checked. This is the current assumption based on documentation and what we can infer from it.

This effectively means that esp32s3 currently supports 1of1/1of2/1of3 configurations

Describe the solution you'd like.

ideally there was built-in support in the bootloader implementation and configuration to pick between 1of1/1of2/2of2/1of3/2of3/3of3 solutions

some of these options are clearly not going to work well with key revocation but that should not be a blocker alone.

Describe alternatives you've considered.

one could fork the bootloader - it seems that the hooks may not be enough without a lot of re-implementation and/or copy and paste, and the other example with the function call_start_cpu0 overridden may also not be enough alone.

Also both of the above options don't seem to have access to a number of private includes

so perhaps forking the full bootloader and have a common function between this and ota to verify that the app image has at least k of m signatures as specified in some Kconfig or even hardcoded in the bootloader is forked.This assuming that the bootloader checks the ones not set to 0xFF otherwise the loop checking for the secore boot signature blocks needs to be changed too.

Additional context.

a k of n solution like 2of3 is safer for the signers and for the end users of a product as they eliminate a single point of failure

it goes very well in hand with deterministic builds for what is worth (but this is already supported luckily!)

@greenaddress greenaddress added the Type: Feature Request Feature request for IDF label Sep 15, 2024
@espressif-bot espressif-bot added the Status: Opened Issue is new label Sep 15, 2024
@github-actions github-actions bot changed the title esp32s3, multiple signatures required for new firmwares (both at boot and during ota) esp32s3, multiple signatures required for new firmwares (both at boot and during ota) (IDFGH-13711) Sep 15, 2024
@KonstantinKondrashov
Copy link
Collaborator

KonstantinKondrashov commented Sep 18, 2024

Hi @greenaddress!

esp32s3 supports up to 3 signatures blocks being setup during secureboot bootloader flashing however it seems that by default it only requires 1 at any given time during OTA

Yes, it is correct.
OTA and bootloader do the same verification for 1 key. But if it was revoked or invalid it goes to the next key, and so on.
The digests of every public key are stored in eFuse bloks. So if you have 3 keys it means 3 eFuse blocks are used. If you use only 1 signature then unused secure boot blocks have to be revoked. From the doc:

To ensure no trusted keys can be added later by an attacker, each unused key digest slot should be revoked with KEY_REVOKEX. It will be checked during app startup in esp_secure_boot_init_checks() and fixed unless CONFIG_SECURE_BOOT_ALLOW_UNUSED_DIGEST_SLOTS is enabled.

Note that the efuse blocks are not set to FF if they are not used.
There are 3 revoke fields (1 bit each) for every key.

the other signatures block for future updates can be set to 0xFF

It is related to Signature Block Format. The signature blocks are appended to the binary at the end. So unused blocks filled 0xFF.

@greenaddress
Copy link
Author

@KonstantinKondrashov assume for the time being i do not wish to use the revocation system and that I want to use all 3 digest slots

Additionally I want to enforce the fact that the bootloader and images always have at least 2 block signatures matching the in the digest, what would be the best way within the espressif design? (currently as you agreed it only checks for 1)

I would like to have any 2 valid keys out of the 3 digests, with one not being present or if present also needs to be valid (clearly it has to be present in the bootloader as its signature blocks get then used for the secure boot final setup)

So far it seems what we want is not doable via hooks/override of function and that instead it may require a full bootloader fork via copy in components.

Is this something planned for development? Could you provide pointers if you have ongoing development and in any case would you have any recommendations?

@igrr would also appreciate your input on this as comments/experience has helped us many times before

@KonstantinKondrashov
Copy link
Collaborator

KonstantinKondrashov commented Sep 18, 2024

I would like to have any 2 valid keys out of the 3 digests, with one not being present or if present also needs to be valid (clearly it has to be present in the bootloader as its signature blocks get then used for the secure boot final setup)

Ok, you want bootloader and OTA verify 2 (or even 3 if presented) signatures instead of 1. Currently we do not support this scheme. For now, you can append up to 3 signatures (see the doc). You want to have a verification scheme which will depend on a Kconfig option (how many signatures need to be verified):

  • 1of 1
  • 1of 2
  • 2of 2
  • 1of 3
  • 2of 3
  • 3of 3 (if any of them fail then it fails)
    (Currently we support only 1of 1, 1of 2, 1of 3).

As I can see it is feasible the only question is whether we need this feature or not. Does it give additional security?
cc @mahavirj.

@greenaddress
Copy link
Author

@KonstantinKondrashov Yes it gives more security, especially the 2of2/2of3/3of3 because with any 1ofX any given key being compromised allows for bad firmware being signed. A key could easily be compromised on a computer, but 2 or 3 keys on 3 different computers, ideally with 3 different architectures/os could provide for a better defense.

@KonstantinKondrashov
Copy link
Collaborator

There are some disadvantages in your scheme:

  • Boot time will be increased.
  • In our scheme, Applications should be signed with only one key at a time, to minimize the exposure of unused private keys. In your case, the exposure of unused private keys might be an issue.
  • The ROM bootloader will still verify the 1 not revoked key to load the 2nd stage bootloader. There is no additional security added.

@greenaddress
Copy link
Author

@KonstantinKondrashov of course there are trade offs and each configuration depends on trade offs each project needs.

I understand if espressif decides not to support such configurations but I would still like to explore how best to implement it

@mahavirj
Copy link
Member

@greenaddress

Additionally I want to enforce the fact that the bootloader and images always have at least 2 block signatures matching the in the digest, what would be the best way within the espressif design? (currently as you agreed it only checks for 1)

To support this requirement, both bootloader (reference) and app (reference) verification schemes will need changes. Even after doing this, the verification workflow from ROM loader -> 2nd stage bootloader shall still verify authenticity of only 1 signature block.

FWIW, the idea with key revocation is documented here: https://docs.espressif.com/projects/esp-idf/en/latest/esp32s3/security/secure-boot-v2.html#key-revocation

Yes it gives more security, especially the 2of2/2of3/3of3 because with any 1ofX any given key being compromised allows for bad firmware being signed

Could you please elaborate on the more security aspect here? It will help us better understand your requirement.

@greenaddress
Copy link
Author

@mahavirj thanks a lot

we understand the the first stage bootloader in rom only does the verification for 1 key with secure boot, but we would still like to enforce at least 2 signatures verified in the 2nd stage bootloader and during app update/ota as a partial defense.

It appears we identified and implemented the changes for the app side of things (during app update/ota).

for the 2nd stage bootloader side of things we need to modify the function esp_secure_boot_verify_sbv2_signature_block in file bootloader_support/src/secure_boot_v2/secure_boot_signatures_bootloader.c such that it calls ets_secure_boot_verify_signature multiple times.

Given ets_secure_boot_verify_signature is in rom we don't have access to it and we don't know exactly how it behaves.

For example, does the function check if the index of the signature in the sig_block matches the index of the signature in trusted_key_digests, or just that there is one that satisfies the signature? or yet, does ets_secure_boot_verify_signature only check the first block signature in block_sig only? the intention is to call the function up to 3 times and blind the other trusted_key_digests by setting them to 0.

Would you be willing to provide some pseudo code for what the function ets_secure_boot_verify_signature does internally?

the app update based on mbedtls seem to not check order of the signatures but we cant tell about the rom, if the first time secure boot v2 was enabled one used 3 signatures, in any occasion in which the second or third key is used does it have to be in the right signature block or order doesn't matter?

@mahavirj
Copy link
Member

@greenaddress

Given ets_secure_boot_verify_signature is in rom we don't have access to it and we don't know exactly how it behaves.

This function verifies that at-least 1 signature block to satisfy the verification requirement against the trusted_keys digest. There is no specific ordering requirement. For your case, please try with:

  • first iteration - keep the applicable signature block at index 0 and dummy ones at index 1/2
  • second iteration - replace the signature block with next one at index 0

@greenaddress
Copy link
Author

greenaddress commented Sep 23, 2024

Thanks @mahavirj , our initial plan was to only pass 1 trusted_key (at its original index i) at the time and blind the other ones by setting them to 0.

Say we take the code from esp-idf v5.3.1 component bootloader_support:
File
https://github.com/espressif/esp-idf/blob/v5.3.1/components/bootloader_support/src/secure_boot_v2/secure_boot_signatures_bootloader.c

and we go and modify function esp_secure_boot_verify_sbv2_signature_block

esp_err_t esp_secure_boot_verify_sbv2_signature_block(const ets_secure_boot_signature_t *sig_block, const uint8_t *image_digest, uint8_t *verified_digest)
{
    esp_image_sig_public_key_digests_t trusted = {0};
    bool efuse_keys_are_not_set = false;
    if (get_secure_boot_key_digests(&trusted) != ESP_OK) {
        if (esp_secure_boot_enabled()) {
            ESP_LOGE(TAG, "Could not read eFuse secure boot digests!");
            return ESP_FAIL;
        } else {
            ESP_LOGI(TAG, "Secure boot V2 is not enabled yet and eFuse digest keys are not set");
            efuse_keys_are_not_set = true;
            ESP_FAULT_ASSERT(!esp_secure_boot_enabled());
        }
    }

    if (!esp_secure_boot_enabled()) {
        // It is the first boot. eFuse secure boot bit is not set yet. eFuse block(s) can be written or not.
        // Generating the SHA of the public key components in the signature block
        for (unsigned i = 0; i < SECURE_BOOT_NUM_BLOCKS; i++) {
            if (validate_signature_block(&sig_block->block[i]) == ESP_OK) {
                if (efuse_keys_are_not_set) {
                    // if efuse key digests are not in eFuse yet due to it is the first boot
                    // then use digests from app to skip error in ets_secure_boot_verify_signature().
                    bootloader_sha256_handle_t sig_block_sha = bootloader_sha256_start();
#if CONFIG_SECURE_SIGNED_APPS_RSA_SCHEME
                    bootloader_sha256_data(sig_block_sha, &sig_block->block[i].key, sizeof(sig_block->block[i].key));
#elif CONFIG_SECURE_SIGNED_APPS_ECDSA_V2_SCHEME
                    bootloader_sha256_data(sig_block_sha, &sig_block->block[i].ecdsa.key, sizeof(sig_block->block[i].ecdsa.key));
#endif
                    bootloader_sha256_finish(sig_block_sha, trusted.key_digests[i]);
                }
            }
        }
        ESP_FAULT_ASSERT(!esp_secure_boot_enabled());
    }

#if CONFIG_SECURE_BOOT_V2_RSA_2_OF_3                                            
    if (memcmp(&sig_block->block[0].key, &sig_block->block[1].key, sizeof(sig_block->block[0].key)) == 0 ||
        memcmp(&sig_block->block[1].key, &sig_block->block[2].key, sizeof(sig_block->block[0].key)) == 0 ||
        memcmp(&sig_block->block[2].key, &sig_block->block[0].key, sizeof(sig_block->block[0].key)) == 0) {
        return ESP_ERR_IMAGE_INVALID;                                           
    }                                                                           
#endif    

#if CONFIG_SECURE_SIGNED_APPS_RSA_SCHEME
    ESP_LOGI(TAG, "Verifying with RSA-PSS...");
#else
    ESP_LOGI(TAG, "Verifying with ECDSA...");
#endif

#if CONFIG_IDF_TARGET_ESP32
    int sb_result = ets_secure_boot_verify_signature(sig_block, image_digest, trusted.key_digests[0], verified_digest);
#else
#if CONFIG_SECURE_BOOT_V2_RSA_2_OF_3                                            
    size_t validated_keys = 0;                                                  
    int sb_result = SB_FAILED;                                                  
    for (unsigned i = 0; i < SECURE_BOOT_NUM_BLOCKS; i++) {                     
        ets_secure_boot_key_digests_t blinded_trusted_key_digests = {0};        
        blinded_trusted_key_digests.key_digests[i] = &trusted.key_digests[i];
                                                                                
        // Key revocation happens in ROM bootloader.                            
        // Do NOT allow key revocation while verifying application              
        blinded_trusted_key_digests.allow_key_revoke = false;                   
                                                                                
        int sb_sub_result = ets_secure_boot_verify_signature(sig_block, image_digest, &blinded_trusted_key_digests, verified_digest);
        if (sb_sub_result == SB_SUCCESS) {                                      
            validated_keys++;                                                   
            if (validated_keys >= 2) {                                          
                sb_result = sb_sub_result;                                      
                break;                                                          
            }                                                                   
        }                                                                       
    }                                                                           
#else 
    ets_secure_boot_key_digests_t trusted_key_digests = {0};
    for (unsigned i = 0; i < SECURE_BOOT_NUM_BLOCKS; i++) {
        trusted_key_digests.key_digests[i] = &trusted.key_digests[i];
    }

    // Key revocation happens in ROM bootloader.
    // Do NOT allow key revocation while verifying application
    trusted_key_digests.allow_key_revoke = false;

    int sb_result = ets_secure_boot_verify_signature(sig_block, image_digest, &trusted_key_digests, verified_digest);
#endif
#endif // CONFIG_IDF_TARGET_ESP32

    if (sb_result != SB_SUCCESS) {
        ESP_LOGE(TAG, "Secure Boot V2 verification failed.");
        return ESP_ERR_IMAGE_INVALID;
    } else {
        ESP_LOGI(TAG, "Signature verified successfully!");
        return ESP_OK;
    }
}

The code is intended to now enforce that the public keys in the app image are all different and then checks that at least 2 signatures in the app image match one of the public key digest in trusted_key

The key line IMHO is

        blinded_trusted_key_digests.key_digests[i] = &trusted.key_digests[i];

which could also have been (noticed we are setting index 0 rather than i)

        blinded_trusted_key_digests.key_digests[0] = &trusted.key_digests[i];

But you seem to suggest we leave the trusted_key untouched and we iterate the sigblock?
The sigblock is bootloader memory mapped, so likely we would need to make a copy in order to map accordingly the first index and put dummies/00/FF in the other ones

thanks for the guidance

@greenaddress
Copy link
Author

@mahavirj so instead of blinding the trusted if we blind the untrusted app image sig blocks we get something like this which should be more efficient:

    size_t validated_keys = 0;                                                  
    int sb_result = SB_FAILED;                                                  
                                                                                
    ets_secure_boot_sig_block_t sig_block_copy[3] = {0};                        
    for (unsigned i = 0; i < SECURE_BOOT_NUM_BLOCKS; i++) {                     
        memcpy(&sig_block_copy[0], &sig_block->block[i], sizeof(ets_secure_boot_sig_block_t));
        int sb_sub_result = ets_secure_boot_verify_signature((ets_secure_boot_signature_t*)&sig_block_copy[0], image_digest, &trusted_key_digests, verified_digest);
        if (sb_sub_result == SB_SUCCESS) {                                      
            validated_keys++;                                                   
            if (validated_keys >= 2) {        
                sb_result = sb_sub_result;                                      
                break;                                                          
            }                                                                   
        }                                                                       
    }   

is this what you meant?

@mahavirj
Copy link
Member

@greenaddress Yes, something along these lines. You may also add an additional check to see that signature is also different between 2 blocks (in addition to public keys).

@greenaddress
Copy link
Author

greenaddress commented Sep 24, 2024

Thanks @mahavirj

I don't understand why we need t check the signatures are different when we check the public keys are different - the function ets_secure_boot_verify_signature will then make sure the chain of checks signature -> public key/digest should match and prevent any abuse?

Also, is there a way to override bootloader_support for both application and bootloader without having to include it twice?

At the moment to override the bootloader verification one needs to include in the repo root directory the following

bootloader_components/bootloader_support/ 
bootloader_components/main/ 

but to override the application/ota verification is also necessary to keep a copy of bootloader_support inside components in the repo root directory.

So you end up with:

bootloader_components/bootloader_support/ 
bootloader_components/main/ 
main/main.c
components/bootloader_support

with idf v5.3.1. With far older versions it used to be enough to override bootloader in components but that no longer seems the case.

thanks

@greenaddress
Copy link
Author

greenaddress commented Sep 24, 2024

looks like I found a workaround to not have the bootloader_support twice:
in idf_component.yml

+  bootloader_support:
+      path: ../bootloader_components/bootloader_support
+      rules:
+      - if: "target in [esp32s3]"

actually the above didn't work! because the dependencies lock file then contained the full path rather than relative path which would obviously break the lock when the files are missing. I think that's a bug, when does it make sense to have full absolute paths for dependencies lock when they are relative in the idf_components.yml?

this instead worked without breaking the deps locks:

+set(EXTRA_COMPONENT_DIRS bootloader_components/bootloader_support)

@mahavirj
Copy link
Member

I don't understand why we need t check the signatures are different when we check the public keys are different - the function ets_secure_boot_verify_signature will then make sure the chain of checks signature -> public key/digest should match and prevent any abuse?

Yes, probably not required. Approach that I had in mind was to check the signature against an individual digest block and then for second iteration skip the digest the succeeded first time. This way we don't need any outer checks on public key being different etc. I just created a reference for this approach here. But, the approach you highlighted above should also work. Recommend doing a more thorough testing and also considering any additional anti-FI checks in the code here.

Interim, as I had already mentioned, I don't see any particular advantage of this approach. Hence, closing this issue from ESP-IDF side. But if you still see any value addition from security perspective then please drop a comment here.

I think that's a bug, when does it make sense to have full absolute paths for dependencies lock when they are relative in the idf_components.yml?

This probably deserves a separate discussion. I would request that you open up a tracker on idf-component-manager project.

Thanks.

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new labels Sep 26, 2024
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: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

4 participants