| Message ID | 20210512233239.866967-1-hauke@hauke-m.de |
|---|---|
| State | Accepted |
| Delegated to: | Hauke Mehrtens |
| Headers | show |
| Series | openwrt-keyring: Only copy sign key for snapshots | expand |
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@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. > 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*. > 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. So ideally we already start shipping the correct keys before activating the extra security measurements. > > package/system/openwrt-keyring/Makefile | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/package/system/openwrt-keyring/Makefile b/package/system/openwrt-keyring/Makefile > index 6f3aa65622d5..ceaccf1fc527 100644 > --- a/package/system/openwrt-keyring/Makefile > +++ b/package/system/openwrt-keyring/Makefile > @@ -32,7 +32,8 @@ Build/Compile= > > define Package/openwrt-keyring/install > $(INSTALL_DIR) $(1)/etc/opkg/keys/ > - $(INSTALL_DATA) $(PKG_BUILD_DIR)/usign/* $(1)/etc/opkg/keys/ > + # Public usign key for unattended snapshot builds > + $(INSTALL_DATA) $(PKG_BUILD_DIR)/usign/b5043e70f9a75cde $(1)/etc/opkg/keys/ > endef > > $(eval $(call BuildPackage,openwrt-keyring))
Paul Spooren <mail@aparcar.org> writes: > On 5/13/21 1:32 AM, Hauke Mehrtens wrote: >> 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. > > So ideally we already start shipping the correct keys before > activating the extra security measurements. I wonder if I have understood the current signing scheme correctly: - create an expiring certificate signed by the private signing key - sign image with private signing key and append both certificate and signature - validate image signature using certificate - validate ceritificate using public signing key If this is correct, then I don't think it will fly. The problem is the expiration of the redundant certificate. This means that the image has an absolute expiration date. You don't want that. You might have expiring keys. But the images, including their signatures, should last forever. Or as long as the key is considered valid. I also have a small issue with the creation of the certificate for home builders, but that's a minor problem and rather simple to fix. However, it just hides the underlying problem by moving the image expiration date from the past to up to a year in the future. It just highlighted the certificate issue when I started building invalid images because the included certificate was older than a year, and already expired by the time it was appended to the image Bjørn
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@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? Currently we do not ship ucert by default and this is needed to check the image signature. > So ideally we already start shipping the correct keys before activating > the extra security measurements. > Hauke
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@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) > > > So ideally we already start shipping the correct keys before activating > > > the extra security measurements. > > > > Hauke > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
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@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@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
On Sat, May 15, 2021 at 04:28:58PM +0200, Hauke Mehrtens wrote: > 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@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@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? Yes and no :) No, because in this way ucert is pretty useless. Yes, because this is how buildbots are configured to do things at this point: https://git.openwrt.org/?p=buildbot.git;a=blob;f=phase1/master.cfg;h=a85382ae4fd2ee52d0d102fc90be7f721a2dfe86;hb=HEAD#l986 The goal of ucert is to allow key delegation, as described in the README.md in ucert.git. Like this we could have builders generate keys with short lifetime, have them signed by a more protected instance and have the option to revoke them, if needed.
On 5/15/21 4:44 PM, Daniel Golle wrote: > On Sat, May 15, 2021 at 04:28:58PM +0200, Hauke Mehrtens wrote: >> 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@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@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? > > Yes and no :) > No, because in this way ucert is pretty useless. > > Yes, because this is how buildbots are configured to do things at this > point: > https://git.openwrt.org/?p=buildbot.git;a=blob;f=phase1/master.cfg;h=a85382ae4fd2ee52d0d102fc90be7f721a2dfe86;hb=HEAD#l986 > > The goal of ucert is to allow key delegation, as described in the > README.md in ucert.git. Like this we could have builders generate > keys with short lifetime, have them signed by a more protected > instance and have the option to revoke them, if needed. Ok thank you Daniel for the clarification. So people are fine with the original patch to only include the master key into the image. I will send seperate patches for 21.02 and 19.07. Hauke
diff --git a/package/system/openwrt-keyring/Makefile b/package/system/openwrt-keyring/Makefile index 6f3aa65622d5..ceaccf1fc527 100644 --- a/package/system/openwrt-keyring/Makefile +++ b/package/system/openwrt-keyring/Makefile @@ -32,7 +32,8 @@ Build/Compile= define Package/openwrt-keyring/install $(INSTALL_DIR) $(1)/etc/opkg/keys/ - $(INSTALL_DATA) $(PKG_BUILD_DIR)/usign/* $(1)/etc/opkg/keys/ + # Public usign key for unattended snapshot builds + $(INSTALL_DATA) $(PKG_BUILD_DIR)/usign/b5043e70f9a75cde $(1)/etc/opkg/keys/ endef $(eval $(call BuildPackage,openwrt-keyring))
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@hauke-m.de> --- 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. 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? 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. package/system/openwrt-keyring/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)