| Message ID | 20240109002429.1129950-1-coxu@redhat.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v2] integrity: eliminate unnecessary "Problem loading X.509 certificate" msg | expand |
Hi Mimi, Could you take a look at this version of patch? If it escaped your attention because it got buried in the same thread, sorry for that. And I won't send new version as a reply to previous version in the future. On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote: >Currently when the kernel fails to add a cert to the .machine keyring, >it will throw an error immediately in the function integrity_add_key. > >Since the kernel will try adding to the .platform keyring next or throw >an error (in the caller of integrity_add_key i.e. add_to_machine_keyring), >so there is no need to throw an error immediately in integrity_add_key. > >Reported-by: itrymybest80@protonmail.com >Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331 >Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine") >Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com> >Signed-off-by: Coiby Xu <coxu@redhat.com> >--- >v2 > - improve patch subject [Mimi] > - add Fixes tag [Jarkko] > - add Reviewed-by tag from Eric >--- > security/integrity/digsig.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c >index df387de29bfa..45c3e5dda355 100644 >--- a/security/integrity/digsig.c >+++ b/security/integrity/digsig.c >@@ -179,7 +179,8 @@ static int __init integrity_add_key(const unsigned int id, const void *data, > KEY_ALLOC_NOT_IN_QUOTA); > if (IS_ERR(key)) { > rc = PTR_ERR(key); >- pr_err("Problem loading X.509 certificate %d\n", rc); >+ if (id != INTEGRITY_KEYRING_MACHINE) >+ pr_err("Problem loading X.509 certificate %d\n", rc); > } else { > pr_notice("Loaded X.509 cert '%s'\n", > key_ref_to_ptr(key)->description); >-- >2.43.0 >
On Fri, 2024-02-16 at 19:10 +0800, Coiby Xu wrote: > Hi Mimi, > > Could you take a look at this version of patch? If it escaped your > attention because it got buried in the same thread, sorry for that. And > I won't send new version as a reply to previous version in the future. Thanks for the reminder. Mimi
Hi Dmitry, Eric, James, Mimi, Paul, Serge, On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote: > Currently when the kernel fails to add a cert to the .machine keyring, > it will throw an error immediately in the function integrity_add_key. > > Since the kernel will try adding to the .platform keyring next or throw > an error (in the caller of integrity_add_key i.e. add_to_machine_keyring), > so there is no need to throw an error immediately in integrity_add_key. > > Reported-by: itrymybest80@protonmail.com > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331 > Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine") > Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com> > Signed-off-by: Coiby Xu <coxu@redhat.com> Any chance this patch can be merged? This is breaking (at least) Fedora at the moment. Thanks! Maxime
On Wed, 2024-03-06 at 11:57 +0100, Maxime Ripard wrote: > Hi Dmitry, Eric, James, Mimi, Paul, Serge, > > On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote: > > Currently when the kernel fails to add a cert to the .machine keyring, > > it will throw an error immediately in the function integrity_add_key. > > > > Since the kernel will try adding to the .platform keyring next or throw > > an error (in the caller of integrity_add_key i.e. add_to_machine_keyring), > > so there is no need to throw an error immediately in integrity_add_key. > > > > Reported-by: itrymybest80@protonmail.com > > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331 > > Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine") > > Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com> > > Signed-off-by: Coiby Xu <coxu@redhat.com> > > Any chance this patch can be merged? This is breaking (at least) Fedora > at the moment. https://git.kernel.org/torvalds/c/29cd507cbec282e13dcf8f38072a100af96b2bb7 Mimi
On Wed, Mar 06, 2024 at 06:55:00AM -0500, Mimi Zohar wrote: > On Wed, 2024-03-06 at 11:57 +0100, Maxime Ripard wrote: > > Hi Dmitry, Eric, James, Mimi, Paul, Serge, > > > > On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote: > > > Currently when the kernel fails to add a cert to the .machine keyring, > > > it will throw an error immediately in the function integrity_add_key. > > > > > > Since the kernel will try adding to the .platform keyring next or throw > > > an error (in the caller of integrity_add_key i.e. add_to_machine_keyring), > > > so there is no need to throw an error immediately in integrity_add_key. > > > > > > Reported-by: itrymybest80@protonmail.com > > > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331 > > > Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine") > > > Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com> > > > Signed-off-by: Coiby Xu <coxu@redhat.com> > > > > Any chance this patch can be merged? This is breaking (at least) Fedora > > at the moment. > > https://git.kernel.org/torvalds/c/29cd507cbec282e13dcf8f38072a100af96b2bb7 Oh, awesome, we missed it. Thanks! Maxime
On Wed, Mar 06, 2024 at 01:40:01PM +0100, Maxime Ripard wrote: >On Wed, Mar 06, 2024 at 06:55:00AM -0500, Mimi Zohar wrote: >> On Wed, 2024-03-06 at 11:57 +0100, Maxime Ripard wrote: >> > Hi Dmitry, Eric, James, Mimi, Paul, Serge, >> > >> > On Tue, Jan 09, 2024 at 08:24:28AM +0800, Coiby Xu wrote: >> > > Currently when the kernel fails to add a cert to the .machine keyring, >> > > it will throw an error immediately in the function integrity_add_key. >> > > >> > > Since the kernel will try adding to the .platform keyring next or throw >> > > an error (in the caller of integrity_add_key i.e. add_to_machine_keyring), >> > > so there is no need to throw an error immediately in integrity_add_key. >> > > >> > > Reported-by: itrymybest80@protonmail.com >> > > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331 >> > > Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine") >> > > Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com> >> > > Signed-off-by: Coiby Xu <coxu@redhat.com> >> > >> > Any chance this patch can be merged? This is breaking (at least) Fedora >> > at the moment. >> >> https://git.kernel.org/torvalds/c/29cd507cbec282e13dcf8f38072a100af96b2bb7 > >Oh, awesome, we missed it. Oh, I missed the emails about Mimi's PR sent to Linus as well. Btw, I'm curious to ask why you used the word "breaking" because I thought these KERN_ERR errors shouldn't cause any real problem.
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index df387de29bfa..45c3e5dda355 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -179,7 +179,8 @@ static int __init integrity_add_key(const unsigned int id, const void *data, KEY_ALLOC_NOT_IN_QUOTA); if (IS_ERR(key)) { rc = PTR_ERR(key); - pr_err("Problem loading X.509 certificate %d\n", rc); + if (id != INTEGRITY_KEYRING_MACHINE) + pr_err("Problem loading X.509 certificate %d\n", rc); } else { pr_notice("Loaded X.509 cert '%s'\n", key_ref_to_ptr(key)->description);
Currently when the kernel fails to add a cert to the .machine keyring, it will throw an error immediately in the function integrity_add_key. Since the kernel will try adding to the .platform keyring next or throw an error (in the caller of integrity_add_key i.e. add_to_machine_keyring), so there is no need to throw an error immediately in integrity_add_key. Reported-by: itrymybest80@protonmail.com Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331 Fixes: d19967764ba8 ("integrity: Introduce a Linux keyring called machine") Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com> Signed-off-by: Coiby Xu <coxu@redhat.com> --- v2 - improve patch subject [Mimi] - add Fixes tag [Jarkko] - add Reviewed-by tag from Eric --- security/integrity/digsig.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)