From: Casey Schaufler <casey@schaufler-ca.com>
To: Stephen Smalley <sds@tycho.nsa.gov>,
casey.schaufler@intel.com, jmorris@namei.org,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org
Cc: keescook@chromium.org, john.johansen@canonical.com,
penguin-kernel@i-love.sakura.ne.jp, paul@paul-moore.com
Subject: Re: [PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context
Date: Mon, 27 Jan 2020 14:49:53 -0800 [thread overview]
Message-ID: <fa300128-0371-d588-17f0-525c6d118a13@schaufler-ca.com> (raw)
In-Reply-To: <6bd3e393-e1df-7117-d15a-81cb1946807b@tycho.nsa.gov>
On 1/24/2020 12:16 PM, Stephen Smalley wrote:
> On 1/24/20 2:28 PM, Casey Schaufler wrote:
>> On 1/24/2020 8:20 AM, Stephen Smalley wrote:
>>> On 1/24/20 9:42 AM, Stephen Smalley wrote:
>>>> On 1/23/20 7:23 PM, Casey Schaufler wrote:
>>>>> Add an entry /proc/.../attr/context which displays the full
>>>>> process security "context" in compound format:'
>>>>> lsm1\0value\0lsm2\0value\0...
>>>>> This entry is not writable.
>>>>>
>>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>>> Cc: linux-api@vger.kernel.org
>>>>
>>>> As previously discussed, there are issues with AppArmor's implementation of getprocattr() particularly around the trailing newline that dbus-daemon and perhaps others would like to see go away in any new interface. Hence, I don't think we should implement this new API using the existing getprocattr() hook lest it also be locked into the current behavior forever.
>>>
>>> Also, it would be good if whatever hook is introduced to support /proc/pid/attr/context could also be leveraged by the SO_PEERCONTEXT implementation in the future so that we are guaranteed a consistent result between the two interfaces, unlike the current situation for /proc/self/attr/current versus SO_PEERSEC.
>>
>> I don't believe that a new hook is necessary, and that introducing one
>> just to deal with a '\n' would be pedantic. We really have two rational
>> options. AppArmor could drop the '\n' from their "context". Or, we can
>> simply document that the /proc/pid/attr/context interface will trim any
>> trailing whitespace. I understand that this would be a break from the
>> notion that the LSM infrastructure does not constrain what a module uses
>> for its own data. On the other hand, we have been saying that "context"s
>> are strings, and ignoring trailing whitespace is usual behavior for
>> strings.
>
> Well, you can either introduce a new common underlying hook for use by /proc/pid/attr/context and SO_PEERCONTEXT to produce the string that is to be returned to userspace (in order to guarantee consistency in format and allowing them to be directly compared, which I think is what the dbus maintainers wanted),
The getprocattr() hooks are already required to provide a nul terminated string.
The behavior of /proc/self/attr/current with regard to trailing whitespace or
the terminating nul is left to the security module.
The behavior of /proc/self/attr/context is new, and as such it can be defined
to be reasonable and consistent.
> or you can modify every security module to provide that guarantee in its existing getprocattr and getpeersec* hook functions (SELinux already provides this guarantee; Smack and AppArmor produce slightly different results with respect to \0 and/or \n), or you can have the framework trim the values it gets from the security modules before composing them.
Changing "every security module" turns out to be trivial, really.
The security_getprocattr() interface doesn't change, the getprocattr
hooks get a bool argument indicating if newlines are allowed, the
AppArmor code changes to check the bool and security_getprocattr()
sets to bool specially for the "context" case.
The getpeersec() interface use for SO_PEERCONTEXT isn't any more difficult.
The security modules are free to include a terminating nul or not in
the existing use case, but a bool can tell them what the behavior must
be when we get to that interface.
It's not like there are all that many security modules to bring in line
at this point. Establishing sane rules of behavior is overdue. With the
current set of new modules sniffing at the door it really is time to
tighten these up.
> But you need to do one of those things before this interface gets merged upstream.
>
> Aside from the trailing newline and \0 issues, AppArmor also has a whitespace-separated (mode) field that may or may not be present in the contexts it presently returns, ala "/usr/sbin/cupsd (enforce)". Not sure what they want for the new interfaces.
Using '\0' as a field terminator addresses that issue. That's a
primary motivator.
next prev parent reply other threads:[~2020-01-27 22:49 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200124002306.3552-1-casey.ref@schaufler-ca.com>
2020-01-24 0:22 ` [PATCH v14 00/23] LSM: Module stacking for AppArmor Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 01/23] LSM: Infrastructure management of the sock security Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 02/23] LSM: Create and manage the lsmblob data structure Casey Schaufler
2020-01-24 14:21 ` Stephen Smalley
2020-01-24 0:22 ` [PATCH v14 03/23] LSM: Use lsmblob in security_audit_rule_match Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 04/23] LSM: Use lsmblob in security_kernel_act_as Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 05/23] net: Prepare UDS for security module stacking Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 06/23] Use lsmblob in security_secctx_to_secid Casey Schaufler
2020-01-24 14:29 ` Stephen Smalley
2020-01-24 0:22 ` [PATCH v14 07/23] LSM: Use lsmblob in security_secid_to_secctx Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 08/23] LSM: Use lsmblob in security_ipc_getsecid Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 09/23] LSM: Use lsmblob in security_task_getsecid Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 10/23] LSM: Use lsmblob in security_inode_getsecid Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 11/23] LSM: Use lsmblob in security_cred_getsecid Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 12/23] IMA: Change internal interfaces to use lsmblobs Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 13/23] LSM: Specify which LSM to display Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 14/23] LSM: Ensure the correct LSM context releaser Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 15/23] LSM: Use lsmcontext in security_secid_to_secctx Casey Schaufler
2020-01-24 0:22 ` [PATCH v14 16/23] LSM: Use lsmcontext in security_inode_getsecctx Casey Schaufler
2020-01-24 0:23 ` [PATCH v14 17/23] LSM: security_secid_to_secctx in netlink netfilter Casey Schaufler
2020-01-24 0:23 ` [PATCH v14 18/23] NET: Store LSM netlabel data in a lsmblob Casey Schaufler
2020-01-24 14:36 ` Stephen Smalley
2020-01-24 0:23 ` [PATCH v14 19/23] LSM: Verify LSM display sanity in binder Casey Schaufler
2020-01-24 0:23 ` [PATCH v14 20/23] Audit: Add subj_LSM fields when necessary Casey Schaufler
2020-01-24 0:23 ` [PATCH v14 21/23] Audit: Include object data for all security modules Casey Schaufler
2020-01-24 0:23 ` [PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context Casey Schaufler
2020-01-24 14:42 ` Stephen Smalley
2020-01-24 16:20 ` Stephen Smalley
2020-01-24 19:28 ` Casey Schaufler
2020-01-24 20:16 ` Stephen Smalley
2020-01-27 20:05 ` Simon McVittie
2020-02-03 20:54 ` John Johansen
2020-01-27 22:49 ` Casey Schaufler [this message]
2020-01-31 22:10 ` Casey Schaufler
2020-02-03 18:54 ` Stephen Smalley
2020-02-03 19:37 ` Stephen Smalley
2020-02-03 21:39 ` Casey Schaufler
2020-02-04 13:37 ` Stephen Smalley
2020-02-04 17:14 ` Casey Schaufler
2020-02-10 11:56 ` Simon McVittie
2020-02-10 13:25 ` Stephen Smalley
2020-02-10 14:55 ` Stephen Smalley
2020-02-10 18:32 ` Casey Schaufler
2020-02-10 19:00 ` John Johansen
2020-02-11 15:59 ` Stephen Smalley
2020-02-11 17:58 ` John Johansen
2020-02-11 18:35 ` Casey Schaufler
2020-02-11 19:11 ` John Johansen
2020-02-10 18:56 ` John Johansen
2020-02-03 21:02 ` John Johansen
2020-02-03 21:43 ` Casey Schaufler
2020-02-03 22:49 ` John Johansen
2020-02-03 20:59 ` John Johansen
2020-01-24 0:23 ` [PATCH v14 23/23] AppArmor: Remove the exclusive flag Casey Schaufler
2020-01-24 15:05 ` [PATCH v14 00/23] LSM: Module stacking for AppArmor Stephen Smalley
2020-01-24 21:04 ` Stephen Smalley
2020-01-24 21:49 ` Casey Schaufler
2020-01-27 16:14 ` Stephen Smalley
2020-01-27 16:56 ` KASAN slab-out-of-bounds in tun_chr_open/sock_init_data (Was: Re: [PATCH v14 00/23] LSM: Module stacking for AppArmor) Stephen Smalley
2020-01-27 17:34 ` Casey Schaufler
2020-01-27 17:16 ` [PATCH v14 00/23] LSM: Module stacking for AppArmor Casey Schaufler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fa300128-0371-d588-17f0-525c6d118a13@schaufler-ca.com \
--to=casey@schaufler-ca.com \
--cc=casey.schaufler@intel.com \
--cc=jmorris@namei.org \
--cc=john.johansen@canonical.com \
--cc=keescook@chromium.org \
--cc=linux-security-module@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=sds@tycho.nsa.gov \
--cc=selinux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).