Re: [gentoo-dev] [PATCH v3] kernel-build.eclass: work around permissions issue with module signing

[Andrew Ammerlaan <andrewammerlaan@gentoo.org>]

v3:

 From dbf92605437b4a457bad2da92f69baab23fcfa44 Mon Sep 17 00:00:00 2001
From: Violet Purcell <vimproved@inventati.org>
Date: Mon, 27 Nov 2023 12:12:09 -0500
Subject: [PATCH] kernel-build.eclass: work around permissions issue with
  module signing

Currently, using a custom path for MODULES_SIGN_KEY requires the key to
be readable by portage:portage. This is not ideal for security, since
the file has to be either owned by portage:portage or readable by all
users in this case. Instead, export the contents of MODULES_SIGN_KEY to
a variable in pkg_setup, and then create a temporary file with it in
src_configure to ensure that the temporary key is readable by the user
that the kernel is being built as. The variable is then unset so it does
not end up in the final environment file.

Co-authored-by: Andrew Ammerlaan <andrewammerlaan@gentoo.org>
Signed-off-by: Violet Purcell <vimproved@inventati.org>
---
  eclass/kernel-build.eclass | 18 ++++++++++++------
  1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/eclass/kernel-build.eclass b/eclass/kernel-build.eclass
index f5529c319f9fc..94b499f82fc1e 100644
--- a/eclass/kernel-build.eclass
+++ b/eclass/kernel-build.eclass
@@ -114,6 +114,13 @@ kernel-build_pkg_setup() {
  	python-any-r1_pkg_setup
  	if [[ ${KERNEL_IUSE_MODULES_SIGN} ]]; then
  		secureboot_pkg_setup
+		if [[ -e ${MODULES_SIGN_KEY} && ${MODULES_SIGN_KEY} != pkcs11:* ]]; then
+			if [[ -e ${MODULES_SIGN_CERT} && ${MODULES_SIGN_CERT} != 
${MODULES_SIGN_KEY} ]]; then
+				MODULES_SIGN_KEY_CONTENTS="$(cat "${MODULES_SIGN_CERT}" 
"${MODULES_SIGN_KEY}" || die)"
+			else
+				MODULES_SIGN_KEY_CONTENTS="$(cat "${MODULES_SIGN_KEY}" || die)"
+			fi
+		fi
  	fi
  }

@@ -422,12 +429,11 @@ kernel-build_merge_configs() {
  				CONFIG_MODULE_SIG_FORCE=y
  				CONFIG_MODULE_SIG_${MODULES_SIGN_HASH^^}=y
  			EOF
-			if [[ -e ${MODULES_SIGN_KEY} && -e ${MODULES_SIGN_CERT} &&
-				${MODULES_SIGN_KEY} != ${MODULES_SIGN_CERT} &&
-				${MODULES_SIGN_KEY} != pkcs11:* ]]
-			then
-				cat "${MODULES_SIGN_CERT}" "${MODULES_SIGN_KEY}" > 
"${T}/kernel_key.pem" || die
-				MODULES_SIGN_KEY="${T}/kernel_key.pem"
+			if [[ -n "${MODULES_SIGN_KEY_CONTENTS}" ]]; then
+				(umask 066; touch "${T}/kernel_key.pem" || die)
+				echo "${MODULES_SIGN_KEY_CONTENTS}" > "${T}/kernel_key.pem" || die
+				unset MODULES_SIGN_KEY_CONTENTS
+				export MODULES_SIGN_KEY="${T}/kernel_key.pem"
  			fi
  			if [[ ${MODULES_SIGN_KEY} == pkcs11:* || -r ${MODULES_SIGN_KEY} ]]; 
then
  				echo "CONFIG_MODULE_SIG_KEY=\"${MODULES_SIGN_KEY}\"" \


On 27/11/2023 18:50, Michał Górny wrote:
> On Mon, 2023-11-27 at 12:12 -0500, Violet Purcell wrote:
>> Currently, using a custom path for MODULES_SIGN_KEY requires the key to
>> be readable by portage:portage. This is not ideal for security, since
>> the file has to be either owned by portage:portage or readable by all
>> users in this case. Instead, export the contents of MODULES_SIGN_KEY to
>> a variable in pkg_setup, and then create a temporary file with it in
>> src_configure to ensure that the temporary key is readable by the user
>> that the kernel is being built as. The variable is then unset so it does
>> not end up in the final environment file.
>>
>> Signed-off-by: Violet Purcell <vimproved@inventati.org>
>> ---
>>   eclass/kernel-build.eclass | 19 +++++++++++++------
>>   1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/eclass/kernel-build.eclass b/eclass/kernel-build.eclass
>> index 4f7e4d047739..cf958c86ff29 100644
>> --- a/eclass/kernel-build.eclass
>> +++ b/eclass/kernel-build.eclass
>> @@ -114,6 +114,13 @@ kernel-build_pkg_setup() {
>>   	python-any-r1_pkg_setup
>>   	if [[ ${KERNEL_IUSE_MODULES_SIGN} ]]; then
>>   		secureboot_pkg_setup
>> +		if [[ -e ${MODULES_SIGN_KEY} && ${MODULES_SIGN_KEY} != pkcs11:* ]]; then
>> +			if [[ -e ${MODULES_SIGN_CERT} && ${MODULES_SIGN_CERT} != ${MODULES_SIGN_KEY} ]]; then
>> +				export MODULES_SIGN_KEY_CONTENTS="$(cat "${MODULES_SIGN_CERT}" "${MODULES_SIGN_KEY}")"
>> +			else
>> +				export MODULES_SIGN_KEY_CONTENTS="$(< "${MODULES_SIGN_KEY}")"
> 
> You don't need to export it.  Unexported variables are also preserved.
> 
>> +			fi
>> +		fi
>>   	fi
>>   }
>>   
>> @@ -427,12 +434,12 @@ kernel-build_merge_configs() {
>>   				CONFIG_MODULE_SIG_FORCE=y
>>   				CONFIG_MODULE_SIG_${MODULES_SIGN_HASH^^}=y
>>   			EOF
>> -			if [[ -e ${MODULES_SIGN_KEY} && -e ${MODULES_SIGN_CERT} &&
>> -				${MODULES_SIGN_KEY} != ${MODULES_SIGN_CERT} &&
>> -				${MODULES_SIGN_KEY} != pkcs11:* ]]
>> -			then
>> -				cat "${MODULES_SIGN_CERT}" "${MODULES_SIGN_KEY}" > "${T}/kernel_key.pem" || die
>> -				MODULES_SIGN_KEY="${T}/kernel_key.pem"
>> +			if [[ -n "${MODULES_SIGN_KEY_CONTENTS}" ]]; then
>> +				touch "${T}/kernel_key.pem" || die
>> +				chmod 0600 "${T}/kernel_key.pem" || die
> 
> This creates a race condition whereupon the file can be opened between
> the call to touch and chmod.  It's better to use a subshell and set
> umask.
> 
>> +				echo "${MODULES_SIGN_KEY_CONTENTS}" > "${T}/kernel_key.pem" || die
>> +				unset MODULES_SIGN_KEY_CONTENTS
>> +				export MODULES_SIGN_KEY="${T}/kernel_key.pem"
>>   			fi
>>   			if [[ ${MODULES_SIGN_KEY} == pkcs11:* || -r ${MODULES_SIGN_KEY} ]]; then
>>   				echo "CONFIG_MODULE_SIG_KEY=\"${MODULES_SIGN_KEY}\"" \
>