[PATCH] build: opkg-key variable key folder

[Paul Spooren]

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.

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