[PATCH] openwrt-keyring: Only copy sign key for snapshots

[Hauke Mehrtens]

On 5/15/21 1:34 AM, Daniel Golle wrote:
> On Fri, May 14, 2021 at 11:31:27PM +0200, Hauke Mehrtens wrote:
>> On 5/14/21 12:17 PM, Paul Spooren wrote:
>>> Hi,
>>>
>>> On 5/13/21 1:32 AM, Hauke Mehrtens wrote:
>>>> Instead of adding all public signature keys from the openwrt-keyring
>>>> repository only add the key which is used to sign the master feeds.
>>>>
>>>> If one of the other keys would be compromised this would not affect
>>>> users of master snapshot builds.
>>>>
>>>> Signed-off-by: Hauke Mehrtens <hauke at hauke-m.de>
>>>> ---
>>>
>>> Thanks for working on this.
>>>
>>> I'm still in favor to include a *openwrt-next* key which becomes the
>>> signing key for the next release. This way a upgrade step between
>>> release branches is possible.
>>
>> I would prefer to create it closer to the next release.
>>
>>>> As far as I know the other keys are not compromised, this is just a
>>>> precaution.
>>>>
>>>> I would do similar changes to 21.02 and 19.07 to only add the key which
>>>> is used for this specific release.
>>> In case of 19.07 please add 21.02 release keys as well, since it's *the
>>
>>> next key*.
>>
>> Yes, good idea.
>>
>>>> Instead of adding just this single key, should we add all keys of
>>>> currently maintained releases like 19.07, 21.02 and master key into all
>>>> 3 branches?
>>> How about adding keys like that:
>>> 19.07: 19.07 + 21.02 keys
>>> 21.02: 21.02 + openwrt-next keys
>>> snapshot: snapshot key
>>>
>>> The snapshot key stays the same "forever", it shouldn't be included in
>>> releases.
>>>
>>>> The signature verification of sysupgrade images is currently not used as
>>>> far as I know, so normal we do not need the keys for of other releases.
>>>
>>> If the `ucert` package is installed and the env variable
>>> `REQUIRE_IMAGE_SIGNATURE` is set, the images are verified. This should
>>> eventually become the default.
>>
>> How reliable is this working?
> 
> I've been using ucert on many devices for a long time for now.
> In order to be more secure, the signed data should be normalized
> (ie. sorted and non-relevant data removed), which has not been done
> yet. Right now, hash collissions could be constructed by changing
> the order of fields and/or adding useless additional data -- however,
> that would still mean having to break SHA256.
> 
> Generally, to be considered more than just a small extra barrier
> or even a security risk, much more review would be needed. See:
> 
> https://git.openwrt.org/?p=project/ucert.git;a=blob;f=README.md;hb=refs/heads/master#l6
> 
>>
>> Currently we do not ship ucert by default and this is needed to check the
>> image signature.
> 
> People can, however, install ucert which enabled signature checks
> of future sysupgrade. When using 'auc' or 'luci-app-attendedsysupgrade'
> for upgrade, all explicitely installed packages are also kept accross
> updates, and that can include 'ucert' (which is what I've been doing
> for a while now on my local devices)

Ok this is nice.

I tried to check the signature of the 21.02-rc1 release and it failed:
-------------------------------------------------------------------
root at OpenWrt:/tmp# REQUIRE_IMAGE_SIGNATURE=1 sysupgrade -T 
openwrt-21.02.0-rc1-ath79-generic-tplink_tl-wdr4300-v1-squashfs-sysupgrade.bin 

cert_verify: cannot parse cert
Image check failed.
-------------------------------------------------------------------

With a self build image it works.

It contains "# fake certificate" where I would expect the certificate.

Is this expected?

Hauke