Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Alexei Starovoitov @ 2019-08-13 21:58 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Song Liu, Kees Cook, Networking, bpf, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team, Lorenz Bauer, Jann Horn, Greg KH,
	Linux API, LSM List
In-Reply-To: <CALCETrXEHL3+NAY6P6vUj7Pvd9ZpZsYC6VCLXOaNxb90a_POGw@mail.gmail.com>

On Tue, Aug 06, 2019 at 10:24:25PM -0700, Andy Lutomirski wrote:
> >
> > Inside containers and inside nested containers we need to start processes
> > that will use bpf. All of the processes are trusted.
> 
> Trusted by whom?  In a non-nested container, the container manager
> *might* be trusted by the outside world.  In a *nested* container,
> unless the inner container management is controlled from outside the
> outer container, it's not trusted.  I don't know much about how
> Facebook's containers work, but the LXC/LXD/Podman world is moving
> very strongly toward user namespaces and maximally-untrusted
> containers, and I think bpf() should work in that context.

agree that containers (namespaces) reduce amount of trust necessary
for apps to run, but the end goal is not security though.
Linux has become a single user system.
If user can ssh into the host they can become root.
If arbitrary code can run on the host it will be break out of any sandbox.
Containers are not providing the level of security that is enough
to run arbitrary code. VMs can do it better, but cpu bugs don't make it easy.
Containers are used to make production systems safer.
Some people call it more 'secure', but it's clearly not secure for
arbitrary code and that is what kernel.unprivileged_bpf_disabled allows.
When we say 'unprivileged bpf' we really mean arbitrary malicious bpf program.
It's been a constant source of pain. The constant blinding, randomization,
verifier speculative analysis, all spectre v1, v2, v4 mitigations
are simply not worth it. It's a lot of complex kernel code without users.
There is not a single use case to allow arbitrary malicious bpf
program to be loaded and executed.
As soon as we have /dev/bpf to allow all of bpf to be used without root
we will set sysctl kernel.unprivileged_bpf_disabled=1
Hence I prefer this /dev/bpf mechanism to be as simple a possible.
The applications that will use it are going to be just as trusted as systemd.

> > To solve your concern of bypassing all capable checks...
> > How about we do /dev/bpf/full_verifier first?
> > It will replace capable() checks in the verifier only.
> 
> I'm not convinced that "in the verifier" is the right distinction.
> Telling administrators that some setting lets certain users bypass
> bpf() verifier checks doesn't have a clear enough meaning.  

linux is a single user system. there are no administrators any more.
No doubt, folks will disagree, but that game is over.
At least on bpf side it's done.

> I propose,
> instead, that the current capable() checks be divided into three
> categories:

I don't see a use case for these categories.
All bpf programs extend the kernel in some way.
The kernel vs user is one category.
Conceptually CAP_BPF is enough. It would be similar to CAP_NET_ADMIN.
When application has CAP_NET_ADMIN it covers all of networking knobs.
There is no use case that would warrant fine grain CAP_ROUTE_ADMIN,
CAP_ETHTOOL_ADMIN, CAP_ETH0_ADMIN, etc.
Similarly CAP_BPF as the only knob is enough.
The only disadvantage of CAP_BPF is that it's not possible to
pass it from one systemd-like daemon to another systemd-like daemon.
Hence /dev/bpf idea and passing file descriptor.

> This type of thing actually fits quite nicely into an idea I've been
> thinking about for a while called "implicit rights". In very brief
> summary, there would be objects called /dev/rights/xyz, where xyz is
> the same of a "right".  If there is a readable object of the right
> type at the literal path "/dev/rights/xyz", then you have right xyz.
> There's a bit more flexibility on top of this.  BPF could use
> /dev/rights/bpf/maptypes/lpm and
> /dev/rights/bpf/verifier/bounded_loops, for example.  Other non-BPF
> use cases include a biggie:
> /dev/rights/namespace/create_unprivileged_userns.
> /dev/rights/bind_port/80 would be nice, too.

The concept of "implicit rights" is very nice and I'm sure it will
be a good fit somewhere, but I don't see why use it in bpf space.
There is no use case for fine grain partition of bpf features.


^ permalink raw reply

* Re: [Non-DoD Source] Re: [RFC PATCH v2] security, capability: pass object information to security_capable
From: Richard Guy Briggs @ 2019-08-13 21:27 UTC (permalink / raw)
  To: Aaron Goidel
  Cc: Paul Moore, mortonm, john.johansen, selinux, James Morris, luto,
	linux-security-module, linux-audit, Nicholas Franck,
	Stephen Smalley, Serge Hallyn
In-Reply-To: <b79617aa-2b40-40bf-9f35-0f5be8e34d3f@tycho.nsa.gov>

On 2019-08-13 11:01, Aaron Goidel wrote:
> On 8/8/19 12:30 PM, Paul Moore wrote:
> > On Thu, Aug 1, 2019 at 10:43 AM Aaron Goidel <acgoide@tycho.nsa.gov> wrote:
> > > From: Nicholas Franck <nhfran2@tycho.nsa.gov>
> > > 
> > > At present security_capable does not pass any object information
> > > and therefore can neither audit the particular object nor take it
> > > into account. Augment the security_capable interface to support
> > > passing supplementary data. Use this facility initially to convey
> > > the inode for capability checks relevant to inodes. This only
> > > addresses capable_wrt_inode_uidgid calls; other capability checks
> > > relevant to inodes will be addressed in subsequent changes. In the
> > > future, this will be further extended to pass object information for
> > > other capability checks such as the target task for CAP_KILL.
> > > 
> > > In SELinux this new information is leveraged here to include the inode
> > > in the audit message. In the future, it could also be used to perform
> > > a per inode capability checks.
> > > 
> > > It would be possible to fold the existing opts argument into this new
> > > supplementary data structure. This was omitted from this change to
> > > minimize changes.
> > > 
> > > Signed-off-by: Nicholas Franck <nhfran2@tycho.nsa.gov>
> > > Signed-off-by: Aaron Goidel <acgoide@tycho.nsa.gov>
> > > ---
> > > v2:
> > > - Changed order of audit prints so optional information comes second
> > > ---
> > >   include/linux/capability.h             |  7 ++++++
> > >   include/linux/lsm_audit.h              |  5 +++-
> > >   include/linux/lsm_hooks.h              |  3 ++-
> > >   include/linux/security.h               | 23 +++++++++++++-----
> > >   kernel/capability.c                    | 33 ++++++++++++++++++--------
> > >   kernel/seccomp.c                       |  2 +-
> > >   security/apparmor/capability.c         |  8 ++++---
> > >   security/apparmor/include/capability.h |  4 +++-
> > >   security/apparmor/ipc.c                |  2 +-
> > >   security/apparmor/lsm.c                |  5 ++--
> > >   security/apparmor/resource.c           |  2 +-
> > >   security/commoncap.c                   | 11 +++++----
> > >   security/lsm_audit.c                   | 21 ++++++++++++++--
> > >   security/safesetid/lsm.c               |  3 ++-
> > >   security/security.c                    |  5 ++--
> > >   security/selinux/hooks.c               | 20 +++++++++-------
> > >   security/smack/smack_access.c          |  2 +-
> > >   17 files changed, 110 insertions(+), 46 deletions(-)
> > 
> > You should CC the linux-audit list, I've added them on this mail.
> > 
> > I had hoped to see some thought put into the idea of dynamically
> > emitting the proper audit records as I mentioned in the previous patch
> > set, but regardless there are some comments on this code as written
> > ...
> > 
> > > diff --git a/security/lsm_audit.c b/security/lsm_audit.c
> > > index 33028c098ef3..18cc7c956b69 100644
> > > --- a/security/lsm_audit.c
> > > +++ b/security/lsm_audit.c
> > > @@ -229,9 +229,26 @@ static void dump_common_audit_data(struct audit_buffer *ab,
> > >          case LSM_AUDIT_DATA_IPC:
> > >                  audit_log_format(ab, " key=%d ", a->u.ipc_id);
> > >                  break;
> > > -       case LSM_AUDIT_DATA_CAP:
> > > -               audit_log_format(ab, " capability=%d ", a->u.cap);
> > > +       case LSM_AUDIT_DATA_CAP: {
> > > +               const struct inode *inode;
> > > +
> > > +               audit_log_format(ab, " capability=%d ", a->u.cap_struct.cap);
> > > +               if (a->u.cap_struct.cad) {
> > > +                       switch (a->u.cap_struct.cad->type) {
> > > +                       case CAP_AUX_DATA_INODE: {
> > > +                               inode = a->u.cap_struct.cad->u.inode;
> > > +
> > > +                               audit_log_format(ab, " dev=");
> > > +                               audit_log_untrustedstring(ab,
> > > +                                       inode->i_sb->s_id);
> > > +                               audit_log_format(ab, " ino=%lu",
> > > +                                       inode->i_ino);
> > > +                               break;
> > > +                       }
> > 
> > Since you are declaring "inode" further up, there doesn't appear to be
> > any need for the CAP_AUX_DATA_INODE braces, please remove them.
> > 
> > The general recommended practice when it comes to "sometimes" fields
> > in an audit record, is to always record them in the record, but use a
> > value of "?" when there is nothing relevant to record.  For example,
> > when *not* recording inode information you would do something like the
> > following:
> > 
> >    audit_log_format(ab, " dev=? ino=?");
> > 
> The issue this brings up is what happens when this is expanded to more
> cases? Assuming there will be a case here for logging audit data for task
> based capabilities (CAP_AUX_DATA_TASK), what do we want to have happen if we
> are recording *neither* inode information nor task information (say a PID)?
> If we log something in the inode case, we presumably don't want to call
> audit_log_format(ab, " dev=?, pid=?") as well. (And vice versa for when we
> log a pid and no inode).

Yup.  This record is already a mess due to that.

The general solution is to either use a new record type for each
variant, or to use an auxiliary record for each additional piece of
information.  The long term solution that has been suggested it to move
away from a text message format.

> > > +                       }
> > > +               }
> > >                  break;
> > > +       }
> > >          case LSM_AUDIT_DATA_PATH: {
> > >                  struct inode *inode;
> 
> Aaron

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply

* [PATCH 6/6] Document locked_down LSM hook
From: Matthew Garrett @ 2019-08-13 19:21 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, Matthew Garrett, kbuild test robot,
	Matthew Garrett
In-Reply-To: <20190813192126.122370-1-matthewgarrett@google.com>

The kbuild test robot pointed out that this wasn't documented.

Reported-by: kbuild test robot <lkp@intel.com>
Fixes: c360be6c ("security: Add a "locked down" LSM hook")
Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 include/linux/lsm_hooks.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 5ec2912c8661..2f4ba9062fb8 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1446,6 +1446,11 @@
  * @bpf_prog_free_security:
  *	Clean up the security information stored inside bpf prog.
  *
+ * @locked_down
+ *     Determine whether a kernel feature that potentially enables arbitrary
+ *     code execution in kernel space should be permitted.
+ *
+ *     @what: kernel feature being accessed
  */
 union security_list_options {
 	int (*binder_set_context_mgr)(struct task_struct *mgr);
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related

* [PATCH 5/6] kexec: s/KEXEC_VERIFY_SIG/KEXEC_SIG/ for consistency
From: Matthew Garrett @ 2019-08-13 19:21 UTC (permalink / raw)
  To: jmorris; +Cc: linux-security-module, Matthew Garrett, Matthew Garrett
In-Reply-To: <20190813192126.122370-1-matthewgarrett@google.com>

47b88836 ("kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and
KEXEC_SIG_FORCE") removed CONFIG_KEXEC_VERIFY_SIG, but it's still present
in other architectures and we end up with build breakage as a result.
Make this consistent across architectures.

Fixes: 47b88836 ("kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE")
Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 arch/arm64/Kconfig                      | 6 +++---
 arch/s390/Kconfig                       | 2 +-
 arch/s390/configs/debug_defconfig       | 2 +-
 arch/s390/configs/defconfig             | 2 +-
 arch/s390/configs/performance_defconfig | 2 +-
 arch/s390/kernel/kexec_elf.c            | 4 ++--
 arch/s390/kernel/kexec_image.c          | 4 ++--
 arch/s390/kernel/machine_kexec_file.c   | 4 ++--
 arch/x86/kernel/ima_arch.c              | 4 ++--
 security/integrity/ima/Kconfig          | 2 +-
 security/integrity/ima/ima_main.c       | 2 +-
 11 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 697ea0510729..f940500a941b 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -961,7 +961,7 @@ config KEXEC_FILE
 	  for kernel and initramfs as opposed to list of segments as
 	  accepted by previous system call.
 
-config KEXEC_VERIFY_SIG
+config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
 	depends on KEXEC_FILE
 	help
@@ -976,13 +976,13 @@ config KEXEC_VERIFY_SIG
 config KEXEC_IMAGE_VERIFY_SIG
 	bool "Enable Image signature verification support"
 	default y
-	depends on KEXEC_VERIFY_SIG
+	depends on KEXEC_SIG
 	depends on EFI && SIGNED_PE_FILE_VERIFICATION
 	help
 	  Enable Image signature verification support.
 
 comment "Support for PE file signature verification disabled"
-	depends on KEXEC_VERIFY_SIG
+	depends on KEXEC_SIG
 	depends on !EFI || !SIGNED_PE_FILE_VERIFICATION
 
 config CRASH_DUMP
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 109243fdb6ec..c4a423f30d49 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -555,7 +555,7 @@ config ARCH_HAS_KEXEC_PURGATORY
 	def_bool y
 	depends on KEXEC_FILE
 
-config KEXEC_VERIFY_SIG
+config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
 	depends on KEXEC_FILE && SYSTEM_DATA_VERIFICATION
 	help
diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
index b0920b35f87b..525e0a6addb9 100644
--- a/arch/s390/configs/debug_defconfig
+++ b/arch/s390/configs/debug_defconfig
@@ -64,7 +64,7 @@ CONFIG_NUMA=y
 CONFIG_PREEMPT=y
 CONFIG_HZ_100=y
 CONFIG_KEXEC_FILE=y
-CONFIG_KEXEC_VERIFY_SIG=y
+CONFIG_KEXEC_SIG=y
 CONFIG_EXPOLINE=y
 CONFIG_EXPOLINE_AUTO=y
 CONFIG_MEMORY_HOTPLUG=y
diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
index c59b922cb6c5..4c37279acdb4 100644
--- a/arch/s390/configs/defconfig
+++ b/arch/s390/configs/defconfig
@@ -39,7 +39,7 @@ CONFIG_NR_CPUS=256
 CONFIG_NUMA=y
 CONFIG_HZ_100=y
 CONFIG_KEXEC_FILE=y
-CONFIG_KEXEC_VERIFY_SIG=y
+CONFIG_KEXEC_SIG=y
 CONFIG_CRASH_DUMP=y
 CONFIG_HIBERNATION=y
 CONFIG_PM_DEBUG=y
diff --git a/arch/s390/configs/performance_defconfig b/arch/s390/configs/performance_defconfig
index 09aa5cb14873..158ad0f0d433 100644
--- a/arch/s390/configs/performance_defconfig
+++ b/arch/s390/configs/performance_defconfig
@@ -65,7 +65,7 @@ CONFIG_NR_CPUS=512
 CONFIG_NUMA=y
 CONFIG_HZ_100=y
 CONFIG_KEXEC_FILE=y
-CONFIG_KEXEC_VERIFY_SIG=y
+CONFIG_KEXEC_SIG=y
 CONFIG_EXPOLINE=y
 CONFIG_EXPOLINE_AUTO=y
 CONFIG_MEMORY_HOTPLUG=y
diff --git a/arch/s390/kernel/kexec_elf.c b/arch/s390/kernel/kexec_elf.c
index 6d0635ceddd0..9b4f37a4edf1 100644
--- a/arch/s390/kernel/kexec_elf.c
+++ b/arch/s390/kernel/kexec_elf.c
@@ -130,7 +130,7 @@ static int s390_elf_probe(const char *buf, unsigned long len)
 const struct kexec_file_ops s390_kexec_elf_ops = {
 	.probe = s390_elf_probe,
 	.load = s390_elf_load,
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC__SIG
 	.verify_sig = s390_verify_sig,
-#endif /* CONFIG_KEXEC_VERIFY_SIG */
+#endif /* CONFIG_KEXEC_SIG */
 };
diff --git a/arch/s390/kernel/kexec_image.c b/arch/s390/kernel/kexec_image.c
index 58318bf89fd9..af23eff5774d 100644
--- a/arch/s390/kernel/kexec_image.c
+++ b/arch/s390/kernel/kexec_image.c
@@ -59,7 +59,7 @@ static int s390_image_probe(const char *buf, unsigned long len)
 const struct kexec_file_ops s390_kexec_image_ops = {
 	.probe = s390_image_probe,
 	.load = s390_image_load,
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
 	.verify_sig = s390_verify_sig,
-#endif /* CONFIG_KEXEC_VERIFY_SIG */
+#endif /* CONFIG_KEXEC_SIG */
 };
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index fbdd3ea73667..c0f33ba49a9a 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -22,7 +22,7 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 	NULL,
 };
 
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
 /*
  * Module signature information block.
  *
@@ -90,7 +90,7 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 				      VERIFYING_MODULE_SIGNATURE,
 				      NULL, NULL);
 }
-#endif /* CONFIG_KEXEC_VERIFY_SIG */
+#endif /* CONFIG_KEXEC_SIG */
 
 static int kexec_file_update_purgatory(struct kimage *image,
 				       struct s390_load_data *data)
diff --git a/arch/x86/kernel/ima_arch.c b/arch/x86/kernel/ima_arch.c
index 64b973f0e985..b98890894731 100644
--- a/arch/x86/kernel/ima_arch.c
+++ b/arch/x86/kernel/ima_arch.c
@@ -66,9 +66,9 @@ bool arch_ima_get_secureboot(void)
 
 /* secureboot arch rules */
 static const char * const sb_arch_rules[] = {
-#if !IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG)
+#if !IS_ENABLED(CONFIG_KEXEC_SIG)
 	"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig",
-#endif /* CONFIG_KEXEC_VERIFY_SIG */
+#endif /* CONFIG_KEXEC_SIG */
 	"measure func=KEXEC_KERNEL_CHECK",
 #if !IS_ENABLED(CONFIG_MODULE_SIG)
 	"appraise func=MODULE_CHECK appraise_type=imasig",
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 2692c7358c2c..32cd25fa44a5 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -160,7 +160,7 @@ config IMA_APPRAISE
 
 config IMA_ARCH_POLICY
         bool "Enable loading an IMA architecture specific policy"
-        depends on KEXEC_VERIFY_SIG || IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
+        depends on KEXEC_SIG || IMA_APPRAISE && INTEGRITY_ASYMMETRIC_KEYS
         default n
         help
           This option enables loading an IMA architecture specific policy
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index f302cbc387f8..1747bc7bcb60 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -541,7 +541,7 @@ int ima_load_data(enum kernel_load_data_id id)
 
 	switch (id) {
 	case LOADING_KEXEC_IMAGE:
-		if (IS_ENABLED(CONFIG_KEXEC_VERIFY_SIG)
+		if (IS_ENABLED(CONFIG_KEXEC_SIG)
 		    && arch_ima_get_secureboot()) {
 			pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file_load syscall.\n");
 			return -EACCES;
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related

* [PATCH 4/6] security: fix ptr_ret.cocci warnings
From: Matthew Garrett @ 2019-08-13 19:21 UTC (permalink / raw)
  To: jmorris; +Cc: linux-security-module, kbuild test robot, Matthew Garrett
In-Reply-To: <20190813192126.122370-1-matthewgarrett@google.com>

From: kbuild test robot <lkp@intel.com>

security/lockdown/lockdown.c:157:1-3: WARNING: PTR_ERR_OR_ZERO can be used

 Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

Fixes: 80d14015a8e3 ("security: Add a static lockdown policy LSM")
Signed-off-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
 security/lockdown/lockdown.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index f6c74cf6a798..0068cec77c05 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -176,10 +176,7 @@ static int __init lockdown_secfs_init(void)
 
 	dentry = securityfs_create_file("lockdown", 0600, NULL, NULL,
 					&lockdown_ops);
-	if (IS_ERR(dentry))
-		return PTR_ERR(dentry);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(dentry);
 }
 
 core_initcall(lockdown_secfs_init);
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related

* [PATCH 3/6] Avoid build warning when !CONFIG_KEXEC_SIG
From: Matthew Garrett @ 2019-08-13 19:21 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security-module, Matthew Garrett, Matthew Garrett,
	Jiri Bohac, Dave Young, kexec
In-Reply-To: <20190813192126.122370-1-matthewgarrett@google.com>

Refactor the signature validation and lockdown integration a little in
order to avoid an unused variable.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: Dave Young <dyoung@redhat.com>
Cc: kexec@lists.infradead.org
---
 kernel/kexec_file.c | 72 ++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 27 deletions(-)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index be0c13076056..e878587715b9 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -177,36 +177,13 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 	image->image_loader_data = NULL;
 }
 
-/*
- * In file mode list of segments is prepared by kernel. Copy relevant
- * data from user space, do error checking, prepare segment list
- */
+#ifdef CONFIG_KEXEC_SIG
 static int
-kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
-			     const char __user *cmdline_ptr,
-			     unsigned long cmdline_len, unsigned flags)
+kimage_validate_signature(struct kimage *image)
 {
 	const char *reason;
 	int ret;
-	void *ldata;
-	loff_t size;
-
-	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
-				       &size, INT_MAX, READING_KEXEC_IMAGE);
-	if (ret)
-		return ret;
-	image->kernel_buf_len = size;
-
-	/* IMA needs to pass the measurement list to the next kernel. */
-	ima_add_kexec_buffer(image);
 
-	/* Call arch image probe handlers */
-	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
-					    image->kernel_buf_len);
-	if (ret)
-		goto out;
-
-#ifdef CONFIG_KEXEC_SIG
 	ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
 					   image->kernel_buf_len);
 	switch (ret) {
@@ -228,7 +205,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 	decide:
 		if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
 			pr_notice("%s rejected\n", reason);
-			goto out;
+			break;
 		}
 
 		ret = 0;
@@ -251,9 +228,44 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 		 */
 	default:
 		pr_notice("kernel signature verification failed (%d).\n", ret);
-		goto out;
+		break;
 	}
+
+	return ret;
+}
+#endif
+
+/*
+ * In file mode list of segments is prepared by kernel. Copy relevant
+ * data from user space, do error checking, prepare segment list
+ */
+static int
+kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
+			     const char __user *cmdline_ptr,
+			     unsigned long cmdline_len, unsigned flags)
+{
+	int ret;
+	void *ldata;
+	loff_t size;
+
+	ret = kernel_read_file_from_fd(kernel_fd, &image->kernel_buf,
+				       &size, INT_MAX, READING_KEXEC_IMAGE);
+	if (ret)
+		return ret;
+	image->kernel_buf_len = size;
+
+	/* Call arch image probe handlers */
+	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
+					    image->kernel_buf_len);
+	if (ret)
+		goto out;
+
+#ifdef CONFIG_KEXEC_SIG
+	ret = kimage_validate_signature(image);
+	if (ret)
+		goto out;
 #endif
+
 	/* It is possible that there no initramfs is being loaded */
 	if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
 		ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
@@ -279,8 +291,14 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 			ret = -EINVAL;
 			goto out;
 		}
+
+		ima_kexec_cmdline(image->cmdline_buf,
+				  image->cmdline_buf_len - 1);
 	}
 
+	/* IMA needs to pass the measurement list to the next kernel. */
+	ima_add_kexec_buffer(image);
+
 	/* Call arch image load handlers */
 	ldata = arch_kexec_kernel_image_load(image);
 
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related

* [PATCH 2/6] early_security_init() needs a stub got !CONFIG_SECURITY
From: Matthew Garrett @ 2019-08-13 19:21 UTC (permalink / raw)
  To: jmorris; +Cc: linux-security-module, Stephen Rothwell
In-Reply-To: <20190813192126.122370-1-matthewgarrett@google.com>

From: Stephen Rothwell <sfr@canb.auug.org.au>

An arm multi_v7_defconfig fails like this:

init/main.c: In function 'start_kernel':
init/main.c:596:2: error: implicit declaration of function 'early_security_init'; did you mean 'security_init'? [-Werror=implicit-function-declaration]
  early_security_init();
  ^~~~~~~~~~~~~~~~~~~
  security_init

Fixes: 45d29f9e9b8b ("security: Support early LSMs")
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 include/linux/security.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/security.h b/include/linux/security.h
index 5748ccc2a42e..429f9f03372b 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -473,6 +473,11 @@ static inline int security_init(void)
 	return 0;
 }
 
+static inline int early_security_init(void)
+{
+	return 0;
+}
+
 static inline int security_binder_set_context_mgr(struct task_struct *mgr)
 {
 	return 0;
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related

* [PATCH 1/6] tracefs: Fix potential null dereference in default_file_open()
From: Matthew Garrett @ 2019-08-13 19:21 UTC (permalink / raw)
  To: jmorris; +Cc: linux-security-module, Ben Hutchings
In-Reply-To: <20190813192126.122370-1-matthewgarrett@google.com>

From: Ben Hutchings <ben@decadent.org.uk>

The "open" operation in struct file_operations is optional, and
ftrace_event_id_fops does not set it.  In default_file_open(), after
all other checks have passed, return 0 if the underlying struct
file_operations does not implement open.

Fixes: 757ff7244358 ("tracefs: Restrict tracefs when the kernel is …")
References: https://bugs.debian.org/934304
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 fs/tracefs/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 34da48036e08..761af8ce4015 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -42,6 +42,8 @@ static int default_open_file(struct inode *inode, struct file *filp)
 		return ret;
 
 	real_fops = dentry->d_fsdata;
+	if (!real_fops->open)
+		return 0;
 	return real_fops->open(inode, filp);
 }
 
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply related

* [PATCH 0/6] lockdown fixups
From: Matthew Garrett @ 2019-08-13 19:21 UTC (permalink / raw)
  To: jmorris; +Cc: linux-security-module

Fixups for the lockdown patchset. Potentially makes more sense to merge
these into the relevant original patches rather than keeping them
separate, but sending them for review.



^ permalink raw reply

* Re: [Non-DoD Source] Re: [RFC PATCH v2] security, capability: pass object information to security_capable
From: Aaron Goidel @ 2019-08-13 15:01 UTC (permalink / raw)
  To: Paul Moore
  Cc: selinux, linux-security-module, luto, James Morris,
	Stephen Smalley, keescook, Serge Hallyn, john.johansen, casey,
	mortonm, rgb, Nicholas Franck, linux-audit
In-Reply-To: <CAHC9VhTSWiz45vh+M+sgu+ePwgFPZ4Mr8GmRZQjsGWQSzkjbLg@mail.gmail.com>

On 8/8/19 12:30 PM, Paul Moore wrote:
> On Thu, Aug 1, 2019 at 10:43 AM Aaron Goidel <acgoide@tycho.nsa.gov> wrote:
>> From: Nicholas Franck <nhfran2@tycho.nsa.gov>
>>
>> At present security_capable does not pass any object information
>> and therefore can neither audit the particular object nor take it
>> into account. Augment the security_capable interface to support
>> passing supplementary data. Use this facility initially to convey
>> the inode for capability checks relevant to inodes. This only
>> addresses capable_wrt_inode_uidgid calls; other capability checks
>> relevant to inodes will be addressed in subsequent changes. In the
>> future, this will be further extended to pass object information for
>> other capability checks such as the target task for CAP_KILL.
>>
>> In SELinux this new information is leveraged here to include the inode
>> in the audit message. In the future, it could also be used to perform
>> a per inode capability checks.
>>
>> It would be possible to fold the existing opts argument into this new
>> supplementary data structure. This was omitted from this change to
>> minimize changes.
>>
>> Signed-off-by: Nicholas Franck <nhfran2@tycho.nsa.gov>
>> Signed-off-by: Aaron Goidel <acgoide@tycho.nsa.gov>
>> ---
>> v2:
>> - Changed order of audit prints so optional information comes second
>> ---
>>   include/linux/capability.h             |  7 ++++++
>>   include/linux/lsm_audit.h              |  5 +++-
>>   include/linux/lsm_hooks.h              |  3 ++-
>>   include/linux/security.h               | 23 +++++++++++++-----
>>   kernel/capability.c                    | 33 ++++++++++++++++++--------
>>   kernel/seccomp.c                       |  2 +-
>>   security/apparmor/capability.c         |  8 ++++---
>>   security/apparmor/include/capability.h |  4 +++-
>>   security/apparmor/ipc.c                |  2 +-
>>   security/apparmor/lsm.c                |  5 ++--
>>   security/apparmor/resource.c           |  2 +-
>>   security/commoncap.c                   | 11 +++++----
>>   security/lsm_audit.c                   | 21 ++++++++++++++--
>>   security/safesetid/lsm.c               |  3 ++-
>>   security/security.c                    |  5 ++--
>>   security/selinux/hooks.c               | 20 +++++++++-------
>>   security/smack/smack_access.c          |  2 +-
>>   17 files changed, 110 insertions(+), 46 deletions(-)
> 
> You should CC the linux-audit list, I've added them on this mail.
> 
> I had hoped to see some thought put into the idea of dynamically
> emitting the proper audit records as I mentioned in the previous patch
> set, but regardless there are some comments on this code as written
> ...
> 
>> diff --git a/security/lsm_audit.c b/security/lsm_audit.c
>> index 33028c098ef3..18cc7c956b69 100644
>> --- a/security/lsm_audit.c
>> +++ b/security/lsm_audit.c
>> @@ -229,9 +229,26 @@ static void dump_common_audit_data(struct audit_buffer *ab,
>>          case LSM_AUDIT_DATA_IPC:
>>                  audit_log_format(ab, " key=%d ", a->u.ipc_id);
>>                  break;
>> -       case LSM_AUDIT_DATA_CAP:
>> -               audit_log_format(ab, " capability=%d ", a->u.cap);
>> +       case LSM_AUDIT_DATA_CAP: {
>> +               const struct inode *inode;
>> +
>> +               audit_log_format(ab, " capability=%d ", a->u.cap_struct.cap);
>> +               if (a->u.cap_struct.cad) {
>> +                       switch (a->u.cap_struct.cad->type) {
>> +                       case CAP_AUX_DATA_INODE: {
>> +                               inode = a->u.cap_struct.cad->u.inode;
>> +
>> +                               audit_log_format(ab, " dev=");
>> +                               audit_log_untrustedstring(ab,
>> +                                       inode->i_sb->s_id);
>> +                               audit_log_format(ab, " ino=%lu",
>> +                                       inode->i_ino);
>> +                               break;
>> +                       }
> 
> Since you are declaring "inode" further up, there doesn't appear to be
> any need for the CAP_AUX_DATA_INODE braces, please remove them.
> 
> The general recommended practice when it comes to "sometimes" fields
> in an audit record, is to always record them in the record, but use a
> value of "?" when there is nothing relevant to record.  For example,
> when *not* recording inode information you would do something like the
> following:
> 
>    audit_log_format(ab, " dev=? ino=?");
> 
The issue this brings up is what happens when this is expanded to more 
cases? Assuming there will be a case here for logging audit data for 
task based capabilities (CAP_AUX_DATA_TASK), what do we want to have 
happen if we are recording *neither* inode information nor task 
information (say a PID)? If we log something in the inode case, we 
presumably don't want to call audit_log_format(ab, " dev=?, pid=?") as 
well. (And vice versa for when we log a pid and no inode).
>> +                       }
>> +               }
>>                  break;
>> +       }
>>          case LSM_AUDIT_DATA_PATH: {
>>                  struct inode *inode;
>>
> 

-- 
Aaron

^ permalink raw reply

* Re: [PATCH] Add flags option to get xattr method paired to __vfs_getxattr
From: Mark Salyzyn @ 2019-08-13 14:44 UTC (permalink / raw)
  To: Sasha Levin, linux-kernel
  Cc: kernel-team, Stephen Smalley, linux-security-module, stable
In-Reply-To: <20190813124817.0B9EF2085A@mail.kernel.org>

On 8/13/19 5:48 AM, Sasha Levin wrote:
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v5.2.8, v4.19.66, v4.14.138, v4.9.189, v4.4.189.
>
> v5.2.8: Build failed! Errors:
>      fs/afs/xattr.c:156:12: error: initialization of ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t,  int)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int,  int)’} from incompatible pointer type ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int)’} [-Werror=incompatible-pointer-types]
>      fs/afs/xattr.c:327:9: error: initialization of ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t,  int)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int,  int)’} from incompatible pointer type ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int)’} [-Werror=incompatible-pointer-types]
>      drivers/staging/erofs/xattr.c:492:9: error: initialization of ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t,  int)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int,  int)’} from incompatible pointer type ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int)’} [-Werror=incompatible-pointer-types]
>      drivers/staging/erofs/xattr.c:499:9: error: initialization of ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t,  int)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int,  int)’} from incompatible pointer type ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int)’} [-Werror=incompatible-pointer-types]
>      drivers/staging/erofs/xattr.c:506:9: error: initialization of ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t,  int)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int,  int)’} from incompatible pointer type ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, size_t)’ {aka ‘int (*)(const struct xattr_handler *, struct dentry *, struct inode *, const char *, void *, long unsigned int)’} [-Werror=incompatible-pointer-types]
>
> v4.19.66: Failed to apply! Possible dependencies:
>      05895219627c ("kernfs: clean up struct kernfs_iattrs")
>      0ac6075a32fc ("kernfs: use simple_xattrs for security attributes")
>      1537ad15c9c5 ("kernfs: fix xattr name handling in LSM helpers")
>      426dcd4b600f ("hexagon: switch to NO_BOOTMEM")
>      57c8a661d95d ("mm: remove include/linux/bootmem.h")
>      6471f52af786 ("alpha: switch to NO_BOOTMEM")
>      b230d5aba2d1 ("LSM: add new hook for kernfs node initialization")
>      bcec54bf3118 ("mips: switch to NO_BOOTMEM")
>      d0c9c153b4bd ("kernfs: do not alloc iattrs in kernfs_xattr_get")
>      e0a9317d9004 ("hexagon: use generic dma_noncoherent_ops")
>      e262e32d6bde ("vfs: Suppress MS_* flag defs within the kernel unless explicitly enabled")
>      ec882da5cda9 ("selinux: implement the kernfs_init_security hook")
>      f406f222d4b2 ("hexagon: implement the sync_sg_for_device DMA operation")
>
> v4.14.138: Failed to apply! Possible dependencies:
>      05895219627c ("kernfs: clean up struct kernfs_iattrs")
>      067cae47771c ("bpf: Use char in prog and map name")
>      0ac6075a32fc ("kernfs: use simple_xattrs for security attributes")
>      1537ad15c9c5 ("kernfs: fix xattr name handling in LSM helpers")
>      488dee96bb62 ("kernfs: allow creating kernfs objects with arbitrary uid/gid")
>      5cf9dd55a0ec ("afs: Prospectively look up extra files when doing a single lookup")
>      88cda1c9da02 ("bpf: libbpf: Provide basic API support to specify BPF obj name")
>      95582b008388 ("vfs: change inode times to use struct timespec64")
>      ad5b177bd73f ("bpf: Add map_name to bpf_map_info")
>      afdb09c720b6 ("security: bpf: Add LSM hooks for bpf object related syscall")
>      b230d5aba2d1 ("LSM: add new hook for kernfs node initialization")
>      cb4d2b3f03d8 ("bpf: Add name, load_time, uid and map_ids to bpf_prog_info")
>      d0c9c153b4bd ("kernfs: do not alloc iattrs in kernfs_xattr_get")
>      dd9fbcb8e103 ("afs: Rearrange status mapping")
>      df0ce17331e2 ("security: convert security hooks to use hlist")
>      e45b2546e23c ("fuse: Ensure posix acls are translated outside of init_user_ns")
>
> v4.9.189: Failed to apply! Possible dependencies:
>      1537ad15c9c5 ("kernfs: fix xattr name handling in LSM helpers")
>      4eb5aaa3af8a ("sched/headers: Prepare for new header dependencies before moving code to <linux/sched/autogroup.h>")
>      4f17722c7256 ("sched/headers: Prepare for new header dependencies before moving code to <linux/sched/loadavg.h>")
>      5eca1c10cbaa ("sched/headers: Clean up <linux/sched.h>")
>      6e84f31522f9 ("sched/headers: Prepare for new header dependencies before moving code to <linux/sched/mm.h>")
>      91b467e0a3f5 ("afs: Make afs_readpages() fetch data in bulk")
>      944c74f472f9 ("afs: Distinguish mountpoints from symlinks by file mode alone")
>      acb04058de49 ("sched/clock: Fix hotplug crash")
>      ae7e81c077d6 ("sched/headers: Prepare for new header dependencies before moving code to <uapi/linux/sched/types.h>")
>      b230d5aba2d1 ("LSM: add new hook for kernfs node initialization")
>      b5a062344419 ("kernfs: Declare two local data structures static")
>      d3e3b7eac886 ("afs: Add metadata xattrs")
>      df0ce17331e2 ("security: convert security hooks to use hlist")
>      e45b2546e23c ("fuse: Ensure posix acls are translated outside of init_user_ns")
>      e4e55b47ed9a ("LSM: Revive security_task_alloc() hook and per "struct task_struct" security blob.")
>      e601757102cf ("sched/headers: Prepare for new header dependencies before moving code to <linux/sched/clock.h>")
>      ea8b1c4a6019 ("drivers: psci: PSCI checker module")
>      edd3ba94c4e5 ("afs: Convert to separately allocated bdi")
>      ee6a3d19f15b ("sched/headers: Remove the <linux/topology.h> include from <linux/sched.h>")
>      fd7712337ff0 ("sched/headers: Prepare to remove the <linux/gfp.h> include from <linux/sched.h>")
>
> v4.4.189: Failed to apply! Possible dependencies:
>      1182fca3bc00 ("Orangefs: kernel client part 5")
>      24c8d0804be0 ("Orangefs: Clean up pvfs2_devreq_read.")
>      274dcf55bd4a ("Orangefs: kernel client part 3")
>      2c590d5fb698 ("Orangefs: kernel client update 1.")
>      3c2fcfcb6858 ("orangefs: make wait_for_direct_io() take iov_iter")
>      4d1c44043b26 ("Orangefs: use iov_iter interface")
>      548049495cb4 ("Orangefs: fix some checkpatch.pl complaints that had creeped in.")
>      5c278228bbfe ("orangefs: explicitly pass the size to pvfs_bufmap_copy_to_iovec()")
>      5db11c21a929 ("Orangefs: kernel client part 2")
>      5f0e3c953fd9 ("orangefs: make postcopy_buffers() take iov_iter")
>      8092895f759e ("orangefs: validate the response in decode_dirents()")
>      84d02150dea7 ("Orangefs: sooth most sparse complaints")
>      88309aae3ddb ("Orangefs: fix dir_emit code in pvfs2_readdir.")
>      894ac432b48b ("Orangefs: Clean up error decoding.")
>      8bb8aefd5afb ("OrangeFS: Change almost all instances of the string PVFS2 to OrangeFS.")
>      9172abbcd371 ("btrfs: Use xattr handler infrastructure")
>      9be68b08719c ("orangefs: get rid of dec_string and enc_string")
>      a5c126a52269 ("orangefs: make precopy_buffers() take iov_iter")
>      b296821a7c42 ("xattr_handler: pass dentry and inode as separate arguments of ->get()")
>      ef4af94edcf8 ("orangefs: switch decode_dirents() to use of kcalloc()")
>      f0ed4418d46d ("Orangefs: Remove upcall trailers which are not used.")
>      f7ab093f74bf ("Orangefs: kernel client part 1")
>      f7be4ee07fb7 ("Orangefs: kernel client part 4")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
>
> --
> Thanks,
> Sasha

Wait for upstream of course.

Given the conflicts, I can provide back-ports once upstream upon 
request. It should be noted that the backports should be mechanical and 
trivial (skip non-existent filesystems like orangfs, drop separate inode 
argument that did not exist in earlier kernels).

I will submit the next spin (missed a few filesystems, build errors) 
with references to the requested stable trees again, so noise will continue.

Sincerely -- Mark Salyzyn



^ permalink raw reply

* Re: [RFC/RFT v3 2/3] KEYS: trusted: move tpm2 trusted keys code
From: Sumit Garg @ 2019-08-13  7:59 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: keyrings, linux-integrity,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
	linux-security-module, dhowells, Herbert Xu, davem, peterhuewe,
	jgg, jejb, Arnd Bergmann, Greg Kroah-Hartman, Mimi Zohar,
	James Morris, Serge E. Hallyn, Casey Schaufler, Ard Biesheuvel,
	Daniel Thompson, Linux Kernel Mailing List,
	tee-dev @ lists . linaro . org
In-Reply-To: <20190808151500.ypfcqowklalu76uq@linux.intel.com>

On Thu, 8 Aug 2019 at 20:46, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Thu, Aug 08, 2019 at 06:51:38PM +0530, Sumit Garg wrote:
> > It seems to be a functional change which I think requires proper unit
> > testing. I am afraid that I don't posses a TPM device to test this and
> > also very less conversant with tpm_buf code.
> >
> > So what I have done here is to rename existing TPM 1.x trusted keys
> > code to use tpm1_buf.
> >
> > And I would be happy to integrate a tested patch if anyone familiar
> > could work on this.
>
> I can test it on TPM 1.2.
>

I have posted v4 with changes as you requested. I hope they work well
with a real TPM 1.x or TPM 2.0 device.

-Sumit

> /Jarkko

^ permalink raw reply

* [RFC/RFT v4 5/5] KEYS: trusted: Add generic trusted keys framework
From: Sumit Garg @ 2019-08-13  7:53 UTC (permalink / raw)
  To: keyrings, linux-integrity, linux-crypto, linux-security-module
  Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
	arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
	daniel.thompson, linux-kernel, tee-dev, Sumit Garg
In-Reply-To: <1565682784-10234-1-git-send-email-sumit.garg@linaro.org>

Current trusted keys framework is tightly coupled to use TPM device as
an underlying implementation which makes it difficult for implementations
like Trusted Execution Environment (TEE) etc. to provide trusked keys
support in case platform doesn't posses a TPM device.

So this patch tries to add generic trusted keys framework where underlying
implemtations like TPM, TEE etc. could be easily plugged-in.

Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 include/keys/trusted-type.h                 |  45 ++++
 include/keys/trusted_tpm.h                  |  15 --
 security/keys/trusted-keys/Makefile         |   1 +
 security/keys/trusted-keys/trusted-common.c | 343 +++++++++++++++++++++++++++
 security/keys/trusted-keys/trusted-tpm.c    | 345 +++++-----------------------
 5 files changed, 447 insertions(+), 302 deletions(-)
 create mode 100644 security/keys/trusted-keys/trusted-common.c

diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a..5559010 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -40,6 +40,51 @@ struct trusted_key_options {
 	uint32_t policyhandle;
 };
 
+struct trusted_key_ops {
+	/*
+	 * flag to indicate if trusted key implementation supports migration
+	 * or not.
+	 */
+	unsigned char migratable;
+
+	/* trusted key init */
+	int (*init)(void);
+
+	/* seal a trusted key */
+	int (*seal)(struct trusted_key_payload *p, char *datablob);
+
+	/* unseal a trusted key */
+	int (*unseal)(struct trusted_key_payload *p, char *datablob);
+
+	/* get random trusted key */
+	int (*get_random)(unsigned char *key, size_t key_len);
+
+	/* trusted key cleanup */
+	void (*cleanup)(void);
+};
+
 extern struct key_type key_type_trusted;
+#if defined(CONFIG_TCG_TPM)
+extern struct trusted_key_ops tpm_trusted_key_ops;
+#endif
+
+#define TRUSTED_DEBUG 0
+
+#if TRUSTED_DEBUG
+static inline void dump_payload(struct trusted_key_payload *p)
+{
+	pr_info("trusted_key: key_len %d\n", p->key_len);
+	print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
+		       16, 1, p->key, p->key_len, 0);
+	pr_info("trusted_key: bloblen %d\n", p->blob_len);
+	print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
+		       16, 1, p->blob, p->blob_len, 0);
+	pr_info("trusted_key: migratable %d\n", p->migratable);
+}
+#else
+static inline void dump_payload(struct trusted_key_payload *p)
+{
+}
+#endif
 
 #endif /* _KEYS_TRUSTED_TYPE_H */
diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
index 0d72106..7b59344 100644
--- a/include/keys/trusted_tpm.h
+++ b/include/keys/trusted_tpm.h
@@ -57,17 +57,6 @@ static inline void dump_options(struct trusted_key_options *o)
 		       16, 1, o->pcrinfo, o->pcrinfo_len, 0);
 }
 
