Provide better handling of unsupported crypto when verifying a PKCS#7 message.
If we can't bridge the gap between a pair of X.509 certs or between a signed
info block and an X.509 cert because it involves some crypto we don't support,
that's not necessarily the end of the world as there may be other ways points
at which we can intersect with a ring of trusted keys.
Instead, only produce ENOPKG immediately if all the signed info blocks in a
PKCS#7 message require unsupported crypto to bridge to the first X.509 cert.
Otherwise, we defer the generation of ENOPKG until we get ENOKEY during trust
validation.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Make use of the new match string preparsing to overhaul key identification
when searching for asymmetric keys. The following changes are made:
(1) Use the previously created asymmetric_key_id struct to hold the following
key IDs derived from the X.509 certificate or PKCS#7 message:
id: serial number + issuer
skid: subjKeyId + subject
authority: authKeyId + issuer
(2) Replace the hex fingerprint attached to key->type_data[1] with an
asymmetric_key_ids struct containing the id and the skid (if present).
(3) Make the asymmetric_type match data preparse select one of two searches:
(a) An iterative search for the key ID given if prefixed with "id:". The
prefix is expected to be followed by a hex string giving the ID to
search for. The criterion key ID is checked against all key IDs
recorded on the key.
(b) A direct search if the key ID is not prefixed with "id:". This will
look for an exact match on the key description.
(4) Make x509_request_asymmetric_key() take a key ID. This is then converted
into "id:<hex>" and passed into keyring_search() where match preparsing
will turn it back into a binary ID.
(5) X.509 certificate verification then takes the authority key ID and looks
up a key that matches it to find the public key for the certificate
signature.
(6) PKCS#7 certificate verification then takes the id key ID and looks up a
key that matches it to find the public key for the signed information
block signature.
Additional changes:
(1) Multiple subjKeyId and authKeyId values on an X.509 certificate cause the
cert to be rejected with -EBADMSG.
(2) The 'fingerprint' ID is gone. This was primarily intended to convey PGP
public key fingerprints. If PGP is supported in future, this should
generate a key ID that carries the fingerprint.
(3) Th ca_keyid= kernel command line option is now converted to a key ID and
used to match the authority key ID. Possibly this should only match the
actual authKeyId part and not the issuer as well.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Implement the first step in using binary key IDs for asymmetric keys rather
than hex string keys.
The previously added match data preparsing will be able to convert hex
criterion strings into binary which can then be compared more rapidly.
Further, we actually want more then one ID string per public key. The problem
is that X.509 certs refer to other X.509 certs by matching Issuer + AuthKeyId
to Subject + SubjKeyId, but PKCS#7 messages match against X.509 Issuer +
SerialNumber.
This patch just provides facilities for a later patch to make use of.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Make the key matching functions pointed to by key_match_data::cmp return bool
rather than int.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
A previous patch added a ->match_preparse() method to the key type. This is
allowed to override the function called by the iteration algorithm.
Therefore, we can just set a default that simply checks for an exact match of
the key description with the original criterion data and allow match_preparse
to override it as needed.
The key_type::match op is then redundant and can be removed, as can the
user_match() function.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Remove key_type::def_lookup_type as it's no longer used. The information now
defaults to KEYRING_SEARCH_LOOKUP_DIRECT but may be overridden by
type->match_preparse().
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Preparse the match data. This provides several advantages:
(1) The preparser can reject invalid criteria up front.
(2) The preparser can convert the criteria to binary data if necessary (the
asymmetric key type really wants to do binary comparison of the key IDs).
(3) The preparser can set the type of search to be performed. This means
that it's not then a one-off setting in the key type.
(4) The preparser can set an appropriate comparator function.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Provide a function to convert a buffer of binary data into an unterminated
ascii hex string representation of that data.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIVAwUAVBhlXROxKuMESys7AQIQghAAmxHRD9kD7AJrlcL187gTXl5dRyxQNhX0
myT9A1/01jHJh6mPPKS5jt5ooenwfcbvBkdUARLNLq6j7urIzFk+UtSmfcN1OMXw
3y15nuJKbSCZRzPMR94Z0Ik23YED9dmc4ubxW7E+psoWWIZUvFt4GaGw7ei77O39
8Pbt/n7nIx2+s8aiXte2Om/zqzyMEmcLd3Mzlqxe4GX1jo/ThNQZ348EL+ECxJEl
3OKe7i7oRfa48+SybmhW5Bx/2f/aQMcIQX04+akOFMIC505rUg6CTzphcrTyV5/s
6FdQexRquf8/Ei/6DMAYPhnumRfWJ5x9txiNSEY4i11AIjo6Bt65vaLPuNniYRNI
b6Wn8SSE8Ucrq5RrmNlSmoJCs7r1NE+JdOaEPO0MDVkOouaja8daISmveV2AfZfF
bITOQgEw3QRpyL2FYdwa39/NXCONBILfL5HvNyXEfPEHBhI8igTgEyXYRwNHV9jT
dsVFTc9ZIrjksLt3CDh4Z8xdyZbyojYdRCfH/wna9aAkZpwwGfSYCcR3dE6SK26y
IjkqEVJoCFHEnvLUQkAQrc/2qWX2D1qHrcjVLwzwbM5G66YIPeLcJlZK2FbiY0Ay
Yc/kUJY0hU5W+TfFb1hhjO5G2DTTw8Ou6MGcxSTE3HwzqICjDhE7BwFZdVikHRP0
xMtjfJnMwuM=
=v+dv
-----END PGP SIGNATURE-----
Merge tag 'keys-fixes-20140916' into keys-next
Merge in keyrings fixes, at least some of which later patches depend on:
(1) Reinstate the production of EPERM for key types beginning with '.' in
requests from userspace.
(2) Tidy up the cleanup of PKCS#7 message signed information blocks and fix a
bug this made more obvious.
Signed-off-by: David Howells <dhowells@redhat.coM>
Fix the parser cleanup code to drain parsed out X.509 certs in the case that
the decode fails and we jump to error_decode.
The function is rearranged so that the same cleanup code is used in the success
case as the error case - just that the message descriptor under construction is
only released if it is still pointed to by the context struct at that point.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
The code to free a signed info block is repeated several times, so move the
code to do it into a function of its own. This gives us a place to add clean
ups for stuff that gets added to pkcs7_signed_info.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Reinstate the generation of EPERM for a key type name beginning with a '.' in
a userspace call. Types whose name begins with a '.' are internal only.
The test was removed by:
commit a4e3b8d79a
Author: Mimi Zohar <zohar@linux.vnet.ibm.com>
Date: Thu May 22 14:02:23 2014 -0400
Subject: KEYS: special dot prefixed keyring name bug fix
I think we want to keep the restriction on type name so that userspace can't
add keys of a special internal type.
Note that removal of the test causes several of the tests in the keyutils
testsuite to fail.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
The kernel print macros use the KBUILD_MODNAME, which is initialized
to the module name. The current integrity/Makefile makes every file
as its own module, so pr_xxx messages are prefixed with the file name
instead of the module. Similar to the evm/Makefile and ima/Makefile,
this patch fixes the integrity/Makefile to use the single name
'integrity'.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
The integrity subsystem has lots of options and takes more than
half of the security menu. This patch consolidates the options
under "integrity", which are hidden if not enabled. This change
does not affect existing configurations. Re-configuration is not
needed.
Changes v4:
- no need to change "integrity subsystem" to menuconfig as
options are hidden, when not enabled. (Mimi)
- add INTEGRITY Kconfig help description
Changes v3:
- dependency to INTEGRITY removed when behind 'if INTEGRITY'
Changes v2:
- previous patch moved integrity out of the 'security' menu.
This version keeps integrity as a security option (Mimi).
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
For better visual appearance it is better to co-locate
asymmetric key options together with signature support.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
IMA uses only one template. This patch initializes only required
template to avoid unnecessary memory allocations.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Reviewed-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
In all cases except ima_bprm_check() the filename was not defined
and ima_d_path() was used to find the full path. Unfortunately,
the bprm filename is a relative pathname (eg. ./<dir>/filename).
ima_bprm_check() selects between bprm->interp and bprm->filename.
The following dump demonstrates the differences between using
filename and interp.
bprm->filename
filename: ./foo.sh, pathname: /root/bin/foo.sh
filename: ./foo.sh, pathname: /bin/dash
bprm->interp
filename: ./foo.sh, pathname: /root/bin/foo.sh
filename: /bin/sh, pathname: /bin/dash
In both cases the pathnames are currently the same. This patch
removes usage of filename and interp in favor of d_absolute_path.
Changes v3:
- 11 extra bytes for "deleted" not needed (Mimi)
- purpose "replace relative bprm filename with full pathname" (Mimi)
Changes v2:
- use d_absolute_path() instead of d_path to work in chroot environments.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
ima_get_action() sets the "action" flags based on policy.
Before collecting, measuring, appraising, or auditing the
file, the "action" flag is updated based on the cached
iint->flags.
This patch removes the subsequent unnecessary appraisal
test in ima_appraise_measurement().
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Add missing keywords to the function definition to cleanup
to discard initialization code.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Reviewed-by: Roberto Sassu <roberto.sassu@polito.it>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
'function' variable value can be changed instead of
allocating extra '_func' variable.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Precede bit testing before string comparison makes code
faster. Also refactor statement as a single line pointer
assignment. Logic is following: we set 'xattr_ptr' to read
xattr value when we will do appraisal or in any case when
measurement template is other than 'ima'.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Commit f381c27 "integrity: move ima inode integrity data management"
(re)moved few functions but left their declarations in header files.
This patch removes them and also removes duplicated declaration of
integrity_iint_find().
Commit c7de7ad "ima: remove unused cleanup functions". This patch
removes these definitions as well.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
If file has IMA signature, IMA in enforce mode, but key is missing
then file access is blocked and single error message is printed.
If IMA appraisal is enabled in fix mode, then system runs as usual
but might produce tons of 'Request for unknown key' messages.
This patch switches 'pr_warn' to 'pr_err_ratelimited'.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Empty files and missing xattrs do not guarantee that a file was
just created. This patch passes FILE_CREATED flag to IMA to
reliably identify new files.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: <stable@vger.kernel.org> 3.14+
Unless an LSM labels a file during d_instantiate(), newly created
files are not labeled with an initial security.evm xattr, until
the file closes. EVM, before allowing a protected, security xattr
to be written, verifies the existing 'security.evm' value is good.
For newly created files without a security.evm label, this
verification prevents writing any protected, security xattrs,
until the file closes.
Following is the example when this happens:
fd = open("foo", O_CREAT | O_WRONLY, 0644);
setxattr("foo", "security.SMACK64", value, sizeof(value), 0);
close(fd);
While INTEGRITY_NOXATTRS status is handled in other places, such
as evm_inode_setattr(), it does not handle it in all cases in
evm_protect_xattr(). By limiting the use of INTEGRITY_NOXATTRS to
newly created files, we can now allow setting "protected" xattrs.
Changelog:
- limit the use of INTEGRITY_NOXATTRS to IMA identified new files
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: <stable@vger.kernel.org> 3.14+
On ima_file_free(), newly created empty files are not labeled with
an initial security.ima value, because the iversion did not change.
Commit dff6efc "fs: fix iversion handling" introduced a change in
iversion behavior. To verify this change use the shell command:
$ (exec >foo)
$ getfattr -h -e hex -d -m security foo
This patch defines the IMA_NEW_FILE flag. The flag is initially
set, when IMA detects that a new file is created, and subsequently
checked on the ima_file_free() hook to set the initial security.ima
value.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: <stable@vger.kernel.org> 3.14+
This patch fixes a bug, where evm_verify_hmac() returns INTEGRITY_PASS
if inode->i_op->getxattr() returns an error in evm_find_protected_xattrs.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Printing in base signature handling should have a prefix, so set pr_fmt().
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
This patch fixes checkpatch 'return' warnings introduced with commit
9819cf2 "checkpatch: warn on unnecessary void function return statements".
Use scripts/checkpatch.pl --file security/integrity/evm/evm_main.c
to produce the warnings.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
3.16 commit aad4f8bb42
'switch simple generic_file_aio_read() users to ->read_iter()'
replaced ->aio_read with ->read_iter in most of the file systems
and introduced new_sync_read() as a replacement for do_sync_read().
Most of file systems set '->read' and ima_kernel_read is not affected.
When ->read is not set, this patch adopts fallback call changes from the
vfs_read.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: <stable@vger.kernel.org> 3.16+
This patch fixes the case where the file's signature/hash xattr contains
an invalid hash algorithm. Although we can not verify the xattr, we still
need to measure the file. Use the default IMA hash algorithm.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
The patch 3bcced39ea: "ima: use ahash API for file hash
calculation" from Feb 26, 2014, leads to the following static checker
warning:
security/integrity/ima/ima_crypto.c:204 ima_alloc_atfm()
error: buffer overflow 'hash_algo_name' 17 <= 17
Unlike shash tfm memory, which is allocated on initialization, the
ahash tfm memory allocation is deferred until needed.
This patch fixes the case where ima_ahash_tfm has not yet been
allocated and the file's signature/hash xattr contains an invalid hash
algorithm. Although we can not verify the xattr, we still need to
measure the file. Use the default IMA hash algorithm.
Changelog:
- set valid algo before testing tfm - based on Dmitry's comment
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Renaming an unused formal parameter in the static inline function
security_inode_init_security eliminates many W=2 warnings.
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Need to export x509_request_asymmetric_key() so that PKCS#7 can use it if
compiled as a module.
Reported-by: James Morris <jmorris@namei.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Historically the NetLabel LSM secattr catmap functions and data
structures have had very long names which makes a mess of the NetLabel
code and anyone who uses NetLabel. This patch renames the catmap
functions and structures from "*_secattr_catmap_*" to just "*_catmap_*"
which improves things greatly.
There are no substantial code or logic changes in this patch.
Signed-off-by: Paul Moore <pmoore@redhat.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
The two NetLabel LSM secattr catmap walk functions didn't handle
certain edge conditions correctly, causing incorrect security labels
to be generated in some cases. This patch corrects these problems and
converts the functions to use the new _netlbl_secattr_catmap_getnode()
function in order to reduce the amount of repeated code.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Moore <pmoore@redhat.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
The NetLabel secattr catmap functions, and the SELinux import/export
glue routines, were broken in many horrible ways and the SELinux glue
code fiddled with the NetLabel catmap structures in ways that we
probably shouldn't allow. At some point this "worked", but that was
likely due to a bit of dumb luck and sub-par testing (both inflicted
by yours truly). This patch corrects these problems by basically
gutting the code in favor of something less obtuse and restoring the
NetLabel abstractions in the SELinux catmap glue code.
Everything is working now, and if it decides to break itself in the
future this code will be much easier to debug than the code it
replaces.
One noteworthy side effect of the changes is that it is no longer
necessary to allocate a NetLabel catmap before calling one of the
NetLabel APIs to set a bit in the catmap. NetLabel will automatically
allocate the catmap nodes when needed, resulting in less allocations
when the lowest bit is greater than 255 and less code in the LSMs.
Cc: stable@vger.kernel.org
Reported-by: Christian Evans <frodox@zoho.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
The NetLabel category (catmap) functions have a problem in that they
assume categories will be set in an increasing manner, e.g. the next
category set will always be larger than the last. Unfortunately, this
is not a valid assumption and could result in problems when attempting
to set categories less than the startbit in the lowest catmap node.
In some cases kernel panics and other nasties can result.
This patch corrects the problem by checking for this and allocating a
new catmap node instance and placing it at the front of the list.
Cc: stable@vger.kernel.org
Reported-by: Christian Evans <frodox@zoho.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
Tested-by: Casey Schaufler <casey@schaufler-ca.com>
X.509 certificate issuer and subject fields are mandatory fields in the ASN.1
and so their existence needn't be tested for. They are guaranteed to end up
with an empty string if the name material has nothing we can use (see
x509_fabricate_name()).
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Instead of looping by ourselves we may use %*phN specifier to dump a small
buffer.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
[ PHuewe: removed now unused variable i ]
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
Some Atmel TPMs provide completely wrong timeouts from their
TPM_CAP_PROP_TIS_TIMEOUT query. This patch detects that and returns
new correct values via a DID/VID table in the TIS driver.
Tested on ARM using an AT97SC3204T FW version 37.16
Cc: <stable@vger.kernel.org>
[PHuewe: without this fix these 'broken' Atmel TPMs won't function on
older kernels]
Signed-off-by: "Berg, Christopher" <Christopher.Berg@atmel.com>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
Regression in 41ab999c. Call to tpm_chip_put is missing. This
will cause TPM device driver not to unload if tmp_get_random()
is called.
Cc: <stable@vger.kernel.org> # 3.7+
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
