[PATCH 1/1] x509: only prefix strip raw serial numbers

[PATCH 1/1] x509: only prefix strip raw serial numbers

[Andy Whitcroft]

In the commit below we added support for use of the subKeyId rather than
the raw serial number when forming the in kernel ID:

  commit dd2f6c4481debfa389c1f2b2b1d5bd6449c42611
  Author: David Howells <dhowells@redhat.com>
  Date:   Fri Oct 3 16:17:02 2014 +0100

    X.509: If available, use the raw subjKeyId to form the key description

However as part of this we subject the subjKeyId to the below prefix strip:

    if (srlen > 1 && *q == 0) {
	    srlen--;
	    q++;
    }

This leads us to truncate the id for kernel module signing keys and to
fail to recognise our own modules:

  [    1.572423] Loaded X.509 cert 'Build time autogenerated kernel
    key: 62a7c3d2da278be024da4af8652c071f3fea33'
  [    1.646153] Request for unknown module key 'Build time autogenerated
    kernel key: 0062a7c3d2da278be024da4af8652c071f3fea33' err -11

Only apply the prefix strip to raw serial number.

Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 crypto/asymmetric_keys/x509_public_key.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

	While we are here the prefix strip seems pretty odd, only removing
	just one 0 byte.  Is this meant to strip them all (as a while),
	or was the intent to strip leading 0s from the hex form?  Do we
	have any background to this change?

	-apw

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 24f17e6..0e16d5e 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -306,10 +306,10 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	} else {
 		srlen = cert->raw_serial_size;
 		q = cert->raw_serial;
-	}
-	if (srlen > 1 && *q == 0) {
-		srlen--;
-		q++;
+		if (srlen > 1 && *q == 0) {
+			srlen--;
+			q++;
+		}
 	}
 
 	ret = -ENOMEM;
-- 
2.5.0

Re: [PATCH 1/1] x509: only prefix strip raw serial numbers

[David Howells]

Andy Whitcroft <apw@canonical.com> wrote:

> This leads us to truncate the id for kernel module signing keys and to
> fail to recognise our own modules:
> 
>   [    1.572423] Loaded X.509 cert 'Build time autogenerated kernel
>     key: 62a7c3d2da278be024da4af8652c071f3fea33'
>   [    1.646153] Request for unknown module key 'Build time autogenerated
>     kernel key: 0062a7c3d2da278be024da4af8652c071f3fea33' err -11

I don't suppose you've saved the key and a random small module that I can have
a play with?

What version of the kernel are you using, btw?

David

Re: [PATCH 1/1] x509: only prefix strip raw serial numbers

[Andy Whitcroft]

On Tue, Sep 15, 2015 at 10:59:43AM +0100, David Howells wrote:
> Andy Whitcroft <apw@canonical.com> wrote:
> 
> > This leads us to truncate the id for kernel module signing keys and to
> > fail to recognise our own modules:
> > 
> >   [    1.572423] Loaded X.509 cert 'Build time autogenerated kernel
> >     key: 62a7c3d2da278be024da4af8652c071f3fea33'
> >   [    1.646153] Request for unknown module key 'Build time autogenerated
> >     kernel key: 0062a7c3d2da278be024da4af8652c071f3fea33' err -11
> 
> I don't suppose you've saved the key and a random small module that I can have
> a play with?

Sorry no, the key was an ephemeral key in those builds.  I did run a few
key builds to generate a new key with 0's to confirm this was possible.

> What version of the kernel are you using, btw?

Ahh yes, this was against a v4.2 final, I see that sign-file is all
changing to use openssl, so I will go confirm that this is not different
as a result.

-apw

Re: [PATCH 1/1] x509: only prefix strip raw serial numbers

[David Howells]

Hi Andy,

Okay, it seems that the 00-stripping you pointed out is the problem.  Does
this patch fix it?  Note that patch won't necessarily apply post-4.2.

David
---
commit fefc5570aa2c88985f62f0f3335428c867103763
Author: David Howells <dhowells@redhat.com>
Date:   Wed Sep 16 23:10:24 2015 +0100

    MODSIGN: Don't strip leading 00's from key ID when constructing key description
    
    Don't strip leading zeros from the crypto key ID when using it to construct
    the struct key description as the signature in kernels up to and including
    4.2 matched this aspect of the key.  This means that 1 in 256 keys won't
    actually match if their key ID begins with 00.
    
    The key ID is stored in the module signature as binary and so must be
    converted to text in order to invoke request_key() - but it isn't stripped
    at this point.
    
    Something like this is likely to be observed in dmesg when the key is loaded:
    
    [    1.572423] Loaded X.509 cert 'Build time autogenerated kernel
        key: 62a7c3d2da278be024da4af8652c071f3fea33'
    
    followed by this when we try and use it:
    
      [    1.646153] Request for unknown module key 'Build time autogenerated
        kernel key: 0062a7c3d2da278be024da4af8652c071f3fea33' err -11
    
    The 'Loaded' line should show an extra '00' on the front of the hex string.
    
    This problem should not affect 4.3-rc1 and onwards because there the key
    should be matched on one of its auxiliary identities rather than the key
    struct's description string.
    
    Reported-by: Arjan van de Ven <arjan@linux.intel.com>
    Reported-by: Andy Whitcroft <apw@canonical.com>
    Signed-off-by: David Howells <dhowells@redhat.com>

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 24f17e6c5904..4c850ac474e2 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -307,10 +307,6 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 		srlen = cert->raw_serial_size;
 		q = cert->raw_serial;
 	}
-	if (srlen > 1 && *q == 0) {
-		srlen--;
-		q++;
-	}
 
 	ret = -ENOMEM;
 	desc = kmalloc(sulen + 2 + srlen * 2 + 1, GFP_KERNEL);