-static inline void dump_payload(struct trusted_key_payload *p)
-{
-	pr_info("trusted_key: key_len %d\n", p->key_len);
-	print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
-		       16, 1, p->key, p->key_len, 0);
-	pr_info("trusted_key: bloblen %d\n", p->blob_len);
-	print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
-		       16, 1, p->blob, p->blob_len, 0);
-	pr_info("trusted_key: migratable %d\n", p->migratable);
-}
-
 static inline void dump_sess(struct osapsess *s)
 {
 	print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
@@ -93,10 +82,6 @@ static inline void dump_options(struct trusted_key_options *o)
 {
 }
 
-static inline void dump_payload(struct trusted_key_payload *p)
-{
-}
-
 static inline void dump_sess(struct osapsess *s)
 {
 }
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index fb42e94..9a4f721 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -4,5 +4,6 @@
 #
 
 obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
+trusted-y += trusted-common.o
 trusted-y += trusted-tpm.o
 trusted-y += trusted-tpm2.o
diff --git a/security/keys/trusted-keys/trusted-common.c b/security/keys/trusted-keys/trusted-common.c
new file mode 100644
index 0000000..8f00fde
--- /dev/null
+++ b/security/keys/trusted-keys/trusted-common.c
@@ -0,0 +1,343 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2010 IBM Corporation
+ * Copyright (c) 2019, Linaro Limited
+ *
+ * Author:
+ * David Safford <safford@us.ibm.com>
+ * Added generic trusted key framework: Sumit Garg <sumit.garg@linaro.org>
+ *
+ * See Documentation/security/keys/trusted-encrypted.rst
+ */
+
+#include <keys/user-type.h>
+#include <keys/trusted-type.h>
+#include <linux/capability.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/key-type.h>
+#include <linux/module.h>
+#include <linux/parser.h>
+#include <linux/rcupdate.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+
+static struct trusted_key_ops *available_tk_ops[] = {
+#if defined(CONFIG_TCG_TPM)
+	&tpm_trusted_key_ops,
+#endif
+};
+static struct trusted_key_ops *tk_ops;
+
+enum {
+	Opt_err,
+	Opt_new, Opt_load, Opt_update,
+};
+
+static const match_table_t key_tokens = {
+	{Opt_new, "new"},
+	{Opt_load, "load"},
+	{Opt_update, "update"},
+	{Opt_err, NULL}
+};
+
+/*
+ * datablob_parse - parse the keyctl data and fill in the
+ *                  payload structure
+ *
+ * On success returns 0, otherwise -EINVAL.
+ */
+static int datablob_parse(char *datablob, struct trusted_key_payload *p)
+{
+	substring_t args[MAX_OPT_ARGS];
+	long keylen;
+	int ret = -EINVAL;
+	int key_cmd;
+	char *c;
+
+	/* main command */
+	c = strsep(&datablob, " \t");
+	if (!c)
+		return -EINVAL;
+	key_cmd = match_token(c, key_tokens, args);
+	switch (key_cmd) {
+	case Opt_new:
+		/* first argument is key size */
+		c = strsep(&datablob, " \t");
+		if (!c)
+			return -EINVAL;
+		ret = kstrtol(c, 10, &keylen);
+		if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
+			return -EINVAL;
+		p->key_len = keylen;
+		ret = Opt_new;
+		break;
+	case Opt_load:
+		/* first argument is sealed blob */
+		c = strsep(&datablob, " \t");
+		if (!c)
+			return -EINVAL;
+		p->blob_len = strlen(c) / 2;
+		if (p->blob_len > MAX_BLOB_SIZE)
+			return -EINVAL;
+		ret = hex2bin(p->blob, c, p->blob_len);
+		if (ret < 0)
+			return -EINVAL;
+		ret = Opt_load;
+		break;
+	case Opt_update:
+		ret = Opt_update;
+		break;
+	case Opt_err:
+		return -EINVAL;
+	}
+	return ret;
+}
+
+static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
+{
+	struct trusted_key_payload *p = NULL;
+	int ret;
+
+	ret = key_payload_reserve(key, sizeof(*p));
+	if (ret < 0)
+		return p;
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+
+	p->migratable = tk_ops->migratable;
+
+	return p;
+}
+
+/*
+ * trusted_instantiate - create a new trusted key
+ *
+ * Unseal an existing trusted blob or, for a new key, get a
+ * random key, then seal and create a trusted key-type key,
+ * adding it to the specified keyring.
+ *
+ * On success, return 0. Otherwise return errno.
+ */
+static int trusted_instantiate(struct key *key,
+			       struct key_preparsed_payload *prep)
+{
+	struct trusted_key_payload *payload = NULL;
+	size_t datalen = prep->datalen;
+	char *datablob;
+	int ret = 0;
+	int key_cmd;
+	size_t key_len;
+
+	if (datalen <= 0 || datalen > 32767 || !prep->data)
+		return -EINVAL;
+
+	datablob = kmalloc(datalen + 1, GFP_KERNEL);
+	if (!datablob)
+		return -ENOMEM;
+	memcpy(datablob, prep->data, datalen);
+	datablob[datalen] = '\0';
+
+	payload = trusted_payload_alloc(key);
+	if (!payload) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	key_cmd = datablob_parse(datablob, payload);
+	if (key_cmd < 0) {
+		ret = key_cmd;
+		goto out;
+	}
+
+	dump_payload(payload);
+
+	switch (key_cmd) {
+	case Opt_load:
+		ret = tk_ops->unseal(payload, datablob);
+		dump_payload(payload);
+		if (ret < 0)
+			pr_info("trusted_key: key_unseal failed (%d)\n", ret);
+		break;
+	case Opt_new:
+		key_len = payload->key_len;
+		ret = tk_ops->get_random(payload->key, key_len);
+		if (ret != key_len) {
+			pr_info("trusted_key: key_create failed (%d)\n", ret);
+			goto out;
+		}
+
+		ret = tk_ops->seal(payload, datablob);
+		if (ret < 0)
+			pr_info("trusted_key: key_seal failed (%d)\n", ret);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+out:
+	kzfree(datablob);
+	if (!ret)
+		rcu_assign_keypointer(key, payload);
+	else
+		kzfree(payload);
+	return ret;
+}
+
+static void trusted_rcu_free(struct rcu_head *rcu)
+{
+	struct trusted_key_payload *p;
+
+	p = container_of(rcu, struct trusted_key_payload, rcu);
+	kzfree(p);
+}
+
+/*
+ * trusted_update - reseal an existing key with new PCR values
+ */
+static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
+{
+	struct trusted_key_payload *p;
+	struct trusted_key_payload *new_p;
+	size_t datalen = prep->datalen;
+	char *datablob;
+	int ret = 0;
+
+	if (key_is_negative(key))
+		return -ENOKEY;
+	p = key->payload.data[0];
+	if (!p->migratable)
+		return -EPERM;
+	if (datalen <= 0 || datalen > 32767 || !prep->data)
+		return -EINVAL;
+
+	datablob = kmalloc(datalen + 1, GFP_KERNEL);
+	if (!datablob)
+		return -ENOMEM;
+
+	new_p = trusted_payload_alloc(key);
+	if (!new_p) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	memcpy(datablob, prep->data, datalen);
+	datablob[datalen] = '\0';
+	ret = datablob_parse(datablob, new_p);
+	if (ret != Opt_update) {
+		ret = -EINVAL;
+		kzfree(new_p);
+		goto out;
+	}
+
+	/* copy old key values, and reseal with new pcrs */
+	new_p->migratable = p->migratable;
+	new_p->key_len = p->key_len;
+	memcpy(new_p->key, p->key, p->key_len);
+	dump_payload(p);
+	dump_payload(new_p);
+
+	ret = tk_ops->seal(new_p, datablob);
+	if (ret < 0) {
+		pr_info("trusted_key: key_seal failed (%d)\n", ret);
+		kzfree(new_p);
+		goto out;
+	}
+
+	rcu_assign_keypointer(key, new_p);
+	call_rcu(&p->rcu, trusted_rcu_free);
+out:
+	kzfree(datablob);
+	return ret;
+}
+
+/*
+ * trusted_read - copy the sealed blob data to userspace in hex.
+ * On success, return to userspace the trusted key datablob size.
+ */
+static long trusted_read(const struct key *key, char __user *buffer,
+			 size_t buflen)
+{
+	const struct trusted_key_payload *p;
+	char *ascii_buf;
+	char *bufp;
+	int i;
+
+	p = dereference_key_locked(key);
+	if (!p)
+		return -EINVAL;
+
+	if (buffer && buflen >= 2 * p->blob_len) {
+		ascii_buf = kmalloc_array(2, p->blob_len, GFP_KERNEL);
+		if (!ascii_buf)
+			return -ENOMEM;
+
+		bufp = ascii_buf;
+		for (i = 0; i < p->blob_len; i++)
+			bufp = hex_byte_pack(bufp, p->blob[i]);
+		if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) {
+			kzfree(ascii_buf);
+			return -EFAULT;
+		}
+		kzfree(ascii_buf);
+	}
+	return 2 * p->blob_len;
+}
+
+/*
+ * trusted_destroy - clear and free the key's payload
+ */
+static void trusted_destroy(struct key *key)
+{
+	kzfree(key->payload.data[0]);
+}
+
+struct key_type key_type_trusted = {
+	.name = "trusted",
+	.instantiate = trusted_instantiate,
+	.update = trusted_update,
+	.destroy = trusted_destroy,
+	.describe = user_describe,
+	.read = trusted_read,
+};
+EXPORT_SYMBOL_GPL(key_type_trusted);
+
+static int __init init_trusted(void)
+{
+	int i, ret = 0;
+
+	for (i = 0; i < sizeof(available_tk_ops); i++) {
+		tk_ops = available_tk_ops[i];
+
+		if (!(tk_ops && tk_ops->init && tk_ops->seal &&
+		      tk_ops->unseal && tk_ops->get_random))
+			continue;
+
+		ret = tk_ops->init();
+		if (ret) {
+			if (tk_ops->cleanup)
+				tk_ops->cleanup();
+		} else {
+			break;
+		}
+	}
+
+	/*
+	 * encrypted_keys.ko depends on successful load of this module even if
+	 * trusted key implementation is not found.
+	 */
+	if (ret == -ENODEV)
+		return 0;
+
+	return ret;
+}
+
+static void __exit cleanup_trusted(void)
+{
+	if (tk_ops->cleanup)
+		tk_ops->cleanup();
+}
+
+late_initcall(init_trusted);
+module_exit(cleanup_trusted);
+
+MODULE_LICENSE("GPL");
diff --git a/security/keys/trusted-keys/trusted-tpm.c b/security/keys/trusted-keys/trusted-tpm.c
index 66687e7..dab7c32 100644
--- a/security/keys/trusted-keys/trusted-tpm.c
+++ b/security/keys/trusted-keys/trusted-tpm.c
@@ -1,29 +1,26 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (C) 2010 IBM Corporation
+ * Copyright (c) 2019, Linaro Limited
  *
  * Author:
  * David Safford <safford@us.ibm.com>
+ * Switch to generic trusted key framework: Sumit Garg <sumit.garg@linaro.org>
  *
  * See Documentation/security/keys/trusted-encrypted.rst
  */
 
 #include <crypto/hash_info.h>
-#include <linux/uaccess.h>
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/parser.h>
 #include <linux/string.h>
 #include <linux/err.h>
-#include <keys/user-type.h>
 #include <keys/trusted-type.h>
 #include <linux/key-type.h>
-#include <linux/rcupdate.h>
 #include <linux/crypto.h>
 #include <crypto/hash.h>
 #include <crypto/sha.h>
-#include <linux/capability.h>
 #include <linux/tpm.h>
 #include <linux/tpm_command.h>
 
@@ -705,7 +702,6 @@ static int key_unseal(struct trusted_key_payload *p,
 
 enum {
 	Opt_err,
-	Opt_new, Opt_load, Opt_update,
 	Opt_keyhandle, Opt_keyauth, Opt_blobauth,
 	Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
 	Opt_hash,
@@ -714,9 +710,6 @@ enum {
 };
 
 static const match_table_t key_tokens = {
-	{Opt_new, "new"},
-	{Opt_load, "load"},
-	{Opt_update, "update"},
 	{Opt_keyhandle, "keyhandle=%s"},
 	{Opt_keyauth, "keyauth=%s"},
 	{Opt_blobauth, "blobauth=%s"},
@@ -843,71 +836,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 	return 0;
 }
 
-/*
- * datablob_parse - parse the keyctl data and fill in the
- * 		    payload and options structures
- *
- * On success returns 0, otherwise -EINVAL.
- */
-static int datablob_parse(char *datablob, struct trusted_key_payload *p,
-			  struct trusted_key_options *o)
-{
-	substring_t args[MAX_OPT_ARGS];
-	long keylen;
-	int ret = -EINVAL;
-	int key_cmd;
-	char *c;
-
-	/* main command */
-	c = strsep(&datablob, " \t");
-	if (!c)
-		return -EINVAL;
-	key_cmd = match_token(c, key_tokens, args);
-	switch (key_cmd) {
-	case Opt_new:
-		/* first argument is key size */
-		c = strsep(&datablob, " \t");
-		if (!c)
-			return -EINVAL;
-		ret = kstrtol(c, 10, &keylen);
-		if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
-			return -EINVAL;
-		p->key_len = keylen;
-		ret = getoptions(datablob, p, o);
-		if (ret < 0)
-			return ret;
-		ret = Opt_new;
-		break;
-	case Opt_load:
-		/* first argument is sealed blob */
-		c = strsep(&datablob, " \t");
-		if (!c)
-			return -EINVAL;
-		p->blob_len = strlen(c) / 2;
-		if (p->blob_len > MAX_BLOB_SIZE)
-			return -EINVAL;
-		ret = hex2bin(p->blob, c, p->blob_len);
-		if (ret < 0)
-			return -EINVAL;
-		ret = getoptions(datablob, p, o);
-		if (ret < 0)
-			return ret;
-		ret = Opt_load;
-		break;
-	case Opt_update:
-		/* all arguments are options */
-		ret = getoptions(datablob, p, o);
-		if (ret < 0)
-			return ret;
-		ret = Opt_update;
-		break;
-	case Opt_err:
-		return -EINVAL;
-		break;
-	}
-	return ret;
-}
-
 static struct trusted_key_options *trusted_options_alloc(void)
 {
 	struct trusted_key_options *options;
@@ -928,258 +856,99 @@ static struct trusted_key_options *trusted_options_alloc(void)
 	return options;
 }
 
-static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
+static int tpm_tk_seal(struct trusted_key_payload *p, char *datablob)
 {
-	struct trusted_key_payload *p = NULL;
-	int ret;
-
-	ret = key_payload_reserve(key, sizeof *p);
-	if (ret < 0)
-		return p;
-	p = kzalloc(sizeof *p, GFP_KERNEL);
-	if (p)
-		p->migratable = 1; /* migratable by default */
-	return p;
-}
-
-/*
- * trusted_instantiate - create a new trusted key
- *
- * Unseal an existing trusted blob or, for a new key, get a
- * random key, then seal and create a trusted key-type key,
- * adding it to the specified keyring.
- *
- * On success, return 0. Otherwise return errno.
- */
-static int trusted_instantiate(struct key *key,
-			       struct key_preparsed_payload *prep)
-{
-	struct trusted_key_payload *payload = NULL;
 	struct trusted_key_options *options = NULL;
-	size_t datalen = prep->datalen;
-	char *datablob;
 	int ret = 0;
-	int key_cmd;
-	size_t key_len;
 	int tpm2;
 
 	tpm2 = tpm_is_tpm2(chip);
 	if (tpm2 < 0)
 		return tpm2;
 
-	if (datalen <= 0 || datalen > 32767 || !prep->data)
-		return -EINVAL;
-
-	datablob = kmalloc(datalen + 1, GFP_KERNEL);
-	if (!datablob)
-		return -ENOMEM;
-	memcpy(datablob, prep->data, datalen);
-	datablob[datalen] = '\0';
-
 	options = trusted_options_alloc();
-	if (!options) {
-		ret = -ENOMEM;
-		goto out;
-	}
-	payload = trusted_payload_alloc(key);
-	if (!payload) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!options)
+		return -ENOMEM;
 
-	key_cmd = datablob_parse(datablob, payload, options);
-	if (key_cmd < 0) {
-		ret = key_cmd;
+	ret = getoptions(datablob, p, options);
+	if (ret < 0)
 		goto out;
-	}
+	dump_options(options);
 
 	if (!options->keyhandle) {
 		ret = -EINVAL;
 		goto out;
 	}
 
-	dump_payload(payload);
-	dump_options(options);
+	if (tpm2)
+		ret = tpm_seal_trusted(chip, p, options);
+	else
+		ret = key_seal(p, options);
+	if (ret < 0) {
+		pr_info("tpm_trusted_key: key_seal failed (%d)\n", ret);
+		goto out;
+	}
 
-	switch (key_cmd) {
-	case Opt_load:
-		if (tpm2)
-			ret = tpm_unseal_trusted(chip, payload, options);
-		else
-			ret = key_unseal(payload, options);
-		dump_payload(payload);
-		dump_options(options);
-		if (ret < 0)
-			pr_info("trusted_key: key_unseal failed (%d)\n", ret);
-		break;
-	case Opt_new:
-		key_len = payload->key_len;
-		ret = tpm_get_random(chip, payload->key, key_len);
-		if (ret != key_len) {
-			pr_info("trusted_key: key_create failed (%d)\n", ret);
+	if (options->pcrlock) {
+		ret = pcrlock(options->pcrlock);
+		if (ret < 0) {
+			pr_info("tpm_trusted_key: pcrlock failed (%d)\n", ret);
 			goto out;
 		}
-		if (tpm2)
-			ret = tpm_seal_trusted(chip, payload, options);
-		else
-			ret = key_seal(payload, options);
-		if (ret < 0)
-			pr_info("trusted_key: key_seal failed (%d)\n", ret);
-		break;
-	default:
-		ret = -EINVAL;
-		goto out;
 	}
-	if (!ret && options->pcrlock)
-		ret = pcrlock(options->pcrlock);
 out:
-	kzfree(datablob);
 	kzfree(options);
-	if (!ret)
-		rcu_assign_keypointer(key, payload);
-	else
-		kzfree(payload);
 	return ret;
 }
 
-static void trusted_rcu_free(struct rcu_head *rcu)
-{
-	struct trusted_key_payload *p;
-
-	p = container_of(rcu, struct trusted_key_payload, rcu);
-	kzfree(p);
-}
-
-/*
- * trusted_update - reseal an existing key with new PCR values
- */
-static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
+static int tpm_tk_unseal(struct trusted_key_payload *p, char *datablob)
 {
-	struct trusted_key_payload *p;
-	struct trusted_key_payload *new_p;
-	struct trusted_key_options *new_o;
-	size_t datalen = prep->datalen;
-	char *datablob;
+	struct trusted_key_options *options = NULL;
 	int ret = 0;
+	int tpm2;
 
-	if (key_is_negative(key))
-		return -ENOKEY;
-	p = key->payload.data[0];
-	if (!p->migratable)
-		return -EPERM;
-	if (datalen <= 0 || datalen > 32767 || !prep->data)
-		return -EINVAL;
+	tpm2 = tpm_is_tpm2(chip);
+	if (tpm2 < 0)
+		return tpm2;
 
-	datablob = kmalloc(datalen + 1, GFP_KERNEL);
-	if (!datablob)
+	options = trusted_options_alloc();
+	if (!options)
 		return -ENOMEM;
-	new_o = trusted_options_alloc();
-	if (!new_o) {
-		ret = -ENOMEM;
-		goto out;
-	}
-	new_p = trusted_payload_alloc(key);
-	if (!new_p) {
-		ret = -ENOMEM;
-		goto out;
-	}
 
-	memcpy(datablob, prep->data, datalen);
-	datablob[datalen] = '\0';
-	ret = datablob_parse(datablob, new_p, new_o);
-	if (ret != Opt_update) {
-		ret = -EINVAL;
-		kzfree(new_p);
+	ret = getoptions(datablob, p, options);
+	if (ret < 0)
 		goto out;
-	}
+	dump_options(options);
 
-	if (!new_o->keyhandle) {
+	if (!options->keyhandle) {
 		ret = -EINVAL;
-		kzfree(new_p);
 		goto out;
 	}
 
-	/* copy old key values, and reseal with new pcrs */
-	new_p->migratable = p->migratable;
-	new_p->key_len = p->key_len;
-	memcpy(new_p->key, p->key, p->key_len);
-	dump_payload(p);
-	dump_payload(new_p);
+	if (tpm2)
+		ret = tpm_unseal_trusted(chip, p, options);
+	else
+		ret = key_unseal(p, options);
+	if (ret < 0)
+		pr_info("tpm_trusted_key: key_unseal failed (%d)\n", ret);
 
-	ret = key_seal(new_p, new_o);
-	if (ret < 0) {
-		pr_info("trusted_key: key_seal failed (%d)\n", ret);
-		kzfree(new_p);
-		goto out;
-	}
-	if (new_o->pcrlock) {
-		ret = pcrlock(new_o->pcrlock);
+	if (options->pcrlock) {
+		ret = pcrlock(options->pcrlock);
 		if (ret < 0) {
-			pr_info("trusted_key: pcrlock failed (%d)\n", ret);
-			kzfree(new_p);
+			pr_info("tpm_trusted_key: pcrlock failed (%d)\n", ret);
 			goto out;
 		}
 	}
-	rcu_assign_keypointer(key, new_p);
-	call_rcu(&p->rcu, trusted_rcu_free);
 out:
-	kzfree(datablob);
-	kzfree(new_o);
+	kzfree(options);
 	return ret;
 }
 
-/*
- * trusted_read - copy the sealed blob data to userspace in hex.
- * On success, return to userspace the trusted key datablob size.
- */
-static long trusted_read(const struct key *key, char __user *buffer,
-			 size_t buflen)
-{
-	const struct trusted_key_payload *p;
-	char *ascii_buf;
-	char *bufp;
-	int i;
-
-	p = dereference_key_locked(key);
-	if (!p)
-		return -EINVAL;
-
-	if (buffer && buflen >= 2 * p->blob_len) {
-		ascii_buf = kmalloc_array(2, p->blob_len, GFP_KERNEL);
-		if (!ascii_buf)
-			return -ENOMEM;
-
-		bufp = ascii_buf;
-		for (i = 0; i < p->blob_len; i++)
-			bufp = hex_byte_pack(bufp, p->blob[i]);
-		if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) {
-			kzfree(ascii_buf);
-			return -EFAULT;
-		}
-		kzfree(ascii_buf);
-	}
-	return 2 * p->blob_len;
-}
-
-/*
- * trusted_destroy - clear and free the key's payload
- */
-static void trusted_destroy(struct key *key)
+int tpm_tk_get_random(unsigned char *key, size_t key_len)
 {
-	kzfree(key->payload.data[0]);
+	return tpm_get_random(chip, key, key_len);
 }
 
-struct key_type key_type_trusted = {
-	.name = "trusted",
-	.instantiate = trusted_instantiate,
-	.update = trusted_update,
-	.destroy = trusted_destroy,
-	.describe = user_describe,
-	.read = trusted_read,
-};
-
-EXPORT_SYMBOL_GPL(key_type_trusted);
-
 static void trusted_shash_release(void)
 {
 	if (hashalg)
@@ -1194,14 +963,14 @@ static int __init trusted_shash_alloc(void)
 
 	hmacalg = crypto_alloc_shash(hmac_alg, 0, 0);
 	if (IS_ERR(hmacalg)) {
-		pr_info("trusted_key: could not allocate crypto %s\n",
+		pr_info("tpm_trusted_key: could not allocate crypto %s\n",
 			hmac_alg);
 		return PTR_ERR(hmacalg);
 	}
 
 	hashalg = crypto_alloc_shash(hash_alg, 0, 0);
 	if (IS_ERR(hashalg)) {
-		pr_info("trusted_key: could not allocate crypto %s\n",
+		pr_info("tpm_trusted_key: could not allocate crypto %s\n",
 			hash_alg);
 		ret = PTR_ERR(hashalg);
 		goto hashalg_fail;
@@ -1237,16 +1006,13 @@ static int __init init_digests(void)
 	return 0;
 }
 
-static int __init init_trusted(void)
+static int __init init_tpm_trusted(void)
 {
 	int ret;
 
-	/* encrypted_keys.ko depends on successful load of this module even if
-	 * TPM is not used.
-	 */
 	chip = tpm_default_chip();
 	if (!chip)
-		return 0;
+		return -ENODEV;
 
 	ret = init_digests();
 	if (ret < 0)
@@ -1267,7 +1033,7 @@ static int __init init_trusted(void)
 	return ret;
 }
 
-static void __exit cleanup_trusted(void)
+static void __exit cleanup_tpm_trusted(void)
 {
 	if (chip) {
 		put_device(&chip->dev);
@@ -1277,7 +1043,12 @@ static void __exit cleanup_trusted(void)
 	}
 }
 
-late_initcall(init_trusted);
-module_exit(cleanup_trusted);
-
-MODULE_LICENSE("GPL");
+struct trusted_key_ops tpm_trusted_key_ops = {
+	.migratable = 1, /* migratable by default */
+	.init = init_tpm_trusted,
+	.seal = tpm_tk_seal,
+	.unseal = tpm_tk_unseal,
+	.get_random = tpm_tk_get_random,
+	.cleanup = cleanup_tpm_trusted,
+};
+EXPORT_SYMBOL_GPL(tpm_trusted_key_ops);
-- 
2.7.4


^ permalink raw reply related

* [RFC/RFT v4 4/5] KEYS: trusted: move tpm2 trusted keys code
From: Sumit Garg @ 2019-08-13  7:53 UTC (permalink / raw)
  To: keyrings, linux-integrity, linux-crypto, linux-security-module
  Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
	arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
	daniel.thompson, linux-kernel, tee-dev, Sumit Garg
In-Reply-To: <1565682784-10234-1-git-send-email-sumit.garg@linaro.org>

Move TPM2 trusted keys code to trusted keys subsystem. The reason
being it's better to consolidate all the trusted keys code to a single
location so that it can be maintained sanely.

Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/char/tpm/tpm-chip.c               |   1 +
 drivers/char/tpm/tpm-interface.c          |  56 -----
 drivers/char/tpm/tpm.h                    |  16 --
 drivers/char/tpm/tpm2-cmd.c               | 308 +-----------------------
 include/keys/trusted_tpm.h                |   7 +
 include/linux/tpm.h                       |  56 +++--
 security/keys/trusted-keys/Makefile       |   1 +
 security/keys/trusted-keys/trusted-tpm2.c | 378 ++++++++++++++++++++++++++++++
 8 files changed, 429 insertions(+), 394 deletions(-)
 create mode 100644 security/keys/trusted-keys/trusted-tpm2.c

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index d47ad10..49450c1 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -243,6 +243,7 @@ struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip)
 		return NULL;
 	return chip;
 }
+EXPORT_SYMBOL_GPL(tpm_find_get_ops);
 
 /**
  * tpm_dev_release() - free chip memory and the device number
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 1b4f95c..208e5ba 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -454,62 +454,6 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 }
 EXPORT_SYMBOL_GPL(tpm_get_random);
 
-/**
- * tpm_seal_trusted() - seal a trusted key payload
- * @chip:	a &struct tpm_chip instance, %NULL for the default chip
- * @options:	authentication values and other options
- * @payload:	the key data in clear and encrypted form
- *
- * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
- * the keyring subsystem.
- *
- * Return: same as with tpm_transmit_cmd()
- */
-int tpm_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload,
-		     struct trusted_key_options *options)
-{
-	int rc;
-
-	chip = tpm_find_get_ops(chip);
-	if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
-		return -ENODEV;
-
-	rc = tpm2_seal_trusted(chip, payload, options);
-
-	tpm_put_ops(chip);
-	return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_seal_trusted);
-
-/**
- * tpm_unseal_trusted() - unseal a trusted key
- * @chip:	a &struct tpm_chip instance, %NULL for the default chip
- * @options:	authentication values and other options
- * @payload:	the key data in clear and encrypted form
- *
- * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
- * the keyring subsystem.
- *
- * Return: same as with tpm_transmit_cmd()
- */
-int tpm_unseal_trusted(struct tpm_chip *chip,
-		       struct trusted_key_payload *payload,
-		       struct trusted_key_options *options)
-{
-	int rc;
-
-	chip = tpm_find_get_ops(chip);
-	if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
-		return -ENODEV;
-
-	rc = tpm2_unseal_trusted(chip, payload, options);
-
-	tpm_put_ops(chip);
-
-	return rc;
-}
-EXPORT_SYMBOL_GPL(tpm_unseal_trusted);
-
 static int __init tpm_init(void)
 {
 	int rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 0b5498a..1c07ce6 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -164,8 +164,6 @@ extern const struct file_operations tpmrm_fops;
 extern struct idr dev_nums_idr;
 
 ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
-			 size_t min_rsp_body_length, const char *desc);
 int tpm_get_timeouts(struct tpm_chip *);
 int tpm_auto_startup(struct tpm_chip *chip);
 
@@ -192,9 +190,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
 
 int tpm_chip_start(struct tpm_chip *chip);
 void tpm_chip_stop(struct tpm_chip *chip);
-struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
 __must_check int tpm_try_get_ops(struct tpm_chip *chip);
-void tpm_put_ops(struct tpm_chip *chip);
 
 struct tpm_chip *tpm_chip_alloc(struct device *dev,
 				const struct tpm_class_ops *ops);
@@ -214,24 +210,12 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
 }
 #endif
 
-static inline u32 tpm2_rc_value(u32 rc)
-{
-	return (rc & BIT(7)) ? rc & 0xff : rc;
-}
-
 int tpm2_get_timeouts(struct tpm_chip *chip);
 int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 		  struct tpm_digest *digest, u16 *digest_size_ptr);
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 		    struct tpm_digest *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
-void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
-int tpm2_seal_trusted(struct tpm_chip *chip,
-		      struct trusted_key_payload *payload,
-		      struct trusted_key_options *options);
-int tpm2_unseal_trusted(struct tpm_chip *chip,
-			struct trusted_key_payload *payload,
-			struct trusted_key_options *options);
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 			u32 *value, const char *desc);
 
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index d103545..8bb34890 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -13,20 +13,6 @@
 
 #include "tpm.h"
 #include <crypto/hash_info.h>
-#include <keys/trusted-type.h>
-
-enum tpm2_object_attributes {
-	TPM2_OA_USER_WITH_AUTH		= BIT(6),
-};
-
-enum tpm2_session_attributes {
-	TPM2_SA_CONTINUE_SESSION	= BIT(0),
-};
-
-struct tpm2_hash {
-	unsigned int crypto_id;
-	unsigned int tpm_id;
-};
 
 static struct tpm2_hash tpm2_hash_map[] = {
 	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
@@ -376,299 +362,7 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 	tpm_transmit_cmd(chip, &buf, 0, "flushing context");
 	tpm_buf_destroy(&buf);
 }
-
-/**
- * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
- *
- * @buf: an allocated tpm_buf instance
- * @session_handle: session handle
- * @nonce: the session nonce, may be NULL if not used
- * @nonce_len: the session nonce length, may be 0 if not used
- * @attributes: the session attributes
- * @hmac: the session HMAC or password, may be NULL if not used
- * @hmac_len: the session HMAC or password length, maybe 0 if not used
- */
-static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
-				 const u8 *nonce, u16 nonce_len,
-				 u8 attributes,
-				 const u8 *hmac, u16 hmac_len)
-{
-	tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
-	tpm_buf_append_u32(buf, session_handle);
-	tpm_buf_append_u16(buf, nonce_len);
-
-	if (nonce && nonce_len)
-		tpm_buf_append(buf, nonce, nonce_len);
-
-	tpm_buf_append_u8(buf, attributes);
-	tpm_buf_append_u16(buf, hmac_len);
-
-	if (hmac && hmac_len)
-		tpm_buf_append(buf, hmac, hmac_len);
-}
-
-/**
- * tpm2_seal_trusted() - seal the payload of a trusted key
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- *
- * Return: < 0 on error and 0 on success.
- */
-int tpm2_seal_trusted(struct tpm_chip *chip,
-		      struct trusted_key_payload *payload,
-		      struct trusted_key_options *options)
-{
-	unsigned int blob_len;
-	struct tpm_buf buf;
-	u32 hash;
-	int i;
-	int rc;
-
-	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
-		if (options->hash == tpm2_hash_map[i].crypto_id) {
-			hash = tpm2_hash_map[i].tpm_id;
-			break;
-		}
-	}
-
-	if (i == ARRAY_SIZE(tpm2_hash_map))
-		return -EINVAL;
-
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
-	if (rc)
-		return rc;
-
-	tpm_buf_append_u32(&buf, options->keyhandle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     0 /* session_attributes */,
-			     options->keyauth /* hmac */,
-			     TPM_DIGEST_SIZE);
-
-	/* sensitive */
-	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
-
-	tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
-	tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
-	tpm_buf_append_u16(&buf, payload->key_len + 1);
-	tpm_buf_append(&buf, payload->key, payload->key_len);
-	tpm_buf_append_u8(&buf, payload->migratable);
-
-	/* public */
-	tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
-	tpm_buf_append_u16(&buf, TPM_ALG_KEYEDHASH);
-	tpm_buf_append_u16(&buf, hash);
-
-	/* policy */
-	if (options->policydigest_len) {
-		tpm_buf_append_u32(&buf, 0);
-		tpm_buf_append_u16(&buf, options->policydigest_len);
-		tpm_buf_append(&buf, options->policydigest,
-			       options->policydigest_len);
-	} else {
-		tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
-		tpm_buf_append_u16(&buf, 0);
-	}
-
-	/* public parameters */
-	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
-	tpm_buf_append_u16(&buf, 0);
-
-	/* outside info */
-	tpm_buf_append_u16(&buf, 0);
-
-	/* creation PCR */
-	tpm_buf_append_u32(&buf, 0);
-
-	if (buf.flags & TPM_BUF_OVERFLOW) {
-		rc = -E2BIG;
-		goto out;
-	}
-
-	rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
-	if (rc)
-		goto out;
-
-	blob_len = be32_to_cpup((__be32 *) &buf.data[TPM_HEADER_SIZE]);
-	if (blob_len > MAX_BLOB_SIZE) {
-		rc = -E2BIG;
-		goto out;
-	}
-	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
-		rc = -EFAULT;
-		goto out;
-	}
-
-	memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
-	payload->blob_len = blob_len;
-
-out:
-	tpm_buf_destroy(&buf);
-
-	if (rc > 0) {
-		if (tpm2_rc_value(rc) == TPM2_RC_HASH)
-			rc = -EINVAL;
-		else
-			rc = -EPERM;
-	}
-
-	return rc;
-}
-
-/**
- * tpm2_load_cmd() - execute a TPM2_Load command
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: returned blob handle
- *
- * Return: 0 on success.
- *        -E2BIG on wrong payload size.
- *        -EPERM on tpm error status.
- *        < 0 error from tpm_transmit_cmd.
- */
-static int tpm2_load_cmd(struct tpm_chip *chip,
-			 struct trusted_key_payload *payload,
-			 struct trusted_key_options *options,
-			 u32 *blob_handle)
-{
-	struct tpm_buf buf;
-	unsigned int private_len;
-	unsigned int public_len;
-	unsigned int blob_len;
-	int rc;
-
-	private_len = be16_to_cpup((__be16 *) &payload->blob[0]);
-	if (private_len > (payload->blob_len - 2))
-		return -E2BIG;
-
-	public_len = be16_to_cpup((__be16 *) &payload->blob[2 + private_len]);
-	blob_len = private_len + public_len + 4;
-	if (blob_len > payload->blob_len)
-		return -E2BIG;
-
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
-	if (rc)
-		return rc;
-
-	tpm_buf_append_u32(&buf, options->keyhandle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     0 /* session_attributes */,
-			     options->keyauth /* hmac */,
-			     TPM_DIGEST_SIZE);
-
-	tpm_buf_append(&buf, payload->blob, blob_len);
-
-	if (buf.flags & TPM_BUF_OVERFLOW) {
-		rc = -E2BIG;
-		goto out;
-	}
-
-	rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
-	if (!rc)
-		*blob_handle = be32_to_cpup(
-			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
-
-out:
-	tpm_buf_destroy(&buf);
-
-	if (rc > 0)
-		rc = -EPERM;
-
-	return rc;
-}
-
-/**
- * tpm2_unseal_cmd() - execute a TPM2_Unload command
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- * @blob_handle: blob handle
- *
- * Return: 0 on success
- *         -EPERM on tpm error status
- *         < 0 error from tpm_transmit_cmd
- */
-static int tpm2_unseal_cmd(struct tpm_chip *chip,
-			   struct trusted_key_payload *payload,
-			   struct trusted_key_options *options,
-			   u32 blob_handle)
-{
-	struct tpm_buf buf;
-	u16 data_len;
-	u8 *data;
-	int rc;
-
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
-	if (rc)
-		return rc;
-
-	tpm_buf_append_u32(&buf, blob_handle);
-	tpm2_buf_append_auth(&buf,
-			     options->policyhandle ?
-			     options->policyhandle : TPM2_RS_PW,
-			     NULL /* nonce */, 0,
-			     TPM2_SA_CONTINUE_SESSION,
-			     options->blobauth /* hmac */,
-			     TPM_DIGEST_SIZE);
-
-	rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
-	if (rc > 0)
-		rc = -EPERM;
-
-	if (!rc) {
-		data_len = be16_to_cpup(
-			(__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
-		if (data_len < MIN_KEY_SIZE ||  data_len > MAX_KEY_SIZE + 1) {
-			rc = -EFAULT;
-			goto out;
-		}
-
-		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
-			rc = -EFAULT;
-			goto out;
-		}
-		data = &buf.data[TPM_HEADER_SIZE + 6];
-
-		memcpy(payload->key, data, data_len - 1);
-		payload->key_len = data_len - 1;
-		payload->migratable = data[data_len - 1];
-	}
-
-out:
-	tpm_buf_destroy(&buf);
-	return rc;
-}
-
-/**
- * tpm2_unseal_trusted() - unseal the payload of a trusted key
- *
- * @chip: TPM chip to use
- * @payload: the key data in clear and encrypted form
- * @options: authentication values and other options
- *
- * Return: Same as with tpm_transmit_cmd.
- */
-int tpm2_unseal_trusted(struct tpm_chip *chip,
-			struct trusted_key_payload *payload,
-			struct trusted_key_options *options)
-{
-	u32 blob_handle;
-	int rc;
-
-	rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
-	if (rc)
-		return rc;
-
-	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
-	tpm2_flush_context(chip, blob_handle);
-	return rc;
-}
+EXPORT_SYMBOL_GPL(tpm2_flush_context);
 
 struct tpm2_get_cap_out {
 	u8 more_data;
diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
index 9bdf5f4..0d72106 100644
--- a/include/keys/trusted_tpm.h
+++ b/include/keys/trusted_tpm.h
@@ -37,6 +37,13 @@ int TSS_checkhmac1(unsigned char *buffer,
 int trusted_tpm_send(unsigned char *cmd, size_t buflen);
 int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce);
 
+int tpm_seal_trusted(struct tpm_chip *chip,
+		     struct trusted_key_payload *payload,
+		     struct trusted_key_options *options);
+int tpm_unseal_trusted(struct tpm_chip *chip,
+		       struct trusted_key_payload *payload,
+		       struct trusted_key_options *options);
+
 #define TPM_DEBUG 0
 
 #if TPM_DEBUG
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 130c167..895179f 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -294,6 +294,19 @@ struct tpm_buf {
 	u8 *data;
 };
 
+enum tpm2_object_attributes {
+	TPM2_OA_USER_WITH_AUTH		= BIT(6),
+};
+
+enum tpm2_session_attributes {
+	TPM2_SA_CONTINUE_SESSION	= BIT(0),
+};
+
+struct tpm2_hash {
+	unsigned int crypto_id;
+	unsigned int tpm_id;
+};
+
 static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
 {
 	struct tpm_header *head = (struct tpm_header *)buf->data;
@@ -375,6 +388,11 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 	tpm_buf_append(buf, (u8 *) &value2, 4);
 }
 
+static inline u32 tpm2_rc_value(u32 rc)
+{
+	return (rc & BIT(7)) ? rc & 0xff : rc;
+}
+
 #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
 
 extern int tpm_is_tpm2(struct tpm_chip *chip);
@@ -384,13 +402,12 @@ extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 			  struct tpm_digest *digests);
 extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
 extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
-extern int tpm_seal_trusted(struct tpm_chip *chip,
-			    struct trusted_key_payload *payload,
-			    struct trusted_key_options *options);
-extern int tpm_unseal_trusted(struct tpm_chip *chip,
-			      struct trusted_key_payload *payload,
-			      struct trusted_key_options *options);
 extern struct tpm_chip *tpm_default_chip(void);
+extern struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
+extern void tpm_put_ops(struct tpm_chip *chip);
+extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
+				size_t min_rsp_body_length, const char *desc);
+extern void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
 #else
 static inline int tpm_is_tpm2(struct tpm_chip *chip)
 {
@@ -418,21 +435,30 @@ static inline int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max)
 	return -ENODEV;
 }
 
-static inline int tpm_seal_trusted(struct tpm_chip *chip,
-				   struct trusted_key_payload *payload,
-				   struct trusted_key_options *options)
+static inline struct tpm_chip *tpm_default_chip(void)
 {
-	return -ENODEV;
+	return NULL;
 }
-static inline int tpm_unseal_trusted(struct tpm_chip *chip,
-				     struct trusted_key_payload *payload,
-				     struct trusted_key_options *options)
+
+static inline struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip)
+{
+	return NULL;
+}
+
+static inline void tpm_put_ops(struct tpm_chip *chip)
+{
+}
+
+static inline ssize_t tpm_transmit_cmd(struct tpm_chip *chip,
+				       struct tpm_buf *buf,
+				       size_t min_rsp_body_length,
+				       const char *desc)
 {
 	return -ENODEV;
 }
-static inline struct tpm_chip *tpm_default_chip(void)
+
+static inline void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 {
-	return NULL;
 }
 #endif
 #endif
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index 4e8963a..fb42e94 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -5,3 +5,4 @@
 
 obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
 trusted-y += trusted-tpm.o
+trusted-y += trusted-tpm2.o
diff --git a/security/keys/trusted-keys/trusted-tpm2.c b/security/keys/trusted-keys/trusted-tpm2.c
new file mode 100644
index 0000000..98892ed7
--- /dev/null
+++ b/security/keys/trusted-keys/trusted-tpm2.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * Authors:
+ * Leendert van Doorn <leendert@watson.ibm.com>
+ * Dave Safford <safford@watson.ibm.com>
+ * Reiner Sailer <sailer@watson.ibm.com>
+ * Kylene Hall <kjhall@us.ibm.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * Trusted Keys code for TCG/TCPA TPM2 (trusted platform module).
+ */
+
+#include <linux/string.h>
+#include <linux/err.h>
+#include <linux/tpm.h>
+#include <linux/tpm_command.h>
+
+#include <keys/trusted-type.h>
+#include <keys/trusted_tpm.h>
+
+static struct tpm2_hash tpm2_hash_map[] = {
+	{HASH_ALGO_SHA1, TPM_ALG_SHA1},
+	{HASH_ALGO_SHA256, TPM_ALG_SHA256},
+	{HASH_ALGO_SHA384, TPM_ALG_SHA384},
+	{HASH_ALGO_SHA512, TPM_ALG_SHA512},
+	{HASH_ALGO_SM3_256, TPM_ALG_SM3_256},
+};
+
+/**
+ * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
+ *
+ * @buf: an allocated tpm_buf instance
+ * @session_handle: session handle
+ * @nonce: the session nonce, may be NULL if not used
+ * @nonce_len: the session nonce length, may be 0 if not used
+ * @attributes: the session attributes
+ * @hmac: the session HMAC or password, may be NULL if not used
+ * @hmac_len: the session HMAC or password length, maybe 0 if not used
+ */
+static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
+				 const u8 *nonce, u16 nonce_len,
+				 u8 attributes,
+				 const u8 *hmac, u16 hmac_len)
+{
+	tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
+	tpm_buf_append_u32(buf, session_handle);
+	tpm_buf_append_u16(buf, nonce_len);
+
+	if (nonce && nonce_len)
+		tpm_buf_append(buf, nonce, nonce_len);
+
+	tpm_buf_append_u8(buf, attributes);
+	tpm_buf_append_u16(buf, hmac_len);
+
+	if (hmac && hmac_len)
+		tpm_buf_append(buf, hmac, hmac_len);
+}
+
+/**
+ * tpm2_seal_trusted() - seal the payload of a trusted key
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ *
+ * Return: < 0 on error and 0 on success.
+ */
+int tpm2_seal_trusted(struct tpm_chip *chip,
+		      struct trusted_key_payload *payload,
+		      struct trusted_key_options *options)
+{
+	unsigned int blob_len;
+	struct tpm_buf buf;
+	u32 hash;
+	int i;
+	int rc;
+
+	for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
+		if (options->hash == tpm2_hash_map[i].crypto_id) {
+			hash = tpm2_hash_map[i].tpm_id;
+			break;
+		}
+	}
+
+	if (i == ARRAY_SIZE(tpm2_hash_map))
+		return -EINVAL;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, options->keyhandle);
+	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+			     NULL /* nonce */, 0,
+			     0 /* session_attributes */,
+			     options->keyauth /* hmac */,
+			     TPM_DIGEST_SIZE);
+
+	/* sensitive */
+	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
+
+	tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
+	tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
+	tpm_buf_append_u16(&buf, payload->key_len + 1);
+	tpm_buf_append(&buf, payload->key, payload->key_len);
+	tpm_buf_append_u8(&buf, payload->migratable);
+
+	/* public */
+	tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
+	tpm_buf_append_u16(&buf, TPM_ALG_KEYEDHASH);
+	tpm_buf_append_u16(&buf, hash);
+
+	/* policy */
+	if (options->policydigest_len) {
+		tpm_buf_append_u32(&buf, 0);
+		tpm_buf_append_u16(&buf, options->policydigest_len);
+		tpm_buf_append(&buf, options->policydigest,
+			       options->policydigest_len);
+	} else {
+		tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
+		tpm_buf_append_u16(&buf, 0);
+	}
+
+	/* public parameters */
+	tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+	tpm_buf_append_u16(&buf, 0);
+
+	/* outside info */
+	tpm_buf_append_u16(&buf, 0);
+
+	/* creation PCR */
+	tpm_buf_append_u32(&buf, 0);
+
+	if (buf.flags & TPM_BUF_OVERFLOW) {
+		rc = -E2BIG;
+		goto out;
+	}
+
+	rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
+	if (rc)
+		goto out;
+
+	blob_len = be32_to_cpup((__be32 *) &buf.data[TPM_HEADER_SIZE]);
+	if (blob_len > MAX_BLOB_SIZE) {
+		rc = -E2BIG;
+		goto out;
+	}
+	if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
+		rc = -EFAULT;
+		goto out;
+	}
+
+	memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
+	payload->blob_len = blob_len;
+
+out:
+	tpm_buf_destroy(&buf);
+
+	if (rc > 0) {
+		if (tpm2_rc_value(rc) == TPM2_RC_HASH)
+			rc = -EINVAL;
+		else
+			rc = -EPERM;
+	}
+
+	return rc;
+}
+
+/**
+ * tpm_seal_trusted() - seal a trusted key payload
+ * @chip:	a &struct tpm_chip instance, %NULL for the default chip
+ * @options:	authentication values and other options
+ * @payload:	the key data in clear and encrypted form
+ *
+ * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
+ * the keyring subsystem.
+ *
+ * Return: same as with tpm_transmit_cmd()
+ */
+int tpm_seal_trusted(struct tpm_chip *chip, struct trusted_key_payload *payload,
+		     struct trusted_key_options *options)
+{
+	int rc;
+
+	chip = tpm_find_get_ops(chip);
+	if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+		return -ENODEV;
+
+	rc = tpm2_seal_trusted(chip, payload, options);
+
+	tpm_put_ops(chip);
+	return rc;
+}
+
+/**
+ * tpm2_load_cmd() - execute a TPM2_Load command
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ * @blob_handle: returned blob handle
+ *
+ * Return: 0 on success.
+ *        -E2BIG on wrong payload size.
+ *        -EPERM on tpm error status.
+ *        < 0 error from tpm_transmit_cmd.
+ */
+static int tpm2_load_cmd(struct tpm_chip *chip,
+			 struct trusted_key_payload *payload,
+			 struct trusted_key_options *options,
+			 u32 *blob_handle)
+{
+	struct tpm_buf buf;
+	unsigned int private_len;
+	unsigned int public_len;
+	unsigned int blob_len;
+	int rc;
+
+	private_len = be16_to_cpup((__be16 *) &payload->blob[0]);
+	if (private_len > (payload->blob_len - 2))
+		return -E2BIG;
+
+	public_len = be16_to_cpup((__be16 *) &payload->blob[2 + private_len]);
+	blob_len = private_len + public_len + 4;
+	if (blob_len > payload->blob_len)
+		return -E2BIG;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, options->keyhandle);
+	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+			     NULL /* nonce */, 0,
+			     0 /* session_attributes */,
+			     options->keyauth /* hmac */,
+			     TPM_DIGEST_SIZE);
+
+	tpm_buf_append(&buf, payload->blob, blob_len);
+
+	if (buf.flags & TPM_BUF_OVERFLOW) {
+		rc = -E2BIG;
+		goto out;
+	}
+
+	rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
+	if (!rc)
+		*blob_handle = be32_to_cpup(
+			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
+
+out:
+	tpm_buf_destroy(&buf);
+
+	if (rc > 0)
+		rc = -EPERM;
+
+	return rc;
+}
+
+/**
+ * tpm2_unseal_cmd() - execute a TPM2_Unload command
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ * @blob_handle: blob handle
+ *
+ * Return: 0 on success
+ *         -EPERM on tpm error status
+ *         < 0 error from tpm_transmit_cmd
+ */
+static int tpm2_unseal_cmd(struct tpm_chip *chip,
+			   struct trusted_key_payload *payload,
+			   struct trusted_key_options *options,
+			   u32 blob_handle)
+{
+	struct tpm_buf buf;
+	u16 data_len;
+	u8 *data;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&buf, blob_handle);
+	tpm2_buf_append_auth(&buf,
+			     options->policyhandle ?
+			     options->policyhandle : TPM2_RS_PW,
+			     NULL /* nonce */, 0,
+			     TPM2_SA_CONTINUE_SESSION,
+			     options->blobauth /* hmac */,
+			     TPM_DIGEST_SIZE);
+
+	rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
+	if (rc > 0)
+		rc = -EPERM;
+
+	if (!rc) {
+		data_len = be16_to_cpup(
+			(__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
+		if (data_len < MIN_KEY_SIZE ||  data_len > MAX_KEY_SIZE + 1) {
+			rc = -EFAULT;
+			goto out;
+		}
+
+		if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
+			rc = -EFAULT;
+			goto out;
+		}
+		data = &buf.data[TPM_HEADER_SIZE + 6];
+
+		memcpy(payload->key, data, data_len - 1);
+		payload->key_len = data_len - 1;
+		payload->migratable = data[data_len - 1];
+	}
+
+out:
+	tpm_buf_destroy(&buf);
+	return rc;
+}
+
+/**
+ * tpm2_unseal_trusted() - unseal the payload of a trusted key
+ *
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ *
+ * Return: Same as with tpm_transmit_cmd.
+ */
+int tpm2_unseal_trusted(struct tpm_chip *chip,
+			struct trusted_key_payload *payload,
+			struct trusted_key_options *options)
+{
+	u32 blob_handle;
+	int rc;
+
+	rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
+	if (rc)
+		return rc;
+
+	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+	tpm2_flush_context(chip, blob_handle);
+	return rc;
+}
+
+/**
+ * tpm_unseal_trusted() - unseal a trusted key
+ * @chip:	a &struct tpm_chip instance, %NULL for the default chip
+ * @options:	authentication values and other options
+ * @payload:	the key data in clear and encrypted form
+ *
+ * Note: only TPM 2.0 chip are supported. TPM 1.x implementation is located in
+ * the keyring subsystem.
+ *
+ * Return: same as with tpm_transmit_cmd()
+ */
+int tpm_unseal_trusted(struct tpm_chip *chip,
+		       struct trusted_key_payload *payload,
+		       struct trusted_key_options *options)
+{
+	int rc;
+
+	chip = tpm_find_get_ops(chip);
+	if (!chip || !(chip->flags & TPM_CHIP_FLAG_TPM2))
+		return -ENODEV;
+
+	rc = tpm2_unseal_trusted(chip, payload, options);
+
+	tpm_put_ops(chip);
+
+	return rc;
+}
-- 
2.7.4


^ permalink raw reply related

* [RFC/RFT v4 3/5] KEYS: trusted: create trusted keys subsystem
From: Sumit Garg @ 2019-08-13  7:53 UTC (permalink / raw)
  To: keyrings, linux-integrity, linux-crypto, linux-security-module
  Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
	arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
	daniel.thompson, linux-kernel, tee-dev, Sumit Garg
In-Reply-To: <1565682784-10234-1-git-send-email-sumit.garg@linaro.org>

Move existing code to trusted keys subsystem. Also, rename files with
"tpm" as suffix which provides the underlying implementation.

Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 crypto/asymmetric_keys/asym_tpm.c                       | 2 +-
 include/keys/{trusted.h => trusted_tpm.h}               | 4 ++--
 security/keys/Makefile                                  | 2 +-
 security/keys/trusted-keys/Makefile                     | 7 +++++++
 security/keys/{trusted.c => trusted-keys/trusted-tpm.c} | 2 +-
 5 files changed, 12 insertions(+), 5 deletions(-)
 rename include/keys/{trusted.h => trusted_tpm.h} (98%)
 create mode 100644 security/keys/trusted-keys/Makefile
 rename security/keys/{trusted.c => trusted-keys/trusted-tpm.c} (99%)

diff --git a/crypto/asymmetric_keys/asym_tpm.c b/crypto/asymmetric_keys/asym_tpm.c
index 76d2ce3..ec3f309 100644
--- a/crypto/asymmetric_keys/asym_tpm.c
+++ b/crypto/asymmetric_keys/asym_tpm.c
@@ -13,7 +13,7 @@
 #include <crypto/sha.h>
 #include <asm/unaligned.h>
 #include <keys/asymmetric-subtype.h>
-#include <keys/trusted.h>
+#include <keys/trusted_tpm.h>
 #include <crypto/asym_tpm_subtype.h>
 #include <crypto/public_key.h>
 
diff --git a/include/keys/trusted.h b/include/keys/trusted_tpm.h
similarity index 98%
rename from include/keys/trusted.h
rename to include/keys/trusted_tpm.h
index 29e3e9b..9bdf5f4 100644
--- a/include/keys/trusted.h
+++ b/include/keys/trusted_tpm.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __TRUSTED_KEY_H
-#define __TRUSTED_KEY_H
+#ifndef __TRUSTED_TPM_H
+#define __TRUSTED_TPM_H
 
 /* implementation specific TPM constants */
 #define MAX_BUF_SIZE			1024
diff --git a/security/keys/Makefile b/security/keys/Makefile
index 9cef540..074f275 100644
--- a/security/keys/Makefile
+++ b/security/keys/Makefile
@@ -28,5 +28,5 @@ obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += keyctl_pkey.o
 # Key types
 #
 obj-$(CONFIG_BIG_KEYS) += big_key.o
-obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
+obj-$(CONFIG_TRUSTED_KEYS) += trusted-keys/
 obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
new file mode 100644
index 0000000..4e8963a
--- /dev/null
+++ b/security/keys/trusted-keys/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for trusted keys
+#
+
+obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
+trusted-y += trusted-tpm.o
diff --git a/security/keys/trusted.c b/security/keys/trusted-keys/trusted-tpm.c
similarity index 99%
rename from security/keys/trusted.c
rename to security/keys/trusted-keys/trusted-tpm.c
index f7134d6..66687e7 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted-keys/trusted-tpm.c
@@ -27,7 +27,7 @@
 #include <linux/tpm.h>
 #include <linux/tpm_command.h>
 
-#include <keys/trusted.h>
+#include <keys/trusted_tpm.h>
 
 static const char hmac_alg[] = "hmac(sha1)";
 static const char hash_alg[] = "sha1";
-- 
2.7.4


^ permalink raw reply related

* [RFC/RFT v4 2/5] KEYS: trusted: use common tpm_buf for TPM1.x code
From: Sumit Garg @ 2019-08-13  7:53 UTC (permalink / raw)
  To: keyrings, linux-integrity, linux-crypto, linux-security-module
  Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
	arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
	daniel.thompson, linux-kernel, tee-dev, Sumit Garg
In-Reply-To: <1565682784-10234-1-git-send-email-sumit.garg@linaro.org>

Utilize common heap based tpm_buf code for TPM1.x trusted keys rather
than using stack based tpm1_buf code. Also, remove tpm1_buf code.

Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 include/keys/trusted.h  | 37 +------------------
 security/keys/trusted.c | 98 ++++++++++++++++++++++---------------------------
 2 files changed, 44 insertions(+), 91 deletions(-)

diff --git a/include/keys/trusted.h b/include/keys/trusted.h
index 841ae11..29e3e9b 100644
--- a/include/keys/trusted.h
+++ b/include/keys/trusted.h
@@ -5,10 +5,6 @@
 /* implementation specific TPM constants */
 #define MAX_BUF_SIZE			1024
 #define TPM_GETRANDOM_SIZE		14
-#define TPM_OSAP_SIZE			36
-#define TPM_OIAP_SIZE			10
-#define TPM_SEAL_SIZE			87
-#define TPM_UNSEAL_SIZE			104
 #define TPM_SIZE_OFFSET			2
 #define TPM_RETURN_OFFSET		6
 #define TPM_DATA_OFFSET			10
@@ -17,13 +13,6 @@
 #define LOAD32N(buffer, offset)	(*(uint32_t *)&buffer[offset])
 #define LOAD16(buffer, offset)	(ntohs(*(uint16_t *)&buffer[offset]))
 
-struct tpm1_buf {
-	int len;
-	unsigned char data[MAX_BUF_SIZE];
-};
-
-#define INIT_BUF(tb) (tb->len = 0)
-
 struct osapsess {
 	uint32_t handle;
 	unsigned char secret[SHA1_DIGEST_SIZE];
@@ -46,7 +35,7 @@ int TSS_checkhmac1(unsigned char *buffer,
 			  unsigned int keylen, ...);
 
 int trusted_tpm_send(unsigned char *cmd, size_t buflen);
-int oiap(struct tpm1_buf *tb, uint32_t *handle, unsigned char *nonce);
+int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce);
 
 #define TPM_DEBUG 0
 
@@ -109,28 +98,4 @@ static inline void dump_tpm_buf(unsigned char *buf)
 {
 }
 #endif
-
-static inline void store8(struct tpm1_buf *buf, const unsigned char value)
-{
-	buf->data[buf->len++] = value;
-}
-
-static inline void store16(struct tpm1_buf *buf, const uint16_t value)
-{
-	*(uint16_t *) & buf->data[buf->len] = htons(value);
-	buf->len += sizeof value;
-}
-
-static inline void store32(struct tpm1_buf *buf, const uint32_t value)
-{
-	*(uint32_t *) & buf->data[buf->len] = htonl(value);
-	buf->len += sizeof value;
-}
-
-static inline void storebytes(struct tpm1_buf *buf, const unsigned char *in,
-			      const int len)
-{
-	memcpy(buf->data + buf->len, in, len);
-	buf->len += len;
-}
 #endif
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 0736671..f7134d6 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -395,7 +395,7 @@ static int pcrlock(const int pcrnum)
 /*
  * Create an object specific authorisation protocol (OSAP) session
  */
