From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BBB64C433DB for ; Wed, 17 Mar 2021 22:57:13 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id E827D64F07 for ; Wed, 17 Mar 2021 22:57:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E827D64F07 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=paul-moore.com Authentication-Results: mail.kernel.org; spf=tempfail smtp.mailfrom=linux-audit-bounces@redhat.com Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-28-LIjP7YlqPgeMMiWqszryBA-1; Wed, 17 Mar 2021 18:57:08 -0400 X-MC-Unique: LIjP7YlqPgeMMiWqszryBA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7439510082EE; Wed, 17 Mar 2021 22:57:04 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 760C25D9CA; Wed, 17 Mar 2021 22:57:03 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id D96751809C83; Wed, 17 Mar 2021 22:57:01 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 12HMuXOA006488 for ; Wed, 17 Mar 2021 18:56:34 -0400 Received: by smtp.corp.redhat.com (Postfix) id B8F6C2026987; Wed, 17 Mar 2021 22:56:33 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast04.extmail.prod.ext.rdu2.redhat.com [10.11.55.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B35542026D64 for ; Wed, 17 Mar 2021 22:56:31 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0C00F100DE77 for ; Wed, 17 Mar 2021 22:56:31 +0000 (UTC) Received: from mail-ej1-f48.google.com (mail-ej1-f48.google.com [209.85.218.48]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-535-QYWfLHQxMEiWrHM1q6_d0A-1; Wed, 17 Mar 2021 18:56:28 -0400 X-MC-Unique: QYWfLHQxMEiWrHM1q6_d0A-1 Received: by mail-ej1-f48.google.com with SMTP id k10so931552ejg.0 for ; Wed, 17 Mar 2021 15:56:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=a+08TCMXZyjlZrUSLxo3iiA6zUw9BGeYa2hlwFsECIY=; b=DzM8MUXR00xk8e7KOhH7o9BD6AWg4hoKu14rHFuPPNzCLtgEFH4O98+yGCMkJWU7xR 6MoJxvBtVIiSsMgou7KeOWXWu/NrKSa7hM3NNTH+jY038F5kprlnc/3+d43WqWKkLl1y 83Oj/evIGQ9/FUxeU0KsI6j5ufz7h4g33ZwwU9FH6X+E/oss5jgSKBP5HDFF1McJPFLT F4L73L8V+fwop/zzEqZ/PLJcNNQYSr9UOC2yQ4nLYtfSy5E038gXUrOfFU0AB89anhmu rQNX/wo31NWcM04WyzzpdkeCU2c4SodgDsY0w03vZ0+/OONqdLNmLnLZ9pnXgMgsO5EM jWQA== X-Gm-Message-State: AOAM531Z5QJ+1WN1wzovtKbr+Q0hABT3mOK7npa2z93eThJ4O5v2kbJo SxZbBs2eElQNQGx/KbE3pyHlcvdp0n3B1JBM0Zxa X-Google-Smtp-Source: ABdhPJwQ6DhvzGENYehzx0SNW4HCm0PCWmztvPNIkgcIJ3iZTziPQupBbExCf+lV7Fg3ZHzMlKrtUDUImmoF8nDBLek= X-Received: by 2002:a17:906:a443:: with SMTP id cb3mr37538189ejb.542.1616021787231; Wed, 17 Mar 2021 15:56:27 -0700 (PDT) MIME-Version: 1.0 References: <161377712068.87807.12246856567527156637.stgit@sifl> <161377735153.87807.7492842242100187888.stgit@sifl> In-Reply-To: From: Paul Moore Date: Wed, 17 Mar 2021 18:56:16 -0400 Message-ID: Subject: Re: [RFC PATCH 2/4] selinux: clarify task subjective and objective credentials To: John Johansen , Jeffrey Vander Stoep X-Mimecast-Impersonation-Protect: Policy=CLT - Impersonation Protection Definition; Similar Internal Domain=false; Similar Monitored External Domain=false; Custom External Domain=false; Mimecast External Domain=false; Newly Observed Domain=false; Internal User Name=false; Custom Display Name List=false; Reply-to Address Mismatch=false; Targeted Threat Dictionary=false; Mimecast Threat Dictionary=false; Custom Threat Dictionary=false X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-loop: linux-audit@redhat.com Cc: selinux@vger.kernel.org, linux-security-module@vger.kernel.org, linux-audit@redhat.com X-BeenThere: linux-audit@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Linux Audit Discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=linux-audit-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Wed, Mar 10, 2021 at 11:32 PM Paul Moore wrote: > On Tue, Mar 9, 2021 at 10:06 PM John Johansen > wrote: > > On 2/19/21 3:29 PM, Paul Moore wrote: ... > > > @@ -2034,11 +2047,8 @@ static inline u32 open_file_to_av(struct file *file) > > > > > > static int selinux_binder_set_context_mgr(struct task_struct *mgr) > > > { > > > - u32 mysid = current_sid(); > > > - u32 mgrsid = task_sid(mgr); > > > - > > > return avc_has_perm(&selinux_state, > > > - mysid, mgrsid, SECCLASS_BINDER, > > > + current_sid(), task_sid_obj(mgr), SECCLASS_BINDER, > > > BINDER__SET_CONTEXT_MGR, NULL); > > > } > > > > > > @@ -2046,8 +2056,8 @@ static int selinux_binder_transaction(struct task_struct *from, > > > struct task_struct *to) > > > { > > > u32 mysid = current_sid(); > > > - u32 fromsid = task_sid(from); > > > - u32 tosid = task_sid(to); > > > + u32 fromsid = task_sid_subj(from); > > > > fromsid potentially gets used as both the subject and the object the following > > permission checks. It makes sense to use the same cred for both checks but > > what I am not sure about yet is whether its actually safe to use the subject > > sid when the task isn't current. > > > > ie. I am still trying to determine if there is a race here between the transaction > > request and the permission check. > > Okay, I see what you are concerned about now ... and unfortunately I'm > not seeing a lot of precedence in the kernel for this type of usage > either; the closest I can find is something like task_lock(), but that > doesn't seem to cover the subjective creds. In fact, looking at > override_creds(), there is nothing preventing a task from changing > it's subjective creds at any point in time. > > Beyond the task_sid_subj() code here, looking back at patch 1 and the > use of security_task_getsecid_subj() we look to be mostly safe (where > safe means we are only inspecting the current task) with the exception > of the binder code once again. There are some other exceptions but > they are in the ptrace and audit code, both of which should be okay > given the nature and calling context of the code. > > The problem really does seem to be just binder, and as I look at > binder userspace example code, I'm starting to wonder if binder is > setup properly to operate sanely in a situation where a process > overrides its subject creds. It may be that we always need to use the > objective/real creds with binder. Jeff, any binder insight here you > can share with us? > > > > + u32 tosid = task_sid_subj(to); > > its not clear to me that using the subj for to is correct > > Yes, I believe you are correct. Jeff, I know you looked at this code > already, but I'm guessing you may have missed this (just as I did when > I wrote it); are you okay with changing 'tosid' in > selinux_binder_transaction() to the task's objective credentials? Hearing no comments from the Android/binder folks, I'm in the process of switching this patchset to always use the objective creds in the case of binder. It's safe and I'm not sure binder is really prepared for the idea of a task changing it's creds anyway. Once the kernel builds and passes some basic sanity checks I'll repost the patches for review and inclusion, minus the AppArmor patch. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit