[nrf noup] bootutil: loader: fix a hardfault when the external second…

.github/workflows/backport.yml

@@ -0,0 +1,19 @@
+name: Backport
+on:
+  pull_request:
+    types:
+      - closed
+      - labeled
+
+jobs:
+  backport:
+    runs-on: ubuntu-18.04
+    name: Backport
+    steps:
+      - name: Backport Bot
+        uses: Gaurav0/[email protected]
+        with:
+          bot_username: NordicBuilder
+          bot_token: 151a9b45052f9ee8be5a59963d31ad7b92c3ecb5
+          bot_token_key: 67bb1f1f998d546859786a4088917c65415c0ebd
+          github_token: ${{ secrets.GITHUB_TOKEN }}

.gitlint

@@ -0,0 +1,57 @@
+# All these sections are optional, edit this file as you like.
+[general]
+ignore=title-trailing-punctuation, T3, title-max-length, T1, body-hard-tab, B3, B1
+# verbosity should be a value between 1 and 3, the commandline -v flags take precedence over this
+verbosity = 3
+# By default gitlint will ignore merge commits. Set to 'false' to disable.
+ignore-merge-commits=true
+# Enable debug mode (prints more output). Disabled by default
+debug = false
+
+# Set the extra-path where gitlint will search for user defined rules
+# See http://jorisroovers.github.io/gitlint/user_defined_rules for details
+extra-path=../../zephyr/scripts/gitlint
+
+[title-max-length-no-revert]
+line-length=72
+
+[body-min-line-count]
+min-line-count=1
+
+[body-max-line-count]
+max-line-count=200
+
+[title-starts-with-subsystem]
+regex = ^(?!subsys:)(([^:]+):)(\s([^:]+):)*\s(.+)$
+
+[title-must-not-contain-word]
+# Comma-separated list of words that should not occur in the title. Matching is case
+# insensitive. It's fine if the keyword occurs as part of a larger word (so "WIPING"
+# will not cause a violation, but "WIP: my title" will.
+words=wip
+
+[title-match-regex]
+# python like regex (https://docs.python.org/2/library/re.html) that the
+# commit-msg title must be matched to.
+# Note that the regex can contradict with other rules if not used correctly
+# (e.g. title-must-not-contain-word).
+#regex=^US[0-9]*
+
+[max-line-length-with-exceptions]
+# B1 = body-max-line-length
+line-length=72
+
+[body-min-length]
+min-length=3
+
+[body-is-missing]
+# Whether to ignore this rule on merge commits (which typically only have a title)
+# default = True
+ignore-merge-commits=false
+
+[body-changed-file-mention]
+# List of files that need to be explicitly mentioned in the body when they are changed
+# This is useful for when developers often erroneously edit certain files or git submodules.
+# By specifying this rule, developers can only change the file when they explicitly reference
+# it in the commit message.
+#files=gitlint/rules.py,README.md

Jenkinsfile

@@ -0,0 +1,6 @@
+@Library("CI_LIB") _
+
+def pipeline = new ncs.sdk_mcuboot.Main()
+
+pipeline.run(JOB_NAME)
+

boot/bootutil/include/bootutil/crypto/ecdsa_p256.h

@@ -14,6 +14,7 @@
 
 #if (defined(MCUBOOT_USE_TINYCRYPT) + \
      defined(MCUBOOT_USE_CC310) + \
+     defined(MCUBOOT_USE_NRF_EXTERNAL_CRYPTO) + \
      defined(MCUBOOT_USE_MBED_TLS)) != 1
     #error "One crypto backend must be defined: either CC310, TINYCRYPT, or MBED_TLS"
 #endif
@@ -35,6 +36,11 @@
     #define BOOTUTIL_CRYPTO_ECDSA_P256_HASH_SIZE (4 * 8)
 #endif
 
+#if defined(MCUBOOT_USE_NRF_EXTERNAL_CRYPTO)
+    #include <bl_crypto.h>
+    #define BOOTUTIL_CRYPTO_ECDSA_P256_HASH_SIZE (4 * 8)
+#endif /* MCUBOOT_USE_NRF_EXTERNAL_CRYPTO */
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -158,6 +164,43 @@ static inline int bootutil_ecdsa_p256_verify(bootutil_ecdsa_p256_context *ctx,
 }
 #endif /* MCUBOOT_USE_MBED_TLS */
 
+#if defined(MCUBOOT_USE_NRF_EXTERNAL_CRYPTO)
+typedef uintptr_t bootutil_ecdsa_p256_context;
+
+static inline void bootutil_ecdsa_p256_init(bootutil_ecdsa_p256_context *ctx)
+{
+    (void)ctx;
+}
+
+static inline void bootutil_ecdsa_p256_drop(bootutil_ecdsa_p256_context *ctx)
+{
+    (void)ctx;
+}
+
+static inline int bootutil_ecdsa_p256_verify(bootutil_ecdsa_p256_context *ctx,
+                                             uint8_t *pk, size_t pk_len,
+                                             uint8_t *hash,
+                                             uint8_t *sig, size_t sig_len)
+{
+    (void)ctx;
+    (void)pk_len;
+    (void)sig_len;
+
+	/* As described on the compact representation in IETF protocols,
+	 * the first byte of the key defines if the ECC points are
+	 * compressed (0x2 or 0x3) or uncompressed (0x4).
+	 * We only support uncompressed keys.
+	 */
+	if (pk[0] != 0x04)
+		return -1;
+
+	pk++;
+
+    return bl_secp256r1_validate(hash, BOOTUTIL_CRYPTO_ECDSA_P256_HASH_SIZE,
+                                 pk, sig);
+}
+#endif /* MCUBOOT_USE_NRF_EXTERNAL_CRYPTO */
+
 #ifdef __cplusplus
 }
 #endif

boot/bootutil/include/bootutil/crypto/sha256.h

@@ -22,6 +22,7 @@
 
 #if (defined(MCUBOOT_USE_MBED_TLS) + \
      defined(MCUBOOT_USE_TINYCRYPT) + \
+     defined(MCUBOOT_USE_NRF_EXTERNAL_CRYPTO) + \
      defined(MCUBOOT_USE_CC310)) != 1
     #error "One crypto backend must be defined: either CC310, MBED_TLS or TINYCRYPT"
 #endif
@@ -139,6 +140,37 @@ static inline int bootutil_sha256_finish(bootutil_sha256_context *ctx,
 }
 #endif /* MCUBOOT_USE_CC310 */
 
+#if defined(MCUBOOT_USE_NRF_EXTERNAL_CRYPTO)
+
+#include <bl_crypto.h>
+
+typedef bl_sha256_ctx_t bootutil_sha256_context;
+
+static inline void bootutil_sha256_init(bootutil_sha256_context *ctx)
+{
+    bl_sha256_init(ctx);
+}
+
+static inline void bootutil_sha256_drop(bootutil_sha256_context *ctx)
+{
+    (void)ctx;
+}
+
+static inline int bootutil_sha256_update(bootutil_sha256_context *ctx,
+                                          const void *data,
+                                          uint32_t data_len)
+{
+    return bl_sha256_update(ctx, data, data_len);
+}
+
+static inline int bootutil_sha256_finish(bootutil_sha256_context *ctx,
+                                          uint8_t *output)
+{
+    bl_sha256_finalize(ctx, output);
+    return 0;
+}
+#endif /* MCUBOOT_USE_NRF_EXTERNAL_CRYPTO */
+
 #ifdef __cplusplus
 }
 #endif

boot/bootutil/src/image_ec256.c

@@ -34,8 +34,11 @@
 #if defined(MCUBOOT_USE_CC310) || defined(MCUBOOT_USE_MBED_TLS)
 #define NUM_ECC_BYTES (256 / 8)
 #endif
+#ifdef MCUBOOT_USE_NRF_EXTERNAL_CRYPTO
+#define NUM_ECC_BYTES (256 / 8)
+#endif
 #if defined(MCUBOOT_USE_TINYCRYPT) || defined(MCUBOOT_USE_CC310) || \
-    defined(MCUBOOT_USE_MBED_TLS)
+    defined(MCUBOOT_USE_MBED_TLS) || defined (MCUBOOT_USE_NRF_EXTERNAL_CRYPTO)
 #include "bootutil/sign_key.h"
 
 #include "mbedtls/oid.h"

boot/bootutil/src/loader.c

@@ -47,6 +47,10 @@
 #include "bootutil/ramload.h"
 #include "bootutil/boot_hooks.h"
 
+#if defined(CONFIG_SOC_NRF5340_CPUAPP) && defined(PM_CPUNET_B0N_ADDRESS)
+#include <dfu/pcd.h> 
+#endif
+
 #ifdef MCUBOOT_ENC_IMAGES
 #include "bootutil/enc_key.h"
 #endif
@@ -107,6 +111,15 @@ boot_read_image_headers(struct boot_loader_state *state, bool require_all,
              *
              * Failure to read any headers is a fatal error.
              */
+#ifdef PM_S1_ADDRESS
+            /* Patch needed for NCS. The primary slot of the second image
+             * (image 1) will not contain a valid image header until an upgrade
+             * of mcuboot has happened (filling S1 with the new version).
+             */
+            if (BOOT_CURR_IMG(state) == 1 && i == 0) {
+                continue;
+            }
+#endif /* PM_S1_ADDRESS */
             if (i > 0 && !require_all) {
                 return 0;
             } else {
@@ -798,7 +811,24 @@ boot_validate_slot(struct boot_loader_state *state, int slot,
             goto out;
         }
 
-        if (reset_value < pri_fa->fa_off || reset_value> (pri_fa->fa_off + pri_fa->fa_size)) {
+        uint32_t min_addr, max_addr;
+
+#ifdef PM_CPUNET_APP_ADDRESS
+        /* The primary slot for the network core is emulated in RAM.
+         * Its flash_area hasn't got relevant boundaries.
+         * Therfore need to override its boundaries for the check.
+         */
+        if (BOOT_CURR_IMG(state) == 1) {
+            min_addr = PM_CPUNET_APP_ADDRESS;
+            max_addr = PM_CPUNET_APP_ADDRESS + PM_CPUNET_APP_SIZE;
+        } else
+#endif
+        {
+            min_addr = pri_fa->fa_off;
+            max_addr = pri_fa->fa_off + pri_fa->fa_size;
+        }
+
+        if (reset_value < min_addr || reset_value> (max_addr)) {
             BOOT_LOG_ERR("Reset address of image in secondary slot is not in the primary slot");
             BOOT_LOG_ERR("Erasing image from secondary slot");
 
@@ -882,6 +912,51 @@ boot_validated_swap_type(struct boot_loader_state *state,
     int swap_type;
     fih_int fih_rc = FIH_FAILURE;
 
+#if defined(PM_S1_ADDRESS)
+    const struct flash_area *secondary_fa =
+        BOOT_IMG_AREA(state, BOOT_SECONDARY_SLOT);
+    struct image_header *hdr = (struct image_header *)secondary_fa->fa_off;
+    uint32_t vtable_addr = 0;
+    uint32_t *vtable = 0;
+    uint32_t reset_addr = 0;
+    /* Patch needed for NCS. Since image 0 (the app) and image 1 (the other
+     * B1 slot S0 or S1) share the same secondary slot, we need to check
+     * whether the update candidate in the secondary slot is intended for
+     * image 0 or image 1 primary by looking at the address of the reset
+     * vector. Note that there are good reasons for not using img_num from
+     * the swap info.
+     */
+
+    if (hdr->ih_magic == IMAGE_MAGIC) {
+        vtable_addr = (uint32_t)hdr + hdr->ih_hdr_size;
+        vtable = (uint32_t *)(vtable_addr);
+        reset_addr = vtable[1];
+#ifdef PM_S1_ADDRESS
+#ifdef PM_CPUNET_B0N_ADDRESS
+        if(reset_addr < PM_CPUNET_B0N_ADDRESS)
+#endif
+        {
+            const struct flash_area *primary_fa;
+            int rc = flash_area_open(flash_area_id_from_multi_image_slot(
+                        BOOT_CURR_IMG(state),
+                        BOOT_PRIMARY_SLOT),
+                    &primary_fa);
+
+            if (rc != 0) {
+                return BOOT_SWAP_TYPE_FAIL;
+            }
+            /* Get start and end of primary slot for current image */
+            if (reset_addr < primary_fa->fa_off ||
+                    reset_addr > (primary_fa->fa_off + primary_fa->fa_size)) {
+                /* The image in the secondary slot is not intended for this image
+                */
+                return BOOT_SWAP_TYPE_NONE;
+            }
+        }
+#endif /* PM_S1_ADDRESS */
+    }
+#endif /* PM_S1_ADDRESS */
+
     swap_type = boot_swap_type_multi(BOOT_CURR_IMG(state));
     if (BOOT_IS_UPGRADE(swap_type)) {
         /* Boot loader wants to switch to the secondary slot.
@@ -894,7 +969,7 @@ boot_validated_swap_type(struct boot_loader_state *state,
             } else {
                 swap_type = BOOT_SWAP_TYPE_FAIL;
             }
-        }
+        } 
     }
 
     return swap_type;
@@ -1485,7 +1560,7 @@ boot_verify_dependencies(struct boot_loader_state *state)
         if (rc == 0) {
             /* All dependencies've been satisfied, continue with next image. */
             BOOT_CURR_IMG(state)++;
-        } else {
+	} else if (rc == BOOT_EBADIMAGE) {
             /* Cannot upgrade due to non-met dependencies, so disable all
              * image upgrades.
              */
@@ -1494,7 +1569,10 @@ boot_verify_dependencies(struct boot_loader_state *state)
                 BOOT_SWAP_TYPE(state) = BOOT_SWAP_TYPE_NONE;
             }
             break;
-        }
+	} else {
+	    /* Other error happened, images are inconsistent */
+		return rc;
+	}
     }
     return rc;
 }
@@ -2093,10 +2171,23 @@ context_boot_go(struct boot_loader_state *state, struct boot_rsp *rsp)
         }
 
 #ifdef MCUBOOT_VALIDATE_PRIMARY_SLOT
