Boot: Introduce rollback protection

- Add image security counter verification (read image security counter
  from the manifest and compare it against the stored security counter)
  as a mandatory part of the image validation process.
- Store the newest value of security counter in a non-volatile (NV)
  counter.
- Add security counter interface to MCUBoot.

Change-Id: I608508e707d01c3777788bc754810407fae610e2
Signed-off-by: David Vincze <david.vincze@arm.com>
diff --git a/bl2/ext/mcuboot/CMakeLists.txt b/bl2/ext/mcuboot/CMakeLists.txt
index 71bf456..06e0d5b 100644
--- a/bl2/ext/mcuboot/CMakeLists.txt
+++ b/bl2/ext/mcuboot/CMakeLists.txt
@@ -40,7 +40,7 @@
 set(BUILD_STARTUP On)
 set(BUILD_TARGET_CFG Off)
 set(BUILD_TARGET_HARDWARE_KEYS Off)
-set(BUILD_TARGET_NV_COUNTERS Off)
+set(BUILD_TARGET_NV_COUNTERS On)
 set(BUILD_CMSIS_DRIVERS On)
 set(BUILD_TIME Off)
 set(BUILD_UART_STDOUT On)
@@ -64,6 +64,7 @@
 		"${MCUBOOT_DIR}/bootutil/src/image_rsa.c"
 		"${MCUBOOT_DIR}/bootutil/src/caps.c"
 		"${TFM_ROOT_DIR}/bl2/src/boot_record.c"
+		"${TFM_ROOT_DIR}/bl2/src/security_cnt.c"
 	)
 
 #Define location of mbedtls source, build, and installation directory.
diff --git a/bl2/ext/mcuboot/bl2_main.c b/bl2/ext/mcuboot/bl2_main.c
index b63cc78..a7bf3dc 100644
--- a/bl2/ext/mcuboot/bl2_main.c
+++ b/bl2/ext/mcuboot/bl2_main.c
@@ -28,6 +28,7 @@
 #include "bootutil/bootutil.h"
 #include "flash_map/flash_map.h"
 #include "bl2/include/boot_record.h"
+#include "security_cnt.h"
 
 /* Avoids the semihosting issue */
 #if defined (__ARMCC_VERSION) && (__ARMCC_VERSION >= 6010050)
@@ -130,6 +131,13 @@
             ;
     }
 
+    rc = boot_nv_security_counter_init();
+    if (rc != 0) {
+        BOOT_LOG_ERR("Error while initializing the security counter");
+        while (1)
+            ;
+    }
+
     rc = boot_go(&rsp);
     if (rc != 0) {
         BOOT_LOG_ERR("Unable to find bootable image");
diff --git a/bl2/ext/mcuboot/bootutil/include/bootutil/image.h b/bl2/ext/mcuboot/bootutil/include/bootutil/image.h
index 871f3fb..643997a 100644
--- a/bl2/ext/mcuboot/bootutil/include/bootutil/image.h
+++ b/bl2/ext/mcuboot/bootutil/include/bootutil/image.h
@@ -114,6 +114,10 @@
                           uint8_t *tmp_buf, uint32_t tmp_buf_sz,
                           uint8_t *seed, int seed_len, uint8_t *out_hash);
 
+int32_t bootutil_get_img_security_cnt(struct image_header *hdr,
+                                      const struct flash_area *fap,
+                                      uint32_t *security_cnt);
+
 #ifdef MCUBOOT_RAM_LOADING
 int bootutil_check_hash_after_loading(struct image_header *hdr);
 #endif
diff --git a/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h b/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
index 918d2d1..1eb5a4a 100644
--- a/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
+++ b/bl2/ext/mcuboot/bootutil/src/bootutil_priv.h
@@ -44,6 +44,7 @@
 #define BOOT_EBADSTATUS 5
 #define BOOT_ENOMEM     6
 #define BOOT_EBADARGS   7
+#define BOOT_EBADMAGIC  8
 
 #define BOOT_TMPBUF_SZ  256
 
diff --git a/bl2/ext/mcuboot/bootutil/src/image_validate.c b/bl2/ext/mcuboot/bootutil/src/image_validate.c
index 68f1c40..b9db6de 100644
--- a/bl2/ext/mcuboot/bootutil/src/image_validate.c
+++ b/bl2/ext/mcuboot/bootutil/src/image_validate.c
@@ -33,6 +33,7 @@
 #include "bootutil/image.h"
 #include "bootutil/sha256.h"
 #include "bootutil/sign_key.h"
+#include "security_cnt.h"
 
 #ifdef MCUBOOT_SIGN_RSA
 #include "mbedtls/rsa.h"
@@ -164,7 +165,7 @@
     info = *((struct image_tlv_info *)(load_address + off));
 
     if (info.it_magic != IMAGE_TLV_INFO_MAGIC) {
-        return -1;
+        return BOOT_EBADMAGIC;
     }
     end = off + info.it_tlv_tot;
     off += sizeof(info);
@@ -173,7 +174,7 @@
      * Traverse through all of the TLVs, performing any checks we know
      * and are able to do.
      */
-    for (; off < end; off += sizeof(tlv) + tlv.it_len) {
+    while (off < end) {
         tlv = *((struct image_tlv *)(load_address + off));
         tlv_sz = sizeof(tlv);
 
@@ -192,6 +193,14 @@
 
             sha256_valid = 1;
         }
+
+        /* Avoid integer overflow. */
+        if ((UINT32_MAX - off) < (sizeof(tlv) + tlv.it_len)) {
+            /* Potential overflow. */
+            break;
+        } else {
+            off += sizeof(tlv) + tlv.it_len;
+        }
     }
 
     if (!sha256_valid) {
@@ -202,6 +211,97 @@
 }
 #endif /* MCUBOOT_RAM_LOADING */
 
+/**
+ * Reads the value of an image's security counter.
+ *
+ * @param hdr           Pointer to the image header structure.
+ * @param fap           Pointer to a description structure of the image's
+ *                      flash area.
+ * @param security_cnt  Pointer to store the security counter value.
+ *
+ * @return              0 on success; nonzero on failure.
+ */
+int32_t
+bootutil_get_img_security_cnt(struct image_header *hdr,
+                              const struct flash_area *fap,
+                              uint32_t *img_security_cnt)
+{
+    struct image_tlv_info info;
+    struct image_tlv tlv;
+    uint32_t off;
+    uint32_t end;
+    uint32_t found = 0;
+    int32_t rc;
+
+    if ((hdr == NULL) ||
+        (fap == NULL) ||
+        (img_security_cnt == NULL)) {
+        /* Invalid parameter. */
+        return BOOT_EBADARGS;
+    }
+
+    /* The TLVs come after the image. */
+    off = hdr->ih_hdr_size + hdr->ih_img_size;
+
+    /* The TLV area always starts with an image_tlv_info structure. */
+    rc = flash_area_read(fap, off, &info, sizeof(info));
+    if (rc != 0) {
+        return BOOT_EFLASH;
+    }
+
+    if (info.it_magic != IMAGE_TLV_INFO_MAGIC) {
+        return BOOT_EBADMAGIC;
+    }
+
+    /* The security counter TLV is in the protected part of the TLV area. */
+    if (hdr->ih_protect_tlv_size != 0) {
+        end = off + (uint32_t)hdr->ih_protect_tlv_size;
+        off += sizeof(info);
+
+        /* Traverse through the protected TLV area to find the
+         * security counter TLV.
+         */
+        while (off < end) {
+            rc = flash_area_read(fap, off, &tlv, sizeof(tlv));
+            if (rc != 0) {
+                return BOOT_EFLASH;
+            }
+
+            if (tlv.it_type == IMAGE_TLV_SEC_CNT) {
+
+                if (tlv.it_len != sizeof(*img_security_cnt)) {
+                    /* Security counter is not valid. */
+                    break;
+                }
+
+                rc = flash_area_read(fap, off + sizeof(tlv),
+                                     img_security_cnt, tlv.it_len);
+                if (rc != 0) {
+                    return BOOT_EFLASH;
+                }
+
+                /* Security counter has been found. */
+                found = 1;
+                break;
+            }
+
+            /* Avoid integer overflow. */
+            if ((UINT32_MAX - off) < (sizeof(tlv) + tlv.it_len)) {
+                /* Potential overflow. */
+                break;
+            } else {
+                off += sizeof(tlv) + tlv.it_len;
+            }
+        }
+    }
+
+    if (found) {
+        return 0;
+    }
+
+    return -1;
+}
+
 /*
  * Verify the integrity of the image.
  * Return non-zero if image could not be validated/does not validate.
@@ -222,6 +322,9 @@
     struct image_tlv tlv;
     uint8_t buf[256];
     uint8_t hash[32] = {0};
+    uint32_t security_cnt;
+    uint32_t img_security_cnt;
+    int32_t security_counter_valid = 0;
     int rc;
 
     rc = bootutil_img_hash(hdr, fap, tmp_buf, tmp_buf_sz, hash,
@@ -242,7 +345,7 @@
         return rc;
     }
     if (info.it_magic != IMAGE_TLV_INFO_MAGIC) {
-        return -1;
+        return BOOT_EBADMAGIC;
     }
     end = off + info.it_tlv_tot;
     off += sizeof(info);
@@ -251,7 +354,7 @@
      * Traverse through all of the TLVs, performing any checks we know
      * and are able to do.
      */
-    for (; off < end; off += sizeof(tlv) + tlv.it_len) {
+    while (off < end) {
         rc = flash_area_read(fap, off, &tlv, sizeof(tlv));
         if (rc) {
             return rc;
@@ -311,10 +414,49 @@
             }
             key_id = -1;
 #endif
+        } else if (tlv.it_type == IMAGE_TLV_SEC_CNT) {
+            /*
+             * Verify the image's security counter.
+             * This must always be present.
+             */
+            if (tlv.it_len != sizeof(img_security_cnt)) {
+                /* Security counter is not valid. */
+                return -1;
+            }
+
+            rc = flash_area_read(fap, off + sizeof(tlv),
+                                 &img_security_cnt, tlv.it_len);
+            if (rc) {
+                return rc;
+            }
+
+            rc = boot_nv_security_counter_get(0, &security_cnt);
+            if (rc) {
+                return rc;
+            }
+
+            /* Compare the new image's security counter value against the
+             * stored security counter value.
+             */
+            if (img_security_cnt < security_cnt) {
+                /* The image's security counter is not accepted. */
+                return -1;
+            }
+
+            /* The image's security counter has been successfully verified. */
+            security_counter_valid = 1;
+        }
+
+        /* Avoid integer overflow. */
+        if ((UINT32_MAX - off) < (sizeof(tlv) + tlv.it_len)) {
+            /* Potential overflow. */
+            break;
+        } else {
+            off += sizeof(tlv) + tlv.it_len;
         }
     }
 
-    if (!sha256_valid) {
+    if (!sha256_valid || !security_counter_valid) {
         return -1;
     }
 
diff --git a/bl2/ext/mcuboot/bootutil/src/loader.c b/bl2/ext/mcuboot/bootutil/src/loader.c
index 42459c3..0a40342 100644
--- a/bl2/ext/mcuboot/bootutil/src/loader.c
+++ b/bl2/ext/mcuboot/bootutil/src/loader.c
@@ -41,6 +41,7 @@
 #include "bootutil_priv.h"
 #include "bl2/include/tfm_boot_status.h"
 #include "bl2/include/boot_record.h"
+#include "security_cnt.h"
 
 #define BOOT_LOG_LEVEL BOOT_LOG_LEVEL_INFO
 #include "bootutil/bootutil_log.h"
@@ -260,8 +261,8 @@
     return 0;
 }
 
-/*
- * Validate image hash/signature in a slot.
+/**
+ * Validate image hash/signature and security counter in a slot.
  */
 static int
 boot_image_check(struct image_header *hdr, const struct flash_area *fap)
@@ -320,6 +321,8 @@
         }
         BOOT_LOG_ERR("Authentication failed! Image in slot %d is not valid.",
                      slot);
+
+        flash_area_close(fap);
         return -1;
     }
 
@@ -329,6 +332,44 @@
     return 0;
 }
 
+/**
+ * Updates the stored security counter value with the image's security counter
+ * value which resides in the given slot if it's greater than the stored value.
+ *
+ * @param slot      Slot number of the image.
+ * @param hdr       Pointer to the image header structure of the image that is
+ *                  currently stored in the given slot.
+ *
+ * @return          0 on success; nonzero on failure.
+ */
+static int
+boot_update_security_counter(int slot, struct image_header *hdr)
+{
+    const struct flash_area *fap = NULL;
+    uint32_t img_security_cnt;
+    int rc;
+
+    rc = flash_area_open(flash_area_id_from_image_slot(slot), &fap);
+    if (rc != 0) {
+        rc = BOOT_EFLASH;
+        goto done;
+    }
+
+    rc = bootutil_get_img_security_cnt(hdr, fap, &img_security_cnt);
+    if (rc != 0) {
+        goto done;
+    }
+
+    rc = boot_nv_security_counter_update(0, img_security_cnt);
+    if (rc != 0) {
+        goto done;
+    }
+
+done:
+    flash_area_close(fap);
+    return rc;
+}
+
 #if !defined(MCUBOOT_NO_SWAP) && !defined(MCUBOOT_OVERWRITE_ONLY)
 /*
  * Compute the total size of the given image.  Includes the size of
@@ -1094,6 +1135,17 @@
     rc = boot_copy_sector(FLASH_AREA_IMAGE_1, FLASH_AREA_IMAGE_0,
                           0, 0, size);
 
+    /* Update the stored security counter with the new image's security counter
+     * value. Both slots hold the new image at this point, but slot 1's image
+     * header must be passed because the read image headers in the boot_data
+     * structure have not been updated yet.
+     */
+    rc = boot_update_security_counter(0, boot_img_hdr(&boot_data, 1));
+    if (rc != 0) {
+        BOOT_LOG_ERR("Security counter update failed after image upgrade.");
+        return rc;
+    }
+
     /*
      * Erases header and trailer. The trailer is erased because when a new
      * image is written without a trailer as is the case when using newt, the
@@ -1399,6 +1451,25 @@
         slot = 1;
         reload_headers = true;
 #ifndef MCUBOOT_OVERWRITE_ONLY
+        if (swap_type == BOOT_SWAP_TYPE_PERM) {
+            /* Update the stored security counter with the new image's security
+             * counter value (the one in the primary slot). Slot 0 holds the
+             * new image at this point, but slot 1's image header must be
+             * passed because the read image headers in the boot_data structure
+             * have not been updated yet.
+             *
+             * In case of a permanent image swap mcuboot will never attempt to
+             * revert the images on the next reboot. Therefore, the security
+             * counter must be increased right after the image upgrade.
+             */
+            rc = boot_update_security_counter(0, boot_img_hdr(&boot_data, 1));
+            if (rc != 0) {
+                BOOT_LOG_ERR("Security counter update failed after "
+                             "image upgrade.");
+                goto out;
+            }
+        }
+
         rc = boot_set_copy_done();
         if (rc != 0) {
             swap_type = BOOT_SWAP_TYPE_PANIC;
@@ -1459,6 +1530,24 @@
     }
 #endif /* MCUBOOT_VALIDATE_SLOT0 */
 
+    /* Update the stored security counter with the active image's security
+     * counter value. It will be updated only if the new security counter is
+     * greater than the stored value.
+     *
+     * In case of a successful image swapping when the swap type is TEST the
+     * security counter can be increased only after a reset, when the swap type
+     * is NONE and the image has marked itself "OK" (the image_ok flag has been
+     * set). This way a "revert" swap can be performed if it's necessary.
+     */
+    if (swap_type == BOOT_SWAP_TYPE_NONE) {
+        rc = boot_update_security_counter(0, boot_img_hdr(&boot_data, 0));
+        if (rc != 0) {
+            BOOT_LOG_ERR("Security counter update failed after image "
+                         "validation.");
+            goto out;
+        }
+    }
+
     /* Always boot from the primary slot. */
     rsp->br_flash_dev_id = boot_img_fa_device_id(&boot_data, 0);
     rsp->br_image_off = boot_img_slot_off(&boot_data, 0);
@@ -1746,6 +1835,16 @@
         /* The slot variable now refers to the newest image's slot in flash */
         newest_image_header = boot_img_hdr(&boot_data, slot);
 
+        /* Update the security counter with the newest image's security
+         * counter value.
+         */
+        rc = boot_update_security_counter(slot, newest_image_header);
+        if (rc != 0) {
+            BOOT_LOG_ERR("Security counter update failed after image "
+                         "validation.");
+            goto out;
+        }
+
         #ifdef MCUBOOT_RAM_LOADING
         if (newest_image_header->ih_flags & IMAGE_F_RAM_LOAD) {
             /* Copy image to the load address from where it
@@ -1790,6 +1889,7 @@
     } else {
         /* No candidate image available */
         rc = BOOT_EBADIMAGE;
+        goto out;
     }
 
     /* Save boot status to shared memory area */
diff --git a/bl2/ext/mcuboot/include/security_cnt.h b/bl2/ext/mcuboot/include/security_cnt.h
new file mode 100644
index 0000000..7c17a94
--- /dev/null
+++ b/bl2/ext/mcuboot/include/security_cnt.h
@@ -0,0 +1,64 @@
+/*
+ *  Copyright (c) 2019, Arm Limited. All rights reserved.
+ *
+ *  SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef __SECURITY_CNT_H__
+#define __SECURITY_CNT_H__
+
+/**
+ * @file security_cnt.h
+ *
+ * @note The interface must be implemented in a fail-safe way that is
+ *       resistant to asynchronous power failures or it can use hardware
+ *       counters that have this capability, if supported by the platform.
+ *       When a counter incrementation was interrupted it must be able to
+ *       continue the incrementation process or recover the previous consistent
+ *       status of the counters. If the counters have reached a stable status
+ *       (every counter incrementation operation has finished), from that point
+ *       their value cannot decrease due to any kind of power failure.
+ */
+
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Initialises the security counters.
+ *
+ * @return                  0 on success; nonzero on failure.
+ */
+int32_t boot_nv_security_counter_init(void);
+
+/**
+ * Reads the stored value of a given image's security counter.
+ *
+ * @param image_id          Index of the image (from 0).
+ * @param security_cnt      Pointer to store the security counter value.
+ *
+ * @return                  0 on success; nonzero on failure.
+ */
+int32_t boot_nv_security_counter_get(uint32_t image_id, uint32_t *security_cnt);
+
+/**
+ * Updates the stored value of a given image's security counter with a new
+ * security counter value if the new one is greater.
+ *
+ * @param image_id          Index of the image (from 0).
+ * @param img_security_cnt  New security counter value. The new value must be
+ *                          between 0 and UINT32_MAX and it must be greater than
+ *                          or equal to the current security counter value.
+ *
+ * @return                  0 on success; nonzero on failure.
+ */
+int32_t boot_nv_security_counter_update(uint32_t image_id,
+                                        uint32_t img_security_cnt);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __SECURITY_CNT_H__ */
diff --git a/bl2/src/security_cnt.c b/bl2/src/security_cnt.c
new file mode 100644
index 0000000..03c438c
--- /dev/null
+++ b/bl2/src/security_cnt.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2019, Arm Limited. All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ *
+ */
+
+#include "../ext/mcuboot/include/security_cnt.h"
+#include "../../platform/include/tfm_plat_nv_counters.h"
+#include "../../platform/include/tfm_plat_defs.h"
+#include <stdint.h>
+
+#define TFM_BOOT_NV_COUNTER_0    PLAT_NV_COUNTER_3   /* NV counter of Image 0 */
+#define TFM_BOOT_NV_COUNTER_MAX  PLAT_NV_COUNTER_MAX
+
+static enum tfm_nv_counter_t get_nv_counter_from_image_id(uint32_t image_id)
+{
+    uint32_t nv_counter;
+
+    /* Avoid integer overflow */
+    if ((UINT32_MAX - TFM_BOOT_NV_COUNTER_0) < image_id) {
+        return TFM_BOOT_NV_COUNTER_MAX;
+    }
+
+    nv_counter = TFM_BOOT_NV_COUNTER_0 + image_id;
+
+    /* Check the existence of the enumerated counter value */
+    if (nv_counter >= TFM_BOOT_NV_COUNTER_MAX) {
+        return TFM_BOOT_NV_COUNTER_MAX;
+    }
+
+    return (enum tfm_nv_counter_t)nv_counter;
+}
+
+int32_t boot_nv_security_counter_init(void)
+{
+    enum tfm_plat_err_t err;
+
+    err = tfm_plat_init_nv_counter();
+    if (err != TFM_PLAT_ERR_SUCCESS) {
+        return -1;
+    }
+
+    return 0;
+}
+
+int32_t boot_nv_security_counter_get(uint32_t image_id, uint32_t *security_cnt)
+{
+    enum tfm_nv_counter_t nv_counter;
+    enum tfm_plat_err_t err;
+
+    /* Check if it's a null-pointer. */
+    if (!security_cnt) {
+        return -1;
+    }
+
+    nv_counter = get_nv_counter_from_image_id(image_id);
+    if (nv_counter == TFM_BOOT_NV_COUNTER_MAX) {
+        return -1;
+    }
+
+    err = tfm_plat_read_nv_counter(nv_counter,
+                                   sizeof(*security_cnt),
+                                   (uint8_t *)security_cnt);
+    if (err != TFM_PLAT_ERR_SUCCESS) {
+        return -1;
+    }
+
+    return 0;
+}
+
+int32_t boot_nv_security_counter_update(uint32_t image_id,
+                                        uint32_t img_security_cnt)
+{
+    enum tfm_nv_counter_t nv_counter;
+    enum tfm_plat_err_t err;
+
+    nv_counter = get_nv_counter_from_image_id(image_id);
+    if (nv_counter == TFM_BOOT_NV_COUNTER_MAX) {
+        return -1;
+    }
+
+    err = tfm_plat_set_nv_counter(nv_counter, img_security_cnt);
+    if (err != TFM_PLAT_ERR_SUCCESS) {
+        return -1;
+    }
+
+    return 0;
+}
diff --git a/docs/user_guides/tfm_secure_boot.rst b/docs/user_guides/tfm_secure_boot.rst
index 2aa91d4..9097538 100644
--- a/docs/user_guides/tfm_secure_boot.rst
+++ b/docs/user_guides/tfm_secure_boot.rst
@@ -246,6 +246,29 @@
 versioning, please start a clean build without specifying the image version
 number at all.
 
+Security counter
+================
+Each signed image contains a security counter in its manifest. It is used by the
+bootloader and its aim is to have an independent (from the image version)
+counter to ensure rollback protection by comparing the new image's security
+counter against the original (currently active) image's security counter during
+the image upgrade process. It is added to the manifest (to the TLV area that is
+appended to the end of the image) by one of the python scripts when signing the
+image. The value of the security counter is security critical data and it is in
+the integrity protected part of the image. The last valid security counter is
+always stored in a non-volatile and trusted component of the device and its
+value should always be increased if a security flaw was fixed in the current
+image version. The value of the security counter can be specified at build time
+in the cmake configuration step::
+
+    cmake -G"Unix Makefiles" -DTARGET_PLATFORM=AN521 -DCOMPILER=ARMCLANG -DSECURITY_COUNTER=42 ../
+
+The security counter can be independent from the image version, but not
+necessarily. Alternatively, if it is not specified at build time with the
+``SECURITY_COUNTER`` option the python script will automatically generate it
+from the image version number (not including the build number) and this value
+will be added to the signed image.
+
 ************************
 Testing firmware upgrade
 ************************