[RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[David Howells]

If a certificate is self-signed, don't bother checking the validity of the
signature.  The cert cannot be checked by validation against the next one
in the chain as this is the root of the chain.  Trust for this certificate
can only be determined by whether we obtained it from a trusted location
(ie. it was built into the kernel at compile time).

This also fixes a bug whereby certificates were being assumed to be
self-signed if they had neither AKID nor SKID, the symptoms of which show
up as an attempt to load a certificate failing with -ERANGE or -EBADMSG.
This is produced from the RSA module when the result of calculating "m =
s^e mod n" is checked.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: David Woodhouse <David.Woodhouse@intel.com>
cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
---

 crypto/asymmetric_keys/x509_public_key.c |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 2a44b3752471..26e1937af7f4 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -255,6 +255,9 @@ static int x509_validate_trust(struct x509_certificate *cert,
 	struct key *key;
 	int ret = 1;
 
+	if (!cert->akid_id || !cert->akid_skid)
+		return 1;
+
 	if (!trust_keyring)
 		return -EOPNOTSUPP;
 
@@ -312,17 +315,21 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
 	cert->pub->id_type = PKEY_ID_X509;
 
-	/* Check the signature on the key if it appears to be self-signed */
-	if ((!cert->akid_skid && !cert->akid_id) ||
-	    asymmetric_key_id_same(cert->skid, cert->akid_skid) ||
-	    asymmetric_key_id_same(cert->id, cert->akid_id)) {
-		ret = x509_check_signature(cert->pub, cert); /* self-signed */
-		if (ret < 0)
-			goto error_free_cert;
-	} else if (!prep->trusted) {
+	/* See if we can derive the trustability of this certificate.
+	 *
+	 * When it comes to self-signed certificates, we cannot evaluate
+	 * trustedness except by the fact that we obtained it from a trusted
+	 * location.  So we just rely on x509_validate_trust() failing in this
+	 * case.
+	 *
+	 * Note that there's a possibility of a self-signed cert matching a
+	 * cert that we have (most likely a duplicate that we already trust) -
+	 * in which case it will be marked trusted.
+	 */
+	if (!prep->trusted) {
 		ret = x509_validate_trust(cert, get_system_trusted_keyring());
 		if (!ret)
-			prep->trusted = 1;
+			prep->trusted = true;
 	}
 
 	/* Propose a description */

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> If a certificate is self-signed, don't bother checking the validity of the
> signature.  The cert cannot be checked by validation against the next one
> in the chain as this is the root of the chain.  Trust for this certificate
> can only be determined by whether we obtained it from a trusted location
> (ie. it was built into the kernel at compile time).
> 
> This also fixes a bug whereby certificates were being assumed to be
> self-signed if they had neither AKID nor SKID, the symptoms of which show
> up as an attempt to load a certificate failing with -ERANGE or -EBADMSG.
> This is produced from the RSA module when the result of calculating "m =
> s^e mod n" is checked.

Oops - I forgot to change the patch description.

David

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[Mimi Zohar]

On Tue, 2016-01-05 at 15:47 +0000, David Howells wrote:
> If a certificate is self-signed, don't bother checking the validity of the
> signature.  The cert cannot be checked by validation against the next one
> in the chain as this is the root of the chain.  Trust for this certificate
> can only be determined by whether we obtained it from a trusted location
> (ie. it was built into the kernel at compile time).
> 
> This also fixes a bug whereby certificates were being assumed to be
> self-signed if they had neither AKID nor SKID, the symptoms of which show
> up as an attempt to load a certificate failing with -ERANGE or -EBADMSG.
> This is produced from the RSA module when the result of calculating "m =
> s^e mod n" is checked.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: David Woodhouse <David.Woodhouse@intel.com>
> cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
> 
>  crypto/asymmetric_keys/x509_public_key.c |   25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index 2a44b3752471..26e1937af7f4 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -255,6 +255,9 @@ static int x509_validate_trust(struct x509_certificate *cert,
>  	struct key *key;
>  	int ret = 1;
> 
> +	if (!cert->akid_id || !cert->akid_skid)
> +		return 1;
> +
>  	if (!trust_keyring)
>  		return -EOPNOTSUPP;
> 
> @@ -312,17 +315,21 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>  	cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
>  	cert->pub->id_type = PKEY_ID_X509;
> 
> -	/* Check the signature on the key if it appears to be self-signed */
> -	if ((!cert->akid_skid && !cert->akid_id) ||
> -	    asymmetric_key_id_same(cert->skid, cert->akid_skid) ||
> -	    asymmetric_key_id_same(cert->id, cert->akid_id)) {
> -		ret = x509_check_signature(cert->pub, cert); /* self-signed */
> -		if (ret < 0)
> -			goto error_free_cert;
> -	} else if (!prep->trusted) {
> +	/* See if we can derive the trustability of this certificate.
> +	 *
> +	 * When it comes to self-signed certificates, we cannot evaluate
> +	 * trustedness except by the fact that we obtained it from a trusted
> +	 * location.  So we just rely on x509_validate_trust() failing in this
> +	 * case.
> +	 *
> +	 * Note that there's a possibility of a self-signed cert matching a
> +	 * cert that we have (most likely a duplicate that we already trust) -
> +	 * in which case it will be marked trusted.
> +	 */
> +	if (!prep->trusted) {
>  		ret = x509_validate_trust(cert, get_system_trusted_keyring());
>  		if (!ret)
> -			prep->trusted = 1;
> +			prep->trusted = true;
>  	}

You're missing Petko's patch:
41c89b6 IMA: create machine owner and blacklist keyrings

Mimi

> 
>  	/* Propose a description */
> 

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[David Howells]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> You're missing Petko's patch:
> 41c89b6 IMA: create machine owner and blacklist keyrings

Hmmm...  This is wrong.  x509_key_preparse() shouldn't be polling the IMA MOK
keyring under all circumstances.

David

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[Mimi Zohar]

On Tue, 2016-01-05 at 16:39 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > You're missing Petko's patch:
> > 41c89b6 IMA: create machine owner and blacklist keyrings
> 
> Hmmm...  This is wrong.  x509_key_preparse() shouldn't be polling the IMA MOK
> keyring under all circumstances.

The x509_validate_trust() was originally added for IMA to ensure, on a
secure boot system, a certificate chain of trust rooted in hardware.
The IMA MOK keyring extends this certificate chain of trust to the
running system.

The IMA MOK keyring is a build configuration option that defaults to
off.

Mimi

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[David Howells]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> The x509_validate_trust() was originally added for IMA to ensure, on a
> secure boot system, a certificate chain of trust rooted in hardware.
> The IMA MOK keyring extends this certificate chain of trust to the
> running system.

The problem is that because 'trusted' is a boolean, a key in the IMA MOK
keyring will permit addition to the system keyring.

David

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[Mimi Zohar]

On Wed, 2016-01-06 at 13:21 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > The x509_validate_trust() was originally added for IMA to ensure, on a
> > secure boot system, a certificate chain of trust rooted in hardware.
> > The IMA MOK keyring extends this certificate chain of trust to the
> > running system.
> 
> The problem is that because 'trusted' is a boolean, a key in the IMA MOK
> keyring will permit addition to the system keyring.

Once the builtin keys are loaded onto the system keyring, isn't the
system keyring locked?  Or is this the only mechanism used for locking?

Mimi

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[David Howells]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> Once the builtin keys are loaded onto the system keyring, isn't the
> system keyring locked?

No.

David

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[Petko Manolov]

On 16-01-06 13:21:27, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > The x509_validate_trust() was originally added for IMA to ensure, on a 
> > secure boot system, a certificate chain of trust rooted in hardware. The IMA 
> > MOK keyring extends this certificate chain of trust to the running system.
> 
> The problem is that because 'trusted' is a boolean, a key in the IMA MOK 
> keyring will permit addition to the system keyring.

If this is true the i am clearly doing the wrong thing.  The CA hierarchy should 
run top-bottom, not the other way around.

IMA MOK was introduced mainly because .system keyring was static at the time.  
Assuming i have my root certificate in .system how can i add more keys to this 
keyring?  The new keys have been signed by my root CA?  Is this possible since 
your October patch-set or i've been missing something this whole time?


		Petko

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[David Howells]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> You're missing Petko's patch:
> 41c89b6 IMA: create machine owner and blacklist keyrings

It should also be cc'd to the keyrings mailing list.

David

Re: [RFC PATCH] X.509: Don't check the signature on apparently self-signed keys [ver #2]

[Petko Manolov]

On 16-01-05 16:40:31, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > You're missing Petko's patch:
> > 41c89b6 IMA: create machine owner and blacklist keyrings
> 
> It should also be cc'd to the keyrings mailing list.

Right.

If i am not terribly mistaken there's no way to revoke a certificate that is in 
a CA hierarchy with the system keyring on top of it.  Certain scenarios require 
us to revoke them as it was presented at the last year's LSS.

If x509_key_preparse() is not the right place then where shall i place the 
check?


		Petko