From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gentoo-dev+bounces-97571-garchives=archives.gentoo.org@lists.gentoo.org>
Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits))
	(No client certificate requested)
	by finch.gentoo.org (Postfix) with ESMTPS id 9A4FE158094
	for <garchives@archives.gentoo.org>; Mon, 27 Jun 2022 19:46:40 +0000 (UTC)
Received: from pigeon.gentoo.org (localhost [127.0.0.1])
	by pigeon.gentoo.org (Postfix) with SMTP id 6DA13E0B23;
	Mon, 27 Jun 2022 19:46:37 +0000 (UTC)
Received: from smtp.gentoo.org (smtp.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (4096 bits))
	(No client certificate requested)
	by pigeon.gentoo.org (Postfix) with ESMTPS id 1CB70E0AD0
	for <gentoo-dev@lists.gentoo.org>; Mon, 27 Jun 2022 19:46:37 +0000 (UTC)
Message-ID: <f1871b56f53ecf75193e7d8c34cce42f80f822e9.camel@gentoo.org>
Subject: Re: [gentoo-dev] [PATCH] linux-mod.eclass: support module signing
From: Georgy Yakovlev <gyakovlev@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Date: Mon, 27 Jun 2022 12:46:34 -0700
In-Reply-To: <20220627183531.palnmdpvgzf44ssk@fuuko>
References: <20220621181959.920941-1-concord@gentoo.org>
	 <84e99a74d64f0d9dd326af0f2c54b9d5717b2f8d.camel@gentoo.org>
	 <9317f3aa1815d9ef219625794c06a8fb3057d707.camel@gentoo.org>
	 <20220627183531.palnmdpvgzf44ssk@fuuko>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.2 
Precedence: bulk
List-Post: <mailto:gentoo-dev@lists.gentoo.org>
List-Help: <mailto:gentoo-dev+help@lists.gentoo.org>
List-Unsubscribe: <mailto:gentoo-dev+unsubscribe@lists.gentoo.org>
List-Subscribe: <mailto:gentoo-dev+subscribe@lists.gentoo.org>
List-Id: Gentoo Linux mail <gentoo-dev.gentoo.org>
X-BeenThere: gentoo-dev@lists.gentoo.org
Reply-to: gentoo-dev@lists.gentoo.org
X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply
MIME-Version: 1.0
X-Archives-Salt: cf36a832-7509-45aa-93aa-9eb9b825fce7
X-Archives-Hash: 79dc852dd7cbf26ee6318857b6bccc4a

On Mon, 2022-06-27 at 14:35 -0400, Kenton Groombridge wrote:
> On 22/06/26 04:15AM, Georgy Yakovlev wrote:
> > On Sun, 2022-06-26 at 03:52 -0700, Georgy Yakovlev wrote:
> > > On Tue, 2022-06-21 at 14:19 -0400, Kenton Groombridge wrote:
> > > > eee74b9fca1 adds support for module compression, but this
> > > > breaks
> > > > loading
> > > > out of tree modules when module signing is enforced because
> > > > modules
> > > > must
> > > > be signed before they are compressed. Additionally, the
> > > > recommended
> > > > Portage hook[1] no longer works with this change.
> > > >=20
> > > > Add module signing support in linux-mod.eclass which more or
> > > > less
> > > > does
> > > > exactly what the aforementioned Portage hook does. If the
> > > > kernel
> > > > configuration has CONFIG_MODULE_SIG_ALL=3Dy, then read the hash
> > > > and
> > > > keys
> > > > from the kernel configuration and call the sign_file tool to
> > > > sign
> > > > the
> > > > module before it is compressed.
> > > >=20
> > > > Bug: https://bugs.gentoo.org/show_bug.cgi?id=3D447352
> > > > Signed-off-by: Kenton Groombridge <concord@gentoo.org>
> > > > ---
> > > > =C2=A0eclass/linux-mod.eclass | 16 ++++++++++++++++
> > > > =C2=A01 file changed, 16 insertions(+)
> > > >=20
> > > > diff --git a/eclass/linux-mod.eclass b/eclass/linux-mod.eclass
> > > > index b7c13cbf7e7..fd40f6d7c6c 100644
> > > > --- a/eclass/linux-mod.eclass
> > > > +++ b/eclass/linux-mod.eclass
> > > > @@ -712,6 +712,22 @@ linux-mod_src_install() {
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0cd "${objdir}" || die "${objdir} does not
> > > > exist"
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0insinto
> > > > "${INSTALL_MOD_PATH}"/lib/modules/${KV_FULL}/${libdir}
> > > > =C2=A0
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0# check here for CONFIG_MODULE_SIG_ALL and sign
> > > > the
> > > > module being built if enabled.
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0# modules must be signed before they are
> > > > compressed.
> > > > +
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0if linux_chkconfig_present MODULE_SIG_ALL; then
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0loc=
al
> > > > module_sig_hash=3D"$(linux_chkconfig_string MODULE_SIG_HASH)"
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0loc=
al
> > > > module_sig_key=3D"$(linux_chkconfig_string MODULE_SIG_KEY)"
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0mod=
ule_sig_key=3D"${module_sig_key:-
> > > > certs/signing_key.pem}"
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if =
[[ "${module_sig_key#pkcs11:}" =3D=3D
> > > > "${module_sig_key}" && "${module_sig_key#/}" =3D=3D
> > > > "${module_sig_key}"
> > > > ]]; then
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0local
> > > > key_path=3D"${KERNEL_DIR}/${module_sig_key}"
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0els=
e
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0local
> > > > key_path=3D"${module_sig_key}"
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0fi
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0loc=
al
> > > > cert_path=3D"${KERNEL_DIR}/certs/signing_key.x509"
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0"${=
KERNEL_DIR}"/scripts/sign-file
> > > > ${module_sig_hash//\"} ${key_path//\"} ${cert_path}
> > > > ${modulename}.${KV_OBJ}
> > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0fi
> > > > +
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0# check here for
> > > > CONFIG_MODULE_COMPRESS_<compression
> > > > option> (NONE, GZIP, XZ, ZSTD)=20
> > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0# and similarily compress the module being
> > > > built if
> > > > !=3D NONE.
> > > > =C2=A0
> > >=20
> > >=20
> > > Hi,
> > >=20
> > > I've spent some time in the past ( circa 2018 ) to get this in,
> > > but=20
> > > gave up due to various reasons, I was not a gentoo dev yet at the
> > > time.
> > >=20
> > > I can't see how posted implementation will work tbh.
> > > portage will strip signature out of the module, unless you
> > > prevent
> > > stripping completely or package uses EAPI>=3D7, and omits stripping
> > > modules via dostrip -x on the ko object.
> > > kernel will NOT load module with stripped signature.
> > >=20
> > > so either you have to sign in pkg_postinst phase, or prevent
> > > stripping.
> > > signing in postinst is not ideal, because if breaks recorded file
> > > checksums in vdb.
> > >=20
> > > here's old fork of eclass I made, maybe you can find some helpful
> > > code
> > > in there
> > >=20
> > > https://github.com/gyakovlev/linux-mod.eclass/blob/master/linux-mod.e=
class
> > >=20
> > > old ML discussion we had:
> > > https://archives.gentoo.org/gentoo-dev/message/4b15b1c851f379a1f802e2=
f2895cdfa8
> > >=20
> > > You will also need a dependency on openssl, since sign-file uses
> > > it.
> > >=20
> > > lmk if you need more info, I might remember more details, but for
> > > now
> > > that's all I have. I'll try to help get it done, but my
> > > availability
> > > is
> > > spotty due to limited time.
> >=20
> > after reading my old code again and thinking more I think I know
> > what's
> > going on.
> > =C2=A01. I've actually solved checksum/strip problem by signing in pkg-
> > preinst
> > =C2=A02. my method will likely fail with compressed modules.
> > =C2=A03. your method likely works only if modules are compressed -
> > because
> > portage does not strip those I think.
> >=20
>=20
> This is exactly what I was thinking. I'm pretty sure I wasn't seeing
> the
> problematic signature stripping behavior because I have module
> compression enabled.
>=20
> Also good point about the OpenSSL dependency. That's something I
> didn't
> consider.
>=20
> > so looks like we need to combine both methods and do the following:
> > =C2=A0- if signing requested without compression - sign in pkg_preinst.
> > =C2=A0- if signing requested with compression - sign in src_install
> >=20
>=20
> Why can't we do both in pkg_preinst? I am thinking it would be best
> if
> we drop the current compression implementation and rework your old
> code
> to handle both compression and signing since the signing code is more
> or
> less already complete.

i'm not sure if sign-file can sign compressed modules.
if we let kernel build handle compression - we have to sign prior to
compression.
if we compress modules ourselves then yes, we could sign first indeed.

but preinst has it's own issues, you've already seen floppym's remark.

>=20
> > Do I make sense? I still haven't tested it, just guessing as I read
> > my
> > old bash code.
> >=20