Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
From: Elena Petrova <lenaptr@google.com>
To: Stephan Mueller <smueller@chronox.de>
Cc: "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
	<linux-crypto@vger.kernel.org>,
	Eric Biggers <ebiggers@kernel.org>,
	Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH 0/1] crypto: af_alg - add extra parameters for DRBG interface
Date: Tue, 14 Jul 2020 16:23:05 +0100
Message-ID: <CABvBcwauK_JyVzONdwJRGU81ZH5sYuiJSH0F2g+i5qCe363+fQ@mail.gmail.com> (raw)
In-Reply-To: <2941213.7s5MMGUR32@tauon.chronox.de>

Hi Stephan,

On Tue, 14 Jul 2020 at 06:17, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Montag, 13. Juli 2020, 18:48:56 CEST schrieb Elena Petrova:
>
> Hi Elena,
>
> > This patch extends the userspace RNG interface to make it usable for
> > CAVS testing. This is achieved by adding ALG_SET_DRBG_ENTROPY
> > option to the setsockopt interface for specifying the entropy, and using
> > sendmsg syscall for specifying the additional data.
> >
> > See libkcapi patch [1] to test the added functionality. The libkcapi
> > patch is not intended for merging into libkcapi as is: it is only a
> > quick plug to manually verify that the extended AF_ALG RNG interface
> > generates the expected output on DRBG800-90A CAVS inputs.
>
> As I am responsible for developing such CAVS/ACVP harness as well, I played
> with the idea of going through AF_ALG. I discarded it because I do not see the
> benefit why we should add an interface solely for the purpose of testing.
> Further, it is a potentially dangerous one because the created instance of the
> DRBG is "seeded" from data provided by the caller.
>
> Thus, I do not see the benefit from adding that extension, widening a user
> space interface solely for the purpose of CAVS testing. I would not see any
> other benefit we have with this extension. In particular, this interface would
> then be always there. What I could live with is an interface that can be
> enabled at compile time for those who want it.

Thanks for reviewing this patch. I understand your concern about the erroneous
use of the entropy input by non-testing applications. This was an approach that
I had discussed with Ard. I should have included you, my apologies. I'll  post
v2 with the CAVS testing stuff hidden under CONFIG_ option with appropriate help
text.

With regards to the usefulness, let me elaborate. This effort of extending the
drbg interface is driven by Android needs for having the kernel crypto
certified. I started from having an out-of-tree chrdev driver for Google Pixel
kernels that was exposing the required crypto functionality, and it wasn't ideal
in the following ways:
 * it primarily consisted of copypasted code from testmgr.c
 * it was hard for me to keep the code up to date because I'm not aware of
   ongoing changes to crypto
 * it is hard for other people and/or organisations to re-use it, hense a lot of
   duplicated effort is going into CAVS: you have a private driver, I have mine,
   Jaya from HP <jayalakshmi.bhat@hp.com>, who's been asking linux-crypto a few
   CAVS questions, has to develop theirs...

In general Android is trying to eliminate out-of-tree code. CAVS testing
functionality in particular needs to be upstream because we are switching all
Android devices to using a Generic Kernel Image (GKI)
[https://lwn.net/Articles/771974/] based on the upstream kernel.

> Besides, when you want to do CAVS testing, the following ciphers are still not
> testable and thus this patch would only be a partial solution to get the
> testing covered:
>
> - AES KW (you cannot get the final IV out of the kernel - I played with the
> idea to return the IV through AF_ALG, but discarded it because of the concern
> above)
>
> - OFB/CFB MCT testing (you need the IV from the last round - same issue as for
> AES KW)
>
> - RSA
>
> - DH
>
> - ECDH

For Android certification purposes, we only need to test the modules which are
actually being used. In our case it's what af_alg already exposes plus DRBG.
If, say, HP would need RSA, they could submit their own patch.

As for exposing bits of the internal state for some algorithms, I hope guarding
the testing functionality with a CONFIG_ option would solve the security part of
the problem.

> With these issues, I would assume you are better off creating your own kernel
> module just like I did that externalizes the crypto API to user space but is
> only available on your test kernel and will not affect all other users.

I considered publishing my kernel driver on GitHub, but there appears to be a
sufficiently large number of users to justify having this functionality
upstream.

Hope that addresses your concerns.

> Ciao
> Stephan

Thanks,
Elena

  reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-13 16:48 Elena Petrova
2020-07-13 16:48 ` [PATCH 1/1] " Elena Petrova
2020-07-13 17:10   ` Eric Biggers
2020-07-16 14:23     ` Elena Petrova
2020-07-16 16:40       ` [PATCH v2] " Elena Petrova
2020-07-20 17:35         ` Stephan Mueller
2020-07-21 12:55           ` Elena Petrova
2020-07-21 13:18             ` Stephan Mueller
2020-07-28 16:16               ` Elena Petrova
2020-07-20 17:42         ` Stephan Müller
2020-07-22 15:59         ` Eric Biggers
2020-07-28 15:51           ` [PATCH v3] " Elena Petrova
2020-07-28 17:36             ` Eric Biggers
2020-07-29 15:45               ` [PATCH v4] " Elena Petrova
2020-07-29 19:26                 ` Stephan Müller
2020-07-31  7:23                 ` Herbert Xu
2020-08-03 14:48                   ` Elena Petrova
2020-08-03 15:10                     ` Stephan Mueller
2020-08-03 15:30                       ` Elena Petrova
2020-08-04  2:18                     ` Herbert Xu
2020-07-13 17:25   ` [PATCH 1/1] " Eric Biggers
2020-07-31  7:26     ` Herbert Xu
2020-08-13 16:00       ` Elena Petrova
2020-08-13 16:01         ` [PATCH v4] " Elena Petrova
2020-08-13 16:04           ` Elena Petrova
2020-08-13 16:08             ` [PATCH v5] " Elena Petrova
2020-08-13 19:32               ` Eric Biggers
2020-08-21  4:24                 ` Herbert Xu
2020-09-08 17:04                   ` [PATCH v6] " Elena Petrova
2020-09-09  4:35                     ` Eric Biggers
2020-09-09 18:29                       ` [PATCH v7] " Elena Petrova
2020-09-09 21:00                         ` Eric Biggers
2020-09-16 11:07                           ` [PATCH v8] " Elena Petrova
2020-09-18  6:43                             ` Herbert Xu
2020-09-18 15:42                               ` [PATCH v9] " Elena Petrova
2020-09-08 17:23                   ` [PATCH v5] " Elena Petrova
2020-09-08 17:18                 ` Elena Petrova
2020-07-14  5:17 ` [PATCH 0/1] " Stephan Mueller
2020-07-14 15:23   ` Elena Petrova [this message]
2020-07-14 15:34     ` Stephan Mueller
2020-07-16 14:41       ` Elena Petrova
2020-07-16 14:49         ` Stephan Mueller
2020-07-16 14:59           ` Stephan Mueller

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=CABvBcwauK_JyVzONdwJRGU81ZH5sYuiJSH0F2g+i5qCe363+fQ@mail.gmail.com \
    --to=lenaptr@google.com \
    --cc=ardb@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=smueller@chronox.de \
    /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

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org
	public-inbox-index linux-crypto

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git