Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Neil Brown <neilb@suse.de>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] Documentation: Add an explanation of NFSv4 client
 identifiers
Thread-Topic: [PATCH] Documentation: Add an explanation of NFSv4 client
 identifiers
Thread-Index: AQHYTeNKNj3im2ccyUWrdlE9NE+XpKzr2+yAgACIEgCAAmwqgIAAu/AA
Date:   Thu, 14 Apr 2022 15:25:57 +0000
Message-ID: <DCBB40E7-3043-4B57-991F-D1439237C491@oracle.com>
References: <164970912423.2037.12497015321508491456.stgit@bazille.1015granger.net>
 <164974719723.11576.583440068909686735@noble.neil.brown.name>
 <4918188E-9271-47F2-8F5A-D6D5BEB85F36@oracle.com>
 <164990959799.11576.7740616159032491033@noble.neil.brown.name>
In-Reply-To: <164990959799.11576.7740616159032491033@noble.neil.brown.name>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?mcagAJMdCCH1hj8h2EHOEucwWBveRBeiSKRzSsmX2bUs2J/0FNoGuNoZlnAx?=
 =?us-ascii?Q?pviCISpSvGBh90q0cI09OrbBqJwYfzL8En1xo6q3wOX1bRDnQjOKC9qH8mIY?=
 =?us-ascii?Q?BsrBBXoDgiEgdeL6qlI6nPkxBXPbHeQUTkrjRoJ0cmFyGFq6yHplpwqcqYca?=
 =?us-ascii?Q?Y7zIR2X2TjgrMTgvUK8vP9NpaFbNJpJ+BSOYYw9n8E8LOBWZO/0UzlbmI3lx?=
 =?us-ascii?Q?u9RIzksndxWpc1mKU50+KnhjbOwzVSAQJH1Q+3jRhS1xQ6dFQoCJnmBYRu7h?=
 =?us-ascii?Q?KIPzzm8yczVdYv5yk6j9B4KRkALpnVwaf4NPipUx8OXOmsECkKNpwH2vczcb?=
 =?us-ascii?Q?pBv+GvZTDkQsXILJ/fLVTPi+TyBNfi/IqgVMbvNGdJCa3m4q2JbAMEgBhCwr?=
 =?us-ascii?Q?s4qzk4LcMOlxhwZzKP8kJipTqY9tr+sc60GYpnSjvUTYImQkjdCqF+7FeCio?=
 =?us-ascii?Q?f0NGTL0R9/dfNpuXFOT/LkQQ7+8BblaRSZHqujN4UJYosws7Xt+VihBkPnF+?=
 =?us-ascii?Q?ACyqMh6ixdXhKXEtUVEvSdFPx/kGPG3GrXwrV/e/mKSr3BZda7ZJAY+tH0KM?=
 =?us-ascii?Q?0PJ9pQtM5wVkM0sCpW2YVDPq+K8nCAAnRIjk+JSGlCr1Xam+4Lm/5jXbhsTb?=
 =?us-ascii?Q?U9dX1v89sKPptdX3mkkstrtpGAFBhX0iqT6Sd6pJ387k9ob3a1Dt6IBiIxf2?=
 =?us-ascii?Q?KdkrOBO8UK9TGaNtAJoJ72up2vTswLm+awEWNNMcXU3ivR0DpbkL0zS9Ktyo?=
 =?us-ascii?Q?7Z4cDQxDpga3MUVsVm0tO7Oz2z6y3p5kfVzXUczvXKXGW0vNpAYOz4rvrKRT?=
 =?us-ascii?Q?blTuaVhd3NZre1ZUwdALu+bI93CxrJjrVtD/UkwbSZA17BKJj+1PYe+0SRuc?=
 =?us-ascii?Q?ke56pw7ro8GRDsRCenDNLwdUgAsaAw8+UuH6tmDsDjKcJW+4dGXxIEXOfj+T?=
 =?us-ascii?Q?2S6Us6uNYSx0UuPJafUSorh9W/iiezy9vm+74jQ2y1JE59MHLQVL14WWzyub?=
 =?us-ascii?Q?i0W9Pg0+TJUMSo6dlFnGIT7birYtCpqPnrMIs0ACgarDDKYDI2bMLrx0Z8N0?=
 =?us-ascii?Q?jlSYfrSUf4B2kr//uD5Uwu0xwW6lBq0gXadRuw3L2tHrZ7t1sy8GRTVaxURw?=
 =?us-ascii?Q?O2NfaLbgr3ijhoUW83HQmY6xe5Z0iPGGPZaN9Qji0f6uQkvabwkFSbn5w247?=
 =?us-ascii?Q?yW/nd7Kb0MHUNptqEe6TOnryAZJBasId5OP52nsCStz4envIWQxDVsWmusmw?=
 =?us-ascii?Q?/hBcROWyHnsW4Ai3ItTnevkRS5Dk8NhmPA3x3flqHiLCjRXJbbMLlgwmIRQu?=
 =?us-ascii?Q?yfhRhOVVo2Qy7jgDIWisaBn03oqOs1F4AN8nAsRn9xb0VN/AVlw4yCsqtEfT?=
 =?us-ascii?Q?hkXQavlZiTFDzOJpbvEVHUdzB5z5IlfZNuL+wAHMkCiEYFrwT5iwH5NhH0OV?=
 =?us-ascii?Q?fU5x/aMSGbgzAkv/GD1SQ+W0gxv4tWKxoi6B3CXudM3yuPPzQTKlbaelaOO/?=
 =?us-ascii?Q?9YfO2rIoh+dKKzws/LXI1+k5sRX1W6G8w5PJpvnF4cfPsjngMi9vZXdQw0pE?=
 =?us-ascii?Q?LxN8HCX6iI5cAunXvODb/oOY1OM2XeVFFkx/2Kox1g99LfzezRhBUC40mUco?=
 =?us-ascii?Q?lIK2vZlU9dSXG2JVUP3h//uPRV9dd8J22VaeeGPxWSHXFzyW98BbFNefrib6?=
 =?us-ascii?Q?i9Pg8XedxynQPaNedkOBjb4QKXg1iDobx0JjR1Y8RDYiCbu1Wx7XMNdQAmdo?=
 =?us-ascii?Q?UW86ahvd+FjM4uoYUw+EHl/E+z9oC5A=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <014DA7E5A81BF943801B88A4894E7153@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: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: be96b089-15bf-4301-be96-08da1e2b12b7
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Apr 2022 15:25:57.1867
 (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: mpz0XGii7c6zV+JLxyBwxv0eWARDl9440gZz1u/0zDUeLMMn5V2Dnjin06jqQECw2CFqbfPHMtazxZGHT7IhOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB5533
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-14_04:2022-04-14,2022-04-14 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0 mlxscore=0
 bulkscore=0 phishscore=0 spamscore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204140082
X-Proofpoint-GUID: N-BFsjVZTK1yc7TbnGhembPfNt4G2Ba8
X-Proofpoint-ORIG-GUID: N-BFsjVZTK1yc7TbnGhembPfNt4G2Ba8
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Apr 14, 2022, at 12:13 AM, NeilBrown <neilb@suse.de> wrote:
>=20
> On Wed, 13 Apr 2022, Chuck Lever III wrote:
>>=20
>>> On Apr 12, 2022, at 3:06 AM, NeilBrown <neilb@suse.de> wrote:
>>>=20
>>> On Tue, 12 Apr 2022, Chuck Lever wrote:
>>>> +
>>>> +If a client's "co_ownerid" string or principal are not stable,
>>>> +state recovery after a server or client reboot is not guaranteed.
>>>> +If a client unexpectedly restarts but presents a different
>>>> +"co_ownerid" string or principal to the server, the server orphans
>>>> +the client's previous open and lock state. This blocks access to
>>>> +locked files until the server removes the orphaned state.
>>>> +
>>>> +If the server restarts and a client presents a changed "co_ownerid"
>>>> +string or principal to the server, the server will not allow the
>>>> +client to reclaim its open and lock state, and may give those locks
>>>> +to other clients in the mean time. This is referred to as "lock
>>>> +stealing".
>>>=20
>>> This is not a possible scenario with Linux NFS client.  The client
>>> assembles the string once from various sources, then uses it
>>> consistently at least until unmount or reboot.  Is it worth mentioning?
>>=20
>> Neil, thanks for the eyes-on. I've integrated the other suggestions
>> in your reply. However there are some corner cases here that I'd
>> like to consider before proceeding.
>>=20
>> Generally, preserving the cl_owner_id string is good defense against
>> lock stealing. Looks like the Linux NFS client didn't do that before
>> ceb3a16c070c ("NFSv4: Cache the NFSv4/v4.1 client owner_id in the
>> struct nfs_client").
>>=20
>> If a server filesystem is migrated to a server that the client hasn't
>> contacted before, and the client's uniquifier or hostname has changed
>> since the client established its lease with the first server, there
>> is the possibility of lock stealing during transparent state migration.
>>=20
>> I'm also not certain how the Linux NFS client preserves the principal
>> that was used when a lease is first established. It's going to use
>> Kerberos if possible, but what if the kernel's cred cache expires and
>> the keytab has been altered in the meantime? I haven't walked through
>> that code carefully enough to understand whether there is still a
>> vulnerability.
>>=20
>=20
> I don't think id stability relates to lock stealing.
>=20
> - global uniqueness guards against stealing
> - stability guards against misplacing locks during migration ("stolen"
> - seems an inappropriate term as no entity an be blamed for stealing).

An entity can be blamed: the client didn't adequately protect it's
locks during a migration or reboot recovery. That permits other
clients to lock those files instead.

The issue I was most concerned about describing here was principal
changes, not co_ownerid changes. But if either one changes, there
is going to be a potential problem.


> Certainly stability of both the identity and the credential are
> important.  If that stability is not provided then that is a kernel bug.
> As you say, ceb3a16c070c fixed a bug in this area.
> I don't think this document is an appropriate place to warn against
> kernel bugs - that doesn't fit with the purpose given in the intro.
>=20
> I don't know know if the credential can change either - I hope not.
> IF the credential can actually change, that would be a kernel bug.
> IF the same credential cannot be renewed, that is a separate problem,
> but should be treated like any other credential that cannot be renewed.
>=20
> I won't argue strongly against this text - I just don't see how it is
> appropriate and think it could be confusing.

Well, it seems like this document is perhaps warning against something
that is not likely but still a risk. The warning doesn't seem to cause
more harm -- rather it seems like keeping the principal and co_ownerid
string stable is perhaps a little too restrictive, but that isn't a
constraint that introduces more risk to data, IMO.

I think we can agree on the initial text for this document, and admit
that it might need some editing as we go along.


--
Chuck Lever