Patch "X.509: reject invalid BIT STRING for subjectPublicKey" has been added to the 3.18-stable tree

Patch "X.509: reject invalid BIT STRING for subjectPublicKey" has been added to the 3.18-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    X.509: reject invalid BIT STRING for subjectPublicKey

to the 3.18-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     x.509-reject-invalid-bit-string-for-subjectpublickey.patch
and it can be found in the queue-3.18 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From 0f30cbea005bd3077bd98cd29277d7fc2699c1da Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Fri, 8 Dec 2017 15:13:27 +0000
Subject: X.509: reject invalid BIT STRING for subjectPublicKey

From: Eric Biggers <ebiggers@google.com>

commit 0f30cbea005bd3077bd98cd29277d7fc2699c1da upstream.

Adding a specially crafted X.509 certificate whose subjectPublicKey
ASN.1 value is zero-length caused x509_extract_key_data() to set the
public key size to SIZE_MAX, as it subtracted the nonexistent BIT STRING
metadata byte.  Then, x509_cert_parse() called kmemdup() with that bogus
size, triggering the WARN_ON_ONCE() in kmalloc_slab().

This appears to be harmless, but it still must be fixed since WARNs are
never supposed to be user-triggerable.

Fix it by updating x509_cert_parse() to validate that the value has a
BIT STRING metadata byte, and that the byte is 0 which indicates that
the number of bits in the bitstring is a multiple of 8.

It would be nice to handle the metadata byte in asn1_ber_decoder()
instead.  But that would be tricky because in the general case a BIT
STRING could be implicitly tagged, and/or could legitimately have a
length that is not a whole number of bytes.

Here was the WARN (cleaned up slightly):

    WARNING: CPU: 1 PID: 202 at mm/slab_common.c:971 kmalloc_slab+0x5d/0x70 mm/slab_common.c:971
    Modules linked in:
    CPU: 1 PID: 202 Comm: keyctl Tainted: G    B            4.14.0-09238-g1d3b78bbc6e9 #26
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014
    task: ffff880033014180 task.stack: ffff8800305c8000
    Call Trace:
     __do_kmalloc mm/slab.c:3706 [inline]
     __kmalloc_track_caller+0x22/0x2e0 mm/slab.c:3726
     kmemdup+0x17/0x40 mm/util.c:118
     kmemdup include/linux/string.h:414 [inline]
     x509_cert_parse+0x2cb/0x620 crypto/asymmetric_keys/x509_cert_parser.c:106
     x509_key_preparse+0x61/0x750 crypto/asymmetric_keys/x509_public_key.c:174
     asymmetric_key_preparse+0xa4/0x150 crypto/asymmetric_keys/asymmetric_type.c:388
     key_create_or_update+0x4d4/0x10a0 security/keys/key.c:850
     SYSC_add_key security/keys/keyctl.c:122 [inline]
     SyS_add_key+0xe8/0x290 security/keys/keyctl.c:62
     entry_SYSCALL_64_fastpath+0x1f/0x96

Fixes: 42d5ec27f873 ("X.509: Add an ASN.1 decoder")
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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 crypto/asymmetric_keys/x509_cert_parser.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -381,6 +381,8 @@ int x509_extract_key_data(void *context,
 	ctx->cert->pub->pkey_algo = PKEY_ALGO_RSA;
 
 	/* Discard the BIT STRING metadata */
+	if (vlen < 1 || *(const u8 *)value != 0)
+		return -EBADMSG;
 	ctx->key = value + 1;
 	ctx->key_size = vlen - 1;
 	return 0;


Patches currently in stable-queue which might be from ebiggers@google.com are

queue-3.18/x.509-reject-invalid-bit-string-for-subjectpublickey.patch
queue-3.18/asn.1-check-for-error-from-asn1_op_end__act-actions.patch
queue-3.18/keys-add-missing-permission-check-for-request_key-destination.patch