| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Two error return paths are neglecting to free allocated object td,
causing a memory leak. Fix this by returning via the error return
path that securely kfree's td.
Fixes clang scan-build warning:
security/keys/trusted-keys/trusted_tpm1.c:496:10: warning: Potential
memory leak [unix.Malloc]
Cc: stable@vger.kernel.org
Fixes: 5df16caada3f ("KEYS: trusted: Fix incorrect handling of tpm_get_random()")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
|
|
The generic framework patch broke the current TPM trusted keys because
it doesn't correctly remove the values consumed by the generic parser
before passing them on to the implementation specific parser. Fix
this by having the generic parser return the string minus the consumed
tokens.
Additionally, there may be no tokens left for the implementation
specific parser, so make it handle the NULL case correctly and finally
fix a TPM 1.2 specific check for no keyhandle.
Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
Tested-by: Sumit Garg <sumit.garg@linaro.org>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
|
|
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 trusted keys
support in case platform doesn't posses a TPM device.
Add a generic trusted keys framework where underlying implementations
can be easily plugged in. Create struct trusted_key_ops to achieve this,
which contains necessary functions of a backend.
Also, define a module parameter in order to select a particular trust
source in case a platform support multiple trust sources. In case its
not specified then implementation itetrates through trust sources list
starting with TPM and assign the first trust source as a backend which
has initiazed successfully during iteration.
Note that current implementation only supports a single trust source at
runtime which is either selectable at compile time or during boot via
aforementioned module parameter.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
|
|
Modify the TPM2 key format blob output to export and import in the
ASN.1 form for TPM2 sealed object keys. For compatibility with prior
trusted keys, the importer will also accept two TPM2B quantities
representing the public and private parts of the key. However, the
export via keyctl pipe will only output the ASN.1 format.
The benefit of the ASN.1 format is that it's a standard and thus the
exported key can be used by userspace tools (openssl_tpm2_engine,
openconnect and tpm2-tss-engine). The format includes policy
specifications, thus it gets us out of having to construct policy
handles in userspace and the format includes the parent meaning you
don't have to keep passing it in each time.
This patch only implements basic handling for the ASN.1 format, so
keys with passwords but no policy.
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
|
|
In TPM 1.2 an authorization was a 20 byte number. The spec actually
recommended you to hash variable length passwords and use the sha1
hash as the authorization. Because the spec doesn't require this
hashing, the current authorization for trusted keys is a 40 digit hex
number. For TPM 2.0 the spec allows the passing in of variable length
passwords and passphrases directly, so we should allow that in trusted
keys for ease of use. Update the 'blobauth' parameter to take this
into account, so we can now use plain text passwords for the keys.
so before
keyctl add trusted kmk "new 32 blobauth=f572d396fae9206628714fb2ce00f72e94f2258fkeyhandle=81000001" @u
after we will accept both the old hex sha1 form as well as a new
directly supplied password:
keyctl add trusted kmk "new 32 blobauth=hello keyhandle=81000001" @u
Since a sha1 hex code must be exactly 40 bytes long and a direct
password must be 20 or less, we use the length as the discriminator
for which form is input.
Note this is both and enhancement and a potential bug fix. The TPM
2.0 spec requires us to strip leading zeros, meaning empyty
authorization is a zero length HMAC whereas we're currently passing in
20 bytes of zeros. A lot of TPMs simply accept this as OK, but the
Microsoft TPM emulator rejects it with TPM_RC_BAD_AUTH, so this patch
makes the Microsoft TPM emulator work with trusted keys.
Fixes: 0fe5480303a1 ("keys, trusted: seal/unseal with TPM 2.0 chips")
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
|
|
Consider the following transcript:
$ keyctl add trusted kmk "new 32 blobauth=helloworld keyhandle=80000000 migratable=1" @u
add_key: Invalid argument
The documentation has the following description:
migratable= 0|1 indicating permission to reseal to new PCR values,
default 1 (resealing allowed)
The consequence is that "migratable=1" should succeed. Fix this by
allowing this condition to pass instead of return -EINVAL.
[*] Documentation/security/keys/trusted-encrypted.rst
Cc: stable@vger.kernel.org
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
|
|
When tpm_get_random() was introduced, it defined the following API for the
return value:
1. A positive value tells how many bytes of random data was generated.
2. A negative value on error.
However, in the call sites the API was used incorrectly, i.e. as it would
only return negative values and otherwise zero. Returning he positive read
counts to the user space does not make any possible sense.
Fix this by returning -EIO when tpm_get_random() returns a positive value.
Fixes: 41ab999c80f1 ("tpm: Move tpm_get_random api into the TPM device driver")
Cc: stable@vger.kernel.org
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Kent Yoder <key@linux.vnet.ibm.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
|
|
Currently <crypto/sha.h> contains declarations for both SHA-1 and SHA-2,
and <crypto/sha3.h> contains declarations for SHA-3.
This organization is inconsistent, but more importantly SHA-1 is no
longer considered to be cryptographically secure. So to the extent
possible, SHA-1 shouldn't be grouped together with any of the other SHA
versions, and usage of it should be phased out.
Therefore, split <crypto/sha.h> into two headers <crypto/sha1.h> and
<crypto/sha2.h>, and make everyone explicitly specify whether they want
the declarations for SHA-1, SHA-2, or both.
This avoids making the SHA-1 declarations visible to files that don't
want anything to do with SHA-1. It also prepares for potentially moving
sha1.h into a new insecure/ or dangerous/ directory.
Signed-off-by: Eric Biggers <ebiggers@google.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
|
|
As said by Linus:
A symmetric naming is only helpful if it implies symmetries in use.
Otherwise it's actively misleading.
In "kzalloc()", the z is meaningful and an important part of what the
caller wants.
In "kzfree()", the z is actively detrimental, because maybe in the
future we really _might_ want to use that "memfill(0xdeadbeef)" or
something. The "zero" part of the interface isn't even _relevant_.
The main reason that kzfree() exists is to clear sensitive information
that should not be leaked to other future users of the same memory
objects.
Rename kzfree() to kfree_sensitive() to follow the example of the recently
added kvfree_sensitive() and make the intention of the API more explicit.
In addition, memzero_explicit() is used to clear the memory to make sure
that it won't get optimized away by the compiler.
The renaming is done by using the command sequence:
git grep -w --name-only kzfree |\
xargs sed -i 's/kzfree/kfree_sensitive/'
followed by some editing of the kfree_sensitive() kerneldoc and adding
a kzfree backward compatibility macro in slab.h.
[akpm@linux-foundation.org: fs/crypto/inline_crypt.c needs linux/slab.h]
[akpm@linux-foundation.org: fix fs/crypto/inline_crypt.c some more]
Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Waiman Long <longman@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Acked-by: David Howells <dhowells@redhat.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Joe Perches <joe@perches.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "Jason A . Donenfeld" <Jason@zx2c4.com>
Link: http://lkml.kernel.org/r/20200616154311.12314-3-longman@redhat.com
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
A lockdep circular locking dependency report was seen when running a
keyutils test:
[12537.027242] ======================================================
[12537.059309] WARNING: possible circular locking dependency detected
[12537.088148] 4.18.0-147.7.1.el8_1.x86_64+debug #1 Tainted: G OE --------- - -
[12537.125253] ------------------------------------------------------
[12537.153189] keyctl/25598 is trying to acquire lock:
[12537.175087] 000000007c39f96c (&mm->mmap_sem){++++}, at: __might_fault+0xc4/0x1b0
[12537.208365]
[12537.208365] but task is already holding lock:
[12537.234507] 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220
[12537.270476]
[12537.270476] which lock already depends on the new lock.
[12537.270476]
[12537.307209]
[12537.307209] the existing dependency chain (in reverse order) is:
[12537.340754]
[12537.340754] -> #3 (&type->lock_class){++++}:
[12537.367434] down_write+0x4d/0x110
[12537.385202] __key_link_begin+0x87/0x280
[12537.405232] request_key_and_link+0x483/0xf70
[12537.427221] request_key+0x3c/0x80
[12537.444839] dns_query+0x1db/0x5a5 [dns_resolver]
[12537.468445] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs]
[12537.496731] cifs_reconnect+0xe04/0x2500 [cifs]
[12537.519418] cifs_readv_from_socket+0x461/0x690 [cifs]
[12537.546263] cifs_read_from_socket+0xa0/0xe0 [cifs]
[12537.573551] cifs_demultiplex_thread+0x311/0x2db0 [cifs]
[12537.601045] kthread+0x30c/0x3d0
[12537.617906] ret_from_fork+0x3a/0x50
[12537.636225]
[12537.636225] -> #2 (root_key_user.cons_lock){+.+.}:
[12537.664525] __mutex_lock+0x105/0x11f0
[12537.683734] request_key_and_link+0x35a/0xf70
[12537.705640] request_key+0x3c/0x80
[12537.723304] dns_query+0x1db/0x5a5 [dns_resolver]
[12537.746773] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs]
[12537.775607] cifs_reconnect+0xe04/0x2500 [cifs]
[12537.798322] cifs_readv_from_socket+0x461/0x690 [cifs]
[12537.823369] cifs_read_from_socket+0xa0/0xe0 [cifs]
[12537.847262] cifs_demultiplex_thread+0x311/0x2db0 [cifs]
[12537.873477] kthread+0x30c/0x3d0
[12537.890281] ret_from_fork+0x3a/0x50
[12537.908649]
[12537.908649] -> #1 (&tcp_ses->srv_mutex){+.+.}:
[12537.935225] __mutex_lock+0x105/0x11f0
[12537.954450] cifs_call_async+0x102/0x7f0 [cifs]
[12537.977250] smb2_async_readv+0x6c3/0xc90 [cifs]
[12538.000659] cifs_readpages+0x120a/0x1e50 [cifs]
[12538.023920] read_pages+0xf5/0x560
[12538.041583] __do_page_cache_readahead+0x41d/0x4b0
[12538.067047] ondemand_readahead+0x44c/0xc10
[12538.092069] filemap_fault+0xec1/0x1830
[12538.111637] __do_fault+0x82/0x260
[12538.129216] do_fault+0x419/0xfb0
[12538.146390] __handle_mm_fault+0x862/0xdf0
[12538.167408] handle_mm_fault+0x154/0x550
[12538.187401] __do_page_fault+0x42f/0xa60
[12538.207395] do_page_fault+0x38/0x5e0
[12538.225777] page_fault+0x1e/0x30
[12538.243010]
[12538.243010] -> #0 (&mm->mmap_sem){++++}:
[12538.267875] lock_acquire+0x14c/0x420
[12538.286848] __might_fault+0x119/0x1b0
[12538.306006] keyring_read_iterator+0x7e/0x170
[12538.327936] assoc_array_subtree_iterate+0x97/0x280
[12538.352154] keyring_read+0xe9/0x110
[12538.370558] keyctl_read_key+0x1b9/0x220
[12538.391470] do_syscall_64+0xa5/0x4b0
[12538.410511] entry_SYSCALL_64_after_hwframe+0x6a/0xdf
[12538.435535]
[12538.435535] other info that might help us debug this:
[12538.435535]
[12538.472829] Chain exists of:
[12538.472829] &mm->mmap_sem --> root_key_user.cons_lock --> &type->lock_class
[12538.472829]
[12538.524820] Possible unsafe locking scenario:
[12538.524820]
[12538.551431] CPU0 CPU1
[12538.572654] ---- ----
[12538.595865] lock(&type->lock_class);
[12538.613737] lock(root_key_user.cons_lock);
[12538.644234] lock(&type->lock_class);
[12538.672410] lock(&mm->mmap_sem);
[12538.687758]
[12538.687758] *** DEADLOCK ***
[12538.687758]
[12538.714455] 1 lock held by keyctl/25598:
[12538.732097] #0: 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220
[12538.770573]
[12538.770573] stack backtrace:
[12538.790136] CPU: 2 PID: 25598 Comm: keyctl Kdump: loaded Tainted: G
[12538.844855] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS P89 12/27/2015
[12538.881963] Call Trace:
[12538.892897] dump_stack+0x9a/0xf0
[12538.907908] print_circular_bug.isra.25.cold.50+0x1bc/0x279
[12538.932891] ? save_trace+0xd6/0x250
[12538.948979] check_prev_add.constprop.32+0xc36/0x14f0
[12538.971643] ? keyring_compare_object+0x104/0x190
[12538.992738] ? check_usage+0x550/0x550
[12539.009845] ? sched_clock+0x5/0x10
[12539.025484] ? sched_clock_cpu+0x18/0x1e0
[12539.043555] __lock_acquire+0x1f12/0x38d0
[12539.061551] ? trace_hardirqs_on+0x10/0x10
[12539.080554] lock_acquire+0x14c/0x420
[12539.100330] ? __might_fault+0xc4/0x1b0
[12539.119079] __might_fault+0x119/0x1b0
[12539.135869] ? __might_fault+0xc4/0x1b0
[12539.153234] keyring_read_iterator+0x7e/0x170
[12539.172787] ? keyring_read+0x110/0x110
[12539.190059] assoc_array_subtree_iterate+0x97/0x280
[12539.211526] keyring_read+0xe9/0x110
[12539.227561] ? keyring_gc_check_iterator+0xc0/0xc0
[12539.249076] keyctl_read_key+0x1b9/0x220
[12539.266660] do_syscall_64+0xa5/0x4b0
[12539.283091] entry_SYSCALL_64_after_hwframe+0x6a/0xdf
One way to prevent this deadlock scenario from happening is to not
allow writing to userspace while holding the key semaphore. Instead,
an internal buffer is allocated for getting the keys out from the
read method first before copying them out to userspace without holding
the lock.
That requires taking out the __user modifier from all the relevant
read methods as well as additional changes to not use any userspace
write helpers. That is,
1) The put_user() call is replaced by a direct copy.
2) The copy_to_user() call is replaced by memcpy().
3) All the fault handling code is removed.
Compiling on a x86-64 system, the size of the rxrpc_read() function is
reduced from 3795 bytes to 2384 bytes with this patch.
Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2")
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Waiman Long <longman@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
|
|
Fixes gcc '-Wunused-but-set-variable' warning:
security/keys/trusted-keys/trusted_tpm1.c: In function tpm_unseal:
security/keys/trusted-keys/trusted_tpm1.c:588:11: warning: variable keyhndl set but not used [-Wunused-but-set-variable]
Fixes: 00aa975bd031 ("KEYS: trusted: Create trusted keys subsystem")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: zhengbin <zhengbin13@huawei.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
|
|
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.
Also, utilize existing tpm_send() exported API which wraps the internal
tpm_transmit_cmd() API.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
|
|
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>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
|
