| Message ID | 161377712068.87807.12246856567527156637.stgit@sifl (mailing list archive) |
|---|---|
| Headers | show |
| Series | Split security_task_getsecid() into subj and obj variants | expand |
On 2/19/2021 3:28 PM, Paul Moore wrote: > As discussed briefly on the list (lore link below), we are a little > sloppy when it comes to using task credentials, mixing both the > subjective and object credentials. This patch set attempts to fix > this by replacing security_task_getsecid() with two new hooks that > return either the subjective (_subj) or objective (_obj) credentials. > > https://lore.kernel.org/linux-security-module/806848326.0ifERbkFSE@x2/T/ > > Casey and John, I made a quick pass through the Smack and AppArmor > code in an effort to try and do the right thing, but I will admit > that I haven't tested those changes, just the SELinux code. I > would really appreciate your help in reviewing those changes. If > you find it easier, feel free to wholesale replace my Smack/AppArmor > patch with one of your own. A quick test pass didn't show up anything obviously amiss with the Smack changes. I have will do some more through inspection, but they look fine so far. > > --- > > Paul Moore (4): > lsm: separate security_task_getsecid() into subjective and objective variants > selinux: clarify task subjective and objective credentials > smack: differentiate between subjective and objective task credentials > apparmor: differentiate between subjective and objective task credentials > > > security/apparmor/domain.c | 2 +- > security/apparmor/include/cred.h | 19 +++++-- > security/apparmor/include/task.h | 3 +- > security/apparmor/lsm.c | 23 ++++++--- > security/apparmor/task.c | 23 +++++++-- > security/selinux/hooks.c | 85 ++++++++++++++++++-------------- > security/smack/smack.h | 18 ++++++- > security/smack/smack_lsm.c | 40 ++++++++++----- > 8 files changed, 147 insertions(+), 66 deletions(-) > > -- > Signature
On Fri, Feb 19, 2021 at 8:49 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 2/19/2021 3:28 PM, Paul Moore wrote: > > As discussed briefly on the list (lore link below), we are a little > > sloppy when it comes to using task credentials, mixing both the > > subjective and object credentials. This patch set attempts to fix > > this by replacing security_task_getsecid() with two new hooks that > > return either the subjective (_subj) or objective (_obj) credentials. > > > > https://lore.kernel.org/linux-security-module/806848326.0ifERbkFSE@x2/T/ > > > > Casey and John, I made a quick pass through the Smack and AppArmor > > code in an effort to try and do the right thing, but I will admit > > that I haven't tested those changes, just the SELinux code. I > > would really appreciate your help in reviewing those changes. If > > you find it easier, feel free to wholesale replace my Smack/AppArmor > > patch with one of your own. > > A quick test pass didn't show up anything obviously > amiss with the Smack changes. I have will do some more > through inspection, but they look fine so far. Thanks for testing it out and giving it a look. Beyond the Smack specific changes, I'm also interested in making sure all the hook callers are correct; I believe I made the correct substitutions, but a second (or third (or fourth ...)) set of eyes is never a bad idea.
On 2/20/2021 6:41 AM, Paul Moore wrote: > On Fri, Feb 19, 2021 at 8:49 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 2/19/2021 3:28 PM, Paul Moore wrote: >>> As discussed briefly on the list (lore link below), we are a little >>> sloppy when it comes to using task credentials, mixing both the >>> subjective and object credentials. This patch set attempts to fix >>> this by replacing security_task_getsecid() with two new hooks that >>> return either the subjective (_subj) or objective (_obj) credentials. >>> >>> https://lore.kernel.org/linux-security-module/806848326.0ifERbkFSE@x2/T/ >>> >>> Casey and John, I made a quick pass through the Smack and AppArmor >>> code in an effort to try and do the right thing, but I will admit >>> that I haven't tested those changes, just the SELinux code. I >>> would really appreciate your help in reviewing those changes. If >>> you find it easier, feel free to wholesale replace my Smack/AppArmor >>> patch with one of your own. >> A quick test pass didn't show up anything obviously >> amiss with the Smack changes. I have will do some more >> through inspection, but they look fine so far. > Thanks for testing it out and giving it a look. Beyond the Smack > specific changes, I'm also interested in making sure all the hook > callers are correct; I believe I made the correct substitutions, but a > second (or third (or fourth ...)) set of eyes is never a bad idea. I'm still not seeing anything that looks wrong. I'd suggest that Mimi have a look at the IMA bits.
On Mon, 2021-02-22 at 15:58 -0800, Casey Schaufler wrote: > On 2/20/2021 6:41 AM, Paul Moore wrote: > > On Fri, Feb 19, 2021 at 8:49 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > >> On 2/19/2021 3:28 PM, Paul Moore wrote: > >>> As discussed briefly on the list (lore link below), we are a little > >>> sloppy when it comes to using task credentials, mixing both the > >>> subjective and object credentials. This patch set attempts to fix > >>> this by replacing security_task_getsecid() with two new hooks that > >>> return either the subjective (_subj) or objective (_obj) credentials. > >>> > >>> https://lore.kernel.org/linux-security-module/806848326.0ifERbkFSE@x2/T/ > >>> > >>> Casey and John, I made a quick pass through the Smack and AppArmor > >>> code in an effort to try and do the right thing, but I will admit > >>> that I haven't tested those changes, just the SELinux code. I > >>> would really appreciate your help in reviewing those changes. If > >>> you find it easier, feel free to wholesale replace my Smack/AppArmor > >>> patch with one of your own. > >> A quick test pass didn't show up anything obviously > >> amiss with the Smack changes. I have will do some more > >> through inspection, but they look fine so far. > > Thanks for testing it out and giving it a look. Beyond the Smack > > specific changes, I'm also interested in making sure all the hook > > callers are correct; I believe I made the correct substitutions, but a > > second (or third (or fourth ...)) set of eyes is never a bad idea. > > I'm still not seeing anything that looks wrong. I'd suggest that Mimi > have a look at the IMA bits. Thanks, Casey, Paul. The IMA changes look fine. IMA policy rules are normally written in terms of a file's LSM labels, the obj_type, so hopefully this change has minimal, if any, impact. Mimi
On Tue, Feb 23, 2021 at 9:14 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > On Mon, 2021-02-22 at 15:58 -0800, Casey Schaufler wrote: > > On 2/20/2021 6:41 AM, Paul Moore wrote: > > > On Fri, Feb 19, 2021 at 8:49 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > >> On 2/19/2021 3:28 PM, Paul Moore wrote: > > >>> As discussed briefly on the list (lore link below), we are a little > > >>> sloppy when it comes to using task credentials, mixing both the > > >>> subjective and object credentials. This patch set attempts to fix > > >>> this by replacing security_task_getsecid() with two new hooks that > > >>> return either the subjective (_subj) or objective (_obj) credentials. > > >>> > > >>> https://lore.kernel.org/linux-security-module/806848326.0ifERbkFSE@x2/T/ > > >>> > > >>> Casey and John, I made a quick pass through the Smack and AppArmor > > >>> code in an effort to try and do the right thing, but I will admit > > >>> that I haven't tested those changes, just the SELinux code. I > > >>> would really appreciate your help in reviewing those changes. If > > >>> you find it easier, feel free to wholesale replace my Smack/AppArmor > > >>> patch with one of your own. > > >> A quick test pass didn't show up anything obviously > > >> amiss with the Smack changes. I have will do some more > > >> through inspection, but they look fine so far. > > > Thanks for testing it out and giving it a look. Beyond the Smack > > > specific changes, I'm also interested in making sure all the hook > > > callers are correct; I believe I made the correct substitutions, but a > > > second (or third (or fourth ...)) set of eyes is never a bad idea. > > > > I'm still not seeing anything that looks wrong. I'd suggest that Mimi > > have a look at the IMA bits. > > Thanks, Casey, Paul. The IMA changes look fine. IMA policy rules are > normally written in terms of a file's LSM labels, the obj_type, so > hopefully this change has minimal, if any, impact. Thanks Mimi I appreciate the additional review. Would you mind sending your ACK for the IMA related patches in the patchset?
On Mon, Feb 22, 2021 at 6:59 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 2/20/2021 6:41 AM, Paul Moore wrote: > > On Fri, Feb 19, 2021 at 8:49 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > >> On 2/19/2021 3:28 PM, Paul Moore wrote: > >>> As discussed briefly on the list (lore link below), we are a little > >>> sloppy when it comes to using task credentials, mixing both the > >>> subjective and object credentials. This patch set attempts to fix > >>> this by replacing security_task_getsecid() with two new hooks that > >>> return either the subjective (_subj) or objective (_obj) credentials. > >>> > >>> https://lore.kernel.org/linux-security-module/806848326.0ifERbkFSE@x2/T/ > >>> > >>> Casey and John, I made a quick pass through the Smack and AppArmor > >>> code in an effort to try and do the right thing, but I will admit > >>> that I haven't tested those changes, just the SELinux code. I > >>> would really appreciate your help in reviewing those changes. If > >>> you find it easier, feel free to wholesale replace my Smack/AppArmor > >>> patch with one of your own. > >> A quick test pass didn't show up anything obviously > >> amiss with the Smack changes. I have will do some more > >> through inspection, but they look fine so far. > > Thanks for testing it out and giving it a look. Beyond the Smack > > specific changes, I'm also interested in making sure all the hook > > callers are correct; I believe I made the correct substitutions, but a > > second (or third (or fourth ...)) set of eyes is never a bad idea. > > I'm still not seeing anything that looks wrong. I'd suggest that Mimi > have a look at the IMA bits. Assuming you are still good with these changes Casey, any chance I can get an ACK on the LSM and Smack patches?
On 3/3/2021 4:46 PM, Paul Moore wrote: > On Mon, Feb 22, 2021 at 6:59 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 2/20/2021 6:41 AM, Paul Moore wrote: >>> On Fri, Feb 19, 2021 at 8:49 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >>>> On 2/19/2021 3:28 PM, Paul Moore wrote: >>>>> As discussed briefly on the list (lore link below), we are a little >>>>> sloppy when it comes to using task credentials, mixing both the >>>>> subjective and object credentials. This patch set attempts to fix >>>>> this by replacing security_task_getsecid() with two new hooks that >>>>> return either the subjective (_subj) or objective (_obj) credentials. >>>>> >>>>> https://lore.kernel.org/linux-security-module/806848326.0ifERbkFSE@x2/T/ >>>>> >>>>> Casey and John, I made a quick pass through the Smack and AppArmor >>>>> code in an effort to try and do the right thing, but I will admit >>>>> that I haven't tested those changes, just the SELinux code. I >>>>> would really appreciate your help in reviewing those changes. If >>>>> you find it easier, feel free to wholesale replace my Smack/AppArmor >>>>> patch with one of your own. >>>> A quick test pass didn't show up anything obviously >>>> amiss with the Smack changes. I have will do some more >>>> through inspection, but they look fine so far. >>> Thanks for testing it out and giving it a look. Beyond the Smack >>> specific changes, I'm also interested in making sure all the hook >>> callers are correct; I believe I made the correct substitutions, but a >>> second (or third (or fourth ...)) set of eyes is never a bad idea. >> I'm still not seeing anything that looks wrong. I'd suggest that Mimi >> have a look at the IMA bits. > Assuming you are still good with these changes Casey, any chance I can > get an ACK on the LSM and Smack patches? Yes. You can add my: Acked-by: Casey Schaufler <casey@schaufler-ca.com> to both.
On Wed, Mar 3, 2021 at 9:21 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 3/3/2021 4:46 PM, Paul Moore wrote: ... > > Assuming you are still good with these changes Casey, any chance I can > > get an ACK on the LSM and Smack patches? > > Yes. You can add my: > > Acked-by: Casey Schaufler <casey@schaufler-ca.com> > > to both. Done, thanks Casey. I talked to John and he is working on the AppArmor tweaks so I'll hold off reposting until I see an update from him (nothing beyond the ACKs has changed anyway).
As discussed briefly on the list (lore link below), we are a little sloppy when it comes to using task credentials, mixing both the subjective and object credentials. This patch set attempts to fix this by replacing security_task_getsecid() with two new hooks that return either the subjective (_subj) or objective (_obj) credentials. https://lore.kernel.org/linux-security-module/806848326.0ifERbkFSE@x2/T/ Casey and John, I made a quick pass through the Smack and AppArmor code in an effort to try and do the right thing, but I will admit that I haven't tested those changes, just the SELinux code. I would really appreciate your help in reviewing those changes. If you find it easier, feel free to wholesale replace my Smack/AppArmor patch with one of your own. --- Paul Moore (4): lsm: separate security_task_getsecid() into subjective and objective variants selinux: clarify task subjective and objective credentials smack: differentiate between subjective and objective task credentials apparmor: differentiate between subjective and objective task credentials security/apparmor/domain.c | 2 +- security/apparmor/include/cred.h | 19 +++++-- security/apparmor/include/task.h | 3 +- security/apparmor/lsm.c | 23 ++++++--- security/apparmor/task.c | 23 +++++++-- security/selinux/hooks.c | 85 ++++++++++++++++++-------------- security/smack/smack.h | 18 ++++++- security/smack/smack_lsm.c | 40 ++++++++++----- 8 files changed, 147 insertions(+), 66 deletions(-) -- Signature