From:   Eric Snowberg <eric.snowberg@oracle.com>
To:     Mimi Zohar <zohar@linux.ibm.com>,
        Jarkko Sakkinen <jarkko@kernel.org>
CC:     "keyrings@vger.kernel.org" <keyrings@vger.kernel.org>,
        "linux-integrity@vger.kernel.org" <linux-integrity@vger.kernel.org>,
        "dhowells@redhat.com" <dhowells@redhat.com>,
        "dwmw2@infradead.org" <dwmw2@infradead.org>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "jmorris@namei.org" <jmorris@namei.org>,
        "serge@hallyn.com" <serge@hallyn.com>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "torvalds@linux-foundation.org" <torvalds@linux-foundation.org>,
        "weiyongjun1@huawei.com" <weiyongjun1@huawei.com>,
        "nayna@linux.ibm.com" <nayna@linux.ibm.com>,
        "ebiggers@google.com" <ebiggers@google.com>,
        "ardb@kernel.org" <ardb@kernel.org>,
        "nramas@linux.microsoft.com" <nramas@linux.microsoft.com>,
        "lszubowi@redhat.com" <lszubowi@redhat.com>,
        "jason@zx2c4.com" <jason@zx2c4.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
        "linux-security-module@vger.kernel.org" 
        <linux-security-module@vger.kernel.org>,
        "James.Bottomley@hansenpartnership.com" 
        <James.Bottomley@hansenpartnership.com>,
        "pjones@redhat.com" <pjones@redhat.com>,
        Konrad Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH v8 09/17] KEYS: Rename
 get_builtin_and_secondary_restriction
Thread-Topic: [PATCH v8 09/17] KEYS: Rename
 get_builtin_and_secondary_restriction
Thread-Index: AQHX4O2pDdgPDu39Wk2pZOJ3EdBmzqwWj2cAgAXMOQCAAR68AIAAN5SAgBZLdwA=
Date:   Wed, 15 Dec 2021 18:14:44 +0000
Message-ID: <31FE813A-91D5-46B8-9658-21F3F1418643@oracle.com>
References: <20211124044124.998170-1-eric.snowberg@oracle.com>
 <20211124044124.998170-10-eric.snowberg@oracle.com>
 <fb1d583f588e3f46fdadbe3cf6288bb098ff45f8.camel@kernel.org>
 <8906F8A4-313F-45E5-8ABD-A1A2D07BFD93@oracle.com> <YadOLrHb14MEfphi@iki.fi>
 <61f5d74f861ce1015831649d3bca9272a2e3b7bf.camel@linux.ibm.com>
