integrity: don't throw an error immediately when failed to add a cert to the .machine keyring
| Message ID | 20231227044156.166009-1-coxu@redhat.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel+bounces-11819-ouuuleilei=gmail.com@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id
tb2csp1242298dyb;
Tue, 26 Dec 2023 20:42:24 -0800 (PST)
X-Google-Smtp-Source:
AGHT+IHQz+bK1AYaVlKTOP+Lm43dTmXMmWF6bId0Gkm57159PPC20iUuYmA0omXDxAZ2HS2ITeo6
X-Received: by 2002:a05:620a:a14:b0:781:5c8c:8f2e with SMTP id
i20-20020a05620a0a1400b007815c8c8f2emr1174327qka.55.1703652143987;
Tue, 26 Dec 2023 20:42:23 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1703652143; cv=none;
d=google.com; s=arc-20160816;
b=P14fXx5oPQ4TxguwoWZOmKp6YpQfgDqX42oOUXzclfvd7QGYw9LHvRpZGobrITAkmq
KuqZ4MSS5dHIuykXanyct/Co3aBLxQ0bYzkA+6j/ZRUoTntPgf/iuQzaOgKhxC5S2kob
QM02+eayxjNTtyA0DXZ5s6bCthBoG/0N54EGpBotEdHPe8uF2xBE/gjI822vFSJgnYez
1Fi/Y9TUcuUjlNPyxFANMVO4JWcHfYLtCPxexrzJtZN6xSx5VWJipfOR+Ec/oQo2esdt
0ZA/wGIzMC1NauOcF4g9SZwTvrhI0A0Oe95GJkdD6iCh3tx0ty9Sh1GVyoiipuNLjl5I
MbJw==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=content-transfer-encoding:mime-version:list-unsubscribe
:list-subscribe:list-id:precedence:message-id:date:subject:cc:to
:from:dkim-signature;
bh=OFJIO0ZcDJiyym1avHSfZOIU1zn9ynuH2Tm3h86pSaQ=;
fh=UjlRlWLDH6Zmv4UqAvTO1R76ZG6OYOyyCA4BAaoJsGs=;
b=n+hdNzB84dXf3l+5EX2CF9TIxjRBRu2Vs1Il0o+UAHMxmCwTxxJhmMMf61jiFCNgwy
knkP5Jo9vr8IeIirrMo3ewLYsPyfzDqEugnwXUmNGRWQp+Hz1686zEb5xqNRY7XrKG4U
s4wu8sZrhJYEN0/3VKB9spqThJAKrmRFMrXrpTuEMdWjtUgmzYklqcyaNFUdTAUzci5N
XENRgqZmcCwq8GFXafBDLFilF0M/Aesst8BXxiLlq0aAXf0cKdgJjYj2lzlODTzl8B2e
3tmxIBGhFn3EtVVbvWIECIWN3bBv2laXsjTjBnAspxJLv+CSFoJpdwbR9csFIhyfAvvX
qi5A==
ARC-Authentication-Results: i=1; mx.google.com;
dkim=pass header.i=@redhat.com header.s=mimecast20190719
header.b="f7/Pgxvg";
spf=pass (google.com: domain of
linux-kernel+bounces-11819-ouuuleilei=gmail.com@vger.kernel.org designates
2604:1380:45d1:ec00::1 as permitted sender)
smtp.mailfrom="linux-kernel+bounces-11819-ouuuleilei=gmail.com@vger.kernel.org";
dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com
Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org.
[2604:1380:45d1:ec00::1])
by mx.google.com with ESMTPS id
u17-20020a05620a023100b0077fab06b88csi13779459qkm.619.2023.12.26.20.42.23
for <ouuuleilei@gmail.com>
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Tue, 26 Dec 2023 20:42:23 -0800 (PST)
Received-SPF: pass (google.com: domain of
linux-kernel+bounces-11819-ouuuleilei=gmail.com@vger.kernel.org designates
2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Authentication-Results: mx.google.com;
dkim=pass header.i=@redhat.com header.s=mimecast20190719
header.b="f7/Pgxvg";
spf=pass (google.com: domain of
linux-kernel+bounces-11819-ouuuleilei=gmail.com@vger.kernel.org designates
2604:1380:45d1:ec00::1 as permitted sender)
smtp.mailfrom="linux-kernel+bounces-11819-ouuuleilei=gmail.com@vger.kernel.org";
dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com
Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org
[52.25.139.140])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by ny.mirrors.kernel.org (Postfix) with ESMTPS id C54A71C221EA
for <ouuuleilei@gmail.com>; Wed, 27 Dec 2023 04:42:23 +0000 (UTC)
Received: from localhost.localdomain (localhost.localdomain [127.0.0.1])
by smtp.subspace.kernel.org (Postfix) with ESMTP id DFBBE5666;
Wed, 27 Dec 2023 04:42:08 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org;
dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com
header.b="f7/Pgxvg"
X-Original-To: linux-kernel@vger.kernel.org
Received: from us-smtp-delivery-124.mimecast.com
(us-smtp-delivery-124.mimecast.com [170.10.133.124])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by smtp.subspace.kernel.org (Postfix) with ESMTPS id D1F815228
for <linux-kernel@vger.kernel.org>; Wed, 27 Dec 2023 04:42:04 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org;
dmarc=pass (p=none dis=none) header.from=redhat.com
Authentication-Results: smtp.subspace.kernel.org;
spf=pass smtp.mailfrom=redhat.com
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
s=mimecast20190719; t=1703652123;
h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
to:to:cc:cc:mime-version:mime-version:
content-transfer-encoding:content-transfer-encoding;
bh=OFJIO0ZcDJiyym1avHSfZOIU1zn9ynuH2Tm3h86pSaQ=;
b=f7/PgxvgU8i4dlWFxsJ0lGvr/wD8aMKJvMnqpilPrwFvErLhkV4mUUHmaO9ol1FgSSLNou
AIeQ5M1IVmHWDDgj8Cvuo4RXShTUzHz5mwWwvYKbHNfbqKLgQumrpm94BWazMRf8u5ZUiY
C93SzoxWYwNwiFjUg27qJCrS3dloBIk=
Received: from mail-pf1-f198.google.com (mail-pf1-f198.google.com
[209.85.210.198]) by relay.mimecast.com with ESMTP with STARTTLS
(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id
us-mta-564-CDNGwJgyNHa8e48H49E-iQ-1; Tue, 26 Dec 2023 23:42:01 -0500
X-MC-Unique: CDNGwJgyNHa8e48H49E-iQ-1
Received: by mail-pf1-f198.google.com with SMTP id
d2e1a72fcca58-6d9b8ff5643so410287b3a.1
for <linux-kernel@vger.kernel.org>;
Tue, 26 Dec 2023 20:42:01 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1703652120; x=1704256920;
h=content-transfer-encoding:mime-version:message-id:date:subject:cc
:to:from:x-gm-message-state:from:to:cc:subject:date:message-id
:reply-to;
bh=OFJIO0ZcDJiyym1avHSfZOIU1zn9ynuH2Tm3h86pSaQ=;
b=hKz1niykGYURbIWoNzhsoFOB6GJZgN7sbAZGxGUKgTzy2tLDnrJio/MWixE0RI2eZB
w2+kON5acm7nTkYU+BHCc/BvVhcvRxg/b4OIe1q1PkJ48dn0ioyzhDZu+oUZsyvIGRCY
OjnNGAj4OuMSYryS/uSTkOah3k4EkA+z93LuTjpfPvIX34ChE6p9r88aJ73wfkBJnL74
AgAU8GC4lvtnO/McylRgeKw/NAl+RYxDUfHte/C7+kAi2vNM0UhWGEzwt1hP5EyooZbi
AjJFn49MWf1uIdCRvDSwSh50XgC4+y1EfebZYP2d/tjpzf6ad+tatdLsQHOPhJMqE0Id
Pp9Q==
X-Gm-Message-State: AOJu0Yz+i00/cCasPfD9FQAmC5o3YgeUBU+lncJv0UTdO/zbi2q44TTb
3dZ39bc21M7MOa0DGQgFtbazenLLvVJMcqKlbcVKJphj6wIzhkxK8l/4zaYVPF+unzN6l8y5UKu
ml5x2O5L/C/0Gdi/bcAo0QnqNtsMOrkjd
X-Received: by 2002:a62:ed06:0:b0:6d9:663a:aba1 with SMTP id
u6-20020a62ed06000000b006d9663aaba1mr2684497pfh.43.1703652119955;
Tue, 26 Dec 2023 20:41:59 -0800 (PST)
X-Received: by 2002:a62:ed06:0:b0:6d9:663a:aba1 with SMTP id
u6-20020a62ed06000000b006d9663aaba1mr2684488pfh.43.1703652119543;
Tue, 26 Dec 2023 20:41:59 -0800 (PST)
Received: from localhost ([43.228.180.230])
by smtp.gmail.com with ESMTPSA id
a8-20020a656548000000b005cdf90c21ecsm6929316pgw.67.2023.12.26.20.41.58
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Tue, 26 Dec 2023 20:41:59 -0800 (PST)
From: Coiby Xu <coxu@redhat.com>
To: linux-integrity@vger.kernel.org
Cc: itrymybest80@protonmail.com,
Mimi Zohar <zohar@linux.ibm.com>,
Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
Paul Moore <paul@paul-moore.com>,
James Morris <jmorris@namei.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
linux-security-module@vger.kernel.org (open list:SECURITY SUBSYSTEM),
linux-kernel@vger.kernel.org (open list)
Subject: [PATCH] integrity: don't throw an error immediately when failed to
add a cert to the .machine keyring
Date: Wed, 27 Dec 2023 12:41:56 +0800
Message-ID: <20231227044156.166009-1-coxu@redhat.com>
X-Mailer: git-send-email 2.43.0
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-getmail-retrieved-from-mailbox: INBOX
X-GMAIL-THRID: 1786408750607705651
X-GMAIL-MSGID: 1786408750607705651
|
| Series |
integrity: don't throw an error immediately when failed to add a cert to the .machine keyring
|
|
Commit Message
Coiby Xu
Dec. 27, 2023, 4:41 a.m. UTC
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
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
security/integrity/digsig.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
> On Dec 26, 2023, at 9:41 PM, Coiby Xu <coxu@redhat.com> 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 > Signed-off-by: Coiby Xu <coxu@redhat.com> Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com>
Hi Coiby, According to https://docs.kernel.org/process/submitting-patches.html,the summary line should be no more than 70 - 75 characters. On Wed, 2023-12-27 at 12:41 +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 > Signed-off-by: Coiby Xu <coxu@redhat.com> Otherwise, the patch looks good.
On Wed Dec 27, 2023 at 6:41 AM EET, 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 Missing "Firstname Lastname". > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2239331 > Signed-off-by: Coiby Xu <coxu@redhat.com> > --- > 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); BR, Jarkko
On Wed, Jan 03, 2024 at 04:09:29PM +0200, Jarkko Sakkinen wrote: >On Wed Dec 27, 2023 at 6:41 AM EET, 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 > >Missing "Firstname Lastname". Thanks for raising this concern! I've asked the reporter if he/she can share his/her name.
On Tue, Jan 02, 2024 at 12:54:02PM -0500, Mimi Zohar wrote: >Hi Coiby, Hi Mimi, > >According to https://docs.kernel.org/process/submitting-patches.html,the summary line should be no more than 70 - 75 characters. Thanks for pointing me to this limit! How about integrity: eliminate harmless error "Problem loading X.509 certificate -126"? > >On Wed, 2023-12-27 at 12:41 +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 >> Signed-off-by: Coiby Xu <coxu@redhat.com> > >Otherwise, the patch looks good. Thanks for reviewing the patch! > >-- >thanks, > >Mim > > >
On Tue, Jan 02, 2024 at 05:33:53PM +0000, Eric Snowberg wrote: > > >> On Dec 26, 2023, at 9:41 PM, Coiby Xu <coxu@redhat.com> 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 >> Signed-off-by: Coiby Xu <coxu@redhat.com> > >Reviewed-by: Eric Snowberg <eric.snowberg@oracle.com> Thank you for reviewing the patch!
On Fri, 2024-01-05 at 21:27 +0800, Coiby Xu wrote: > On Tue, Jan 02, 2024 at 12:54:02PM -0500, Mimi Zohar wrote: > >Hi Coiby, > > Hi Mimi, > > > > >According to https://docs.kernel.org/process/submitting-patches.html,the > summary line should be no more than 70 - 75 characters. > > Thanks for pointing me to this limit! How about > integrity: eliminate harmless error "Problem loading X.509 certificate -126" Still >75. How about the following? integrity: eliminate unnecessary "Problem loading X.509 certificate" msg Mimi > > > > >On Wed, 2023-12-27 at 12:41 +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 > >> Signed-off-by: Coiby Xu <coxu@redhat.com> > > > >Otherwise, the patch looks good. > > Thanks for reviewing the patch!
On Fri Jan 5, 2024 at 3:20 PM EET, Coiby Xu wrote: > On Wed, Jan 03, 2024 at 04:09:29PM +0200, Jarkko Sakkinen wrote: > >On Wed Dec 27, 2023 at 6:41 AM EET, 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 > > > >Missing "Firstname Lastname". > > Thanks for raising this concern! I've asked the reporter if he/she can > share his/her name. Also, it is lacking fixes tag. Fixes tag is mandatory, name part would be super nice to have :-) Since this categories as a bug fix, getting them in is 1st priority and that thus does not absolutely block applying the change. Thanks for going trouble trying to query it, however. BR, Jarkko
On Fri, Jan 05, 2024 at 06:02:38PM +0200, Jarkko Sakkinen wrote: >On Fri Jan 5, 2024 at 3:20 PM EET, Coiby Xu wrote: >> On Wed, Jan 03, 2024 at 04:09:29PM +0200, Jarkko Sakkinen wrote: >> >On Wed Dec 27, 2023 at 6:41 AM EET, 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 >> > >> >Missing "Firstname Lastname". >> >> Thanks for raising this concern! I've asked the reporter if he/she can >> share his/her name. > >Also, it is lacking fixes tag. Thanks for catching this issue! I've included the Fixes tag in v2. > >Fixes tag is mandatory, name part would be super nice to have :-) Since >this categories as a bug fix, getting them in is 1st priority and that >thus does not absolutely block applying the change. Thanks for going >trouble trying to query it, however. Thanks for the explanation! As I still get no reply from the reporter, so I guess we have to accept the name part for now. > >BR, Jarkko >
On Fri, Jan 05, 2024 at 09:59:14AM -0500, Mimi Zohar wrote: >On Fri, 2024-01-05 at 21:27 +0800, Coiby Xu wrote: >> On Tue, Jan 02, 2024 at 12:54:02PM -0500, Mimi Zohar wrote: >> >Hi Coiby, >> >> Hi Mimi, >> >> > >> >According to https://docs.kernel.org/process/submitting-patches.html,the >> summary line should be no more than 70 - 75 characters. >> >> Thanks for pointing me to this limit! How about >> integrity: eliminate harmless error "Problem loading X.509 certificate -126" > >Still >75. How about the following? > >integrity: eliminate unnecessary "Problem loading X.509 certificate" msg Thanks, v2 now uses the above subject. I thought the limit applies to the "summary phrase" instead of the whole "summary" and a second look proved me wrong.
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);