mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: dhowells@redhat.com, herbert@gondor.hengli.com.au,
	pjones@redhat.com, rusty@rustcorp.com.au,
	linux-crypto@vger.kernel.org, zohar@us.ibm.com,
	dmitry.kasatkin@intel.com, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 14/16] X.509: Add an ASN.1 decoder
Date: Tue, 18 Sep 2012 18:34:12 +0100	[thread overview]
Message-ID: <13189.1347989652@warthog.procyon.org.uk> (raw)
In-Reply-To: <20120914103930.1e16ad8b@pyramind.ukuu.org.uk>

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> Why do this in the kernel.That appears to be completely insane.

A number of reasons:

 (1) The UEFI signature/key database may contain ASN.1 X.509 certificates and
     we may need to use those very early in the boot process, during initrd.

 (2) Even if userspace is available, offloading the key parsing to userspace
     means we have to have some way to trust what we get back.

 (3) Giving the kernel ASN.1 X.509 certs allows the kernel to verify the
     signature on that cert against a key it gets from the UEFI db.  For that,
     you need the raw cert.

> Can you prove it runs in a short bounded time for all inputs,

Possibly.  I'll have to think about that.  It may be relatively
straightforward.

 (1) The ASN.1 decoder is limited (currently) to a maximum of 64k of data to
     parse.

 (2) The decoder never goes backward through the data.

 (3) The decoder has a strictly limited recursion/nesting stack.

The decoder uses a state machine of a sort produced by the compiler.

 (1) Most nodes in this only have one transition.

 (2) Simple optional nodes are simply marked skippable.

 (3) There are some nodes that are like subroutine calls (for multiple-use
     constructed types), but these must return to the point directly after the
     call.

 (4) There are some nodes that have two transitions.  These are used for
     optional constructed type values (particularly multiple-use ones).
     Basically, they are jump-to-subroutine or skip.  The return must go back
     to the next node.

Writing a perl script to check the sanity of the compiler output should be easy
enough, though it would have to assume that the driver is sane.

> has it been fuzz tested extensively ?

Fuzz testing from a script is very easy, eg:

	#!/bin/sh
	cd /tmp
	sync
	declare -i n i j k
	while true
	do
	    n=$RANDOM
	    j=$RANDOM
	    j=j%10
	    k=0
	    echo $n $j
	    dd if=/dev/urandom of=/tmp/data bs=$n count=1
	    for ((i=1; i<n; i=i+k))
	    do
		dd if=/tmp/data bs=$i count=1 2>/dev/null |
		keyctl padd asymmetric foo @s 2>/dev/null
		k=k+1
		if [ $k -eq 10 ]
		    then
		    echo -n .
		    k=0
		fi
	    done
	    echo
	done

Though I haven't done a great deal of such testing as I want to be able to use
my test machine for other stuff too.  I can, however, run multiple fuzzers simultaneously.

I have also passed a number of bits of X.509 and PKCS#7 through it, and also
some valid ASN.1 that isn't what the decoder is expecting.

	#!/bin/sh

	file=/tmp/x509cert
	if [ "$1" != "" ]
	then
	    file=$1
	fi

	cd /tmp
	sync

	while true
	do
	    openssl req -new -x509 -outform PEM -keyout $file.pem -nodes -subj "/CN=GB/O=Red Hat/OU=Magrathea/CN=Slartibartfast" -out $file.x509 || exit $?

	    openssl x509 -in $file.x509 -inform PEM -outform DER >$file.x509.asn1 || exit $?
	    keyctl padd asymmetric bar @s <$file.x509.asn1 || exit $?

	    n=$RANDOM
	    if [ $n -lt 10 ]; then n=10; fi
	    dd if=/dev/urandom of=$file.stuff bs=$n count=1

	    openssl smime -sign -inkey $file.pem -signer $file.x509 -keyform PEM \
		-in $file.stuff -out $file.pkcs7 -binary -outform DER || exit $?

	    keyctl padd asymmetric baz @s <$file.pkcs7
	done

Unfortunately, the above is somewhat limited in what it can produce.

David

  parent reply	other threads:[~2012-09-18 17:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-13 23:48 [RFC][PATCH 00/16] Asymmetric / Public-key cryptography key type David Howells
2012-09-13 23:48 ` [PATCH 01/16] KEYS: Add payload preparsing opportunity prior to key instantiate or update David Howells
2012-09-13 23:48 ` [PATCH 02/16] MPILIB: Provide count_leading/trailing_zeros() based on arch functions David Howells
2012-09-13 23:48 ` [PATCH 03/16] KEYS: Document asymmetric key type David Howells
2012-09-13 23:48 ` [PATCH 04/16] KEYS: Implement " David Howells
2012-09-13 23:48 ` [PATCH 05/16] KEYS: Asymmetric key pluggable data parsers David Howells
2012-09-13 23:48 ` [PATCH 06/16] KEYS: Asymmetric public-key algorithm crypto key subtype David Howells
2012-09-13 23:49 ` [PATCH 07/16] KEYS: Provide signature verification with an asymmetric key David Howells
2012-09-13 23:49 ` [PATCH 08/16] MPILIB: Reinstate mpi_cmp[_ui]() and export for RSA signature verification David Howells
2012-09-13 23:49 ` [PATCH 09/16] RSA: Implement signature verification algorithm [PKCS#1 / RFC3447] David Howells
2012-09-13 23:49 ` [PATCH 10/16] RSA: Fix signature verification for shorter signatures David Howells
2012-09-13 23:49 ` [PATCH 11/16] X.509: Implement simple static OID registry David Howells
2012-09-13 23:49 ` [PATCH 12/16] X.509: Add utility functions to render OIDs as strings David Howells
2012-09-13 23:49 ` [PATCH 13/16] X.509: Add simple ASN.1 grammar compiler David Howells
2012-09-13 23:50 ` [PATCH 14/16] X.509: Add an ASN.1 decoder David Howells
2012-09-14  9:39   ` Alan Cox
2012-09-18 17:34   ` David Howells [this message]
2012-09-18 18:51     ` Alan Cox
2012-09-18 22:19       ` Peter Jones
2012-09-19  4:17       ` James Morris
2012-09-20  9:45       ` David Howells
2012-09-18 22:03   ` David Howells
2012-09-18 22:26   ` David Howells
2012-09-19 13:05   ` David Howells
2012-09-13 23:50 ` [PATCH 15/16] MPILIB: Provide a function to read raw data into an MPI David Howells
2012-09-13 23:50 ` [PATCH 16/16] X.509: Add a crypto key parser for binary (DER) X.509 certificates David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=13189.1347989652@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=dmitry.kasatkin@intel.com \
    --cc=herbert@gondor.hengli.com.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=pjones@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=zohar@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome