Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Stephan Mueller <smueller@chronox.de>
To:     Elena Petrova <lenaptr@google.com>
Cc:     "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@vger.kernel.org>, Eric Biggers <ebiggers@kernel.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Jeffrey Vander Stoep <jeffv@google.com>
Subject: Re: [PATCH v2] crypto: af_alg - add extra parameters for DRBG interface
Date:   Tue, 21 Jul 2020 15:18:54 +0200
Message-ID: <9149882.4vTCxPXJkl@tauon.chronox.de>
In-Reply-To: <CABvBcwZ5mQPVFNpw=mY29cXo8oU8yviW5FZEFdKBNLvaudH6Ow@mail.gmail.com>
References: <CABvBcwY44BPa+TaDwxWaEogpg3Kdkq8o9cR5gSqNGF-o6d3jrw@mail.gmail.com> <13569541.ZYm5mLc6kN@tauon.chronox.de> <CABvBcwZ5mQPVFNpw=mY29cXo8oU8yviW5FZEFdKBNLvaudH6Ow@mail.gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

Am Dienstag, 21. Juli 2020, 14:55:14 CEST schrieb Elena Petrova:

Hi Elena,

> > > +#ifdef CONFIG_CRYPTO_CAVS_DRBG
> > > +static int rng_setentropy(void *private, const u8 *entropy, unsigned
> > > int
> > > len) +{
> > > +     struct rng_parent_ctx *pctx = private;
> > > +     u8 *kentropy = NULL;
> > > +
> > > +     if (!capable(CAP_SYS_ADMIN))
> > > +             return -EPERM;
> > > +
> > > +     if (pctx->entropy)
> > > +             return -EINVAL;
> > > +
> > > +     if (len > MAXSIZE)
> > > +             len = MAXSIZE;
> > > +
> > > +     if (len) {
> > > +             kentropy = memdup_user(entropy, len);
> > > +             if (IS_ERR(kentropy))
> > > +                     return PTR_ERR(kentropy);
> > > +     }
> > > +
> > > +     crypto_rng_alg(pctx->drng)->set_ent(pctx->drng, kentropy, len);
> > > +     pctx->entropy = kentropy;
> > 
> > Why do you need to keep kentropy around? For the check above whether
> > entropy was set, wouldn't a boolean suffice?
> 
> I need to keep the pointer to free it after use. Unlike the setting of
> the key, DRBG saves the entropy pointer in one of its internal
> structures, but doesn't do any memory
> management. I had only two ideas on how to prevent memory leaks:
> either change drbg code to deal with the memory, or save the pointer
> somewhere inside the socket. I opted for the latter. But if you know a
> better approach I'm happy to rework my code accordingly.

I was thinking of calling crypto_rng_alg(pctx->drng)->seed() directly after 
set_ent. This call performs a DRBG instantatiate where the entropy buffer is 
used. See crypto_drbg_reset_test for the approach.

But maybe you are right, the test "entropy" buffer inside the DRBG currently 
cannot be reset. So, for sanity purposes, you need to keep it around.

Ciao
Stephan