In-Reply-To: <61f5d74f861ce1015831649d3bca9272a2e3b7bf.camel@linux.ibm.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?cgWnJK64xH0TWy9AkJFCcXOLCP8sek7F/1JddlYP82gQUF0jfQi2IGkScuBQ?=
 =?us-ascii?Q?xabto89yvy88D9ZfGmEkYf1E/tuw/nvD43XLNuNx/YQaVez73XD4KCJV/yPz?=
 =?us-ascii?Q?9IgUuhyOowhDKczZgPCOPoqsnGTG2/LBdVH4qff0CQpUCn6zOrWqOrrecbOh?=
 =?us-ascii?Q?OpWJDgnC6TPKtTfqKyuckvPoaXFUi4qTuKVQ+VXJUoJninI8lT3EyasR5YKU?=
 =?us-ascii?Q?5szAONSDv3RyJc5Mzj/xwZNQPcwIkEMfgVyr8/ziiqRbGOEGiueBpSBwBgrr?=
 =?us-ascii?Q?ZMhdWXK/5y7aPWILO419a6C66RXqf3Mx2IpZl98w3chNrDfKdjaKa8tAvaO3?=
 =?us-ascii?Q?WeOWZmdgvUUfbd1864Wmi5CnVoDE1xgVxPrcYCVhNOF/v30qVBVA7DIMsG0b?=
 =?us-ascii?Q?xCecfTLUSvbgHyAhM3ZnNNlxGgE2fPfTFFRSCEA6fH4+DgPJ8f7otnoZNxXE?=
 =?us-ascii?Q?HsBGGu5iiv8yRPE24h1Lvw8rXiiiCHZCEXBExl++TDn/5WLIyjdlBd5sZAA+?=
 =?us-ascii?Q?iqOG2ShqCMMJS8kuhBoArD5U7Rm0/8u6Kz4QX6GajTexl34Bo0YzZOZ0jtG1?=
 =?us-ascii?Q?jupOte/gMfNM53+s8acU6agR0asgndKhf7de0A//JwunVknbg2wY2xIla9sq?=
 =?us-ascii?Q?UVHAMcppz38Kv0ZhlrLCGY7vXP5C42agJnTBcnbPTxj1iUUA4eVMHgPST0kR?=
 =?us-ascii?Q?AQ9qVWw3gAB5Hw3SS7eJGzASo8RI6nSDMlSoXW0UunJgHP9hAYUp7Y/HHzfa?=
 =?us-ascii?Q?r7mJdsM+gToPr8XT7mSboTTvgIvDmb4jpREIuIcHWF25vnWs4UCJ8IgTJT/u?=
 =?us-ascii?Q?uhtqhuGgw7xX2YKYGtfLCaCYoTiqti/TjBJ3GXtE3Qx0d397uIb06c8y7PWl?=
 =?us-ascii?Q?arcSidUqrKVizjRbhdZ0h7bzBirUgBPYuY4seUkHQkJew4lUPOK94MCz7PKa?=
 =?us-ascii?Q?dnu43sggDUku6dj59Ha6UYh5JKtq2d6ACR101NfUpUbqmMH0EaMrI2CDwhpI?=
 =?us-ascii?Q?tco04lKH69NyDQfZIUiPRzsIgHHxic0RAgmFQxMgAALZJVV3DDB5YaB14Kj+?=
 =?us-ascii?Q?1S04va/1kHZGsmfk9pYc0iPSHiacKm4wZYrCbxa+4XpDu+rK5omNZ+2X9kGz?=
 =?us-ascii?Q?Pv8utcj6Vyko5j2ggQnwEidcwqJs1JTJQG0MoO6gUmsoEcNs8IZMMS5Pec0D?=
 =?us-ascii?Q?xlo62nS35+Yh9vNJOikyTwwvk3rjuCkfAKvrtSEDqmYLpihUXYSfYuflDik6?=
 =?us-ascii?Q?WN6KyPE178UBoowz8QD9RAeuhaTNhaapJ0rLGJlu2px+hMwAegkOJ8jlJH3l?=
 =?us-ascii?Q?vxnO3UULinM2Dsb+AOnFq+5+abgk9lSI3CH6sSsbVD04QrJevSK090WHAvYG?=
 =?us-ascii?Q?miytPESeFvUnU44ZyZyKaDNY1+JKniBEPlWGu/btv3GPcppHfq4ClFeHtmBM?=
 =?us-ascii?Q?G+7gQPNISgtYrixM89A9jctWzEawW3C95P0eRO1jySJe61KllxQu5dQ3Xltu?=
 =?us-ascii?Q?6W69GsjNHr37y47ZdicyowqaYD1wvU6Ewj12rSVeH8VRTvwWr0mDgdPEj0O5?=
 =?us-ascii?Q?Va/TkWbMpOPnCn7CxPBZob/1Bl607LpDf1nxkKz1uMN72rxFL6ddqsCjjKdQ?=
 =?us-ascii?Q?i8mwWHX00OQ/dVGdIOphrmpNhXsGl2OvCR33Zr7j75KGQgZU75ofSnsRQDgz?=
 =?us-ascii?Q?fjzEZhrZBdEnoowyHF/B8rraX1rBU3zpRbtfVZVi/e5fSH3C?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <BBD756BBB259AD418485FD83577B4CB6@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4150.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 99e81cd5-7d6b-413e-c459-08d9bff6c54e
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Dec 2021 18:14:44.1276
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: bXla887beIeLxpwcqj0LhDhTz6YIzXfKcqOFgrvR/xT+9PG1G+chlE9fbCJ1ECcfboWjHxr8StU35tcxQcD7vCNoy/70mmWtOFn+UOvOEJ8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB4827
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10199 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 bulkscore=0 spamscore=0
 mlxlogscore=999 suspectscore=0 adultscore=0 malwarescore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2112150101
X-Proofpoint-GUID: Ui4k1O4r0BhR4ly9aPHpzXQDYU4cPFJg
X-Proofpoint-ORIG-GUID: Ui4k1O4r0BhR4ly9aPHpzXQDYU4cPFJg
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org


> On Dec 1, 2021, at 6:46 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>=20
> On Wed, 2021-12-01 at 12:27 +0200, Jarkko Sakkinen wrote:
>> On Tue, Nov 30, 2021 at 05:21:45PM +0000, Eric Snowberg wrote:
>>>=20
>>>=20
>>>> On Nov 26, 2021, at 5:49 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote=
:
>>>>=20
>>>> On Tue, 2021-11-23 at 23:41 -0500, Eric Snowberg wrote:
>>>>> In preparation for returning either the existing
>>>>> restrict_link_by_builtin_and_secondary_trusted or the upcoming
>>>>> restriction that includes the trusted builtin, secondary and
>>>>> machine keys, to improve clarity, rename
>>>>> get_builtin_and_secondary_restriction to get_secondary_restriction.
>>>>>=20
>>>>> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>>>>> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
>>>>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>>>>> ---
>>>>> v6: Initial version
>>>>> v7: Unmodified from v7
>>>>> v8: Code unmodified from v7, added Mimi's Reviewed-by
>>>>> ---
>>>>> certs/system_keyring.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>=20
>>>>> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
>>>>> index 692365dee2bd..8f1f87579819 100644
>>>>> --- a/certs/system_keyring.c
>>>>> +++ b/certs/system_keyring.c
>>>>> @@ -77,7 +77,7 @@ int restrict_link_by_builtin_and_secondary_trusted(
>>>>>  * Allocate a struct key_restriction for the "builtin and secondary t=
rust"
>>>>>  * keyring. Only for use in system_trusted_keyring_init().
>>>>>  */
>>>>> -static __init struct key_restriction *get_builtin_and_secondary_rest=
riction(void)
>>>>> +static __init struct key_restriction *get_secondary_restriction(void=
)
>>>>> {
>>>>>        struct key_restriction *restriction;
>>>>>=20
>>>>> @@ -117,7 +117,7 @@ static __init int system_trusted_keyring_init(voi=
d)
>>>>>                               KEY_USR_VIEW | KEY_USR_READ | KEY_USR_S=
EARCH |
>>>>>                               KEY_USR_WRITE),
>>>>>                              KEY_ALLOC_NOT_IN_QUOTA,
>>>>> -                             get_builtin_and_secondary_restriction()=
,
>>>>> +                             get_secondary_restriction(),
>>>>>                              NULL);
>>>>>        if (IS_ERR(secondary_trusted_keys))
>>>>>                panic("Can't allocate secondary trusted keyring\n");
>>>>=20
>>>> This is wrong order.
>>>>=20
>>>> You should first do the changes that make the old name
>>>> obsolete and only after that have a patch that does the
>>>> rename. Unfortunately, this patch cannot possibly acked
>>>> with the current order.
>>>=20
>>> I can change the order, but I'm confused how this would work for a git =
bisect.=20
>>> If the rename happens afterwards, now two patches will always need to b=
e=20
>>> reverted instead of the possibility of one.  Is this your expectation?
>=20
> If the keyring name change is independent of any other changes, as
> Jarkko suggested, nothing would break.
>=20
>> I'd drop this patch altogether. Old name is a bit ugly but does it matte=
r
>> all that much?
>=20
> The name "get_builtin_and_secondary_restriction" implies trust based on
> keys in the ".builtin_trusted_keys" and ".secondary_trusted_keys"
> keyrings.  This patch set is extending that to include keys on the new
> ".machine" keyring, by linking it to the secondary keyring.  Is leaving
> the name unchanged really an option?
>=20
>>=20
>> You already 16 patches without this.
>=20
> Agreed, it's a lot.  In the past, I've asked Eric to see if some of
> them could be squashed.

The series grew based on requests added in each round.  How about
I drop IMA support from the next round?  This would eliminate nine patches
from the series (5, 6, 7, 9-14), leaving six patches to introduce and enabl=
e=20
the new machine keyring (3, 4, 8, 15-17).  The first two patches could=20
be taken today.  The only reason the first two are included is to get this =
series=20
through the kernel test robot.

This would allow the machine keyring to be used for module signing.  Afterw=
ards=20
I could introduce the CA restriction behind another Kconfig in a different =
series to=20
add back IMA support. Let me know if this would be a better approach.