From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-190f.mail.infomaniak.ch (smtp-190f.mail.infomaniak.ch [185.125.25.15]) (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 DCBF674E09 for ; Mon, 29 Jul 2024 15:02:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.125.25.15 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722265372; cv=none; b=oSc2VzcJjukg5ifw9+qTxBpy60SgdbUY97e2vzLSlkL9yRp/DL2MILL7qo6uNPvVd4TQ+oRCkT6+VnaEw5k7jof8e+qecs+LIYIgoLaDykuQHKi6HrT9V4dbPtTwCTztegvYN7wcuFWvx+2cT/QD0kQA0M478aYIV8mO0ubaC3s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722265372; c=relaxed/simple; bh=onBbiaEWw8YNvIODJmcayzpy+8J+PVNXsDSfrhQlNes=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Y7Hs9m9LLLnNxWj2NVwlQeAxPLuIQg+drULN39iqZaMzXCw8AADvrCIj/5VND8Wp070KWjp/nHFFWMQmaOtYTuClxabNkYmb5qSopsr1A7YYJnjtZ2YD8KwIHMf5AN+WY/BmY8Jr1gqsTZqnEnJXgvaz20rQWUpRqm1KsHgrhCU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=digikod.net; spf=pass smtp.mailfrom=digikod.net; dkim=pass (1024-bit key) header.d=digikod.net header.i=@digikod.net header.b=k/8V+Utu; arc=none smtp.client-ip=185.125.25.15 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=digikod.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=digikod.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=digikod.net header.i=@digikod.net header.b="k/8V+Utu" Received: from smtp-3-0000.mail.infomaniak.ch (smtp-3-0000.mail.infomaniak.ch [10.4.36.107]) by smtp-4-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 4WXhRm5vdzzWrC; Mon, 29 Jul 2024 17:02:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=digikod.net; s=20191114; t=1722265364; bh=9LGQN8Vc3Rs6TsiKBFEp9Lr0Cof6R+g+itZnhj1fmxs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=k/8V+UtuS28xmHYtqebOtZ1h+gw0saUyWy9nNjqNzdJQMGh2tqUYuDTa1kHMWYr0c dpnU0keYfYF7c3K2Y0A5MFAD/7rp9OTFKT9b8V/D8Gliaam664/PMqKrHMq0u1hxpY D26x6Kn+Sk7GNFUJD/vlEwpzYkDufHqKq6dhO0fs= Received: from unknown by smtp-3-0000.mail.infomaniak.ch (Postfix) with ESMTPA id 4WXhRm0dkYz6N6; Mon, 29 Jul 2024 17:02:44 +0200 (CEST) Date: Mon, 29 Jul 2024 17:02:41 +0200 From: =?utf-8?Q?Micka=C3=ABl_Sala=C3=BCn?= To: Jann Horn Cc: David Howells , Jarkko Sakkinen , =?utf-8?Q?G=C3=BCnther?= Noack , James Morris , Kees Cook , Paul Moore , keyrings@vger.kernel.org, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org Subject: Re: [PATCH v1] keys: Restrict KEYCTL_SESSION_TO_PARENT according to ptrace_may_access() Message-ID: <20240729.rayi3Chi9aef@digikod.net> References: <20240729125846.1043211-1-mic@digikod.net> <20240729.cho6saegoHei@digikod.net> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Infomaniak-Routing: alpha On Mon, Jul 29, 2024 at 04:21:01PM +0200, Jann Horn wrote: > On Mon, Jul 29, 2024 at 4:09 PM Mickaël Salaün wrote: > > On Mon, Jul 29, 2024 at 03:49:29PM +0200, Jann Horn wrote: > > > On Mon, Jul 29, 2024 at 2:59 PM Mickaël Salaün wrote: > > > > A process can modify its parent's credentials with > > > > KEYCTL_SESSION_TO_PARENT when their EUID and EGID are the same. This > > > > doesn't take into account all possible access controls. > > > > > > > > Enforce the same access checks as for impersonating a process. > > > > > > > > The current credentials checks are untouch because they check against > > > > EUID and EGID, whereas ptrace_may_access() checks against UID and GID. > > > > > > FWIW, my understanding is that the intended usecase of > > > KEYCTL_SESSION_TO_PARENT is that command-line tools (like "keyctl > > > new_session" and "e4crypt new_session") want to be able to change the > > > keyring of the parent process that spawned them (which I think is > > > usually a shell?); and Yama LSM, which I think is fairly widely used > > > at this point, by default prevents a child process from using > > > PTRACE_MODE_ATTACH on its parent. > > > > About Yama, the patched keyctl_session_to_parent() function already > > check if the current's and the parent's credentials are the same before > > this new ptrace_may_access() check. > > prepare_exec_creds() in execve() always creates new credentials which > are stored in bprm->cred and then later committed in begin_new_exec(). > Also, fork() always copies the credentials in copy_creds(). > So the "mycred == pcred" condition in keyctl_session_to_parent() > basically never applies, I think. > Also: When that condition is true, the whole operation is a no-op, > since if the credentials are the same, then the session keyring that > the credentials point to must also be the same. Correct, it's not a content comparison. We could compare the credential's data for this specific KEYCTL_SESSION_TO_PARENT call, I guess this should not be performance sensitive.