diff mbox series

[RFC,v1] module: sign with sha512 by default to avoid build errors

Message ID 42aa307d7ffae1851b4a8787f5c276dd0b3beece.1728543368.git.linux@leemhuis.info (mailing list archive)
State Handled Elsewhere
Headers show
Series [RFC,v1] module: sign with sha512 by default to avoid build errors | expand

Checks

Context Check Description
mcgrof/vmtest-modules-next-PR success PR summary
mcgrof/vmtest-modules-next-VM_Test-0 pending Logs for build-kernel

Commit Message

Thorsten Leemhuis Oct. 10, 2024, 7 a.m. UTC 
Avoid build errors with allmodconfig on Fedora Linux 41+ by reordering
the Kconfig choices so modules are signed with sha512 by default. That
way sha1 will be avoided, which beforehand was chosen by default on
x86_64 when running allmodconfig -- which on the latest Fedora leads to
the following build error when building the certs/ directory:

  80A20474797F0000:error:03000098:digital envelope routines:do_sigver_init:invalid digest:crypto/evp/m_sigver.c:342:
  make[4]: *** [.../certs/Makefile:53: certs/signing_key.pem] Error 1
  make[4]: *** Deleting file 'certs/signing_key.pem'
  make[4]: *** Waiting for unfinished jobs....
  make[3]: *** [.../scripts/Makefile.build:478: certs] Error 2
  make[2]: *** [.../Makefile:1936: .] Error 2
  make[1]: *** [.../Makefile:224: __sub-make] Error 2
  make[1]: Leaving directory '...'
  make: *** [Makefile:224: __sub-make] Error 2

OpenSSL causes that error, as it now distrusts sha1 signatures by
default on Fedora[1]. This can be worked around locally by switching to
an earlier policy using 'update-crypto-policies --set FEDORA40'.

This change makes things work by default again and will avoid similar
problems on other distributions, as those sooner or later are likely to
apply similar measures; for regular users this likely is a wise move,
too.

Link: https://fedoraproject.org/wiki/Changes/OpenSSLDistrustsha1SigVer [1]
Signed-off-by: Thorsten Leemhuis <linux@leemhuis.info>
---

Lo! This is a submission in the style of "I don't know what I'm doing
and this patch is mainly meant to start a discussion to resolve a
problem I ran into". The patch solved the problem for me, but there
might be a better way to make Kconfig use sha512 by default -- ideally
while keeping the menu in an order that makes more sense for humans.

I furthermore chose sha512 without any strong reasons; I first
considered sha256, but then settled on sha512 because Fedora uses it. So
I'm not attached to this. Ciao, Thorsten
---
 kernel/module/Kconfig | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)


base-commit: d3d1556696c1a993eec54ac585fe5bf677e07474

Comments

Thorsten Leemhuis Oct. 10, 2024, 7:11 a.m. UTC | #1 
On 10.10.24 09:00, Thorsten Leemhuis wrote:
> Avoid build errors with allmodconfig on Fedora Linux 41+ by reordering
> the Kconfig choices so modules are signed with sha512 by default. That
> way sha1 will be avoided, which beforehand was chosen by default on
> x86_64 when running allmodconfig -- which on the latest Fedora leads to
> the following build error when building the certs/ directory:
> [...]
> Link: https://fedoraproject.org/wiki/Changes/OpenSSLDistrustsha1SigVer [1]

Sorry, one search-and-replace went to far, this is meant to be:
https://fedoraproject.org/wiki/Changes/OpenSSLDistrustSHA1SigVer

Ciao, Thorsten
Sedat Dilek Oct. 10, 2024, 8:29 a.m. UTC | #2 
On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> On 10.10.24 09:00, Thorsten Leemhuis wrote:
> > Avoid build errors with allmodconfig on Fedora Linux 41+ by reordering
> > the Kconfig choices so modules are signed with sha512 by default. That
> > way sha1 will be avoided, which beforehand was chosen by default on
> > x86_64 when running allmodconfig -- which on the latest Fedora leads to
> > the following build error when building the certs/ directory:
> > [...]
> > Link: https://fedoraproject.org/wiki/Changes/OpenSSLDistrustsha1SigVer [1]
>
> Sorry, one search-and-replace went to far, this is meant to be:
> https://fedoraproject.org/wiki/Changes/OpenSSLDistrustSHA1SigVer
>
> Ciao, Thorsten
>

Hi Thorsten,

That was wrong in the original code which you moved:

+config MODULE_SIG_SHA384
+       bool "SHA-384"
+       select CRYPTO_SHA512 <--- SHA*384*

BR,
-sed@-
Sedat Dilek Oct. 10, 2024, 8:42 a.m. UTC | #3 
On Thu, Oct 10, 2024 at 10:29 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >
> > On 10.10.24 09:00, Thorsten Leemhuis wrote:
> > > Avoid build errors with allmodconfig on Fedora Linux 41+ by reordering
> > > the Kconfig choices so modules are signed with sha512 by default. That
> > > way sha1 will be avoided, which beforehand was chosen by default on
> > > x86_64 when running allmodconfig -- which on the latest Fedora leads to
> > > the following build error when building the certs/ directory:
> > > [...]
> > > Link: https://fedoraproject.org/wiki/Changes/OpenSSLDistrustsha1SigVer [1]
> >
> > Sorry, one search-and-replace went to far, this is meant to be:
> > https://fedoraproject.org/wiki/Changes/OpenSSLDistrustSHA1SigVer
> >
> > Ciao, Thorsten
> >
>
> Hi Thorsten,
>
> That was wrong in the original code which you moved:
>
> +config MODULE_SIG_SHA384
> +       bool "SHA-384"
> +       select CRYPTO_SHA512 <--- SHA*384*
>
> BR,
> -sed@-

Thorsten, please fix it!

-sed@-

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/Kconfig#n249
https://git.kernel.org/linus/ea0b6dcf71d216dc11733ac19b26df0f5d0fd6c2
Thorsten Leemhuis Oct. 10, 2024, 8:57 a.m. UTC | #4 
On 10.10.24 10:42, Sedat Dilek wrote:
> On Thu, Oct 10, 2024 at 10:29 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>> On 10.10.24 09:00, Thorsten Leemhuis wrote:
>>
>> That was wrong in the original code which you moved:
>>
>> +config MODULE_SIG_SHA384
>> +       bool "SHA-384"
>> +       select CRYPTO_SHA512 <--- SHA*384*
> 
> Thorsten, please fix it!

That looks intentional to me -- and CRYPTO_SHA384 from a quick look does
not even exist.

But that's not at all my area of expertise, so I would not want to touch
it anyway.

Ciao, Thorsten

P.S.: Vegard Nossum mentioned in the fediverse that I could also solve
the problem the patch is about by adding "default MODULE_SIG_SHA512" to
the "choice" section; haven't tried that, but that sounds like a better
solution. Will likely give it a try, unless someone brings up unwanted
side effects this might cause.
Sedat Dilek Oct. 10, 2024, 9:09 a.m. UTC | #5 
On Thu, Oct 10, 2024 at 10:57 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> On 10.10.24 10:42, Sedat Dilek wrote:
> > On Thu, Oct 10, 2024 at 10:29 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >> On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >>> On 10.10.24 09:00, Thorsten Leemhuis wrote:
> >>
> >> That was wrong in the original code which you moved:
> >>
> >> +config MODULE_SIG_SHA384
> >> +       bool "SHA-384"
> >> +       select CRYPTO_SHA512 <--- SHA*384*
> >
> > Thorsten, please fix it!
>
> That looks intentional to me -- and CRYPTO_SHA384 from a quick look does
> not even exist.
>
> But that's not at all my area of expertise, so I would not want to touch
> it anyway.
>
> Ciao, Thorsten
>
> P.S.: Vegard Nossum mentioned in the fediverse that I could also solve
> the problem the patch is about by adding "default MODULE_SIG_SHA512" to
> the "choice" section; haven't tried that, but that sounds like a better
> solution. Will likely give it a try, unless someone brings up unwanted
> side effects this might cause.
>

Not an expert in this area, too.

Looks like I misread it.

SHA224 selects XXX_SHA*256* so the code might be intentional.

-sed@-
Sedat Dilek Oct. 10, 2024, 9:36 a.m. UTC | #6 
On Thu, Oct 10, 2024 at 10:57 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> On 10.10.24 10:42, Sedat Dilek wrote:
> > On Thu, Oct 10, 2024 at 10:29 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >> On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >>> On 10.10.24 09:00, Thorsten Leemhuis wrote:
> >>
> >> That was wrong in the original code which you moved:
> >>
> >> +config MODULE_SIG_SHA384
> >> +       bool "SHA-384"
> >> +       select CRYPTO_SHA512 <--- SHA*384*
> >
> > Thorsten, please fix it!
>
> That looks intentional to me -- and CRYPTO_SHA384 from a quick look does
> not even exist.
>
> But that's not at all my area of expertise, so I would not want to touch
> it anyway.
>
> Ciao, Thorsten
>
> P.S.: Vegard Nossum mentioned in the fediverse that I could also solve
> the problem the patch is about by adding "default MODULE_SIG_SHA512" to
> the "choice" section; haven't tried that, but that sounds like a better
> solution. Will likely give it a try, unless someone brings up unwanted
> side effects this might cause.
>
>

SHA1 | SHA256 | SHA512 <--- IMHO enough.

-sed@-
Sami Tolvanen Oct. 10, 2024, 3:52 p.m. UTC | #7 
Hi,

On Thu, Oct 10, 2024 at 1:57 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> On 10.10.24 10:42, Sedat Dilek wrote:
> > On Thu, Oct 10, 2024 at 10:29 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >> On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >>> On 10.10.24 09:00, Thorsten Leemhuis wrote:
> >>
> >> That was wrong in the original code which you moved:
> >>
> >> +config MODULE_SIG_SHA384
> >> +       bool "SHA-384"
> >> +       select CRYPTO_SHA512 <--- SHA*384*
> >
> > Thorsten, please fix it!
>
> That looks intentional to me -- and CRYPTO_SHA384 from a quick look does
> not even exist.

It is intentional. SHA-384 is just truncated SHA-512 with different
initial hash values. The same with SHA-224/256.

> But that's not at all my area of expertise, so I would not want to touch
> it anyway.
>
> Ciao, Thorsten
>
> P.S.: Vegard Nossum mentioned in the fediverse that I could also solve
> the problem the patch is about by adding "default MODULE_SIG_SHA512" to
> the "choice" section; haven't tried that, but that sounds like a better
> solution. Will likely give it a try, unless someone brings up unwanted
> side effects this might cause.

Yes, that would be a much better way to change the default. Overall,
moving away from SHA-1 seems like a good idea and SHA-512 feels like a
reasonable choice. Luis, do you see any issues with changing the
default here?

Sami
Thorsten Leemhuis Oct. 11, 2024, 10:27 a.m. UTC | #8 
On 10.10.24 17:52, Sami Tolvanen wrote:

Thx for your feedback!

> On Thu, Oct 10, 2024 at 1:57 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> On 10.10.24 10:42, Sedat Dilek wrote:
>>> On Thu, Oct 10, 2024 at 10:29 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>>> On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>>>> On 10.10.24 09:00, Thorsten Leemhuis wrote:
>
>> P.S.: Vegard Nossum mentioned in the fediverse that I could also solve
>> the problem the patch is about by adding "default MODULE_SIG_SHA512" to
>> the "choice" section; haven't tried that, but that sounds like a better
>> solution. Will likely give it a try, unless someone brings up unwanted
>> side effects this might cause.
> 
> Yes, that would be a much better way to change the default. Overall,
> moving away from SHA-1 seems like a good idea and SHA-512 feels like a
> reasonable choice. Luis, do you see any issues with changing the
> default here?

So, how do I make such a default choice work without breaking the
current magic, which looks like this:

"""
config MODULE_SIG_HASH
	string
	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
	default "sha1" if MODULE_SIG_SHA1
	default "sha256" if MODULE_SIG_SHA256
	default "sha384" if MODULE_SIG_SHA384
	default "sha512" if MODULE_SIG_SHA512
	default "sha3-256" if MODULE_SIG_SHA3_256
	default "sha3-384" if MODULE_SIG_SHA3_384
	default "sha3-512" if MODULE_SIG_SHA3_512
"""

Reordering those did not do the trick. And I suspect adding a

  default "sha512"

would break the magic. Would dropping sha1, sha256 and sha384 from the
list be a middle ground that could work for everyone?

Ciao, Thorsten
Thorsten Leemhuis Oct. 11, 2024, noon UTC | #9 
On 11.10.24 12:27, Thorsten Leemhuis wrote:
> On 10.10.24 17:52, Sami Tolvanen wrote:
> Thx for your feedback!
>> On Thu, Oct 10, 2024 at 1:57 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>> On 10.10.24 10:42, Sedat Dilek wrote:
>>>> On Thu, Oct 10, 2024 at 10:29 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>>>> On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
>>>>>> On 10.10.24 09:00, Thorsten Leemhuis wrote:
>>
>>> P.S.: Vegard Nossum mentioned in the fediverse that I could also solve
>>> the problem the patch is about by adding "default MODULE_SIG_SHA512" to
>>> the "choice" section; haven't tried that, but that sounds like a better
>>> solution. Will likely give it a try, unless someone brings up unwanted
>>> side effects this might cause.
>>
>> Yes, that would be a much better way to change the default. Overall,
>> moving away from SHA-1 seems like a good idea and SHA-512 feels like a
>> reasonable choice. Luis, do you see any issues with changing the
>> default here?
> 
> So, how do I make such a default choice work without breaking the
> current magic, which looks like this:
> [...]

Ignore that, I was missing something obvious and got mislead by my
brain, sorry for the noise. Will send a updated patch in a few days to
give Luis and others a chance to raise objections reg. switching to SHA512.

Ciao, Thorsten
Luis Chamberlain Oct. 11, 2024, 11:14 p.m. UTC | #10 
On Fri, Oct 11, 2024 at 02:00:47PM +0200, Thorsten Leemhuis wrote:
> On 11.10.24 12:27, Thorsten Leemhuis wrote:
> > On 10.10.24 17:52, Sami Tolvanen wrote:
> > Thx for your feedback!
> >> On Thu, Oct 10, 2024 at 1:57 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >>> On 10.10.24 10:42, Sedat Dilek wrote:
> >>>> On Thu, Oct 10, 2024 at 10:29 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >>>>> On Thu, Oct 10, 2024 at 10:19 AM Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >>>>>> On 10.10.24 09:00, Thorsten Leemhuis wrote:
> >>
> >>> P.S.: Vegard Nossum mentioned in the fediverse that I could also solve
> >>> the problem the patch is about by adding "default MODULE_SIG_SHA512" to
> >>> the "choice" section; haven't tried that, but that sounds like a better
> >>> solution. Will likely give it a try, unless someone brings up unwanted
> >>> side effects this might cause.
> >>
> >> Yes, that would be a much better way to change the default. Overall,
> >> moving away from SHA-1 seems like a good idea and SHA-512 feels like a
> >> reasonable choice. Luis, do you see any issues with changing the
> >> default here?
> > 
> > So, how do I make such a default choice work without breaking the
> > current magic, which looks like this:
> > [...]
> 
> Ignore that, I was missing something obvious and got mislead by my
> brain, sorry for the noise. Will send a updated patch in a few days to
> give Luis and others a chance to raise objections reg. switching to SHA512.

The commmit log goes something like this:

Fix build by switching to sha512 by default.

The commit log should be imperative about the crap show issue without
the build considerations. Beat down the current default, call it names,
give URLs to back it up. You "noticed" this issue because the build
fails.

  Luis
diff mbox series

Patch

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 7c6588148d42d3..3647ff25d49d67 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -238,18 +238,6 @@  choice
 	  possible to load a signed module containing the algorithm to check
 	  the signature on that module.
 
-config MODULE_SIG_SHA1
-	bool "SHA-1"
-	select CRYPTO_SHA1
-
-config MODULE_SIG_SHA256
-	bool "SHA-256"
-	select CRYPTO_SHA256
-
-config MODULE_SIG_SHA384
-	bool "SHA-384"
-	select CRYPTO_SHA512
-
 config MODULE_SIG_SHA512
 	bool "SHA-512"
 	select CRYPTO_SHA512
@@ -266,6 +254,18 @@  config MODULE_SIG_SHA3_512
 	bool "SHA3-512"
 	select CRYPTO_SHA3
 
+config MODULE_SIG_SHA384
+	bool "SHA-384"
+	select CRYPTO_SHA512
+
+config MODULE_SIG_SHA256
+	bool "SHA-256"
+	select CRYPTO_SHA256
+
+config MODULE_SIG_SHA1
+	bool "SHA-1"
+	select CRYPTO_SHA1
+
 endchoice
 
 config MODULE_SIG_HASH