* [PATCH v4 1/2] LSM: SafeSetID: gate setgid transitions
From: mortonm @ 2019-02-28 23:55 UTC (permalink / raw)
To: jmorris, serge, keescook, casey, sds, linux-security-module; +Cc: Micah Morton
In-Reply-To: <5552a810-8072-e0fe-4a4c-76f937f0c3f4@schaufler-ca.com>
From: Micah Morton <mortonm@chromium.org>
This patch generalizes the 'task_fix_setuid' LSM hook to enable hooking
setgid transitions as well as setuid transitions. The hook is renamed to
'task_fix_setid'. The patch introduces calls to this hook from the
setgid functions in kernel/sys.c. This will allow the SafeSetID LSM to
govern setgid transitions in addition to setuid transitions. This patch
also makes sure the setgid functions in kernel/sys.c call
security_capable_setid rather than the ordinary security_capable
function, so that the security_capable hook in the SafeSetID LSM knows
it is being invoked from a setid function.
Signed-off-by: Micah Morton <mortonm@chromium.org>
---
Changes since the last patch: Add break statements for the
setgid-related case statements in cap_task_fix_setid in
security/commoncap.c. We don't want those cases to fall through to the
default statement and return -EINVAL. Are the setreuid and setuid cases
for this function always returning -EINVAL or am I missing something
really obvious?.. Seems strange if that is the case.
Documentation/admin-guide/LSM/SafeSetID.rst | 2 +-
include/linux/lsm_hooks.h | 8 ++---
include/linux/security.h | 36 ++++++++++++++-------
kernel/sys.c | 35 ++++++++++++++------
security/commoncap.c | 25 +++++++++-----
security/safesetid/lsm.c | 12 +++----
security/security.c | 4 +--
7 files changed, 79 insertions(+), 43 deletions(-)
diff --git a/Documentation/admin-guide/LSM/SafeSetID.rst b/Documentation/admin-guide/LSM/SafeSetID.rst
index 212434ef65ad..670a6544fd39 100644
--- a/Documentation/admin-guide/LSM/SafeSetID.rst
+++ b/Documentation/admin-guide/LSM/SafeSetID.rst
@@ -88,7 +88,7 @@ other system interactions, including use of pid namespaces and device creation.
Use an existing LSM
-------------------
None of the other in-tree LSMs have the capability to gate setid transitions, or
-even employ the security_task_fix_setuid hook at all. SELinux says of that hook:
+even employ the security_task_fix_setid hook at all. SELinux says of that hook:
"Since setuid only affects the current process, and since the SELinux controls
are not based on the Linux identity attributes, SELinux does not need to control
this operation."
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 22fc786d723a..47fd04410fde 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -594,14 +594,14 @@
* @size length of the file contents.
* @id kernel read file identifier
* Return 0 if permission is granted.
- * @task_fix_setuid:
+ * @task_fix_setid:
* Update the module's state after setting one or more of the user
* identity attributes of the current process. The @flags parameter
* indicates which of the set*uid system calls invoked this hook. If
* @new is the set of credentials that will be installed. Modifications
* should be made to this rather than to @current->cred.
* @old is the set of credentials that are being replaces
- * @flags contains one of the LSM_SETID_* values.
+ * @flags contains one of the LSM_SET*ID_* values.
* Return 0 on success.
* @task_setpgid:
* Check permission before setting the process group identifier of the
@@ -1594,7 +1594,7 @@ union security_list_options {
int (*kernel_read_file)(struct file *file, enum kernel_read_file_id id);
int (*kernel_post_read_file)(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
- int (*task_fix_setuid)(struct cred *new, const struct cred *old,
+ int (*task_fix_setid)(struct cred *new, const struct cred *old,
int flags);
int (*task_setpgid)(struct task_struct *p, pid_t pgid);
int (*task_getpgid)(struct task_struct *p);
@@ -1886,7 +1886,7 @@ struct security_hook_heads {
struct hlist_head kernel_read_file;
struct hlist_head kernel_post_read_file;
struct hlist_head kernel_module_request;
- struct hlist_head task_fix_setuid;
+ struct hlist_head task_fix_setid;
struct hlist_head task_setpgid;
struct hlist_head task_getpgid;
struct hlist_head task_getsid;
diff --git a/include/linux/security.h b/include/linux/security.h
index 13537a49ae97..76df3e22fed1 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -95,7 +95,7 @@ extern int cap_inode_getsecurity(struct inode *inode, const char *name,
extern int cap_mmap_addr(unsigned long addr);
extern int cap_mmap_file(struct file *file, unsigned long reqprot,
unsigned long prot, unsigned long flags);
-extern int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags);
+extern int cap_task_fix_setid(struct cred *new, const struct cred *old, int flags);
extern int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
unsigned long arg4, unsigned long arg5);
extern int cap_task_setscheduler(struct task_struct *p);
@@ -128,17 +128,29 @@ extern unsigned long dac_mmap_min_addr;
/*
* Values used in the task_security_ops calls
*/
-/* setuid or setgid, id0 == uid or gid */
-#define LSM_SETID_ID 1
+/* setuid, id0 == uid */
+#define LSM_SETUID_ID 1
-/* setreuid or setregid, id0 == real, id1 == eff */
-#define LSM_SETID_RE 2
+/* setreuid, id0 == real, id1 == eff */
+#define LSM_SETUID_RE 2
-/* setresuid or setresgid, id0 == real, id1 == eff, uid2 == saved */
-#define LSM_SETID_RES 4
+/* setresuid, id0 == real, id1 == eff, uid2 == saved */
+#define LSM_SETUID_RES 4
-/* setfsuid or setfsgid, id0 == fsuid or fsgid */
-#define LSM_SETID_FS 8
+/* setfsuid, id0 == fsgid */
+#define LSM_SETUID_FS 8
+
+/* setgid, id0 == gid */
+#define LSM_SETGID_ID 16
+
+/* setregid, id0 == real, id1 == eff */
+#define LSM_SETGID_RE 32
+
+/* setresgid, id0 == real, id1 == eff, uid2 == saved */
+#define LSM_SETGID_RES 64
+
+/* setfsgid, id0 == fsgid */
+#define LSM_SETGID_FS 128
/* Flags for security_task_prlimit(). */
#define LSM_PRLIMIT_READ 1
@@ -324,7 +336,7 @@ int security_kernel_load_data(enum kernel_load_data_id id);
int security_kernel_read_file(struct file *file, enum kernel_read_file_id id);
int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
-int security_task_fix_setuid(struct cred *new, const struct cred *old,
+int security_task_fix_setid(struct cred *new, const struct cred *old,
int flags);
int security_task_setpgid(struct task_struct *p, pid_t pgid);
int security_task_getpgid(struct task_struct *p);
@@ -923,11 +935,11 @@ static inline int security_kernel_post_read_file(struct file *file,
return 0;
}
-static inline int security_task_fix_setuid(struct cred *new,
+static inline int security_task_fix_setid(struct cred *new,
const struct cred *old,
int flags)
{
- return cap_task_fix_setuid(new, old, flags);
+ return cap_task_fix_setid(new, old, flags);
}
static inline int security_task_setpgid(struct task_struct *p, pid_t pgid)
diff --git a/kernel/sys.c b/kernel/sys.c
index c5f875048aef..615b44939238 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -372,7 +372,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
if (rgid != (gid_t) -1) {
if (gid_eq(old->gid, krgid) ||
gid_eq(old->egid, krgid) ||
- ns_capable(old->user_ns, CAP_SETGID))
+ ns_capable_setid(old->user_ns, CAP_SETGID))
new->gid = krgid;
else
goto error;
@@ -381,7 +381,7 @@ long __sys_setregid(gid_t rgid, gid_t egid)
if (gid_eq(old->gid, kegid) ||
gid_eq(old->egid, kegid) ||
gid_eq(old->sgid, kegid) ||
- ns_capable(old->user_ns, CAP_SETGID))
+ ns_capable_setid(old->user_ns, CAP_SETGID))
new->egid = kegid;
else
goto error;
@@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid)
new->sgid = new->egid;
new->fsgid = new->egid;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_RE);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -427,13 +431,17 @@ long __sys_setgid(gid_t gid)
old = current_cred();
retval = -EPERM;
- if (ns_capable(old->user_ns, CAP_SETGID))
+ if (ns_capable_setid(old->user_ns, CAP_SETGID))
new->gid = new->egid = new->sgid = new->fsgid = kgid;
else if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->sgid))
new->egid = new->fsgid = kgid;
else
goto error;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_ID);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -539,7 +547,7 @@ long __sys_setreuid(uid_t ruid, uid_t euid)
new->suid = new->euid;
new->fsuid = new->euid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_RE);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_RE);
if (retval < 0)
goto error;
@@ -597,7 +605,7 @@ long __sys_setuid(uid_t uid)
new->fsuid = new->euid = kuid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_ID);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_ID);
if (retval < 0)
goto error;
@@ -672,7 +680,7 @@ long __sys_setresuid(uid_t ruid, uid_t euid, uid_t suid)
new->suid = ksuid;
new->fsuid = new->euid;
- retval = security_task_fix_setuid(new, old, LSM_SETID_RES);
+ retval = security_task_fix_setid(new, old, LSM_SETUID_RES);
if (retval < 0)
goto error;
@@ -735,7 +743,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
old = current_cred();
retval = -EPERM;
- if (!ns_capable(old->user_ns, CAP_SETGID)) {
+ if (!ns_capable_setid(old->user_ns, CAP_SETGID)) {
if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) &&
!gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid))
goto error;
@@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
new->sgid = ksgid;
new->fsgid = new->egid;
+ retval = security_task_fix_setid(new, old, LSM_SETGID_RES);
+ if (retval < 0)
+ goto error;
+
return commit_creds(new);
error:
@@ -817,7 +829,7 @@ long __sys_setfsuid(uid_t uid)
ns_capable_setid(old->user_ns, CAP_SETUID)) {
if (!uid_eq(kuid, old->fsuid)) {
new->fsuid = kuid;
- if (security_task_fix_setuid(new, old, LSM_SETID_FS) == 0)
+ if (security_task_fix_setid(new, old, LSM_SETUID_FS) == 0)
goto change_okay;
}
}
@@ -858,10 +870,13 @@ long __sys_setfsgid(gid_t gid)
if (gid_eq(kgid, old->gid) || gid_eq(kgid, old->egid) ||
gid_eq(kgid, old->sgid) || gid_eq(kgid, old->fsgid) ||
- ns_capable(old->user_ns, CAP_SETGID)) {
+ ns_capable_setid(old->user_ns, CAP_SETGID)) {
if (!gid_eq(kgid, old->fsgid)) {
new->fsgid = kgid;
- goto change_okay;
+ if (security_task_fix_setid(new,
+ old,
+ LSM_SETGID_FS) == 0)
+ goto change_okay;
}
}
diff --git a/security/commoncap.c b/security/commoncap.c
index f1d117c3d8ae..6f514d91d010 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1026,27 +1026,27 @@ static inline void cap_emulate_setxuid(struct cred *new, const struct cred *old)
}
/**
- * cap_task_fix_setuid - Fix up the results of setuid() call
+ * cap_task_fix_setid - Fix up the results of setid() call
* @new: The proposed credentials
* @old: The current task's current credentials
* @flags: Indications of what has changed
*
- * Fix up the results of setuid() call before the credential changes are
+ * Fix up the results of setid() call before the credential changes are
* actually applied, returning 0 to grant the changes, -ve to deny them.
*/
-int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
+int cap_task_fix_setid(struct cred *new, const struct cred *old, int flags)
{
switch (flags) {
- case LSM_SETID_RE:
- case LSM_SETID_ID:
- case LSM_SETID_RES:
+ case LSM_SETUID_RE:
+ case LSM_SETUID_ID:
+ case LSM_SETUID_RES:
/* juggle the capabilities to follow [RES]UID changes unless
* otherwise suppressed */
if (!issecure(SECURE_NO_SETUID_FIXUP))
cap_emulate_setxuid(new, old);
break;
- case LSM_SETID_FS:
+ case LSM_SETUID_FS:
/* juggle the capabilties to follow FSUID changes, unless
* otherwise suppressed
*
@@ -1066,6 +1066,15 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags)
}
break;
+ case LSM_SETGID_RE:
+ break;
+ case LSM_SETGID_ID:
+ break;
+ case LSM_SETGID_RES:
+ break;
+ case LSM_SETGID_FS:
+ break;
+
default:
return -EINVAL;
}
@@ -1355,7 +1364,7 @@ struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity),
LSM_HOOK_INIT(mmap_addr, cap_mmap_addr),
LSM_HOOK_INIT(mmap_file, cap_mmap_file),
- LSM_HOOK_INIT(task_fix_setuid, cap_task_fix_setuid),
+ LSM_HOOK_INIT(task_fix_setid, cap_task_fix_setid),
LSM_HOOK_INIT(task_prctl, cap_task_prctl),
LSM_HOOK_INIT(task_setscheduler, cap_task_setscheduler),
LSM_HOOK_INIT(task_setioprio, cap_task_setioprio),
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index cecd38e2ac80..5deffa92f25f 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -120,7 +120,7 @@ static int check_uid_transition(kuid_t parent, kuid_t child)
* set*uid to user under new cred struct, or the UID transition is allowed (by
* Linux set*uid rules) even without CAP_SETUID.
*/
-static int safesetid_task_fix_setuid(struct cred *new,
+static int safesetid_task_fix_setid(struct cred *new,
const struct cred *old,
int flags)
{
@@ -130,7 +130,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return 0;
switch (flags) {
- case LSM_SETID_RE:
+ case LSM_SETUID_RE:
/*
* Users for which setuid restrictions exist can only set the
* real UID to the real UID or the effective UID, unless an
@@ -152,7 +152,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return check_uid_transition(old->euid, new->euid);
}
break;
- case LSM_SETID_ID:
+ case LSM_SETUID_ID:
/*
* Users for which setuid restrictions exist cannot change the
* real UID or saved set-UID unless an explicit whitelist
@@ -163,7 +163,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
if (!uid_eq(old->suid, new->suid))
return check_uid_transition(old->suid, new->suid);
break;
- case LSM_SETID_RES:
+ case LSM_SETUID_RES:
/*
* Users for which setuid restrictions exist cannot change the
* real UID, effective UID, or saved set-UID to anything but
@@ -187,7 +187,7 @@ static int safesetid_task_fix_setuid(struct cred *new,
return check_uid_transition(old->suid, new->suid);
}
break;
- case LSM_SETID_FS:
+ case LSM_SETUID_FS:
/*
* Users for which setuid restrictions exist cannot change the
* filesystem UID to anything but one of: the current real UID,
@@ -256,7 +256,7 @@ void flush_safesetid_whitelist_entries(void)
}
static struct security_hook_list safesetid_security_hooks[] = {
- LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
+ LSM_HOOK_INIT(task_fix_setid, safesetid_task_fix_setid),
LSM_HOOK_INIT(capable, safesetid_security_capable)
};
diff --git a/security/security.c b/security/security.c
index ed9b8cbf21cf..450784fd1d2b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1568,10 +1568,10 @@ int security_kernel_load_data(enum kernel_load_data_id id)
}
EXPORT_SYMBOL_GPL(security_kernel_load_data);
-int security_task_fix_setuid(struct cred *new, const struct cred *old,
+int security_task_fix_setid(struct cred *new, const struct cred *old,
int flags)
{
- return call_int_hook(task_fix_setuid, 0, new, old, flags);
+ return call_int_hook(task_fix_setid, 0, new, old, flags);
}
int security_task_setpgid(struct task_struct *p, pid_t pgid)
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* Re: [PULL REQUEST] Lock down patches
From: Mimi Zohar @ 2019-03-01 0:05 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <CACdnJuvwnh0PKr5jUTm2h=P5-u-oLgBjvThX+DxcegShXReCdA@mail.gmail.com>
On Thu, 2019-02-28 at 15:13 -0800, Matthew Garrett wrote:
> On Thu, Feb 28, 2019 at 2:20 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Thu, 2019-02-28 at 13:28 -0800, Matthew Garrett wrote:
> > > This PR is mostly the same as the previous attempt, but with the
> > > following changes:
> >
> > Where/when was this latest version of the patches posted?
>
> They should have followed this, but git-send-email choked on some
> reviewed-by: lines so I'm just trying to sort that out.
I'm a little perplexed as to why you would send a pull request, before
re-posting the patches with the changes for review.
>
> > >
> > > 1) The integration between EFI secure boot and the lockdown state has
> > > been removed
> > > 2) A new CONFIG_KERNEL_LOCK_DOWN_FORCE kconfig option has been added,
> > > which will always enable lockdown regardless of the kernel command
> > > line
> > > 3) The integration with IMA has been dropped for now. Requiring the
> > > use of the IMA secure boot policy when lockdown is enabled isn't
> > > practical for most distributions at the moment, as there's still not a
> > > great deal of infrastructure for shipping packages with appropriate
> > > IMA signatures, and it makes it complicated for end users to manage
> > > custom IMA policies.
> >
> > I'm all in favor of dropping the original attempt to coordinate
> > between the kexec PE and IMA kernel image signatures and the kernel
> > appended and IMA modules signatures, but there has been quite a bit of
> > work recently coordinating the different types of signatures.
> >
> > Preventing systems which do use IMA for signature verification, should
> > not limit their ability to enable "lock down". Does this version of
> > the "lock down" patches coordinate the different kexec kernel image
> > and kernel module signature verification methods?
>
> It's a little more complicated than this. We can't just rely on IMA
> appraisal - it has to be based on digital signatures, and the existing
> patch only made that implicit by enabling the secure_boot policy.
Right, which is the reason the IMA architecture specific policy
requires file signatures. [1][2]
> I
> think we do want to integrate these, but there's a few things we need
> to take into account:
>
> 1) An integrated solution can't depend on xattrs, both because of the
> lagging support for distributing those signatures but also because we
> need to support filesystems that don't support xattrs
That's not a valid reason for preventing systems that do use IMA for
verifying the kexec kernel image signature or kernel module signatures
from enabling "lock down". This just means that there needs to be
some coordination between the different signature verification
methods. [1][2]
> 2) An integrated solution can't depend on the current secure_boot
> policy because that requires signed IMA policy updates, but
> distributions have no way of knowing what IMA policy end users require
Both the "CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS" and the IMA
architecture policy rules persist after loading a custom policy.
Neither of them require loading or signing a custom policy.
> In any case, I do agree that we should aim to make this more
> reasonable - having orthogonal signing code doesn't benefit anyone.
> Once there's solid agreement on that we can extend this support.
>
Having multiple signature verification methods is going to be around
for a while. The solution is to coordinate the signature verification
methods, without requiring both types of signatures. [1][2]
[For distros that do enable IMA, enabling the IMA architecture
specific policy enforces the kexec kernel image and kernel modules
signature verification.]
Mimi
[1] Subject: [PATCH v2 0/5] selftests/ima: add kexec and kernel module tests
[2] Patches available from the "next-queued-testing" branch
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/
^ permalink raw reply
* Re: [PULL REQUEST] Lock down patches
From: Matthew Garrett @ 2019-03-01 1:01 UTC (permalink / raw)
To: Mimi Zohar; +Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <1551398720.10911.270.camel@linux.ibm.com>
On Thu, Feb 28, 2019 at 4:05 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2019-02-28 at 15:13 -0800, Matthew Garrett wrote:
> > On Thu, Feb 28, 2019 at 2:20 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > Where/when was this latest version of the patches posted?
> >
> > They should have followed this, but git-send-email choked on some
> > reviewed-by: lines so I'm just trying to sort that out.
>
> I'm a little perplexed as to why you would send a pull request, before
> re-posting the patches with the changes for review.
They should be there now. There's no substantive change to the
patches, other than having dropped a few from the series.
> > It's a little more complicated than this. We can't just rely on IMA
> > appraisal - it has to be based on digital signatures, and the existing
> > patch only made that implicit by enabling the secure_boot policy.
>
> Right, which is the reason the IMA architecture specific policy
> requires file signatures. [1][2]
The current patches seem to require ima signatures - shouldn't this
allow ima digests as long as there's an evm signature?
> > I
> > think we do want to integrate these, but there's a few things we need
> > to take into account:
> >
> > 1) An integrated solution can't depend on xattrs, both because of the
> > lagging support for distributing those signatures but also because we
> > need to support filesystems that don't support xattrs
>
> That's not a valid reason for preventing systems that do use IMA for
> verifying the kexec kernel image signature or kernel module signatures
> from enabling "lock down". This just means that there needs to be
> some coordination between the different signature verification
> methods. [1][2]
I agree, but the current form of the integration makes it impossible
for anyone using an IMA-enabled kernel (but not using IMA) to do
anything unless they have IMA signatures. It's a problem we need to
solve, I just don't think it's a problem we need to solve before
merging the patchset.
> > 2) An integrated solution can't depend on the current secure_boot
> > policy because that requires signed IMA policy updates, but
> > distributions have no way of knowing what IMA policy end users require
>
> Both the "CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS" and the IMA
> architecture policy rules persist after loading a custom policy.
> Neither of them require loading or signing a custom policy.
The previous version of the lockdown patchset sets the secure_boot
policy when lockdown is enabled, which does require that any custom
policy be signed.
> > In any case, I do agree that we should aim to make this more
> > reasonable - having orthogonal signing code doesn't benefit anyone.
> > Once there's solid agreement on that we can extend this support.
> >
>
> Having multiple signature verification methods is going to be around
> for a while. The solution is to coordinate the signature verification
> methods, without requiring both types of signatures. [1][2]
Agree, and once we have a solution to this we should integrate that
with lockdown. I don't think merging this first makes that any harder.
Importantly, this version of the patchset doesn't enable lockdown
automatically unless explicitly configured to do so, which means you
can build a lockdown kernel without interfering with IMA.
^ permalink raw reply
* Re: [PULL REQUEST] Lock down patches
From: Mimi Zohar @ 2019-03-01 1:44 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <CACdnJutqg26pk5xnK2ii3ia9JabAnBptE3YQ3yLvVT93as666Q@mail.gmail.com>
On Thu, 2019-02-28 at 17:01 -0800, Matthew Garrett wrote:
> > That's not a valid reason for preventing systems that do use IMA for
> > verifying the kexec kernel image signature or kernel module signatures
> > from enabling "lock down". This just means that there needs to be
> > some coordination between the different signature verification
> > methods. [1][2]
>
> I agree, but the current form of the integration makes it impossible
> for anyone using an IMA-enabled kernel (but not using IMA) to do
> anything unless they have IMA signatures. It's a problem we need to
> solve, I just don't think it's a problem we need to solve before
> merging the patchset.
That's simply not true. Have you even looked at the IMA architecture
patches?
fcf338449af5 x86/ima: require signed kernel modules
d958083a8f64 x86/ima: define arch_get_ima_policy() for x86
Mimi
^ permalink raw reply
* Re: [PATCH 08/27] kexec_file: Restrict at runtime if the kernel is locked down
From: Mimi Zohar @ 2019-03-01 2:05 UTC (permalink / raw)
To: Matthew Garrett, jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190228231203.212359-8-matthewgarrett@google.com>
Hi Jiri,
Thank you for thinking about IMA.
On Thu, 2019-02-28 at 15:11 -0800, Matthew Garrett wrote:
> From: Jiri Bohac <jbohac@suse.cz>
>
> When KEXEC_SIG is not enabled, kernel should not load images through
> kexec_file systemcall if the kernel is locked down unless IMA can be used
> to validate the image.
This patch is a partial solution, but it doesn't take into account the
architecture specific or custom policies.
Mimi
> [Modified by David Howells to fit with modifications to the previous patch
> and to return -EPERM if the kernel is locked down for consistency with
> other lockdowns]
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Jiri Bohac <jbohac@suse.cz>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> cc: Chun-Yi Lee <jlee@suse.com>
> cc: kexec@lists.infradead.org
> ---
> include/linux/ima.h | 6 ++++++
> kernel/kexec_file.c | 8 ++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index b5e16b8c50b7..b35ed0725a05 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -127,4 +127,10 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
> return 0;
> }
> #endif /* CONFIG_IMA_APPRAISE */
> +
> +static inline bool is_ima_kexec_appraise_enabled(void)
> +{
> + return IS_ENABLED(CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS);
> +}
> +
> #endif /* _LINUX_IMA_H */
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 67f3a866eabe..b4e938dff4be 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -239,6 +239,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> }
>
> ret = 0;
> + if (is_ima_kexec_appraise_enabled())
> + break;
> +
> + if (kernel_is_locked_down(reason)) {
> + ret = -EPERM;
> + goto out;
> + }
> +
> break;
>
> /* All other errors are fatal, including nomem, unparseable
^ permalink raw reply
* Re: [PULL REQUEST] Lock down patches
From: Matthew Garrett @ 2019-03-01 3:33 UTC (permalink / raw)
To: Mimi Zohar; +Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <1551404654.10911.276.camel@linux.ibm.com>
On Thu, Feb 28, 2019 at 5:45 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2019-02-28 at 17:01 -0800, Matthew Garrett wrote:
>
> > > That's not a valid reason for preventing systems that do use IMA for
> > > verifying the kexec kernel image signature or kernel module signatures
> > > from enabling "lock down". This just means that there needs to be
> > > some coordination between the different signature verification
> > > methods. [1][2]
> >
> > I agree, but the current form of the integration makes it impossible
> > for anyone using an IMA-enabled kernel (but not using IMA) to do
> > anything unless they have IMA signatures. It's a problem we need to
> > solve, I just don't think it's a problem we need to solve before
> > merging the patchset.
>
> That's simply not true. Have you even looked at the IMA architecture
> patches?
Sorry, I think we're talking at cross purposes - I was referring to
your patch "ima: require secure_boot rules in lockdown mode"
(https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down&id=7fa3734bd31a4b3fe71358fcba8d4878e5005b7f).
If the goal is just to use the architecture rules then I don't see any
conflict, and as far as I can tell things would just work as is if I
drop the ima portion from "kexec_file: Restrict at runtime if the
kernel is locked down"? Apologies, I'd thought that the secure_boot
ruleset was still intended to be used in a lockdown environment.
^ permalink raw reply
* Re: [PULL REQUEST] Lock down patches
From: Mimi Zohar @ 2019-03-01 4:16 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <CACdnJuvn4uC075+Z99CaO3YGjJ+4nviPj=4TEQpkndC3JwFovg@mail.gmail.com>
On Thu, 2019-02-28 at 19:33 -0800, Matthew Garrett wrote:
> On Thu, Feb 28, 2019 at 5:45 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2019-02-28 at 17:01 -0800, Matthew Garrett wrote:
> >
> > > > That's not a valid reason for preventing systems that do use IMA for
> > > > verifying the kexec kernel image signature or kernel module signatures
> > > > from enabling "lock down". This just means that there needs to be
> > > > some coordination between the different signature verification
> > > > methods. [1][2]
> > >
> > > I agree, but the current form of the integration makes it impossible
> > > for anyone using an IMA-enabled kernel (but not using IMA) to do
> > > anything unless they have IMA signatures. It's a problem we need to
> > > solve, I just don't think it's a problem we need to solve before
> > > merging the patchset.
> >
> > That's simply not true. Have you even looked at the IMA architecture
> > patches?
>
> Sorry, I think we're talking at cross purposes - I was referring to
> your patch "ima: require secure_boot rules in lockdown mode"
> (https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down&id=7fa3734bd31a4b3fe71358fcba8d4878e5005b7f).
With the "secure_boot" rules it was difficult to coordinate the
different signature verification methods. Plus they weren't
persistent after loading a custom policy.
> If the goal is just to use the architecture rules then I don't see any
> conflict,
yes
> and as far as I can tell things would just work as is if I
> drop the ima portion from "kexec_file: Restrict at runtime if the
> kernel is locked down"?
That code is a remnant left over from when the "secure_boot" policy
was enabled. However, dropping the IMA portion there would result in
allowing only PE signed kernel images. (On Power, for example, there
aren't any PE signatures.)
My suggestion would be to drop this patch and require the architecture
specific policy in "lock down" mode.
> Apologies, I'd thought that the secure_boot
> ruleset was still intended to be used in a lockdown environment.
No, not any longer.
Mimi
^ permalink raw reply
* [PATCH] keys: fix missing __user in KEYCTL_PKEY_QUERY
From: Ben Dooks @ 2019-03-01 11:30 UTC (permalink / raw)
To: keyrings
Cc: linux-security-module, dhowells, jmorris, serge, linux-kernel,
Ben Dooks
The arg5 of KEYCTL_PKEY_QUERY should have a __user pointer tag on
it as it is a user pointer. This clears the following sparse warning
for this:
security/keys/keyctl.c:1755:43: warning: incorrect type in argument 3 (different address spaces)
security/keys/keyctl.c:1755:43: expected struct keyctl_pkey_query [noderef] <asn:1>*<noident>
security/keys/keyctl.c:1755:43: got struct keyctl_pkey_query *<noident>
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
security/keys/keyctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 7bbe03593e58..3e4053a217c3 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1752,7 +1752,7 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
return -EINVAL;
return keyctl_pkey_query((key_serial_t)arg2,
(const char __user *)arg4,
- (struct keyctl_pkey_query *)arg5);
+ (struct keyctl_pkey_query __user *)arg5);
case KEYCTL_PKEY_ENCRYPT:
case KEYCTL_PKEY_DECRYPT:
--
2.20.1
^ permalink raw reply related
* RE: [PATCH 01/97] LSM: Infrastructure management of the superblock
From: Edwin Zimmerman @ 2019-03-01 14:02 UTC (permalink / raw)
To: 'Casey Schaufler', jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <20190228221933.2551-2-casey@schaufler-ca.com>
On Thursday2/28/2019 Casey Schaufler <casey@schaufler-ca.com> wrote:
> From: Casey Schaufler <cschaufler@localhost.localdomain>
>
> Move management of the superblock->sb_security blob out
> of the individual security modules and into the security
> infrastructure. Instead of allocating the blobs from within
> the modules the modules tell the infrastructure how much
> space is required, and the space is allocated there.
>
...
> @@ -296,12 +297,13 @@ static void __init ordered_lsm_init(void)
> for (lsm = ordered_lsms; *lsm; lsm++)
> prepare_lsm(*lsm);
>
> - init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
> - init_debug("file blob size = %d\n", blob_sizes.lbs_file);
> - init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
> - init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
> - init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
> - init_debug("task blob size = %d\n", blob_sizes.lbs_task);
> + init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
> + init_debug("file blob size = %d\n", blob_sizes.lbs_file);
> + init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
> + init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
> + init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
> + init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
> + init_debug("task blob size = %d\n", blob_sizes.lbs_task);
>
Any reason this couldn't be a single line, or am I missing something?
It took a second look to realize that the only line actually being added was
+ init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
^ permalink raw reply
* Re: [PATCH 05/97] LSM: Create an lsm_export data structure.
From: Stephen Smalley @ 2019-03-01 14:00 UTC (permalink / raw)
To: Casey Schaufler, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <20190228221933.2551-6-casey@schaufler-ca.com>
On 2/28/19 5:18 PM, Casey Schaufler wrote:
> When more than one security module is exporting data to
> audit and networking sub-systems a single 32 bit integer
> is no longer sufficient to represent the data. Add a
> structure to be used instead.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/security.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 13537a49ae97..a79fe8ef9d84 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -73,6 +73,18 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +/* Data exported by the security modules */
> +struct lsm_export {
> + u32 selinux;
> + u32 smack;
> + u32 apparmor;
> + u32 flags;
> +};
> +#define LSM_EXPORT_NONE 0x00
> +#define LSM_EXPORT_SELINUX 0x01
> +#define LSM_EXPORT_SMACK 0x02
> +#define LSM_EXPORT_APPARMOR 0x04
Can this be generalized to avoid hardcoding the names of specific
security modules in the field and symbol names? Possibly just an array
of secids with the indices dynamically assigned by the infrastructure at
registration time? We don't really want to have to patch this structure
every time someone adds a new security module that needs audit and/or
network facilities, right?
> +
> /* These functions are in security/commoncap.c */
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> int cap, unsigned int opts);
>
^ permalink raw reply
* Re: [PATCH 00/97] LSM: Complete module stacking
From: Stephen Smalley @ 2019-03-01 14:17 UTC (permalink / raw)
To: Casey Schaufler, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <20190228221933.2551-1-casey@schaufler-ca.com>
On 2/28/19 5:17 PM, Casey Schaufler wrote:
> This is a preliminary version of the complete stacking
> implementation. The patches need to be cleaned up, and
> several are not strictly necessary. There is likely to
> be work required in the audit sub-system. It does address
> all the shared data, including CIPSO headers. It should
> handle CALIPSO once Smack supports it. I will be revising
> the set after 5.1.
>
> Complete the transition from module based blob management
> to infrastructure based blob management. This includes
> the socket, superblock and key blobs.
>
> Change the LSM infrastructure from exposing secids to
> exposing an opaque "lsm_export" structure that can contain
> information for multiple active security modules. Update
> all of the security modules to use information from the
> lsm_export structure. Update the LSM interfaces that expose
> secids for more than one module to use the export structure.
> Update all the users of these interfaces.
>
> Change the LSM infrastructure from using a string/size pair
> for security "contexts" to a "lsm_context" structure that
> can represent information for multiple modules. This contains
> information that allows the "context" to be properly freed
> regardless of where it is allocated and where it is used.
>
> Add an interface to identify which security module data
> should be presented with SO_PEERSEC. /proc/.../attr/display
> will set and report the name of the LSM for which the
> security_secid_to_secctx() will use to translate to text.
> If it is not explicitly set, the first security module that
> supplies secid (now lsm_export) interfaces will be used.
> To ensure consistency, a set of module hooks dealing with
> the secid/context processing is maintained with each process
> that explicitly sets it.
>
> Before sending a network packet verify that all interested
> security modules agree on the labeling. Fail if the labeling
> cannot be reconciled. This requires a new Netlabel interface
> to compare proposed labels, and a change to the return values
> from the existing netlabel attribute setting functions.
Have you run any benchmarks to assess the performance impact of these
changes?
>
> git://github.com/cschaufler/lsm-stacking.git#5.0-rc3-plus-a
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> fs/kernfs/dir.c | 6 +-
> fs/kernfs/inode.c | 31 +-
> fs/kernfs/kernfs-internal.h | 4 +-
> fs/nfs/inode.c | 13 +-
> fs/nfs/internal.h | 8 +-
> fs/nfs/nfs4proc.c | 17 +-
> fs/nfs/nfs4xdr.c | 16 +-
> fs/nfsd/nfs4proc.c | 8 +-
> fs/nfsd/nfs4xdr.c | 14 +-
> fs/nfsd/vfs.c | 7 +-
> fs/proc/base.c | 1 +
> include/linux/cred.h | 3 +-
> include/linux/lsm_hooks.h | 122 ++--
> include/linux/nfs4.h | 8 +-
> include/linux/security.h | 165 +++--
> include/net/netlabel.h | 18 +-
> include/net/route.h | 55 +-
> include/net/scm.h | 14 +-
> include/net/sock.h | 14 +-
> include/uapi/linux/netfilter/xt_CONNMARK.h | 45 +-
> include/uapi/linux/netfilter/xt_DSCP.h | 27 +-
> include/uapi/linux/netfilter/xt_MARK.h | 17 +-
> include/uapi/linux/netfilter/xt_RATEEST.h | 38 +-
> include/uapi/linux/netfilter/xt_TCPMSS.h | 13 +-
> include/uapi/linux/netfilter_ipv4/ipt_ECN.h | 40 +-
> include/uapi/linux/netfilter_ipv4/ipt_TTL.h | 14 +-
> include/uapi/linux/netfilter_ipv6/ip6t_HL.h | 14 +-
> kernel/audit.c | 60 +-
> kernel/audit.h | 9 +-
> kernel/auditfilter.c | 6 +-
> kernel/auditsc.c | 61 +-
> kernel/cred.c | 15 +-
> net/ipv4/cipso_ipv4.c | 13 +-
> net/ipv4/ip_sockglue.c | 14 +-
> net/ipv4/route.c | 61 ++
> net/netfilter/nf_conntrack_netlink.c | 27 +-
> net/netfilter/nf_conntrack_standalone.c | 16 +-
> net/netfilter/nfnetlink_queue.c | 35 +-
> net/netfilter/nft_meta.c | 8 +-
> net/netfilter/xt_DSCP.c | 149 ++---
> net/netfilter/xt_HL.c | 164 ++---
> net/netfilter/xt_RATEEST.c | 278 +++------
> net/netfilter/xt_SECMARK.c | 9 +-
> net/netfilter/xt_TCPMSS.c | 378 +++---------
> net/netlabel/netlabel_kapi.c | 125 +++-
> net/netlabel/netlabel_unlabeled.c | 99 ++-
> net/netlabel/netlabel_unlabeled.h | 2 +-
> net/netlabel/netlabel_user.c | 13 +-
> net/netlabel/netlabel_user.h | 2 +-
> net/socket.c | 17 +
> net/unix/af_unix.c | 11 +-
> security/apparmor/audit.c | 4 +-
> security/apparmor/include/audit.h | 2 +-
> security/apparmor/include/net.h | 6 +-
> security/apparmor/include/secid.h | 9 +-
> security/apparmor/lsm.c | 64 +-
> security/apparmor/secid.c | 42 +-
> security/integrity/ima/ima.h | 14 +-
> security/integrity/ima/ima_api.c | 9 +-
> security/integrity/ima/ima_appraise.c | 6 +-
> security/integrity/ima/ima_main.c | 34 +-
> security/integrity/ima/ima_policy.c | 19 +-
> security/security.c | 682 ++++++++++++++++++---
> security/selinux/hooks.c | 308 +++++-----
> security/selinux/include/audit.h | 6 +-
> security/selinux/include/netlabel.h | 7 +
> security/selinux/include/objsec.h | 43 +-
> security/selinux/netlabel.c | 69 ++-
> security/selinux/ss/services.c | 19 +-
> security/smack/smack.h | 34 +
> security/smack/smack_access.c | 14 +-
> security/smack/smack_lsm.c | 389 ++++++------
> security/smack/smack_netfilter.c | 48 +-
> security/smack/smackfs.c | 23 +-
> .../Z6.0+pooncelock+poonceLock+pombonce.litmus | 12 +-
> 75 files changed, 2369 insertions(+), 1798 deletions(-)
>
^ permalink raw reply
* RE: [PATCH 11/97] LSM: Use lsm_export in the kernel_ask_as hooks
From: Edwin Zimmerman @ 2019-03-01 14:59 UTC (permalink / raw)
To: 'Casey Schaufler', jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <20190228221933.2551-12-casey@schaufler-ca.com>
On Thursday, 2/28/2019 at 5:18 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> Convert the kernel_ask_as hooks to use the lsm_export
Should be act_as, not? ^^
> structure instead of a u32 secid. There is some scaffolding
> involved that will be removed when security_kernel_ask_as()
> is updated.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hooks.h | 4 ++--
> security/security.c | 15 ++++++++++++++-
> security/selinux/hooks.c | 17 ++++++++++++++---
> security/smack/smack_lsm.c | 12 +++++++++++-
> 4 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 44597189fea4..796eb441be95 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -562,7 +562,7 @@
> * @kernel_act_as:
> * Set the credentials for a kernel service to act as (subjective context).
> * @new points to the credentials to be modified.
> - * @secid specifies the security ID to be set
> + * @l specifies the security data to be set
> * The current task must be the one that nominated @secid.
> * Return 0 if successful.
> * @kernel_create_files_as:
> @@ -1588,7 +1588,7 @@ union security_list_options {
> gfp_t gfp);
> void (*cred_transfer)(struct cred *new, const struct cred *old);
> void (*cred_getsecid)(const struct cred *c, struct lsm_export *l);
> - int (*kernel_act_as)(struct cred *new, u32 secid);
> + int (*kernel_act_as)(struct cred *new, struct lsm_export *l);
> int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
> int (*kernel_module_request)(char *kmod_name);
> int (*kernel_load_data)(enum kernel_load_data_id id);
> diff --git a/security/security.c b/security/security.c
> index 909b6b8d1a50..1a29fe08a5d9 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -738,6 +738,15 @@ static inline void lsm_export_secid(struct lsm_export *data, u32 *secid)
> }
> }
>
> +static inline void lsm_export_to_all(struct lsm_export *data, u32 secid)
> +{
> + data->selinux = secid;
> + data->smack = secid;
> + data->apparmor = secid;
> + data->flags = LSM_EXPORT_SELINUX | LSM_EXPORT_SMACK |
> + LSM_EXPORT_APPARMOR;
> +}
> +
> /* Security operations */
>
> int security_binder_set_context_mgr(struct task_struct *mgr)
> @@ -1633,7 +1642,11 @@ EXPORT_SYMBOL(security_cred_getsecid);
>
> int security_kernel_act_as(struct cred *new, u32 secid)
> {
> - return call_int_hook(kernel_act_as, 0, new, secid);
> + struct lsm_export data = { .flags = LSM_EXPORT_NONE };
> +
> + lsm_export_to_all(&data, secid);
> +
> + return call_int_hook(kernel_act_as, 0, new, &data);
> }
>
> int security_kernel_create_files_as(struct cred *new, struct inode *inode)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 6f61a894f7c5..efcd905bdabf 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -220,6 +220,14 @@ static inline void selinux_export_secid(struct lsm_export *l, u32 secid)
> l->flags |= LSM_EXPORT_SELINUX;
> }
>
> +static inline void selinux_import_secid(struct lsm_export *l, u32 *secid)
> +{
> + if (l->flags | LSM_EXPORT_SELINUX)
> + *secid = l->selinux;
> + else
> + *secid = SECSID_NULL;
> +}
> +
> /*
> * get the security ID of a set of credentials
> */
> @@ -3669,19 +3677,22 @@ static void selinux_cred_getsecid(const struct cred *c, struct lsm_export *l)
> * set the security data for a kernel service
> * - all the creation contexts are set to unlabelled
> */
> -static int selinux_kernel_act_as(struct cred *new, u32 secid)
> +static int selinux_kernel_act_as(struct cred *new, struct lsm_export *l)
> {
> struct task_security_struct *tsec = selinux_cred(new);
> + u32 nsid;
> u32 sid = current_sid();
> int ret;
>
> + selinux_import_secid(l, &nsid);
> +
> ret = avc_has_perm(&selinux_state,
> - sid, secid,
> + sid, nsid,
> SECCLASS_KERNEL_SERVICE,
> KERNEL_SERVICE__USE_AS_OVERRIDE,
> NULL);
> if (ret == 0) {
> - tsec->sid = secid;
> + tsec->sid = nsid;
> tsec->create_sid = 0;
> tsec->keycreate_sid = 0;
> tsec->sockcreate_sid = 0;
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d5ff34a5803b..0e1f6ef25eb2 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -473,6 +473,14 @@ static inline void smack_export_secid(struct lsm_export *l, u32 secid)
> l->flags |= LSM_EXPORT_SMACK;
> }
>
> +static inline void smack_import_secid(struct lsm_export *l, u32 *secid)
> +{
> + if (l->flags | LSM_EXPORT_SMACK)
> + *secid = l->smack;
> + else
> + *secid = 0;
> +}
> +
> /*
> * LSM hooks.
> * We he, that is fun!
> @@ -1910,10 +1918,12 @@ static void smack_cred_getsecid(const struct cred *cred, struct lsm_export *l)
> *
> * Set the security data for a kernel service.
> */
> -static int smack_kernel_act_as(struct cred *new, u32 secid)
> +static int smack_kernel_act_as(struct cred *new, struct lsm_export *l)
> {
> + u32 secid;
> struct task_smack *new_tsp = smack_cred(new);
>
> + smack_import_secid(l, &secid);
> new_tsp->smk_task = smack_from_secid(secid);
> return 0;
> }
> --
> 2.17.0
^ permalink raw reply
* [RFC PATCH 1/2] drivers: crypto: caam: key: Add caam_tk key type
From: Franck LENORMAND @ 2019-03-01 16:09 UTC (permalink / raw)
To: linux-kernel, linux-security-module, keyrings
Cc: franck.lenormand, horia.geanta, silvano.dininno, agk, snitzer,
dm-devel, dhowells, jmorris, serge
In-Reply-To: <1551456599-10603-1-git-send-email-franck.lenormand@nxp.com>
This patch adds a module which creates a new key type which
can be used by the user with the linux key retention service.
The key created by this module are black keys appended with
a tag to create a tag key.
Such a key can be passed to the linux crypto API for the
transforms:
- tk(cbc(aes))
The configuration string passed to the key service has 3
forms:
- new <black key encryption> <size in bytes>
- set <black key encryption> <hex of a key>
- load <black key encryption> <hex of a blob>
with <black key encryption> = ecb | ccm
When reading or printing a key, it will return a binary blob
which can be saved to a file through powercycle. The blob
can then be loaded.
V2: Expect the data to be loaded to be prepended by ':hex:'
Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
---
drivers/crypto/caam/caam_key.c | 623 +++++++++++++++++++++++++++++++++++++++++
drivers/crypto/caam/caam_key.h | 58 ++++
2 files changed, 681 insertions(+)
create mode 100644 drivers/crypto/caam/caam_key.c
create mode 100644 drivers/crypto/caam/caam_key.h
diff --git a/drivers/crypto/caam/caam_key.c b/drivers/crypto/caam/caam_key.c
new file mode 100644
index 0000000..5d89c9d
--- /dev/null
+++ b/drivers/crypto/caam/caam_key.c
@@ -0,0 +1,623 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2018 NXP
+ * caam key is generated using NXP CAAM hardware block. CAAM generates the
+ * random number (used as a key) and creates its blob for the user.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/parser.h>
+#include <linux/string.h>
+#include <linux/key-type.h>
+#include <linux/rcupdate.h>
+#include <linux/completion.h>
+#include <linux/module.h>
+
+#include "desc.h"
+#include "desc_constr.h"
+#include "caam_desc.h"
+#include "caam_key.h"
+#include "caam_util.h"
+
+/* Key modifier for CAAM key blobbing */
+static const char caam_key_modifier[KEYMOD_SIZE_GM] = {
+ 'C', 'A', 'A', 'M', '_', 'K', 'E', 'Y',
+ '_', 'T', 'Y', 'P', 'E', '_', 'V', '1',
+};
+
+/* Operation supported */
+enum caam_key_op {
+ OP_ERROR = -1,
+ OP_NEW_KEY,
+ OP_SET_KEY,
+ OP_LOAD_BLOB,
+};
+
+/* Tokens for the operation to do */
+static const match_table_t key_cmd_tokens = {
+ {OP_NEW_KEY, "new"},
+ {OP_SET_KEY, "set"},
+ {OP_LOAD_BLOB, "load"},
+ {OP_ERROR, NULL}
+};
+
+enum caam_key_fmt {
+ FMT_ERROR = -1,
+ FMT_ECB,
+ FMT_CCM,
+};
+
+/* Tokens for the type of encryption of the black key */
+static const char FMT_ECB_txt[] = "ecb";
+static const char FMT_CCM_txt[] = "ccm";
+
+static const match_table_t key_fmt_tokens = {
+ {FMT_ECB, FMT_ECB_txt},
+ {FMT_CCM, FMT_CCM_txt},
+ {FMT_ERROR, NULL}
+};
+
+int caam_key_tag_black_key(struct caam_key_payload *ckpayload,
+ size_t black_key_max_len, u8 auth, u8 trusted)
+{
+ struct tag_object_conf tag;
+ enum tag_type type;
+ int ret;
+ u32 size_tagged = black_key_max_len;
+
+ if (!ckpayload)
+ return -EINVAL;
+
+ if (!is_auth(auth) || !is_trusted_key(trusted))
+ return -EINVAL;
+
+ if (auth == KEY_COVER_ECB) {
+ if (trusted == UNTRUSTED_KEY)
+ type = TAG_TYPE_BLACK_KEY_ECB;
+ else
+ type = TAG_TYPE_BLACK_KEY_ECB_TRUSTED;
+ } else {
+ if (trusted == UNTRUSTED_KEY)
+ type = TAG_TYPE_BLACK_KEY_CCM;
+ else
+ type = TAG_TYPE_BLACK_KEY_CCM_TRUSTED;
+ }
+
+ /* Prepare the tag */
+ init_tag_object_header(&tag.header, type);
+ init_blackey_conf(&tag.conf.bk_conf, ckpayload->key_len,
+ auth == KEY_COVER_CCM,
+ trusted == TRUSTED_KEY);
+
+ ret = set_tag_object_conf(&tag, ckpayload->black_key,
+ ckpayload->black_key_len, &size_tagged);
+ if (ret) {
+ pr_err("Tagging fail: %d\n", ret);
+ goto exit;
+ }
+
+ /* Update the size of the black key tagged */
+ ckpayload->black_key_len = size_tagged;
+
+exit:
+ return ret;
+}
+
+static int caam_transform(enum caam_key_op key_cmd,
+ struct caam_key_payload *ckpayload)
+{
+ int ret;
+ struct device *jrdev;
+ u8 key_cover;
+
+ /* Allocate caam job ring for operation to be performed from CAAM */
+ jrdev = caam_jr_alloc();
+ if (!jrdev) {
+ pr_info("caam_jr_alloc failed\n");
+ ret = -ENODEV;
+ goto out;
+ }
+
+ if (ckpayload->key_fmt_val == FMT_ECB)
+ key_cover = KEY_COVER_ECB;
+ else
+ key_cover = KEY_COVER_CCM;
+
+ switch (key_cmd) {
+ case OP_LOAD_BLOB:
+#ifdef DEBUG
+ print_hex_dump(KERN_ERR, "input blob: ",
+ DUMP_PREFIX_OFFSET, 16, 4, ckpayload->blob,
+ ckpayload->blob_len, 0);
+#endif
+ /* Decapsulate the black blob into a black key */
+ ret = caam_blob_decap(jrdev,
+ ckpayload->blob, ckpayload->blob_len,
+ DATA_GENMEM, BLACK_BLOB,
+ ckpayload->key_mod,
+ &ckpayload->key_mod_len, DATA_GENMEM,
+ ckpayload->black_key,
+ &ckpayload->black_key_len, DATA_GENMEM,
+ BLACK_KEY, &ckpayload->key_len,
+ key_cover, UNTRUSTED_KEY);
+ if (ret) {
+ pr_info("key_blob decap fail: %d\n", ret);
+ goto free_jr;
+ }
+
+ break;
+ case OP_SET_KEY:
+
+#ifdef DEBUG
+ print_hex_dump(KERN_ERR, "input key: ",
+ DUMP_PREFIX_OFFSET, 16, 4, ckpayload->key,
+ ckpayload->key_len, 0);
+#endif
+
+ /* Cover the input key */
+ ret = caam_black_key(jrdev,
+ ckpayload->key, ckpayload->key_len,
+ DATA_GENMEM,
+ ckpayload->black_key,
+ &ckpayload->black_key_len, DATA_GENMEM,
+ key_cover, UNTRUSTED_KEY);
+ /*
+ * Clear the input key
+ * TODO: Make it secure to not be removed by compiler
+ */
+ memset(ckpayload->key, 0, ckpayload->key_len);
+
+ if (ret) {
+ pr_info("key covering fail: (%d)\n", ret);
+ goto free_jr;
+ }
+
+ /* Encapsulate the key */
+ ret = caam_blob_encap(jrdev,
+ ckpayload->black_key,
+ ckpayload->black_key_len, DATA_GENMEM,
+ BLACK_KEY, ckpayload->key_len, key_cover,
+ UNTRUSTED_KEY,
+ ckpayload->key_mod,
+ &ckpayload->key_mod_len, DATA_GENMEM,
+ ckpayload->blob, &ckpayload->blob_len,
+ DATA_GENMEM, BLACK_BLOB);
+ if (ret) {
+ pr_info("Blob encapsulation of key fail: %d\n", ret);
+ goto free_jr;
+ }
+
+ break;
+ case OP_NEW_KEY:
+ /*
+ * We need random data to create a key however we do not
+ * want
+ */
+ ret = caam_random_black_key(jrdev,
+ ckpayload->key_len,
+ ckpayload->black_key,
+ &ckpayload->black_key_len,
+ DATA_GENMEM, key_cover,
+ UNTRUSTED_KEY);
+
+ if (ret) {
+ pr_info("Random key covering fail: %d\n", ret);
+ goto free_jr;
+ }
+
+ /* Encapsulate the key */
+ ret = caam_blob_encap(jrdev,
+ ckpayload->black_key,
+ ckpayload->black_key_len, DATA_GENMEM,
+ BLACK_KEY, ckpayload->key_len, key_cover,
+ UNTRUSTED_KEY,
+ ckpayload->key_mod,
+ &ckpayload->key_mod_len, DATA_GENMEM,
+ ckpayload->blob, &ckpayload->blob_len,
+ DATA_GENMEM, BLACK_BLOB);
+ if (ret) {
+ pr_info("Blob encapsulation of random fail: %d\n", ret);
+ goto free_jr;
+ }
+
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+#ifdef DEBUG
+ print_hex_dump(KERN_ERR, "black key: ",
+ DUMP_PREFIX_OFFSET, 16, 4, ckpayload->black_key,
+ ckpayload->black_key_len, 0);
+ print_hex_dump(KERN_ERR, "blob: ",
+ DUMP_PREFIX_OFFSET, 16, 4, ckpayload->blob,
+ ckpayload->blob_len, 0);
+#endif
+
+ /* Tag the black key so it can be passed to CAAM crypto API */
+ ret = caam_key_tag_black_key(ckpayload,
+ ARRAY_SIZE(ckpayload->black_key),
+ key_cover, UNTRUSTED_KEY);
+ if (ret) {
+ pr_info("Black key tagging fail: %d\n", ret);
+ goto free_jr;
+ }
+
+#ifdef DEBUG
+ print_hex_dump(KERN_ERR, "tagged black key: ",
+ DUMP_PREFIX_OFFSET, 16, 4, ckpayload->black_key,
+ ckpayload->black_key_len, 0);
+#endif
+
+ /* Update the aliased user_key_payload */
+ ckpayload->upayload.datalen = ckpayload->black_key_len;
+ memcpy(ckpayload->upayload.data, ckpayload->black_key,
+ ckpayload->upayload.datalen);
+
+free_jr:
+ caam_jr_free(jrdev);
+
+out:
+ if (ret)
+ pr_err("Operation %s(%d) failed\n",
+ key_cmd_tokens[key_cmd].pattern, key_cmd);
+
+ return ret;
+}
+
+/*
+ * parse_inputdata - parse the keyctl input data and fill in the
+ * payload structure for key or its blob.
+ * param[in]: data pointer to the data to be parsed for creating key.
+ * param[in]: p pointer to caam key payload structure to fill parsed data
+ * On success returns 0, otherwise -EINVAL.
+ */
+static enum caam_key_op parse_inputdata(char *data,
+ struct caam_key_payload *ckpayload)
+{
+ substring_t args[MAX_OPT_ARGS];
+ long keylen = 0;
+ int ret = 0;
+ enum caam_key_op op_to_do = OP_ERROR;
+ int key_cmd = -EINVAL;
+ int key_fmt = -EINVAL;
+ char *c = NULL;
+ const char *hex_format = ":hex:";
+ u32 hex_format_size;
+
+ c = strsep(&data, " \t");
+ if (!c) {
+ ret = -EINVAL;
+ pr_err("Failed to find 1st arg\n");
+ goto out;
+ }
+
+ /* Get the keyctl command i.e. new_key or load_blob etc */
+ key_cmd = match_token(c, key_cmd_tokens, args);
+
+ /* Skip spaces to get the 1st argument */
+ c = strsep(&data, " \t");
+ if (!c) {
+ ret = -EINVAL;
+ pr_err("Failed to find 2nd arg\n");
+ goto out;
+ }
+
+ /* Get the keyctl format i.e. ecb or ccm etc */
+ key_fmt = match_token(c, key_fmt_tokens, args);
+
+ /* Skip spaces to get second argument */
+ c = strsep(&data, " \t");
+ if (!c) {
+ ret = -EINVAL;
+ pr_err("Failed to find 3rd arg\n");
+ goto out;
+ }
+
+ switch (key_fmt) {
+ case FMT_ECB:
+ ckpayload->key_fmt_val = KEY_COVER_ECB;
+ break;
+ case FMT_CCM:
+ ckpayload->key_fmt_val = KEY_COVER_CCM;
+ break;
+ case FMT_ERROR:
+ ret = -EINVAL;
+ pr_err("Format %d not supported\n", key_fmt);
+ goto out;
+ }
+
+ /* Prepare arguments */
+ switch (key_cmd) {
+ case OP_NEW_KEY:
+ /* Second argument is key size */
+ ret = kstrtol(c, 10, &keylen);
+ if (ret < 0 || keylen < MIN_KEY_SIZE ||
+ keylen > MAX_KEY_SIZE) {
+ ret = -EINVAL;
+ pr_err("Failed to retrieve key length\n");
+ goto out;
+ }
+
+ ckpayload->key_len = keylen;
+
+ ckpayload->black_key_len = ARRAY_SIZE(ckpayload->black_key);
+ ckpayload->blob_len = ARRAY_SIZE(ckpayload->blob);
+
+ op_to_do = OP_NEW_KEY;
+
+ break;
+ case OP_SET_KEY:
+ /* Second argument is key data for CAAM*/
+
+ /* key_len = No of characters in key/2 */
+ ckpayload->key_len = strlen(c) / 2;
+ if (ckpayload->blob_len > MAX_KEY_SIZE) {
+ ret = -EINVAL;
+ pr_err("Failed to compute key length\n");
+ goto out;
+ }
+
+ ret = hex2bin(ckpayload->key, c, ckpayload->key_len);
+ if (ret < 0) {
+ ret = -EINVAL;
+ pr_err("Failed to retrieve key data\n");
+ goto out;
+ }
+
+ ckpayload->black_key_len = ARRAY_SIZE(ckpayload->black_key);
+ ckpayload->blob_len = ARRAY_SIZE(ckpayload->blob);
+
+ op_to_do = OP_SET_KEY;
+
+ break;
+ case OP_LOAD_BLOB:
+ /* Second argument is blob data for CAAM */
+ hex_format_size = strlen(hex_format);
+
+ /* The blob is prepended by the format */
+ if (strncmp(c, hex_format, hex_format_size) != 0) {
+ ret = -EINVAL;
+ pr_err("Failed to match blob format\n");
+ goto out;
+ }
+
+ /* Advance the pointer */
+ c += hex_format_size;
+
+ /* Blob_len = No of characters in blob/2 */
+ ckpayload->blob_len = strlen(c) / 2;
+ if (ckpayload->blob_len > MAX_BLOB_SIZE) {
+ ret = -EINVAL;
+ pr_err("Failed to compute blob length\n");
+ goto out;
+ }
+
+ ret = hex2bin(ckpayload->blob, c, ckpayload->blob_len);
+ if (ret < 0) {
+ ret = -EINVAL;
+ pr_err("Failed to retrieve blob data\n");
+ goto out;
+ }
+
+ ckpayload->key_len = ARRAY_SIZE(ckpayload->key);
+ ckpayload->black_key_len = ARRAY_SIZE(ckpayload->black_key);
+
+ op_to_do = OP_LOAD_BLOB;
+
+ break;
+ case OP_ERROR:
+ ret = -EINVAL;
+ pr_err("Command %d not supported\n", key_cmd);
+ break;
+ }
+
+ ckpayload->key_mod = caam_key_modifier;
+ ckpayload->key_mod_len = ARRAY_SIZE(caam_key_modifier);
+
+out:
+ return (ret == 0) ? op_to_do : OP_ERROR;
+}
+
+static struct caam_key_payload *caam_payload_alloc(struct key *key)
+{
+ struct caam_key_payload *ckpayload = NULL;
+ int ret = 0;
+
+ ret = key_payload_reserve(key, sizeof(*ckpayload));
+ if (ret < 0) {
+ pr_err("Failed to reserve payload\n");
+ goto out;
+ }
+
+ ckpayload = kzalloc(sizeof(*ckpayload), GFP_KERNEL);
+ if (!ckpayload)
+ goto out;
+
+out:
+ return ckpayload;
+}
+
+/*
+ * caam_destroy - clear and free the key's payload
+ */
+static void caam_destroy(struct key *key)
+{
+ struct caam_key_payload *ckpayload = NULL;
+
+ /* Retrieve the payload */
+ ckpayload = dereference_key_locked(key);
+ if (!ckpayload)
+ pr_err("Fail to retrieve key payload\n");
+
+ kzfree(ckpayload);
+}
+
+/*
+ * caam_instantiate - create a new caam type key.
+ * Supports the operation to generate a new key. A random number
+ * is generated from CAAM as key data and the corresponding red blob
+ * is formed and stored as key_blob.
+ * Also supports the operation to load the blob and key is derived using
+ * that blob from CAAM.
+ * On success, return 0. Otherwise return errno.
+ */
+static int caam_instantiate(struct key *key,
+ struct key_preparsed_payload *prep)
+{
+ struct caam_key_payload *ckpayload;
+ size_t datalen;
+ char *data = NULL;
+ int key_cmd = 0;
+ int ret = 0;
+
+ if (!key || !prep) {
+ ret = -EINVAL;
+ pr_err("Input data incorrect\n");
+ goto out;
+ }
+
+ datalen = prep->datalen;
+
+ if (datalen <= 0 || datalen > 32767) {
+ ret = -EINVAL;
+ pr_err("Payload data size incorrect\n");
+ goto out;
+ }
+
+ /* Allocate memory to get a parsable string */
+ data = kmalloc(datalen + 1, GFP_KERNEL);
+ if (!data) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ memcpy(data, prep->data, datalen);
+ data[datalen] = '\0';
+
+ ckpayload = caam_payload_alloc(key);
+ if (!ckpayload) {
+ pr_err("Fail to allocate payload\n");
+ ret = -ENOMEM;
+ goto free_data;
+ }
+
+ /* Initialize and fill the payload */
+ key_cmd = parse_inputdata(data, ckpayload);
+ if (key_cmd == OP_ERROR) {
+ pr_err("Fail to parse data\n");
+ ret = key_cmd;
+ goto free_payload;
+ }
+
+ /* Create the black key and/or the blob */
+ caam_transform(key_cmd, ckpayload);
+ if (ret != 0) {
+ pr_info("transform fail (%d)\n", ret);
+ goto free_payload;
+ }
+
+ /* Store the payload to the key */
+ rcu_assign_keypointer(key, ckpayload);
+
+ goto out;
+
+free_payload:
+ kzfree(ckpayload);
+
+free_data:
+ kzfree(data);
+
+out:
+ return ret;
+}
+
+/*
+ * caam_read - copy the blob data to userspace.
+ * param[in]: key pointer to key struct
+ * param[in]: buffer pointer to user data for creating key
+ * param[in]: buflen is the length of the buffer
+ * On success, return to userspace the caam key data size.
+ */
+static long caam_read(const struct key *key, char __user *buffer, size_t buflen)
+{
+ const struct caam_key_payload *ckpayload = NULL;
+ size_t size_to_copy;
+ size_t size_copied = 0;
+ unsigned long not_copied;
+ char *to = buffer;
+
+ /* Retrieve the payload */
+ ckpayload = dereference_key_locked(key);
+ if (!ckpayload) {
+ pr_err("Fail to retrieve key payload\n");
+ return -EINVAL;
+ }
+
+ /* Check all the data can be copied */
+ size_to_copy = ckpayload->blob_len;
+
+ /* If buflen == 0, the user request the size needed */
+ if (buflen == 0)
+ return size_to_copy;
+
+ /* Check the buffer */
+ if (!buffer) {
+ pr_err("Buffer not set\n");
+ return -EINVAL;
+ }
+
+ /* Check the buffer is big enough */
+ if (size_to_copy > buflen) {
+ pr_err("Buffer length too short\n");
+ return -ENOMEM;
+ }
+
+ /* Copy blob */
+ not_copied = copy_to_user(to, ckpayload->blob, ckpayload->blob_len);
+ if (not_copied != 0) {
+ pr_err("Copy of black blob failed\n");
+ return -EIO;
+ }
+ size_copied += ckpayload->blob_len;
+
+ if (size_to_copy != size_copied)
+ pr_info("Mismatch between size computed and copied\n");
+
+ return size_copied;
+}
+
+/* Description of the key type for CAAM keys */
+struct key_type key_type_caam_tk = {
+ .name = "caam_tk",
+ .instantiate = caam_instantiate,
+ .destroy = caam_destroy,
+ .read = caam_read,
+};
+EXPORT_SYMBOL_GPL(key_type_caam_tk);
+
+static int __init init_caam_key(void)
+{
+ int ret;
+
+ ret = register_key_type(&key_type_caam_tk);
+ if (ret) {
+ pr_err("Failed to register key storage %s\n",
+ key_type_caam_tk.name);
+ return -EIO;
+ }
+
+ return ret;
+}
+
+static void __exit cleanup_caam_key(void)
+{
+ unregister_key_type(&key_type_caam_tk);
+}
+
+late_initcall(init_caam_key);
+module_exit(cleanup_caam_key);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/crypto/caam/caam_key.h b/drivers/crypto/caam/caam_key.h
new file mode 100644
index 0000000..93273ea
--- /dev/null
+++ b/drivers/crypto/caam/caam_key.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 NXP.
+ *
+ */
+
+#ifndef _KEYS_caam_TYPE_H
+#define _KEYS_caam_TYPE_H
+
+#include <linux/rcupdate.h>
+#include <linux/key-type.h>
+#include <keys/user-type.h>
+#include "caam_desc.h"
+#include "tag_object.h"
+
+extern struct key_type key_type_caam_tk;
+
+/* Minimum key size to be used is 32 bytes and maximum key size fixed
+ * is 128 bytes.
+ * Blob size to be kept is Maximum key size + blob header added by CAAM.
+ */
+
+#define MIN_KEY_SIZE 16
+#define MAX_KEY_SIZE 128
+
+#define MAX_BLACK_KEY_SIZE (MAX_KEY_SIZE + CCM_OVERHEAD +\
+ TAG_OVERHEAD)
+
+#define MAX_BLOB_SIZE (MAX_KEY_SIZE + BLOB_OVERHEAD)
+
+struct caam_key_payload {
+ /*
+ * The aliasing of the structure allow user to see this payload
+ * as a user defined payload
+ *
+ * The structure has to be set during execution
+ */
+ struct aliased_user_key_payload {
+ struct rcu_head rcu;
+ unsigned short datalen;
+ char data[MAX_BLACK_KEY_SIZE];
+ } upayload;
+
+ size_t key_len;
+ unsigned char key[MAX_KEY_SIZE + 1];
+ int key_fmt_val;
+
+ size_t black_key_len;
+ unsigned char black_key[MAX_BLACK_KEY_SIZE];
+
+ size_t blob_len;
+ unsigned char blob[MAX_BLOB_SIZE];
+
+ size_t key_mod_len;
+ const void *key_mod;
+};
+
+#endif
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 2/2] dm-crypt: Use any key type which is registered
From: Franck LENORMAND @ 2019-03-01 16:09 UTC (permalink / raw)
To: linux-kernel, linux-security-module, keyrings
Cc: franck.lenormand, horia.geanta, silvano.dininno, agk, snitzer,
dm-devel, dhowells, jmorris, serge
In-Reply-To: <1551456599-10603-1-git-send-email-franck.lenormand@nxp.com>
There was only 2 key_type supported by dm-crypt which limits other
implementations.
This patch allows to use any key_type which is registered obtaining
the key_type from key framework.
This also remove the compilation dependency between dm-crypt and
key implementations.
Signed-off-by: Franck LENORMAND <franck.lenormand@nxp.com>
---
drivers/md/dm-crypt.c | 11 ++++++-----
include/linux/key-type.h | 2 ++
security/keys/key.c | 42 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index dd538e6..e25efc2 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -35,6 +35,7 @@
#include <crypto/authenc.h>
#include <linux/rtnetlink.h> /* for struct rtattr and RTA macros only */
#include <keys/user-type.h>
+#include <linux/key-type.h>
#include <linux/device-mapper.h>
@@ -2010,6 +2011,7 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
int ret;
struct key *key;
const struct user_key_payload *ukp;
+ struct key_type *type;
/*
* Reject key_string with whitespace. dm core currently lacks code for
@@ -2025,16 +2027,15 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
if (!key_desc || key_desc == key_string || !strlen(key_desc + 1))
return -EINVAL;
- if (strncmp(key_string, "logon:", key_desc - key_string + 1) &&
- strncmp(key_string, "user:", key_desc - key_string + 1))
- return -EINVAL;
+ type = get_key_type(key_string, key_desc - key_string);
+ if (!type)
+ return -ENOENT;
new_key_string = kstrdup(key_string, GFP_KERNEL);
if (!new_key_string)
return -ENOMEM;
- key = request_key(key_string[0] == 'l' ? &key_type_logon : &key_type_user,
- key_desc + 1, NULL);
+ key = request_key(type, key_desc + 1, NULL);
if (IS_ERR(key)) {
kzfree(new_key_string);
return PTR_ERR(key);
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index bc9af55..2b2167b 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -176,6 +176,8 @@ extern struct key_type key_type_keyring;
extern int register_key_type(struct key_type *ktype);
extern void unregister_key_type(struct key_type *ktype);
+extern struct key_type *get_key_type(const char *type_name, size_t string_size);
+
extern int key_payload_reserve(struct key *key, size_t datalen);
extern int key_instantiate_and_link(struct key *key,
const void *data,
diff --git a/security/keys/key.c b/security/keys/key.c
index 44a80d6..ef76114 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -1156,6 +1156,48 @@ void unregister_key_type(struct key_type *ktype)
}
EXPORT_SYMBOL(unregister_key_type);
+/**
+ * get_key_type - Get the type of key using its name
+ * @type_name: Name of the key type to get
+ * @string_size: Size of the string to match
+ *
+ * The functions support null ended string (string_size == 0) as well as
+ * pointer on a string matching a number of characters (string_size > 0)
+ *
+ * Returns a pointer on the key type if successful, -ENOENT if the key type
+ * is not registered.
+ */
+struct key_type *get_key_type(const char *type_name, size_t string_size)
+{
+ struct key_type *p;
+ struct key_type *ktype = ERR_PTR(-ENOENT);
+
+ if (!type_name)
+ return ktype;
+
+ down_write(&key_types_sem);
+
+ /* Search the key type in the list */
+ list_for_each_entry(p, &key_types_list, link) {
+ if (string_size) {
+ if (strncmp(p->name, type_name, string_size) == 0) {
+ ktype = p;
+ break;
+ }
+ } else {
+ if (strcmp(p->name, type_name) == 0) {
+ ktype = p;
+ break;
+ }
+ }
+ }
+
+ up_read(&key_types_sem);
+
+ return ktype;
+}
+EXPORT_SYMBOL(get_key_type);
+
/*
* Initialise the key management state.
*/
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 0/2] Create CAAM HW key in linux keyring and use in dmcrypt
From: Franck LENORMAND @ 2019-03-01 16:09 UTC (permalink / raw)
To: linux-kernel, linux-security-module, keyrings
Cc: franck.lenormand, horia.geanta, silvano.dininno, agk, snitzer,
dm-devel, dhowells, jmorris, serge
This RFC aims to provide better security of users using CAAM block allowing
them to encrypt their disks with a hardware key. A hardware key is a
key only know by the hardware hence not known neither by the userspace nor
the kernelspace.
The use of hardware keys allows to mitigate attacks against systems using
such type of keys as the key is more complicated to obtain. In effect, even
a compromised kernel could not divulge the key used to encrypt the data.
The user or the kernel can only configure a device mapper using the target
crypt and provide the HW key to encrypt a disk.
The CAAM possesses a mechanism to generate HW keys called "black key".
The CAAM has the capacity to encrypt a plain key ("red key") with a key
only accessible by the hardware hence creating a "black key". The
encryption key used change at each reset of the CAAM: shutdown, reboot.
generate a black key: <plain key> -> CAAM -> <black key>
To keep a black key between reset, it must be stored in filesystem in a
structure called "black blob". This structure contains the "black key" but
also encryption information. It is also generated by the CAAM using a key
only accessible by the hardware but which doesn't change between reset of
the CAAM.
generate a black blob: <black key> -> CAAM -> <black blob> -> FS
obtain the black key: FS -> <black blob> -> CAAM -> <black key>
The creation of such structures and its use was not exposed to userspace so
it was complicated to use and required custom development. We would like to
ease this using interface which are known and used:
- Linux key retention service : Allow to generate or load keys in a
keyring which can be used by applications.
- dm-crypt : device mapper allowing to encrypt data.
The capacity to generate or load keys already available in the Linux key
retention service does not allows to exploit CAAM capabilities hence we
need to create a new key_type. The new key type "caam_tk" allows to:
- Create a black key from random
- Create a black key from a red key
- Load a black blob to retrieve the black key
The patch 01 presents the implementation of the key type is available in
the RFC. This implementation is presented to give context but cannot be
compiled as it requires other components not presented in this RFC.
We target dm-crypt to perform the disk encryption. It can retrieve keys
from the Linux key retention service however it only supports the key
type "user" and "logon". In order to use keys from our new key type, we
need to either:
- add support for our new key type : dm-crypt need a reference on the key
type structure.
- support any key type : the reference on the key type is retrieved from
the key subsystem based on the name of the key type
The patch 02 present the implementation of the second solution. The main
advantage of this implementation are:
- The reference on the key type is no more needed at compile time
- All future key type will be supported
- Only the key type registered can be obtained (key type as module)
We think that this modification of dm-crypt and the key subsystem will ease
the use of dm-crypt and the development of key type.
Franck LENORMAND (2):
drivers: crypto: caam: key: Add caam_tk key type
dm-crypt: Use any key type which is registered
drivers/crypto/caam/caam_key.c | 623 +++++++++++++++++++++++++++++++++++++++++
drivers/crypto/caam/caam_key.h | 58 ++++
drivers/md/dm-crypt.c | 11 +-
include/linux/key-type.h | 2 +
security/keys/key.c | 42 +++
5 files changed, 731 insertions(+), 5 deletions(-)
create mode 100644 drivers/crypto/caam/caam_key.c
create mode 100644 drivers/crypto/caam/caam_key.h
--
2.7.4
^ permalink raw reply
* Re: [PATCH 05/97] LSM: Create an lsm_export data structure.
From: Casey Schaufler @ 2019-03-01 16:46 UTC (permalink / raw)
To: Stephen Smalley, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <7199670f-a38a-bf5e-4c3b-d340caa35071@tycho.nsa.gov>
On 3/1/2019 6:00 AM, Stephen Smalley wrote:
> On 2/28/19 5:18 PM, Casey Schaufler wrote:
>> When more than one security module is exporting data to
>> audit and networking sub-systems a single 32 bit integer
>> is no longer sufficient to represent the data. Add a
>> structure to be used instead.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> include/linux/security.h | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 13537a49ae97..a79fe8ef9d84 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -73,6 +73,18 @@ enum lsm_event {
>> LSM_POLICY_CHANGE,
>> };
>> +/* Data exported by the security modules */
>> +struct lsm_export {
>> + u32 selinux;
>> + u32 smack;
>> + u32 apparmor;
>> + u32 flags;
>> +};
>> +#define LSM_EXPORT_NONE 0x00
>> +#define LSM_EXPORT_SELINUX 0x01
>> +#define LSM_EXPORT_SMACK 0x02
>> +#define LSM_EXPORT_APPARMOR 0x04
>
> Can this be generalized to avoid hardcoding the names of specific
> security modules in the field and symbol names? Possibly just an
> array of secids with the indices dynamically assigned by the
> infrastructure at registration time?
Yes, it can. I considered doing so very seriously. The reason
not to do it is data lifecycle management. In today's code secids
are often allocated on the stack and passed to code that holds the
value indefinitely. If every assignment became an allocate and copy
operation there would have to be reference counting and a lot more
intelligence.
The other advantage to the scheme used here is that an lsm_export
can include something other than a secid should a security module
so choose. It's not in this set, but I plan to change the Smack
entry from a u32 to a struct smack_known *, making several operations
much more efficient. Of course, that could be done using blob
management, but the complexity increases yet again.
> We don't really want to have to patch this structure every time
> someone adds a new security module that needs audit and/or network
> facilities, right?
It's not a design that is being proposed without consideration.
I seem to recall hearing the lifecycle arguments as a primary
rationale for secids more than once.
>
>> +
>> /* These functions are in security/commoncap.c */
>> extern int cap_capable(const struct cred *cred, struct
>> user_namespace *ns,
>> int cap, unsigned int opts);
>>
>
^ permalink raw reply
* Re: [PATCH 01/97] LSM: Infrastructure management of the superblock
From: Casey Schaufler @ 2019-03-01 16:50 UTC (permalink / raw)
To: Edwin Zimmerman, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <000701d4d037$6e9d7aa0$4bd86fe0$@211mainstreet.net>
On 3/1/2019 6:02 AM, Edwin Zimmerman wrote:
> On Thursday2/28/2019 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> From: Casey Schaufler <cschaufler@localhost.localdomain>
>>
>> Move management of the superblock->sb_security blob out
>> of the individual security modules and into the security
>> infrastructure. Instead of allocating the blobs from within
>> the modules the modules tell the infrastructure how much
>> space is required, and the space is allocated there.
>>
> ...
>> @@ -296,12 +297,13 @@ static void __init ordered_lsm_init(void)
>> for (lsm = ordered_lsms; *lsm; lsm++)
>> prepare_lsm(*lsm);
>>
>> - init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
>> - init_debug("file blob size = %d\n", blob_sizes.lbs_file);
>> - init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
>> - init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
>> - init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
>> - init_debug("task blob size = %d\n", blob_sizes.lbs_task);
>> + init_debug("cred blob size = %d\n", blob_sizes.lbs_cred);
>> + init_debug("file blob size = %d\n", blob_sizes.lbs_file);
>> + init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
>> + init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
>> + init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
>> + init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
>> + init_debug("task blob size = %d\n", blob_sizes.lbs_task);
>>
> Any reason this couldn't be a single line, or am I missing something?
Indentation. If the preceding patches had gotten the indentation
right this would be the one line patch you suggest. To be fixed in
the next version.
> It took a second look to realize that the only line actually being added was
> + init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
>
>
^ permalink raw reply
* Re: [PATCH 11/97] LSM: Use lsm_export in the kernel_ask_as hooks
From: Casey Schaufler @ 2019-03-01 16:59 UTC (permalink / raw)
To: Edwin Zimmerman, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <000b01d4d03f$567e1e90$037a5bb0$@211mainstreet.net>
On 3/1/2019 6:59 AM, Edwin Zimmerman wrote:
> On Thursday, 2/28/2019 at 5:18 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Convert the kernel_ask_as hooks to use the lsm_export
> Should be act_as, not? ^^
You are correct.
^ permalink raw reply
* Re: [PATCH 00/97] LSM: Complete module stacking
From: Casey Schaufler @ 2019-03-01 17:06 UTC (permalink / raw)
To: Stephen Smalley, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <c334385a-89e8-169a-3e92-4b816145391d@tycho.nsa.gov>
On 3/1/2019 6:17 AM, Stephen Smalley wrote:
> On 2/28/19 5:17 PM, Casey Schaufler wrote:
>> This is a preliminary version of the complete stacking
>> implementation. The patches need to be cleaned up, and
>> several are not strictly necessary. There is likely to
>> be work required in the audit sub-system. It does address
>> all the shared data, including CIPSO headers. It should
>> handle CALIPSO once Smack supports it. I will be revising
>> the set after 5.1.
>>
>> Complete the transition from module based blob management
>> to infrastructure based blob management. This includes
>> the socket, superblock and key blobs.
>>
>> Change the LSM infrastructure from exposing secids to
>> exposing an opaque "lsm_export" structure that can contain
>> information for multiple active security modules. Update
>> all of the security modules to use information from the
>> lsm_export structure. Update the LSM interfaces that expose
>> secids for more than one module to use the export structure.
>> Update all the users of these interfaces.
>>
>> Change the LSM infrastructure from using a string/size pair
>> for security "contexts" to a "lsm_context" structure that
>> can represent information for multiple modules. This contains
>> information that allows the "context" to be properly freed
>> regardless of where it is allocated and where it is used.
>>
>> Add an interface to identify which security module data
>> should be presented with SO_PEERSEC. /proc/.../attr/display
>> will set and report the name of the LSM for which the
>> security_secid_to_secctx() will use to translate to text.
>> If it is not explicitly set, the first security module that
>> supplies secid (now lsm_export) interfaces will be used.
>> To ensure consistency, a set of module hooks dealing with
>> the secid/context processing is maintained with each process
>> that explicitly sets it.
>>
>> Before sending a network packet verify that all interested
>> security modules agree on the labeling. Fail if the labeling
>> cannot be reconciled. This requires a new Netlabel interface
>> to compare proposed labels, and a change to the return values
>> from the existing netlabel attribute setting functions.
>
> Have you run any benchmarks to assess the performance impact of these
> changes?
Nothing I can publish. Benchmarking is getting close
to the top of the list.
^ permalink raw reply
* Re: [PATCH] LSM: Update list of SECURITYFS users in Kconfig
From: James Morris @ 2019-03-01 17:54 UTC (permalink / raw)
To: Petr Vorel; +Cc: linux-security-module
In-Reply-To: <20190228235448.14279-1-pvorel@suse.cz>
On Fri, 1 Mar 2019, Petr Vorel wrote:
> + Remove modules not using it (SELinux and SMACK aren't
> the only ones not using it).
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-general
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH] keys: fix missing __user in KEYCTL_PKEY_QUERY
From: Serge E. Hallyn @ 2019-03-01 22:28 UTC (permalink / raw)
To: Ben Dooks
Cc: keyrings, linux-security-module, dhowells, jmorris, serge,
linux-kernel
In-Reply-To: <20190301113026.17721-1-ben.dooks@codethink.co.uk>
On Fri, Mar 01, 2019 at 11:30:26AM +0000, Ben Dooks wrote:
> The arg5 of KEYCTL_PKEY_QUERY should have a __user pointer tag on
> it as it is a user pointer. This clears the following sparse warning
> for this:
>
> security/keys/keyctl.c:1755:43: warning: incorrect type in argument 3 (different address spaces)
> security/keys/keyctl.c:1755:43: expected struct keyctl_pkey_query [noderef] <asn:1>*<noident>
> security/keys/keyctl.c:1755:43: got struct keyctl_pkey_query *<noident>
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Acked-by: Serge Hallyn <serge@hallyn.com>
> ---
> security/keys/keyctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index 7bbe03593e58..3e4053a217c3 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1752,7 +1752,7 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
> return -EINVAL;
> return keyctl_pkey_query((key_serial_t)arg2,
> (const char __user *)arg4,
> - (struct keyctl_pkey_query *)arg5);
> + (struct keyctl_pkey_query __user *)arg5);
>
> case KEYCTL_PKEY_ENCRYPT:
> case KEYCTL_PKEY_DECRYPT:
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH 15/43] vfs: Add configuration parser helpers
From: Al Viro @ 2019-03-03 2:53 UTC (permalink / raw)
To: David Howells; +Cc: linux-fsdevel, torvalds, ebiederm, linux-security-module
In-Reply-To: <155059382463.12449.9058984258656142511.stgit@warthog.procyon.org.uk>
On Tue, Feb 19, 2019 at 04:30:24PM +0000, David Howells wrote:
> Because the new API passes in key,value parameters, match_token() cannot be
> used with it. Instead, provide three new helpers to aid with parsing:
[snip]
... and the same, with horrible macros taken out and shot. The only difference
is in the way fsparam_...() family is defined; it's *NOT* a weird vararg now.
All users in that series are two-argument ones, so they remain as-is.
Not-ready-for-merge NFS (and even less ready btrfs) patch series had been
adjusted to match (with results of macro expansion unchanged compared to
the earlier variant).
commit 31d921c7fb9691722ba9503b64153cdc322a7fa8
Author: David Howells <dhowells@redhat.com>
Date: Thu Nov 1 23:07:24 2018 +0000
vfs: Add configuration parser helpers
Because the new API passes in key,value parameters, match_token() cannot be
used with it. Instead, provide three new helpers to aid with parsing:
(1) fs_parse(). This takes a parameter and a simple static description of
all the parameters and maps the key name to an ID. It returns 1 on a
match, 0 on no match if unknowns should be ignored and some other
negative error code on a parse error.
The parameter description includes a list of key names to IDs, desired
parameter types and a list of enumeration name -> ID mappings.
[!] Note that for the moment I've required that the key->ID mapping
array is expected to be sorted and unterminated. The size of the
array is noted in the fsconfig_parser struct. This allows me to use
bsearch(), but I'm not sure any performance gain is worth the hassle
of requiring people to keep the array sorted.
The parameter type array is sized according to the number of parameter
IDs and is indexed directly. The optional enum mapping array is an
unterminated, unsorted list and the size goes into the fsconfig_parser
struct.
The function can do some additional things:
(a) If it's not ambiguous and no value is given, the prefix "no" on
a key name is permitted to indicate that the parameter should
be considered negatory.
(b) If the desired type is a single simple integer, it will perform
an appropriate conversion and store the result in a union in
the parse result.
(c) If the desired type is an enumeration, {key ID, name} will be
looked up in the enumeration list and the matching value will
be stored in the parse result union.
(d) Optionally generate an error if the key is unrecognised.
This is called something like:
enum rdt_param {
Opt_cdp,
Opt_cdpl2,
Opt_mba_mpbs,
nr__rdt_params
};
const struct fs_parameter_spec rdt_param_specs[nr__rdt_params] = {
[Opt_cdp] = { fs_param_is_bool },
[Opt_cdpl2] = { fs_param_is_bool },
[Opt_mba_mpbs] = { fs_param_is_bool },
};
const const char *const rdt_param_keys[nr__rdt_params] = {
[Opt_cdp] = "cdp",
[Opt_cdpl2] = "cdpl2",
[Opt_mba_mpbs] = "mba_mbps",
};
const struct fs_parameter_description rdt_parser = {
.name = "rdt",
.nr_params = nr__rdt_params,
.keys = rdt_param_keys,
.specs = rdt_param_specs,
.no_source = true,
};
int rdt_parse_param(struct fs_context *fc,
struct fs_parameter *param)
{
struct fs_parse_result parse;
struct rdt_fs_context *ctx = rdt_fc2context(fc);
int ret;
ret = fs_parse(fc, &rdt_parser, param, &parse);
if (ret < 0)
return ret;
switch (parse.key) {
case Opt_cdp:
ctx->enable_cdpl3 = true;
return 0;
case Opt_cdpl2:
ctx->enable_cdpl2 = true;
return 0;
case Opt_mba_mpbs:
ctx->enable_mba_mbps = true;
return 0;
}
return -EINVAL;
}
(2) fs_lookup_param(). This takes a { dirfd, path, LOOKUP_EMPTY? } or
string value and performs an appropriate path lookup to convert it
into a path object, which it will then return.
If the desired type was a blockdev, the type of the looked up inode
will be checked to make sure it is one.
This can be used like:
enum foo_param {
Opt_source,
nr__foo_params
};
const struct fs_parameter_spec foo_param_specs[nr__foo_params] = {
[Opt_source] = { fs_param_is_blockdev },
};
const char *char foo_param_keys[nr__foo_params] = {
[Opt_source] = "source",
};
const struct constant_table foo_param_alt_keys[] = {
{ "device", Opt_source },
};
const struct fs_parameter_description foo_parser = {
.name = "foo",
.nr_params = nr__foo_params,
.nr_alt_keys = ARRAY_SIZE(foo_param_alt_keys),
.keys = foo_param_keys,
.alt_keys = foo_param_alt_keys,
.specs = foo_param_specs,
};
int foo_parse_param(struct fs_context *fc,
struct fs_parameter *param)
{
struct fs_parse_result parse;
struct foo_fs_context *ctx = foo_fc2context(fc);
int ret;
ret = fs_parse(fc, &foo_parser, param, &parse);
if (ret < 0)
return ret;
switch (parse.key) {
case Opt_source:
return fs_lookup_param(fc, &foo_parser, param,
&parse, &ctx->source);
default:
return -EINVAL;
}
}
(3) lookup_constant(). This takes a table of named constants and looks up
the given name within it. The table is expected to be sorted such
that bsearch() be used upon it.
Possibly I should require the table be terminated and just use a
for-loop to scan it instead of using bsearch() to reduce hassle.
Tables look something like:
static const struct constant_table bool_names[] = {
{ "0", false },
{ "1", true },
{ "false", false },
{ "no", false },
{ "true", true },
{ "yes", true },
};
and a lookup is done with something like:
b = lookup_constant(bool_names, param->string, -1);
Additionally, optional validation routines for the parameter description
are provided that can be enabled at compile time. A later patch will
invoke these when a filesystem is registered.
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
diff --git a/fs/Kconfig b/fs/Kconfig
index ac474a61be37..25700b152c75 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -8,6 +8,13 @@ menu "File systems"
config DCACHE_WORD_ACCESS
bool
+config VALIDATE_FS_PARSER
+ bool "Validate filesystem parameter description"
+ default y
+ help
+ Enable this to perform validation of the parameter description for a
+ filesystem when it is registered.
+
if BLOCK
config FS_IOMAP
diff --git a/fs/Makefile b/fs/Makefile
index 5563cf34f7c2..9a0b8003f069 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -13,7 +13,7 @@ obj-y := open.o read_write.o file_table.o super.o \
seq_file.o xattr.o libfs.o fs-writeback.o \
pnode.o splice.o sync.o utimes.o d_path.o \
stack.o fs_struct.o statfs.o fs_pin.o nsfs.o \
- fs_context.o
+ fs_context.o fs_parser.o
ifeq ($(CONFIG_BLOCK),y)
obj-y += buffer.o block_dev.o direct-io.o mpage.o
diff --git a/fs/fs_parser.c b/fs/fs_parser.c
new file mode 100644
index 000000000000..842e8f749db6
--- /dev/null
+++ b/fs/fs_parser.c
@@ -0,0 +1,447 @@
+/* Filesystem parameter parser.
+ *
+ * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/export.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
+#include <linux/slab.h>
+#include <linux/security.h>
+#include <linux/namei.h>
+#include "internal.h"
+
+static const struct constant_table bool_names[] = {
+ { "0", false },
+ { "1", true },
+ { "false", false },
+ { "no", false },
+ { "true", true },
+ { "yes", true },
+};
+
+/**
+ * lookup_constant - Look up a constant by name in an ordered table
+ * @tbl: The table of constants to search.
+ * @tbl_size: The size of the table.
+ * @name: The name to look up.
+ * @not_found: The value to return if the name is not found.
+ */
+int __lookup_constant(const struct constant_table *tbl, size_t tbl_size,
+ const char *name, int not_found)
+{
+ unsigned int i;
+
+ for (i = 0; i < tbl_size; i++)
+ if (strcmp(name, tbl[i].name) == 0)
+ return tbl[i].value;
+
+ return not_found;
+}
+EXPORT_SYMBOL(__lookup_constant);
+
+static const struct fs_parameter_spec *fs_lookup_key(
+ const struct fs_parameter_description *desc,
+ const char *name)
+{
+ const struct fs_parameter_spec *p;
+
+ if (!desc->specs)
+ return NULL;
+
+ for (p = desc->specs; p->name; p++)
+ if (strcmp(p->name, name) == 0)
+ return p;
+
+ return NULL;
+}
+
+/*
+ * fs_parse - Parse a filesystem configuration parameter
+ * @fc: The filesystem context to log errors through.
+ * @desc: The parameter description to use.
+ * @param: The parameter.
+ * @result: Where to place the result of the parse
+ *
+ * Parse a filesystem configuration parameter and attempt a conversion for a
+ * simple parameter for which this is requested. If successful, the determined
+ * parameter ID is placed into @result->key, the desired type is indicated in
+ * @result->t and any converted value is placed into an appropriate member of
+ * the union in @result.
+ *
+ * The function returns the parameter number if the parameter was matched,
+ * -ENOPARAM if it wasn't matched and @desc->ignore_unknown indicated that
+ * unknown parameters are okay and -EINVAL if there was a conversion issue or
+ * the parameter wasn't recognised and unknowns aren't okay.
+ */
+int fs_parse(struct fs_context *fc,
+ const struct fs_parameter_description *desc,
+ struct fs_parameter *param,
+ struct fs_parse_result *result)
+{
+ const struct fs_parameter_spec *p;
+ const struct fs_parameter_enum *e;
+ int ret = -ENOPARAM, b;
+
+ result->has_value = !!param->string;
+ result->negated = false;
+ result->uint_64 = 0;
+
+ p = fs_lookup_key(desc, param->key);
+ if (!p) {
+ /* If we didn't find something that looks like "noxxx", see if
+ * "xxx" takes the "no"-form negative - but only if there
+ * wasn't an value.
+ */
+ if (result->has_value)
+ goto unknown_parameter;
+ if (param->key[0] != 'n' || param->key[1] != 'o' || !param->key[2])
+ goto unknown_parameter;
+
+ p = fs_lookup_key(desc, param->key + 2);
+ if (!p)
+ goto unknown_parameter;
+ if (!(p->flags & fs_param_neg_with_no))
+ goto unknown_parameter;
+ result->boolean = false;
+ result->negated = true;
+ }
+
+ if (p->flags & fs_param_deprecated)
+ warnf(fc, "%s: Deprecated parameter '%s'",
+ desc->name, param->key);
+
+ if (result->negated)
+ goto okay;
+
+ /* Certain parameter types only take a string and convert it. */
+ switch (p->type) {
+ case __fs_param_wasnt_defined:
+ return -EINVAL;
+ case fs_param_is_u32:
+ case fs_param_is_u32_octal:
+ case fs_param_is_u32_hex:
+ case fs_param_is_s32:
+ case fs_param_is_u64:
+ case fs_param_is_enum:
+ case fs_param_is_string:
+ if (param->type != fs_value_is_string)
+ goto bad_value;
+ if (!result->has_value) {
+ if (p->flags & fs_param_v_optional)
+ goto okay;
+ goto bad_value;
+ }
+ /* Fall through */
+ default:
+ break;
+ }
+
+ /* Try to turn the type we were given into the type desired by the
+ * parameter and give an error if we can't.
+ */
+ switch (p->type) {
+ case fs_param_is_flag:
+ if (param->type != fs_value_is_flag &&
+ (param->type != fs_value_is_string || result->has_value))
+ return invalf(fc, "%s: Unexpected value for '%s'",
+ desc->name, param->key);
+ result->boolean = true;
+ goto okay;
+
+ case fs_param_is_bool:
+ switch (param->type) {
+ case fs_value_is_flag:
+ result->boolean = true;
+ goto okay;
+ case fs_value_is_string:
+ if (param->size == 0) {
+ result->boolean = true;
+ goto okay;
+ }
+ b = lookup_constant(bool_names, param->string, -1);
+ if (b == -1)
+ goto bad_value;
+ result->boolean = b;
+ goto okay;
+ default:
+ goto bad_value;
+ }
+
+ case fs_param_is_u32:
+ ret = kstrtouint(param->string, 0, &result->uint_32);
+ goto maybe_okay;
+ case fs_param_is_u32_octal:
+ ret = kstrtouint(param->string, 8, &result->uint_32);
+ goto maybe_okay;
+ case fs_param_is_u32_hex:
+ ret = kstrtouint(param->string, 16, &result->uint_32);
+ goto maybe_okay;
+ case fs_param_is_s32:
+ ret = kstrtoint(param->string, 0, &result->int_32);
+ goto maybe_okay;
+ case fs_param_is_u64:
+ ret = kstrtoull(param->string, 0, &result->uint_64);
+ goto maybe_okay;
+
+ case fs_param_is_enum:
+ for (e = desc->enums; e->name[0]; e++) {
+ if (e->opt == p->opt &&
+ strcmp(e->name, param->string) == 0) {
+ result->uint_32 = e->value;
+ goto okay;
+ }
+ }
+ goto bad_value;
+
+ case fs_param_is_string:
+ goto okay;
+ case fs_param_is_blob:
+ if (param->type != fs_value_is_blob)
+ goto bad_value;
+ goto okay;
+
+ case fs_param_is_fd: {
+ if (param->type != fs_value_is_file)
+ goto bad_value;
+ goto okay;
+ }
+
+ case fs_param_is_blockdev:
+ case fs_param_is_path:
+ goto okay;
+ default:
+ BUG();
+ }
+
+maybe_okay:
+ if (ret < 0)
+ goto bad_value;
+okay:
+ return p->opt;
+
+bad_value:
+ return invalf(fc, "%s: Bad value for '%s'", desc->name, param->key);
+unknown_parameter:
+ return -ENOPARAM;
+}
+EXPORT_SYMBOL(fs_parse);
+
+/**
+ * fs_lookup_param - Look up a path referred to by a parameter
+ * @fc: The filesystem context to log errors through.
+ * @param: The parameter.
+ * @want_bdev: T if want a blockdev
+ * @_path: The result of the lookup
+ */
+int fs_lookup_param(struct fs_context *fc,
+ struct fs_parameter *param,
+ bool want_bdev,
+ struct path *_path)
+{
+ struct filename *f;
+ unsigned int flags = 0;
+ bool put_f;
+ int ret;
+
+ switch (param->type) {
+ case fs_value_is_string:
+ f = getname_kernel(param->string);
+ if (IS_ERR(f))
+ return PTR_ERR(f);
+ put_f = true;
+ break;
+ case fs_value_is_filename_empty:
+ flags = LOOKUP_EMPTY;
+ /* Fall through */
+ case fs_value_is_filename:
+ f = param->name;
+ put_f = false;
+ break;
+ default:
+ return invalf(fc, "%s: not usable as path", param->key);
+ }
+
+ ret = filename_lookup(param->dirfd, f, flags, _path, NULL);
+ if (ret < 0) {
+ errorf(fc, "%s: Lookup failure for '%s'", param->key, f->name);
+ goto out;
+ }
+
+ if (want_bdev &&
+ !S_ISBLK(d_backing_inode(_path->dentry)->i_mode)) {
+ path_put(_path);
+ _path->dentry = NULL;
+ _path->mnt = NULL;
+ errorf(fc, "%s: Non-blockdev passed as '%s'",
+ param->key, f->name);
+ ret = -ENOTBLK;
+ }
+
+out:
+ if (put_f)
+ putname(f);
+ return ret;
+}
+EXPORT_SYMBOL(fs_lookup_param);
+
+#ifdef CONFIG_VALIDATE_FS_PARSER
+/**
+ * validate_constant_table - Validate a constant table
+ * @name: Name to use in reporting
+ * @tbl: The constant table to validate.
+ * @tbl_size: The size of the table.
+ * @low: The lowest permissible value.
+ * @high: The highest permissible value.
+ * @special: One special permissible value outside of the range.
+ */
+bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size,
+ int low, int high, int special)
+{
+ size_t i;
+ bool good = true;
+
+ if (tbl_size == 0) {
+ pr_warn("VALIDATE C-TBL: Empty\n");
+ return true;
+ }
+
+ for (i = 0; i < tbl_size; i++) {
+ if (!tbl[i].name) {
+ pr_err("VALIDATE C-TBL[%zu]: Null\n", i);
+ good = false;
+ } else if (i > 0 && tbl[i - 1].name) {
+ int c = strcmp(tbl[i-1].name, tbl[i].name);
+
+ if (c == 0) {
+ pr_err("VALIDATE C-TBL[%zu]: Duplicate %s\n",
+ i, tbl[i].name);
+ good = false;
+ }
+ if (c > 0) {
+ pr_err("VALIDATE C-TBL[%zu]: Missorted %s>=%s\n",
+ i, tbl[i-1].name, tbl[i].name);
+ good = false;
+ }
+ }
+
+ if (tbl[i].value != special &&
+ (tbl[i].value < low || tbl[i].value > high)) {
+ pr_err("VALIDATE C-TBL[%zu]: %s->%d const out of range (%d-%d)\n",
+ i, tbl[i].name, tbl[i].value, low, high);
+ good = false;
+ }
+ }
+
+ return good;
+}
+
+/**
+ * fs_validate_description - Validate a parameter description
+ * @desc: The parameter description to validate.
+ */
+bool fs_validate_description(const struct fs_parameter_description *desc)
+{
+ const struct fs_parameter_spec *param, *p2;
+ const struct fs_parameter_enum *e;
+ const char *name = desc->name;
+ unsigned int nr_params = 0;
+ bool good = true, enums = false;
+
+ pr_notice("*** VALIDATE %s ***\n", name);
+
+ if (!name[0]) {
+ pr_err("VALIDATE Parser: No name\n");
+ name = "Unknown";
+ good = false;
+ }
+
+ if (desc->specs) {
+ for (param = desc->specs; param->name; param++) {
+ enum fs_parameter_type t = param->type;
+
+ /* Check that the type is in range */
+ if (t == __fs_param_wasnt_defined ||
+ t >= nr__fs_parameter_type) {
+ pr_err("VALIDATE %s: PARAM[%s] Bad type %u\n",
+ name, param->name, t);
+ good = false;
+ } else if (t == fs_param_is_enum) {
+ enums = true;
+ }
+
+ /* Check for duplicate parameter names */
+ for (p2 = desc->specs; p2 < param; p2++) {
+ if (strcmp(param->name, p2->name) == 0) {
+ pr_err("VALIDATE %s: PARAM[%s]: Duplicate\n",
+ name, param->name);
+ good = false;
+ }
+ }
+ }
+
+ nr_params = param - desc->specs;
+ }
+
+ if (desc->enums) {
+ if (!nr_params) {
+ pr_err("VALIDATE %s: Enum table but no parameters\n",
+ name);
+ good = false;
+ goto no_enums;
+ }
+ if (!enums) {
+ pr_err("VALIDATE %s: Enum table but no enum-type values\n",
+ name);
+ good = false;
+ goto no_enums;
+ }
+
+ for (e = desc->enums; e->name[0]; e++) {
+ /* Check that all entries in the enum table have at
+ * least one parameter that uses them.
+ */
+ for (param = desc->specs; param->name; param++) {
+ if (param->opt == e->opt &&
+ param->type != fs_param_is_enum) {
+ pr_err("VALIDATE %s: e[%lu] enum val for %s\n",
+ name, e - desc->enums, param->name);
+ good = false;
+ }
+ }
+ }
+
+ /* Check that all enum-type parameters have at least one enum
+ * value in the enum table.
+ */
+ for (param = desc->specs; param->name; param++) {
+ if (param->type != fs_param_is_enum)
+ continue;
+ for (e = desc->enums; e->name[0]; e++)
+ if (e->opt == param->opt)
+ break;
+ if (!e->name[0]) {
+ pr_err("VALIDATE %s: PARAM[%s] enum with no values\n",
+ name, param->name);
+ good = false;
+ }
+ }
+ } else {
+ if (enums) {
+ pr_err("VALIDATE %s: enum-type values, but no enum table\n",
+ name);
+ good = false;
+ goto no_enums;
+ }
+ }
+
+no_enums:
+ return good;
+}
+#endif /* CONFIG_VALIDATE_FS_PARSER */
diff --git a/fs/internal.h b/fs/internal.h
index 8f8d07cc433f..6a8b71643af4 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -61,6 +61,8 @@ extern void fc_drop_locked(struct fs_context *);
/*
* namei.c
*/
+extern int filename_lookup(int dfd, struct filename *name, unsigned flags,
+ struct path *path, struct path *root);
extern int user_path_mountpoint_at(int, const char __user *, unsigned int, struct path *);
extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
const char *, unsigned int, struct path *);
diff --git a/fs/namei.c b/fs/namei.c
index 914178cdbe94..a85deb55d0c9 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2333,8 +2333,8 @@ static int path_lookupat(struct nameidata *nd, unsigned flags, struct path *path
return err;
}
-static int filename_lookup(int dfd, struct filename *name, unsigned flags,
- struct path *path, struct path *root)
+int filename_lookup(int dfd, struct filename *name, unsigned flags,
+ struct path *path, struct path *root)
{
int retval;
struct nameidata nd;
diff --git a/include/linux/errno.h b/include/linux/errno.h
index 3cba627577d6..d73f597a2484 100644
--- a/include/linux/errno.h
+++ b/include/linux/errno.h
@@ -18,6 +18,7 @@
#define ERESTART_RESTARTBLOCK 516 /* restart by calling sys_restart_syscall */
#define EPROBE_DEFER 517 /* Driver requests probe retry */
#define EOPENSTALE 518 /* open found a stale dentry */
+#define ENOPARAM 519 /* Parameter not supported */
/* Defined for the NFSv3 protocol */
#define EBADHANDLE 521 /* Illegal NFS file handle */
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index d208cc40b868..899027c94788 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -35,6 +35,35 @@ enum fs_context_purpose {
};
/*
+ * Type of parameter value.
+ */
+enum fs_value_type {
+ fs_value_is_undefined,
+ fs_value_is_flag, /* Value not given a value */
+ fs_value_is_string, /* Value is a string */
+ fs_value_is_blob, /* Value is a binary blob */
+ fs_value_is_filename, /* Value is a filename* + dirfd */
+ fs_value_is_filename_empty, /* Value is a filename* + dirfd + AT_EMPTY_PATH */
+ fs_value_is_file, /* Value is a file* */
+};
+
+/*
+ * Configuration parameter.
+ */
+struct fs_parameter {
+ const char *key; /* Parameter name */
+ enum fs_value_type type:8; /* The type of value here */
+ union {
+ char *string;
+ void *blob;
+ struct filename *name;
+ struct file *file;
+ };
+ size_t size;
+ int dirfd;
+};
+
+/*
* Filesystem context for holding the parameters used in the creation or
* reconfiguration of a superblock.
*
diff --git a/include/linux/fs_parser.h b/include/linux/fs_parser.h
new file mode 100644
index 000000000000..d966f96ffe62
--- /dev/null
+++ b/include/linux/fs_parser.h
@@ -0,0 +1,151 @@
+/* Filesystem parameter description and parser
+ *
+ * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _LINUX_FS_PARSER_H
+#define _LINUX_FS_PARSER_H
+
+#include <linux/fs_context.h>
+
+struct path;
+
+struct constant_table {
+ const char *name;
+ int value;
+};
+
+/*
+ * The type of parameter expected.
+ */
+enum fs_parameter_type {
+ __fs_param_wasnt_defined,
+ fs_param_is_flag,
+ fs_param_is_bool,
+ fs_param_is_u32,
+ fs_param_is_u32_octal,
+ fs_param_is_u32_hex,
+ fs_param_is_s32,
+ fs_param_is_u64,
+ fs_param_is_enum,
+ fs_param_is_string,
+ fs_param_is_blob,
+ fs_param_is_blockdev,
+ fs_param_is_path,
+ fs_param_is_fd,
+ nr__fs_parameter_type,
+};
+
+/*
+ * Specification of the type of value a parameter wants.
+ *
+ * Note that the fsparam_flag(), fsparam_string(), fsparam_u32(), ... macros
+ * should be used to generate elements of this type.
+ */
+struct fs_parameter_spec {
+ const char *name;
+ u8 opt; /* Option number (returned by fs_parse()) */
+ enum fs_parameter_type type:8; /* The desired parameter type */
+ unsigned short flags;
+#define fs_param_v_optional 0x0001 /* The value is optional */
+#define fs_param_neg_with_no 0x0002 /* "noxxx" is negative param */
+#define fs_param_neg_with_empty 0x0004 /* "xxx=" is negative param */
+#define fs_param_deprecated 0x0008 /* The param is deprecated */
+};
+
+struct fs_parameter_enum {
+ u8 opt; /* Option number (as fs_parameter_spec::opt) */
+ char name[14];
+ u8 value;
+};
+
+struct fs_parameter_description {
+ const char name[16]; /* Name for logging purposes */
+ const struct fs_parameter_spec *specs; /* List of param specifications */
+ const struct fs_parameter_enum *enums; /* Enum values */
+};
+
+/*
+ * Result of parse.
+ */
+struct fs_parse_result {
+ bool negated; /* T if param was "noxxx" */
+ bool has_value; /* T if value supplied to param */
+ union {
+ bool boolean; /* For spec_bool */
+ int int_32; /* For spec_s32/spec_enum */
+ unsigned int uint_32; /* For spec_u32{,_octal,_hex}/spec_enum */
+ u64 uint_64; /* For spec_u64 */
+ };
+};
+
+extern int fs_parse(struct fs_context *fc,
+ const struct fs_parameter_description *desc,
+ struct fs_parameter *value,
+ struct fs_parse_result *result);
+extern int fs_lookup_param(struct fs_context *fc,
+ struct fs_parameter *param,
+ bool want_bdev,
+ struct path *_path);
+
+extern int __lookup_constant(const struct constant_table tbl[], size_t tbl_size,
+ const char *name, int not_found);
+#define lookup_constant(t, n, nf) __lookup_constant(t, ARRAY_SIZE(t), (n), (nf))
+
+#ifdef CONFIG_VALIDATE_FS_PARSER
+extern bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size,
+ int low, int high, int special);
+extern bool fs_validate_description(const struct fs_parameter_description *desc);
+#else
+static inline bool validate_constant_table(const struct constant_table *tbl, size_t tbl_size,
+ int low, int high, int special)
+{ return true; }
+static inline bool fs_validate_description(const struct fs_parameter_description *desc)
+{ return true; }
+#endif
+
+/*
+ * Parameter type, name, index and flags element constructors. Use as:
+ *
+ * fsparam_xxxx("foo", Opt_foo)
+ *
+ * If existing helpers are not enough, direct use of __fsparam() would
+ * work, but any such case is probably a sign that new helper is needed.
+ * Helpers will remain stable; low-level implementation may change.
+ */
+#define __fsparam(TYPE, NAME, OPT, FLAGS) \
+ { \
+ .name = NAME, \
+ .opt = OPT, \
+ .type = TYPE, \
+ .flags = FLAGS \
+ }
+
+#define fsparam_flag(NAME, OPT) __fsparam(fs_param_is_flag, NAME, OPT, 0)
+#define fsparam_flag_no(NAME, OPT) \
+ __fsparam(fs_param_is_flag, NAME, OPT, \
+ fs_param_neg_with_no)
+#define fsparam_bool(NAME, OPT) __fsparam(fs_param_is_bool, NAME, OPT, 0)
+#define fsparam_u32(NAME, OPT) __fsparam(fs_param_is_u32, NAME, OPT, 0)
+#define fsparam_u32oct(NAME, OPT) \
+ __fsparam(fs_param_is_u32_octal, NAME, OPT, 0)
+#define fsparam_u32hex(NAME, OPT) \
+ __fsparam(fs_param_is_u32_hex, NAME, OPT, 0)
+#define fsparam_s32(NAME, OPT) __fsparam(fs_param_is_s32, NAME, OPT, 0)
+#define fsparam_u64(NAME, OPT) __fsparam(fs_param_is_u64, NAME, OPT, 0)
+#define fsparam_enum(NAME, OPT) __fsparam(fs_param_is_enum, NAME, OPT, 0)
+#define fsparam_string(NAME, OPT) \
+ __fsparam(fs_param_is_string, NAME, OPT, 0)
+#define fsparam_blob(NAME, OPT) __fsparam(fs_param_is_blob, NAME, OPT, 0)
+#define fsparam_bdev(NAME, OPT) __fsparam(fs_param_is_blockdev, NAME, OPT, 0)
+#define fsparam_path(NAME, OPT) __fsparam(fs_param_is_path, NAME, OPT, 0)
+#define fsparam_fd(NAME, OPT) __fsparam(fs_param_is_fd, NAME, OPT, 0)
+
+
+#endif /* _LINUX_FS_PARSER_H */
^ permalink raw reply related
* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: Tetsuo Handa @ 2019-03-04 13:35 UTC (permalink / raw)
To: James Morris; +Cc: linux-security-module
In-Reply-To: <1551362770-8655-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>
James, please include this patch for 5.1-rc1, for failing to include
this patch will prevent various trees (SELinux/Smack/AppArmor) from
proper testing due to this problem because syzbot is enabling both
TOMOYO and one of SELinux/Smack/AppArmor via lsm= boot parameter.
By including this patch and building kernels with this config option
enabled, syzbot will be able to continue proper testing.
On 2019/02/28 23:06, Tetsuo Handa wrote:
> syzbot is reporting kernel panic triggered by memory allocation fault
> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
> useful, we need to assign a profile other than "disabled" (no-op) mode.
> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
> "learning" mode using a kernel config option. This option must not be
> enabled for kernels built for production system, for this option also
> disables domain/program checks when modifying policy configuration via
> /sys/kernel/security/tomoyo/ interface.
>
> [1] https://syzkaller.appspot.com/bug?extid=29569ed06425fcf67a95
>
> Reported-by: syzbot <syzbot+e1b8084e532b6ee7afab@syzkaller.appspotmail.com>
> Reported-by: syzbot <syzbot+29569ed06425fcf67a95@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> security/tomoyo/Kconfig | 10 ++++++++++
> security/tomoyo/common.c | 13 ++++++++++++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
^ permalink raw reply
* Re: [PATCH] tomoyo: Add a kernel config option for fuzzing testing.
From: Stephen Smalley @ 2019-03-04 14:34 UTC (permalink / raw)
To: Tetsuo Handa, James Morris; +Cc: linux-security-module
In-Reply-To: <19e3dbac-d3ac-53b2-6e98-faf927bc72b3@i-love.sakura.ne.jp>
On 3/4/19 8:35 AM, Tetsuo Handa wrote:
> James, please include this patch for 5.1-rc1, for failing to include
> this patch will prevent various trees (SELinux/Smack/AppArmor) from
> proper testing due to this problem because syzbot is enabling both
> TOMOYO and one of SELinux/Smack/AppArmor via lsm= boot parameter.
>
> By including this patch and building kernels with this config option
> enabled, syzbot will be able to continue proper testing.
Could you clarify the status of upstream TOMOYO? Is its MAINTAINERS
entry still accurate? Is it still actively maintained? Its existing
documentation (in-tree and the tomoyo.osdn.jp site) seem to suggest that
using the pre-LSM version and/or AKARI are preferred to using the
upstream version. Is that still true, and do you envision it changing?
>
> On 2019/02/28 23:06, Tetsuo Handa wrote:
>> syzbot is reporting kernel panic triggered by memory allocation fault
>> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
>> useful, we need to assign a profile other than "disabled" (no-op) mode.
>> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
>> "learning" mode using a kernel config option. This option must not be
>> enabled for kernels built for production system, for this option also
>> disables domain/program checks when modifying policy configuration via
>> /sys/kernel/security/tomoyo/ interface.
>>
>> [1] https://syzkaller.appspot.com/bug?extid=29569ed06425fcf67a95
>>
>> Reported-by: syzbot <syzbot+e1b8084e532b6ee7afab@syzkaller.appspotmail.com>
>> Reported-by: syzbot <syzbot+29569ed06425fcf67a95@syzkaller.appspotmail.com>
>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> ---
>> security/tomoyo/Kconfig | 10 ++++++++++
>> security/tomoyo/common.c | 13 ++++++++++++-
>> 2 files changed, 22 insertions(+), 1 deletion(-)
>>
^ permalink raw reply
* [PATCH] vfs: Move kernel_read_file() to fs/read_write.c
From: David Howells @ 2019-03-04 15:11 UTC (permalink / raw)
To: viro
Cc: Mimi Zohar, dhowells, linux-fsdevel, linux-security-module,
linux-integrity, linux-kernel
Move kernel_read_file() to fs/read_write.c and out of fs/exec.c as it's not
actually used by anything in the execve subsystem.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
fs/exec.c | 106 -------------------------------------------------------
fs/read_write.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+), 106 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index fb72d36f7823..cbb1a9cd25ca 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -61,7 +61,6 @@
#include <linux/pipe_fs_i.h>
#include <linux/oom.h>
#include <linux/compat.h>
-#include <linux/vmalloc.h>
#include <linux/uaccess.h>
#include <asm/mmu_context.h>
@@ -892,111 +891,6 @@ struct file *open_exec(const char *name)
}
EXPORT_SYMBOL(open_exec);
-int kernel_read_file(struct file *file, void **buf, loff_t *size,
- loff_t max_size, enum kernel_read_file_id id)
-{
- loff_t i_size, pos;
- ssize_t bytes = 0;
- int ret;
-
- if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
- return -EINVAL;
-
- ret = deny_write_access(file);
- if (ret)
- return ret;
-
- ret = security_kernel_read_file(file, id);
- if (ret)
- goto out;
-
- i_size = i_size_read(file_inode(file));
- if (i_size <= 0) {
- ret = -EINVAL;
- goto out;
- }
- if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
- ret = -EFBIG;
- goto out;
- }
-
- if (id != READING_FIRMWARE_PREALLOC_BUFFER)
- *buf = vmalloc(i_size);
- if (!*buf) {
- ret = -ENOMEM;
- goto out;
- }
-
- pos = 0;
- while (pos < i_size) {
- bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
- if (bytes < 0) {
- ret = bytes;
- goto out;
- }
-
- if (bytes == 0)
- break;
- }
-
- if (pos != i_size) {
- ret = -EIO;
- goto out_free;
- }
-
- ret = security_kernel_post_read_file(file, *buf, i_size, id);
- if (!ret)
- *size = pos;
-
-out_free:
- if (ret < 0) {
- if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
- vfree(*buf);
- *buf = NULL;
- }
- }
-
-out:
- allow_write_access(file);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file);
-
-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;
- int ret;
-
- if (!path || !*path)
- return -EINVAL;
-
- file = filp_open(path, O_RDONLY, 0);
- if (IS_ERR(file))
- return PTR_ERR(file);
-
- ret = kernel_read_file(file, buf, size, max_size, id);
- fput(file);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
-
-int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
- enum kernel_read_file_id id)
-{
- struct fd f = fdget(fd);
- int ret = -EBADF;
-
- if (!f.file)
- goto out;
-
- ret = kernel_read_file(f.file, buf, size, max_size, id);
-out:
- fdput(f);
- return ret;
-}
-EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
-
ssize_t read_code(struct file *file, unsigned long addr, loff_t pos, size_t len)
{
ssize_t res = vfs_read(file, (void __user *)addr, len, &pos);
diff --git a/fs/read_write.c b/fs/read_write.c
index ff3c5e6f87cf..555dcaec00ac 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -20,6 +20,7 @@
#include <linux/compat.h>
#include <linux/mount.h>
#include <linux/fs.h>
+#include <linux/vmalloc.h>
#include "internal.h"
#include <linux/uaccess.h>
@@ -1362,6 +1363,111 @@ COMPAT_SYSCALL_DEFINE6(pwritev2, compat_ulong_t, fd,
#endif
+int kernel_read_file(struct file *file, void **buf, loff_t *size,
+ loff_t max_size, enum kernel_read_file_id id)
+{
+ loff_t i_size, pos;
+ ssize_t bytes = 0;
+ int ret;
+
+ if (!S_ISREG(file_inode(file)->i_mode) || max_size < 0)
+ return -EINVAL;
+
+ ret = deny_write_access(file);
+ if (ret)
+ return ret;
+
+ ret = security_kernel_read_file(file, id);
+ if (ret)
+ goto out;
+
+ i_size = i_size_read(file_inode(file));
+ if (i_size <= 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+ if (i_size > SIZE_MAX || (max_size > 0 && i_size > max_size)) {
+ ret = -EFBIG;
+ goto out;
+ }
+
+ if (id != READING_FIRMWARE_PREALLOC_BUFFER)
+ *buf = vmalloc(i_size);
+ if (!*buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ pos = 0;
+ while (pos < i_size) {
+ bytes = kernel_read(file, *buf + pos, i_size - pos, &pos);
+ if (bytes < 0) {
+ ret = bytes;
+ goto out;
+ }
+
+ if (bytes == 0)
+ break;
+ }
+
+ if (pos != i_size) {
+ ret = -EIO;
+ goto out_free;
+ }
+
+ ret = security_kernel_post_read_file(file, *buf, i_size, id);
+ if (!ret)
+ *size = pos;
+
+out_free:
+ if (ret < 0) {
+ if (id != READING_FIRMWARE_PREALLOC_BUFFER) {
+ vfree(*buf);
+ *buf = NULL;
+ }
+ }
+
+out:
+ allow_write_access(file);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file);
+
+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;
+ int ret;
+
+ if (!path || !*path)
+ return -EINVAL;
+
+ file = filp_open(path, O_RDONLY, 0);
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ ret = kernel_read_file(file, buf, size, max_size, id);
+ fput(file);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
+
+int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
+ enum kernel_read_file_id id)
+{
+ struct fd f = fdget(fd);
+ int ret = -EBADF;
+
+ if (!f.file)
+ goto out;
+
+ ret = kernel_read_file(f.file, buf, size, max_size, id);
+out:
+ fdput(f);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(kernel_read_file_from_fd);
+
static ssize_t do_sendfile(int out_fd, int in_fd, loff_t *ppos,
size_t count, loff_t max)
{
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox