Commit Graph

20 Commits

Author SHA1 Message Date
Eric Biggers
6459ae3866 PKCS#7: fix direct verification of SignerInfo signature
If none of the certificates in a SignerInfo's certificate chain match a
trusted key, nor is the last certificate signed by a trusted key, then
pkcs7_validate_trust_one() tries to check whether the SignerInfo's
signature was made directly by a trusted key.  But, it actually fails to
set the 'sig' variable correctly, so it actually verifies the last
signature seen.  That will only be the SignerInfo's signature if the
certificate chain is empty; otherwise it will actually be the last
certificate's signature.

This is not by itself a security problem, since verifying any of the
certificates in the chain should be sufficient to verify the SignerInfo.
Still, it's not working as intended so it should be fixed.

Fix it by setting 'sig' correctly for the direct verification case.

Fixes: 757932e6da ("PKCS#7: Handle PKCS#7 messages that contain no X.509 certs")
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
2018-02-22 14:38:33 +00:00
Eric Biggers
7204eb8590 pkcs7: fix check for self-signed certificate
pkcs7_validate_trust_one() used 'x509->next == x509' to identify a
self-signed certificate.  That's wrong; ->next is simply the link in the
linked list of certificates in the PKCS#7 message.  It should be
checking ->signer instead.  Fix it.

Fortunately this didn't actually matter because when we re-visited
'x509' on the next iteration via 'x509->signer', it was already seen and
not verified, so we returned -ENOKEY anyway.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: James Morris <james.l.morris@oracle.com>
2017-12-08 15:13:28 +00:00
David Howells
9eb029893a KEYS: Generalise x509_request_asymmetric_key()
Generalise x509_request_asymmetric_key().  It doesn't really have any
dependencies on X.509 features as it uses generalised IDs and the
public_key structs that contain data extracted from X.509.

Signed-off-by: David Howells <dhowells@redhat.com>
2016-04-11 22:41:56 +01:00
David Howells
bda850cd21 PKCS#7: Make trust determination dependent on contents of trust keyring
Make the determination of the trustworthiness of a key dependent on whether
a key that can verify it is present in the supplied ring of trusted keys
rather than whether or not the verifying key has KEY_FLAG_TRUSTED set.

verify_pkcs7_signature() will return -ENOKEY if the PKCS#7 message trust
chain cannot be verified.

Signed-off-by: David Howells <dhowells@redhat.com>
2016-04-06 16:14:24 +01:00
David Howells
566a117a8b PKCS#7: Make the signature a pointer rather than embedding it
Point to the public_key_signature struct from the pkcs7_signed_info struct
rather than embedding it.  This makes the code consistent with the X.509
signature handling and makes it possible to have a common cleanup function.

We also save a copy of the digest in the signature without sharing the
memory with the crypto layer metadata.

Signed-off-by: David Howells <dhowells@redhat.com>
2016-04-06 16:13:33 +01:00
David Howells
77d0910d15 X.509: Retain the key verification data
Retain the key verification data (ie. the struct public_key_signature)
including the digest and the key identifiers.

Note that this means that we need to take a separate copy of the digest in
x509_get_sig_params() rather than lumping it in with the crypto layer data.

Signed-off-by: David Howells <dhowells@redhat.com>
2016-04-06 16:13:33 +01:00
Linus Torvalds
62f444e054 Merge branch 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6
Pull crypto fix from Herbert Xu:
 "This fixes a bug in pkcs7_validate_trust and its users where the
  output value may in fact be taken from uninitialised memory"

* 'linus' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6:
  PKCS#7: pkcs7_validate_trust(): initialize the _trusted output argument
2016-03-30 13:28:34 -05:00
Nicolai Stange
e54358915d PKCS#7: pkcs7_validate_trust(): initialize the _trusted output argument
Despite what the DocBook comment to pkcs7_validate_trust() says, the
*_trusted argument is never set to false.

pkcs7_validate_trust() only positively sets *_trusted upon encountering
a trusted PKCS#7 SignedInfo block.

This is quite unfortunate since its callers, system_verify_data() for
example, depend on pkcs7_validate_trust() clearing *_trusted on non-trust.

Indeed, UBSAN splats when attempting to load the uninitialized local
variable 'trusted' from system_verify_data() in pkcs7_validate_trust():

  UBSAN: Undefined behaviour in crypto/asymmetric_keys/pkcs7_trust.c:194:14
  load of value 82 is not a valid value for type '_Bool'
  [...]
  Call Trace:
    [<ffffffff818c4d35>] dump_stack+0xbc/0x117
    [<ffffffff818c4c79>] ? _atomic_dec_and_lock+0x169/0x169
    [<ffffffff8194113b>] ubsan_epilogue+0xd/0x4e
    [<ffffffff819419fa>] __ubsan_handle_load_invalid_value+0x111/0x158
    [<ffffffff819418e9>] ? val_to_string.constprop.12+0xcf/0xcf
    [<ffffffff818334a4>] ? x509_request_asymmetric_key+0x114/0x370
    [<ffffffff814b83f0>] ? kfree+0x220/0x370
    [<ffffffff818312c2>] ? public_key_verify_signature_2+0x32/0x50
    [<ffffffff81835e04>] pkcs7_validate_trust+0x524/0x5f0
    [<ffffffff813c391a>] system_verify_data+0xca/0x170
    [<ffffffff813c3850>] ? top_trace_array+0x9b/0x9b
    [<ffffffff81510b29>] ? __vfs_read+0x279/0x3d0
    [<ffffffff8129372f>] mod_verify_sig+0x1ff/0x290
    [...]

