Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring
> Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893: > > Author: Petko Manolov > Date: Wed Dec 2 17:47:55 2015 +0200 > IMA: create machine owner and blacklist keyrings > If you need this applied to a tree, please state which. -- 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
[GIT PULL] keys bugfix
This is a resend of just the first (critical) fix. Please pull. The following changes since commit 8db7b3c54401d83a4dc370a59b8692854000ea03: Merge branch 'parisc-4.4-4' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux (2015-12-25 13:19:50 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus2 David Howells (1): KEYS: Fix race between read and revoke security/keys/keyctl.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) --- commit b4a1b4f5047e4f54e194681125c74c0aa64d637d Author: David Howells Date: Fri Dec 18 01:34:26 2015 + KEYS: Fix race between read and revoke This fixes CVE-2015-7550. There's a race between keyctl_read() and keyctl_revoke(). If the revoke happens between keyctl_read() checking the validity of a key and the key's semaphore being taken, then the key type read method will see a revoked key. This causes a problem for the user-defined key type because it assumes in its read method that there will always be a payload in a non-revoked key and doesn't check for a NULL pointer. Fix this by making keyctl_read() check the validity of a key after taking semaphore instead of before. I think the bug was introduced with the original keyrings code. This was discovered by a multithreaded test program generated by syzkaller (http://github.com/google/syzkaller). Here's a cleaned up version: #include #include #include void *thr0(void *arg) { key_serial_t key = (unsigned long)arg; keyctl_revoke(key); return 0; } void *thr1(void *arg) { key_serial_t key = (unsigned long)arg; char buffer[16]; keyctl_read(key, buffer, 16); return 0; } int main() { key_serial_t key = add_key("user", "%", "foo", 3, KEY_SPEC_USER_KEYRING); pthread_t th[5]; pthread_create(&th[0], 0, thr0, (void *)(unsigned long)key); pthread_create(&th[1], 0, thr1, (void *)(unsigned long)key); pthread_create(&th[2], 0, thr0, (void *)(unsigned long)key); pthread_create(&th[3], 0, thr1, (void *)(unsigned long)key); pthread_join(th[0], 0); pthread_join(th[1], 0); pthread_join(th[2], 0); pthread_join(th[3], 0); return 0; } Build as: cc -o keyctl-race keyctl-race.c -lkeyutils -lpthread Run as: while keyctl-race; do :; done as it may need several iterations to crash the kernel. The crash can be summarised as: BUG: unable to handle kernel NULL pointer dereference at 0010 IP: [] user_read+0x56/0xa3 ... Call Trace: [] keyctl_read_key+0xb6/0xd7 [] SyS_keyctl+0x83/0xe0 [] entry_SYSCALL_64_fastpath+0x12/0x6f Reported-by: Dmitry Vyukov Signed-off-by: David Howells Tested-by: Dmitry Vyukov Cc: sta...@vger.kernel.org Signed-off-by: James Morris diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index fb111ea..1c3872a 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -751,16 +751,16 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) /* the key is probably readable - now try to read it */ can_read_key: - ret = key_validate(key); - if (ret == 0) { - ret = -EOPNOTSUPP; - if (key->type->read) { - /* read the data with the semaphore held (since we -* might sleep) */ - down_read(&key->sem); + ret = -EOPNOTSUPP; + if (key->type->read) { + /* Read the data with the semaphore held (since we might sleep) +* to protect against the key being updated or revoked. +*/ + down_read(&key->sem); + ret = key_validate(key); + if (ret == 0) ret = key->type->read(key, buffer, buflen); - up_read(&key->sem); - } + up_read(&key->sem); } error2: -- 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: [GIT PULL] linux-integrity changes for 4.5
On Mon, 21 Dec 2015, Mimi Zohar wrote: > Hi James, > > Lots of changes this time. This pull request adds support, by Dmitry > Kasatkin, for: making the EVM keyring a trusted keyring, such that only > keys signed by a key on the system keyring can be loaded onto the EVM > keyring, loading the EVM keys onto the EVM trusted keyring by the > kernel, enabling EVM when either the x509 or symmetric keys are > available and loading the EVM symmetric key from hardware. > > As described by Mark Baushke and Petko Manalov at LSS 2015 in their talk > "IMA/EVM: Real Applications for Embedded Networking Systems", this pull > request includes support for two new IMA trusted keyrings named .ima_mok > and .ima_blacklist. Keys being loaded on either the EVM or IMA trusted > keyrings can be validated against either the system trusted keyring or > the intermediary .ima_mok keyring and prevented from being loaded if on > the .ima_blacklist keyring. > > Lastly, support for extending and displaying the IMA policy. > Applied. -- 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: [PULL] Smack - Changes for 4.5
On Tue, 22 Dec 2015, Casey Schaufler wrote: > The following changes since commit ebd68df3f24b318d391d15c458d6f43f340ba36a: > > Sync to Linus v4.4-rc2 for LSM developers. (2015-11-23 22:46:28 +1100) > > are available in the git repository at: > > https://github.com/cschaufler/smack-next.git smack-for-4.5 > > for you to fetch changes up to 81bd0d56298f93af6ac233d8a7e8b29aa4b094b7: > > Smack: type confusion in smak sendmsg() handler (2015-12-17 10:21:56 -0800) > > > Casey Schaufler (1): > Smack: File receive for sockets > > Roman Kubiak (1): > Smack: type confusion in smak sendmsg() handler > > security/smack/smack_lsm.c | 24 +++- > 1 file changed, 23 insertions(+), 1 deletion(-) > Applied. -- 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: [GIT PULL] tpmdd updates for Linux 4.5
On Mon, 21 Dec 2015, Jarkko Sakkinen wrote: > Hi > > Here are tpmdd updates for Linux 4.5. Sorry I didn't send this already > last week but I had to hold until I get ack from Peter and Mimi before > doing anything. Patches are quite well baked for a while now with the > exception of small fix from Stefan to tpm_ibmvtpm, which I considered > trivial enough to be included. > Applied. -- 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: [GIT PULL] SELinux patches for 4.5
On Thu, 24 Dec 2015, Paul Moore wrote: > Hi James, > > Nine patches for v4.5; there are a handful of minor fixes (constify > parameters, warning rate-limits, etc.) but there are a couple of significant > patches that invalidate/revalidate inode labels (needed for gfs2) and make > validate_trans decisions visible via selinuxfs. All the patches pass the > selinux-testsuite and have been included in the pcmoore/kernel-secnext Fedora > COPR repository[1] for some time now, all looks good. > > As of about five minutes ago, selinux#upstream applied cleanly on top of > linux-security#next so I don't expect you should have any problems merging > the > code. Applied. -- 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: [GIT PULL] Keys fixes
On Fri, 18 Dec 2015, Linus Torvalds wrote: > So there is no way in hell I am pulling this. > Sorry for the confusion. Please pull the first patch from here: The following changes since commit 73796d8bf27372e26c2b79881947304c14c2d353: Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-12-17 14:05:22 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus2 David Howells (1): KEYS: Fix race between read and revoke --- security/keys/keyctl.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) commit b4a1b4f5047e4f54e194681125c74c0aa64d637d Author: David Howells Date: Fri Dec 18 01:34:26 2015 + KEYS: Fix race between read and revoke This fixes CVE-2015-7550. There's a race between keyctl_read() and keyctl_revoke(). If the revoke happens between keyctl_read() checking the validity of a key and the key's semaphore being taken, then the key type read method will see a revoked key. This causes a problem for the user-defined key type because it assumes in its read method that there will always be a payload in a non-revoked key and doesn't check for a NULL pointer. Fix this by making keyctl_read() check the validity of a key after taking semaphore instead of before. I think the bug was introduced with the original keyrings code. This was discovered by a multithreaded test program generated by syzkaller (http://github.com/google/syzkaller). Here's a cleaned up version: #include #include #include void *thr0(void *arg) { key_serial_t key = (unsigned long)arg; keyctl_revoke(key); return 0; } void *thr1(void *arg) { key_serial_t key = (unsigned long)arg; char buffer[16]; keyctl_read(key, buffer, 16); return 0; } int main() { key_serial_t key = add_key("user", "%", "foo", 3, KEY_SPEC_USER_KEYRING); pthread_t th[5]; pthread_create(&th[0], 0, thr0, (void *)(unsigned long)key); pthread_create(&th[1], 0, thr1, (void *)(unsigned long)key); pthread_create(&th[2], 0, thr0, (void *)(unsigned long)key); pthread_create(&th[3], 0, thr1, (void *)(unsigned long)key); pthread_join(th[0], 0); pthread_join(th[1], 0); pthread_join(th[2], 0); pthread_join(th[3], 0); return 0; } Build as: cc -o keyctl-race keyctl-race.c -lkeyutils -lpthread Run as: while keyctl-race; do :; done as it may need several iterations to crash the kernel. The crash can be summarised as: BUG: unable to handle kernel NULL pointer dereference at 0010 IP: [] user_read+0x56/0xa3 ... Call Trace: [] keyctl_read_key+0xb6/0xd7 [] SyS_keyctl+0x83/0xe0 [] entry_SYSCALL_64_fastpath+0x12/0x6f Reported-by: Dmitry Vyukov Signed-off-by: David Howells Tested-by: Dmitry Vyukov Cc: sta...@vger.kernel.org Signed-off-by: James Morris diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index fb111ea..1c3872a 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -751,16 +751,16 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen) /* the key is probably readable - now try to read it */ can_read_key: - ret = key_validate(key); - if (ret == 0) { - ret = -EOPNOTSUPP; - if (key->type->read) { - /* read the data with the semaphore held (since we -* might sleep) */ - down_read(&key->sem); + ret = -EOPNOTSUPP; + if (key->type->read) { + /* Read the data with the semaphore held (since we might sleep) +* to protect against the key being updated or revoked. +*/ + down_read(&key->sem); + ret = key_validate(key); + if (ret == 0) ret = key->type->read(key, buffer, buflen); - up_read(&key->sem); - } + up_read(&key->sem); } error2: -- 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
[GIT PULL] Keys fixes
Please pull these fixes for the keys subsystem, including a fix for CVE-2015-7550. The following changes since commit 73796d8bf27372e26c2b79881947304c14c2d353: Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-12-17 14:05:22 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus David Howells (7): KEYS: Fix race between read and revoke X.509: Fix determination of self-signedness X.509: Fix leap year handling again Handle leap seconds in mktime64() X.509: Support leap seconds Handle both ISO 8601 encodings of midnight in mktime64() X.509: Handle midnight alternative notation in GeneralizedTime crypto/asymmetric_keys/x509_cert_parser.c | 24 +--- crypto/asymmetric_keys/x509_public_key.c | 11 --- include/linux/time.h | 13 ++--- kernel/time/time.c| 19 +++ security/keys/keyctl.c| 18 +- 5 files changed, 55 insertions(+), 30 deletions(-) -- 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 2/2] keys, trusted: seal with a policy
On Mon, 7 Dec 2015, Jarkko Sakkinen wrote: > On Fri, Nov 20, 2015 at 01:34:35PM +1100, James Morris wrote: > > On Wed, 18 Nov 2015, Jarkko Sakkinen wrote: > > > > > On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote: > > > > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote: > > > > > > > > > } > > > > > break; > > > > > + case Opt_policydigest: > > > > > + if (!tpm2 || > > > > > + strlen(args[0].from) != (2 * > > > > > opt->digest_len)) > > > > > + return -EINVAL; > > > > > + kfree(opt->policydigest); > > > > > + opt->policydigest = kzalloc(opt->digest_len, > > > > > + GFP_KERNEL); > > > > > > > > Is it correct to kfree opt->policydigest here before allocating it? > > > > > > I think so. The same option might be encountered multiple times. > > > > This would surely signify an error? > > I'm following the semantics of other options. That's why I implemented > it that way for example: > > keyctl add trusted kmk "new 32 keyhandle=0x8000 keyhandle=0x8000" > > is perfectly OK. I just thought that it'd be more odd if this option > behaved in a different way... It seems broken to me -- if you're messing up keyctl commands you might want to know about it, but we should remain consistent. -- 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
[GIT PULL] security: updated fixes for 4.4
Please pull this updated set, which now includes a fix for SELinux policy processing (regression introduced by fa1aa143ac4a), as well as the previously posted fix for the user-triggerable oops in the Keys code. --- The following changes since commit 6ffeba9607343f15303a399bc402a538800d89d9: Merge tag 'dm-4.4-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm (2015-11-24 12:53:11 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus2 David Howells (1): KEYS: Fix handling of stored error in a negatively instantiated user key James Morris (1): Merge branch 'upstream' of git://git.infradead.org/users/pcmoore/selinux into for-linus2 Stephen Smalley (1): selinux: fix bug in conditional rules handling security/keys/encrypted-keys/encrypted.c |2 ++ security/keys/trusted.c |5 - security/keys/user_defined.c |5 - security/selinux/ss/conditional.c|4 ++-- 4 files changed, 12 insertions(+), 4 deletions(-) -- 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
[GIT PULL] security: KEYS: Fix handling of stored error in a negatively instantiated user key
Please pull this fix for the keys subsystem, for 4.4, from David Howells. Note: this oops is triggerable by non-privileged users. The following changes since commit 6ffeba9607343f15303a399bc402a538800d89d9: Merge tag 'dm-4.4-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm (2015-11-24 12:53:11 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus David Howells (1): KEYS: Fix handling of stored error in a negatively instantiated user key security/keys/encrypted-keys/encrypted.c |2 ++ security/keys/trusted.c |5 - security/keys/user_defined.c |5 - 3 files changed, 10 insertions(+), 2 deletions(-) --- commit 096fe9eaea40a17e125569f9e657e34cdb6d73bd Author: David Howells Date: Tue Nov 24 21:36:31 2015 + KEYS: Fix handling of stored error in a negatively instantiated user key If a user key gets negatively instantiated, an error code is cached in the payload area. A negatively instantiated key may be then be positively instantiated by updating it with valid data. However, the ->update key type method must be aware that the error code may be there. The following may be used to trigger the bug in the user key type: keyctl request2 user user "" @u keyctl add user user "a" @u which manifests itself as: BUG: unable to handle kernel paging request at ff8a IP: [] __call_rcu.constprop.76+0x1f/0x280 kernel/rcu/tree.c:3046 PGD 7cc30067 PUD 0 Oops: 0002 [#1] SMP Modules linked in: CPU: 3 PID: 2644 Comm: a.out Not tainted 4.3.0+ #49 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 task: 88003ddea700 ti: 88003dd88000 task.ti: 88003dd88000 RIP: 0010:[] [] __call_rcu.constprop.76+0x1f/0x280 [] __call_rcu.constprop.76+0x1f/0x280 kernel/rcu/tree.c:3046 RSP: 0018:88003dd8bdb0 EFLAGS: 00010246 RAX: ff82 RBX: RCX: 0001 RDX: 81e3fe40 RSI: RDI: ff82 RBP: 88003dd8bde0 R08: 88007d2d2da0 R09: R10: R11: 88003e8073c0 R12: ff82 R13: 88003dd8be68 R14: 88007d027600 R15: 88003ddea700 FS: 00b92880(0063) GS:88007fd0() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: ff8a CR3: 7cc5f000 CR4: 06e0 Stack: 88003dd8bdf0 81160a8a ff82 88003dd8be68 88007d027600 88003dd8bdf0 810a39e5 88003dd8be20 812a31ab 88007d027600 88007d027620 Call Trace: [] kfree_call_rcu+0x15/0x20 kernel/rcu/tree.c:3136 [] user_update+0x8b/0xb0 security/keys/user_defined.c:129 [< inline >] __key_update security/keys/key.c:730 [] key_create_or_update+0x291/0x440 security/keys/key.c:908 [< inline >] SYSC_add_key security/keys/keyctl.c:125 [] SyS_add_key+0x101/0x1e0 security/keys/keyctl.c:60 [] entry_SYSCALL_64_fastpath+0x12/0x6a arch/x86/entry/entry_64.S:185 Note the error code (-ENOKEY) in EDX. A similar bug can be tripped by: keyctl request2 trusted user "" @u keyctl add trusted user "a" @u This should also affect encrypted keys - but that has to be correctly parameterised or it will fail with EINVAL before getting to the bit that will crashes. Reported-by: Dmitry Vyukov Signed-off-by: David Howells Acked-by: Mimi Zohar Signed-off-by: James Morris diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c index 927db9f..696ccfa 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -845,6 +845,8 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep) size_t datalen = prep->datalen; int ret = 0; + if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) + return -ENOKEY; if (datalen <= 0 || datalen > 32767 || !prep->data) return -EINVAL; diff --git a/security/keys/trusted.c b/security/keys/trusted.c index 903dace..16dec53 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -1007,13 +1007,16 @@ static void trusted_rcu_free(struct rcu_head *rcu) */ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) { - struct trusted_key_payload *p = key->payload.data[0]; + struct trusted_key_payload *
Re: [PATCH] KEYS: Fix handling of stored error in a negatively instantiated user key
On Tue, 24 Nov 2015, David Howells wrote: > Hi James, > > Can this be passed straight to Linus please? Is this triggerable by normal users? -- 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
Security next tree synced to v4.4-rc2
For LSM developers who might be waiting for a resync to Linus... -- 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 2/2] keys, trusted: seal with a policy
On Wed, 18 Nov 2015, Jarkko Sakkinen wrote: > On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote: > > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote: > > > > > } > > > break; > > > + case Opt_policydigest: > > > + if (!tpm2 || > > > + strlen(args[0].from) != (2 * opt->digest_len)) > > > + return -EINVAL; > > > + kfree(opt->policydigest); > > > + opt->policydigest = kzalloc(opt->digest_len, > > > + GFP_KERNEL); > > > > Is it correct to kfree opt->policydigest here before allocating it? > > I think so. The same option might be encountered multiple times. This would surely signify an error? -- 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 v3 0/7] User namespace mount updates
On Wed, 18 Nov 2015, Richard Weinberger wrote: > On Wed, Nov 18, 2015 at 4:13 PM, Al Viro wrote: > > On Wed, Nov 18, 2015 at 09:05:12AM -0600, Seth Forshee wrote: > > > >> Yes, the host admin. I'm not talking about trusting the admin inside the > >> container at all. > > > > Then why not have the same host admin just plain mount it when setting the > > container up and be done with that? From the host namespace, before > > spawning > > the docker instance or whatever framework you are using. IDGI... > > Because hosting companies sell containers as "full virtual machines" > and customers expect to be able mount stuff like disk images they upload. I don't think this is a valid reason for merging functionality into the kernel. -- 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 2/2] keys, trusted: seal with a policy
On Tue, 17 Nov 2015, Jarkko Sakkinen wrote: > } > break; > + case Opt_policydigest: > + if (!tpm2 || > + strlen(args[0].from) != (2 * opt->digest_len)) > + return -EINVAL; > + kfree(opt->policydigest); > + opt->policydigest = kzalloc(opt->digest_len, > + GFP_KERNEL); Is it correct to kfree opt->policydigest here before allocating it? > + if (!opt->policydigest) > + return -ENOMEM; > + res = hex2bin(opt->policydigest, args[0].from, > + opt->digest_len); > + if (res < 0) > + return -EINVAL; Do you need to kfree it here on error? -- 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 v3 7/7] Smack: Handle labels consistently in untrusted mounts
On Tue, 17 Nov 2015, Seth Forshee wrote: > + sbsp = inode->i_sb->s_security; > + if ((sbsp->smk_flags & SMK_SB_UNTRUSTED) && Where is SMK_SB_UNTRUSTED defined? I can't see it in this patch series, mainline or security next. -- 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 v3 6/7] userns: Replace in_userns with current_in_userns
On Tue, 17 Nov 2015, Seth Forshee wrote: > All current callers of in_userns pass current_user_ns as the > first argument. Simplify by replacing in_userns with > current_in_userns which checks whether current_user_ns is in the > namespace supplied as an argument. > > Signed-off-by: Seth Forshee Nice cleanup. Acked-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 v3 5/7] selinux: Add support for unprivileged mounts from user namespaces
On Tue, 17 Nov 2015, Seth Forshee wrote: > Security labels from unprivileged mounts in user namespaces must > be ignored. Force superblocks from user namespaces whose labeling > behavior is to use xattrs to use mountpoint labeling instead. > For the mountpoint label, default to converting the current task > context into a form suitable for file objects, but also allow the > policy writer to specify a different label through policy > transition rules. > > Pieced together from code snippets provided by Stephen Smalley. > > Signed-off-by: Seth Forshee > Acked-by: Stephen Smalley Acked-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 v3 4/7] fs: Treat foreign mounts as nosuid
On Tue, 17 Nov 2015, Seth Forshee wrote: > From: Andy Lutomirski > > If a process gets access to a mount from a different user > namespace, that process should not be able to take advantage of > setuid files or selinux entrypoints from that filesystem. Prevent > this by treating mounts from other mount namespaces and those not > owned by current_user_ns() or an ancestor as nosuid. > > This will make it safer to allow more complex filesystems to be > mounted in non-root user namespaces. > > This does not remove the need for MNT_LOCK_NOSUID. The setuid, > setgid, and file capability bits can no longer be abused if code in > a user namespace were to clear nosuid on an untrusted filesystem, > but this patch, by itself, is insufficient to protect the system > from abuse of files that, when execed, would increase MAC privilege. > > As a more concrete explanation, any task that can manipulate a > vfsmount associated with a given user namespace already has > capabilities in that namespace and all of its descendents. If they > can cause a malicious setuid, setgid, or file-caps executable to > appear in that mount, then that executable will only allow them to > elevate privileges in exactly the set of namespaces in which they > are already privileges. > > On the other hand, if they can cause a malicious executable to > appear with a dangerous MAC label, running it could change the > caller's security context in a way that should not have been > possible, even inside the namespace in which the task is confined. > > As a hardening measure, this would have made CVE-2014-5207 much > more difficult to exploit. > > Signed-off-by: Andy Lutomirski > Signed-off-by: Seth Forshee Acked-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 2/2] security/capability.h: cap_issubset/isclear can be boolean
On Tue, 17 Nov 2015, Yaowei Bai wrote: > This patch makes cap_issubset/isclear return bool due to these > functions only using either one or zero as their return > value. > > No functional change. > > Signed-off-by: Yaowei Bai Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next -- 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 1/2] security: remove unused cap_is_fs_cap function
On Tue, 17 Nov 2015, Yaowei Bai wrote: > Since commit 3bc1fa8a ("LSM: remove BSD secure level security module") > there is no user of cap_is_fs_cap any more, so remove it. > > Signed-off-by: Yaowei Bai Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next -- 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
[GIT PULL] Security subsystem bugfixes for 4.4
This includes several fixes for TPM, as well as a fix for the x.509 certificate parser to address CVE-2015-5327. Please pull. --- The following changes since commit 5d50ac70fe98518dbf620bfba8184254663125eb: Merge tag 'xfs-for-linus-4.4' of git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs (2015-11-11 20:18:48 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus Christophe JAILLET (1): TPM: Avoid reference to potentially freed memory David Howells (1): X.509: Fix the time validation [ver #2] James Morris (1): Merge tag 'tpmdd-next-20151110' of https://github.com/jsakkine/linux-tpmdd into for-linus Jarkko Sakkinen (3): TPM: revert the list handling logic fixed in 398a1e7 tpm: fix missing migratable flag in sealing functionality for TPM2 tpm: fix compat 'ppi' link handling in tpm_chip_register() Martin Wilck (2): tpm_tis: free irq after probing tpm_tis: restore IRQ vector in IO memory after failed probing crypto/asymmetric_keys/x509_cert_parser.c | 12 +++- drivers/char/tpm/tpm-chip.c | 20 +++- drivers/char/tpm/tpm2-cmd.c | 15 ++- drivers/char/tpm/tpm_of.c |3 ++- drivers/char/tpm/tpm_tis.c|8 +++- 5 files changed, 37 insertions(+), 21 deletions(-) -- 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] X.509: Fix the time validation
On Wed, 11 Nov 2015, David Howells wrote: > This fixes CVE-2015-5327. It affects kernels from 4.3-rc1 onwards. This doesn't apply to current Linus, please fix and resend. -- 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 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
[GIT PULL] Security subsystem update for 4.4
Please pull. This is mostly maintenance updates across the subsystem, with a notable update for TPM 2.0, and addition of Jarkko Sakkinen as a maintainer of that. The following changes since commit 5062ecdb662bf3aed6dc975019c53ffcd3b01d1c: Merge tag 'regmap-v4.4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap (2015-11-02 16:16:24 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next Arnd Bergmann (1): apparmor: clarify CRYPTO dependency David Howells (3): KEYS: Provide a script to extract the sys cert list from a vmlinux file KEYS: Provide a script to extract a module signature KEYS: Merge the type-specific data with the payload data Dmitry Kasatkin (1): integrity: prevent loading untrusted certificates on the IMA trusted keyring Geert Uytterhoeven (1): tpm: Allow compile test of GPIO consumers if !GPIOLIB Geliang Tang (3): smack: smk_ipv6_port_list should be static KEYS: use kvfree() in add_key selinux: ioctl_has_perm should be static Hon Ching \(Vicky\) Lo (6): vTPM: fix memory allocation flag for rtce buffer at kernel boot vTPM: fix searching for the right vTPM node in device tree vTPM: reformat event log to be byte-aligned vTPM: get the buffer allocated for event log instead of the actual log vTPM: support little endian guests TPM: remove unnecessary little endian conversion Insu Yun (1): keys: Be more consistent in selection of union members used James Morris (4): Merge branch 'next' of git://git.kernel.org/.../zohar/linux-integrity into next Merge branch 'smack-for-4.4' of https://github.com/cschaufler/smack-next into next Merge branch 'upstream' of git://git.infradead.org/users/pcmoore/selinux into next Merge tag 'keys-next-20151021' of git://git.kernel.org/.../dhowells/linux-fs into next Jarkko Sakkinen (10): tpm, tpm_crb: fix unaligned read of the command buffer address tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0 sysfs: added __compat_only_sysfs_link_entry_to_kobj() tpm: move the PPI attributes to character device directory. tpm: update PPI documentation to address the location change. tpm: introduce tpm_buf keys, trusted: move struct trusted_key_options to trusted-type.h tpm: seal/unseal for TPM 2.0 keys, trusted: seal/unseal with TPM 2.0 chips MAINTAINERS: add new maintainer for TPM DEVICE DRIVER Jeff Vander Stoep (1): selinux: do not check open perm on ftruncate call José Bollo (1): Smack: Minor initialisation improvement Krzysztof Kozlowski (1): char: Drop owner assignment from i2c_driver Lukasz Pawelczyk (1): Smack: fix a NULL dereference in wrong smack_import_entry() usage Paul Gortmaker (1): certs: add .gitignore to stop git nagging about x509_certificate_list Paul Moore (1): selinux: change CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE default Rasmus Villemoes (5): selinux: introduce security_context_str_to_sid selinux: remove pointless cast in selinux_inode_setsecurity() selinux: use kmemdup in security_sid_to_context_core() selinux: use kstrdup() in security_get_bools() selinux: use sprintf return value Roman Kubiak (1): Smack: pipefs fix in smack_d_instantiate Sangwoo (1): selinux: Use a kmem_cache for allocation struct file_security_struct Zbigniew Jasinski (1): Smack: limited capability for changing process label Documentation/ABI/testing/sysfs-driver-ppi | 19 +- Documentation/crypto/asymmetric-keys.txt | 27 ++-- Documentation/security/Smack.txt | 10 + Documentation/security/keys.txt | 41 +++-- MAINTAINERS |1 + arch/powerpc/kernel/prom_init.c | 40 +++- certs/.gitignore |4 + crypto/asymmetric_keys/asymmetric_keys.h |5 - crypto/asymmetric_keys/asymmetric_type.c | 44 +++-- crypto/asymmetric_keys/public_key.c |4 +- crypto/asymmetric_keys/signature.c |2 +- crypto/asymmetric_keys/x509_parser.h |1 + crypto/asymmetric_keys/x509_public_key.c |9 +- drivers/char/tpm/st33zp24/Kconfig|2 +- drivers/char/tpm/st33zp24/i2c.c |1 - drivers/char/tpm/tpm-chip.c | 24 ++- drivers/char/tpm/tpm-interface.c | 76 +++ drivers/char/tpm/tpm.h | 134 +++- drivers/char/tpm/tpm2-cmd.c | 250 +- drivers/char/tpm/tpm_crb.c | 39 ++-- drivers/char/tpm/tpm_eventlog.c | 78 +-- drivers/char/tpm/tpm_eventlog.h
Re: [PATCH v3 5/7] security: Add hook to invalidate inode security labels
On Mon, 26 Oct 2015, Andreas Gruenbacher wrote: > Add a hook to invalidate an inode's security label when the cached > information becomes invalid. > > Implement the new hook in selinux: set a flag when a security label becomes > invalid. When hitting a security label which has been marked as invalid in > inode_has_perm, try reloading the label. > > If an inode does not have any dentries attached, we cannot reload its > security label because we cannot use the getxattr inode operation. In that > case, continue using the old, invalid label until a dentry becomes > available. > > Signed-off-by: Andreas Gruenbacher 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 v3 5/7] security: Add hook to invalidate inode security labels
On Mon, 26 Oct 2015, Andreas Gruenbacher wrote: > Add a hook to invalidate an inode's security label when the cached > information becomes invalid. > > Implement the new hook in selinux: set a flag when a security label becomes > invalid. When hitting a security label which has been marked as invalid in > inode_has_perm, try reloading the label. > > If an inode does not have any dentries attached, we cannot reload its > security label because we cannot use the getxattr inode operation. In that > case, continue using the old, invalid label until a dentry becomes > available. > > Signed-off-by: Andreas Gruenbacher 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: [GIT PULL] KEYS: Miscellaneous patches for next
On Thu, 22 Oct 2015, David Howells wrote: > Hi James, > > Could you pull these changes into your next branch please? > > There are three groups: > > (1) Miscellaneous cleanups. > > (2) Add scripts for extracting system cert list and module sigs. > > (3) Condense the type-specific data in the key struct into the payload > data as it doesn't really make any sense to keep them separate. > Pulled. Have these been in next yet? -- 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: [GIT PULL] SELinux patches for 4.4
On Wed, 21 Oct 2015, Paul Moore wrote: > Hi James, > > Nine SELinux patches in total for v4.4, although six of those patches are > either trivial, minor cleanups, or both. The remaining three patches aren't > too bad: one changes the CHECKREQPROT default to check the actual memory > protections, one stops us from checking file:open on ftruncate() calls, and > one converts the file_security_struct over to kmem_cache. > > All pass the SELinux testsuite and should apply cleanly on top of your next > branch. > Pulled, thanks. -- 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] apparmor: clarify CRYPTO dependency
On Wed, 21 Oct 2015, Arnd Bergmann wrote: > The crypto framework can be built as a loadable module, but the > apparmor hash code can only be built-in, which then causes a > link error: > > security/built-in.o: In function `aa_calc_profile_hash': > integrity_audit.c:(.text+0x21610): undefined reference to > `crypto_shash_update' > security/built-in.o: In function `init_profile_hash': > integrity_audit.c:(.init.text+0xb4c): undefined reference to > `crypto_alloc_shash' > > This changes Apparmor to use 'select CRYPTO' like a lot of other > subsystems do. > > Signed-off-by: Arnd Bergmann Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next -- 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 0/3] RFC: Secure Memory Allocation Framework
On Wed, 21 Oct 2015, Benjamin Gaignard wrote: > > The outcome of the previous RFC about how do secure data path was the need > of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551) > Have you addressed all the questions raised by Alan here: https://lkml.org/lkml/2015/5/8/629 Also, is there any application of this beyond DRM? - James -- 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 1/3] create SMAF module
On Wed, 21 Oct 2015, Benjamin Gaignard wrote: > Secure Memory Allocation Framework goal is to be able > to allocate memory that can be securing. > There is so much ways to allocate and securing memory that SMAF > doesn't do it by itself but need help of additional modules. > To be sure to use the correct allocation method SMAF implement > deferred allocation (i.e. allocate memory when only really needed) > > Allocation modules (smaf-alloctor.h): > SMAF could manage with multiple allocation modules at same time. > To select the good one SMAF call match() to be sure that a module > can allocate memory for a given list of devices. It is to the module > to check if the devices are compatible or not with it allocation > method. > > Securing module (smaf-secure.h): > The way of how securing memory it is done is platform specific. > Secure module is responsible of grant/revoke memory access. > This documentation is highly inadequate. What does "allocate memory that can be securing" mean? -- 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: [PULL] Smack - Changes for 4.4
On Mon, 19 Oct 2015, Casey Schaufler wrote: > The following changes since commit 049e6dde7e57f0054fdc49102e7ef4830c698b46: > > Linux 4.3-rc4 (2015-10-04 16:57:17 +0100) > > are available in the git repository at: > > https://github.com/cschaufler/smack-next.git smack-for-4.4 > Pulled, thanks. -- 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
[GIT PULL] Keys bugfixes
Please pull these key susbystem fixes for 4.3, per the message from David Howells: "Here are two patches, the first of which at least should go upstream immediately: (1) Prevent a user-triggerable crash in the keyrings destructor when a negatively instantiated keyring is garbage collected. I have also seen this triggered for user type keys. (2) Prevent the user from using requesting that a keyring be created and instantiated through an upcall. Doing so is probably safe since the keyring type ignores the arguments to its instantiation function - but we probably shouldn't let keyrings be created in this manner." --- The following changes since commit 1099f86044111e9a7807f09523e42d4c9d0fb781: Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-10-19 09:55:40 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git for-linus David Howells (2): KEYS: Fix crash when attempt to garbage collect an uninstantiated keyring KEYS: Don't permit request_key() to construct a new keyring security/keys/gc.c |6 -- security/keys/request_key.c |3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) -- 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: [PULL REQUEST] IMA changes for 4.4
On Mon, 19 Oct 2015, Mimi Zohar wrote: > Hi James, > > This pull request is for a single bug fix from Dimtry to properly load > only signed certificates onto the trusted IMA keyring from the kernel. > (This patch has been in the linux-next tree). > > thanks, > > Mimi > > The following changes since commit > 049e6dde7e57f0054fdc49102e7ef4830c698b46: > > Linux 4.3-rc4 (2015-10-04 16:57:17 +0100) > > are available in the git repository at: > > > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git > next > Pulled, thanks. -- 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: [PULL REQUEST] Fixes and Updates of TPM Subsystem
On Mon, 19 Oct 2015, Peter Hüwe wrote: > Hi James, > > I know it's superduper late, but maybe I am lucky enough. > Can you please pull the following Fixes and Updates at the earliest > convenience? > If possible even for 4.4? > > Most of the patches have been lying around for a long time due to my fault - > but at least they are mature enough now. > Pulled, thanks. -- James Morris
Re: Will you be updating security#next for 4.4?
On Mon, 5 Oct 2015, Casey Schaufler wrote: > Hi James. I'm starting my patch processing for 4.4 and wondered > if you're planning to update security#next any time soon. > Now merged to -rc4. -- 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: NFS/LSM: allow NFS to control all of its own mount options
On Tue, 19 Feb 2008, Christoph Hellwig wrote: > Please don't introduce a special case for just nfs. All filesystems > should control their mount options, so please provide some library > helpers for context= handling and move it into all filesystems that > can support selinux. It's not so much a special case for NFS, just that NFS happens to use binary mount options. So, I guess it could be put into a library for other potential filesystems with binary mount options. To clarify: The SELinux options are indeed filesystem independent, and the FS should really not need to be concerned at all with them. For everything except NFS, we parse text options looking for context=, then use that value from within SELinux as the label for all files in the mount. Previously, as Eric mentions, we were using a method initially approved by the NFS folk, where, for NFS, SELinux was peeking around inside the binary options. We were then asked to change that so that NFS (or other binary-option FS) would obtain the values itself and call into LSM with them. This is what Eric's latest patch enables (a previous patch installed the infrastructure for it). While this code could be put into a library if desired, there is no need to make any changes for filesystems with text options (i.e. the general case). - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/37] Security: Add a kernel_service object class to SELinux
On Fri, 8 Feb 2008, David Howells wrote: > +++ b/security/selinux/include/flask.h > @@ -51,6 +51,7 @@ > #define SECCLASS_DCCP_SOCKET 60 > #define SECCLASS_MEMPROTECT 61 > #define SECCLASS_PEER68 > +#define SECCLASS_KERNEL_SERVICE 69 I just pushed a patch to Linus from Stephen which uses this class number; you'll likely need to bump it to 70. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/37] Security: De-embed task security record from task and use refcounting
On Fri, 8 Feb 2008, David Howells wrote: > Remove the temporarily embedded task security record from task_struct. > Instead > it is made to dangle from the task_struct::sec and task_struct::act_as > pointers > with references counted for each. ... These patches are kind of huge. > +static int selinux_task_dup_security(struct task_security *sec) > +{ > + struct task_security_struct *tsec1, *tsec2; > + > + tsec1 = sec->security; > + > + tsec2 = kmemdup(tsec1, sizeof(*tsec1), GFP_KERNEL); > + if (!tsec2) > + return -ENOMEM; > + > + tsec2->osid = tsec1->osid; > + tsec2->sid = tsec1->sid; > + > + tsec2->exec_sid = tsec1->exec_sid; > + tsec2->create_sid = tsec1->create_sid; > + tsec2->keycreate_sid = tsec1->keycreate_sid; > + tsec2->sockcreate_sid = tsec1->sockcreate_sid; > + tsec2->ptrace_sid = SECINITSID_UNLABELED; > + sec->security = tsec2; > + > + return 0; > } Why manually copy these fields after a kmemdup? What about the task backpointer? (i.e. tsec2->task) -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/37] Security: Separate task security context from task_struct
On Fri, 8 Feb 2008, David Howells wrote: > Separate the task security context from task_struct. At this point, the > security data is temporarily embedded in the task_struct with two pointers > pointing to it. > > Alpha needs further alteration as it refers to UID & GID in entry.S via asm > offsets. > > Sparc needs further alteration as it refers to UID & GID in sclow.S via asm > offsets. > > Signed-off-by: David Howells <[EMAIL PROTECTED]> Reviewed-by: James Morris <[EMAIL PROTECTED]> (SELinux stuff mostly). -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/37] Security: Change current->fs[ug]id to current_fs[ug]id()
On Fri, 8 Feb 2008, David Howells wrote: > Change current->fs[ug]id to current_fs[ug]id() so that fsgid and fsuid can be > separated from the task_struct. > > Signed-off-by: David Howells <[EMAIL PROTECTED]> Reviewed-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] per-process securebits
On Fri, 1 Feb 2008, Andrew Morton wrote: > Really? I'd feel a lot more comfortable if yesterday's version 1 had led > to a stream of comments from suitably-knowledgeable kernel developers which > indicated that those developers had scrutinised this code from every > conceivable angle and had declared themselves 100% happy with it. FWIW, I've reviewed the patch in detail a couple of times, and don't see any issues with it that haven't already been raised by Serge. You can add my reviewed-by. I think it does need more eyes, and some time baking in -mm. File capabilities are at least marked experimental, although it's not clear what the criteria would be to unmark them. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] security: add iptables "security" table for MAC rules
On Tue, 29 Jan 2008, Paul Moore wrote: > That seems reasonable. By the way, this isn't really related, but is it > possible to change the NF_IP_PRI_SELINUX_* constants to NF_IP_PRI_SECURITY_* > for the sake of consistency or are those values already visible to userspace? > They are visible to userspace, and included in glibc headers, but I don't see any userland use of them via google codesearch or know of a possible valid use. > I suppose we could always rename them anyway and just add a #define for > compatibility ... Yep, if you want to. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] security: add iptables "security" table for MAC rules
On Tue, 29 Jan 2008, Paul Moore wrote: > I'm not sure if returning false and failing here is the best thing to do in > terms of backwards compatibility. I think we need some grace period where we > print a warning message and still allow the operation; after some period of > time we can then remove the ability completely and force users to use the > new "security" table. Currently, the patch allows the use of the mangle table, so it is backwards compatible. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] security: add iptables "security" table for MAC rules
The following patch implements a new "security" table for iptables, so that MAC (SELinux etc.) networking rules can be managed separately to standard DAC rules. This is to help with distro integration of the new secmark-based network controls, per various previous discussions. The need for a separate table arises from the fact that existing tools and usage of iptables will likely clash with centralized MAC policy management. The SECMARK and CONNSECMARK targets will still be valid in the mangle table, to prevent breakage of existing users, although I suspect that these targets are not in significant use and we could probably make them valid only in the security table without major issues. (Comments?) I've set the table priority to just after NF_IP_FILTER, to allow DAC rules to take effect before MAC rules. There is also not (yet) any LSM hooking for modifying the MAC rules, as it will be more invasive, and we have coarse coverage via CAP_NET_ADMIN. (Comments?) IPv6 is not done yet as this is just at RFC stage. Please review and let me know if this looks like a viable approach for distro integration. If it is, we will then need to run it past the Netfilter & netdev folk. --- Add a "security" table to iptables for managing Mandatory Access Control (MAC) rules. This is intended for use with the SECMARK and CONNSECMARK targets. Signed-off-by: James Morris <[EMAIL PROTECTED]> --- include/linux/netfilter_ipv4.h|1 + net/ipv4/netfilter/Kconfig| 10 ++ net/ipv4/netfilter/Makefile |1 + net/ipv4/netfilter/iptable_security.c | 181 + net/netfilter/xt_CONNSECMARK.c| 10 ++- net/netfilter/xt_SECMARK.c| 10 ++- 6 files changed, 207 insertions(+), 6 deletions(-) create mode 100644 net/ipv4/netfilter/iptable_security.c diff --git a/include/linux/netfilter_ipv4.h b/include/linux/netfilter_ipv4.h index 1a63adf..aec8961 100644 --- a/include/linux/netfilter_ipv4.h +++ b/include/linux/netfilter_ipv4.h @@ -60,6 +60,7 @@ enum nf_ip_hook_priorities { NF_IP_PRI_MANGLE = -150, NF_IP_PRI_NAT_DST = -100, NF_IP_PRI_FILTER = 0, + NF_IP_PRI_SECURITY = 50, NF_IP_PRI_NAT_SRC = 100, NF_IP_PRI_SELINUX_LAST = 225, NF_IP_PRI_CONNTRACK_HELPER = INT_MAX - 2, diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig index 9aca9c5..84d0d31 100644 --- a/net/ipv4/netfilter/Kconfig +++ b/net/ipv4/netfilter/Kconfig @@ -373,6 +373,16 @@ config IP_NF_RAW If you want to compile it as a module, say M here and read . If unsure, say `N'. +# security table for MAC policy +config IP_NF_SECURITY + tristate "Security table" + depends on IP_NF_IPTABLES + help + This option adds a `security' table to iptables, for use + with Mandatory Access Control (MAC) policy. + + If unsure, say N. + # ARP tables config IP_NF_ARPTABLES tristate "ARP tables support" diff --git a/net/ipv4/netfilter/Makefile b/net/ipv4/netfilter/Makefile index 7456833..90fcdd6 100644 --- a/net/ipv4/netfilter/Makefile +++ b/net/ipv4/netfilter/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_IP_NF_FILTER) += iptable_filter.o obj-$(CONFIG_IP_NF_MANGLE) += iptable_mangle.o obj-$(CONFIG_NF_NAT) += iptable_nat.o obj-$(CONFIG_IP_NF_RAW) += iptable_raw.o +obj-$(CONFIG_IP_NF_SECURITY) += iptable_security.o # matches obj-$(CONFIG_IP_NF_MATCH_ADDRTYPE) += ipt_addrtype.o diff --git a/net/ipv4/netfilter/iptable_security.c b/net/ipv4/netfilter/iptable_security.c new file mode 100644 index 000..640f200 --- /dev/null +++ b/net/ipv4/netfilter/iptable_security.c @@ -0,0 +1,181 @@ +/* + * "security" table + * + * This is for use by Mandatory Access Control (MAC) security models, + * which need to be able to manage security policy in separate context + * to DAC. + * + * Based on iptable_mangle.c + * + * Copyright (C) 1999 Paul `Rusty' Russell & Michael J. Neuling + * Copyright (C) 2000-2004 Netfilter Core Team <[EMAIL PROTECTED]> + * Copyright (C) 2008 Red Hat, Inc., James Morris <[EMAIL PROTECTED]> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include +#include +#include +#include +#include +#include +#include +#include + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("James Morris <[EMAIL PROTECTED]>"); +MODULE_DESCRIPTION("iptables security table, for MAC rules"); + +#define SECURITY_VALID_HOOKS (1 << NF_IP_LOCAL_IN) | \ + (1 << NF_IP_FORWARD) | \ + (1 << NF_IP_LOCAL_OUT) + +static struct +{ + struct ipt_replace repl; + struct ipt_standard entries
Re: "Default Linux Capabilities" default in 2.6.24
On Mon, 28 Jan 2008, Matt LaPlante wrote: > On Thu, 24 Jan 2008 19:12:01 -0600 > Matt LaPlante <[EMAIL PROTECTED]> wrote: > > > > > I'm doing a make oldconfig with the new 2.6.24 kernel. I came to the > > prompt for "Default Linux Capabilities" which defaults to No: > > > > --- > > Default Linux Capabilities (SECURITY_CAPABILITIES) [N/y/?] (NEW) ? > > --- > > > > However the help text recommends saying Yes. > > > > --- > > This enables the "default" Linux capabilities functionality. > > If you are unsure how to answer this question, answer Y. > > --- > > > > Does this seem incongruous? Also, what's the "question"? :) > > > > Thanks, > > Matt LaPlante > > Anyone? I think this should be default y. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Security: constify function pointer tables and fields
On Wed, 23 Jan 2008, Jan Engelhardt wrote: > Signed-off-by: Jan Engelhardt <[EMAIL PROTECTED]> > --- > include/linux/security.h |2 +- > security/keys/proc.c |4 ++-- > security/selinux/selinuxfs.c |2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]
On Wed, 23 Jan 2008, David Howells wrote: > Stephen Smalley <[EMAIL PROTECTED]> wrote: > > > Make sure that you or Dan submits a policy patch to register these > > classes and permissions in the policy when the kernel patch is queued > > for merge. > > Do I just send the attached patch to <[EMAIL PROTECTED]>? That should be enough (and that list is on the cc). >Or do I need to > make a diff from a point in the tree nearer the root? Is there anything else > I need to alter whilst I'm at it? > > David > --- > Index: policy/flask/security_classes > === > --- policy/flask/security_classes (revision 2573) > +++ policy/flask/security_classes (working copy) > @@ -109,4 +109,7 @@ > # network peer labels > class peer > > +# kernel services that need to override task security > +class kernel_service > + > # FLASK > Index: policy/flask/access_vectors > === > --- policy/flask/access_vectors (revision 2573) > +++ policy/flask/access_vectors (working copy) > @@ -736,3 +736,10 @@ > { > recv > } > + > +# kernel services that need to override task security > +class kernel_service > +{ > + use_as_override > + create_files_as > +} > - > To unsubscribe from this list: send the line "unsubscribe > linux-security-module" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/26] Add a secctx_to_secid() LSM hook to go along with the existing
On Wed, 16 Jan 2008, Paul Moore wrote: > On Tuesday 15 January 2008 8:05:27 pm James Morris wrote: > > On Tue, 15 Jan 2008, David Howells wrote: > > > secid_to_secctx() LSM hook. This patch also includes the SELinux > > > implementation for this hook. > > > > > > Signed-off-by: Paul Moore <[EMAIL PROTECTED]> > > > Acked-by: Stephen Smalley <[EMAIL PROTECTED]> > > > > This is useful in its own right, and I would like to push it upstream for > > 2.6.24 unless there are any objections. > > Isn't it a bit late in 2.6.24 to add new functionality, especially when there > isn't an in-tree user for it in 2.6.24? Actually, I meant to go into 2.6.24 after the merge window opens. > > You are right, there are several users of this function currently under > development but I'm pretty sure all of them are targeting 2.6.25 or greater. > With that in mind, I think the prudent thing to is to wait and push this > upstream for 2.6.25. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Permit filesystem local caching
On Tue, 15 Jan 2008, David Howells wrote: > > (*) 04-keys-get-label.diff > > A patch to allow the security label of a key to be retrieved. > Included because of patches 05-08. > > (*) 05-security-current-fsugid.diff > (*) 06-security-separate-task-bits.diff > (*) 07-security-subjective.diff > (*) 08-security-secctx2secid.diff > (*) 09-security-additional-classes.diff > (*) 10-security-kernel_service-class.diff > (*) 11-security-kernel-service.diff All of the security patches look ok to me. From the SELinux pov, this will need to go in after Paul Moore's labeled networking update (hopefully very soon after the next merge window opens). - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/26] Add a secctx_to_secid() LSM hook to go along with the existing
id); > +} > + > static void selinux_release_secctx(char *secdata, u32 seclen) > { > kfree(secdata); > @@ -4937,6 +4942,7 @@ static struct security_operations selinux_ops = { > .setprocattr = selinux_setprocattr, > > .secid_to_secctx = selinux_secid_to_secctx, > + .secctx_to_secid = selinux_secctx_to_secid, > .release_secctx = selinux_release_secctx, > > .unix_stream_connect = > selinux_socket_unix_stream_connect, > > - > To unsubscribe from this list: send the line "unsubscribe > linux-security-module" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/26] Permit filesystem local caching
On Tue, 15 Jan 2008, David Howells wrote: > The SELinux base code will also need updating to have the security class, lest > the following error appear in dmesg: > > context_struct_compute_av: unrecognized class 69 Alternately, what's needed is a newer policy which supports unknown permission classes. Any recent distro policy should have this. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH][RFC] security: call security_file_permission from rw_verify_area
Please review. Tested with SELinux in enforcing mode. --- All instances of rw_verify_area() are followed by a call to security_file_permission(), so just call the latter from the former. Signed-off-by: James Morris <[EMAIL PROTECTED]> --- fs/compat.c |4 --- fs/read_write.c | 63 +-- fs/splice.c |8 --- 3 files changed, 24 insertions(+), 51 deletions(-) diff --git a/fs/compat.c b/fs/compat.c index 15078ce..5216c3f 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -1104,10 +1104,6 @@ static ssize_t compat_do_readv_writev(int type, struct file *file, if (ret < 0) goto out; - ret = security_file_permission(file, type == READ ? MAY_READ:MAY_WRITE); - if (ret) - goto out; - fnv = NULL; if (type == READ) { fn = file->f_op->read; diff --git a/fs/read_write.c b/fs/read_write.c index ea1f94c..c4d3d17 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -197,25 +197,27 @@ int rw_verify_area(int read_write, struct file *file, loff_t *ppos, size_t count { struct inode *inode; loff_t pos; + int retval = -EINVAL; inode = file->f_path.dentry->d_inode; if (unlikely((ssize_t) count < 0)) - goto Einval; + return retval; pos = *ppos; if (unlikely((pos < 0) || (loff_t) (pos + count) < 0)) - goto Einval; + return retval; if (unlikely(inode->i_flock && mandatory_lock(inode))) { - int retval = locks_mandatory_area( + retval = locks_mandatory_area( read_write == READ ? FLOCK_VERIFY_READ : FLOCK_VERIFY_WRITE, inode, file, pos, count); if (retval < 0) return retval; } + retval = security_file_permission(file, + read_write == READ ? MAY_READ : MAY_WRITE); + if (retval) + return retval; return count > MAX_RW_COUNT ? MAX_RW_COUNT : count; - -Einval: - return -EINVAL; } static void wait_on_retry_sync_kiocb(struct kiocb *iocb) @@ -267,18 +269,15 @@ ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos) ret = rw_verify_area(READ, file, pos, count); if (ret >= 0) { count = ret; - ret = security_file_permission (file, MAY_READ); - if (!ret) { - if (file->f_op->read) - ret = file->f_op->read(file, buf, count, pos); - else - ret = do_sync_read(file, buf, count, pos); - if (ret > 0) { - fsnotify_access(file->f_path.dentry); - add_rchar(current, ret); - } - inc_syscr(current); + if (file->f_op->read) + ret = file->f_op->read(file, buf, count, pos); + else + ret = do_sync_read(file, buf, count, pos); + if (ret > 0) { + fsnotify_access(file->f_path.dentry); + add_rchar(current, ret); } + inc_syscr(current); } return ret; @@ -325,18 +324,15 @@ ssize_t vfs_write(struct file *file, const char __user *buf, size_t count, loff_ ret = rw_verify_area(WRITE, file, pos, count); if (ret >= 0) { count = ret; - ret = security_file_permission (file, MAY_WRITE); - if (!ret) { - if (file->f_op->write) - ret = file->f_op->write(file, buf, count, pos); - else - ret = do_sync_write(file, buf, count, pos); - if (ret > 0) { - fsnotify_modify(file->f_path.dentry); - add_wchar(current, ret); - } - inc_syscw(current); + if (file->f_op->write) + ret = file->f_op->write(file, buf, count, pos); + else + ret = do_sync_write(file, buf, count, pos); + if (ret > 0) { + fsnotify_modify(file->f_path.dentry); + add_wchar(current, ret); } + inc_syscw(current); } return ret; @@ -603,9 +599,6 @@ static ssize_t do_readv_writev(int type, struct file *file, ret = rw_verify_area(type, file, pos, tot_len); if (ret < 0) goto out; - ret = security_file_permission(file, type == READ ? MAY_READ : MAY_
Re: [TOMOYO #6 retry 08/21] Utility functions and policy manipulationinterface.
On Sat, 12 Jan 2008, Tetsuo Handa wrote: > Hello. > > James Morris wrote: > > > > TOMOYO Linux uses /sys/kernel/security/tomoyo interface for > > > > configuration. > > > > > > Why aren't you using securityfs for this? (It was designed for LSMs). > > > > Doh, it is using securityfs, don't worry. > > > I got a mm-commits mail titled > "+ sysfs-make-sysfs_deprecated-depend-on-sysfs.patch added to -mm tree" . > > Is sysfs going to be deprecated? Unlikely -- what's in the patches? - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] Adding prctl override support for LSMs
On Wed, 9 Jan 2008, Andrew Morgan wrote: > So far the objection (thanks Stephen for the historical context!) seems > to be "potential for abuse": > > ~ <[EMAIL PROTECTED]> > [PATCH] remove sys_security > > I've been auditing the LSM stuff a bit more.. > > They have registered an implemented a syscall, sys_security > that does nothing but switch into the individual modules > based on the first argument, i.e. it's ioctl() switching > on the security module instead of device node. Yuck. > > Patch below removes it (no intree users), maybe selinux/etc > folks should send their actual syscall for review instead.. > > Since SELinux is now 'in-tree', is this class of objection now moot? Class of objection to a sys_security, or to the prctl override? If the former, I think it would still be considered a poor option, as multiplexor syscalls are generally seen as such for several reasons. I don't think SELinux would need to use it now if it came back. As Stephen mentioned, the prctl override might also be seen as a means to revector/hijack the syscall. I should mention that I'm still not clear on why you need to have a permissive version of this hook. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO #6 retry 08/21] Utility functions and policy manipulation interface.
On Wed, 9 Jan 2008, James Morris wrote: > On Wed, 9 Jan 2008, Kentaro Takeda wrote: > > > Common functions for TOMOYO Linux. > > > > TOMOYO Linux uses /sys/kernel/security/tomoyo interface for configuration. > > Why aren't you using securityfs for this? (It was designed for LSMs). Doh, it is using securityfs, don't worry. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO #6 retry 08/21] Utility functions and policy manipulation interface.
On Wed, 9 Jan 2008, Kentaro Takeda wrote: > Common functions for TOMOYO Linux. > > TOMOYO Linux uses /sys/kernel/security/tomoyo interface for configuration. Why aren't you using securityfs for this? (It was designed for LSMs). - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Exporting capability code/name pairs
On Wed, 2 Jan 2008, KaiGai Kohei wrote: > > Another issue is that securityfs depends on CONFIG_SECURITY, which might be > > undesirable, given that capabilities are a standard feature. > > We can implement this feature on another pseudo filesystems. > Do you think what filesystem is the best candidate? > I prefer procfs or sysfs instead. Sysfs makes more sense, as this information is system-wide and does not relate to specific processes. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] security: remove security_sb_post_mountroot hook
On Sat, 29 Dec 2007, H. Peter Anvin wrote: > The security_sb_post_mountroot() hook is long-since obsolete, and is > fundamentally broken: it is never invoked if someone uses initramfs. > This is particularly damaging, because the existence of this hook has > been used as motivation for not using initramfs. > > Stephen Smalley confirmed on 2007-07-19 that this hook was originally > used by SELinux but can now be safely removed: > > http://marc.info/?l=linux-kernel&m=118485683612916&w=2 Thanks. Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Exporting capability code/name pairs
On Fri, 28 Dec 2007, KaiGai Kohei wrote: > Remaining issues: > - We have to mount securityfs explicitly, or use /etc/fstab. > It can cause a matter when we want to use this feature on > very early phase on boot. (like /sbin/init) Why can't early userspace itself mount securityfs? I'm not even sure this is a good idea at all. Existing capabilities will never disappear, and, as with syscalls, it's probably up to userland to handle new ones not existing. In any case, some more technical issues: > kernel/cap_names.sh generates the body of cap_entries[] array, This needs to be in the scripts directory. The generated header should be made idempotent (#ifdef wrapping), and also include a warning that it is automatically generated (identifying the script which does so), and that is should not be edited. > + d_caps = securityfs_create_dir("capability", NULL); > + if (!d_caps) Wrong way to check for error -- the function returns an ERR_PTR(). > + f_caps[i] = securityfs_create_file(cap_entries[i].name, 0444, > +d_caps, &cap_entries[i], > +&cap_entry_fops); > + if (!f_caps[i]) Ditto. Another issue is that securityfs depends on CONFIG_SECURITY, which might be undesirable, given that capabilities are a standard feature. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Exporting capability code/name pairs
On Fri, 28 Dec 2007, KaiGai Kohei wrote: > + snprintf(tmp, sizeof(tmp), > + cap_entry == &cap_entries[0] ? "0x%08x" : "%u", > + cap_entry->code); > + len = strlen(tmp); You don't need to call strlen(), just use scnprintf() and grab the return value. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Exporting capability code/name pairs
On Thu, 27 Dec 2007, KaiGai Kohei wrote: (Please put the patch above the .sig separator). + len = strlen(tmp); + + if (ofs >= len) + return 0; + + if (len - ofs < count) + count = len - ofs; + + rc = copy_to_user(buffer, tmp + ofs, count); + if (rc) + return rc; + + *ppos += count; Use simple_read_from_buffer(). - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning
On Tue, 25 Dec 2007, Andrew Morton wrote: > On Thu, 20 Dec 2007 15:11:40 +1100 (EST) James Morris <[EMAIL PROTECTED]> > wrote: > > > > > +#ifdef CONFIG_SECURITY > > > > +extern unsigned long mmap_min_addr; > > > > +#endif > > > > + > > > > #include > > > > #include > > > > #include > > > > > > Fine by me. > > > > I'll queue it for -mm & 2.6.25. > > I don't think we need the ifdef. If someone incorrectly refers to this > then they'll get a link-time error rather than a compile-time error (bad). > But we lose an ifdef (good). Done, & rebased the git branch. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning
On Thu, 20 Dec 2007, David Chinner wrote: > > I'm not sure I understand your namespace pollution issue, either. > > doing this globally: > > #ifdef CONFIG_SOMETHING > extern intsome_common_name(int a, int b, int c); > #else > #define some_common_name(a,b,c) 0 > #endif I suspect it may be useful ensure all global identifiers for the key subsystem are prefixed with key_, as 'copy_keys' does seem a little generic. > > +#ifdef CONFIG_SECURITY > > +extern unsigned long mmap_min_addr; > > +#endif > > + > > #include > > #include > > #include > > Fine by me. I'll queue it for -mm & 2.6.25. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning
On Wed, 19 Dec 2007, David Chinner wrote: > Folks, > > I just updated a git tree and started getting errors on a > "copy_keys" macro warning. > > The code I've been working on uses a ->copy_keys() method for > copying the keys in a btree block from one place to another. I've > been working on this code for a while > (http://oss.sgi.com/archives/xfs/2007-11/msg00046.html) and keep the > tree I'm working in reletively up to date (lags linus by a couple of > weeks at most). The update I did this afternoon gave a conflict > warning with the macro in include/linux/key.h. > > Given that I'm not directly including key.h anywhere in the XFS > code, I'm getting the namespace polluted indirectly from some other > include that is necessary. > > As it turns out, this commit from 13 days ago: > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7cd94146cd504016315608e297219f9fb7b1413b > > included security.h in mm.h and that is how I'm seeing the namespace > poisoning coming from key.h when !CONFIG_KEY. > > Including security.h in mm.h means much wider includes for pretty > much the entire kernel, and it opens up namespace issues like this > that never previously existed. > > The patch below (only tested for !CONFIG_KEYS && !CONFIG_SECURITY) > moves security.h into the mmap.c and nommu.c files that need it so > it doesn't end up with kernel wide scope. > > Comments? The idea with this placement was to keep memory management code with other similar code, rather than pushing it into security.h, where it does not functionally belong. Something to not also is that you can't "depend" on security.h not being included all over the place, as LSM does touch a lot of the kernel. Unecessarily including it is bad, of course. I'm not sure I understand your namespace pollution issue, either. In any case, I think the right solution is not to include security.h at all in mm.h, as it is only being done to get a declaration for mmap_min_addr. How about this, instead ? Signed-off-by: James Morris <[EMAIL PROTECTED]> --- mm.h |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/linux/mm.h b/include/linux/mm.h index 1b7b95c..02fbac7 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -12,7 +12,6 @@ #include #include #include -#include struct mempolicy; struct anon_vma; @@ -34,6 +33,10 @@ extern int sysctl_legacy_va_layout; #define sysctl_legacy_va_layout 0 #endif +#ifdef CONFIG_SECURITY +extern unsigned long mmap_min_addr; +#endif + #include #include #include -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v8 05/18] LSM: Add secctx_to_secid() LSM hook
On Fri, 14 Dec 2007, Paul Moore wrote: > Add a secctx_to_secid() LSM hook to go along with the existing > secid_to_secctx() LSM hook. This patch also includes the SELinux > implementation for this hook. Please sign off your patches (especially this one asap, so I can push it upstream). -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v8 10/18] SELinux: Add a network node caching mechanism similar to the sel_netif_*() functions
On Mon, 17 Dec 2007, Paul Moore wrote: > On Monday 17 December 2007 3:35:28 pm Stephen Smalley wrote: > > On Fri, 2007-12-14 at 16:50 -0500, Paul Moore wrote: > > > This patch adds a SELinux IP address/node SID caching mechanism similar > > > to the sel_netif_*() functions. The node SID queries in the SELinux > > > hooks files are also modified to take advantage of this new > > > functionality. In addition, remove the address length information from > > > the sk_buff parsing routines as it is redundant since we already have the > > > address family. > > > > This is very nice - we also need the same kind of cache for port SIDs. > > Thanks. Any problem if we wait until 2.6.26 for a port SID cache? Nope. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] VM/Security: add security hook to do_brk
On Tue, 4 Dec 2007, Alan Cox wrote: > On Tue, Dec 04, 2007 at 11:06:55AM -0500, Eric Paris wrote: > > Given a specifically crafted binary do_brk() can be used to get low > > pages available in userspace virtually memory and can thus be used to > > circumvent the mmap_min_addr low memory protection. Add security checks > > in do_brk(). > > > > Signed-off-by: Eric Paris <[EMAIL PROTECTED]> > > ACK Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Out of tree module using LSM
On Fri, 30 Nov 2007, Crispin Cowan wrote: > > The only case of this so far has been Multiadm, although there seems to be > > no reason for it to stay out of tree. > > > Dazuko. It has the same yucky code issues as Talpa, but AFAIK is pure > GPL2 and thus is clean on the license issues. > > That these modules are valid modules that users want to use, are GPL > clean, and are *not* something LKML wants to upstream because of code > issues, is precisely why the LSM interface makes sense. I think the idea is to try and fix code quality issues through participation in the upstream process, rather than have upstream maintain stable kernel APIs which are naturally mismatched to the unknown requirements of out of tree users. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Out of tree module using LSM
On Fri, 30 Nov 2007, Crispin Cowan wrote: > restored faces a lot of challenges, but I hope that some kind of > solution can be found, because the alternative is to effectively force > vendors like Sophos to do it the "dirty" way by fishing in memory for > the syscall table. I don't think this is quite correct. The alternative is to engage with the kernel community to become part of the development process, to ensure that appropriate APIs are implemented, and to get code upstream before shipping it. In any case, a patch to revert the dynamic aspect of LSM has been posted by Arjan (and acked by myself) for the case of valid out of tree users. The only case of this so far has been Multiadm, although there seems to be no reason for it to stay out of tree. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 -v2] mmap: round mmap hint address above mmap_min_addr
On Mon, 26 Nov 2007, Eric Paris wrote: > If mmap_min_addr is set and a process attempts to mmap (not fixed) with > a non-null hint address less than mmap_min_addr the mapping will fail > the security checks. Since this is just a hint address this patch will > round such a hint address above mmap_min_addr. > > gcj was found to try to be very frugal with vm usage and give hint > addresses in the 8k-32k range. Without this patch all such programs > failed and with the patch they happily get a higher address. > > This patch is wrappad in CONFIG_SECURITY since mmap_min_addr doesn't > exist without it and there would be no security check possible no matter > what. So we should not bother compiling in this rounding if it is just a > waste of time. > > Signed-off-by: Eric Paris <[EMAIL PROTECTED]> Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm > > --- > > include/linux/mm.h | 16 > mm/mmap.c |3 +++ > mm/nommu.c |3 +++ > 3 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 520238c..1b7b95c 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > > struct mempolicy; > struct anon_vma; > @@ -513,6 +514,21 @@ static inline void set_page_links(struct page *page, > enum zone_type zone, > } > > /* > + * If a hint addr is less than mmap_min_addr change hint to be as > + * low as possible but still greater than mmap_min_addr > + */ > +static inline unsigned long round_hint_to_min(unsigned long hint) > +{ > +#ifdef CONFIG_SECURITY > + hint &= PAGE_MASK; > + if (((void *)hint != NULL) && > + (hint < mmap_min_addr)) > + return PAGE_ALIGN(mmap_min_addr); > +#endif > + return hint; > +} > + > +/* > * Some inline functions in vmstat.h depend on page_zone() > */ > #include > diff --git a/mm/mmap.c b/mm/mmap.c > index 938313c..f4cfc6a 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -912,6 +912,9 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned > long addr, > if (!len) > return -EINVAL; > > + if (!(flags & MAP_FIXED)) > + addr = round_hint_to_min(addr); > + > error = arch_mmap_check(addr, len, flags); > if (error) > return error; > diff --git a/mm/nommu.c b/mm/nommu.c > index 35622c5..b989cb9 100644 > --- a/mm/nommu.c > +++ b/mm/nommu.c > @@ -829,6 +829,9 @@ unsigned long do_mmap_pgoff(struct file *file, > void *result; > int ret; > > + if (!(flags & MAP_FIXED)) > + addr = round_hint_to_min(addr); > + > /* decide whether we should attempt the mapping, and if so what sort of >* mapping */ > ret = validate_mmap_request(file, addr, len, prot, flags, pgoff, > > -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] mmap: protect from stack expantion into low vm addresses
On Mon, 26 Nov 2007, Eric Paris wrote: > Add security checks to make sure we are not attempting to expand the > stack into memory protected by mmap_min_addr > > Signed-off-by: Eric Paris <[EMAIL PROTECTED]> Please include the LSMs list in the CC line (added again) for posts relating to security. Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm > --- > > ** Be very careful applying/rediffing this patch. Standard 3 lines of > context from git diff will misapply the first hunk to expand_upwards > instead of properly in expand_downwards. This patch was generated using > -U 4 to make sure it applies in the correct place. ** Seems to have applied correctly for me. > > mm/mmap.c |8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > --- kernel-1/mm/mmap.c > +++ kernel-2/mm/mmap.c > @@ -1614,17 +1614,21 @@ static inline int expand_downwards(struc >* so that the anon_vma locking is not a noop. >*/ > if (unlikely(anon_vma_prepare(vma))) > return -ENOMEM; > + > + address &= PAGE_MASK; > + error = security_file_mmap(0, 0, 0, 0, address, 1); > + if (error) > + return error; > + > anon_vma_lock(vma); > > /* >* vma->vm_start/vm_end cannot change under us because the caller >* is required to hold the mmap_sem in read mode. We need the >* anon_vma lock to serialize against concurrent expand_stacks. >*/ > - address &= PAGE_MASK; > - error = 0; > > /* Somebody else might have raced and expanded it already */ > if (address < vma->vm_start) { > unsigned long size, grow; > > -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -mm 2/2] do_wait: cleanup delay_group_leader() usage
[added LSM list] On Fri, 23 Nov 2007, Oleg Nesterov wrote: > eligible_child() == 2 means delay_group_leader(). With the previous patch this > only matters for EXIT_ZOMBIE task, we can move that special check to the only > place it is really needed. > > Also, with this patch we don't skip security_task_wait() for the group leaders > in a non-empty thread group. I don't really understand the exact semantics of > security_task_wait(), but imho this change is a bugfix. The semantics in terms of SELinux are that calls to wait() type calls need to be mediated by policy, as information can be transfered via the exit status. It looks like a correct bugfix to me, but I'd be interested to see what others have to say. > Also rearrange the code a bit to kill an ugly "check_continued" backdoor. > > Signed-off-by: Oleg Nesterov <[EMAIL PROTECTED]> > > --- PT/kernel/exit.c~6_delay_leader 2007-11-23 20:31:21.0 +0300 > +++ PT/kernel/exit.c 2007-11-23 21:29:44.0 +0300 > @@ -1137,12 +1137,6 @@ static int eligible_child(pid_t pid, int > if (((p->exit_signal != SIGCHLD) ^ ((options & __WCLONE) != 0)) > && !(options & __WALL)) > return 0; > - /* > - * Do not consider thread group leaders that are > - * in a non-empty thread group: > - */ > - if (delay_group_leader(p)) > - return 2; > > err = security_task_wait(p); > if (err) > @@ -1494,10 +1488,9 @@ repeat: > tsk = current; > do { > struct task_struct *p; > - int ret; > > list_for_each_entry(p, &tsk->children, sibling) { > - ret = eligible_child(pid, options, p); > + int ret = eligible_child(pid, options, p); > if (!ret) > continue; > > @@ -1521,19 +1514,17 @@ repeat: > retval = wait_task_stopped(p, > (options & WNOWAIT), infop, > stat_addr, ru); > - } else if (p->exit_state == EXIT_ZOMBIE) { > + } else if (p->exit_state == EXIT_ZOMBIE && > + !delay_group_leader(p)) { > /* > - * Eligible but we cannot release it yet: > + * We don't reap group leaders with subthreads. >*/ > - if (ret == 2) > - goto check_continued; > if (!likely(options & WEXITED)) > continue; > retval = wait_task_zombie(p, > (options & WNOWAIT), infop, > stat_addr, ru); > } else if (p->exit_state != EXIT_DEAD) { > -check_continued: > /* >* It's running now, so it might later >* exit, stop, or stop and then continue. > -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -mm 2/2] do_wait: cleanup delay_group_leader() usage (fwd)
Any thoughts on this? -- Forwarded message -- Date: Fri, 23 Nov 2007 22:59:42 +0300 From: Oleg Nesterov <[EMAIL PROTECTED]> To: Andrew Morton <[EMAIL PROTECTED]> Cc: Eric Paris <[EMAIL PROTECTED]>, James Morris <[EMAIL PROTECTED]>, Roland McGrath <[EMAIL PROTECTED]>, [EMAIL PROTECTED] Subject: [PATCH -mm 2/2] do_wait: cleanup delay_group_leader() usage eligible_child() == 2 means delay_group_leader(). With the previous patch this only matters for EXIT_ZOMBIE task, we can move that special check to the only place it is really needed. Also, with this patch we don't skip security_task_wait() for the group leaders in a non-empty thread group. I don't really understand the exact semantics of security_task_wait(), but imho this change is a bugfix. Also rearrange the code a bit to kill an ugly "check_continued" backdoor. Signed-off-by: Oleg Nesterov <[EMAIL PROTECTED]> --- PT/kernel/exit.c~6_delay_leader 2007-11-23 20:31:21.0 +0300 +++ PT/kernel/exit.c2007-11-23 21:29:44.0 +0300 @@ -1137,12 +1137,6 @@ static int eligible_child(pid_t pid, int if (((p->exit_signal != SIGCHLD) ^ ((options & __WCLONE) != 0)) && !(options & __WALL)) return 0; - /* -* Do not consider thread group leaders that are -* in a non-empty thread group: -*/ - if (delay_group_leader(p)) - return 2; err = security_task_wait(p); if (err) @@ -1494,10 +1488,9 @@ repeat: tsk = current; do { struct task_struct *p; - int ret; list_for_each_entry(p, &tsk->children, sibling) { - ret = eligible_child(pid, options, p); + int ret = eligible_child(pid, options, p); if (!ret) continue; @@ -1521,19 +1514,17 @@ repeat: retval = wait_task_stopped(p, (options & WNOWAIT), infop, stat_addr, ru); - } else if (p->exit_state == EXIT_ZOMBIE) { + } else if (p->exit_state == EXIT_ZOMBIE && + !delay_group_leader(p)) { /* -* Eligible but we cannot release it yet: +* We don't reap group leaders with subthreads. */ - if (ret == 2) - goto check_continued; if (!likely(options & WEXITED)) continue; retval = wait_task_zombie(p, (options & WNOWAIT), infop, stat_addr, ru); } else if (p->exit_state != EXIT_DEAD) { -check_continued: /* * It's running now, so it might later * exit, stop, or stop and then continue. - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6.25] Add packet filtering based on process's security context.
On Thu, 22 Nov 2007, Tetsuo Handa wrote: > This patch allows LSM modules filter incoming connections/datagrams > based on the process's security context who is attempting to pick up. > > There are already hooks to filter incoming connections/datagrams > based on the socket's security context, but these hooks are not > applicable when one wants to do TCP Wrapper-like filtering > (e.g. App1 is permitted to accept TCP connections from 192.168.0.0/16). This functionality looks like it could be useful in that we currently have no direct security mapping from incoming packet to user process, but only to the receiving socket, as you mention. For SELinux, it may help us simplify/clarify policy. It's also been long-desired for netfilter/iptables, to allow ipt_owner to work cleanly for incoming packets. So, this probably needs to be implemented in a way which works for both LSM and netfilter. There have been several discussions on the issue from the netfilter side, although I don't know what the latest status of that is (I've expanded the cc list to hopefully get some more feedback). >From memory, one approach under discussion was to add netfilter hooks to the transport layer, which could be invoked correctly by each type of protocol when the target process is selected. If this is done for netfilter, then an LSM hook is probably not needed at all, as security modules can utilize netfilter hooks directly. > Precautions: This approach has a side effect which unlikely occurs. > > If a socket is shared by multiple processes with differnt policy, > the process who should be able to accept this connection > will not be able to accept this connection > because socket_post_accept() aborts this connection. > But if socket_post_accept() doesn't abort this connection, > the process who must not be able to accept this connection > will repeat accept() forever, which is a worse side effect. > > Similarly, if a socket is shared by multiple processes with differnt policy, > the process who should be able to pick up this datagram > will not be able to pick up this datagram > because socket_post_recv_datagram() discards this datagram. > But if socket_post_recv_datagram() doesn't discard this datagram, > the process who must not be able to pick up this datagram > will repeat recvmsg() forever, which is a worse side effect. > > So, don't give different permissions between processes who share one socket. > Otherwise, some connections/datagrams cannot be delivered to intended process. These semantics changes are concerning, and lead me to wonder if there are any more. Needs more review by networking folk. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO #5 18/18] LSM expansion for TOMOYO Linux.
On Tue, 20 Nov 2007, Tetsuo Handa wrote: > Hello. > > Paul Moore wrote: > > My apologies, I mistakenly read the following if statement in your patch: > > > > + if (skb == skb_peek(&sk->sk_receive_queue)) { > > + __skb_unlink(skb, &sk->sk_receive_queue); > > + atomic_dec(&skb->users); > > + } > > > > I read the conditional as the following: > > > > + if (skb = skb_peek(&sk->sk_receive_queue)) { > > > > ... which would have caused the problems I was describing. I'm sorry for > > all > > of the confusion/frustration, you patient explanations are correct; I was > > wrong in this particular case. > No problem. > > > > To everyone: > > Are there any remaining worries with > skb_recv_datagram()/socket_post_accept()? > > If nobody has objection, I'd like to cut these > skb_recv_datagram()/socket_post_accept() changes > and submit to -mm tree. You should send anything which touches core networking to netdev, too, and get an ack from one of the core developers there. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v6 09/13] SELinux: Better integration between peer labeling subsystems
On Fri, 9 Nov 2007, Paul Moore wrote: > + /* Between selinux_compat_net and selinux_policycap_netpeer this is > + * starting to get a bit messy - we need to setup a timetable for > + * deprecating some of this old/obsolete functionality so we can > + * reclaim some level of sanity in this function. */ I don't think we can do anything which could potentially break userspace now. So, this one really needs to be right :-) -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v6 08/13] SELinux: Add new peer permissions to the Flask definitions
On Fri, 9 Nov 2007, Paul Moore wrote: > Add additional Flask definitions to support the new "peer" object class. Should this be dependent on dynamic class/permission support? Or, will these checks only be invoked if labled networking is configured? -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] NetLabel: Introduce a new kernel configuration API for NetLabel - Version 11 (2.6.24-rc2) Smack: Simplified Mandatory Access Control Kernel
On Fri, 9 Nov 2007, Andrew Morton wrote: > On Thu, 08 Nov 2007 20:48:35 -0800 > Casey Schaufler <[EMAIL PROTECTED]> wrote: > > > From: Paul Moore <[EMAIL PROTECTED]> > > > > Add a new set of configuration functions to the NetLabel/LSM API so that > > LSMs can perform their own configuration of the NetLabel subsystem without > > relying on assistance from userspace. > > > > Signed-off-by: Paul Moore <[EMAIL PROTECTED]> > > You sent it, so this patch needs a Signed-off-by:you, please. Also add Reviewed-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer
On Thu, 1 Nov 2007, David P. Quigley wrote: > This patch modifies the interface to inode_getsecurity to have the function > return a buffer containing the security blob and its length via parameters > instead of relying on the calling function to give it an appropriately sized > buffer. Security blobs obtained with this function should be freed using the > release_secctx LSM hook. This alleviates the problem of the caller having to > guess a length and preallocate a buffer for this function allowing it to be > used elsewhere for Labeled NFS. The patch also removed the unused err > parameter. The conversion is similar to the one performed by Al Viro for the > security_getprocattr hook. > > Signed-off-by: David P. Quigley <[EMAIL PROTECTED]> Acked-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] VFS: Reorder vfs_getxattr to avoid unnecessary calls to the LSM
On Thu, 1 Nov 2007, David P. Quigley wrote: > Originally vfs_getxattr would pull the security xattr variable using > the inode getxattr handle and then proceed to clobber it with a subsequent > call > to the LSM. This patch reorders the two operations such that when the xattr > requested is in the security namespace it first attempts to grab the value > from > the LSM directly. If it fails to obtain the value because there is no module > present or the module does not support the operation it will fall back to > using > the inode getxattr operation. In the event that both are inaccessible it > returns EOPNOTSUPP. > > Signed-off-by: David P. Quigley <[EMAIL PROTECTED]> Acked-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer
On Wed, 31 Oct 2007, David P. Quigley wrote: > On Sat, 2007-10-27 at 08:14 +1000, James Morris wrote: > > On Fri, 26 Oct 2007, Serge E. Hallyn wrote: > > > > > > It wouldn't be much effort to rebase this patch against Linus's latest > > > > tree. I am assuming that the static lsm patch is in there based on the > > > > recent discussion on LKML? > > > > > > Oh, sorry for the two emails. > > > > > > Yeah it's in 2.6.24. So a rebase will be necessary anyway. > > > > That code may still change -- Arjan's update, for example. > > > > What do you think should be done with this patch set? We have already > gone past the 2.6.24 merge window so it seems as if it will have to be > put into 2.6.25. Should I wait until 2.6.24 is released and repost it > then? Is it possible for them to get into 2.6.24 since its early in the > rc cycle? Not as far as I'm aware. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer
On Fri, 26 Oct 2007, Serge E. Hallyn wrote: > > It wouldn't be much effort to rebase this patch against Linus's latest > > tree. I am assuming that the static lsm patch is in there based on the > > recent discussion on LKML? > > Oh, sorry for the two emails. > > Yeah it's in 2.6.24. So a rebase will be necessary anyway. That code may still change -- Arjan's update, for example. -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [AppArmor 34/45] Factor out sysctl pathname code
On Thu, 25 Oct 2007, [EMAIL PROTECTED] wrote: > Convert the selinux sysctl pathname computation code into a standalone > function. > > Signed-off-by: Andreas Gruenbacher <[EMAIL PROTECTED]> > Signed-off-by: John Johansen <[EMAIL PROTECTED]> Reviewed-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] VFS: Reorder vfs_getxattr to avoid unnecessary calls to the LSM
On Mon, 22 Oct 2007, David P. Quigley wrote: > Originally vfs_getxattr would pull the security xattr variable using > the inode getxattr handle and then proceed to clobber it with a subsequent > call > to the LSM. This patch reorders the two operations such that when the xattr > requested is in the security namespace it first attempts to grab the value > from > the LSM directly. If it fails to obtain the value because there is no module > present or the module does not support the operation it will fall back to > using > the inode getxattr operation. In the event that both are inaccessible it > returns EOPNOTSUPP. > > Signed-off-by: David P. Quigley <[EMAIL PROTECTED]> Acked-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer
On Mon, 22 Oct 2007, David P. Quigley wrote: > +static inline int security_inode_getsecurity(const struct inode *inode, > + const char *name, > + void **buffer) It's better to keep function declarations on one line if possible (the 80-col rule can be broken for this). But in any case, it looks ok to me. Acked-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 -mm] capabilities: introduce per-process capability bounding set (v4)
On Wed, 3 Oct 2007, Serge E. Hallyn wrote: > > Signed-off-by: Serge Hallyn <[EMAIL PROTECTED]> Acked-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 -mm] capabilities: define CONFIG_COMMONCAP
On Wed, 3 Oct 2007, Serge E. Hallyn wrote: > >From 54c70ca7671750fe8986451fae91d42107d0ca90 Mon Sep 17 00:00:00 2001 > From: Serge E. Hallyn <[EMAIL PROTECTED]> > Date: Fri, 28 Sep 2007 10:33:33 -0500 > Subject: [PATCH 1/2 -mm] capabilities: define CONFIG_COMMONCAP > > currently the compilation of commoncap.c is determined > through Makefile logic. So there is no single CONFIG > variable which can be relied upon to know whether it > will be compiled. > > Define CONFIG_COMMONCAP to be true when lsm is not > compiled in, or when the capability or rootplug modules > are compiled. These are the cases when commoncap is > currently compiled. Use this variable in security/Makefile > to determine commoncap.c's compilation. > > Apart from being a logic cleanup, this is needed by the > upcoming cap_bset patch so that prctl can know whether > PR_SET_BSET should be allowed. > > Signed-off-by: Serge E. Hallyn <[EMAIL PROTECTED]> Acked-by: James Morris <[EMAIL PROTECTED]> -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO 05/15](repost) Domain transition handler functions.
On Wed, 3 Oct 2007, YOSHIFUJI Hideaki / µÈÆ£±ÑÌÀ wrote: > In article <[EMAIL PROTECTED]> (at Wed, 3 Oct 2007 23:26:57 +0900), Tetsuo > Handa <[EMAIL PROTECTED]> says: > > > Peter Zijlstra wrote: > > > Also, how do you avoid referencing dead data with your sll? I haven't > > > actually looked at your patches, but the simple scheme you outlined > > > didn't handle the iteration + concurrent removal scenario: > > Regarding my singly-linked list, no entries are removed from the list. It's > > append only (like CD-R media). > > I set is_deleted flag of a entry instead of removing the entry from the > > list. > > It is not a good practice. Please free such objects. > BTW, how many objects do you have in the list? Doesn't matter. No list should be able to grow without bounds in the kernel. -- James Morris <[EMAIL PROTECTED]>
Re: [TOMOYO 05/15](repost) Domain transition handler functions.
On Wed, 3 Oct 2007, YOSHIFUJI Hideaki / µÈÆ£±ÑÌÀ wrote: > In article <[EMAIL PROTECTED]> (at Wed, 3 Oct 2007 20:24:52 +0900), Tetsuo > Handa <[EMAIL PROTECTED]> says: > > > It seems that standard kernel list API does not have singly-linked list > > manipulation. > > I'm considering the following list manipulation API. > > Tstsuo, please name it "slist", which is well-known. I'm pretty sure that the singly linked list idea has been rejected a few times. Just use the existing API. -- James Morris <[EMAIL PROTECTED]>
Re: [TOMOYO 03/15](repost) Memory and pathname management functions.
On Tue, 2 Oct 2007, Kentaro Takeda wrote: > +/** > + * tmy_alloc - allocate memory for temporary purpose. > + * @size: requested size in bytes. > + * > + * Returns '\0'-initialized memory region on success. > + * Returns NULL on failure. > + * > + * This function allocates memory for keeping ACL entries. > + * The caller has to call tmy_free() the returned pointer > + * when memory is no longer needed. > + */ Would you please explain why you need another level of memory allocation? What does it do apart from let you check for memory leaks? - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO 14/15](repost) LSM expansion for TOMOYO Linux.
On Tue, 2 Oct 2007, Kentaro Takeda wrote: > --- linux-2.6.orig/net/core/datagram.c2007-10-02 11:11:51.0 > +0900 > +++ linux-2.6/net/core/datagram.c 2007-10-02 11:26:23.0 +0900 > @@ -55,6 +55,7 @@ > #include > #include > #include > +#include > > /* > * Is a socket 'connection oriented' ? > @@ -178,6 +179,27 @@ struct sk_buff *skb_recv_datagram(struct > } else > skb = skb_dequeue(&sk->sk_receive_queue); > > + error = security_post_recv_datagram(sk, skb, flags); > + if (error) { > + unsigned long cpu_flags; > + > + if (!(flags & MSG_PEEK)) > + goto no_peek; > + > + spin_lock_irqsave(&sk->sk_receive_queue.lock, > + cpu_flags); > + if (skb == skb_peek(&sk->sk_receive_queue)) { > + __skb_unlink(skb, > + &sk->sk_receive_queue); > + atomic_dec(&skb->users); > + } > + spin_unlock_irqrestore(&sk->sk_receive_queue.lock, > +cpu_flags); > +no_peek: > + skb_free_datagram(sk, skb); > + goto no_packet; > + } > + > if (skb) > return skb; I'm guessing you need this to determine the receiving process, rather than the socket (which is available via security_sock_rcv_skb()). If so, is this to interactively determine from the user or admin whether the packet should be accepted/denied for the receiving process? - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO 05/15](repost) Domain transition handler functions.
On Tue, 2 Oct 2007, Tetsuo Handa wrote: > Hello. > > Thank you for your comment. > > James Morris wrote: > > Why do you need to avoid spinlocks for these manipulations? > > I don't need to use spinlocks here. > What I need to do here is avoid read/write reordering, > so mb() will be appropriate here. > > struct something { > struct something *next; > void *data; > }; > > struct something *prev_ptr = ...; > struct something *new_ptr = kmalloc(sizeof(*new_ptr), GFP_KERNEL); > new_ptr->next = NULL; > new_ptr->data = some_value; > mb(); > prev_ptr->next = new_ptr; You're introducing a custom API, which is open-coded repeatedly throughout your module. All linked lists (at least, new ones) must use the standard kernel list API. > Performance is not critical because this mb() is called > only when appending new entry to the singly-linked list. Most of these uses appear to be slow path or nested inside other locks, while overall, performance is likely to be dominated by your string matching and permission checking. Use of mb(), which is typically considered less understandable, in this case does not appear to be justified vs. normal locking, and you also need to use the standard list API. If performance really is an issue, then consider RCU. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO 14/15](repost) LSM expansion for TOMOYO Linux.
On Tue, 2 Oct 2007, Kentaro Takeda wrote: > LSM expansion for TOMOYO Linux. > > LSM hooks for sending signal: >* task_kill_unlocked is added in sys_kill >* task_tkill_unlocked is added in sys_tkill >* task_tgkill_unlocked is added in sys_tgkill Why do you need racy unlocked versions, in addition to the existing security_task_kill() hook which is called safely via check_kill_permission() ? - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO 05/15](repost) Domain transition handler functions.
On Tue, 2 Oct 2007, Kentaro Takeda wrote: > + > + mb(); /* Instead of using spinlock. */ > + ptr = domain_initializer_list; > + if (ptr) { > + while (ptr->next) > + ptr = ptr->next; > + ptr->next = new_entry; > + } else > + domain_initializer_list = new_entry; > + Please use standard kernel list handling, per include/linux/list.h Why do you need to avoid spinlocks for these manipulations? - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [TOMOYO 00/15](repost) TOMOYO Linux - MAC based on process invocation history.
It seems that patches 1-3 are missing. I'd also suggest making all of the patches a reply to the first email, so they can be threaded. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] capabilities: per-process capbset
On Mon, 1 Oct 2007, Serge E. Hallyn wrote: > Here is a new per-process capability bounding set patchset > which I expect to send to linux-kernel soon. It makes > the capbset per-process. A process can only permanently > remove bits from it's bounding set, not add them. To > remove bits, CAP_SYS_ADMIN is currently needed. Maybe > that's not the best choice, but some privilege should > probably be required. I'm not clear on why privilege would required for a process to remove capability bits from its set. (Sure, if running setuid). Doesn't that just make it more difficult to write safe applications ? -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Version 3 (2.6.23-rc8) Smack: Simplified Mandatory Access Control Kernel
On Sun, 30 Sep 2007, Andrew Morton wrote: > So with the information which I presently have available to me, I'm > thinking that this should go into 2.6.24. I think the decision to merge Smack is something that needs to be considered in the wider context of overall security architecture. Smack itself looks fine. It seems like clean code with interesting ideas, and appears to be based upon sound principles. Merging Smack, however, would lock the kernel into the LSM API. Presently, as SELinux is the only in-tree user, LSM can still be removed. LSM's weak semantics and pervasive deep hooking of the kernel means that we'll have to continue dealing with several unpleasant issues, such as the abuse of the API by out of tree vendors, with a proliferation of binary/proprietary modules which typically maladapt the API for arbitrary purposes and/or use dangerous techniques. We will continue to waste resources maintaining this capability for them. On a broader scale, we'll miss the potential of Linux having a coherent, semantically strong security architecture. I have written about this in some detail before: http://lwn.net/Articles/240019/ Briefly, SELinux is a security architecture. It provides an extremely high degree of flexibility, in terms of both the types of security models implemented, and security policy for those models. It allows controlled composition of different security models, with a common policy language framework, allowing the entire system to be analyzed. The same ideas and even code can be reused beyond the kernel as post-DAC security is extended into databases, the desktop, distributed applications etc. It is a framework which provides a structured, coherent view of the security of the OS (and ultimately, the entire distributed environment). If LSM remains, security will never be a first class citizen of the kernel. Application developers will see multiple security schemes, and either burn themselves trying to support them, or more likely, ignore them. Core kernel developers will continue to not have enough information to understand what the LSM hooks in their code are supposed to be doing, leading to long term maintenance issues and potential security problems. LSM will remain a magnet for bad ideas. There's a reason we don't have pluggable network stacks, and we are lucky to have had a unified networking framework maintained by people who know to say no to things like STREAMS and TOE. I would question whether this quality of maintainership would exist if the networking core was simply a bunch of deep hooks into which people dumped arbitrary custom stacks. LSM will prevent us from making systemic improvements to security, as there will be no common security architecture. Things like Netfilter would not have been viable with a kernel which simply provided a bunch of hooks for networking stacks and an assortment of implementations. Much of this we have learned from the experience of LSM, and I think it has been valuable for that, but I think we need to consider now whether we are going to continue down this track in a permanent manner. - James -- James Morris <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html