[GIT PULL] SafeSetID LSM changes for v5.8

[GIT PULL] SafeSetID LSM changes for v5.8

[Micah Morton]

The following changes since commit 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162:

  Linux 5.7 (2020-05-31 16:49:15 -0700)

are available in the Git repository at:

  https://github.com/micah-morton/linux.git tags/LSM-add-setgid-hook-5.8

for you to fetch changes up to 04d244bcf92f525011e3df34b21fc39b0591ba93:

  security: Add LSM hooks to set*gid syscalls (2020-06-09 10:22:13 -0700)

----------------------------------------------------------------
Add additional LSM hooks for SafeSetID

SafeSetID is capable of making allow/deny decisions for set*uid calls
on a system, and we want to add similar functionality for set*gid
calls. The work to do that is not yet complete, so probably won't make
it in for v5.8, but we are looking to get this simple patch in for
v5.8 since we have it ready. We are planning on the rest of the work
for extending the SafeSetID LSM being merged during the v5.9 merge
window.

This patch was sent to the security mailing list and there were no objections.

Signed-off-by: Micah Morton <mortonm@chromium.org>

----------------------------------------------------------------
Micah Morton (1):
      security: Add LSM hooks to set*gid syscalls

 include/linux/lsm_hook_defs.h |  2 ++
 include/linux/lsm_hooks.h     |  9 +++++++++
 include/linux/security.h      |  9 +++++++++
 kernel/sys.c                  | 15 ++++++++++++++-
 security/security.c           |  6 ++++++
 5 files changed, 40 insertions(+), 1 deletion(-)

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[Linus Torvalds]

Finally emptied my normal pull request queue and starting to look at
things I wanted to look at more closely..

On Tue, Jun 9, 2020 at 11:26 AM Micah Morton <mortonm@chromium.org> wrote:
>
> This patch was sent to the security mailing list and there were no objections.

That patch as committed has both the wrong authorship, and the wrong
sign-off chain.

Not pulling.

                 Linus

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[Micah Morton]

I amended the author on the lone commit in this pull request. For some
reason I was thinking using the "From:" line in the commit body was
how I should make things show up as Thomas as the author and me as the
committer, but looks like that’s not true.

I also removed my own Signed-off-by line from the pull request body
and included it in the commit instead of the Reviewed-by line.

Thanks,
Micah


The following changes since commit 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162:

  Linux 5.7 (2020-05-31 16:49:15 -0700)

are available in the Git repository at:

  https://github.com/micah-morton/linux.git
tags/LSM-add-setgid-hook-5.8-author-fix

for you to fetch changes up to 39030e1351aa1aa7443bb2da24426573077c83da:

  security: Add LSM hooks to set*gid syscalls (2020-06-14 10:52:02 -0700)

----------------------------------------------------------------
Add additional LSM hooks for SafeSetID

SafeSetID is capable of making allow/deny decisions for set*uid calls
on a system, and we want to add similar functionality for set*gid
calls. The work to do that is not yet complete, so probably won't make
it in for v5.8, but we are looking to get this simple patch in for
v5.8 since we have it ready. We are planning on the rest of the work
for extending the SafeSetID LSM being merged during the v5.9 merge
window.

This patch was sent to the security mailing list and there were no objections.

----------------------------------------------------------------
Thomas Cedeno (1):
      security: Add LSM hooks to set*gid syscalls

 include/linux/lsm_hook_defs.h |  2 ++
 include/linux/lsm_hooks.h     |  9 +++++++++
 include/linux/security.h      |  9 +++++++++
 kernel/sys.c                  | 15 ++++++++++++++-
 security/security.c           |  6 ++++++
 5 files changed, 40 insertions(+), 1 deletion(-)

On Fri, Jun 12, 2020 at 2:23 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Finally emptied my normal pull request queue and starting to look at
> things I wanted to look at more closely..
>
> On Tue, Jun 9, 2020 at 11:26 AM Micah Morton <mortonm@chromium.org> wrote:
> >
> > This patch was sent to the security mailing list and there were no objections.
>
> That patch as committed has both the wrong authorship, and the wrong
> sign-off chain.
>
> Not pulling.
>
>                  Linus

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[Linus Torvalds]

On Sun, Jun 14, 2020 at 11:04 AM Micah Morton <mortonm@chromium.org> wrote:
>
> I amended the author on the lone commit in this pull request. For some
> reason I was thinking using the "From:" line in the commit body was
> how I should make things show up as Thomas as the author and me as the
> committer, but looks like that’s not true.

That's how we do things in email, since you want a separate author for
the emailed patch than from the author of the email itself.

But git itself very much has that difference between "author" and
"committer" internally, and all the usual email application tools will
take the separate "From:" line from the email, and make that be the
author in git.

(And then the sign-off chain is where we describe the whole path,
because git only has the concept of those two end-points: the original
author, and the final committer, but no concept of the path in between
the two, nor does it have the concept of the copyright and license
agreement implications of the sign-offs).

> I also removed my own Signed-off-by line from the pull request body
> and included it in the commit instead of the Reviewed-by line.

Good. You will get credit for the pull request in the merge commit
itself as a "Pull xyz from Micah Morton", so that path of history gets
encoded that way.

But the sign-off chain is supposed to be there for each individual commit.

(I don't always notice those things, but afaik there is automation in
place in -next that should warn about commits with incomplete sign-off
chains. Did that not trigger for some reason in this case?).

                  Linus

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[Micah Morton]

On Sun, Jun 14, 2020 at 11:39 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, Jun 14, 2020 at 11:04 AM Micah Morton <mortonm@chromium.org> wrote:
> >
> > I amended the author on the lone commit in this pull request. For some
> > reason I was thinking using the "From:" line in the commit body was
> > how I should make things show up as Thomas as the author and me as the
> > committer, but looks like that’s not true.
>
> That's how we do things in email, since you want a separate author for
> the emailed patch than from the author of the email itself.
>
> But git itself very much has that difference between "author" and
> "committer" internally, and all the usual email application tools will
> take the separate "From:" line from the email, and make that be the
> author in git.

Ah makes sense, thanks!

>
> (And then the sign-off chain is where we describe the whole path,
> because git only has the concept of those two end-points: the original
> author, and the final committer, but no concept of the path in between
> the two, nor does it have the concept of the copyright and license
> agreement implications of the sign-offs).
>
> > I also removed my own Signed-off-by line from the pull request body
> > and included it in the commit instead of the Reviewed-by line.
>
> Good. You will get credit for the pull request in the merge commit
> itself as a "Pull xyz from Micah Morton", so that path of history gets
> encoded that way.
>
> But the sign-off chain is supposed to be there for each individual commit.
>
> (I don't always notice those things, but afaik there is automation in
> place in -next that should warn about commits with incomplete sign-off
> chains. Did that not trigger for some reason in this case?).

This change hasn't had any bake time in linux-next. I didn't deem it
sufficiently risky or complex to warrant such integration testing.

That said I'm a little fuzzy on where to draw the line for which kinds
of changes really should be required to have bake time in -next. If
you think this is one of those cases, we can hold off on this until we
have some bake time for v5.9.

Either way this is a good reminder for the v5.9 merge window when we
plan to have more featureful changes going in for SafeSetID (although
those will be completely internal to the LSM, so low chance of
breaking anything outside the module).

>
>                   Linus

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[Linus Torvalds]

On Sun, Jun 14, 2020 at 12:12 PM Micah Morton <mortonm@chromium.org> wrote:
>
> That said I'm a little fuzzy on where to draw the line for which kinds
> of changes really should be required to have bake time in -next. If
> you think this is one of those cases, we can hold off on this until we
> have some bake time for v5.9.

It's merged, but in general the rule for "bake in -next" should be
absolutely everything.

The only exception is just pure and plain fixes.

This SafeSetID change should in fact have been there for two different
reasons: not only was it a new feature rather than a fix (in
linux-next just for testing), it was one that crossed subsystem
borders (should be in linux-next just for cross-subsystem testing). It
touched files that very much aren't touched by just you.

"Looks obvious" has nothing to do with avoiding linux-next.

I suspect most of the bugs we have tend to be in code that "looked
obvious" to somebody.

                     Linus

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[Micah Morton]

On Sun, Jun 14, 2020 at 12:20 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sun, Jun 14, 2020 at 12:12 PM Micah Morton <mortonm@chromium.org> wrote:
> >
> > That said I'm a little fuzzy on where to draw the line for which kinds
> > of changes really should be required to have bake time in -next. If
> > you think this is one of those cases, we can hold off on this until we
> > have some bake time for v5.9.
>
> It's merged, but in general the rule for "bake in -next" should be
> absolutely everything.
>
> The only exception is just pure and plain fixes.

Sounds good, that makes it pretty clear.

Thanks

>
> This SafeSetID change should in fact have been there for two different
> reasons: not only was it a new feature rather than a fix (in
> linux-next just for testing), it was one that crossed subsystem
> borders (should be in linux-next just for cross-subsystem testing). It
> touched files that very much aren't touched by just you.
>
> "Looks obvious" has nothing to do with avoiding linux-next.
>
> I suspect most of the bugs we have tend to be in code that "looked
> obvious" to somebody.
>
>                      Linus

Re: Re: [GIT PULL] SafeSetID LSM changes for v5.8

[pr-tracker-bot]

The pull request you sent on Sun, 14 Jun 2020 11:03:58 -0700:

> https://github.com/micah-morton/linux.git tags/LSM-add-setgid-hook-5.8-author-fix

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/4a87b197c1da6b16608d5110709e0b3308e25dcd

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[James Morris]

On Sun, 14 Jun 2020, Micah Morton wrote:

> This patch was sent to the security mailing list and there were no objections.

Standard practice for new or modified LSM hooks is that they are reviewed 
and acked by maintainers of major LSMs (SELinux, AppArmor, and Smack, at 
least).

"No objections" should be considered "not reviewed".

Can you add your tree to linux-next?
https://www.kernel.org/doc/man-pages/linux-next.html

-- 
James Morris
<jmorris@namei.org>

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[Micah Morton]

On Sun, Jun 14, 2020 at 10:21 PM James Morris <jmorris@namei.org> wrote:
>
> On Sun, 14 Jun 2020, Micah Morton wrote:
>
> > This patch was sent to the security mailing list and there were no objections.
>
> Standard practice for new or modified LSM hooks is that they are reviewed
> and acked by maintainers of major LSMs (SELinux, AppArmor, and Smack, at
> least).
>
> "No objections" should be considered "not reviewed".
>
> Can you add your tree to linux-next?
> https://www.kernel.org/doc/man-pages/linux-next.html

Sure, I can do that. I should just send an email to Stephen Rothwell
asking him to include the -next branch from my tree?

>
> --
> James Morris
> <jmorris@namei.org>
>

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[James Morris]

On Mon, 15 Jun 2020, Micah Morton wrote:

> On Sun, Jun 14, 2020 at 10:21 PM James Morris <jmorris@namei.org> wrote:
> >
> > On Sun, 14 Jun 2020, Micah Morton wrote:
> >
> > > This patch was sent to the security mailing list and there were no objections.
> >
> > Standard practice for new or modified LSM hooks is that they are reviewed
> > and acked by maintainers of major LSMs (SELinux, AppArmor, and Smack, at
> > least).
> >
> > "No objections" should be considered "not reviewed".
> >
> > Can you add your tree to linux-next?
> > https://www.kernel.org/doc/man-pages/linux-next.html
> 
> Sure, I can do that. I should just send an email to Stephen Rothwell
> asking him to include the -next branch from my tree?

Yep, thanks.

> 
> >
> > --
> > James Morris
> > <jmorris@namei.org>
> >
> 

-- 
James Morris
<jmorris@namei.org>

Re: [GIT PULL] SafeSetID LSM changes for v5.8

[Micah Morton]

On Mon, Jun 15, 2020 at 11:03 AM James Morris <jmorris@namei.org> wrote:
>
> On Mon, 15 Jun 2020, Micah Morton wrote:
>
> > On Sun, Jun 14, 2020 at 10:21 PM James Morris <jmorris@namei.org> wrote:
> > >
> > > On Sun, 14 Jun 2020, Micah Morton wrote:
> > >
> > > > This patch was sent to the security mailing list and there were no objections.
> > >
> > > Standard practice for new or modified LSM hooks is that they are reviewed
> > > and acked by maintainers of major LSMs (SELinux, AppArmor, and Smack, at
> > > least).
> > >
> > > "No objections" should be considered "not reviewed".
> > >
> > > Can you add your tree to linux-next?
> > > https://www.kernel.org/doc/man-pages/linux-next.html
> >
> > Sure, I can do that. I should just send an email to Stephen Rothwell
> > asking him to include the -next branch from my tree?
>
> Yep, thanks.

The commit is in -next as of next-20200615

Thanks

>
> >
> > >
> > > --
> > > James Morris
> > > <jmorris@namei.org>
> > >
> >
>
> --
> James Morris
> <jmorris@namei.org>
>