The implication is that pkcs7_validate_trust() effectively grants trust
when it really shouldn't have.

Fix this by explicitly setting *_trusted to false at the very beginning
of pkcs7_validate_trust().

Cc: <stable@vger.kernel.org>
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2016-03-23 20:57:24 +08:00
Tadeusz Struk
db6c43bd21 crypto: KEYS: convert public key and digsig asym to the akcipher api
This patch converts the module verification code to the new akcipher API.

Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David Howells <dhowells@redhat.com>
2016-02-10 10:13:27 +00:00
Herbert Xu
f75516a815 crypto: keys - Revert "convert public key to akcipher api"
This needs to go through the security tree so I'm reverting the
patches for now.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2016-02-09 16:18:01 +08:00
Tadeusz Struk
57f96bbab9 crypto: asymmetric_keys - convert public key and digsig asym to the akcipher api
This patch converts the module verification code to the new akcipher API.

Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
2016-02-06 15:33:25 +08:00
David Howells
4573b64a31 X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier
If an X.509 certificate has an AuthorityKeyIdentifier extension that provides
an issuer and serialNumber, then make it so that these are used in preference
to the keyIdentifier field also held therein for searching for the signing
certificate.

If both the issuer+serialNumber and the keyIdentifier are supplied, then the
certificate is looked up by the former but the latter is checked as well.  If
the latter doesn't match the subjectKeyIdentifier of the parent certificate,
EKEYREJECTED is returned.

This makes it possible to chain X.509 certificates based on the issuer and
serialNumber fields rather than on subjectKeyIdentifier.  This is necessary as
we are having to deal with keys that are represented by X.509 certificates
that lack a subjectKeyIdentifier.

Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Vivek Goyal <vgoyal@redhat.com>
2015-08-07 16:26:13 +01:00
David Howells
b92e6570a9 X.509: Extract both parts of the AuthorityKeyIdentifier
Extract both parts of the AuthorityKeyIdentifier, not just the keyIdentifier,
as the second part can be used to match X.509 certificates by issuer and
serialNumber.

Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Vivek Goyal <vgoyal@redhat.com>
2015-08-07 16:26:13 +01:00
Dmitry Kasatkin
f1b731dbc2 KEYS: Restore partial ID matching functionality for asymmetric keys
Bring back the functionality whereby an asymmetric key can be matched with a
partial match on one of its IDs.

Whilst we're at it, allow for the possibility of having an increased number of
IDs.

Reported-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: David Howells <dhowells@redhat.com>
2014-10-06 15:21:05 +01:00
David Howells
757932e6da PKCS#7: Handle PKCS#7 messages that contain no X.509 certs
The X.509 certificate list in a PKCS#7 message is optional.  To save space, we
can omit the inclusion of any X.509 certificates if we are sure that we can
look the relevant public key up by the serial number and issuer given in a
signed info block.

This also supports use of a signed info block for which we can't find a
matching X.509 cert in the certificate list, though it be populated.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
2014-09-16 17:36:17 +01:00
David Howells
4155942000 PKCS#7: Better handling of unsupported crypto
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>
2014-09-16 17:36:15 +01:00
David Howells
46963b774d KEYS: Overhaul key identification when searching for asymmetric keys
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>
2014-09-16 17:36:13 +01:00
David Howells
15155b9a45 PKCS#7: Add a missing static
Add a missing static (found by checker).

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
2014-09-16 17:07:07 +01:00
David Howells
5ce43ad282 PKCS#7: Use x509_request_asymmetric_key()
pkcs7_request_asymmetric_key() and x509_request_asymmetric_key() do the same
thing, the latter being a copy of the former created by the IMA folks, so drop
the PKCS#7 version as the X.509 location is more general.

Whilst we're at it, rename the arguments of x509_request_asymmetric_key() to
better reflect what the values being passed in are intended to match on an
X.509 cert.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
2014-07-29 13:07:58 +01:00
David Howells
08815b62d7 PKCS#7: Find intersection between PKCS#7 message and known, trusted keys
Find the intersection between the X.509 certificate chain contained in a PKCS#7
message and a set of keys that we already know and trust.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
2014-07-08 13:50:15 +01:00