[PATCH] build: opkg-key variable key folder

[Daniel Golle]

On Wed, Aug 26, 2020 at 11:57:55AM -1000, Paul Spooren wrote:
> 
> On 26.08.20 09:17, Baptiste Jonglez wrote:
> > On 25-08-20, Paul Spooren wrote:
> > > The key folder is used by `opkg` and `usign` to store and retrieve
> > > trusted public keys. Using `opkg-key` outside a running device is
> > > unfeasible as the key folder is hard coded to `/etc/opkg/keys`.
> > > 
> > > This commit adds a variable OPKG_KEYS which defaults to `/etc/opkg/keys`
> > > if unset, however allows set arbitrary key folder locations.
> > > 
> > > Arbitrary key folder locations are useful to add signature verification
> > > to the ImageBuilders.
> > It should be noted that this increases the attack surface.
> > 
> > Especially env variables are easy to set and hard to notice.  This could
> > allow an attacker to provide a rogue keyring to opkg by just setting an
> > environment variable, and this would be quite hard to trace.
> 
> The `opkg` process likely runs as root user, so you have so sneak in env
> variables to a root process, which from my understanding root privileges.
> 
> Also the current script uses a relative path to `usign`, which means instead
> of providing your own fake keys via OPKG_KEYS, you could just modify the
> PATH variable to have a fake version of `usign` running, returning a
> successful exit code to anything.
> 
> > I would prefer one of two alternatives:
> > 
> > 1) take a cmdline argument
> 
> Where would that cmdline be added? From the ImageBuilder Makefile? In that
> case it'd go through `opkg` to then be passed to `opkg-key`. This would
> somewhat force you to use that specific script, while there exists also a
> `gnupg` version[1].
> 
> [1]: https://git.openwrt.org/?p=project/opkg-lede.git;a=blob;f=utils/opkg-key;h=266bb6628a9cae8096f482c4b1d78fe3c3bcb2ca;hb=HEAD
> 
> > 2) provide different opkg-key scripts for device and host usage.  Each
> >     script can use a ~hard-coded path to the expected key location (possibly
> >     using $TOPDIR etc).
> > 
> > While solution 2) would still use env variables for host usage, it would
> > only apply to this host usage of opkg: we do not compromise the security
> > of device-based opkg.
> 
> I'm somewhat against maintaining two scripts if not really necessary. If we
> go that road we need absolute path for usign, cat, zcat etc.

I agree with Paul's arguments and was about to pull this change in.
Any objections?



> 
> Paul
> 
> > > Signed-off-by: Paul Spooren <mail at aparcar.org>
> > > ---
> > >   package/system/opkg/files/opkg-key | 10 ++++++----
> > >   1 file changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/package/system/opkg/files/opkg-key b/package/system/opkg/files/opkg-key
> > > index ae5e8a4591..51d1857ad5 100755
> > > --- a/package/system/opkg/files/opkg-key
> > > +++ b/package/system/opkg/files/opkg-key
> > > @@ -1,5 +1,7 @@
> > >   #!/bin/sh
> > > +OPKG_KEYS="${OPKG_KEYS:-/etc/opkg/keys}"
> > > +
> > >   usage() {
> > >   	cat <<EOF
> > >   Usage: $0 <command> <arguments...>
> > > @@ -19,7 +21,7 @@ opkg_key_verify() {
> > >   	(
> > >   		zcat "$msgfile" 2>/dev/null ||
> > >   		cat "$msgfile" 2>/dev/null
> > > -	) | usign -V -P /etc/opkg/keys -q -x "$sigfile" -m -
> > > +	) | usign -V -P "$OPKG_KEYS" -q -x "$sigfile" -m -
> > >   }
> > >   opkg_key_add() {
> > > @@ -27,8 +29,8 @@ opkg_key_add() {
> > >   	[ -n "$key" ] || usage
> > >   	[ -f "$key" ] || echo "Cannot open file $1"
> > >   	local fingerprint="$(usign -F -p "$key")"
> > > -	mkdir -p "/etc/opkg/keys"
> > > -	cp "$key" "/etc/opkg/keys/$fingerprint"
> > > +	mkdir -p "$OPKG_KEYS"
> > > +	cp "$key" "$OPKG_KEYS/$fingerprint"
> > >   }
> > >   opkg_key_remove() {
> > > @@ -36,7 +38,7 @@ opkg_key_remove() {
> > >   	[ -n "$key" ] || usage
> > >   	[ -f "$key" ] || echo "Cannot open file $1"
> > >   	local fingerprint="$(usign -F -p "$key")"
> > > -	rm -f "/etc/opkg/keys/$fingerprint"
> > > +	rm -f "$OPKG_KEYS/$fingerprint"
> > >   }
> > >   case "$1" in
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel