Re: [PATCH 3/3] tpm: fix missing migratable flag in sealing functionality for TPM2
Hi
Other fixes are ready for the pull request but for this patch peer
check might be useful.
I'm anyway sending the pull request with the five pull patches over
here even if I don't get 'Tested-by:':
https://github.com/jsakkine/linux-tpmdd/commits/fixes
I've tested this patch with fTPM and dTPM and it does not have any
side-effects to TPM 1.2.
/Jarkko
On Thu, Nov 05, 2015 at 12:20:23PM +0200, Jarkko Sakkinen wrote:
> The 'migratable' flag was not added to the key payload. This patch
> fixes the problem.
>
> Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
> Signed-off-by: Jarkko Sakkinen
> ---
> drivers/char/tpm/tpm2-cmd.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index bd7039f..c121304 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -443,12 +443,13 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>TPM_DIGEST_SIZE);
>
> /* sensitive */
> - tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len);
> + tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
>
> tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
> tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
> - tpm_buf_append_u16(&buf, payload->key_len);
> + tpm_buf_append_u16(&buf, payload->key_len + 1);
> tpm_buf_append(&buf, payload->key, payload->key_len);
> + tpm_buf_append_u8(&buf, payload->migratable);
>
> /* public */
> tpm_buf_append_u16(&buf, 14);
> @@ -573,6 +574,8 @@ static int tpm2_unseal(struct tpm_chip *chip,
> u32 blob_handle)
> {
> struct tpm_buf buf;
> + u16 data_len;
> + u8 *data;
> int rc;
>
> rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
> @@ -591,11 +594,13 @@ static int tpm2_unseal(struct tpm_chip *chip,
> rc = -EPERM;
>
> if (!rc) {
> - payload->key_len = be16_to_cpup(
> + data_len = be16_to_cpup(
> (__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
> + data = &buf.data[TPM_HEADER_SIZE + 6];
>
> - memcpy(payload->key, &buf.data[TPM_HEADER_SIZE + 6],
> -payload->key_len);
> + memcpy(payload->key, data, data_len - 1);
> + payload->key_len = data_len - 1;
> + payload->migratable = data[data_len - 1];
> }
>
> tpm_buf_destroy(&buf);
> --
> 2.5.0
>
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()
Hello, Paul.
>> Did you want the "Reported-by" tag included? I'm also adding the SELinux
>> list
>> back to the CC line.
Yes, could you please, indeed include lost line "Reported-by: Florian Weimer
".
It was my fault of omitting it while re-composing commit message.
Vladis Dronov | Red Hat, Inc.
| Product Security Engineer |
- Original Message -
From: "Paul Moore"
To: "Vladis Dronov"
Cc: linux-security-module@vger.kernel.org, seli...@tycho.nsa.gov
Sent: Friday, November 6, 2015 6:33:39 PM
Subject: Re: [PATCH v2] selinux: rate-limit unrecognized netlink message
warnings in selinux_nlmsg_perm()
On Wednesday, November 04, 2015 04:02:36 PM Vladis Dronov wrote:
> Any process is able to send netlink messages with invalid types.
> Make the warning rate-limited to prevent too much log spam.
>
> The warning is supposed to help to find misbehaving programs, so
> print the triggering command name and pid.
>
> Signed-off-by: Vladis Dronov
> ---
> security/selinux/hooks.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
Did you want the "Reported-by" tag included? I'm also adding the SELinux list
back to the CC line.
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e4369d8..3d8087d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4787,11 +4787,12 @@ static int selinux_nlmsg_perm(struct sock *sk,
> struct sk_buff *skb) err = selinux_nlmsg_lookup(sksec->sclass,
> nlh->nlmsg_type, &perm); if (err) {
> if (err == -EINVAL) {
> - printk(KERN_WARNING
> -"SELinux: unrecognized netlink message:"
> -" protocol=%hu nlmsg_type=%hu sclass=%s\n",
> + pr_warn_ratelimited("SELinux: unrecognized netlink"
> +" message: protocol=%hu nlmsg_type=%hu sclass=%s"
> +" from %s[%d]\n",
> sk->sk_protocol, nlh->nlmsg_type,
> -secclass_map[sksec->sclass - 1].name);
> +secclass_map[sksec->sclass - 1].name,
> +current->comm, current->pid);
> if (!selinux_enforcing || security_get_allow_unknown())
> err = 0;
> }
--
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: LSM and sending socketpair created descriptors via unix sockets
Helo again. I need to get back to this issue, we still don't have a fix for socketpair() and UDS sockets. I was wondering if the fix i posted is ok (apart from the variables beeing named inode), as you can see this fields in the structure are initialized in smack_sk_alloc_security (to cover those sockets that won't be connected like those created by socketpair() ) and in smack_unix_stream_connect() for all other situations. Do you have some other ideas about this issue, maybe there could be a more generic way to fix this in smack ? I was also wondering about a situation were a descriptor to a socket would be passed around more then once from a process to process, in the end it could actually end up in the original sending process, should we somehow take into account the path such a descriptor would take in the process tree ? bes regards On 09/25/2015 09:09 PM, Casey Schaufler wrote: > On 9/23/2015 7:07 AM, Stephen Smalley wrote: >> On 09/23/2015 05:00 AM, Roman Kubiak wrote: >>> We found a lot of scenarios where the smack_file_receive() function might >>> fail >>> >>> 1) for pipes a different patch is needed for the function to work >>> (otherwise all pipe descriptors >>> get the "_" label), patch below: >>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c >>> index a143328..0ebfb07 100644 >>> --- a/security/smack/smack_lsm.c >>> +++ b/security/smack/smack_lsm.c >>> @@ -3099,6 +3099,9 @@ static void smack_d_instantiate(struct dentry >>> *opt_dentry, struct inode *inode) >>> */ >>> isp->smk_inode = smk_of_current(); >>> break; >>> + case PIPEFS_MAGIC: >>> + isp->smk_inode = smk_of_current(); >>> + break; >> We have the equivalent of this in SELinux already, by way of specifying >> fs_use_task for pipefs in policy and the SECURITY_FS_USE_TASK case in >> selinux inode_doinit_with_dentry(). > > This looks like a fix regardless of the file_receive issue. > >> >>> default: >>> isp->smk_inode = sbsp->smk_root; >>> break; >>> >>> 2) for sockets it's a complicated issue with connected/unconnected sockets, >>> also connected means they might be server/client sockets (those with >>> accept/listen and those with only connect), socketpair is another issue. I >>> tried to cover at least one case, where the descriptor is a connected UDS >>> socket (it has a peer), patch below: >> I don't understand the issue here; for SELinux, we just always call >> file_has_perm() in selinux_file_receive(), and it does not matter >> whether the open file refers to a file, a pipe, a socket, or whatever. > > There's a difference in how SELinux and Smack treat socket based > communications. I won't try to describe exactly how SELinux does > it, because I always get it wrong. Smack treats the communication > as a write operation from the sending process to the receiving > process. The information about Smack label of the receiving process > is stored in the socket structure (which is not itself an object in > the Smack model) and the check is made against that. The Smack label > attached to a UDS inode is not relevant. If the file structure for > a UDS lacks an inode it shouldn't matter at all to Smack. > > For file_receive of a UDS what matters is whether the receiving > process can write to the process at the other end. That has nothing > to do with the inode. > > >> If the open file refers to a socket, whether connected, unconnected, >> local, network, or whatever, that socket has an associated inode that >> was created when the socket was created (and that typically has the same >> label as the process that created the socket), and I can check whether >> the current process (the recipient) is allowed to read and write to >> sockets with that label. > > But Smack doesn't really care about the inode. The UDS file system > object is not interesting to Smack. > >> If you want to permit that, then you can allow it via a rule. If not, >> then don't. What's the problem? You don't need to replicate the actual >> checking that gets done on socket send at this point. You are just >> checking whether the recipient can use the socket at all, not whether it >> can talk to the peer. That should get mediated at send time. > > The check is to see if using the descriptor as is would violate > the system policy. If the receiving process does not have write > access to the process on the far end it shouldn't be using the > descriptor. Unless, of course, the intent is to pass access to > the receiving process that it shouldn't have. Fred being able > to write to Wilma and Betty shouldn't allow Fred to give Wilma > the right to write to Betty. > >> >>> diff --git a/security/smack/smack.h b/security/smack/smack.h >>> index 67ccb7b..7b1fbad 100644 >>> --- a/security/smack/smack.h >>> +++ b/security/smack/smack.
Re: [PATCH v4] keys, trusted: select hash algorithm for TPM2 chips
On Thu, 5 Nov 2015, Jarkko Sakkinen wrote: > v4: > > * Added missing select CRYPTO_HASH_INFO in drivers/char/tpm/Kconfig > > Signed-off-by: Jarkko Sakkinen Reviewed-by: James Morris -- James Morris -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] Allows reading back the current IMA policy;
On Mon, 2015-11-02 at 00:39 +0200, Petko Manolov wrote:
> +
> +#ifdef CONFIG_IMA_READ_POLICY
> +enum {
> + mask_err = -1,
> + mask_exec = 1, mask_write, mask_read, mask_append
> +};
> +
> +static match_table_t mask_tokens = {
> + {mask_exec, "MAY_EXEC"},
> + {mask_write, "MAY_WRITE"},
> + {mask_read, "MAY_READ"},
> + {mask_append, "MAY_APPEND"},
> + {mask_err, NULL}
> +};
> +
> +enum {
> + func_err = -1,
> + func_file = 1, func_mmap, func_bprm,
> + func_module, func_firmware, func_post
> +};
> +
> +static match_table_t func_tokens = {
> + {func_file, "FILE_CHECK"},
> + {func_mmap, "MMAP_CHECK"},
> + {func_bprm, "BPRM_CHECK"},
> + {func_module, "MODULE_CHECK"},
> + {func_firmware, "FIRMWARE_CHECK"},
> + {func_post, "POST_SETATTR"},
> + {func_err, NULL}
> +};
Why are we using match_table_t? Why not define an array of strings
which corresponds to the function hooks or use the __stringify macro?
static const char *ima_hooks_string[] = {"", "FILE_CHECK", "MMAP_CHECK",
"BPRM_CHECK", "MODULE_CHECK", "FIRMWARE_CHECK", "POST_SETATTR"};
In the first case, to display the function hook string would be
"ima_hooks_string[func]". Using __stringify requires the hook name (eg.
__stringify(FILE_CHECK)).
In either case, there would be a lot less code.
Mimi
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/11] Smack namespace
If I understand correctly the security window for 4.4 has been closed now (as changes went to next). Anyway, I updated the series to the latest smack-for-4.4 branch. Including the new relabel-self interface that received namespace treatment as well. Also the RCU fix reported on the list has been included. The latest version is available here: https://github.com/Havner/smack-namespace/tree/smack-namespace-current Also I've uploaded our Linux Test Project branch I use for Smack and Smack namespace testing (including regressions): https://github.com/Havner/ltp It has the basic smack tests rewritten to C. The ones that were scripts before. They are integrated with LTP framework. Inside testcases/kernel/security/smack/ns is a separate set of tests that share some common functions with the former, but are not otherwise integrated with LTP (yet). In this regard this is very much WIP. Those tests have an advantage though that they run a common set of tests in 6 Smack environments: no namespace, user namespace, user namespace + smack map. Each in a privileged and non-privileged scenario. To run them do the following: cd testcases/kernel/security/smack/ns make ./smack_ns_run.sh smackfs has to be mounted in /smack (following the regular tests). mount -o bind /sys/fs/smackfs /smack is enough. -- Lukasz Pawelczyk Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks
On Sun, 8 Nov 2015 13:08:36 +0100 Jann Horn wrote:
> By checking the effective credentials instead of the real UID /
> permitted capabilities, ensure that the calling process actually
> intended to use its credentials.
>
> To ensure that all ptrace checks use the correct caller
> credentials (e.g. in case out-of-tree code or newly added code
> omits the PTRACE_MODE_*CREDS flag), use two new flags and
> require one of them to be set.
>
> The problem was that when a privileged task had temporarily dropped
> its privileges, e.g. by calling setreuid(0, user_uid), with the
> intent to perform following syscalls with the credentials of
> a user, it still passed ptrace access checks that the user would
> not be able to pass.
>
> While an attacker should not be able to convince the privileged
> task to perform a ptrace() syscall, this is a problem because the
> ptrace access check is reused for things in procfs.
>
> In particular, the following somewhat interesting procfs entries
> only rely on ptrace access checks:
>
> /proc/$pid/stat - uses the check for determining whether pointers
> should be visible, useful for bypassing ASLR
> /proc/$pid/maps - also useful for bypassing ASLR
> /proc/$pid/cwd - useful for gaining access to restricted
> directories that contain files with lax permissions, e.g. in
> this scenario:
> lrwxrwxrwx root root /proc/13020/cwd -> /root/foobar
> drwx-- root root /root
> drwxr-xr-x root root /root/foobar
> -rw-r--r-- root root /root/foobar/secret
>
> Therefore, on a system where a root-owned mode 6755 binary
> changes its effective credentials as described and then dumps a
> user-specified file, this could be used by an attacker to reveal
> the memory layout of root's processes or reveal the contents of
> files he is not allowed to access (through /proc/$pid/cwd).
I'll await reviewer input on this one. Meanwhile, a bunch of
minor(ish) things...
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -395,7 +395,8 @@ static int do_task_stat(struct seq_file *m, struct
> pid_namespace *ns,
>
> state = *get_task_state(task);
> vsize = eip = esp = 0;
> - permitted = ptrace_may_access(task, PTRACE_MODE_READ |
> PTRACE_MODE_NOAUDIT);
> + permitted = ptrace_may_access(task,
> + PTRACE_MODE_READ | PTRACE_MODE_NOAUDIT | PTRACE_MODE_FSCREDS);
There's lots of ugliness in the patch to do with fitting code into 80 cols.
Can we do
#define PTRACE_foo (PTRACE_MODE_READ|PTRACE_MODE_FSCREDS)
to avoid all that?
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -57,7 +57,22 @@ extern void exit_ptrace(struct task_struct *tracer, struct
> list_head *dead);
> #define PTRACE_MODE_READ 0x01
> #define PTRACE_MODE_ATTACH 0x02
> #define PTRACE_MODE_NOAUDIT 0x04
> -/* Returns true on success, false on denial. */
> +#define PTRACE_MODE_FSCREDS 0x08
> +#define PTRACE_MODE_REALCREDS 0x10
> +/**
> + * ptrace_may_access - check whether the caller is permitted to access
> + * a target task.
> + * @task: target task
> + * @mode: selects type of access and caller credentials
> + *
> + * Returns true on success, false on denial.
> + *
> + * One of the flags PTRACE_MODE_FSCREDS and PTRACE_MODE_REALCREDS must
> + * be set in @mode to specify whether the access was requested through
> + * a filesystem syscall (should use effective capabilities and fsuid
> + * of the caller) or through an explicit syscall such as
> + * process_vm_writev or ptrace (and should use the real credentials).
> + */
> extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
It is unconventional to put the kernedoc in the header - people have
been trained to look for it in the .c file.
> +++ b/kernel/ptrace.c
> @@ -219,6 +219,13 @@ static int ptrace_has_cap(struct user_namespace *ns,
> unsigned int mode)
> static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> {
> const struct cred *cred = current_cred(), *tcred;
> + kuid_t caller_uid;
> + kgid_t caller_gid;
> +
> + if (!(mode & PTRACE_MODE_FSCREDS) != !(mode & PTRACE_MODE_REALCREDS)) {
So setting either one of these and not the other is an error. How
come?
> + WARN(1, "denying ptrace access check without
> PTRACE_MODE_*CREDS\n");
This warning cannot be triggered by malicious userspace, I trust?
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks
On Mon, Nov 09, 2015 at 12:55:54PM -0800, Andrew Morton wrote: > > --- a/fs/proc/array.c > > +++ b/fs/proc/array.c > > @@ -395,7 +395,8 @@ static int do_task_stat(struct seq_file *m, struct > > pid_namespace *ns, > > > > state = *get_task_state(task); > > vsize = eip = esp = 0; > > - permitted = ptrace_may_access(task, PTRACE_MODE_READ | > > PTRACE_MODE_NOAUDIT); > > + permitted = ptrace_may_access(task, > > + PTRACE_MODE_READ | PTRACE_MODE_NOAUDIT | PTRACE_MODE_FSCREDS); > > There's lots of ugliness in the patch to do with fitting code into 80 cols. > Can we do > > #define PTRACE_foo (PTRACE_MODE_READ|PTRACE_MODE_FSCREDS) > > to avoid all that? Or even simply bypass the 80-cols rule. Making code ugly or less easy to read for sake of an arbitrary rule is often not fun, and that's even more so when it comes to security fixes that people are expected to easily understand next time they put their fingers there. Willy -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks
On Mon, Nov 09, 2015 at 12:55:54PM -0800, Andrew Morton wrote:
> On Sun, 8 Nov 2015 13:08:36 +0100 Jann Horn wrote:
>
> > By checking the effective credentials instead of the real UID /
> > permitted capabilities, ensure that the calling process actually
> > intended to use its credentials.
> >
> > To ensure that all ptrace checks use the correct caller
> > credentials (e.g. in case out-of-tree code or newly added code
> > omits the PTRACE_MODE_*CREDS flag), use two new flags and
> > require one of them to be set.
> >
> > The problem was that when a privileged task had temporarily dropped
> > its privileges, e.g. by calling setreuid(0, user_uid), with the
> > intent to perform following syscalls with the credentials of
> > a user, it still passed ptrace access checks that the user would
> > not be able to pass.
> >
> > While an attacker should not be able to convince the privileged
> > task to perform a ptrace() syscall, this is a problem because the
> > ptrace access check is reused for things in procfs.
> >
> > In particular, the following somewhat interesting procfs entries
> > only rely on ptrace access checks:
> >
> > /proc/$pid/stat - uses the check for determining whether pointers
> > should be visible, useful for bypassing ASLR
> > /proc/$pid/maps - also useful for bypassing ASLR
> > /proc/$pid/cwd - useful for gaining access to restricted
> > directories that contain files with lax permissions, e.g. in
> > this scenario:
> > lrwxrwxrwx root root /proc/13020/cwd -> /root/foobar
> > drwx-- root root /root
> > drwxr-xr-x root root /root/foobar
> > -rw-r--r-- root root /root/foobar/secret
> >
> > Therefore, on a system where a root-owned mode 6755 binary
> > changes its effective credentials as described and then dumps a
> > user-specified file, this could be used by an attacker to reveal
> > the memory layout of root's processes or reveal the contents of
> > files he is not allowed to access (through /proc/$pid/cwd).
>
> I'll await reviewer input on this one. Meanwhile, a bunch of
> minor(ish) things...
>
> > --- a/fs/proc/array.c
> > +++ b/fs/proc/array.c
> > @@ -395,7 +395,8 @@ static int do_task_stat(struct seq_file *m, struct
> > pid_namespace *ns,
> >
> > state = *get_task_state(task);
> > vsize = eip = esp = 0;
> > - permitted = ptrace_may_access(task, PTRACE_MODE_READ |
> > PTRACE_MODE_NOAUDIT);
> > + permitted = ptrace_may_access(task,
> > + PTRACE_MODE_READ | PTRACE_MODE_NOAUDIT | PTRACE_MODE_FSCREDS);
>
> There's lots of ugliness in the patch to do with fitting code into 80 cols.
I agree.
> Can we do
>
> #define PTRACE_foo (PTRACE_MODE_READ|PTRACE_MODE_FSCREDS)
>
> to avoid all that?
Hm. All combinations of the PTRACE_MODE_*CREDS flags with
PTRACE_MODE_{READ,ATTACH} plus optionally PTRACE_MODE_NOAUDIT
make sense, I think. So your suggestion would be to create
four new #defines
PTRACE_MODE_{READ,ATTACH}_{FSCREDS,REALCREDS} and then let
callers OR in the PTRACE_MODE_NOAUDIT flag if needed?
> > --- a/include/linux/ptrace.h
> > +++ b/include/linux/ptrace.h
> > @@ -57,7 +57,22 @@ extern void exit_ptrace(struct task_struct *tracer,
> > struct list_head *dead);
> > #define PTRACE_MODE_READ 0x01
> > #define PTRACE_MODE_ATTACH 0x02
> > #define PTRACE_MODE_NOAUDIT0x04
> > -/* Returns true on success, false on denial. */
> > +#define PTRACE_MODE_FSCREDS 0x08
> > +#define PTRACE_MODE_REALCREDS 0x10
> > +/**
> > + * ptrace_may_access - check whether the caller is permitted to access
> > + * a target task.
> > + * @task: target task
> > + * @mode: selects type of access and caller credentials
> > + *
> > + * Returns true on success, false on denial.
> > + *
> > + * One of the flags PTRACE_MODE_FSCREDS and PTRACE_MODE_REALCREDS must
> > + * be set in @mode to specify whether the access was requested through
> > + * a filesystem syscall (should use effective capabilities and fsuid
> > + * of the caller) or through an explicit syscall such as
> > + * process_vm_writev or ptrace (and should use the real credentials).
> > + */
> > extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
>
> It is unconventional to put the kernedoc in the header - people have
> been trained to look for it in the .c file.
OK, will fix that. I thought it would be appropriate to put it in the
header since that one-line comment was already there.
> > +++ b/kernel/ptrace.c
> > @@ -219,6 +219,13 @@ static int ptrace_has_cap(struct user_namespace *ns,
> > unsigned int mode)
> > static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> > {
> > const struct cred *cred = current_cred(), *tcred;
> > + kuid_t caller_uid;
> > + kgid_t caller_gid;
> > +
> > + if (!(mode & PTRACE_MODE_FSCREDS) != !(mode & PTRACE_MODE_REALCREDS)) {
>
> So setting either one of these and not the other is an error. How
> come?
Oh. Sorry about that. I only added PTRACE_MODE_REALCREDS in this iteration
Re: [PATCH] ptrace: use fsuid, fsgid, effective creds for fs access checks
On Mon, 9 Nov 2015 22:12:09 +0100 Jann Horn wrote:
>
> > Can we do
> >
> > #define PTRACE_foo (PTRACE_MODE_READ|PTRACE_MODE_FSCREDS)
> >
> > to avoid all that?
>
> Hm. All combinations of the PTRACE_MODE_*CREDS flags with
> PTRACE_MODE_{READ,ATTACH} plus optionally PTRACE_MODE_NOAUDIT
> make sense, I think. So your suggestion would be to create
> four new #defines
> PTRACE_MODE_{READ,ATTACH}_{FSCREDS,REALCREDS} and then let
> callers OR in the PTRACE_MODE_NOAUDIT flag if needed?
If these flag combinations have an identifiable concept behind them then
sure, it makes sense to capture that via a well-chosen identifier.
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 03/11] lsm: add file opener's cred to a setprocattr arguments
On Wed, Oct 14, 2015 at 02:41:57PM +0200, Lukasz Pawelczyk wrote: > int (*getprocattr)(struct task_struct *p, char *name, char **value); > - int (*setprocattr)(struct task_struct *p, char *name, void *value, > - size_t size); > + int (*setprocattr)(struct task_struct *p, const struct cred *f_cred, > +char *name, void *value, size_t size); *grumble* Why the hell is that thing taking char *name - not even const char *? -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
