On 01/09/2020 22.48, Thirupathaiah Annapureddy wrote:
> Anti rollback protection is required when there is a need to retire
> previous versions of FIT images due to security flaws in them.
> Currently U-Boot Verified boot does not have rollback protection to
> protect against known security flaws.
This is definitely something we've had on our todo-list/wishlist. But we
haven't had the time to sit down and work out the semantics and
implementation, so thanks for doing this.
I think most of this cover letter should be in the commit log instead.
> This RFC introduces a proposal to add anti-rollback protection for
> FIT images. This protection feature prevents U-Boot from accepting
> an image if it has ever successfully loaded an image with a larger
> anti-rollback version number.
>
> Each sub-image node inside /images node has an
> anti-rollback version number(arbvn) similar to rollback_index in
> Android Verified Boot. This version number is part of signed data and
> it is incremented as security flaws are discovered and fixed.
> U-Boot stores the last seen arbvn for a given image type in platform
> specific tamper-evident storage.
Hmm. What is the purpose of per-image-type version numbers? That seems
to be wrong. For example, we use the the "loadables" property in the
U-Boot FIT to get the SPL to load some firmware blob to a known memory
address where U-Boot proper then knows it to be. If we happened to have
two such blobs for different pieces of hardware, they'd have the same
"type" in the FIT image, but that doesn't mean they should follow the
same versioning.
The way I imagined rollback protection to work (but I really haven't
given this too much thought) was to have an extra property in the
configuration:
configurations {
default = "conf-1";
conf-1 {
description = "...";
kernel = "kernel-1";
fdt = "fdt-1";
ramdisk = "ramdisk-1";
arvbn = "arvbn"; /* <-- */
hash-1 {
algo = "sha1";
};
signature-foo {
algo = "sha1,rsa2048";
sign-images = "kernel", "fdt", "ramdisk",
"arvbn";
key-name-hint = "foo";
};
};
};
with arvbn simply being a small extra "image" node
arvbn {
description = "BSP version";
data = <0x02 0x03 0x01>;
}
(which should of course not be loaded to memory). This requires use of
signed configurations rather than individually signed images, but that's
anyway required for proper security.
The board callbacks would simply be given a pointer to the data part of
that node; that would make the versioning scheme rather flexible instead
of being limited to a single monotonically increasing u32 (hence also
the comparison logic should be in the board callbacks, instead of a
"get/set" interface). For example, one could have a version composed as
$major.$minor;$security, the board logic could prevent both downgrading
to another major version and another security version. I.e., suppose
we've had several BSP releases (in chronological order):
1.0;0
1.1;0
2.0;0
1.2;0
2.1;0
<major bug found and fixed>
1.3;1
2.2;1
2.3;1
1.4;1
<another one...>
2.4;2
1.5;2
With this, a user on 1.3 cannot update to 2.0; he must go at least to
2.2, or he would expose himself to a flaw that had already been fixed.
On the other hand, there should be nothing wrong with downgrading from
1.2 to 1.0. A user on 2.0 must not downgrade to 1.x because the old
major version cannot read the data written by the 2.0 application.
This sort of thing is hard to implement with just a single rollback
number: At the time 2.0 is released, what rollback number should it get?
If we give it 10, we can only produce fixes for nine security bugs in
the 1.x series before that series ends up with a rollback version larger
than the 2.0 one, after which a user on 2.0 would wrongly be able to
downgrade to 1.12.
tl;dr: I'd like this to not only be about rollback protection for
security bugs, but also be able to set other policies for rollback. And
therefore I'd also like it simply to be known as "version" instead of
the somewhat odd arvbn acronym. Nothing prevents the board developer
from just putting a single u32 in the version field and use that as a
single monotonically increasing version.
> As part of signature verification, U-Boot enfroces arvbn based
> protection if enabled. arvbn stored in secure storage is validated with
> arbvn in the sub-image node. If the counter in the FIT image is lower than
> the counter in platform secure storage, image validation has failed
> i.e. verified boot failed. If both counters match or the image counter is
> higher than that in the platform secure storage, the image validation is
> successful. In the higher case, U-Boot stores the new counter in platform
> secure storage.
> Pseudo code is as follows:
>
> ret = board_get_arbvn(type, &plat_arbvn);
> ...
> if (image_arbvn < plat_arbvn) {
> return -EPERM;
> } else if (image_arbvn > plat_arbvn) {
> ret = board_set_arbvn(type, image_arbvn);
> return ret;
> } else {
> return 0;
> }
Hm, I think you should post-pone writing back the new version number
until all other checks have passed, i.e. until just before we pass
control to the new image. That's also a bit simpler if there's only one
version for the whole FIT image since you then wouldn't need to revisit
each image node.
> The following board specific hooks are required to get/set arbvn
> from platform specific tamper-evident storage.
> int board_get_arbvn(uint8_t ih_type, uint32_t *arbvn);
> int board_set_arbvn(uint8_t ih_type, uint32_t arbvn);
>
> As an example, consider this FIT:
> / {
> images {
> kernel-1 {
> data = <data for kernel1>
> arbvn = <1>;
> hash-1 {
> algo = "sha1";
> value = <...kernel hash 1...>
> };
> };
> fdt-1 {
> data = <data for fdt1>;
> hash-1 {
> algo = "sha1";
> value = <...fdt hash 1...>
> };
> };
> };
> configurations {
> default = "conf-1";
> conf-1 {
> kernel = "kernel-1";
> fdt = "fdt-1";
> signature-1 {
> algo = "sha1,rsa2048";
> sign-images = "fdt", "kernel";
> value = <...conf 1 signature...>;
> };
> };
> };
> };
>
> In the above example, kernel-1 image has an arbvn of 1.
> if plat_arbvn is 1, the system will boot with this FIT image.
> if plat_arbvn is 2 or more, U-Boot will prevent the system from booting
> with this FIT image.
Yeah, this seems a bit arbitrary. Why does the kernel have an arvbn, but
not the device tree or (perhaps more importantly) an initramfs? Which
images must have such a number? As I said above, I think the version
number should really be a property of the whole configuration. And of
course, as you also emphasize, the version must be part of what gets
signed (which is the only reason for the indirection to a pseudo-image
node, otherwise it could just be a version=<> property in the
configuration node).
Rasmus
