From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-8faa.mail.infomaniak.ch (smtp-8faa.mail.infomaniak.ch [83.166.143.170]) (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 E5DB11E515 for ; Mon, 29 Jul 2024 14:10:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=83.166.143.170 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722262210; cv=none; b=hdywU1FM0Id4mt/ejx6I7ILddC7ggusWZVXtK8FISfnheYi6TccBR6n2jFuVoouBoFibR6pI2f04QyHw7GF11FmSwvpF5JI6yQCJDmFAn26sG1e+81aEFd4urh8Oig7HZ1RWE/bCT2l7JTESsifTwOASOXhStak7QkJMfY7KK80= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722262210; c=relaxed/simple; bh=vyFLUUl3RqBsYrKATy7kglM3lc3UZ7rHLTHg7tKfzYY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=vB4k3GSg09zLIzBK7HJ+4Efnkc2uPbJOfQ41fn7wuZFw/0qU2w6WAAYjubWzNH+TQw0nzGGrJgxzuFvHIaAUMdt+SNYpeyuUprcU+qBbijoLb6be6JGn0/wA35Xv1whmuBx/wPevZyCqTfXPvNAi5hciA4tbK6Im2jbi37tD3LA= 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=JLaeGXNk; arc=none smtp.client-ip=83.166.143.170 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="JLaeGXNk" Received: from smtp-4-0001.mail.infomaniak.ch (smtp-4-0001.mail.infomaniak.ch [10.7.10.108]) by smtp-3-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 4WXgGv2Q5yzHBp; Mon, 29 Jul 2024 16:09:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=digikod.net; s=20191114; t=1722262199; bh=SIWtCB8Vnu1NtLQ2ovdqmSr4f6/uzaKar+Jlyb4V5Ws=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=JLaeGXNk5g2EqmYDsO5/GLJzLkp3Zqbq00iwlNsEmkVnVfRnCQI8FzHv2AarIn47/ roCNajCc+rcety/GEYRv+KxCpE7ZnH2b8ql9frpq2REcpoVO0ZOkcgmSn/Y4SydR4g 5vfH71J9TGVJmjAuYhXF1O3QMIMxDQzccIuaO8Ww= Received: from unknown by smtp-4-0001.mail.infomaniak.ch (Postfix) with ESMTPA id 4WXgGt2HdNzCSQ; Mon, 29 Jul 2024 16:09:58 +0200 (CEST) Date: Mon, 29 Jul 2024 16:09:56 +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.cho6saegoHei@digikod.net> References: <20240729125846.1043211-1-mic@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 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. So this change should be OK with Yama and most use cases. > > I think KEYCTL_SESSION_TO_PARENT is not a great design, but I'm not > sure if we can improve it much without risking some breakage. > I think this is a security issue that a process can change another process's credentials. If the main use cases is for shell commands, it should be OK. The alternative would be to restore the key_session_to_parent LSM hook [1], and update most LSMs to block this kind of credential tampering, which will lead to the same result but with only partial users being protected. [1] commit 3011a344cdcd ("security: remove dead hook key_session_to_parent")