* Re: George's crazy full state idea (Re: HalfSipHash Acceptable Usage)
From: Hannes Frederic Sowa @ 2016-12-23 12:05 UTC (permalink / raw)
To: George Spelvin, luto
Cc: ak, davem, David.Laight, djb, ebiggers3, eric.dumazet, Jason,
jeanphilippe.aumasson, kernel-hardening, linux-crypto,
linux-kernel, netdev, tom, torvalds, tytso, vegard.nossum
In-Reply-To: <20161223000716.5768.qmail@ns.sciencehorizons.net>
On Thu, 2016-12-22 at 19:07 -0500, George Spelvin wrote:
> Hannes Frederic Sowa wrote:
> > A lockdep test should still be done. ;)
>
> Adding might_lock() annotations will improve coverage a lot.
Might be hard to find the correct lock we take later down the code
path, but if that is possible, certainly.
> > Yes, that does look nice indeed. Accounting for bits instead of bytes
> > shouldn't be a huge problem either. Maybe it gets a bit more verbose in
> > case you can't satisfy a request with one batched entropy block and have
> > to consume randomness from two.
>
> The bit granularity is also for the callers' convenience, so they don't
> have to mask again. Whether get_random_bits rounds up to byte boundaries
> internally or not is something else.
>
> When the current batch runs low, I was actually thinking of throwing
> away the remaining bits and computing a new batch of 512. But it's
> whatever works best at implementation time.
>
> > > > It could only mix the output back in every two calls, in which case
> > > > you can backtrack up to one call but you need to do 2^128 work to
> > > > backtrack farther. But yes, this is getting excessively complicated.
> > > No, if you're willing to accept limited backtrack, this is a perfectly
> > > acceptable solution, and not too complicated. You could do it phase-less
> > > if you like; store the previous output, then after generating the new
> > > one, mix in both. Then overwrite the previous output. (But doing two
> > > rounds of a crypto primtive to avoid one conditional jump is stupid,
> > > so forget that.)
> > Can you quickly explain why we lose the backtracking capability?
>
> Sure. An RNG is (state[i], output[i]) = f(state[i-1]). The goal of
> backtracking is to compute output[i], or better yet state[i-1], given
> state[i].
>
> For example, consider an OFB or CTR mode generator. The state is a key
> and and IV, and you encrypt the IV with the key to produce output, then
> either replace the IV with the output, or increment it. Either way,
> since you still have the key, you can invert the transformation and
> recover the previous IV.
>
> The standard way around this is to use the Davies-Meyer construction:
>
> IV[i] = IV[i-1] + E(IV[i-1], key)
>
> This is the standard way to make a non-invertible random function
> out of an invertible random permutation.
>
> From the sum, there's no easy way to find the ciphertext *or* the
> plaintext that was encrypted. Assuming the encryption is secure,
> the only way to reverse it is brute force: guess IV[i-1] and run the
> operation forward to see if the resultant IV[i] matches.
>
> There are a variety of ways to organize this computation, since the
> guess gives toy both IV[i-1] and E(IV[i-1], key) = IV[i] - IV[i-1], including
> running E forward, backward, or starting from both ends to see if you
> meet in the middle.
>
> The way you add the encryption output to the IV is not very important.
> It can be addition, xor, or some more complex invertible transformation.
> In the case of SipHash, the "encryption" output is smaller than the
> input, so we have to get a bit more creative, but it's still basically
> the same thing.
>
> The problem is that the output which is combined with the IV is too small.
> With only 64 bits, trying all possible values is practical. (The world's
> Bitcoin miners are collectively computing SHA-256(SHA-256(input)) 1.7 * 2^64
> times per second.)
>
> By basically doing two iterations at once and mixing in 128 bits of
> output, the guessing attack is rendered impractical. The only downside
> is that you need to remember and store one result between when it's
> computed and last used. This is part of the state, so an attack can
> find output[i-1], but not anything farther back.
Thanks a lot for the explanation!
> > ChaCha as a block cipher gives a "perfect" permutation from the output
> > of either the CRNG or the CPRNG, which actually itself has backtracking
> > protection.
>
> I'm not quite understanding. The /dev/random implementation uses some
> of the ChaCha output as a new ChaCha key (that's another way to mix output
> back into the state) to prevent backtracking. But this slows it down, and
> again if you want to be efficient, you're generating and storing large batches
> of entropy and storing it in the RNG state.
I was actually referring to the anti-backtrack protection in
/dev/random and also /dev/urandom, from where we reseed every 300
seconds and if our batched entropy runs low with Ted's/Jason's current
patch for get_random_int.
As far as I can understand it, backtracking is not a problem in case of
a reseed event inside extract_crng.
When we hit the chacha20 without doing a reseed we only mutate the
state of chacha, but being an invertible function in its own, a
proposal would be to mix parts of the chacha20 output back into the
state, which, as a result, would cause slowdown because we couldn't
propagate the complete output of the cipher back to the caller (looking
at the function _extract_crng).
Or are you referring that the anti-backtrack protection should happen
in every call from get_random_int?
Thanks,
Hannes
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Daniel Borkmann @ 2016-12-23 11:59 UTC (permalink / raw)
To: Hannes Frederic Sowa, Andy Lutomirski, Alexei Starovoitov
Cc: Jason A. Donenfeld, kernel-hardening@lists.openwall.com,
Theodore Ts'o, Netdev, LKML, Linux Crypto Mailing List,
David Laight, Eric Dumazet, Linus Torvalds, Eric Biggers,
Tom Herbert, Andi Kleen, David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <1482490762.3353.2.camel@stressinduktion.org>
On 12/23/2016 11:59 AM, Hannes Frederic Sowa wrote:
> On Fri, 2016-12-23 at 11:04 +0100, Daniel Borkmann wrote:
>> On 12/22/2016 05:59 PM, Hannes Frederic Sowa wrote:
>>> On Thu, 2016-12-22 at 08:07 -0800, Andy Lutomirski wrote:
[...]
>>> The hashing is not a proper sha1 neither, unfortunately. I think that
>>> is why it will have a custom implementation in iproute2?
>>
>> Still trying to catch up on this admittedly bit confusing thread. I
>> did run automated tests over couple of days comparing the data I got
>> from fdinfo with the one from af_alg and found no mismatch on the test
>> cases varying from min to max possible program sizes. In the process
>> of testing, as you might have seen on netdev, I found couple of other
>> bugs in bpf code along the way and fixed them up as well. So my question,
>> do you or Andy or anyone participating in claiming this have any
>> concrete data or test cases that suggests something different? If yes,
>> I'm very curious to hear about it and willing fix it up, of course.
>> When I'm back from pto I'll prep and cook up my test suite to be
>> included into the selftests/bpf/, should have done this initially,
>> sorry about that. I'll also post something to expose the alg, that
>> sounds fine to me.
>
> Looking into your code closer, I noticed that you indeed seem to do the
> finalization of sha-1 by hand by aligning and padding the buffer
> accordingly and also patching in the necessary payload length.
>
> Apologies for my side for claiming that this is not correct sha1
> output, I was only looking at sha_transform and its implementation and
> couldn't see the padding and finalization round with embedding the data
> length in there and hadn't thought of it being done manually.
>
> Anyway, is it difficult to get the sha finalization into some common
> code library? It is not very bpf specific and crypto code reviewers
> won't find it there at all.
Yes, sure, I'll rework it that way (early next year when I'm back if
that's fine with you).
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v2 11/12] crypto: atmel-authenc: add support to authenc(hmac(shaX), Y(aes)) modes
From: Stephan Müller @ 2016-12-23 11:34 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: herbert, nicolas.ferre, linux-kernel, linux-crypto, davem,
linux-arm-kernel
In-Reply-To: <2a6cc4637621d2ef0d84754817128c43795cb022.1482424395.git.cyrille.pitchen@atmel.com>
Am Donnerstag, 22. Dezember 2016, 17:38:00 CET schrieb Cyrille Pitchen:
Hi Cyrille,
> This patchs allows to combine the AES and SHA hardware accelerators on
> some Atmel SoCs. Doing so, AES blocks are only written to/read from the
> AES hardware. Those blocks are also transferred from the AES to the SHA
> accelerator internally, without additionnal accesses to the system busses.
>
> Hence, the AES and SHA accelerators work in parallel to process all the
> data blocks, instead of serializing the process by (de)crypting those
> blocks first then authenticating them after like the generic
> crypto/authenc.c driver does.
>
> Of course, both the AES and SHA hardware accelerators need to be available
> before we can start to process the data blocks. Hence we use their crypto
> request queue to synchronize both drivers.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
> drivers/crypto/Kconfig | 12 +
> drivers/crypto/atmel-aes-regs.h | 16 ++
> drivers/crypto/atmel-aes.c | 471
> +++++++++++++++++++++++++++++++++++++++- drivers/crypto/atmel-authenc.h |
> 64 ++++++
> drivers/crypto/atmel-sha-regs.h | 14 ++
> drivers/crypto/atmel-sha.c | 344 +++++++++++++++++++++++++++--
> 6 files changed, 906 insertions(+), 15 deletions(-)
> create mode 100644 drivers/crypto/atmel-authenc.h
>
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 79564785ae30..719a868d8ea1 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -415,6 +415,18 @@ config CRYPTO_DEV_BFIN_CRC
> Newer Blackfin processors have CRC hardware. Select this if you
> want to use the Blackfin CRC module.
>
> +config CRYPTO_DEV_ATMEL_AUTHENC
> + tristate "Support for Atmel IPSEC/SSL hw accelerator"
> + depends on (ARCH_AT91 && HAS_DMA) || COMPILE_TEST
> + select CRYPTO_AUTHENC
> + select CRYPTO_DEV_ATMEL_AES
> + select CRYPTO_DEV_ATMEL_SHA
> + help
> + Some Atmel processors can combine the AES and SHA hw accelerators
> + to enhance support of IPSEC/SSL.
> + Select this if you want to use the Atmel modules for
> + authenc(hmac(shaX),Y(cbc)) algorithms.
> +
> config CRYPTO_DEV_ATMEL_AES
> tristate "Support for Atmel AES hw accelerator"
> depends on HAS_DMA
> diff --git a/drivers/crypto/atmel-aes-regs.h
> b/drivers/crypto/atmel-aes-regs.h index 0ec04407b533..7694679802b3 100644
> --- a/drivers/crypto/atmel-aes-regs.h
> +++ b/drivers/crypto/atmel-aes-regs.h
> @@ -68,6 +68,22 @@
> #define AES_CTRR 0x98
> #define AES_GCMHR(x) (0x9c + ((x) * 0x04))
>
> +#define AES_EMR 0xb0
> +#define AES_EMR_APEN BIT(0) /* Auto Padding Enable */
> +#define AES_EMR_APM BIT(1) /* Auto Padding Mode */
> +#define AES_EMR_APM_IPSEC 0x0
> +#define AES_EMR_APM_SSL BIT(1)
> +#define AES_EMR_PLIPEN BIT(4) /* PLIP Enable */
> +#define AES_EMR_PLIPD BIT(5) /* PLIP Decipher */
> +#define AES_EMR_PADLEN_MASK (0xFu << 8)
> +#define AES_EMR_PADLEN_OFFSET 8
> +#define AES_EMR_PADLEN(padlen) (((padlen) << AES_EMR_PADLEN_OFFSET) &\
> + AES_EMR_PADLEN_MASK)
> +#define AES_EMR_NHEAD_MASK (0xFu << 16)
> +#define AES_EMR_NHEAD_OFFSET 16
> +#define AES_EMR_NHEAD(nhead) (((nhead) << AES_EMR_NHEAD_OFFSET) &\
> + AES_EMR_NHEAD_MASK)
> +
> #define AES_TWR(x) (0xc0 + ((x) * 0x04))
> #define AES_ALPHAR(x) (0xd0 + ((x) * 0x04))
>
> diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
> index 9fd2f63b8bc0..3c651e0c3113 100644
> --- a/drivers/crypto/atmel-aes.c
> +++ b/drivers/crypto/atmel-aes.c
> @@ -41,6 +41,7 @@
> #include <linux/platform_data/crypto-atmel.h>
> #include <dt-bindings/dma/at91.h>
> #include "atmel-aes-regs.h"
> +#include "atmel-authenc.h"
>
> #define ATMEL_AES_PRIORITY 300
>
> @@ -78,6 +79,7 @@
> #define AES_FLAGS_INIT BIT(2)
> #define AES_FLAGS_BUSY BIT(3)
> #define AES_FLAGS_DUMP_REG BIT(4)
> +#define AES_FLAGS_OWN_SHA BIT(5)
>
> #define AES_FLAGS_PERSISTENT (AES_FLAGS_INIT | AES_FLAGS_BUSY)
>
> @@ -92,6 +94,7 @@ struct atmel_aes_caps {
> bool has_ctr32;
> bool has_gcm;
> bool has_xts;
> + bool has_authenc;
> u32 max_burst_size;
> };
>
> @@ -144,10 +147,31 @@ struct atmel_aes_xts_ctx {
> u32 key2[AES_KEYSIZE_256 / sizeof(u32)];
> };
>
> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
> +struct atmel_aes_authenc_ctx {
> + struct atmel_aes_base_ctx base;
> + struct atmel_sha_authenc_ctx *auth;
> +};
> +#endif
> +
> struct atmel_aes_reqctx {
> unsigned long mode;
> };
>
> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
> +struct atmel_aes_authenc_reqctx {
> + struct atmel_aes_reqctx base;
> +
> + struct scatterlist src[2];
> + struct scatterlist dst[2];
> + size_t textlen;
> + u32 digest[SHA512_DIGEST_SIZE / sizeof(u32)];
> +
> + /* auth_req MUST be place last. */
> + struct ahash_request auth_req;
> +};
> +#endif
> +
> struct atmel_aes_dma {
> struct dma_chan *chan;
> struct scatterlist *sg;
> @@ -291,6 +315,9 @@ static const char *atmel_aes_reg_name(u32 offset, char
> *tmp, size_t sz) snprintf(tmp, sz, "GCMHR[%u]", (offset - AES_GCMHR(0)) >>
> 2);
> break;
>
> + case AES_EMR:
> + return "EMR";
> +
> case AES_TWR(0):
> case AES_TWR(1):
> case AES_TWR(2):
> @@ -463,8 +490,16 @@ static inline bool atmel_aes_is_encrypt(const struct
> atmel_aes_dev *dd) return (dd->flags & AES_FLAGS_ENCRYPT);
> }
>
> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
> +static void atmel_aes_authenc_complete(struct atmel_aes_dev *dd, int err);
> +#endif
> +
> static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err)
> {
> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
> + atmel_aes_authenc_complete(dd, err);
> +#endif
> +
> clk_disable(dd->iclk);
> dd->flags &= ~AES_FLAGS_BUSY;
>
> @@ -1931,6 +1966,407 @@ static struct crypto_alg aes_xts_alg = {
> }
> };
>
> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
> +/* authenc aead functions */
> +
> +static int atmel_aes_authenc_start(struct atmel_aes_dev *dd);
> +static int atmel_aes_authenc_init(struct atmel_aes_dev *dd, int err,
> + bool is_async);
> +static int atmel_aes_authenc_transfer(struct atmel_aes_dev *dd, int err,
> + bool is_async);
> +static int atmel_aes_authenc_digest(struct atmel_aes_dev *dd);
> +static int atmel_aes_authenc_final(struct atmel_aes_dev *dd, int err,
> + bool is_async);
> +
> +static void atmel_aes_authenc_complete(struct atmel_aes_dev *dd, int err)
> +{
> + struct aead_request *req = aead_request_cast(dd->areq);
> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
> +
> + if (err && (dd->flags & AES_FLAGS_OWN_SHA))
> + atmel_sha_authenc_abort(&rctx->auth_req);
> + dd->flags &= ~AES_FLAGS_OWN_SHA;
> +}
> +
> +static int atmel_aes_authenc_copy_assoc(struct aead_request *req)
> +{
> + size_t buflen, assoclen = req->assoclen;
> + off_t skip = 0;
> + u8 buf[256];
> +
> + while (assoclen) {
> + buflen = min_t(size_t, assoclen, sizeof(buf));
> +
> + if (sg_pcopy_to_buffer(req->src, sg_nents(req->src),
> + buf, buflen, skip) != buflen)
> + return -EINVAL;
> +
> + if (sg_pcopy_from_buffer(req->dst, sg_nents(req->dst),
> + buf, buflen, skip) != buflen)
> + return -EINVAL;
> +
> + skip += buflen;
> + assoclen -= buflen;
> + }
This seems to be a very expansive operation. Wouldn't it be easier, leaner and
with one less memcpy to use the approach of crypto_authenc_copy_assoc?
Instead of copying crypto_authenc_copy_assoc, what about carving the logic in
crypto/authenc.c out into a generic aead helper code as we need to add that to
other AEAD implementations?
> +
> + return 0;
> +}
> +
> +static int atmel_aes_authenc_start(struct atmel_aes_dev *dd)
> +{
> + struct aead_request *req = aead_request_cast(dd->areq);
> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> + struct atmel_aes_authenc_ctx *ctx = crypto_aead_ctx(tfm);
> + int err;
> +
> + atmel_aes_set_mode(dd, &rctx->base);
> +
> + err = atmel_aes_hw_init(dd);
> + if (err)
> + return atmel_aes_complete(dd, err);
> +
> + return atmel_sha_authenc_schedule(&rctx->auth_req, ctx->auth,
> + atmel_aes_authenc_init, dd);
> +}
> +
> +static int atmel_aes_authenc_init(struct atmel_aes_dev *dd, int err,
> + bool is_async)
> +{
> + struct aead_request *req = aead_request_cast(dd->areq);
> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
> +
> + if (is_async)
> + dd->is_async = true;
> + if (err)
> + return atmel_aes_complete(dd, err);
> +
> + /* If here, we've got the ownership of the SHA device. */
> + dd->flags |= AES_FLAGS_OWN_SHA;
> +
> + /* Configure the SHA device. */
> + return atmel_sha_authenc_init(&rctx->auth_req,
> + req->src, req->assoclen,
> + rctx->textlen,
> + atmel_aes_authenc_transfer, dd);
> +}
> +
> +static int atmel_aes_authenc_transfer(struct atmel_aes_dev *dd, int err,
> + bool is_async)
> +{
> + struct aead_request *req = aead_request_cast(dd->areq);
> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
> + bool enc = atmel_aes_is_encrypt(dd);
> + struct scatterlist *src, *dst;
> + u32 iv[AES_BLOCK_SIZE / sizeof(u32)];
> + u32 emr;
> +
> + if (is_async)
> + dd->is_async = true;
> + if (err)
> + return atmel_aes_complete(dd, err);
> +
> + /* Prepare src and dst scatter-lists to transfer cipher/plain texts. */
> + src = scatterwalk_ffwd(rctx->src, req->src, req->assoclen);
> + dst = src;
> +
> + if (req->src != req->dst) {
> + err = atmel_aes_authenc_copy_assoc(req);
> + if (err)
> + return atmel_aes_complete(dd, err);
> +
> + dst = scatterwalk_ffwd(rctx->dst, req->dst, req->assoclen);
> + }
> +
> + /* Configure the AES device. */
> + memcpy(iv, req->iv, sizeof(iv));
> +
> + /*
> + * Here we always set the 2nd parameter of atmel_aes_write_ctrl() to
> + * 'true' even if the data transfer is actually performed by the CPU (so
> + * not by the DMA) because we must force the AES_MR_SMOD bitfield to the
> + * value AES_MR_SMOD_IDATAR0. Indeed, both AES_MR_SMOD and SHA_MR_SMOD
> + * must be set to *_MR_SMOD_IDATAR0.
> + */
> + atmel_aes_write_ctrl(dd, true, iv);
> + emr = AES_EMR_PLIPEN;
> + if (!enc)
> + emr |= AES_EMR_PLIPD;
> + atmel_aes_write(dd, AES_EMR, emr);
> +
> + /* Transfer data. */
> + return atmel_aes_dma_start(dd, src, dst, rctx->textlen,
> + atmel_aes_authenc_digest);
> +}
> +
> +static int atmel_aes_authenc_digest(struct atmel_aes_dev *dd)
> +{
> + struct aead_request *req = aead_request_cast(dd->areq);
> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
> +
> + /* atmel_sha_authenc_final() releases the SHA device. */
> + dd->flags &= ~AES_FLAGS_OWN_SHA;
> + return atmel_sha_authenc_final(&rctx->auth_req,
> + rctx->digest, sizeof(rctx->digest),
> + atmel_aes_authenc_final, dd);
> +}
> +
> +static int atmel_aes_authenc_final(struct atmel_aes_dev *dd, int err,
> + bool is_async)
> +{
> + struct aead_request *req = aead_request_cast(dd->areq);
> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> + bool enc = atmel_aes_is_encrypt(dd);
> + u32 idigest[SHA512_DIGEST_SIZE / sizeof(u32)], *odigest = rctx->digest;
> + u32 offs, authsize;
> +
> + if (is_async)
> + dd->is_async = true;
> + if (err)
> + goto complete;
> +
> + offs = req->assoclen + rctx->textlen;
> + authsize = crypto_aead_authsize(tfm);
> + if (enc) {
> + scatterwalk_map_and_copy(odigest, req->dst, offs, authsize, 1);
> + } else {
> + scatterwalk_map_and_copy(idigest, req->src, offs, authsize, 0);
> + if (crypto_memneq(idigest, odigest, authsize))
> + err = -EBADMSG;
> + }
> +
> +complete:
> + return atmel_aes_complete(dd, err);
> +}
> +
> +static int atmel_aes_authenc_setkey(struct crypto_aead *tfm, const u8 *key,
> + unsigned int keylen)
> +{
> + struct atmel_aes_authenc_ctx *ctx = crypto_aead_ctx(tfm);
> + struct crypto_authenc_keys keys;
> + u32 flags;
> + int err;
> +
> + if (crypto_authenc_extractkeys(&keys, key, keylen) != 0)
> + goto badkey;
> +
> + if (keys.enckeylen > sizeof(ctx->base.key))
> + goto badkey;
> +
> + /* Save auth key. */
> + flags = crypto_aead_get_flags(tfm);
> + err = atmel_sha_authenc_setkey(ctx->auth,
> + keys.authkey, keys.authkeylen,
> + &flags);
> + crypto_aead_set_flags(tfm, flags & CRYPTO_TFM_RES_MASK);
> + if (err)
> + return err;
> +
> + /* Save enc key. */
> + ctx->base.keylen = keys.enckeylen;
> + memcpy(ctx->base.key, keys.enckey, keys.enckeylen);
memzero_explicit(keys) please
> +
> + return 0;
> +
> +badkey:
> + crypto_aead_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return -EINVAL;
> +}
> +
> +static int atmel_aes_authenc_init_tfm(struct crypto_aead *tfm,
> + unsigned long auth_mode)
> +{
> + struct atmel_aes_authenc_ctx *ctx = crypto_aead_ctx(tfm);
> + unsigned int auth_reqsize = atmel_sha_authenc_get_reqsize();
> +
> + ctx->auth = atmel_sha_authenc_spawn(auth_mode);
> + if (IS_ERR(ctx->auth))
> + return PTR_ERR(ctx->auth);
> +
> + crypto_aead_set_reqsize(tfm, (sizeof(struct atmel_aes_authenc_reqctx) +
> + auth_reqsize));
> + ctx->base.start = atmel_aes_authenc_start;
> +
> + return 0;
> +}
> +
> +static int atmel_aes_authenc_hmac_sha1_init_tfm(struct crypto_aead *tfm)
> +{
> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA1);
> +}
> +
> +static int atmel_aes_authenc_hmac_sha224_init_tfm(struct crypto_aead *tfm)
> +{
> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA224);
> +}
> +
> +static int atmel_aes_authenc_hmac_sha256_init_tfm(struct crypto_aead *tfm)
> +{
> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA256);
> +}
> +
> +static int atmel_aes_authenc_hmac_sha384_init_tfm(struct crypto_aead *tfm)
> +{
> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA384);
> +}
> +
> +static int atmel_aes_authenc_hmac_sha512_init_tfm(struct crypto_aead *tfm)
> +{
> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA512);
> +}
> +
> +static void atmel_aes_authenc_exit_tfm(struct crypto_aead *tfm)
> +{
> + struct atmel_aes_authenc_ctx *ctx = crypto_aead_ctx(tfm);
> +
> + atmel_sha_authenc_free(ctx->auth);
> +}
> +
> +static int atmel_aes_authenc_crypt(struct aead_request *req,
> + unsigned long mode)
> +{
> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> + struct atmel_aes_base_ctx *ctx = crypto_aead_ctx(tfm);
> + u32 authsize = crypto_aead_authsize(tfm);
> + bool enc = (mode & AES_FLAGS_ENCRYPT);
> + struct atmel_aes_dev *dd;
> +
> + /* Compute text length. */
> + rctx->textlen = req->cryptlen - (enc ? 0 : authsize);
Is there somewhere a check that authsize is always < req->cryptlen (at least
it escaped me)? Note, this logic will be exposed to user space which may do
funky things.
> +
> + /*
> + * Currently, empty messages are not supported yet:
> + * the SHA auto-padding can be used only on non-empty messages.
> + * Hence a special case needs to be implemented for empty message.
> + */
> + if (!rctx->textlen && !req->assoclen)
> + return -EINVAL;
> +
> + rctx->base.mode = mode;
> + ctx->block_size = AES_BLOCK_SIZE;
> +
> + dd = atmel_aes_find_dev(ctx);
> + if (!dd)
> + return -ENODEV;
> +
> + return atmel_aes_handle_queue(dd, &req->base);
Ciao
Stephan
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Hannes Frederic Sowa @ 2016-12-23 10:59 UTC (permalink / raw)
To: Daniel Borkmann, Andy Lutomirski, Alexei Starovoitov
Cc: Jason A. Donenfeld, kernel-hardening@lists.openwall.com,
Theodore Ts'o, Netdev, LKML, Linux Crypto Mailing List,
David Laight, Eric Dumazet, Linus Torvalds, Eric Biggers,
Tom Herbert, Andi Kleen, David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <585CF6A3.1050107@iogearbox.net>
On Fri, 2016-12-23 at 11:04 +0100, Daniel Borkmann wrote:
> On 12/22/2016 05:59 PM, Hannes Frederic Sowa wrote:
> > On Thu, 2016-12-22 at 08:07 -0800, Andy Lutomirski wrote:
> > > On Thu, Dec 22, 2016 at 7:51 AM, Hannes Frederic Sowa
> > > <hannes@stressinduktion.org> wrote:
> > > > On Thu, 2016-12-22 at 16:41 +0100, Jason A. Donenfeld wrote:
> > > > > On Thu, Dec 22, 2016 at 4:33 PM, Hannes Frederic Sowa
> > > > > <hannes@stressinduktion.org> wrote:
> > > > > > IPv6 you cannot touch anymore. The hashing algorithm is part of uAPI.
> > > > > > You don't want to give people new IPv6 addresses with the same stable
> > > > > > secret (across reboots) after a kernel upgrade. Maybe they lose
> > > > > > connectivity then and it is extra work?
> > > > >
> > > > > Ahh, too bad. So it goes.
> > > >
> > > > If no other users survive we can put it into the ipv6 module.
> > > >
> > > > > > The bpf hash stuff can be changed during this merge window, as it is
> > > > > > not yet in a released kernel. Albeit I would probably have preferred
> > > > > > something like sha256 here, which can be easily replicated by user
> > > > > > space tools (minus the problem of patching out references to not
> > > > > > hashable data, which must be zeroed).
> > > > >
> > > > > Oh, interesting, so time is of the essence then. Do you want to handle
> > > > > changing the new eBPF code to something not-SHA1 before it's too late,
> > > > > as part of a ne
> > >
> > > w patchset that can fast track itself to David? And
> > > > > then I can preserve my large series for the next merge window.
> > > >
> > > > This algorithm should be a non-seeded algorithm, because the hashes
> > > > should be stable and verifiable by user space tooling. Thus this would
> > > > need a hashing algorithm that is hardened against pre-image
> > > > attacks/collision resistance, which siphash is not. I would prefer some
> > > > higher order SHA algorithm for that actually.
> > > >
> > >
> > > You mean:
> > >
> > > commit 7bd509e311f408f7a5132fcdde2069af65fa05ae
> > > Author: Daniel Borkmann <daniel@iogearbox.net>
> > > Date: Sun Dec 4 23:19:41 2016 +0100
> > >
> > > bpf: add prog_digest and expose it via fdinfo/netlink
> > >
> > >
> > > Yes, please! This actually matters for security -- imagine a
> > > malicious program brute-forcing a collision so that it gets loaded
> > > wrong. And this is IMO a use case for SHA-256 or SHA-512/256
> > > (preferably the latter). Speed basically doesn't matter here and
> > > Blake2 is both less stable (didn't they slightly change it recently?)
> > > and much less well studied.
> >
> > We don't prevent ebpf programs being loaded based on the digest but
> > just to uniquely identify loaded programs from user space and match up
> > with their source.
> >
> > There have been talks about signing bpf programs, thus this would
> > probably need another digest algorithm additionally to that one,
> > wasting probably instructions. Probably going somewhere in direction of
> > PKCS#7 might be the thing to do (which leads to the problem to make
> > PKCS#7 attackable by ordinary unpriv users, hmpf).
> >
> > > My inclination would have been to seed them with something that isn't
> > > exposed to userspace for the precise reason that it would prevent user
> > > code from making assumptions about what's in the hash. But if there's
> > > a use case for why user code needs to be able to calculate the hash on
> > > its own, then that's fine. But perhaps the actual fdinfo string
> > > should be "sha256:abcd1234..." to give some flexibility down the road.
> > >
> > > Also:
> > >
> > > + result = (__force __be32 *)fp->digest;
> > > + for (i = 0; i < SHA_DIGEST_WORDS; i++)
> > > + result[i] = cpu_to_be32(fp->digest[i]);
> > >
> > > Everyone, please, please, please don't open-code crypto primitives.
> > > Is this and the code above it even correct? It might be but on a very
> > > brief glance it looks wrong to me. If you're doing this to avoid
> > > depending on crypto, then fix crypto so you can pull in the algorithm
> > > without pulling in the whole crypto core.
> >
> > The hashing is not a proper sha1 neither, unfortunately. I think that
> > is why it will have a custom implementation in iproute2?
>
> Still trying to catch up on this admittedly bit confusing thread. I
> did run automated tests over couple of days comparing the data I got
> from fdinfo with the one from af_alg and found no mismatch on the test
> cases varying from min to max possible program sizes. In the process
> of testing, as you might have seen on netdev, I found couple of other
> bugs in bpf code along the way and fixed them up as well. So my question,
> do you or Andy or anyone participating in claiming this have any
> concrete data or test cases that suggests something different? If yes,
> I'm very curious to hear about it and willing fix it up, of course.
> When I'm back from pto I'll prep and cook up my test suite to be
> included into the selftests/bpf/, should have done this initially,
> sorry about that. I'll also post something to expose the alg, that
> sounds fine to me.
Looking into your code closer, I noticed that you indeed seem to do the
finalization of sha-1 by hand by aligning and padding the buffer
accordingly and also patching in the necessary payload length.
Apologies for my side for claiming that this is not correct sha1
output, I was only looking at sha_transform and its implementation and
couldn't see the padding and finalization round with embedding the data
length in there and hadn't thought of it being done manually.
Anyway, is it difficult to get the sha finalization into some common
code library? It is not very bpf specific and crypto code reviewers
won't find it there at all.
Thanks,
Hannes
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Daniel Borkmann @ 2016-12-23 10:23 UTC (permalink / raw)
To: Andy Lutomirski, Hannes Frederic Sowa
Cc: Alexei Starovoitov, Jason A. Donenfeld,
kernel-hardening@lists.openwall.com, Theodore Ts'o, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <CALCETrW_T1v4qKPJDs5dXwAnAit3M52AWMH-K+GJLb1WoLMuRQ@mail.gmail.com>
On 12/22/2016 06:25 PM, Andy Lutomirski wrote:
> On Thu, Dec 22, 2016 at 8:59 AM, Hannes Frederic Sowa
[...]
>> I wondered if bpf program loading should have used the module loading
>> infrastructure from the beginning...
>
> That would be way too complicated and would be nasty for the unprivileged cases.
Also, there are users be it privileged or not that don't require to have a full
obj loader from user space, but are fine with just hard-coding parts or all of
the insns in their application. Back then we settled with using fds based on
Andy's suggestion, it has both ups and downs as we saw along the way but worked
okay thus far.
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Daniel Borkmann @ 2016-12-23 10:04 UTC (permalink / raw)
To: Hannes Frederic Sowa, Andy Lutomirski, Alexei Starovoitov
Cc: Jason A. Donenfeld, kernel-hardening@lists.openwall.com,
Theodore Ts'o, Netdev, LKML, Linux Crypto Mailing List,
David Laight, Eric Dumazet, Linus Torvalds, Eric Biggers,
Tom Herbert, Andi Kleen, David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <1482425969.2673.5.camel@stressinduktion.org>
On 12/22/2016 05:59 PM, Hannes Frederic Sowa wrote:
> On Thu, 2016-12-22 at 08:07 -0800, Andy Lutomirski wrote:
>> On Thu, Dec 22, 2016 at 7:51 AM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>>> On Thu, 2016-12-22 at 16:41 +0100, Jason A. Donenfeld wrote:
>>>> On Thu, Dec 22, 2016 at 4:33 PM, Hannes Frederic Sowa
>>>> <hannes@stressinduktion.org> wrote:
>>>>> IPv6 you cannot touch anymore. The hashing algorithm is part of uAPI.
>>>>> You don't want to give people new IPv6 addresses with the same stable
>>>>> secret (across reboots) after a kernel upgrade. Maybe they lose
>>>>> connectivity then and it is extra work?
>>>>
>>>> Ahh, too bad. So it goes.
>>>
>>> If no other users survive we can put it into the ipv6 module.
>>>
>>>>> The bpf hash stuff can be changed during this merge window, as it is
>>>>> not yet in a released kernel. Albeit I would probably have preferred
>>>>> something like sha256 here, which can be easily replicated by user
>>>>> space tools (minus the problem of patching out references to not
>>>>> hashable data, which must be zeroed).
>>>>
>>>> Oh, interesting, so time is of the essence then. Do you want to handle
>>>> changing the new eBPF code to something not-SHA1 before it's too late,
>>>> as part of a ne
>>
>> w patchset that can fast track itself to David? And
>>>> then I can preserve my large series for the next merge window.
>>>
>>> This algorithm should be a non-seeded algorithm, because the hashes
>>> should be stable and verifiable by user space tooling. Thus this would
>>> need a hashing algorithm that is hardened against pre-image
>>> attacks/collision resistance, which siphash is not. I would prefer some
>>> higher order SHA algorithm for that actually.
>>>
>>
>> You mean:
>>
>> commit 7bd509e311f408f7a5132fcdde2069af65fa05ae
>> Author: Daniel Borkmann <daniel@iogearbox.net>
>> Date: Sun Dec 4 23:19:41 2016 +0100
>>
>> bpf: add prog_digest and expose it via fdinfo/netlink
>>
>>
>> Yes, please! This actually matters for security -- imagine a
>> malicious program brute-forcing a collision so that it gets loaded
>> wrong. And this is IMO a use case for SHA-256 or SHA-512/256
>> (preferably the latter). Speed basically doesn't matter here and
>> Blake2 is both less stable (didn't they slightly change it recently?)
>> and much less well studied.
>
> We don't prevent ebpf programs being loaded based on the digest but
> just to uniquely identify loaded programs from user space and match up
> with their source.
>
> There have been talks about signing bpf programs, thus this would
> probably need another digest algorithm additionally to that one,
> wasting probably instructions. Probably going somewhere in direction of
> PKCS#7 might be the thing to do (which leads to the problem to make
> PKCS#7 attackable by ordinary unpriv users, hmpf).
>
>> My inclination would have been to seed them with something that isn't
>> exposed to userspace for the precise reason that it would prevent user
>> code from making assumptions about what's in the hash. But if there's
>> a use case for why user code needs to be able to calculate the hash on
>> its own, then that's fine. But perhaps the actual fdinfo string
>> should be "sha256:abcd1234..." to give some flexibility down the road.
>>
>> Also:
>>
>> + result = (__force __be32 *)fp->digest;
>> + for (i = 0; i < SHA_DIGEST_WORDS; i++)
>> + result[i] = cpu_to_be32(fp->digest[i]);
>>
>> Everyone, please, please, please don't open-code crypto primitives.
>> Is this and the code above it even correct? It might be but on a very
>> brief glance it looks wrong to me. If you're doing this to avoid
>> depending on crypto, then fix crypto so you can pull in the algorithm
>> without pulling in the whole crypto core.
>
> The hashing is not a proper sha1 neither, unfortunately. I think that
> is why it will have a custom implementation in iproute2?
Still trying to catch up on this admittedly bit confusing thread. I
did run automated tests over couple of days comparing the data I got
from fdinfo with the one from af_alg and found no mismatch on the test
cases varying from min to max possible program sizes. In the process
of testing, as you might have seen on netdev, I found couple of other
bugs in bpf code along the way and fixed them up as well. So my question,
do you or Andy or anyone participating in claiming this have any
concrete data or test cases that suggests something different? If yes,
I'm very curious to hear about it and willing fix it up, of course.
When I'm back from pto I'll prep and cook up my test suite to be
included into the selftests/bpf/, should have done this initially,
sorry about that. I'll also post something to expose the alg, that
sounds fine to me.
> I wondered if bpf program loading should have used the module loading
> infrastructure from the beginning...
>
>> At the very least, there should be a separate function that calculates
>> the hash of a buffer and that function should explicitly run itself
>> against test vectors of various lengths to make sure that it's
>> calculating what it claims to be calculating. And it doesn't look
>> like the userspace code has landed, so, if this thing isn't
>> calculating SHA1 correctly, it's plausible that no one has noticed.
>
> I hope this was known from the beginning, this is not sha1 unfortunately.
>
> But ebpf elf programs also need preprocessing to get rid of some
> embedded load-depending data, so maybe it was considered to be just
> enough?
>
> Bye,
> Hannes
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [RFC PATCH v2] crypto: Add IV generation algorithms
From: Herbert Xu @ 2016-12-23 7:51 UTC (permalink / raw)
To: Binoy Jayan
Cc: Milan Broz, Oded, Ofir, David S. Miller, linux-crypto, Mark Brown,
Arnd Bergmann, Linux kernel mailing list, Alasdair Kergon,
Mike Snitzer, dm-devel, Shaohua Li, linux-raid, Rajendra
In-Reply-To: <CAHv-k_-EKq2g=Wb+YkPVp9gCXTDEyrxZhQpT5JMSw=WtZ1OC9w@mail.gmail.com>
On Thu, Dec 22, 2016 at 04:25:12PM +0530, Binoy Jayan wrote:
>
> > It doesn't have to live outside of dm-crypt. You can register
> > these IV generators from there if you really want.
>
> Sorry, but I didn't understand this part.
What I mean is that moving the IV generators into the crypto API
does not mean the dm-crypt team giving up control over them. You
could continue to keep them within the dm-crypt code base and
still register them through the normal crypto API mechanism.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: Test AEAD/authenc algorithms from userspace
From: Harsh Jain @ 2016-12-23 5:46 UTC (permalink / raw)
To: Herbert Xu; +Cc: Stephan Mueller, linux-crypto
In-Reply-To: <20161221085429.GB29501@gondor.apana.org.au>
On 21-12-2016 14:24, Herbert Xu wrote:
> On Mon, Dec 19, 2016 at 04:08:11PM +0530, Harsh Jain wrote:
>> Hi Herbert,
>>
>> TLS default mode of operation is MAC-then-Encrypt for Authenc algos.
>> Currently framework only supports EtM used in IPSec. User space
>> programs like openssl cannot use af-alg interface to encrypt/decrypt
>> in TLS mode.
>> Are we going to support Mac-then-Encrypt mode in future kernel releases?
> If someone finally adds TLS to the kernel then we'll likely do
> something about it.
Till that time we cannot use crypto authenc type algos with AF-ALG socket interface for TLS or MtE( separation into 2 operation always not possible). TLS RFC7366 allow users to decide weather to use EtM or MtE in TLS. We can solve this, If we have some way to communicate drivers to operate in TLS mode like in setsockopt or msghdr of sendmsg.
> Otherwise you can just separate it out into
> two operations via af-alg.
Always not possible. If openssl has software implementation of Authec( Cipher and hash with 1 algo) it expects same from af-alg engine only then he will override. Its like if Openssl has super set(AES+ SHA256) available it expect same super set in engine(af-alg) for comparison.
The machines with instruction set extensions has authenc implemented in user space like intel aes-ni.
>
> Cheers,
^ permalink raw reply
* Re: George's crazy full state idea (Re: HalfSipHash Acceptable Usage)
From: George Spelvin @ 2016-12-23 0:07 UTC (permalink / raw)
To: hannes, linux, luto
Cc: ak, davem, David.Laight, djb, ebiggers3, eric.dumazet, Jason,
jeanphilippe.aumasson, kernel-hardening, linux-crypto,
linux-kernel, netdev, tom, torvalds, tytso, vegard.nossum
In-Reply-To: <30d9513a-129b-d246-1461-2326130e118f@stressinduktion.org>
Hannes Frederic Sowa wrote:
> A lockdep test should still be done. ;)
Adding might_lock() annotations will improve coverage a lot.
> Yes, that does look nice indeed. Accounting for bits instead of bytes
> shouldn't be a huge problem either. Maybe it gets a bit more verbose in
> case you can't satisfy a request with one batched entropy block and have
> to consume randomness from two.
The bit granularity is also for the callers' convenience, so they don't
have to mask again. Whether get_random_bits rounds up to byte boundaries
internally or not is something else.
When the current batch runs low, I was actually thinking of throwing
away the remaining bits and computing a new batch of 512. But it's
whatever works best at implementation time.
>>> It could only mix the output back in every two calls, in which case
>>> you can backtrack up to one call but you need to do 2^128 work to
>>> backtrack farther. But yes, this is getting excessively complicated.
>
>> No, if you're willing to accept limited backtrack, this is a perfectly
>> acceptable solution, and not too complicated. You could do it phase-less
>> if you like; store the previous output, then after generating the new
>> one, mix in both. Then overwrite the previous output. (But doing two
>> rounds of a crypto primtive to avoid one conditional jump is stupid,
>> so forget that.)
> Can you quickly explain why we lose the backtracking capability?
Sure. An RNG is (state[i], output[i]) = f(state[i-1]). The goal of
backtracking is to compute output[i], or better yet state[i-1], given
state[i].
For example, consider an OFB or CTR mode generator. The state is a key
and and IV, and you encrypt the IV with the key to produce output, then
either replace the IV with the output, or increment it. Either way,
since you still have the key, you can invert the transformation and
recover the previous IV.
The standard way around this is to use the Davies-Meyer construction:
IV[i] = IV[i-1] + E(IV[i-1], key)
This is the standard way to make a non-invertible random function
out of an invertible random permutation.
>From the sum, there's no easy way to find the ciphertext *or* the
plaintext that was encrypted. Assuming the encryption is secure,
the only way to reverse it is brute force: guess IV[i-1] and run the
operation forward to see if the resultant IV[i] matches.
There are a variety of ways to organize this computation, since the
guess gives toy both IV[i-1] and E(IV[i-1], key) = IV[i] - IV[i-1], including
running E forward, backward, or starting from both ends to see if you
meet in the middle.
The way you add the encryption output to the IV is not very important.
It can be addition, xor, or some more complex invertible transformation.
In the case of SipHash, the "encryption" output is smaller than the
input, so we have to get a bit more creative, but it's still basically
the same thing.
The problem is that the output which is combined with the IV is too small.
With only 64 bits, trying all possible values is practical. (The world's
Bitcoin miners are collectively computing SHA-256(SHA-256(input)) 1.7 * 2^64
times per second.)
By basically doing two iterations at once and mixing in 128 bits of
output, the guessing attack is rendered impractical. The only downside
is that you need to remember and store one result between when it's
computed and last used. This is part of the state, so an attack can
find output[i-1], but not anything farther back.
> ChaCha as a block cipher gives a "perfect" permutation from the output
> of either the CRNG or the CPRNG, which actually itself has backtracking
> protection.
I'm not quite understanding. The /dev/random implementation uses some
of the ChaCha output as a new ChaCha key (that's another way to mix output
back into the state) to prevent backtracking. But this slows it down, and
again if you want to be efficient, you're generating and storing large batches
of entropy and storing it in the RNG state.
^ permalink raw reply
* Re: George's crazy full state idea (Re: HalfSipHash Acceptable Usage)
From: Hannes Frederic Sowa @ 2016-12-22 21:38 UTC (permalink / raw)
To: George Spelvin, luto
Cc: ak, davem, David.Laight, djb, ebiggers3, eric.dumazet, Jason,
jeanphilippe.aumasson, kernel-hardening, linux-crypto,
linux-kernel, netdev, tom, torvalds, tytso, vegard.nossum
In-Reply-To: <20161222211140.2816.qmail@ns.sciencehorizons.net>
On 22.12.2016 22:11, George Spelvin wrote:
>> I do tend to like Ted's version in which we use batched
>> get_random_bytes() output. If it's fast enough, it's simpler and lets
>> us get the full strength of a CSPRNG.
>
> With the ChaCha20 generator, that's fine, although note that this abandons
> anti-backtracking entirely.
>
> It also takes locks, something the previous get_random_int code
> path avoided. Do we need to audit the call sites to ensure that's safe?
We have spin_lock_irq* locks on the way. Of course they can hurt in when
contended. The situation should be the same as with get_random_bytes,
callable from every possible situation in the kernel without fear, so
this should also work for get_random_int. A lockdep test should still be
done. ;)
> And there is the issue that the existing callers assume that there's a
> fixed cost per word. A good half of get_random_long calls are followed by
> "& ~PAGE_MASK" to extract the low 12 bits. Or "& ((1ul << mmap_rnd_bits)
> - 1)" to extract the low 28. If we have a buffer we're going to have to
> pay to refill, it would be nice to use less than 8 bytes to satisfy those.
>
> But that can be a followup patch. I'm thinking
>
> unsigned long get_random_bits(unsigned bits)
> E.g. get_random_bits(PAGE_SHIFT),
> get_random_bits(mmap_rnd_bits),
> u32 imm_rnd = get_random_bits(32)
>
> unsigned get_random_mod(unsigned modulus)
> E.g. get_random_mod(hole) & ~(alignment - 1);
> get_random_mod(port_scan_backoff)
> (Althogh probably drivers/s390/scsi/zfcp_fc.c should be changed
> to prandom.)
>
> with, until the audit is completed:
> #define get_random_int() get_random_bits(32)
> #define get_random_long() get_random_bits(BITS_PER_LONG)
Yes, that does look nice indeed. Accounting for bits instead of bytes
shouldn't be a huge problem either. Maybe it gets a bit more verbose in
case you can't satisfy a request with one batched entropy block and have
to consume randomness from two.
>> It could only mix the output back in every two calls, in which case
>> you can backtrack up to one call but you need to do 2^128 work to
>> backtrack farther. But yes, this is getting excessively complicated.
>
> No, if you're willing to accept limited backtrack, this is a perfectly
> acceptable solution, and not too complicated. You could do it phase-less
> if you like; store the previous output, then after generating the new
> one, mix in both. Then overwrite the previous output. (But doing two
> rounds of a crypto primtive to avoid one conditional jump is stupid,
> so forget that.)
Can you quickly explain why we lose the backtracking capability?
ChaCha as a block cipher gives a "perfect" permutation from the output
of either the CRNG or the CPRNG, which actually itself has backtracking
protection.
Thanks for explaining,
Hannes
^ permalink raw reply
* Re: George's crazy full state idea (Re: HalfSipHash Acceptable Usage)
From: George Spelvin @ 2016-12-22 21:11 UTC (permalink / raw)
To: linux, luto
Cc: ak, davem, David.Laight, djb, ebiggers3, eric.dumazet, hannes,
Jason, jeanphilippe.aumasson, kernel-hardening, linux-crypto,
linux-kernel, netdev, tom, torvalds, tytso, vegard.nossum
In-Reply-To: <CALCETrW63gL1821jCMvxnm8gR+o5ifdOXKnWHsLnqoFK77-epw@mail.gmail.com>
> I do tend to like Ted's version in which we use batched
> get_random_bytes() output. If it's fast enough, it's simpler and lets
> us get the full strength of a CSPRNG.
With the ChaCha20 generator, that's fine, although note that this abandons
anti-backtracking entirely.
It also takes locks, something the previous get_random_int code
path avoided. Do we need to audit the call sites to ensure that's safe?
And there is the issue that the existing callers assume that there's a
fixed cost per word. A good half of get_random_long calls are followed by
"& ~PAGE_MASK" to extract the low 12 bits. Or "& ((1ul << mmap_rnd_bits)
- 1)" to extract the low 28. If we have a buffer we're going to have to
pay to refill, it would be nice to use less than 8 bytes to satisfy those.
But that can be a followup patch. I'm thinking
unsigned long get_random_bits(unsigned bits)
E.g. get_random_bits(PAGE_SHIFT),
get_random_bits(mmap_rnd_bits),
u32 imm_rnd = get_random_bits(32)
unsigned get_random_mod(unsigned modulus)
E.g. get_random_mod(hole) & ~(alignment - 1);
get_random_mod(port_scan_backoff)
(Althogh probably drivers/s390/scsi/zfcp_fc.c should be changed
to prandom.)
with, until the audit is completed:
#define get_random_int() get_random_bits(32)
#define get_random_long() get_random_bits(BITS_PER_LONG)
> It could only mix the output back in every two calls, in which case
> you can backtrack up to one call but you need to do 2^128 work to
> backtrack farther. But yes, this is getting excessively complicated.
No, if you're willing to accept limited backtrack, this is a perfectly
acceptable solution, and not too complicated. You could do it phase-less
if you like; store the previous output, then after generating the new
one, mix in both. Then overwrite the previous output. (But doing two
rounds of a crypto primtive to avoid one conditional jump is stupid,
so forget that.)
>> Hmm, interesting. Although, for ASLR, we could use get_random_bytes()
>> directly and be done with it. It won't be a bottleneck.
Isn't that what you already suggested?
I don't mind fewer primtives; I got a bit fixated on "Replace MD5 with
SipHash". It's just the locking that I want to check isn't a problem.
^ permalink raw reply
* Re: [PATCH v2 11/12] crypto: atmel-authenc: add support to authenc(hmac(shaX),Y(aes)) modes
From: kbuild test robot @ 2016-12-22 21:10 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: kbuild-all, herbert, davem, nicolas.ferre, linux-crypto,
linux-kernel, linux-arm-kernel, Cyrille Pitchen
In-Reply-To: <2a6cc4637621d2ef0d84754817128c43795cb022.1482424395.git.cyrille.pitchen@atmel.com>
[-- Attachment #1: Type: text/plain, Size: 18106 bytes --]
Hi Cyrille,
[auto build test WARNING on cryptodev/master]
[also build test WARNING on next-20161222]
[cannot apply to v4.9]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Cyrille-Pitchen/crypto-atmel-authenc-add-support-to-authenc-hmac-shaX-Y-aes-modes/20161223-015820
base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=alpha
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:305:0,
from include/linux/kernel.h:13,
from drivers/crypto/atmel-sha.c:17:
drivers/crypto/atmel-sha.c: In function 'atmel_sha_xmit_cpu':
>> drivers/crypto/atmel-sha.c:465:19: warning: format '%d' expects argument of type 'int', but argument 6 has type 'size_t {aka long unsigned int}' [-Wformat=]
dev_dbg(dd->dev, "xmit_cpu: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
>> drivers/crypto/atmel-sha.c:465:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dd->dev, "xmit_cpu: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
^~~~~~~
drivers/crypto/atmel-sha.c: In function 'atmel_sha_xmit_pdc':
drivers/crypto/atmel-sha.c:494:19: warning: format '%d' expects argument of type 'int', but argument 6 has type 'size_t {aka long unsigned int}' [-Wformat=]
dev_dbg(dd->dev, "xmit_pdc: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
drivers/crypto/atmel-sha.c:494:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dd->dev, "xmit_pdc: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
^~~~~~~
drivers/crypto/atmel-sha.c: In function 'atmel_sha_xmit_dma':
drivers/crypto/atmel-sha.c:541:19: warning: format '%d' expects argument of type 'int', but argument 6 has type 'size_t {aka long unsigned int}' [-Wformat=]
dev_dbg(dd->dev, "xmit_dma: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
drivers/crypto/atmel-sha.c:541:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dd->dev, "xmit_dma: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
^~~~~~~
drivers/crypto/atmel-sha.c: In function 'atmel_sha_xmit_dma_map':
>> drivers/crypto/atmel-sha.c:620:26: warning: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=]
dev_err(dd->dev, "dma %u bytes error\n", ctx->buflen +
^
In file included from include/linux/printk.h:305:0,
from include/linux/kernel.h:13,
from drivers/crypto/atmel-sha.c:17:
drivers/crypto/atmel-sha.c: In function 'atmel_sha_update_dma_slow':
drivers/crypto/atmel-sha.c:641:19: warning: format '%u' expects argument of type 'unsigned int', but argument 4 has type 'size_t {aka long unsigned int}' [-Wformat=]
dev_dbg(dd->dev, "slow: bufcnt: %u, digcnt: 0x%llx 0x%llx, final: %d\n",
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
drivers/crypto/atmel-sha.c:641:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dd->dev, "slow: bufcnt: %u, digcnt: 0x%llx 0x%llx, final: %d\n",
^~~~~~~
drivers/crypto/atmel-sha.c: In function 'atmel_sha_update_dma_start':
drivers/crypto/atmel-sha.c:669:19: warning: format '%u' expects argument of type 'unsigned int', but argument 6 has type 'size_t {aka long unsigned int}' [-Wformat=]
dev_dbg(dd->dev, "fast: digcnt: 0x%llx 0x%llx, bufcnt: %u, total: %u\n",
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
drivers/crypto/atmel-sha.c:669:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dd->dev, "fast: digcnt: 0x%llx 0x%llx, bufcnt: %u, total: %u\n",
^~~~~~~
drivers/crypto/atmel-sha.c:711:27: warning: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'size_t {aka long unsigned int}' [-Wformat=]
dev_err(dd->dev, "dma %u bytes error\n",
^
In file included from include/linux/printk.h:305:0,
from include/linux/kernel.h:13,
from drivers/crypto/atmel-sha.c:17:
drivers/crypto/atmel-sha.c: In function 'atmel_sha_finish':
drivers/crypto/atmel-sha.c:891:19: warning: format '%d' expects argument of type 'int', but argument 6 has type 'size_t {aka long unsigned int}' [-Wformat=]
dev_dbg(dd->dev, "digcnt: 0x%llx 0x%llx, bufcnt: %d\n", ctx->digcnt[1],
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
drivers/crypto/atmel-sha.c:891:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dd->dev, "digcnt: 0x%llx 0x%llx, bufcnt: %d\n", ctx->digcnt[1],
^~~~~~~
vim +465 drivers/crypto/atmel-sha.c
ebc82efa Nicolas Royer 2012-07-01 459 size_t length, int final)
ebc82efa Nicolas Royer 2012-07-01 460 {
ebc82efa Nicolas Royer 2012-07-01 461 struct atmel_sha_reqctx *ctx = ahash_request_ctx(dd->req);
ebc82efa Nicolas Royer 2012-07-01 462 int count, len32;
ebc82efa Nicolas Royer 2012-07-01 463 const u32 *buffer = (const u32 *)buf;
ebc82efa Nicolas Royer 2012-07-01 464
d4905b38 Nicolas Royer 2013-02-20 @465 dev_dbg(dd->dev, "xmit_cpu: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
d4905b38 Nicolas Royer 2013-02-20 466 ctx->digcnt[1], ctx->digcnt[0], length, final);
ebc82efa Nicolas Royer 2012-07-01 467
ebc82efa Nicolas Royer 2012-07-01 468 atmel_sha_write_ctrl(dd, 0);
ebc82efa Nicolas Royer 2012-07-01 469
ebc82efa Nicolas Royer 2012-07-01 470 /* should be non-zero before next lines to disable clocks later */
d4905b38 Nicolas Royer 2013-02-20 471 ctx->digcnt[0] += length;
d4905b38 Nicolas Royer 2013-02-20 472 if (ctx->digcnt[0] < length)
d4905b38 Nicolas Royer 2013-02-20 473 ctx->digcnt[1]++;
ebc82efa Nicolas Royer 2012-07-01 474
ebc82efa Nicolas Royer 2012-07-01 475 if (final)
ebc82efa Nicolas Royer 2012-07-01 476 dd->flags |= SHA_FLAGS_FINAL; /* catch last interrupt */
ebc82efa Nicolas Royer 2012-07-01 477
ebc82efa Nicolas Royer 2012-07-01 478 len32 = DIV_ROUND_UP(length, sizeof(u32));
ebc82efa Nicolas Royer 2012-07-01 479
ebc82efa Nicolas Royer 2012-07-01 480 dd->flags |= SHA_FLAGS_CPU;
ebc82efa Nicolas Royer 2012-07-01 481
ebc82efa Nicolas Royer 2012-07-01 482 for (count = 0; count < len32; count++)
ebc82efa Nicolas Royer 2012-07-01 483 atmel_sha_write(dd, SHA_REG_DIN(count), buffer[count]);
ebc82efa Nicolas Royer 2012-07-01 484
ebc82efa Nicolas Royer 2012-07-01 485 return -EINPROGRESS;
ebc82efa Nicolas Royer 2012-07-01 486 }
ebc82efa Nicolas Royer 2012-07-01 487
ebc82efa Nicolas Royer 2012-07-01 488 static int atmel_sha_xmit_pdc(struct atmel_sha_dev *dd, dma_addr_t dma_addr1,
ebc82efa Nicolas Royer 2012-07-01 489 size_t length1, dma_addr_t dma_addr2, size_t length2, int final)
ebc82efa Nicolas Royer 2012-07-01 490 {
ebc82efa Nicolas Royer 2012-07-01 491 struct atmel_sha_reqctx *ctx = ahash_request_ctx(dd->req);
ebc82efa Nicolas Royer 2012-07-01 492 int len32;
ebc82efa Nicolas Royer 2012-07-01 493
d4905b38 Nicolas Royer 2013-02-20 494 dev_dbg(dd->dev, "xmit_pdc: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
d4905b38 Nicolas Royer 2013-02-20 495 ctx->digcnt[1], ctx->digcnt[0], length1, final);
ebc82efa Nicolas Royer 2012-07-01 496
ebc82efa Nicolas Royer 2012-07-01 497 len32 = DIV_ROUND_UP(length1, sizeof(u32));
ebc82efa Nicolas Royer 2012-07-01 498 atmel_sha_write(dd, SHA_PTCR, SHA_PTCR_TXTDIS);
ebc82efa Nicolas Royer 2012-07-01 499 atmel_sha_write(dd, SHA_TPR, dma_addr1);
ebc82efa Nicolas Royer 2012-07-01 500 atmel_sha_write(dd, SHA_TCR, len32);
ebc82efa Nicolas Royer 2012-07-01 501
ebc82efa Nicolas Royer 2012-07-01 502 len32 = DIV_ROUND_UP(length2, sizeof(u32));
ebc82efa Nicolas Royer 2012-07-01 503 atmel_sha_write(dd, SHA_TNPR, dma_addr2);
ebc82efa Nicolas Royer 2012-07-01 504 atmel_sha_write(dd, SHA_TNCR, len32);
ebc82efa Nicolas Royer 2012-07-01 505
ebc82efa Nicolas Royer 2012-07-01 506 atmel_sha_write_ctrl(dd, 1);
ebc82efa Nicolas Royer 2012-07-01 507
ebc82efa Nicolas Royer 2012-07-01 508 /* should be non-zero before next lines to disable clocks later */
d4905b38 Nicolas Royer 2013-02-20 509 ctx->digcnt[0] += length1;
d4905b38 Nicolas Royer 2013-02-20 510 if (ctx->digcnt[0] < length1)
d4905b38 Nicolas Royer 2013-02-20 511 ctx->digcnt[1]++;
ebc82efa Nicolas Royer 2012-07-01 512
ebc82efa Nicolas Royer 2012-07-01 513 if (final)
ebc82efa Nicolas Royer 2012-07-01 514 dd->flags |= SHA_FLAGS_FINAL; /* catch last interrupt */
ebc82efa Nicolas Royer 2012-07-01 515
ebc82efa Nicolas Royer 2012-07-01 516 dd->flags |= SHA_FLAGS_DMA_ACTIVE;
ebc82efa Nicolas Royer 2012-07-01 517
ebc82efa Nicolas Royer 2012-07-01 518 /* Start DMA transfer */
ebc82efa Nicolas Royer 2012-07-01 519 atmel_sha_write(dd, SHA_PTCR, SHA_PTCR_TXTEN);
ebc82efa Nicolas Royer 2012-07-01 520
ebc82efa Nicolas Royer 2012-07-01 521 return -EINPROGRESS;
ebc82efa Nicolas Royer 2012-07-01 522 }
ebc82efa Nicolas Royer 2012-07-01 523
d4905b38 Nicolas Royer 2013-02-20 524 static void atmel_sha_dma_callback(void *data)
d4905b38 Nicolas Royer 2013-02-20 525 {
d4905b38 Nicolas Royer 2013-02-20 526 struct atmel_sha_dev *dd = data;
d4905b38 Nicolas Royer 2013-02-20 527
b48b114c Cyrille Pitchen 2016-12-22 528 dd->is_async = true;
b48b114c Cyrille Pitchen 2016-12-22 529
d4905b38 Nicolas Royer 2013-02-20 530 /* dma_lch_in - completed - wait DATRDY */
d4905b38 Nicolas Royer 2013-02-20 531 atmel_sha_write(dd, SHA_IER, SHA_INT_DATARDY);
d4905b38 Nicolas Royer 2013-02-20 532 }
d4905b38 Nicolas Royer 2013-02-20 533
d4905b38 Nicolas Royer 2013-02-20 534 static int atmel_sha_xmit_dma(struct atmel_sha_dev *dd, dma_addr_t dma_addr1,
d4905b38 Nicolas Royer 2013-02-20 535 size_t length1, dma_addr_t dma_addr2, size_t length2, int final)
d4905b38 Nicolas Royer 2013-02-20 536 {
d4905b38 Nicolas Royer 2013-02-20 537 struct atmel_sha_reqctx *ctx = ahash_request_ctx(dd->req);
d4905b38 Nicolas Royer 2013-02-20 538 struct dma_async_tx_descriptor *in_desc;
d4905b38 Nicolas Royer 2013-02-20 539 struct scatterlist sg[2];
d4905b38 Nicolas Royer 2013-02-20 540
d4905b38 Nicolas Royer 2013-02-20 @541 dev_dbg(dd->dev, "xmit_dma: digcnt: 0x%llx 0x%llx, length: %d, final: %d\n",
d4905b38 Nicolas Royer 2013-02-20 542 ctx->digcnt[1], ctx->digcnt[0], length1, final);
d4905b38 Nicolas Royer 2013-02-20 543
d4905b38 Nicolas Royer 2013-02-20 544 dd->dma_lch_in.dma_conf.src_maxburst = 16;
d4905b38 Nicolas Royer 2013-02-20 545 dd->dma_lch_in.dma_conf.dst_maxburst = 16;
d4905b38 Nicolas Royer 2013-02-20 546
d4905b38 Nicolas Royer 2013-02-20 547 dmaengine_slave_config(dd->dma_lch_in.chan, &dd->dma_lch_in.dma_conf);
d4905b38 Nicolas Royer 2013-02-20 548
d4905b38 Nicolas Royer 2013-02-20 549 if (length2) {
d4905b38 Nicolas Royer 2013-02-20 550 sg_init_table(sg, 2);
d4905b38 Nicolas Royer 2013-02-20 551 sg_dma_address(&sg[0]) = dma_addr1;
d4905b38 Nicolas Royer 2013-02-20 552 sg_dma_len(&sg[0]) = length1;
d4905b38 Nicolas Royer 2013-02-20 553 sg_dma_address(&sg[1]) = dma_addr2;
d4905b38 Nicolas Royer 2013-02-20 554 sg_dma_len(&sg[1]) = length2;
d4905b38 Nicolas Royer 2013-02-20 555 in_desc = dmaengine_prep_slave_sg(dd->dma_lch_in.chan, sg, 2,
d4905b38 Nicolas Royer 2013-02-20 556 DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
d4905b38 Nicolas Royer 2013-02-20 557 } else {
d4905b38 Nicolas Royer 2013-02-20 558 sg_init_table(sg, 1);
d4905b38 Nicolas Royer 2013-02-20 559 sg_dma_address(&sg[0]) = dma_addr1;
d4905b38 Nicolas Royer 2013-02-20 560 sg_dma_len(&sg[0]) = length1;
d4905b38 Nicolas Royer 2013-02-20 561 in_desc = dmaengine_prep_slave_sg(dd->dma_lch_in.chan, sg, 1,
d4905b38 Nicolas Royer 2013-02-20 562 DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
d4905b38 Nicolas Royer 2013-02-20 563 }
d4905b38 Nicolas Royer 2013-02-20 564 if (!in_desc)
b48b114c Cyrille Pitchen 2016-12-22 565 atmel_sha_complete(dd, -EINVAL);
d4905b38 Nicolas Royer 2013-02-20 566
d4905b38 Nicolas Royer 2013-02-20 567 in_desc->callback = atmel_sha_dma_callback;
d4905b38 Nicolas Royer 2013-02-20 568 in_desc->callback_param = dd;
d4905b38 Nicolas Royer 2013-02-20 569
d4905b38 Nicolas Royer 2013-02-20 570 atmel_sha_write_ctrl(dd, 1);
d4905b38 Nicolas Royer 2013-02-20 571
d4905b38 Nicolas Royer 2013-02-20 572 /* should be non-zero before next lines to disable clocks later */
d4905b38 Nicolas Royer 2013-02-20 573 ctx->digcnt[0] += length1;
d4905b38 Nicolas Royer 2013-02-20 574 if (ctx->digcnt[0] < length1)
d4905b38 Nicolas Royer 2013-02-20 575 ctx->digcnt[1]++;
d4905b38 Nicolas Royer 2013-02-20 576
d4905b38 Nicolas Royer 2013-02-20 577 if (final)
d4905b38 Nicolas Royer 2013-02-20 578 dd->flags |= SHA_FLAGS_FINAL; /* catch last interrupt */
d4905b38 Nicolas Royer 2013-02-20 579
d4905b38 Nicolas Royer 2013-02-20 580 dd->flags |= SHA_FLAGS_DMA_ACTIVE;
d4905b38 Nicolas Royer 2013-02-20 581
d4905b38 Nicolas Royer 2013-02-20 582 /* Start DMA transfer */
d4905b38 Nicolas Royer 2013-02-20 583 dmaengine_submit(in_desc);
d4905b38 Nicolas Royer 2013-02-20 584 dma_async_issue_pending(dd->dma_lch_in.chan);
d4905b38 Nicolas Royer 2013-02-20 585
d4905b38 Nicolas Royer 2013-02-20 586 return -EINPROGRESS;
d4905b38 Nicolas Royer 2013-02-20 587 }
d4905b38 Nicolas Royer 2013-02-20 588
d4905b38 Nicolas Royer 2013-02-20 589 static int atmel_sha_xmit_start(struct atmel_sha_dev *dd, dma_addr_t dma_addr1,
d4905b38 Nicolas Royer 2013-02-20 590 size_t length1, dma_addr_t dma_addr2, size_t length2, int final)
d4905b38 Nicolas Royer 2013-02-20 591 {
d4905b38 Nicolas Royer 2013-02-20 592 if (dd->caps.has_dma)
d4905b38 Nicolas Royer 2013-02-20 593 return atmel_sha_xmit_dma(dd, dma_addr1, length1,
d4905b38 Nicolas Royer 2013-02-20 594 dma_addr2, length2, final);
d4905b38 Nicolas Royer 2013-02-20 595 else
d4905b38 Nicolas Royer 2013-02-20 596 return atmel_sha_xmit_pdc(dd, dma_addr1, length1,
d4905b38 Nicolas Royer 2013-02-20 597 dma_addr2, length2, final);
d4905b38 Nicolas Royer 2013-02-20 598 }
d4905b38 Nicolas Royer 2013-02-20 599
ebc82efa Nicolas Royer 2012-07-01 600 static int atmel_sha_update_cpu(struct atmel_sha_dev *dd)
ebc82efa Nicolas Royer 2012-07-01 601 {
ebc82efa Nicolas Royer 2012-07-01 602 struct atmel_sha_reqctx *ctx = ahash_request_ctx(dd->req);
ebc82efa Nicolas Royer 2012-07-01 603 int bufcnt;
ebc82efa Nicolas Royer 2012-07-01 604
ebc82efa Nicolas Royer 2012-07-01 605 atmel_sha_append_sg(ctx);
ebc82efa Nicolas Royer 2012-07-01 606 atmel_sha_fill_padding(ctx, 0);
ebc82efa Nicolas Royer 2012-07-01 607 bufcnt = ctx->bufcnt;
ebc82efa Nicolas Royer 2012-07-01 608 ctx->bufcnt = 0;
ebc82efa Nicolas Royer 2012-07-01 609
ebc82efa Nicolas Royer 2012-07-01 610 return atmel_sha_xmit_cpu(dd, ctx->buffer, bufcnt, 1);
ebc82efa Nicolas Royer 2012-07-01 611 }
ebc82efa Nicolas Royer 2012-07-01 612
ebc82efa Nicolas Royer 2012-07-01 613 static int atmel_sha_xmit_dma_map(struct atmel_sha_dev *dd,
ebc82efa Nicolas Royer 2012-07-01 614 struct atmel_sha_reqctx *ctx,
ebc82efa Nicolas Royer 2012-07-01 615 size_t length, int final)
ebc82efa Nicolas Royer 2012-07-01 616 {
ebc82efa Nicolas Royer 2012-07-01 617 ctx->dma_addr = dma_map_single(dd->dev, ctx->buffer,
d4905b38 Nicolas Royer 2013-02-20 618 ctx->buflen + ctx->block_size, DMA_TO_DEVICE);
ebc82efa Nicolas Royer 2012-07-01 619 if (dma_mapping_error(dd->dev, ctx->dma_addr)) {
ebc82efa Nicolas Royer 2012-07-01 @620 dev_err(dd->dev, "dma %u bytes error\n", ctx->buflen +
d4905b38 Nicolas Royer 2013-02-20 621 ctx->block_size);
b48b114c Cyrille Pitchen 2016-12-22 622 atmel_sha_complete(dd, -EINVAL);
ebc82efa Nicolas Royer 2012-07-01 623 }
:::::: The code at line 465 was first introduced by commit
:::::: d4905b38d1f6b60761a6fd16f45ebd1fac8b6e1f crypto: atmel-sha - add support for latest release of the IP (0x410)
:::::: TO: Nicolas Royer <nicolas@eukrea.com>
:::::: CC: Herbert Xu <herbert@gondor.apana.org.au>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47848 bytes --]
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Hannes Frederic Sowa @ 2016-12-22 20:02 UTC (permalink / raw)
To: Andy Lutomirski, Alexei Starovoitov
Cc: Daniel Borkmann, Jason A. Donenfeld,
kernel-hardening@lists.openwall.com, Theodore Ts'o, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <CALCETrUWw2m8k3b8ibOeN-7o9b+8MrnX4btUWqS188WSsLzDjg@mail.gmail.com>
On 22.12.2016 20:56, Andy Lutomirski wrote:
> It's also not quite clear to me why userspace needs to be able to
> calculate the digest on its own. A bpf(BPF_CALC_PROGRAM_DIGEST)
> command that takes a BPF program as input and hashes it would seem to
> serve the same purpose, and that would allow the kernel to key the
> digest and change the algorithm down the road without breaking things.
I think that people expect digests of BPF programs to be stable over
time and reboots.
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Andy Lutomirski @ 2016-12-22 19:56 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Hannes Frederic Sowa, Daniel Borkmann, Jason A. Donenfeld,
kernel-hardening@lists.openwall.com, Theodore Ts'o, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <CAADnVQ+memUT2zcc-wunP0QgWSLWpmnSJNRJZmfDdc+FBb=gEg@mail.gmail.com>
On Thu, Dec 22, 2016 at 11:34 AM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Dec 22, 2016 at 9:25 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Thu, Dec 22, 2016 at 8:59 AM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>>> On Thu, 2016-12-22 at 08:07 -0800, Andy Lutomirski wrote:
>>>
>>> We don't prevent ebpf programs being loaded based on the digest but
>>> just to uniquely identify loaded programs from user space and match up
>>> with their source.
>>
>> The commit log talks about using the hash to see if the program has
>> already been compiled and JITted. If that's done, then a collision
>> will directly cause the kernel to malfunction.
>
> Andy, please read the code.
> we could have used jhash there just as well.
> Collisions are fine.
There's relevant in the code to read yet AFAICS. The code exports it
via fdinfo, and userspace is expected to do something with it. The
commit message says:
When programs are pinned and retrieved by an ELF loader, the loader
can check the program's digest through fdinfo and compare it against
one that was generated over the ELF file's program section to see
if the program needs to be reloaded.
I assume this means that a userspace component is expected to compare
the digest of a loaded program to a digest of a program it wants to
load and to use the result of the comparison to decide whether the
programs are the same. If that's indeed the case (and it sure sounds
like it, and I fully expect CRIU to do very similar things when
support is added), then malicious collisions do matter.
It's also not quite clear to me why userspace needs to be able to
calculate the digest on its own. A bpf(BPF_CALC_PROGRAM_DIGEST)
command that takes a BPF program as input and hashes it would seem to
serve the same purpose, and that would allow the kernel to key the
digest and change the algorithm down the road without breaking things.
Regardless, adding a new hash algorithm that is almost-but-not-quite
SHA-1 and making it a stable interface to userspace is not a good
thing.
--Andy
^ permalink raw reply
* Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5
From: Theodore Ts'o @ 2016-12-22 19:50 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Jason A. Donenfeld, kernel-hardening, Andy Lutomirski, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <32db19ab-0c19-628c-3475-2ccb8050563a@stressinduktion.org>
On Thu, Dec 22, 2016 at 07:08:37PM +0100, Hannes Frederic Sowa wrote:
> I wasn't concerned about performance but more about DoS resilience. I
> wonder how safe half md4 actually is in terms of allowing users to
> generate long hash chains in the filesystem (in terms of length
> extension attacks against half_md4).
>
> In ext4, is it actually possible that a "disrupter" learns about the
> hashing secret in the way how the inodes are returned during getdents?
They'd have to be a local user, who can execute telldir(3) --- in
which case there are plenty of other denial of service attacks one
could carry out that would be far more devastating.
It might also be an issue if the file system is exposed via NFS, but
again, there are so many other ways an attacker could DoS a NFS server
that I don't think of it as a much of a concern.
Keep in mind that worst someone can do is cause directory inserts to
fail with an ENOSPC, and there are plenty of other ways of doing that
--- such as consuming all of the blocks and inodes in the file system,
for example.
So it's a threat, but not a high priority one as far as I'm concerned.
And if this was a problem in actual practice, users could switch to
the TEA based hash, which should be far harder to attack, and
available today.
- Ted
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Alexei Starovoitov @ 2016-12-22 19:34 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Hannes Frederic Sowa, Daniel Borkmann, Jason A. Donenfeld,
kernel-hardening@lists.openwall.com, Theodore Ts'o, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <CALCETrW_T1v4qKPJDs5dXwAnAit3M52AWMH-K+GJLb1WoLMuRQ@mail.gmail.com>
On Thu, Dec 22, 2016 at 9:25 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Dec 22, 2016 at 8:59 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>> On Thu, 2016-12-22 at 08:07 -0800, Andy Lutomirski wrote:
>>
>> We don't prevent ebpf programs being loaded based on the digest but
>> just to uniquely identify loaded programs from user space and match up
>> with their source.
>
> The commit log talks about using the hash to see if the program has
> already been compiled and JITted. If that's done, then a collision
> will directly cause the kernel to malfunction.
Andy, please read the code.
we could have used jhash there just as well.
Collisions are fine.
^ permalink raw reply
* Re: George's crazy full state idea (Re: HalfSipHash Acceptable Usage)
From: Andy Lutomirski @ 2016-12-22 19:32 UTC (permalink / raw)
To: George Spelvin
Cc: Andrew Lutomirski, Andi Kleen, David S. Miller, David Laight,
D. J. Bernstein, Eric Biggers, Eric Dumazet, Hannes Frederic Sowa,
Jason A. Donenfeld, Jean-Philippe Aumasson,
kernel-hardening@lists.openwall.com, Linux Crypto Mailing List,
linux-kernel@vger.kernel.org, Network Development, Tom Herbert,
Linus Torvalds, Ted Ts'o
In-Reply-To: <20161222192445.963.qmail@ns.sciencehorizons.net>
On Thu, Dec 22, 2016 at 11:24 AM, George Spelvin
<linux@sciencehorizons.net> wrote:
>> Having slept on this, I like it less. The problem is that a
>> backtracking attacker doesn't just learn H(random seed || entropy_0 ||
>> secret || ...) -- they learn the internal state of the hash function
>> that generates that value. This probably breaks any attempt to apply
>> security properties of the hash function. For example, the internal
>> state could easily contain a whole bunch of prior outputs it in
>> verbatim.
>
> The problem is, anti-backtracing is in severe tension with your desire
> to use unmodified SipHash.
>
> First of all, I'd like to repeat that it isn't a design goal of the current
> generator and isn't necessary.
Agreed.
> Now, the main point: it's not likely to be solvable.
>
> The problem with unmodified SipHash is that is has only 64 bits of
> output. No mix-back mechanism can get around the fundamental problem
> that that's too small to prevent a brute-force guessing attack. You need
> wider mix-back. And getting more output from unmodified SipHash requires
> more finalization rounds, which is expensive.
It could only mix the output back in every two calls, in which case
you can backtrack up to one call but you need to do 2^128 work to
backtrack farther. But yes, this is getting excessively complicated.
> Finally, your discomfort about an attacker learning the internal state...
> if an attacker knows the key and the input, they can construct the
> internal state. Yes, we could discard the internal state and construct
> a fresh one on the next call to get_random_int, but what are you going
> to key it with? What are you going to feed it? What keeps *that*
> internal state any more secret from an attacker than one that's explicitly
> stored?
I do tend to like Ted's version in which we use batched
get_random_bytes() output. If it's fast enough, it's simpler and lets
us get the full strength of a CSPRNG.
(Aside: some day I want to move all that code from drivers/ to lib/
and teach it to be buildable in userspace, too, so it's easy to play
with it, feed it test vectors, confirm that it's equivalent to a
reference implementation, write up the actual design and try to get
real cryptographers to analyze it, etc.)
> For example, clearly stating the concern over starting new processes
> with predictable layout, and the limits on the fresh entropy supply,
> has made me realize that there *is* a possible source: each exec()
> is passed 128 bits from get_random_bytes in the AT_RANDOM element of
> its auxv. Since get_random_int() accesses "current" anyway, we could
> store some seed material there rather than using "pid". While this is
> not fresh for each call to get_random_int, it *is* fresh for each new
> address space whose layout is being randomized.
Hmm, interesting. Although, for ASLR, we could use get_random_bytes()
directly and be done with it. It won't be a bottleneck.
^ permalink raw reply
* Re: George's crazy full state idea (Re: HalfSipHash Acceptable Usage)
From: George Spelvin @ 2016-12-22 19:24 UTC (permalink / raw)
To: luto, luto
Cc: ak, davem, David.Laight, djb, ebiggers3, eric.dumazet, hannes,
Jason, jeanphilippe.aumasson, kernel-hardening, linux-crypto,
linux-kernel, linux, netdev, tom, torvalds, tytso, vegard.nossum
In-Reply-To: <CALCETrUSM7KDNuVqh169241PNumJVOF5hrNQUq=k5Fnet6nSgA@mail.gmail.com>
> Having slept on this, I like it less. The problem is that a
> backtracking attacker doesn't just learn H(random seed || entropy_0 ||
> secret || ...) -- they learn the internal state of the hash function
> that generates that value. This probably breaks any attempt to apply
> security properties of the hash function. For example, the internal
> state could easily contain a whole bunch of prior outputs it in
> verbatim.
The problem is, anti-backtracing is in severe tension with your desire
to use unmodified SipHash.
First of all, I'd like to repeat that it isn't a design goal of the current
generator and isn't necessary.
The current generator just returns hash[0] from the MD5 state, then
leaves the state stored. The fact that it conceals earlier outputs is
an accident of the Davies-Meyer structure of md5_transform.
It isn't necessary, because every secret generated is stored unencrypted
for as long as it's of value. A few values are used for retransmit
backoffs and random MAC addresses. Those are revealed to the world as
soon as they're used.
Most values are used for ASLR. These address are of interest to an
attacker trying to mount a buffer-overflow attack, but that only lasts
as long as the process is running to receive buffers. After the process
exits, knowledge of its layout is worthless.
And this is stored as long as it's running in kernel-accessible data,
so storing a *second* copy in less conveniently kernel-accessible data
(the RNG state) doesn't make the situation any worse.
In addition to the above, if you're assuming a state capture, then
since we have (for necessary efficieny reasons) a negligible about of
fresh entropy, an attacker has the secret key and can predict *future*
outputs very easily.
Given that information, an attacker doesn't need to learn the layout of
vulnerable server X. If they have a buffer overflow, they can crash
the current instance and wait for a fresh image to be started (with a
known address space) to launch their attack at.
Kernel state capture attacks are a very unlikely attack, mostly because
it's a narrow target a hair's breadth away from the much more interesting
outright kernel compromise (attacker gains write access as well as read)
which renders all this fancy cryptanaysis moot.
Now, the main point: it's not likely to be solvable.
The problem with unmodified SipHash is that is has only 64 bits of
output. No mix-back mechanism can get around the fundamental problem
that that's too small to prevent a brute-force guessing attack. You need
wider mix-back. And getting more output from unmodified SipHash requires
more finalization rounds, which is expensive.
(Aside: 64 bits does have the advantage that it can't be brute-forced on
the attacked machine. It must be exfiltrated to the attacker, and the
solution returned to the attack code. But doing this is getting easier
all the time.)
Adding antibacktracking to SipHash is trivial: just add a Davies-Meyer
structure around its internal state. Remember the internal state before
hashing in the entropy and secret, generate the output, then add the
previous and final states together for storage.
This is a standard textbook construction, very cheap, and doesn't touch
the compression function which is the target of analysis and attacks,
but it requires poking around in SipHash's internal state. (And people
who read the textbooks without understanding them will get upset because
the textbooks all talk about using this construction with block ciphers,
and SipHash's compression function is not advertised as a block cipher.)
Alternative designs exist; you could extract additional output from
earlier rounds of SipHash, using the duplex sponge construction you
mentioned earlier. That output would be used for mixback purposes *only*,
so wouldn't affect the security proof of the "primary" output.
But this is also getting creative with SipHash's internals.
Now, you could use a completely *different* cryptographic primitive
to enforce one-way-ness, and apply SipHash as a strong output transform,
but that doesn't feel like good design, and is probably more expensive.
Finally, your discomfort about an attacker learning the internal state...
if an attacker knows the key and the input, they can construct the
internal state. Yes, we could discard the internal state and construct
a fresh one on the next call to get_random_int, but what are you going
to key it with? What are you going to feed it? What keeps *that*
internal state any more secret from an attacker than one that's explicitly
stored?
Keeping the internal state around is a cacheing optimization, that's all.
*If* you're assuming a state capture, the only thing secret from the
attacker is any fresh entropy collected between the time of capture
and the time of generation. Due to mandatory efficiency requirements,
this is very small.
I really think you're wishing for the impossible here.
A final note: although I'm disagreeing with you, thank you very much for
the informed discussion. Knowing that someone will read and think about
this message carefully has forced me to think it through carefully myself.
For example, clearly stating the concern over starting new processes
with predictable layout, and the limits on the fresh entropy supply,
has made me realize that there *is* a possible source: each exec()
is passed 128 bits from get_random_bytes in the AT_RANDOM element of
its auxv. Since get_random_int() accesses "current" anyway, we could
store some seed material there rather than using "pid". While this is
not fresh for each call to get_random_int, it *is* fresh for each new
address space whose layout is being randomized.
^ permalink raw reply
* Re: [PATCH 11/12] crypto: atmel-authenc: add support to authenc(hmac(shaX),Y(aes)) modes
From: kbuild test robot @ 2016-12-22 18:49 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: kbuild-all, herbert, davem, nicolas.ferre, linux-crypto,
linux-kernel, linux-arm-kernel, Cyrille Pitchen
In-Reply-To: <2bf61b215c2c0729a20017cd8aa02cf99c5ddb1a.1482422983.git.cyrille.pitchen@atmel.com>
[-- Attachment #1: Type: text/plain, Size: 1715 bytes --]
Hi Cyrille,
[auto build test ERROR on cryptodev/master]
[also build test ERROR on next-20161222]
[cannot apply to v4.9]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Cyrille-Pitchen/crypto-atmel-authenc-add-support-to-authenc-hmac-shaX-Y-aes-modes/20161223-012130
base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
warning: (CRYPTO_DEV_ATMEL_AUTHENC) selects CRYPTO_DEV_ATMEL_SHA which has unmet direct dependencies (CRYPTO && CRYPTO_HW && ARCH_AT91)
>> drivers/crypto/atmel-aes.c:44:27: fatal error: atmel-authenc.h: No such file or directory
#include "atmel-authenc.h"
^
compilation terminated.
--
>> drivers/crypto/atmel-sha.c:44:27: fatal error: atmel-authenc.h: No such file or directory
#include "atmel-authenc.h"
^
compilation terminated.
vim +44 drivers/crypto/atmel-aes.c
38 #include <crypto/aes.h>
39 #include <crypto/xts.h>
40 #include <crypto/internal/aead.h>
41 #include <linux/platform_data/crypto-atmel.h>
42 #include <dt-bindings/dma/at91.h>
43 #include "atmel-aes-regs.h"
> 44 #include "atmel-authenc.h"
45
46 #define ATMEL_AES_PRIORITY 300
47
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56834 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/2] crypto: mediatek - add DT bindings documentation
From: Rob Herring @ 2016-12-22 18:41 UTC (permalink / raw)
To: Ryder Lee
Cc: Herbert Xu, David S. Miller, Matthias Brugger,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sean Wang,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Roy Luo,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1482114045-18716-3-git-send-email-ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
On Mon, Dec 19, 2016 at 10:20:45AM +0800, Ryder Lee wrote:
> Add DT bindings documentation for the crypto driver
>
> Signed-off-by: Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> .../devicetree/bindings/crypto/mediatek-crypto.txt | 27 ++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/crypto/mediatek-crypto.txt
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Jason A. Donenfeld @ 2016-12-22 18:19 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Andy Lutomirski, Daniel Borkmann, Alexei Starovoitov,
kernel-hardening@lists.openwall.com, Theodore Ts'o, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <1482425969.2673.5.camel@stressinduktion.org>
On Thu, Dec 22, 2016 at 5:59 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> We don't prevent ebpf programs being loaded based on the digest but
> just to uniquely identify loaded programs from user space and match up
> with their source.
Okay, so in that case, a weak hashing function like SHA1 could result
in a real vulnerability. Therefore, this SHA1 stuff needs to be
reverted immediately, pending a different implementation. If this has
ever shipped in a kernel version, it could even deserve a CVE. No
SHA1!
> The hashing is not a proper sha1 neither, unfortunately. I think that
> is why it will have a custom implementation in iproute2?
Jeepers creepers. So for some ungodly reason, LKML has invented yet
another homebrewed crypto primitive. This story really gets more
horrifying every day. No bueno.
So yea, let's revert and re-commit (repeal and replace? just
kidding...). Out with SHA-1, in with Blake2 or SHA2.
Jason
^ permalink raw reply
* Re: Re: [PATCH v7 3/6] random: use SipHash in place of MD5
From: Jason A. Donenfeld @ 2016-12-22 18:13 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Theodore Ts'o, kernel-hardening, Andy Lutomirski, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <32db19ab-0c19-628c-3475-2ccb8050563a@stressinduktion.org>
On Thu, Dec 22, 2016 at 7:08 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> I wasn't concerned about performance but more about DoS resilience. I
> wonder how safe half md4 actually is in terms of allowing users to
> generate long hash chains in the filesystem (in terms of length
> extension attacks against half_md4).
AFAIK, this is a real vulnerability that needs to be addressed.
Judging by Ted's inquiry about my siphash testing suite, I assume he's
probably tinkering around with it as we speak. :)
Meanwhile I've separated things into several trees:
1. chacha20 rng, already submitted:
https://git.zx2c4.com/linux-dev/log/?h=random-next
2. md5 cleanup, not yet submitted:
https://git.zx2c4.com/linux-dev/log/?h=md5-cleanup
3. md4 cleanup, already submitted:
https://git.zx2c4.com/linux-dev/log/?h=ext4-next-md4-cleanup
4. siphash and networking, not yet submitted as a x/4 series:
https://git.zx2c4.com/linux-dev/log/?h=net-next-siphash
I'll submit (4) in a couple of days, waiting for any comments on the
existing patch-set.
Jason
^ permalink raw reply
* Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5
From: Hannes Frederic Sowa @ 2016-12-22 18:08 UTC (permalink / raw)
To: Theodore Ts'o, Jason A. Donenfeld, kernel-hardening,
Andy Lutomirski, Netdev, LKML, Linux Crypto Mailing List,
David Laight, Eric Dumazet, Linus Torvalds, Eric Biggers,
Tom Herbert, Andi Kleen, David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <20161222155447.u3ayvw4gmorhswjv@thunk.org>
On 22.12.2016 16:54, Theodore Ts'o wrote:
> On Thu, Dec 22, 2016 at 02:10:33PM +0100, Jason A. Donenfeld wrote:
>> On Thu, Dec 22, 2016 at 1:47 PM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>>> following up on what appears to be a random subject: ;)
>>>
>>> IIRC, ext4 code by default still uses half_md4 for hashing of filenames
>>> in the htree. siphash seems to fit this use case pretty good.
>>
>> I saw this too. I'll try to address it in v8 of this series.
>
> This is a separate issue, and this series is getting a bit too
> complex. So I'd suggest pushing this off to a separate change.
>
> Changing the htree hash algorithm is an on-disk format change, and so
> we couldn't roll it out until e2fsprogs gets updated and rolled out
> pretty broadley. In fact George sent me patches to add siphash as a
> hash algorithm for htree a while back (for both the kernel and
> e2fsprogs), but I never got around to testing and applying them,
> mainly because while it's technically faster, I had other higher
> priority issues to work on --- and see previous comments regarding
> pixel peeping. Improving the hash algorithm by tens or even hundreds
> of nanoseconds isn't really going to matter since we only do a htree
> lookup on a file creation or cold cache lookup, and the SSD or HDD I/O
> times will dominate. And from the power perspective, saving
> microwatts of CPU power isn't going to matter if you're going to be
> spinning up the storage device....
I wasn't concerned about performance but more about DoS resilience. I
wonder how safe half md4 actually is in terms of allowing users to
generate long hash chains in the filesystem (in terms of length
extension attacks against half_md4).
In ext4, is it actually possible that a "disrupter" learns about the
hashing secret in the way how the inodes are returned during getdents?
Thanks,
Hannes
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Hannes Frederic Sowa @ 2016-12-22 17:49 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Daniel Borkmann, Alexei Starovoitov, Jason A. Donenfeld,
kernel-hardening@lists.openwall.com, Theodore Ts'o, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <CALCETrW_T1v4qKPJDs5dXwAnAit3M52AWMH-K+GJLb1WoLMuRQ@mail.gmail.com>
On Thu, 2016-12-22 at 09:25 -0800, Andy Lutomirski wrote:
> On Thu, Dec 22, 2016 at 8:59 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > On Thu, 2016-12-22 at 08:07 -0800, Andy Lutomirski wrote:
> > >
> > > You mean:
> > >
> > > commit 7bd509e311f408f7a5132fcdde2069af65fa05ae
> > > Author: Daniel Borkmann <daniel@iogearbox.net>
> > > Date: Sun Dec 4 23:19:41 2016 +0100
> > >
> > > bpf: add prog_digest and expose it via fdinfo/netlink
> > >
> > >
> > > Yes, please! This actually matters for security -- imagine a
> > > malicious program brute-forcing a collision so that it gets loaded
> > > wrong. And this is IMO a use case for SHA-256 or SHA-512/256
> > > (preferably the latter). Speed basically doesn't matter here and
> > > Blake2 is both less stable (didn't they slightly change it recently?)
> > > and much less well studied.
> >
> > We don't prevent ebpf programs being loaded based on the digest but
> > just to uniquely identify loaded programs from user space and match up
> > with their source.
>
> The commit log talks about using the hash to see if the program has
> already been compiled and JITted. If that's done, then a collision
> will directly cause the kernel to malfunction.
Yeah, it still shouldn't crash the kernel but it could cause
malfunctions because assumptions are not met from user space thus it
could act in a strange way:
My personal biggest concern is that users of this API will at some
point in time assume this digist is unique (as a key itself for a
hashtables f.e.), while it is actually not (and not enforced so by the
kernel). If you can get an unpriv ebpf program inserted to the kernel
with the same weak hash, a controller daemon could pick it up and bind
it to another ebpf hook, probably outside of the unpriv realm the user
was in before. Only the sorting matters, which might be unstable and is
not guaranteed by anything in most hash table based data structures.
The API seems flawed to me.
> > > My inclination would have been to seed them with something that isn't
> > > exposed to userspace for the precise reason that it would prevent user
> > > code from making assumptions about what's in the hash. But if there's
> > > a use case for why user code needs to be able to calculate the hash on
> > > its own, then that's fine. But perhaps the actual fdinfo string
> > > should be "sha256:abcd1234..." to give some flexibility down the road.
To add to this, I am very much in favor of that. Right now it doesn't
have a name because it is a custom algorithm. ;)
> > >
> > > Also:
> > >
> > > + result = (__force __be32 *)fp->digest;
> > > + for (i = 0; i < SHA_DIGEST_WORDS; i++)
> > > + result[i] = cpu_to_be32(fp->digest[i]);
> > >
> > > Everyone, please, please, please don't open-code crypto primitives.
> > > Is this and the code above it even correct? It might be but on a very
> > > brief glance it looks wrong to me. If you're doing this to avoid
> > > depending on crypto, then fix crypto so you can pull in the algorithm
> > > without pulling in the whole crypto core.
> >
> > The hashing is not a proper sha1 neither, unfortunately. I think that
> > is why it will have a custom implementation in iproute2?
>
> Putting on crypto hat:
>
> NAK NAK NAK NAK NAK. "The Linux kernel invented a new primitive in
> 2016 when people know better and is going to handle it by porting that
> new primitive to userspace" is not a particularly good argument.
>
> Okay, crypto hack back off.
>
> >
> > I wondered if bpf program loading should have used the module loading
> > infrastructure from the beginning...
>
> That would be way too complicated and would be nasty for the unprivileged cases.
I was more or less just thinking about using the syscalls and user
space representation not the generic infrastructure, as it is anyway
too much concerned with real kernel modules (would probably also solve
your cgroup-ebpf thread, as it can be passed by unique name or name and
hash ;) ). Anyway...
> > > At the very least, there should be a separate function that calculates
> > > the hash of a buffer and that function should explicitly run itself
> > > against test vectors of various lengths to make sure that it's
> > > calculating what it claims to be calculating. And it doesn't look
> > > like the userspace code has landed, so, if this thing isn't
> > > calculating SHA1 correctly, it's plausible that no one has noticed.
> >
> > I hope this was known from the beginning, this is not sha1 unfortunately.
> >
> > But ebpf elf programs also need preprocessing to get rid of some
> > embedded load-depending data, so maybe it was considered to be just
> > enough?
>
> I suspect it was actually an accident.
Maybe, I don't know.
Bye,
Hannes
^ permalink raw reply
* Re: BPF hash algo (Re: [kernel-hardening] Re: [PATCH v7 3/6] random: use SipHash in place of MD5)
From: Andy Lutomirski @ 2016-12-22 17:25 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Daniel Borkmann, Alexei Starovoitov, Jason A. Donenfeld,
kernel-hardening@lists.openwall.com, Theodore Ts'o, Netdev,
LKML, Linux Crypto Mailing List, David Laight, Eric Dumazet,
Linus Torvalds, Eric Biggers, Tom Herbert, Andi Kleen,
David S. Miller, Jean-Philippe Aumasson
In-Reply-To: <1482425969.2673.5.camel@stressinduktion.org>
On Thu, Dec 22, 2016 at 8:59 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Thu, 2016-12-22 at 08:07 -0800, Andy Lutomirski wrote:
>> On Thu, Dec 22, 2016 at 7:51 AM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>> > On Thu, 2016-12-22 at 16:41 +0100, Jason A. Donenfeld wrote:
>> > > Hi Hannes,
>> > >
>> > > On Thu, Dec 22, 2016 at 4:33 PM, Hannes Frederic Sowa
>> > > <hannes@stressinduktion.org> wrote:
>> > > > IPv6 you cannot touch anymore. The hashing algorithm is part of uAPI.
>> > > > You don't want to give people new IPv6 addresses with the same stable
>> > > > secret (across reboots) after a kernel upgrade. Maybe they lose
>> > > > connectivity then and it is extra work?
>> > >
>> > > Ahh, too bad. So it goes.
>> >
>> > If no other users survive we can put it into the ipv6 module.
>> >
>> > > > The bpf hash stuff can be changed during this merge window, as it is
>> > > > not yet in a released kernel. Albeit I would probably have preferred
>> > > > something like sha256 here, which can be easily replicated by user
>> > > > space tools (minus the problem of patching out references to not
>> > > > hashable data, which must be zeroed).
>> > >
>> > > Oh, interesting, so time is of the essence then. Do you want to handle
>> > > changing the new eBPF code to something not-SHA1 before it's too late,
>> > > as part of a ne
>>
>> w patchset that can fast track itself to David? And
>> > > then I can preserve my large series for the next merge window.
>> >
>> > This algorithm should be a non-seeded algorithm, because the hashes
>> > should be stable and verifiable by user space tooling. Thus this would
>> > need a hashing algorithm that is hardened against pre-image
>> > attacks/collision resistance, which siphash is not. I would prefer some
>> > higher order SHA algorithm for that actually.
>> >
>>
>> You mean:
>>
>> commit 7bd509e311f408f7a5132fcdde2069af65fa05ae
>> Author: Daniel Borkmann <daniel@iogearbox.net>
>> Date: Sun Dec 4 23:19:41 2016 +0100
>>
>> bpf: add prog_digest and expose it via fdinfo/netlink
>>
>>
>> Yes, please! This actually matters for security -- imagine a
>> malicious program brute-forcing a collision so that it gets loaded
>> wrong. And this is IMO a use case for SHA-256 or SHA-512/256
>> (preferably the latter). Speed basically doesn't matter here and
>> Blake2 is both less stable (didn't they slightly change it recently?)
>> and much less well studied.
>
> We don't prevent ebpf programs being loaded based on the digest but
> just to uniquely identify loaded programs from user space and match up
> with their source.
The commit log talks about using the hash to see if the program has
already been compiled and JITted. If that's done, then a collision
will directly cause the kernel to malfunction.
>> My inclination would have been to seed them with something that isn't
>> exposed to userspace for the precise reason that it would prevent user
>> code from making assumptions about what's in the hash. But if there's
>> a use case for why user code needs to be able to calculate the hash on
>> its own, then that's fine. But perhaps the actual fdinfo string
>> should be "sha256:abcd1234..." to give some flexibility down the road.
>>
>> Also:
>>
>> + result = (__force __be32 *)fp->digest;
>> + for (i = 0; i < SHA_DIGEST_WORDS; i++)
>> + result[i] = cpu_to_be32(fp->digest[i]);
>>
>> Everyone, please, please, please don't open-code crypto primitives.
>> Is this and the code above it even correct? It might be but on a very
>> brief glance it looks wrong to me. If you're doing this to avoid
>> depending on crypto, then fix crypto so you can pull in the algorithm
>> without pulling in the whole crypto core.
>
> The hashing is not a proper sha1 neither, unfortunately. I think that
> is why it will have a custom implementation in iproute2?
Putting on crypto hat:
NAK NAK NAK NAK NAK. "The Linux kernel invented a new primitive in
2016 when people know better and is going to handle it by porting that
new primitive to userspace" is not a particularly good argument.
Okay, crypto hack back off.
>
> I wondered if bpf program loading should have used the module loading
> infrastructure from the beginning...
That would be way too complicated and would be nasty for the unprivileged cases.
>
>> At the very least, there should be a separate function that calculates
>> the hash of a buffer and that function should explicitly run itself
>> against test vectors of various lengths to make sure that it's
>> calculating what it claims to be calculating. And it doesn't look
>> like the userspace code has landed, so, if this thing isn't
>> calculating SHA1 correctly, it's plausible that no one has noticed.
>
> I hope this was known from the beginning, this is not sha1 unfortunately.
>
> But ebpf elf programs also need preprocessing to get rid of some
> embedded load-depending data, so maybe it was considered to be just
> enough?
I suspect it was actually an accident.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox