[U-Boot] [PATCH v2 0/11] Minor improvements to secure boot and enable on beaglebone

[Simon Glass]

Hi,

On 2 June 2014 23:17, Belisko Marek <marek.belisko at gmail.com> wrote:
> Dear Simon Glass,
>
> On Tue, Jun 3, 2014 at 6:42 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Belisko,
>>
>> On 28 April 2014 00:30, Belisko Marek <marek.belisko at gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Wed, Apr 16, 2014 at 4:41 PM, Simon Glass <sjg at chromium.org> wrote:
>>>> This series fixes a few problems that have come up since the secure boot
>>>> series was merged:
>>>>
>>>> - A recent commit broken the assumption that u-boot.bin ends at a known
>>>> address (thus making things appended to U-Boot inaccessible from the code).
>>>> This is fixed for Beaglebone and also a new test is added to the Makefile
>>>> to ensure that it does not break again. All boards have been tested.
>>>>
>>>> - A way is needed to provide an externally-build device tree binary for
>>>> U-Boot. This allows signing to happen outside the U-Boot build system.
>>>>
>>>> - The .img files generated by an OMAP build need to include the FDT if one
>>>> is appended.
>>>>
>>>> - Adding signatures to an FDT can cause the FDT to run out of space. The
>>>> fix is to regenerate the FDT from scratch with different dtc parameters, so
>>>> pretty painful. Instead, we automatically expand the FDT.
>>>>
>>>> The last two commits enable secure boot on Beaglebone (this will have no
>>>> effect unless signed images are used). This could be moved to a separate
>>>> configuration if required, or these patches could even be ignored:
>>> I've tested this patch series and I found some issues. When I use dtb
>>> build from latest 3.15-rc3 kernel
>>> I got during signing this errors:
>>> Couldn't create signature node: FDT_ERR_NOSPACE
>>> Failed to add verification data for 'signature at 1' signature node in
>>> 'conf at 1' image node
>>>
>>> which was fixed by those 2 small patches:
>>> - this one doesn't overwrite return value because upper layer then
>>> stop with no space error and doesn't allocate more space
>>> --- a/lib/rsa/rsa-sign.c
>>> +++ b/lib/rsa/rsa-sign.c
>>> @@ -405,7 +405,7 @@ int rsa_add_verify_data(struct image_sign_info
>>> *info, void *keydest)
>>>                 if (parent < 0) {
>>>                         fprintf(stderr, "Couldn't create signature node: %s\n",
>>>                                 fdt_strerror(parent));
>>> -                       return -EINVAL;
>>> +                       return parent;

Here we must return a value like -ENOSPC if we want to signal that.
Also it should be done before printing the error.

>>>                 }
>>>         }
>>>
>>> --- a/tools/image-host.c
>>> +++ b/tools/image-host.c
>>> @@ -612,7 +612,7 @@ static int fit_config_process_sig(const char
>>> *keydir, void *keydest,
>>>                 if (ret) {
>>>                         printf("Failed to add verification data for
>>> '%s' signature node in '%s' image node\n",
>>>                                node_name, conf_name);
>>> -                       return ret == FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
>>> +                       return ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;

This looks right to me.

>>>                 }
>>>         }
>>>
>>> With this small changes I can create signed fit image.
[snip]


>>
>> For this I'm not sure, perhaps your load address is wrong?
> Yes I've fixed this issue some time ago. What about those two
> mentioned patches? Without them
> I cannot create FIT properly.

I think your patches are useful - my code was aimed at fixing this
problem for the FIT image, and adding the large signature block, but
it can just as easily happen with the U-Boot fdt.

I will take a look at rolling it into a new version, but if you come
up with a patch on top of my latest series, please send it
(u-boot-x86.git branch 'bone')

Regards,
Simon