Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1725987 > unrolled thread
| Started by | James Morris <jmorris@namei.org> |
|---|---|
| First post | 2017-09-04 12:40 +0200 |
| Last post | 2017-09-10 08:50 +0200 |
| Articles | 20 on this page of 23 — 7 participants |
Back to article view | Back to linux.kernel
[GIT PULL] Security subsystem updates for 4.14 James Morris <jmorris@namei.org> - 2017-09-04 12:40 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-07 20:20 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 James Morris <jmorris@namei.org> - 2017-09-08 06:50 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Christoph Hellwig <hch@infradead.org> - 2017-09-08 09:10 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-08 19:30 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Paul Moore <paul@paul-moore.com> - 2017-09-08 19:40 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 James Morris <jmorris@namei.org> - 2017-09-10 06:40 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 James Morris <jmorris@namei.org> - 2017-09-10 07:00 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Paul Moore <paul@paul-moore.com> - 2017-09-12 00:40 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Kees Cook <keescook@chromium.org> - 2017-09-14 23:10 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 James Morris <jmorris@namei.org> - 2017-09-14 23:20 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Kees Cook <keescook@chromium.org> - 2017-09-14 23:30 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 James Morris <jmorris@namei.org> - 2017-09-08 22:00 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-09-17 09:40 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Christoph Hellwig <hch@infradead.org> - 2017-09-10 10:20 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-09-10 16:10 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Christoph Hellwig <hch@infradead.org> - 2017-09-11 08:40 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-09-11 23:40 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Theodore Ts'o <tytso@mit.edu> - 2017-09-09 00:40 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 James Morris <jmorris@namei.org> - 2017-09-10 04:10 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-09-10 09:20 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Theodore Ts'o <tytso@mit.edu> - 2017-09-10 14:20 +0200
Re: [GIT PULL] Security subsystem updates for 4.14 Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-09-10 08:50 +0200
Page 1 of 2 [1] 2 Next page →
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-09-04 12:40 +0200 |
| Subject | [GIT PULL] Security subsystem updates for 4.14 |
| Message-ID | <ulWXo-4m6-9@gated-at.bofh.it> |
Hi Linus,
Here are the security subsystem updates for 4.14. Highlights:
AppArmor:
- Add mediation of mountpoints and signals
- Add support for absolute root view based labels
- add base infastructure for socket mediation
LSM:
- Remove unused security_task_create() hook
TPM:
- Some constification and minor updates.
IMA:
- A new integrity_read file operation method, avoids races when
calculating file hashes
SELinux:
- from Paul Moore:
"A relatively quiet period for SELinux, 11 patches with only two/three
having any substantive changes. These noteworthy changes include
another tweak to the NNP/nosuid handling, per-file labeling for
cgroups, and an object class fix for AF_UNIX/SOCK_RAW sockets; the rest
of the changes are minor tweaks or administrative updates (Stephen's
email update explains the file explosion in the diffstat)."
Seccomp:
- from Kees Cook:
"Major additions:
- sysctl and seccomp operation to discover available actions. (tyhicks)
- new per-filter configurable logging infrastructure and sysctl. (tyhicks)
- SECCOMP_RET_LOG to log allowed syscalls. (tyhicks)
- SECCOMP_RET_KILL_PROCESS as the new strictest possible action.
- self-tests for new behaviors."
And nothing for Smack, for the first time perhaps.
Please pull.
---
The following changes since commit 81a84ad3cb5711cec79f4dd53a4ce026b092c432:
Merge branch 'docs-next' of git://git.lwn.net/linux (2017-09-03 21:07:29 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next
Antonio Murdaca (1):
selinux: allow per-file labeling for cgroupfs
Arvind Yadav (3):
tpm: tpm_crb: constify acpi_device_id.
tpm: vtpm: constify vio_device_id
selinux: constify nf_hook_ops
Christoph Hellwig (1):
ima: use fs method to read integrity data
Christos Gkekas (1):
apparmor: Fix logical error in verify_header()
Dan Carpenter (1):
apparmor: Fix an error code in aafs_create()
Enric Balletbo i Serra (1):
Documentation: tpm: add powered-while-suspended binding documentation
Geert Uytterhoeven (1):
apparmor: Fix shadowed local variable in unpack_trans_table()
Hamza Attak (1):
tpm: replace msleep() with usleep_range() in TPM 1.2/2.0 generic drivers
James Morris (3):
sync to Linus v4.13-rc2 for subsystem developers to work against
Merge tag 'seccomp-next' of git://git.kernel.org/.../kees/linux into next
Merge tag 'selinux-pr-20170831' of git://git.kernel.org/.../pcmoore/selinux into next
John Johansen (13):
apparmor: Redundant condition: prev_ns. in [label.c:1498]
apparmor: add the ability to mediate signals
apparmor: add mount mediation
apparmor: cleanup conditional check for label in label_print
apparmor: add support for absolute root view based labels
apparmor: make policy_unpack able to audit different info messages
apparmor: add more debug asserts to apparmorfs
apparmor: add base infastructure for socket mediation
apparmor: move new_null_profile to after profile lookup fns()
apparmor: fix race condition in null profile creation
apparmor: ensure unconfined profiles have dfas initialized
apparmor: fix incorrect type assignment when freeing proxies
apparmor: fix build failure on sparc caused by undeclared, signals
Kees Cook (9):
selftests/seccomp: Add tests for basic ptrace actions
selftests/seccomp: Add simple seccomp overhead benchmark
selftests/seccomp: Refactor RET_ERRNO tests
seccomp: Provide matching filter for introspection
seccomp: Rename SECCOMP_RET_KILL to SECCOMP_RET_KILL_THREAD
seccomp: Introduce SECCOMP_RET_KILL_PROCESS
seccomp: Implement SECCOMP_RET_KILL_PROCESS action
selftests/seccomp: Test thread vs process killing
samples: Unrename SECCOMP_RET_KILL
Luis Ressel (1):
selinux: Assign proper class to PF_UNIX/SOCK_RAW sockets
Michal Hocko (1):
selinux: use GFP_NOWAIT in the AVC kmem_caches
Michal Suchanek (1):
tpm: ibmvtpm: simplify crq initialization and document crq format
Mimi Zohar (6):
ima: don't remove the securityfs policy file
libfs: define simple_read_iter_from_buffer
efivarfs: replaces the read file operation with read_iter
ima: always measure and audit files in policy
ima: define "dont_failsafe" policy action rule
ima: define "fs_unsafe" builtin policy
Paul Moore (4):
credits: update Paul Moore's info
selinux: update the selinux info in MAINTAINERS
MAINTAINERS: update the NetLabel and Labeled Networking information
MAINTAINERS: update the NetLabel and Labeled Networking information
Stefan Berger (1):
security: fix description of values returned by cap_inode_need_killpriv
Stephen Smalley (4):
selinux: genheaders should fail if too many permissions are defined
selinux: Generalize support for NNP/nosuid SELinux domain transitions
selinux: update my email address
lsm_audit: update my email address
Tetsuo Handa (2):
LSM: Remove security_task_create() hook.
tomoyo: Update URLs in Documentation/admin-guide/LSM/tomoyo.rst
Tyler Hicks (6):
seccomp: Sysctl to display available actions
seccomp: Operation for checking if an action is available
seccomp: Sysctl to configure actions that are allowed to be logged
seccomp: Selftest for detection of filter flag support
seccomp: Filter flag to log all actions except SECCOMP_RET_ALLOW
seccomp: Action to log before allowing
CREDITS | 8 +-
Documentation/ABI/testing/ima_policy | 3 +-
Documentation/admin-guide/LSM/tomoyo.rst | 24 +-
Documentation/admin-guide/kernel-parameters.txt | 8 +-
.../devicetree/bindings/security/tpm/tpm-i2c.txt | 6 +
Documentation/networking/filter.txt | 2 +-
Documentation/sysctl/kernel.txt | 1 +
Documentation/userspace-api/seccomp_filter.rst | 52 ++-
MAINTAINERS | 29 +-
drivers/char/tpm/tpm-interface.c | 10 +-
drivers/char/tpm/tpm.h | 9 +-
drivers/char/tpm/tpm2-cmd.c | 2 +-
drivers/char/tpm/tpm_crb.c | 2 +-
drivers/char/tpm/tpm_ibmvtpm.c | 98 ++-
drivers/char/tpm/tpm_infineon.c | 6 +-
drivers/char/tpm/tpm_tis_core.c | 8 +-
fs/btrfs/file.c | 1 +
fs/efivarfs/file.c | 12 +-
fs/ext2/file.c | 17 +
fs/ext4/file.c | 20 +
fs/f2fs/file.c | 1 +
fs/jffs2/file.c | 1 +
fs/jfs/file.c | 1 +
fs/libfs.c | 32 +
fs/nilfs2/file.c | 1 +
fs/ramfs/file-mmu.c | 1 +
fs/ramfs/file-nommu.c | 1 +
fs/ubifs/file.c | 1 +
fs/xfs/xfs_file.c | 21 +
include/linux/audit.h | 6 +-
include/linux/fs.h | 3 +
include/linux/lsm_audit.h | 2 +-
include/linux/lsm_hooks.h | 7 -
include/linux/seccomp.h | 3 +-
include/linux/security.h | 6 -
include/uapi/linux/seccomp.h | 23 +-
kernel/fork.c | 4 -
kernel/seccomp.c | 321 +++++++++-
mm/shmem.c | 1 +
scripts/selinux/genheaders/genheaders.c | 7 +-
security/apparmor/.gitignore | 1 +
security/apparmor/Makefile | 43 ++-
security/apparmor/apparmorfs.c | 37 +-
security/apparmor/domain.c | 4 +-
security/apparmor/file.c | 30 +
security/apparmor/include/apparmor.h | 2 +
security/apparmor/include/audit.h | 39 +-
security/apparmor/include/domain.h | 5 +
security/apparmor/include/ipc.h | 6 +
security/apparmor/include/label.h | 1 +
security/apparmor/include/mount.h | 54 ++
security/apparmor/include/net.h | 114 ++++
security/apparmor/include/perms.h | 5 +-
security/apparmor/include/policy.h | 13 +
security/apparmor/include/sig_names.h | 98 +++
security/apparmor/ipc.c | 99 +++
security/apparmor/label.c | 36 +-
security/apparmor/lib.c | 5 +-
security/apparmor/lsm.c | 472 +++++++++++++
security/apparmor/mount.c | 696 ++++++++++++++++++++
security/apparmor/net.c | 184 ++++++
security/apparmor/policy.c | 166 +++---
security/apparmor/policy_ns.c | 2 +
security/apparmor/policy_unpack.c | 105 +++-
security/commoncap.c | 6 +-
security/integrity/iint.c | 20 +-
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 67 ++-
security/integrity/ima/ima_crypto.c | 10 +
security/integrity/ima/ima_fs.c | 4 +-
security/integrity/ima/ima_main.c | 19 +-
security/integrity/ima/ima_policy.c | 41 ++-
security/lsm_audit.c | 2 +-
security/security.c | 5 -
security/selinux/avc.c | 16 +-
security/selinux/hooks.c | 56 ++-
security/selinux/include/avc.h | 2 +-
security/selinux/include/avc_ss.h | 2 +-
security/selinux/include/classmap.h | 2 +
security/selinux/include/objsec.h | 2 +-
security/selinux/include/security.h | 4 +-
security/selinux/ss/avtab.c | 2 +-
security/selinux/ss/avtab.h | 2 +-
security/selinux/ss/constraint.h | 2 +-
security/selinux/ss/context.h | 2 +-
security/selinux/ss/ebitmap.c | 2 +-
security/selinux/ss/ebitmap.h | 2 +-
security/selinux/ss/hashtab.c | 2 +-
security/selinux/ss/hashtab.h | 2 +-
security/selinux/ss/mls.c | 2 +-
security/selinux/ss/mls.h | 2 +-
security/selinux/ss/mls_types.h | 2 +-
security/selinux/ss/policydb.c | 2 +-
security/selinux/ss/policydb.h | 2 +-
security/selinux/ss/services.c | 9 +-
security/selinux/ss/services.h | 2 +-
security/selinux/ss/sidtab.c | 2 +-
security/selinux/ss/sidtab.h | 2 +-
security/selinux/ss/symtab.c | 2 +-
security/selinux/ss/symtab.h | 2 +-
tools/testing/selftests/seccomp/Makefile | 18 +-
.../testing/selftests/seccomp/seccomp_benchmark.c | 99 +++
tools/testing/selftests/seccomp/seccomp_bpf.c | 610 +++++++++++++++---
103 files changed, 3540 insertions(+), 469 deletions(-)
create mode 100644 security/apparmor/include/mount.h
create mode 100644 security/apparmor/include/net.h
create mode 100644 security/apparmor/include/sig_names.h
create mode 100644 security/apparmor/mount.c
create mode 100644 security/apparmor/net.c
create mode 100644 tools/testing/selftests/seccomp/seccomp_benchmark.c
[toc] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-09-07 20:20 +0200 |
| Message-ID | <un9zc-3GP-15@gated-at.bofh.it> |
| In reply to | #1725987 |
On Mon, Sep 4, 2017 at 3:29 AM, James Morris <jmorris@namei.org> wrote:
>
> IMA:
> - A new integrity_read file operation method, avoids races when
> calculating file hashes
Honestly, this seems really odd.
It documents that it needs to be called with i_rwsem held exclusively,
and even has a lockdep assert to that effect (well, not really: the
code claims "exclusive", but the lockdep assert does not), but I'm not
actually seeing anybody doing it.
Quite the reverse, I just see integrity_read_file() doing filp_open()
on the pathname and passing it to integrity_kernel_read() with no
locking.
It really looks like just pure garbage to me. I pulled, and I'm not
unpulling the whole thing. I don't think it's been tested, and I don't
think it can be right.
Tell me why I'm wrong, or tell me why that garbage made it in in the
first place?
Linus
[toc] | [prev] | [next] | [standalone]
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-09-08 06:50 +0200 |
| Message-ID | <unjoR-1Us-7@gated-at.bofh.it> |
| In reply to | #1728361 |
On Thu, 7 Sep 2017, Linus Torvalds wrote: > On Mon, Sep 4, 2017 at 3:29 AM, James Morris <jmorris@namei.org> wrote: > > > > IMA: > > - A new integrity_read file operation method, avoids races when > > calculating file hashes > > Honestly, this seems really odd. > > It documents that it needs to be called with i_rwsem held exclusively, > and even has a lockdep assert to that effect (well, not really: the > code claims "exclusive", but the lockdep assert does not), but I'm not > actually seeing anybody doing it. > > Quite the reverse, I just see integrity_read_file() doing filp_open() > on the pathname and passing it to integrity_kernel_read() with no > locking. > > It really looks like just pure garbage to me. I pulled, and I'm not > unpulling the whole thing. I don't think it's been tested, and I don't > think it can be right. > > Tell me why I'm wrong, or tell me why that garbage made it in in the > first place? Mimi and Christoph worked together on this over several iterations -- I'll let them respond. -- James Morris <jmorris@namei.org>
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2017-09-08 09:10 +0200 |
| Message-ID | <unlAm-3xn-17@gated-at.bofh.it> |
| In reply to | #1728587 |
The reason why I send out the original version of this patch
is because IMA used to call ->read under i_rwsem, and that deadlocked
on XFS and NFS, or ext3/4 with DAX. The call path for that is
process_measurement (takes i_rwsem)
-> ima_collect_measurement
-> ima_calc_file_hash
-> ima_calc_file_ahash / ima_calc_file_shash
-> ima_calc_file_hash_atfm / ima_calc_file_hash_tfm
-> integrity_kernel_read
ima_check_last_writer (takes i_rwsem)
-> ima_update_xattr
-> ima_collect_measurement
-> (as above)
But yes, for the init-time integrity_read_file this is incorrect.
It never tripped up, and I explicitly added the lockdep annotations
so that anything would show up, and it's been half a year since
I sent that first RFC patch..
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-09-08 19:30 +0200 |
| Message-ID | <unvgl-1Cc-15@gated-at.bofh.it> |
| In reply to | #1728636 |
On Fri, Sep 8, 2017 at 12:09 AM, Christoph Hellwig <hch@infradead.org> wrote:
>
> But yes, for the init-time integrity_read_file this is incorrect.
> It never tripped up, and I explicitly added the lockdep annotations
> so that anything would show up, and it's been half a year since
> I sent that first RFC patch..
I don't think anybody actually tests linux-next kernels in any big
way, and the automated tests that do get run probably don't run with
any integrity checking enabled.
Which is why I actually look at the code when merging unexpected stuff.
This is also why I tend to prefer getting multiple branches for
independent things.
Now the whole security pull will be ignored because of this thing. I
refuse to pull garbage where I notice major fundamental problems in
code that has obviously never ever been tested.
Side note: one of the reasons why I _looked_ at this code was because
the exclusive lock requirement was entirely unexplained in the first
place.
Linus
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-09-08 19:40 +0200 |
| Message-ID | <unvq1-1Fs-5@gated-at.bofh.it> |
| In reply to | #1729109 |
On Fri, Sep 8, 2017 at 1:25 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Fri, Sep 8, 2017 at 12:09 AM, Christoph Hellwig <hch@infradead.org> wrote: >> >> But yes, for the init-time integrity_read_file this is incorrect. >> It never tripped up, and I explicitly added the lockdep annotations >> so that anything would show up, and it's been half a year since >> I sent that first RFC patch.. > > I don't think anybody actually tests linux-next kernels in any big > way, and the automated tests that do get run probably don't run with > any integrity checking enabled. > > Which is why I actually look at the code when merging unexpected stuff. > > This is also why I tend to prefer getting multiple branches for > independent things. > > Now the whole security pull will be ignored because of this thing. I > refuse to pull garbage where I notice major fundamental problems in > code that has obviously never ever been tested. Is it time to start sending pull request for each LSM and thing under security/ directly? I'm not sure I have a strong preference either way, I just don't want to see the SELinux changes ignored during the merge window. -- paul moore www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-09-10 06:40 +0200 |
| Message-ID | <uo2ch-75m-3@gated-at.bofh.it> |
| In reply to | #1729118 |
On Fri, 8 Sep 2017, Paul Moore wrote: > > This is also why I tend to prefer getting multiple branches for > > independent things. [...] > > Is it time to start sending pull request for each LSM and thing under > security/ directly? I'm not sure I have a strong preference either > way, I just don't want to see the SELinux changes ignored during the > merge window. They won't be ignored, we just need to get this issue resolved now and figure out how to implement multiple branches in the security tree. Looking at other git repos, the x86 folk have multiple branches. One option for me would be to publish the trees I pull from as branches along side mine, with 'next' being a merge of all of directly applied patchsets and those ready for Linus to pull as one. So, branches in git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security might be: next-selinux (Paul's next branch) next-apparmor-next (JJ's next branch) next-integrity-next (Mimi's) next-tpm-next (Jarkko's) [etc.] next (merge all of the above to here) That way, we have a coherent 'next' branch for people to develop against and to push to Linus, but he can pull individual branches feeding into it if something is broken in one of them. Does that sound useful? -- James Morris <jmorris@namei.org>
[toc] | [prev] | [next] | [standalone]
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-09-10 07:00 +0200 |
| Message-ID | <uo2vD-7cg-1@gated-at.bofh.it> |
| In reply to | #1729899 |
On Sun, 10 Sep 2017, James Morris wrote: > next-apparmor-next (JJ's next branch) > next-integrity-next (Mimi's) > next-tpm-next (Jarkko's) without '-next' on the end... (editing while jetlagged). -- James Morris <jmorris@namei.org>
[toc] | [prev] | [next] | [standalone]
| From | Paul Moore <paul@paul-moore.com> |
|---|---|
| Date | 2017-09-12 00:40 +0200 |
| Message-ID | <uoFwZ-16Z-1@gated-at.bofh.it> |
| In reply to | #1729899 |
On Sun, Sep 10, 2017 at 12:32 AM, James Morris <jmorris@namei.org> wrote: > On Fri, 8 Sep 2017, Paul Moore wrote: > >> > This is also why I tend to prefer getting multiple branches for >> > independent things. > > [...] > >> >> Is it time to start sending pull request for each LSM and thing under >> security/ directly? I'm not sure I have a strong preference either >> way, I just don't want to see the SELinux changes ignored during the >> merge window. > > They won't be ignored, we just need to get this issue resolved now and > figure out how to implement multiple branches in the security tree. Once again, I don't really care too much either way. My only selfish motivation is to make it as frictionless as possible to get the SELinux tree merged into Linus' tree. > Looking at other git repos, the x86 folk have multiple branches. I don't really understand what advantage one repo with multiple branches has over multiple repos, e.g. Linus' just pulling from the individual LSM trees directly. I suppose one could make an argument about linux-next, but I know they prefer to pull from the individual repos directly (they pull selinux/next directly). Is it to help reduce the load on Linus? From my perspective, the linux-security tree only introduces another opportunity for things to go wrong during the merge window (as evidenced by this latest snafu). Help me understand why a single tree with multiple branches is beneficial to multiple trees? Also, to be clear, I'm not picking on IMA or Mimi; this could have easily been SELinux screwing things up for IMA (or Smack, or AppArmor, etc.). > One option for me would be to publish the trees I pull from as branches > along side mine, with 'next' being a merge of all of directly applied > patchsets and those ready for Linus to pull as one. > > So, branches in > git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security > > might be: > > next-selinux (Paul's next branch) > next-apparmor-next (JJ's next branch) > next-integrity-next (Mimi's) > next-tpm-next (Jarkko's) > [etc.] > > next (merge all of the above to here) > > That way, we have a coherent 'next' branch for people to develop against > and to push to Linus, but he can pull individual branches feeding into it > if something is broken in one of them. > > Does that sound useful? -- paul moore www.paul-moore.com
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-09-14 23:10 +0200 |
| Message-ID | <upJyx-1Kz-1@gated-at.bofh.it> |
| In reply to | #1729899 |
On Sat, Sep 9, 2017 at 9:32 PM, James Morris <jmorris@namei.org> wrote: > On Fri, 8 Sep 2017, Paul Moore wrote: > >> > This is also why I tend to prefer getting multiple branches for >> > independent things. > > [...] > >> >> Is it time to start sending pull request for each LSM and thing under >> security/ directly? I'm not sure I have a strong preference either >> way, I just don't want to see the SELinux changes ignored during the >> merge window. > > They won't be ignored, we just need to get this issue resolved now and > figure out how to implement multiple branches in the security tree. > > Looking at other git repos, the x86 folk have multiple branches. Yeah, the x86 approach is what inspired my tree layout. > One option for me would be to publish the trees I pull from as branches > along side mine, with 'next' being a merge of all of directly applied > patchsets and those ready for Linus to pull as one. > > So, branches in > git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security > > might be: > > next-selinux (Paul's next branch) > next-apparmor (JJ's next branch) > next-integrity (Mimi's) > next-tpm (Jarkko's) > [etc.] > > next (merge all of the above to here) > > That way, we have a coherent 'next' branch for people to develop against > and to push to Linus, but he can pull individual branches feeding into it > if something is broken in one of them. > > Does that sound useful? This is what I do with the KSPP tree (since it has a few unrelated things in it), but you run the risk of getting too fine-grain and creating dependencies between trees (e.g. adding a new hook that two LSMs implement means either they depend on each other or both depend on some third "core" tree). How separable are the patches, normally? -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-09-14 23:20 +0200 |
| Message-ID | <upJId-1Oa-3@gated-at.bofh.it> |
| In reply to | #1732573 |
On Thu, 14 Sep 2017, Kees Cook wrote: > How separable are the patches, normally? They're usually mostly separate, but may depend on some core LSM change, or similar. Casey has mentioned off-list that it is useful to have a coherent up to date security branch to work against when making core LSM changes. -- James Morris <jmorris@namei.org>
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-09-14 23:30 +0200 |
| Message-ID | <upJRT-1Rr-15@gated-at.bofh.it> |
| In reply to | #1732587 |
On Thu, Sep 14, 2017 at 2:13 PM, James Morris <jmorris@namei.org> wrote: > On Thu, 14 Sep 2017, Kees Cook wrote: > >> How separable are the patches, normally? > > They're usually mostly separate, but may depend on some core LSM change, > or similar. > > Casey has mentioned off-list that it is useful to have a coherent up to > date security branch to work against when making core LSM changes. Yeah, for sure. This "merge all topics" tree is what I have for KSPP (and it's what I hand to -next). -Kees -- Kees Cook Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-09-08 22:00 +0200 |
| Message-ID | <unxBv-3aB-7@gated-at.bofh.it> |
| In reply to | #1729109 |
On Fri, 8 Sep 2017, Linus Torvalds wrote: > Now the whole security pull will be ignored because of this thing. I > refuse to pull garbage where I notice major fundamental problems in > code that has obviously never ever been tested. If I revert the change from my my tree, will you pull it then? -- James Morris <jmorris@namei.org>
[toc] | [prev] | [next] | [standalone]
| From | Mimi Zohar <zohar@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-09-17 09:40 +0200 |
| Message-ID | <uqClj-4Wh-3@gated-at.bofh.it> |
| In reply to | #1729239 |
On Sat, 2017-09-09 at 05:57 +1000, James Morris wrote: > On Fri, 8 Sep 2017, Linus Torvalds wrote: > > > Now the whole security pull will be ignored because of this thing. I > > refuse to pull garbage where I notice major fundamental problems in > > code that has obviously never ever been tested. > > If I revert the change from my my tree, will you pull it then? Sigh, none of the patches, other than those included in Paul's pull request, are in 4.14-rc1. I feel really horrible! Linus, there would never have been a good time for something like this to happen, but this past week, in case you didn't realize, most of us were at the Linux Security Summit. The timing couldn't have been worse. Mimi
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2017-09-10 10:20 +0200 |
| Message-ID | <uo5Dc-XU-1@gated-at.bofh.it> |
| In reply to | #1729109 |
On Fri, Sep 08, 2017 at 10:25:53AM -0700, Linus Torvalds wrote:
> I don't think anybody actually tests linux-next kernels in any big
> way, and the automated tests that do get run probably don't run with
> any integrity checking enabled.
Well, for the atual IMA deadlock issue I asked Mimi to produce automated
tests and we get started on it. I was pretty pissed about the
assumptions IMA made on the fs, whch weren't documented or automatically
tested - coming from the XFS background where we want all our features
to run through automated tests that was just not how I'd expect thing
to work.
But now as part of that I messed up the other caller of it, so I
shouldn't complain too loud..
That being said - I really hink the certificate loading should not
even go thorught this whole call path, but use our common helper to
load a file into a buffer. Something like the patch below, I'm just
not sure if the last policy argument is what people want or if we'd need
to add a new policy type for certificates.
---
From 3092ca95dfeff0dbd4e03ae62a51762facb15592 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Sun, 10 Sep 2017 09:49:45 +0200
Subject: security/digisig: read file using kernel_read_file_from_path
This avoid using the new integrity_read file operation which requires
i_rwsem already to be held, and avoids a lot of code duplication and
call the proper LSM hooks.
[also constifies the path argument to kernel_read_file_from_path,
as the callers needs it. Should probably be split into a separate patch]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/exec.c | 2 +-
include/linux/fs.h | 2 +-
security/integrity/digsig.c | 8 ++++---
security/integrity/iint.c | 49 ------------------------------------------
security/integrity/integrity.h | 2 --
5 files changed, 7 insertions(+), 56 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 01a9fb9d8ac3..957a8ce294af 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -974,7 +974,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
}
EXPORT_SYMBOL_GPL(kernel_read_file);
-int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
+int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
loff_t max_size, enum kernel_read_file_id id)
{
struct file *file;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2d0e6748e46e..58855daba1eb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2791,7 +2791,7 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
enum kernel_read_file_id);
-extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t,
+extern int kernel_read_file_from_path(const char *, void **, loff_t *, loff_t,
enum kernel_read_file_id);
extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t,
enum kernel_read_file_id);
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 06554c448dce..8112cdeeee3c 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -112,13 +112,15 @@ int __init integrity_init_keyring(const unsigned int id)
int __init integrity_load_x509(const unsigned int id, const char *path)
{
key_ref_t key;
- char *data;
+ void *data = NULL;
+ loff_t size;
int rc;
if (!keyring[id])
return -EINVAL;
- rc = integrity_read_file(path, &data);
+ rc = kernel_read_file_from_path(path, data, &size,
+ 0, READING_POLICY);
if (rc < 0)
return rc;
@@ -139,6 +141,6 @@ int __init integrity_load_x509(const unsigned int id, const char *path)
key_ref_to_ptr(key)->description, path);
key_ref_put(key);
}
- kfree(data);
+ vfree(data);
return 0;
}
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 6fc888ca468e..c84e05866052 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -200,55 +200,6 @@ int integrity_kernel_read(struct file *file, loff_t offset,
}
/*
- * integrity_read_file - read entire file content into the buffer
- *
- * This is function opens a file, allocates the buffer of required
- * size, read entire file content to the buffer and closes the file
- *
- * It is used only by init code.
- *
- */
-int __init integrity_read_file(const char *path, char **data)
-{
- struct file *file;
- loff_t size;
- char *buf;
- int rc = -EINVAL;
-
- if (!path || !*path)
- return -EINVAL;
-
- file = filp_open(path, O_RDONLY, 0);
- if (IS_ERR(file)) {
- rc = PTR_ERR(file);
- pr_err("Unable to open file: %s (%d)", path, rc);
- return rc;
- }
-
- size = i_size_read(file_inode(file));
- if (size <= 0)
- goto out;
-
- buf = kmalloc(size, GFP_KERNEL);
- if (!buf) {
- rc = -ENOMEM;
- goto out;
- }
-
- rc = integrity_kernel_read(file, 0, buf, size);
- if (rc == size) {
- *data = buf;
- } else {
- kfree(buf);
- if (rc >= 0)
- rc = -EIO;
- }
-out:
- fput(file);
- return rc;
-}
-
-/*
* integrity_load_keys - load integrity keys hook
*
* Hooks is called from init/main.c:kernel_init_freeable()
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index a53e7e4ab06c..e1bf040fb110 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -120,8 +120,6 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
int integrity_kernel_read(struct file *file, loff_t offset,
void *addr, unsigned long count);
-int __init integrity_read_file(const char *path, char **data);
-
#define INTEGRITY_KEYRING_EVM 0
#define INTEGRITY_KEYRING_IMA 1
#define INTEGRITY_KEYRING_MODULE 2
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Mimi Zohar <zohar@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-09-10 16:10 +0200 |
| Message-ID | <uob5T-4RA-11@gated-at.bofh.it> |
| In reply to | #1729938 |
On Sun, 2017-09-10 at 01:10 -0700, Christoph Hellwig wrote:
> On Fri, Sep 08, 2017 at 10:25:53AM -0700, Linus Torvalds wrote:
> > I don't think anybody actually tests linux-next kernels in any big
> > way, and the automated tests that do get run probably don't run with
> > any integrity checking enabled.
>
> Well, for the atual IMA deadlock issue I asked Mimi to produce automated
> tests and we get started on it. I was pretty pissed about the
> assumptions IMA made on the fs, whch weren't documented or automatically
> tested - coming from the XFS background where we want all our features
> to run through automated tests that was just not how I'd expect thing
> to work.
>
> But now as part of that I messed up the other caller of it, so I
> shouldn't complain too loud..
>
> That being said - I really hink the certificate loading should not
> even go thorught this whole call path, but use our common helper to
> load a file into a buffer. Something like the patch below, I'm just
> not sure if the last policy argument is what people want or if we'd need
> to add a new policy type for certificates.
We need to differentiate between policies and x509 certificates. In
the policy case, they need to be signed and appraised, while in the
x509 certificate case, the certificate itself is signed so the file
doesn't need to be signed or verified.
>
> ---
> From 3092ca95dfeff0dbd4e03ae62a51762facb15592 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Sun, 10 Sep 2017 09:49:45 +0200
> Subject: security/digisig: read file using kernel_read_file_from_path
>
> This avoid using the new integrity_read file operation which requires
> i_rwsem already to be held, and avoids a lot of code duplication and
> call the proper LSM hooks.
>
> [also constifies the path argument to kernel_read_file_from_path,
> as the callers needs it. Should probably be split into a separate patch]
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/exec.c | 2 +-
> include/linux/fs.h | 2 +-
> security/integrity/digsig.c | 8 ++++---
> security/integrity/iint.c | 49 ------------------------------------------
> security/integrity/integrity.h | 2 --
> 5 files changed, 7 insertions(+), 56 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 01a9fb9d8ac3..957a8ce294af 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -974,7 +974,7 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size,
> }
> EXPORT_SYMBOL_GPL(kernel_read_file);
>
> -int kernel_read_file_from_path(char *path, void **buf, loff_t *size,
> +int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
> loff_t max_size, enum kernel_read_file_id id)
> {
> struct file *file;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 2d0e6748e46e..58855daba1eb 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2791,7 +2791,7 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
> extern int kernel_read(struct file *, loff_t, char *, unsigned long);
> extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
> enum kernel_read_file_id);
> -extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t,
> +extern int kernel_read_file_from_path(const char *, void **, loff_t *, loff_t,
> enum kernel_read_file_id);
> extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t,
> enum kernel_read_file_id);
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 06554c448dce..8112cdeeee3c 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -112,13 +112,15 @@ int __init integrity_init_keyring(const unsigned int id)
> int __init integrity_load_x509(const unsigned int id, const char *path)
> {
> key_ref_t key;
> - char *data;
> + void *data = NULL;
> + loff_t size;
> int rc;
>
> if (!keyring[id])
> return -EINVAL;
>
> - rc = integrity_read_file(path, &data);
> + rc = kernel_read_file_from_path(path, data, &size,
> + 0, READING_POLICY);
READING_X509_CERT?
> if (rc < 0)
> return rc;
>
> @@ -139,6 +141,6 @@ int __init integrity_load_x509(const unsigned int id, const char *path)
> key_ref_to_ptr(key)->description, path);
> key_ref_put(key);
> }
> - kfree(data);
> + vfree(data);
> return 0;
> }
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 6fc888ca468e..c84e05866052 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -200,55 +200,6 @@ int integrity_kernel_read(struct file *file, loff_t offset,
> }
>
> /*
> - * integrity_read_file - read entire file content into the buffer
> - *
> - * This is function opens a file, allocates the buffer of required
> - * size, read entire file content to the buffer and closes the file
> - *
> - * It is used only by init code.
> - *
> - */
> -int __init integrity_read_file(const char *path, char **data)
> -{
> - struct file *file;
> - loff_t size;
> - char *buf;
> - int rc = -EINVAL;
> -
> - if (!path || !*path)
> - return -EINVAL;
> -
> - file = filp_open(path, O_RDONLY, 0);
> - if (IS_ERR(file)) {
> - rc = PTR_ERR(file);
> - pr_err("Unable to open file: %s (%d)", path, rc);
> - return rc;
> - }
> -
> - size = i_size_read(file_inode(file));
> - if (size <= 0)
> - goto out;
> -
> - buf = kmalloc(size, GFP_KERNEL);
> - if (!buf) {
> - rc = -ENOMEM;
> - goto out;
> - }
> -
> - rc = integrity_kernel_read(file, 0, buf, size);
> - if (rc == size) {
> - *data = buf;
> - } else {
> - kfree(buf);
> - if (rc >= 0)
> - rc = -EIO;
> - }
> -out:
> - fput(file);
> - return rc;
> -}
> -
> -/*
> * integrity_load_keys - load integrity keys hook
> *
> * Hooks is called from init/main.c:kernel_init_freeable()
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index a53e7e4ab06c..e1bf040fb110 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -120,8 +120,6 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
> int integrity_kernel_read(struct file *file, loff_t offset,
> void *addr, unsigned long count);
>
> -int __init integrity_read_file(const char *path, char **data);
> -
> #define INTEGRITY_KEYRING_EVM 0
> #define INTEGRITY_KEYRING_IMA 1
> #define INTEGRITY_KEYRING_MODULE 2
[toc] | [prev] | [next] | [standalone]
| From | Christoph Hellwig <hch@infradead.org> |
|---|---|
| Date | 2017-09-11 08:40 +0200 |
| Message-ID | <uoqxX-7vR-11@gated-at.bofh.it> |
| In reply to | #1729981 |
On Sun, Sep 10, 2017 at 10:02:42AM -0400, Mimi Zohar wrote: > We need to differentiate between policies and x509 certificates. In > the policy case, they need to be signed and appraised, while in the > x509 certificate case, the certificate itself is signed so the file > doesn't need to be signed or verified. How about you take this sketch over - I don't know much about the integrity code, and it seems like you actually wrote kernel_read_file_from_path as well - so you're at least 3 times as qualified as I am in this area..
[toc] | [prev] | [next] | [standalone]
| From | Mimi Zohar <zohar@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-09-11 23:40 +0200 |
| Message-ID | <uoEAW-nQ-11@gated-at.bofh.it> |
| In reply to | #1730164 |
On Sun, 2017-09-10 at 23:38 -0700, Christoph Hellwig wrote: > On Sun, Sep 10, 2017 at 10:02:42AM -0400, Mimi Zohar wrote: > > We need to differentiate between policies and x509 certificates. In > > the policy case, they need to be signed and appraised, while in the > > x509 certificate case, the certificate itself is signed so the file > > doesn't need to be signed or verified. > > How about you take this sketch over - I don't know much about the > integrity code, and it seems like you actually wrote > kernel_read_file_from_path as well - so you're at least 3 times as > qualified as I am in this area.. Sure, it's been on my to do list. Mimi
[toc] | [prev] | [next] | [standalone]
| From | Theodore Ts'o <tytso@mit.edu> |
|---|---|
| Date | 2017-09-09 00:40 +0200 |
| Message-ID | <unA6l-4Pc-3@gated-at.bofh.it> |
| In reply to | #1728587 |
On Fri, Sep 08, 2017 at 02:48:51PM +1000, James Morris wrote:
>
> Mimi and Christoph worked together on this over several iterations -- I'll
> let them respond.
Mimi --- we should chat next week in LA. I've been working on a
design internally at work which proposes a generic VFS-layer library
(ala how fscrypt in fs/crypto works) which provides data integrity
using per-file Merkle trees.
The goals of this design:
* Simplicity; for ease in security review and upstream review and
acceptance
* Useful for multiple use cases. It is *not* Android/APK specific,
and indeed can be used for other things
* A better way of providing Linux IMA/EVM support for immutable
files by moving the verification from time-of-open to
time-of-readpage. (This significantly reduces the performance
impact, since we don't need to lock down the file while the kernel
needs to run SHA1 on potentially gigabytes worth of file data.)
* Most use cases for file-level checksums are for files that
don’t change over time (e.g., for Video, Audio, Backup files,
etc.) This allows us to provide a cheap and efficient way to
provide checksum protect against storage-level corruption
fairly easily. So by supporting both SHA and CRC-32, we can
make this feature useful for more than just the security heads. :-)
* Like the encryption/fscrypt feature, most of the code to this
feature can be in a VFS-level library, with minimal hooks needed
to those file systems (ext4, f2fs) that wish to provide this
functionality.
- Ted
[toc] | [prev] | [next] | [standalone]
| From | James Morris <jmorris@namei.org> |
|---|---|
| Date | 2017-09-10 04:10 +0200 |
| Message-ID | <unZR8-5AJ-7@gated-at.bofh.it> |
| In reply to | #1729335 |
On Fri, 8 Sep 2017, Theodore Ts'o wrote: > On Fri, Sep 08, 2017 at 02:48:51PM +1000, James Morris wrote: > > > > Mimi and Christoph worked together on this over several iterations -- I'll > > let them respond. > > Mimi --- we should chat next week in LA. I've been working on a > design internally at work which proposes a generic VFS-layer library > (ala how fscrypt in fs/crypto works) which provides data integrity > using per-file Merkle trees. It's possible we could add a BoF at LSS on the Thursday (from 5:45pm), or even Friday afternoon: http://events.linuxfoundation.org/events/linux-security-summit/program/schedule Let me know if you want to schedule something. -- James Morris <jmorris@namei.org>
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web