-        FIH_CALL(boot_validate_slot, fih_rc, state, BOOT_PRIMARY_SLOT, NULL);
-        if (fih_not_eq(fih_rc, FIH_SUCCESS)) {
-            goto out;
-        }
+#ifdef PM_S1_ADDRESS
+	/* Patch needed for NCS. If secure boot is enabled, then mcuboot
+	 * will be stored in either partition S0 or S1. Image 1 primary
+	 * will point to the 'other' Sx partition. Hence, image 1 primary
+	 * does not contain a valid image until mcuboot has been upgraded.
+	 * Note that B0 will perform validation of the active mcuboot image,
+	 * so there is no security lost by skipping this check for image 1
+	 * primary.
+	 */
+	if (BOOT_CURR_IMG(state) == 0)
+#endif
+	{
+            FIH_CALL(boot_validate_slot, fih_rc, state, BOOT_PRIMARY_SLOT, NULL);
+            if (fih_not_eq(fih_rc, FIH_SUCCESS)) {
+                goto out;
+            }
+	}
 #else
         /* Even if we're not re-validating the primary slot, we could be booting
          * onto an empty flash chip. At least do a basic sanity check that

boot/bootutil/src/swap_move.c

@@ -211,6 +211,18 @@ boot_status_internal_off(const struct boot_status *bs, int elem_sz)
 int
 boot_slots_compatible(struct boot_loader_state *state)
 {
+#ifdef PM_S1_ADDRESS
+    /* Patch needed for NCS. In this case, image 1 primary points to the other
+     * B1 slot (ie S0 or S1), and image 0 primary points to the app.
+     * With this configuration, image 0 and image 1 share the secondary slot.
+     * Hence, the primary slot of image 1 will be *smaller* than image 1's
+     * secondary slot. This is not allowed in upstream mcuboot, so we need
+     * this patch to allow it. Also, all of these checks are redundant when
+     * partition manager is in use, and since we have the same sector size
+     * in all of our flash.
+     */
+        return 1;
+#else
     size_t num_sectors_pri;
     size_t num_sectors_sec;
     size_t sector_sz_pri = 0;
@@ -247,6 +259,7 @@ boot_slots_compatible(struct boot_loader_state *state)
     }
 
     return 1;
+#endif /* PM_S1_ADDRESS */
 }
 
 #define BOOT_LOG_SWAP_STATE(area, state)                            \