Hi Linus,
I'm maintaining the new SafeSetID LSM and was told to set up my own
tree for sending pull requests rather than sending my changes through
James Morris and the security subsystem tree.
This is my first time doing one of these pull requests so hopefully I
didn't screw something up.
Thanks,
Micah
---
The following changes since commit fec88ab0af9706b2201e5daf377c5031c62d11f7:
Merge tag 'for-linus-hmm' of
git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma (2019-07-14
19:42:11 -0700)
are available in the Git repository at:
https://github.com/micah-morton/linux.git tags/safesetid-5.3
for you to fetch changes up to e10337daefecb47209fd2af5f4fab0d1a370737f:
LSM: SafeSetID: fix use of literal -1 in capable hook (2019-07-15
08:08:03 -0700)
----------------------------------------------------------------
SafeSetID patches for 5.3
These changes from Jann Horn fix a couple issues in the recently added
SafeSetID LSM:
(1) There was a simple logic bug in one of the hooks for the LSM where
the code was incorrectly returning early in some cases before all
security checks had been passed.
(2) There was a more high level issue with how this LSM gets configured
that could allow for a program to bypass the security restrictions
by switching to an allowed UID and then again to any other UID on
the system if the target UID of the first transition is
unconstrained on the system. Luckily this is an easy fix that we now
enforce at the time the LSM gets configured.
There are also some changes from Jann that make policy updates for this
LSM atomic. Kees Cook, Jann and myself have reviewed these changes and they
look good from our point of view.
Signed-off-by: Micah Morton <[email protected]>
----------------------------------------------------------------
Jann Horn (10):
LSM: SafeSetID: fix pr_warn() to include newline
LSM: SafeSetID: fix check for setresuid(new1, new2, new3)
LSM: SafeSetID: refactor policy hash table
LSM: SafeSetID: refactor safesetid_security_capable()
LSM: SafeSetID: refactor policy parsing
LSM: SafeSetID: fix userns handling in securityfs
LSM: SafeSetID: rewrite userspace API to atomic updates
LSM: SafeSetID: add read handler
LSM: SafeSetID: verify transitive constrainedness
LSM: SafeSetID: fix use of literal -1 in capable hook
security/safesetid/lsm.c | 276 +++++++++++++-----------------------------
security/safesetid/lsm.h | 34 ++++--
security/safesetid/securityfs.c | 307
+++++++++++++++++++++++++++++------------------
tools/testing/selftests/safesetid/safesetid-test.c | 18 ++-
4 files changed, 306 insertions(+), 329 deletions(-)
On Mon, Jul 15, 2019 at 9:05 AM Micah Morton <[email protected]> wrote:
>
> I'm maintaining the new SafeSetID LSM and was told to set up my own
> tree for sending pull requests rather than sending my changes through
> James Morris and the security subsystem tree.
Yes. It would be good if you also added yourself to the MAINTAINERS
file. Right now there's no entry for security/safesetid at all.
> This is my first time doing one of these pull requests so hopefully I
> didn't screw something up.
So a couple of notes:
- *please* don't rebase your work in the day before
Was this in linux-next? was this tested at all? Hard to tell, since
it was rebased recently, so for all I know it's all completely new
- don't use a random kernel-of-the-day as the base for development
This is related to the rebasing issue, but is true even if you
don't rebase. There is no way that it was a good idea to pick my
random - possibly completely broken - kernel from Sunday afternoon in
the middle of a merge window as a base for development.
If you start development, or if you have to rebase (for some *good*
reason) you need to do so on a good stable base, not on the quick-sand
that is "random kernel of the day during the busiest merge activity".
- Please use the "git pull-request" format and then add any extra
notes you feel are necessary
Yes, your pull request is *almost* git pull-request, but you seem
to have actively removed whitespace making it almost illegible. It's
really hard to pick out the line that has the actual git repository
address, because it's basically hidden inside one big blob of text.
I've pulled this as-is since it's the first time, but I expect better next time.
There are various resources on some cleanliness issues, and people
fairly recently tried to combine it under
Documentation/maintainer/rebasing-and-merging.rst
which covers at least the basics on why not to rebase etc.
And if you *do* end up rebasing, consider the end result "untested",
so then it should have been done before the merge window even started,
and the rebased branch should have been in linux-next. And not sent to
me the very next day.
Linus
On Tue, Jul 16, 2019 at 12:06 PM Linus Torvalds
<[email protected]> wrote:
>
> - Please use the "git pull-request" format and then add any extra
> notes you feel are necessary
>
> Yes, your pull request is *almost* git pull-request, but you seem
> to have actively removed whitespace making it almost illegible. It's
> really hard to pick out the line that has the actual git repository
> address, because it's basically hidden inside one big blob of text.
Extra note on this: you seem to have done "git pull-request" in a wide
window, and then copied-and-pasted it into your MUA.
So the diffstat lines are also very long, and then they line-wrap and
it all looks nasty.
Avoid this by either using a file for the output (that you then edit
for your own added messages), or using a normal 80x25 terminal or
something.
I guess I should ask Junio to add a "--stat=80" to the upstream git
request-pull script.
Linus
The pull request you sent on Mon, 15 Jul 2019 09:04:48 -0700:
> https://github.com/micah-morton/linux.git tags/safesetid-5.3
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/1ec4013bab89058dcc594dfe7b5a20f5d46bbc5f
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
On Tue, Jul 16, 2019 at 12:06 PM Linus Torvalds
<[email protected]> wrote:
>
> On Mon, Jul 15, 2019 at 9:05 AM Micah Morton <[email protected]> wrote:
> >
> > I'm maintaining the new SafeSetID LSM and was told to set up my own
> > tree for sending pull requests rather than sending my changes through
> > James Morris and the security subsystem tree.
>
> Yes. It would be good if you also added yourself to the MAINTAINERS
> file. Right now there's no entry for security/safesetid at all.
Yes, I have a patch for this but was told it would be better to send
the patch through my tree rather than the security tree. I can send a
pull request for that.
>
> > This is my first time doing one of these pull requests so hopefully I
> > didn't screw something up.
>
> So a couple of notes:
>
> - *please* don't rebase your work in the day before
Got it.
>
> Was this in linux-next? was this tested at all? Hard to tell, since
> it was rebased recently, so for all I know it's all completely new
This was not in linux-next, but was tested by Jann on a Chrome OS
device. There's also the selftest for this code. But I can send
non-trivial stuff to linux-next first next time.
>
> - don't use a random kernel-of-the-day as the base for development
Got it.
>
> This is related to the rebasing issue, but is true even if you
> don't rebase. There is no way that it was a good idea to pick my
> random - possibly completely broken - kernel from Sunday afternoon in
> the middle of a merge window as a base for development.
>
> If you start development, or if you have to rebase (for some *good*
> reason) you need to do so on a good stable base, not on the quick-sand
> that is "random kernel of the day during the busiest merge activity".
Makes sense. The development was not actually done on that kernel, I
just grabbed that random kernel for committing the changes on top of
(these changes were developed a little while ago, but they're all self
contained to the SafeSetID LSM), but I'll pick a stable one next time.
>
> - Please use the "git pull-request" format and then add any extra
> notes you feel are necessary
>
> Yes, your pull request is *almost* git pull-request, but you seem
> to have actively removed whitespace making it almost illegible. It's
> really hard to pick out the line that has the actual git repository
> address, because it's basically hidden inside one big blob of text.
>
> I've pulled this as-is since it's the first time, but I expect better next time.
>
> There are various resources on some cleanliness issues, and people
> fairly recently tried to combine it under
>
> Documentation/maintainer/rebasing-and-merging.rst
>
> which covers at least the basics on why not to rebase etc.
Thanks for the pointer. I had not seen that yet.
>
> And if you *do* end up rebasing, consider the end result "untested",
> so then it should have been done before the merge window even started,
> and the rebased branch should have been in linux-next. And not sent to
> me the very next day.
Yep, makes sense.
>
> Linus
Thanks!