-static int osap(struct tpm1_buf *tb, struct osapsess *s,
+static int osap(struct tpm_buf *tb, struct osapsess *s,
 		const unsigned char *key, uint16_t type, uint32_t handle)
 {
 	unsigned char enonce[TPM_NONCE_SIZE];
@@ -406,13 +406,10 @@ static int osap(struct tpm1_buf *tb, struct osapsess *s,
 	if (ret != TPM_NONCE_SIZE)
 		return ret;
 
-	INIT_BUF(tb);
-	store16(tb, TPM_TAG_RQU_COMMAND);
-	store32(tb, TPM_OSAP_SIZE);
-	store32(tb, TPM_ORD_OSAP);
-	store16(tb, type);
-	store32(tb, handle);
-	storebytes(tb, ononce, TPM_NONCE_SIZE);
+	tpm_buf_reset(tb, TPM_TAG_RQU_COMMAND, TPM_ORD_OSAP);
+	tpm_buf_append_u16(tb, type);
+	tpm_buf_append_u32(tb, handle);
+	tpm_buf_append(tb, ononce, TPM_NONCE_SIZE);
 
 	ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
 	if (ret < 0)
@@ -430,17 +427,14 @@ static int osap(struct tpm1_buf *tb, struct osapsess *s,
 /*
  * Create an object independent authorisation protocol (oiap) session
  */
-int oiap(struct tpm1_buf *tb, uint32_t *handle, unsigned char *nonce)
+int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
 {
 	int ret;
 
 	if (!chip)
 		return -ENODEV;
 
-	INIT_BUF(tb);
-	store16(tb, TPM_TAG_RQU_COMMAND);
-	store32(tb, TPM_OIAP_SIZE);
-	store32(tb, TPM_ORD_OIAP);
+	tpm_buf_reset(tb, TPM_TAG_RQU_COMMAND, TPM_ORD_OIAP);
 	ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
 	if (ret < 0)
 		return ret;
@@ -464,7 +458,7 @@ struct tpm_digests {
  * Have the TPM seal(encrypt) the trusted key, possibly based on
  * Platform Configuration Registers (PCRs). AUTH1 for sealing key.
  */
-static int tpm_seal(struct tpm1_buf *tb, uint16_t keytype,
+static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
 		    uint32_t keyhandle, const unsigned char *keyauth,
 		    const unsigned char *data, uint32_t datalen,
 		    unsigned char *blob, uint32_t *bloblen,
@@ -535,20 +529,17 @@ static int tpm_seal(struct tpm1_buf *tb, uint16_t keytype,
 		goto out;
 
 	/* build and send the TPM request packet */
-	INIT_BUF(tb);
-	store16(tb, TPM_TAG_RQU_AUTH1_COMMAND);
-	store32(tb, TPM_SEAL_SIZE + pcrinfosize + datalen);
-	store32(tb, TPM_ORD_SEAL);
-	store32(tb, keyhandle);
-	storebytes(tb, td->encauth, SHA1_DIGEST_SIZE);
-	store32(tb, pcrinfosize);
-	storebytes(tb, pcrinfo, pcrinfosize);
-	store32(tb, datalen);
-	storebytes(tb, data, datalen);
-	store32(tb, sess.handle);
-	storebytes(tb, td->nonceodd, TPM_NONCE_SIZE);
-	store8(tb, cont);
-	storebytes(tb, td->pubauth, SHA1_DIGEST_SIZE);
+	tpm_buf_reset(tb, TPM_TAG_RQU_AUTH1_COMMAND, TPM_ORD_SEAL);
+	tpm_buf_append_u32(tb, keyhandle);
+	tpm_buf_append(tb, td->encauth, SHA1_DIGEST_SIZE);
+	tpm_buf_append_u32(tb, pcrinfosize);
+	tpm_buf_append(tb, pcrinfo, pcrinfosize);
+	tpm_buf_append_u32(tb, datalen);
+	tpm_buf_append(tb, data, datalen);
+	tpm_buf_append_u32(tb, sess.handle);
+	tpm_buf_append(tb, td->nonceodd, TPM_NONCE_SIZE);
+	tpm_buf_append_u8(tb, cont);
+	tpm_buf_append(tb, td->pubauth, SHA1_DIGEST_SIZE);
 
 	ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
 	if (ret < 0)
@@ -579,7 +570,7 @@ static int tpm_seal(struct tpm1_buf *tb, uint16_t keytype,
 /*
  * use the AUTH2_COMMAND form of unseal, to authorize both key and blob
  */
-static int tpm_unseal(struct tpm1_buf *tb,
+static int tpm_unseal(struct tpm_buf *tb,
 		      uint32_t keyhandle, const unsigned char *keyauth,
 		      const unsigned char *blob, int bloblen,
 		      const unsigned char *blobauth,
@@ -628,20 +619,17 @@ static int tpm_unseal(struct tpm1_buf *tb,
 		return ret;
 
 	/* build and send TPM request packet */
-	INIT_BUF(tb);
-	store16(tb, TPM_TAG_RQU_AUTH2_COMMAND);
-	store32(tb, TPM_UNSEAL_SIZE + bloblen);
-	store32(tb, TPM_ORD_UNSEAL);
-	store32(tb, keyhandle);
-	storebytes(tb, blob, bloblen);
-	store32(tb, authhandle1);
-	storebytes(tb, nonceodd, TPM_NONCE_SIZE);
-	store8(tb, cont);
-	storebytes(tb, authdata1, SHA1_DIGEST_SIZE);
-	store32(tb, authhandle2);
-	storebytes(tb, nonceodd, TPM_NONCE_SIZE);
-	store8(tb, cont);
-	storebytes(tb, authdata2, SHA1_DIGEST_SIZE);
+	tpm_buf_reset(tb, TPM_TAG_RQU_AUTH2_COMMAND, TPM_ORD_UNSEAL);
+	tpm_buf_append_u32(tb, keyhandle);
+	tpm_buf_append(tb, blob, bloblen);
+	tpm_buf_append_u32(tb, authhandle1);
+	tpm_buf_append(tb, nonceodd, TPM_NONCE_SIZE);
+	tpm_buf_append_u8(tb, cont);
+	tpm_buf_append(tb, authdata1, SHA1_DIGEST_SIZE);
+	tpm_buf_append_u32(tb, authhandle2);
+	tpm_buf_append(tb, nonceodd, TPM_NONCE_SIZE);
+	tpm_buf_append_u8(tb, cont);
+	tpm_buf_append(tb, authdata2, SHA1_DIGEST_SIZE);
 
 	ret = trusted_tpm_send(tb->data, MAX_BUF_SIZE);
 	if (ret < 0) {
@@ -670,23 +658,23 @@ static int tpm_unseal(struct tpm1_buf *tb,
 static int key_seal(struct trusted_key_payload *p,
 		    struct trusted_key_options *o)
 {
-	struct tpm1_buf *tb;
+	struct tpm_buf tb;
 	int ret;
 
-	tb = kzalloc(sizeof *tb, GFP_KERNEL);
-	if (!tb)
-		return -ENOMEM;
+	ret = tpm_buf_init(&tb, 0, 0);
+	if (ret)
+		return ret;
 
 	/* include migratable flag at end of sealed key */
 	p->key[p->key_len] = p->migratable;
 
-	ret = tpm_seal(tb, o->keytype, o->keyhandle, o->keyauth,
+	ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth,
 		       p->key, p->key_len + 1, p->blob, &p->blob_len,
 		       o->blobauth, o->pcrinfo, o->pcrinfo_len);
 	if (ret < 0)
 		pr_info("trusted_key: srkseal failed (%d)\n", ret);
 
-	kzfree(tb);
+	tpm_buf_destroy(&tb);
 	return ret;
 }
 
@@ -696,14 +684,14 @@ static int key_seal(struct trusted_key_payload *p,
 static int key_unseal(struct trusted_key_payload *p,
 		      struct trusted_key_options *o)
 {
-	struct tpm1_buf *tb;
+	struct tpm_buf tb;
 	int ret;
 
-	tb = kzalloc(sizeof *tb, GFP_KERNEL);
-	if (!tb)
-		return -ENOMEM;
+	ret = tpm_buf_init(&tb, 0, 0);
+	if (ret)
+		return ret;
 
-	ret = tpm_unseal(tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
+	ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
 			 o->blobauth, p->key, &p->key_len);
 	if (ret < 0)
 		pr_info("trusted_key: srkunseal failed (%d)\n", ret);
@@ -711,7 +699,7 @@ static int key_unseal(struct trusted_key_payload *p,
 		/* pull migratable flag out of sealed key */
 		p->migratable = p->key[--p->key_len];
 
-	kzfree(tb);
+	tpm_buf_destroy(&tb);
 	return ret;
 }
 
-- 
2.7.4


^ permalink raw reply related

* [RFC/RFT v4 1/5] tpm: move tpm_buf code to include/linux/
From: Sumit Garg @ 2019-08-13  7:53 UTC (permalink / raw)
  To: keyrings, linux-integrity, linux-crypto, linux-security-module
  Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
	arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
	daniel.thompson, linux-kernel, tee-dev, Sumit Garg
In-Reply-To: <1565682784-10234-1-git-send-email-sumit.garg@linaro.org>

Move tpm_buf code to common include/linux/tpm.h header so that it can
be reused via other subsystems like trusted keys etc.

Also rename trusted keys TPM 1.x buffer implementation to tpm1_buf to
avoid any compilation errors.

Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/char/tpm/tpm.h  | 214 ------------------------------------------------
 include/keys/trusted.h  |  12 +--
 include/linux/tpm.h     | 214 ++++++++++++++++++++++++++++++++++++++++++++++++
 security/keys/trusted.c |  12 +--
 4 files changed, 226 insertions(+), 226 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index e503ffc..0b5498a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -25,7 +25,6 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/tpm.h>
-#include <linux/highmem.h>
 #include <linux/tpm_eventlog.h>
 
 #ifdef CONFIG_X86
@@ -58,123 +57,6 @@ enum tpm_addr {
 #define TPM_ERR_DISABLED        0x7
 #define TPM_ERR_INVALID_POSTINIT 38
 
-#define TPM_HEADER_SIZE		10
-
-enum tpm2_const {
-	TPM2_PLATFORM_PCR       =     24,
-	TPM2_PCR_SELECT_MIN     = ((TPM2_PLATFORM_PCR + 7) / 8),
-};
-
-enum tpm2_timeouts {
-	TPM2_TIMEOUT_A          =    750,
-	TPM2_TIMEOUT_B          =   2000,
-	TPM2_TIMEOUT_C          =    200,
-	TPM2_TIMEOUT_D          =     30,
-	TPM2_DURATION_SHORT     =     20,
-	TPM2_DURATION_MEDIUM    =    750,
-	TPM2_DURATION_LONG      =   2000,
-	TPM2_DURATION_LONG_LONG = 300000,
-	TPM2_DURATION_DEFAULT   = 120000,
-};
-
-enum tpm2_structures {
-	TPM2_ST_NO_SESSIONS	= 0x8001,
-	TPM2_ST_SESSIONS	= 0x8002,
-};
-
-/* Indicates from what layer of the software stack the error comes from */
-#define TSS2_RC_LAYER_SHIFT	 16
-#define TSS2_RESMGR_TPM_RC_LAYER (11 << TSS2_RC_LAYER_SHIFT)
-
-enum tpm2_return_codes {
-	TPM2_RC_SUCCESS		= 0x0000,
-	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
-	TPM2_RC_HANDLE		= 0x008B,
-	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
-	TPM2_RC_FAILURE		= 0x0101,
-	TPM2_RC_DISABLED	= 0x0120,
-	TPM2_RC_COMMAND_CODE    = 0x0143,
-	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
-	TPM2_RC_REFERENCE_H0	= 0x0910,
-	TPM2_RC_RETRY		= 0x0922,
-};
-
-enum tpm2_command_codes {
-	TPM2_CC_FIRST		        = 0x011F,
-	TPM2_CC_HIERARCHY_CONTROL       = 0x0121,
-	TPM2_CC_HIERARCHY_CHANGE_AUTH   = 0x0129,
-	TPM2_CC_CREATE_PRIMARY          = 0x0131,
-	TPM2_CC_SEQUENCE_COMPLETE       = 0x013E,
-	TPM2_CC_SELF_TEST	        = 0x0143,
-	TPM2_CC_STARTUP		        = 0x0144,
-	TPM2_CC_SHUTDOWN	        = 0x0145,
-	TPM2_CC_NV_READ                 = 0x014E,
-	TPM2_CC_CREATE		        = 0x0153,
-	TPM2_CC_LOAD		        = 0x0157,
-	TPM2_CC_SEQUENCE_UPDATE         = 0x015C,
-	TPM2_CC_UNSEAL		        = 0x015E,
-	TPM2_CC_CONTEXT_LOAD	        = 0x0161,
-	TPM2_CC_CONTEXT_SAVE	        = 0x0162,
-	TPM2_CC_FLUSH_CONTEXT	        = 0x0165,
-	TPM2_CC_VERIFY_SIGNATURE        = 0x0177,
-	TPM2_CC_GET_CAPABILITY	        = 0x017A,
-	TPM2_CC_GET_RANDOM	        = 0x017B,
-	TPM2_CC_PCR_READ	        = 0x017E,
-	TPM2_CC_PCR_EXTEND	        = 0x0182,
-	TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
-	TPM2_CC_HASH_SEQUENCE_START     = 0x0186,
-	TPM2_CC_CREATE_LOADED           = 0x0191,
-	TPM2_CC_LAST		        = 0x0193, /* Spec 1.36 */
-};
-
-enum tpm2_permanent_handles {
-	TPM2_RS_PW		= 0x40000009,
-};
-
-enum tpm2_capabilities {
-	TPM2_CAP_HANDLES	= 1,
-	TPM2_CAP_COMMANDS	= 2,
-	TPM2_CAP_PCRS		= 5,
-	TPM2_CAP_TPM_PROPERTIES = 6,
-};
-
-enum tpm2_properties {
-	TPM_PT_TOTAL_COMMANDS	= 0x0129,
-};
-
-enum tpm2_startup_types {
-	TPM2_SU_CLEAR	= 0x0000,
-	TPM2_SU_STATE	= 0x0001,
-};
-
-enum tpm2_cc_attrs {
-	TPM2_CC_ATTR_CHANDLES	= 25,
-	TPM2_CC_ATTR_RHANDLE	= 28,
-};
-
-#define TPM_VID_INTEL    0x8086
-#define TPM_VID_WINBOND  0x1050
-#define TPM_VID_STM      0x104A
-
-enum tpm_chip_flags {
-	TPM_CHIP_FLAG_TPM2		= BIT(1),
-	TPM_CHIP_FLAG_IRQ		= BIT(2),
-	TPM_CHIP_FLAG_VIRTUAL		= BIT(3),
-	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
-	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
-};
-
-#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
-
-struct tpm_header {
-	__be16 tag;
-	__be32 length;
-	union {
-		__be32 ordinal;
-		__be32 return_code;
-	};
-} __packed;
-
 #define TPM_TAG_RQU_COMMAND 193
 
 struct	stclear_flags_t {
@@ -274,102 +156,6 @@ enum tpm_sub_capabilities {
  * compiler warnings about stack frame size. */
 #define TPM_MAX_RNG_DATA	128
 
-/* A string buffer type for constructing TPM commands. This is based on the
- * ideas of string buffer code in security/keys/trusted.h but is heap based
- * in order to keep the stack usage minimal.
- */
-
-enum tpm_buf_flags {
-	TPM_BUF_OVERFLOW	= BIT(0),
-};
-
-struct tpm_buf {
-	struct page *data_page;
-	unsigned int flags;
-	u8 *data;
-};
-
-static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
-{
-	struct tpm_header *head = (struct tpm_header *)buf->data;
-
-	head->tag = cpu_to_be16(tag);
-	head->length = cpu_to_be32(sizeof(*head));
-	head->ordinal = cpu_to_be32(ordinal);
-}
-
-static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
-{
-	buf->data_page = alloc_page(GFP_HIGHUSER);
-	if (!buf->data_page)
-		return -ENOMEM;
-
-	buf->flags = 0;
-	buf->data = kmap(buf->data_page);
-	tpm_buf_reset(buf, tag, ordinal);
-	return 0;
-}
-
-static inline void tpm_buf_destroy(struct tpm_buf *buf)
-{
-	kunmap(buf->data_page);
-	__free_page(buf->data_page);
-}
-
-static inline u32 tpm_buf_length(struct tpm_buf *buf)
-{
-	struct tpm_header *head = (struct tpm_header *)buf->data;
-
-	return be32_to_cpu(head->length);
-}
-
-static inline u16 tpm_buf_tag(struct tpm_buf *buf)
-{
-	struct tpm_header *head = (struct tpm_header *)buf->data;
-
-	return be16_to_cpu(head->tag);
-}
-
-static inline void tpm_buf_append(struct tpm_buf *buf,
-				  const unsigned char *new_data,
-				  unsigned int new_len)
-{
-	struct tpm_header *head = (struct tpm_header *)buf->data;
-	u32 len = tpm_buf_length(buf);
-
-	/* Return silently if overflow has already happened. */
-	if (buf->flags & TPM_BUF_OVERFLOW)
-		return;
-
-	if ((len + new_len) > PAGE_SIZE) {
-		WARN(1, "tpm_buf: overflow\n");
-		buf->flags |= TPM_BUF_OVERFLOW;
-		return;
-	}
-
-	memcpy(&buf->data[len], new_data, new_len);
-	head->length = cpu_to_be32(len + new_len);
-}
-
-static inline void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
-{
-	tpm_buf_append(buf, &value, 1);
-}
-
-static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
-{
-	__be16 value2 = cpu_to_be16(value);
-
-	tpm_buf_append(buf, (u8 *) &value2, 2);
-}
-
-static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
-{
-	__be32 value2 = cpu_to_be32(value);
-
-	tpm_buf_append(buf, (u8 *) &value2, 4);
-}
-
 extern struct class *tpm_class;
 extern struct class *tpmrm_class;
 extern dev_t tpm_devt;
diff --git a/include/keys/trusted.h b/include/keys/trusted.h
index 0071298..841ae11 100644
--- a/include/keys/trusted.h
+++ b/include/keys/trusted.h
@@ -17,7 +17,7 @@
 #define LOAD32N(buffer, offset)	(*(uint32_t *)&buffer[offset])
 #define LOAD16(buffer, offset)	(ntohs(*(uint16_t *)&buffer[offset]))
 
-struct tpm_buf {
+struct tpm1_buf {
 	int len;
 	unsigned char data[MAX_BUF_SIZE];
 };
@@ -46,7 +46,7 @@ int TSS_checkhmac1(unsigned char *buffer,
 			  unsigned int keylen, ...);
 
 int trusted_tpm_send(unsigned char *cmd, size_t buflen);
-int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce);
+int oiap(struct tpm1_buf *tb, uint32_t *handle, unsigned char *nonce);
 
 #define TPM_DEBUG 0
 
@@ -110,24 +110,24 @@ static inline void dump_tpm_buf(unsigned char *buf)
 }
 #endif
 
-static inline void store8(struct tpm_buf *buf, const unsigned char value)
+static inline void store8(struct tpm1_buf *buf, const unsigned char value)
 {
 	buf->data[buf->len++] = value;
 }
 
-static inline void store16(struct tpm_buf *buf, const uint16_t value)
+static inline void store16(struct tpm1_buf *buf, const uint16_t value)
 {
 	*(uint16_t *) & buf->data[buf->len] = htons(value);
 	buf->len += sizeof value;
 }
 
-static inline void store32(struct tpm_buf *buf, const uint32_t value)
+static inline void store32(struct tpm1_buf *buf, const uint32_t value)
 {
 	*(uint32_t *) & buf->data[buf->len] = htonl(value);
 	buf->len += sizeof value;
 }
 
-static inline void storebytes(struct tpm_buf *buf, const unsigned char *in,
+static inline void storebytes(struct tpm1_buf *buf, const unsigned char *in,
 			      const int len)
 {
 	memcpy(buf->data + buf->len, in, len);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 53c0ea9..130c167 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -21,6 +21,7 @@
 #include <linux/acpi.h>
 #include <linux/cdev.h>
 #include <linux/fs.h>
+#include <linux/highmem.h>
 #include <crypto/hash_info.h>
 
 #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
@@ -161,6 +162,219 @@ struct tpm_chip {
 	int locality;
 };
 
+#define TPM_HEADER_SIZE		10
+
+enum tpm2_const {
+	TPM2_PLATFORM_PCR       =     24,
+	TPM2_PCR_SELECT_MIN     = ((TPM2_PLATFORM_PCR + 7) / 8),
+};
+
+enum tpm2_timeouts {
+	TPM2_TIMEOUT_A          =    750,
+	TPM2_TIMEOUT_B          =   2000,
+	TPM2_TIMEOUT_C          =    200,
+	TPM2_TIMEOUT_D          =     30,
+	TPM2_DURATION_SHORT     =     20,
+	TPM2_DURATION_MEDIUM    =    750,
+	TPM2_DURATION_LONG      =   2000,
+	TPM2_DURATION_LONG_LONG = 300000,
+	TPM2_DURATION_DEFAULT   = 120000,
+};
+
+enum tpm2_structures {
+	TPM2_ST_NO_SESSIONS	= 0x8001,
+	TPM2_ST_SESSIONS	= 0x8002,
+};
+
+/* Indicates from what layer of the software stack the error comes from */
+#define TSS2_RC_LAYER_SHIFT	 16
+#define TSS2_RESMGR_TPM_RC_LAYER (11 << TSS2_RC_LAYER_SHIFT)
+
+enum tpm2_return_codes {
+	TPM2_RC_SUCCESS		= 0x0000,
+	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
+	TPM2_RC_HANDLE		= 0x008B,
+	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
+	TPM2_RC_FAILURE		= 0x0101,
+	TPM2_RC_DISABLED	= 0x0120,
+	TPM2_RC_COMMAND_CODE    = 0x0143,
+	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
+	TPM2_RC_REFERENCE_H0	= 0x0910,
+	TPM2_RC_RETRY		= 0x0922,
+};
+
+enum tpm2_command_codes {
+	TPM2_CC_FIRST		        = 0x011F,
+	TPM2_CC_HIERARCHY_CONTROL       = 0x0121,
+	TPM2_CC_HIERARCHY_CHANGE_AUTH   = 0x0129,
+	TPM2_CC_CREATE_PRIMARY          = 0x0131,
+	TPM2_CC_SEQUENCE_COMPLETE       = 0x013E,
+	TPM2_CC_SELF_TEST	        = 0x0143,
+	TPM2_CC_STARTUP		        = 0x0144,
+	TPM2_CC_SHUTDOWN	        = 0x0145,
+	TPM2_CC_NV_READ                 = 0x014E,
+	TPM2_CC_CREATE		        = 0x0153,
+	TPM2_CC_LOAD		        = 0x0157,
+	TPM2_CC_SEQUENCE_UPDATE         = 0x015C,
+	TPM2_CC_UNSEAL		        = 0x015E,
+	TPM2_CC_CONTEXT_LOAD	        = 0x0161,
+	TPM2_CC_CONTEXT_SAVE	        = 0x0162,
+	TPM2_CC_FLUSH_CONTEXT	        = 0x0165,
+	TPM2_CC_VERIFY_SIGNATURE        = 0x0177,
+	TPM2_CC_GET_CAPABILITY	        = 0x017A,
+	TPM2_CC_GET_RANDOM	        = 0x017B,
+	TPM2_CC_PCR_READ	        = 0x017E,
+	TPM2_CC_PCR_EXTEND	        = 0x0182,
+	TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
+	TPM2_CC_HASH_SEQUENCE_START     = 0x0186,
+	TPM2_CC_CREATE_LOADED           = 0x0191,
+	TPM2_CC_LAST		        = 0x0193, /* Spec 1.36 */
+};
+
+enum tpm2_permanent_handles {
+	TPM2_RS_PW		= 0x40000009,
+};
+
+enum tpm2_capabilities {
+	TPM2_CAP_HANDLES	= 1,
+	TPM2_CAP_COMMANDS	= 2,
+	TPM2_CAP_PCRS		= 5,
+	TPM2_CAP_TPM_PROPERTIES = 6,
+};
+
+enum tpm2_properties {
+	TPM_PT_TOTAL_COMMANDS	= 0x0129,
+};
+
+enum tpm2_startup_types {
+	TPM2_SU_CLEAR	= 0x0000,
+	TPM2_SU_STATE	= 0x0001,
+};
+
+enum tpm2_cc_attrs {
+	TPM2_CC_ATTR_CHANDLES	= 25,
+	TPM2_CC_ATTR_RHANDLE	= 28,
+};
+
+#define TPM_VID_INTEL    0x8086
+#define TPM_VID_WINBOND  0x1050
+#define TPM_VID_STM      0x104A
+
+enum tpm_chip_flags {
+	TPM_CHIP_FLAG_TPM2		= BIT(1),
+	TPM_CHIP_FLAG_IRQ		= BIT(2),
+	TPM_CHIP_FLAG_VIRTUAL		= BIT(3),
+	TPM_CHIP_FLAG_HAVE_TIMEOUTS	= BIT(4),
+	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
+};
+
+#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
+
+struct tpm_header {
+	__be16 tag;
+	__be32 length;
+	union {
+		__be32 ordinal;
+		__be32 return_code;
+	};
+} __packed;
+
+/* A string buffer type for constructing TPM commands. This is based on the
+ * ideas of string buffer code in security/keys/trusted.h but is heap based
+ * in order to keep the stack usage minimal.
+ */
+
+enum tpm_buf_flags {
+	TPM_BUF_OVERFLOW	= BIT(0),
+};
+
+struct tpm_buf {
+	struct page *data_page;
+	unsigned int flags;
+	u8 *data;
+};
+
+static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+	struct tpm_header *head = (struct tpm_header *)buf->data;
+
+	head->tag = cpu_to_be16(tag);
+	head->length = cpu_to_be32(sizeof(*head));
+	head->ordinal = cpu_to_be32(ordinal);
+}
+
+static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+{
+	buf->data_page = alloc_page(GFP_HIGHUSER);
+	if (!buf->data_page)
+		return -ENOMEM;
+
+	buf->flags = 0;
+	buf->data = kmap(buf->data_page);
+	tpm_buf_reset(buf, tag, ordinal);
+	return 0;
+}
+
+static inline void tpm_buf_destroy(struct tpm_buf *buf)
+{
+	kunmap(buf->data_page);
+	__free_page(buf->data_page);
+}
+
+static inline u32 tpm_buf_length(struct tpm_buf *buf)
+{
+	struct tpm_header *head = (struct tpm_header *)buf->data;
+
+	return be32_to_cpu(head->length);
+}
+
+static inline u16 tpm_buf_tag(struct tpm_buf *buf)
+{
+	struct tpm_header *head = (struct tpm_header *)buf->data;
+
+	return be16_to_cpu(head->tag);
+}
+
+static inline void tpm_buf_append(struct tpm_buf *buf,
+				  const unsigned char *new_data,
+				  unsigned int new_len)
+{
+	struct tpm_header *head = (struct tpm_header *)buf->data;
+	u32 len = tpm_buf_length(buf);
+
+	/* Return silently if overflow has already happened. */
+	if (buf->flags & TPM_BUF_OVERFLOW)
+		return;
+
+	if ((len + new_len) > PAGE_SIZE) {
+		WARN(1, "tpm_buf: overflow\n");
+		buf->flags |= TPM_BUF_OVERFLOW;
+		return;
+	}
+
+	memcpy(&buf->data[len], new_data, new_len);
+	head->length = cpu_to_be32(len + new_len);
+}
+
+static inline void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value)
+{
+	tpm_buf_append(buf, &value, 1);
+}
+
+static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
+{
+	__be16 value2 = cpu_to_be16(value);
+
+	tpm_buf_append(buf, (u8 *) &value2, 2);
+}
+
+static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
+{
+	__be32 value2 = cpu_to_be32(value);
+
+	tpm_buf_append(buf, (u8 *) &value2, 4);
+}
+
 #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
 
 extern int tpm_is_tpm2(struct tpm_chip *chip);
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 9a94672..0736671 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -395,7 +395,7 @@ static int pcrlock(const int pcrnum)
 /*
  * Create an object specific authorisation protocol (OSAP) session
  */
-static int osap(struct tpm_buf *tb, struct osapsess *s,
+static int osap(struct tpm1_buf *tb, struct osapsess *s,
 		const unsigned char *key, uint16_t type, uint32_t handle)
 {
 	unsigned char enonce[TPM_NONCE_SIZE];
@@ -430,7 +430,7 @@ static int osap(struct tpm_buf *tb, struct osapsess *s,
 /*
  * Create an object independent authorisation protocol (oiap) session
  */
-int oiap(struct tpm_buf *tb, uint32_t *handle, unsigned char *nonce)
+int oiap(struct tpm1_buf *tb, uint32_t *handle, unsigned char *nonce)
 {
 	int ret;
 
@@ -464,7 +464,7 @@ struct tpm_digests {
  * Have the TPM seal(encrypt) the trusted key, possibly based on
  * Platform Configuration Registers (PCRs). AUTH1 for sealing key.
  */
-static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
+static int tpm_seal(struct tpm1_buf *tb, uint16_t keytype,
 		    uint32_t keyhandle, const unsigned char *keyauth,
 		    const unsigned char *data, uint32_t datalen,
 		    unsigned char *blob, uint32_t *bloblen,
@@ -579,7 +579,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
 /*
  * use the AUTH2_COMMAND form of unseal, to authorize both key and blob
  */
-static int tpm_unseal(struct tpm_buf *tb,
+static int tpm_unseal(struct tpm1_buf *tb,
 		      uint32_t keyhandle, const unsigned char *keyauth,
 		      const unsigned char *blob, int bloblen,
 		      const unsigned char *blobauth,
@@ -670,7 +670,7 @@ static int tpm_unseal(struct tpm_buf *tb,
 static int key_seal(struct trusted_key_payload *p,
 		    struct trusted_key_options *o)
 {
-	struct tpm_buf *tb;
+	struct tpm1_buf *tb;
 	int ret;
 
 	tb = kzalloc(sizeof *tb, GFP_KERNEL);
@@ -696,7 +696,7 @@ static int key_seal(struct trusted_key_payload *p,
 static int key_unseal(struct trusted_key_payload *p,
 		      struct trusted_key_options *o)
 {
-	struct tpm_buf *tb;
+	struct tpm1_buf *tb;
 	int ret;
 
 	tb = kzalloc(sizeof *tb, GFP_KERNEL);
-- 
2.7.4


^ permalink raw reply related

* [RFC/RFT v4 0/5] Add generic trusted keys framework/subsystem
From: Sumit Garg @ 2019-08-13  7:52 UTC (permalink / raw)
  To: keyrings, linux-integrity, linux-crypto, linux-security-module
  Cc: dhowells, herbert, davem, peterhuewe, jgg, jejb, jarkko.sakkinen,
	arnd, gregkh, zohar, jmorris, serge, casey, ard.biesheuvel,
	daniel.thompson, linux-kernel, tee-dev, Sumit Garg

This patch-set is an outcome of discussion here [1]. It has evolved very
much since v1 to create, consolidate and generalize trusted keys
subsystem.

This framework has been tested with trusted keys support provided via TEE
but I wasn't able to test it with a TPM device as I don't possess one. It
would be really helpful if others could test this patch-set using a TPM
device.

[1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg30591.html

Changes in v4:
1. Separate patch for export of tpm_buf code to include/linux/tpm.h
2. Change TPM1.x trusted keys code to use common tpm_buf
3. Keep module name as trusted.ko only

Changes in v3:

Move TPM2 trusted keys code to trusted keys subsystem.

Changes in v2:

Split trusted keys abstraction patch for ease of review.

Sumit Garg (5):
  tpm: move tpm_buf code to include/linux/
  KEYS: trusted: use common tpm_buf for TPM1.x code
  KEYS: trusted: create trusted keys subsystem
  KEYS: trusted: move tpm2 trusted keys code
  KEYS: trusted: Add generic trusted keys framework

 crypto/asymmetric_keys/asym_tpm.c                  |   2 +-
 drivers/char/tpm/tpm-chip.c                        |   1 +
 drivers/char/tpm/tpm-interface.c                   |  56 ---
 drivers/char/tpm/tpm.h                             | 230 -----------
 drivers/char/tpm/tpm2-cmd.c                        | 308 +--------------
 include/keys/trusted-type.h                        |  45 +++
 include/keys/{trusted.h => trusted_tpm.h}          |  61 +--
 include/linux/tpm.h                                | 270 ++++++++++++-
 security/keys/Makefile                             |   2 +-
 security/keys/trusted-keys/Makefile                |   9 +
 security/keys/trusted-keys/trusted-common.c        | 343 ++++++++++++++++
 .../keys/{trusted.c => trusted-keys/trusted-tpm.c} | 437 +++++----------------
 security/keys/trusted-keys/trusted-tpm2.c          | 378 ++++++++++++++++++
 13 files changed, 1141 insertions(+), 1001 deletions(-)
 rename include/keys/{trusted.h => trusted_tpm.h} (64%)
 create mode 100644 security/keys/trusted-keys/Makefile
 create mode 100644 security/keys/trusted-keys/trusted-common.c
 rename security/keys/{trusted.c => trusted-keys/trusted-tpm.c} (72%)
 create mode 100644 security/keys/trusted-keys/trusted-tpm2.c

-- 
2.7.4


^ permalink raw reply

* Re: [PATCH V37 27/29] tracefs: Restrict tracefs when the kernel is locked down
From: Marek Szyprowski @ 2019-08-13  7:21 UTC (permalink / raw)
  To: Matthew Garrett, jmorris
  Cc: linux-security-module, linux-kernel, linux-api, Matthew Garrett,
	Steven Rostedt, Krzysztof Kozlowski
In-Reply-To: <a3caa6d3-e3f9-ae41-d87e-253d9dc53d81@samsung.com>

Hi again,

On 2019-08-13 08:10, Marek Szyprowski wrote:
> Hi
>
> On 2019-08-01 00:16, Matthew Garrett wrote:
>> Tracefs may release more information about the kernel than desirable, so
>> restrict it when the kernel is locked down in confidentiality mode by
>> preventing open().
>>
>> Signed-off-by: Matthew Garrett <mjg59@google.com>
>> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> This patch causes the following regression on various Samsung Exynos 
> SoC based boards (ARM 32bit):
>
> [   15.364422] Unable to handle kernel NULL pointer dereference at 
> virtual address 00000000
> [   15.368775] pgd = a530ddbe
> [   15.371447] [00000000] *pgd=bcd7c831
> [   15.374993] Internal error: Oops: 80000007 [#1] PREEMPT SMP ARM
> [   15.380890] Modules linked in:
> [   15.383929] CPU: 0 PID: 1393 Comm: perf Not tainted 
> 5.2.0-00027-g757ff7244358-dirty #6459
> [   15.392086] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> [   15.398164] PC is at 0x0
> [   15.400687] LR is at do_dentry_open+0x22c/0x3b0
> [   15.405193] pc : [<00000000>]    lr : [<c02977c4>]    psr: 60000053
> [   15.411442] sp : e7317dd8  ip : 00000000  fp : 00000000
> [   15.416650] r10: c0187e6c  r9 : c041f8cc  r8 : e72123c8
> [   15.421858] r7 : e7317ec0  r6 : e7d89630  r5 : 00000000  r4 : e72123c0
> [   15.428368] r3 : 00000000  r2 : 5ba370f3  r1 : e72123c0  r0 : e7d89630
> [   15.434880] Flags: nZCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  
> Segment none
> [   15.442083] Control: 10c5387d  Table: 6726404a  DAC: 00000051
> [   15.447812] Process perf (pid: 1393, stack limit = 0x17621431)
> [   15.453628] Stack: (0xe7317dd8 to 0xe7318000)
> ...
> [   15.604842] [<c02977c4>] (do_dentry_open) from [<c02aafc8>] 
> (path_openat+0x5a0/0x1004)
> [   15.612735] [<c02aafc8>] (path_openat) from [<c02acce8>] 
> (do_filp_open+0x6c/0xd8)
> [   15.620200] [<c02acce8>] (do_filp_open) from [<c0298cc4>] 
> (do_sys_open+0x130/0x1f4)
> [   15.627839] [<c0298cc4>] (do_sys_open) from [<c0101000>] 
> (ret_fast_syscall+0x0/0x28)
> [   15.635560] Exception stack(0xe7317fa8 to 0xe7317ff0)
> [   15.640596] 7fa0:                   0022dc0b 001deee0 ffffff9c 
> beb6d764 00020000 00000000
> [   15.648756] 7fc0: 0022dc0b 001deee0 0022dba8 00000142 001ba044 
> 00241d68 001a13d8 beb6e78c
> [   15.656913] 7fe0: b6f7e000 beb6c6f8 9a27c600 b6f69504
> [   15.661952] Code: bad PC value
> [   15.665105] ---[ end trace 7e8b864582108f4a ]---
>
> This is standard ARM 32bit kernel with 
> arch/arm/configs/exynos_defconfig. It is enough to run "perf list" 
> command.
>
>> ---
>>   fs/tracefs/inode.c           | 40 +++++++++++++++++++++++++++++++++++-
>>   include/linux/security.h     |  1 +
>>   security/lockdown/lockdown.c |  1 +
>>   3 files changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
>> index 1387bcd96a79..12a325fb4cbd 100644
>> --- a/fs/tracefs/inode.c
>> +++ b/fs/tracefs/inode.c
>> @@ -21,6 +21,7 @@
>>   #include <linux/seq_file.h>
>>   #include <linux/magic.h>
>>   #include <linux/slab.h>
>> +#include <linux/security.h>
>>     #define TRACEFS_DEFAULT_MODE    0700
>>   @@ -28,6 +29,23 @@ static struct vfsmount *tracefs_mount;
>>   static int tracefs_mount_count;
>>   static bool tracefs_registered;
>>   +static int default_open_file(struct inode *inode, struct file *filp)
>> +{
>> +    struct dentry *dentry = filp->f_path.dentry;
>> +    struct file_operations *real_fops;
>> +    int ret;
>> +
>> +    if (!dentry)
>> +        return -EINVAL;
>> +
>> +    ret = security_locked_down(LOCKDOWN_TRACEFS);
>> +    if (ret)
>> +        return ret;
>> +
>> +    real_fops = dentry->d_fsdata;
>
> real_fops are NULL in my test case.

Too much of a hurry. real_fops are okay in that test case...

>
>> +    return real_fops->open(inode, filp);

... the issue is caused by NULL ->open() callback. Switching the above 
line to:

return real_fops->open ? real_fops->open(inode, filp) : 0;

fixes the issue.

>> +}
>> +
>>   static ssize_t default_read_file(struct file *file, char __user *buf,
>>                    size_t count, loff_t *ppos)
>>   {
>> @@ -210,6 +228,12 @@ static int tracefs_apply_options(struct 
>> super_block *sb)
>>       return 0;
>>   }
>>   +static void tracefs_destroy_inode(struct inode *inode)
>> +{
>> +    if (S_ISREG(inode->i_mode))
>> +        kfree(inode->i_fop);
>> +}
>> +
>>   static int tracefs_reconfigure(struct fs_context *fc)
>>   {
>>       struct super_block *sb = fc->root->d_sb;
>> @@ -236,6 +260,7 @@ static int tracefs_show_options(struct seq_file 
>> *m, struct dentry *root)
>>     static const struct super_operations tracefs_super_operations = {
>>       .statfs        = simple_statfs,
>> +    .destroy_inode  = tracefs_destroy_inode,
>>       .show_options    = tracefs_show_options,
>>   };
>>   @@ -372,6 +397,7 @@ struct dentry *tracefs_create_file(const char 
>> *name, umode_t mode,
>>                      struct dentry *parent, void *data,
>>                      const struct file_operations *fops)
>>   {
>> +    struct file_operations *proxy_fops;
>>       struct dentry *dentry;
>>       struct inode *inode;
>>   @@ -387,8 +413,20 @@ struct dentry *tracefs_create_file(const char 
>> *name, umode_t mode,
>>       if (unlikely(!inode))
>>           return failed_creating(dentry);
>>   +    proxy_fops = kzalloc(sizeof(struct file_operations), GFP_KERNEL);
>> +    if (unlikely(!proxy_fops)) {
>> +        iput(inode);
>> +        return failed_creating(dentry);
>> +    }
>> +
>> +    if (!fops)
>> +        fops = &tracefs_file_operations;
>> +
>> +    dentry->d_fsdata = (void *)fops;
>> +    memcpy(proxy_fops, fops, sizeof(*proxy_fops));
>> +    proxy_fops->open = default_open_file;
>>       inode->i_mode = mode;
>> -    inode->i_fop = fops ? fops : &tracefs_file_operations;
>> +    inode->i_fop = proxy_fops;
>>       inode->i_private = data;
>>       d_instantiate(dentry, inode);
>>       fsnotify_create(dentry->d_parent->d_inode, dentry);
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index d92323b44a3f..807dc0d24982 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -121,6 +121,7 @@ enum lockdown_reason {
>>       LOCKDOWN_KPROBES,
>>       LOCKDOWN_BPF_READ,
>>       LOCKDOWN_PERF,
>> +    LOCKDOWN_TRACEFS,
>>       LOCKDOWN_CONFIDENTIALITY_MAX,
>>   };
>>   diff --git a/security/lockdown/lockdown.c 
>> b/security/lockdown/lockdown.c
>> index 88064ce1c844..173191562047 100644
>> --- a/security/lockdown/lockdown.c
>> +++ b/security/lockdown/lockdown.c
>> @@ -36,6 +36,7 @@ static char 
>> *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
>>       [LOCKDOWN_KPROBES] = "use of kprobes",
>>       [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
>>       [LOCKDOWN_PERF] = "unsafe use of perf",
>> +    [LOCKDOWN_TRACEFS] = "use of tracefs",
>>       [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
>>   };
>
> Best regards

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply

* Re: [PATCH V37 27/29] tracefs: Restrict tracefs when the kernel is locked down
From: Marek Szyprowski @ 2019-08-13  6:10 UTC (permalink / raw)
  To: Matthew Garrett, jmorris
  Cc: linux-security-module, linux-kernel, linux-api, Matthew Garrett,
	Steven Rostedt, Krzysztof Kozlowski
In-Reply-To: <20190731221617.234725-28-matthewgarrett@google.com>

Hi

On 2019-08-01 00:16, Matthew Garrett wrote:
> Tracefs may release more information about the kernel than desirable, so
> restrict it when the kernel is locked down in confidentiality mode by
> preventing open().
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

This patch causes the following regression on various Samsung Exynos SoC 
based boards (ARM 32bit):

[   15.364422] Unable to handle kernel NULL pointer dereference at 
virtual address 00000000
[   15.368775] pgd = a530ddbe
[   15.371447] [00000000] *pgd=bcd7c831
[   15.374993] Internal error: Oops: 80000007 [#1] PREEMPT SMP ARM
[   15.380890] Modules linked in:
[   15.383929] CPU: 0 PID: 1393 Comm: perf Not tainted 
5.2.0-00027-g757ff7244358-dirty #6459
[   15.392086] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[   15.398164] PC is at 0x0
[   15.400687] LR is at do_dentry_open+0x22c/0x3b0
[   15.405193] pc : [<00000000>]    lr : [<c02977c4>]    psr: 60000053
[   15.411442] sp : e7317dd8  ip : 00000000  fp : 00000000
[   15.416650] r10: c0187e6c  r9 : c041f8cc  r8 : e72123c8
[   15.421858] r7 : e7317ec0  r6 : e7d89630  r5 : 00000000  r4 : e72123c0
[   15.428368] r3 : 00000000  r2 : 5ba370f3  r1 : e72123c0  r0 : e7d89630
[   15.434880] Flags: nZCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  
Segment none
[   15.442083] Control: 10c5387d  Table: 6726404a  DAC: 00000051
[   15.447812] Process perf (pid: 1393, stack limit = 0x17621431)
[   15.453628] Stack: (0xe7317dd8 to 0xe7318000)
...
[   15.604842] [<c02977c4>] (do_dentry_open) from [<c02aafc8>] 
(path_openat+0x5a0/0x1004)
[   15.612735] [<c02aafc8>] (path_openat) from [<c02acce8>] 
(do_filp_open+0x6c/0xd8)
[   15.620200] [<c02acce8>] (do_filp_open) from [<c0298cc4>] 
(do_sys_open+0x130/0x1f4)
[   15.627839] [<c0298cc4>] (do_sys_open) from [<c0101000>] 
(ret_fast_syscall+0x0/0x28)
[   15.635560] Exception stack(0xe7317fa8 to 0xe7317ff0)
[   15.640596] 7fa0:                   0022dc0b 001deee0 ffffff9c 
beb6d764 00020000 00000000
[   15.648756] 7fc0: 0022dc0b 001deee0 0022dba8 00000142 001ba044 
00241d68 001a13d8 beb6e78c
[   15.656913] 7fe0: b6f7e000 beb6c6f8 9a27c600 b6f69504
[   15.661952] Code: bad PC value
[   15.665105] ---[ end trace 7e8b864582108f4a ]---

This is standard ARM 32bit kernel with 
arch/arm/configs/exynos_defconfig. It is enough to run "perf list" command.

> ---
>   fs/tracefs/inode.c           | 40 +++++++++++++++++++++++++++++++++++-
>   include/linux/security.h     |  1 +
>   security/lockdown/lockdown.c |  1 +
>   3 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
> index 1387bcd96a79..12a325fb4cbd 100644
> --- a/fs/tracefs/inode.c
> +++ b/fs/tracefs/inode.c
> @@ -21,6 +21,7 @@
>   #include <linux/seq_file.h>
>   #include <linux/magic.h>
>   #include <linux/slab.h>
> +#include <linux/security.h>
>   
>   #define TRACEFS_DEFAULT_MODE	0700
>   
> @@ -28,6 +29,23 @@ static struct vfsmount *tracefs_mount;
>   static int tracefs_mount_count;
>   static bool tracefs_registered;
>   
> +static int default_open_file(struct inode *inode, struct file *filp)
> +{
> +	struct dentry *dentry = filp->f_path.dentry;
> +	struct file_operations *real_fops;
> +	int ret;
> +
> +	if (!dentry)
> +		return -EINVAL;
> +
> +	ret = security_locked_down(LOCKDOWN_TRACEFS);
> +	if (ret)
> +		return ret;
> +
> +	real_fops = dentry->d_fsdata;

real_fops are NULL in my test case.

> +	return real_fops->open(inode, filp);
> +}
> +
>   static ssize_t default_read_file(struct file *file, char __user *buf,
>   				 size_t count, loff_t *ppos)
>   {
> @@ -210,6 +228,12 @@ static int tracefs_apply_options(struct super_block *sb)
>   	return 0;
>   }
>   
> +static void tracefs_destroy_inode(struct inode *inode)
> +{
> +	if (S_ISREG(inode->i_mode))
> +		kfree(inode->i_fop);
> +}
> +
>   static int tracefs_reconfigure(struct fs_context *fc)
>   {
>   	struct super_block *sb = fc->root->d_sb;
> @@ -236,6 +260,7 @@ static int tracefs_show_options(struct seq_file *m, struct dentry *root)
>   
>   static const struct super_operations tracefs_super_operations = {
>   	.statfs		= simple_statfs,
> +	.destroy_inode  = tracefs_destroy_inode,
>   	.show_options	= tracefs_show_options,
>   };
>   
> @@ -372,6 +397,7 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
>   				   struct dentry *parent, void *data,
>   				   const struct file_operations *fops)
>   {
> +	struct file_operations *proxy_fops;
>   	struct dentry *dentry;
>   	struct inode *inode;
>   
> @@ -387,8 +413,20 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
>   	if (unlikely(!inode))
>   		return failed_creating(dentry);
>   
> +	proxy_fops = kzalloc(sizeof(struct file_operations), GFP_KERNEL);
> +	if (unlikely(!proxy_fops)) {
> +		iput(inode);
> +		return failed_creating(dentry);
> +	}
> +
> +	if (!fops)
> +		fops = &tracefs_file_operations;
> +
> +	dentry->d_fsdata = (void *)fops;
> +	memcpy(proxy_fops, fops, sizeof(*proxy_fops));
> +	proxy_fops->open = default_open_file;
>   	inode->i_mode = mode;
> -	inode->i_fop = fops ? fops : &tracefs_file_operations;
> +	inode->i_fop = proxy_fops;
>   	inode->i_private = data;
>   	d_instantiate(dentry, inode);
>   	fsnotify_create(dentry->d_parent->d_inode, dentry);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index d92323b44a3f..807dc0d24982 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -121,6 +121,7 @@ enum lockdown_reason {
>   	LOCKDOWN_KPROBES,
>   	LOCKDOWN_BPF_READ,
>   	LOCKDOWN_PERF,
> +	LOCKDOWN_TRACEFS,
>   	LOCKDOWN_CONFIDENTIALITY_MAX,
>   };
>   
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 88064ce1c844..173191562047 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -36,6 +36,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
>   	[LOCKDOWN_KPROBES] = "use of kprobes",
>   	[LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
>   	[LOCKDOWN_PERF] = "unsafe use of perf",
> +	[LOCKDOWN_TRACEFS] = "use of tracefs",
>   	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
>   };
>   

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply

* Re: [PATCH V38 00/29] security: Add support for locking down the kernel
From: James Morris @ 2019-08-12 22:29 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: LSM List, Linux Kernel Mailing List, Linux API
In-Reply-To: <alpine.LRH.2.21.1908130339130.14197@namei.org>

On Tue, 13 Aug 2019, James Morris wrote:

> On Mon, 12 Aug 2019, Matthew Garrett wrote:
> 
> > On Fri, Aug 9, 2019 at 11:08 PM James Morris <jmorris@namei.org> wrote:
> > > Please verify and test, as I had to make a few minor fixups for my v5.2
> > > base.
> > 
> > Thanks James - there's a few small fixups required, would you like
> > those as separate patches or should I just send you a fixed copy of
> > the original patchset?
> 
> Given there are a few, an updated copy of the patchset will be best.

Actually, that's a lot of patches to resend, just send updates against my 
next-lockdown branch.

-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* Re: [PATCH v3] fanotify, inotify, dnotify, security: add security hook for fs notifications
From: Paul Moore @ 2019-08-12 22:04 UTC (permalink / raw)
  To: Aaron Goidel
  Cc: selinux, linux-security-module, linux-fsdevel, dhowells, jack,
	amir73il, James Morris, Stephen Smalley, linux-kernel,
	Casey Schaufler
In-Reply-To: <20190812152000.18050-1-acgoide@tycho.nsa.gov>

On Mon, Aug 12, 2019 at 11:20 AM Aaron Goidel <acgoide@tycho.nsa.gov> wrote:
> As of now, setting watches on filesystem objects has, at most, applied a
> check for read access to the inode, and in the case of fanotify, requires
> CAP_SYS_ADMIN. No specific security hook or permission check has been
> provided to control the setting of watches. Using any of inotify, dnotify,
> or fanotify, it is possible to observe, not only write-like operations, but
> even read access to a file. Modeling the watch as being merely a read from
> the file is insufficient for the needs of SELinux. This is due to the fact
> that read access should not necessarily imply access to information about
> when another process reads from a file. Furthermore, fanotify watches grant
> more power to an application in the form of permission events. While
> notification events are solely, unidirectional (i.e. they only pass
> information to the receiving application), permission events are blocking.
> Permission events make a request to the receiving application which will
> then reply with a decision as to whether or not that action may be
> completed. This causes the issue of the watching application having the
> ability to exercise control over the triggering process. Without drawing a
> distinction within the permission check, the ability to read would imply
> the greater ability to control an application. Additionally, mount and
> superblock watches apply to all files within the same mount or superblock.
> Read access to one file should not necessarily imply the ability to watch
> all files accessed within a given mount or superblock.
>
> In order to solve these issues, a new LSM hook is implemented and has been
> placed within the system calls for marking filesystem objects with inotify,
> fanotify, and dnotify watches. These calls to the hook are placed at the
> point at which the target path has been resolved and are provided with the
> path struct, the mask of requested notification events, and the type of
> object on which the mark is being set (inode, superblock, or mount). The
> mask and obj_type have already been translated into common FS_* values
> shared by the entirety of the fs notification infrastructure. The path
> struct is passed rather than just the inode so that the mount is available,
> particularly for mount watches. This also allows for use of the hook by
> pathname-based security modules. However, since the hook is intended for
> use even by inode based security modules, it is not placed under the
> CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security
> modules would need to enable all of the path hooks, even though they do not
> use any of them.
>
> This only provides a hook at the point of setting a watch, and presumes
> that permission to set a particular watch implies the ability to receive
> all notification about that object which match the mask. This is all that
> is required for SELinux. If other security modules require additional hooks
> or infrastructure to control delivery of notification, these can be added
> by them. It does not make sense for us to propose hooks for which we have
> no implementation. The understanding that all notifications received by the
> requesting application are all strictly of a type for which the application
> has been granted permission shows that this implementation is sufficient in
> its coverage.
>
> Security modules wishing to provide complete control over fanotify must
> also implement a security_file_open hook that validates that the access
> requested by the watching application is authorized. Fanotify has the issue
> that it returns a file descriptor with the file mode specified during
> fanotify_init() to the watching process on event. This is already covered
> by the LSM security_file_open hook if the security module implements
> checking of the requested file mode there. Otherwise, a watching process
> can obtain escalated access to a file for which it has not been authorized.
>
> The selinux_path_notify hook implementation works by adding five new file
> permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm
> (descriptions about which will follow), and one new filesystem permission:
> watch (which is applied to superblock checks). The hook then decides which
> subset of these permissions must be held by the requesting application
> based on the contents of the provided mask and the obj_type. The
> selinux_file_open hook already checks the requested file mode and therefore
> ensures that a watching process cannot escalate its access through
> fanotify.
>
> The watch, watch_mount, and watch_sb permissions are the baseline
> permissions for setting a watch on an object and each are a requirement for
> any watch to be set on a file, mount, or superblock respectively. It should
> be noted that having either of the other two permissions (watch_reads and
> watch_with_perm) does not imply the watch, watch_mount, or watch_sb
> permission. Superblock watches further require the filesystem watch
> permission to the superblock. As there is no labeled object in view for
> mounts, there is no specific check for mount watches beyond watch_mount to
> the inode. Such a check could be added in the future, if a suitable labeled
> object existed representing the mount.
>
> The watch_reads permission is required to receive notifications from
> read-exclusive events on filesystem objects. These events include accessing
> a file for the purpose of reading and closing a file which has been opened
> read-only. This distinction has been drawn in order to provide a direct
> indication in the policy for this otherwise not obvious capability. Read
> access to a file should not necessarily imply the ability to observe read
> events on a file.
>
> Finally, watch_with_perm only applies to fanotify masks since it is the
> only way to set a mask which allows for the blocking, permission event.
> This permission is needed for any watch which is of this type. Though
> fanotify requires CAP_SYS_ADMIN, this is insufficient as it gives implicit
> trust to root, which we do not do, and does not support least privilege.
>
> Signed-off-by: Aaron Goidel <acgoide@tycho.nsa.gov>
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> Acked-by: Jan Kara <jack@suse.cz>
> ---
> v3:
>   - fixed comment style in security hook
>
> v2:
>   - move initialization of obj_type up to remove duplicate work
>   - convert inotify and fanotify flags to common FS_* flags
> ---
>  fs/notify/dnotify/dnotify.c         | 15 +++++++--
>  fs/notify/fanotify/fanotify_user.c  | 19 ++++++++++--
>  fs/notify/inotify/inotify_user.c    | 14 +++++++--
>  include/linux/lsm_hooks.h           |  9 +++++-
>  include/linux/security.h            | 10 ++++--
>  security/security.c                 |  6 ++++
>  security/selinux/hooks.c            | 47 +++++++++++++++++++++++++++++
>  security/selinux/include/classmap.h |  5 +--
>  8 files changed, 113 insertions(+), 12 deletions(-)

Merged into selinux/next, thanks everyone!

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH V38 00/29] security: Add support for locking down the kernel
From: James Morris @ 2019-08-12 17:39 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: LSM List, Linux Kernel Mailing List, Linux API
In-Reply-To: <CACdnJusx3N_ZoH4=+tqt85K9J5wmUnC-+bTtG_5qSD_TYu74+A@mail.gmail.com>

On Mon, 12 Aug 2019, Matthew Garrett wrote:

> On Fri, Aug 9, 2019 at 11:08 PM James Morris <jmorris@namei.org> wrote:
> > Please verify and test, as I had to make a few minor fixups for my v5.2
> > base.
> 
> Thanks James - there's a few small fixups required, would you like
> those as separate patches or should I just send you a fixed copy of
> the original patchset?

Given there are a few, an updated copy of the patchset will be best.

-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* Re: [PATCH] security: fix ptr_ret.cocci warnings
From: Kees Cook @ 2019-08-12 18:00 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Matthew Garrett, kbuild-all, linux-security-module, James Morris
In-Reply-To: <20190810065846.6giywcprz47xbwij@48261080c7f1>

On Sat, Aug 10, 2019 at 02:58:46PM +0800, kbuild test robot wrote:
> From: kbuild test robot <lkp@intel.com>
> 
> security/lockdown/lockdown.c:157:1-3: WARNING: PTR_ERR_OR_ZERO can be used
> 
> 
>  Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
> 
> Generated by: scripts/coccinelle/api/ptr_ret.cocci
> 
> Fixes: 80d14015a8e3 ("security: Add a static lockdown policy LSM")
> CC: Matthew Garrett <matthewgarrett@google.com>
> Signed-off-by: kbuild test robot <lkp@intel.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
> 
> tree:   https://kernel.googlesource.com/pub/scm/linux/kernel/git/jmorris/linux-security.git next-lockdown
> head:   05ef41e93e1a40d6b2d9846284824ec6f67fe422
> commit: 80d14015a8e3109f9b5e3e39b0bc78e1c3a1f315 [3/29] security: Add a static lockdown policy LSM
> 
>  lockdown.c |    5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -154,10 +154,7 @@ static int __init lockdown_secfs_init(vo
>  
>  	dentry = securityfs_create_file("lockdown", 0600, NULL, NULL,
>  					&lockdown_ops);
> -	if (IS_ERR(dentry))
> -		return PTR_ERR(dentry);
> -
> -	return 0;
> +	return PTR_ERR_OR_ZERO(dentry);
>  }
>  
>  core_initcall(lockdown_secfs_init);

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH V38 00/29] security: Add support for locking down the kernel
From: Matthew Garrett @ 2019-08-12 17:06 UTC (permalink / raw)
  To: James Morris; +Cc: LSM List, Linux Kernel Mailing List, Linux API
In-Reply-To: <alpine.LRH.2.21.1908101608260.25186@namei.org>

On Fri, Aug 9, 2019 at 11:08 PM James Morris <jmorris@namei.org> wrote:
> Please verify and test, as I had to make a few minor fixups for my v5.2
> base.

Thanks James - there's a few small fixups required, would you like
those as separate patches or should I just send you a fixed copy of
the original patchset?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox