Re: scatterwalk_map_and_copy incorrect optimization
On Fri, Dec 09, 2016 at 02:18:01PM +0100, Jason A. Donenfeld wrote: > Hi Herbert, > > The scatterwalk_map_and_copy function copies ordinary buffers to and > from scatterlists. These buffers can, of course, be on the stack, and > this remains the most popular use of this function -- getting info > between stack buffers and DMA regions. It's mostly used for adding or > checking MACs, in the majority of call sites. Its implementation is > relatively straightforward. It maps the DMA region(s) to a vaddr, and > then just calls vanilla memcpy. Pretty uncontroversial. > > However, around ~4.1 an optimization was added to prevent copying when > unnecessary (when the src and dst are the same). The optimization > looks like this: > > if (sg_page(sg) == virt_to_page(buf) && >sg->offset == offset_in_page(buf)) >return; > > There are two problems with this: This code no longer exists in the current tree. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: algif for compression?
abed mohammad kamaluddin wrote: > > We are also looking for user-space access to the kernel crypto layer > compression algorithms. I have gone through AF_ALG but couldn’t find > support for compression ops through it. This is required for our > hardware zip engines whose driver hooks up to the crypto layer. > > I was going through the lists and stumbled across this thread. Has > there been any updates or efforts put up in this direction since this > thread is a few years old. If not, are there any alternate > implementations that allow user-space access to compression? I have > gone through cryptodev and see the same limitation there. > > Would appreciate any pointers in this regard. The compression interface is currently in a state of flux. We should make it settle down first before exporting it to user-space. For a start it would be good to actually switch IPsec/IPcomp over to the new compression interface. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scatterwalk_map_and_copy incorrect optimization
Jason A. Donenfeld wrote: > Hah, looks like I missed [1] by a couple weeks. Looks like it's been > settled then. > > Is this a stable@ candidate? Not really since it shouldn't cause any problems unless the stack is vmalloced. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
Why did you drop me from the CC list when you were replying to my email? Eric Biggers wrote: > On Sat, Dec 10, 2016 at 01:32:08PM +0800, Herbert Xu wrote: > >> Are you sure? Any instance of *_ON_STACK must only be used with >> sync algorithms and most drivers under drivers/crypto declare >> themselves as async. > > Why exactly is that? Obviously, it wouldn't work if you returned from the > stack > frame before the request completed, but does anything stop someone from using > an > *_ON_STACK() request and then waiting for the request to complete before > returning from the stack frame? The *_ON_STACK variants (except SHASH of course) were simply hacks to help legacy crypto API users to cope with the new async interface. In general we should avoid using the sync interface when possible. It's a bad idea for the obvious reason that most of our async algorithms want to DMA and that doesn't work very well when you're using memory from the stack. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
On Sat, Dec 10, 2016 at 04:16:43PM +0800, Herbert Xu wrote: > Why did you drop me from the CC list when you were replying to > my email? > Sorry --- this thread is Cc'ed to the kernel-hardening mailing list (which was somewhat recently revived), and I replied to the email that reached me from there. It looks like it currently behaves a little differently from the vger mailing lists, in that it replaces "Reply-To" with the address of the mailing list itself rather than the sender. So that's how you got dropped. It also seems to add a prefix to the subject... I > >> Are you sure? Any instance of *_ON_STACK must only be used with > >> sync algorithms and most drivers under drivers/crypto declare > >> themselves as async. > > > > Why exactly is that? Obviously, it wouldn't work if you returned from the > > stack > > frame before the request completed, but does anything stop someone from > > using an > > *_ON_STACK() request and then waiting for the request to complete before > > returning from the stack frame? > > The *_ON_STACK variants (except SHASH of course) were simply hacks > to help legacy crypto API users to cope with the new async interface. > In general we should avoid using the sync interface when possible. > > It's a bad idea for the obvious reason that most of our async > algorithms want to DMA and that doesn't work very well when you're > using memory from the stack. Sure, I just feel that the idea of "is this algorithm asynchronous?" is being conflated with the idea of "does this algorithm operate on physical memory?". Also, if *_ON_STACK are really not allowed with asynchronous algorithms can there at least be a comment or a WARN_ON() to express this? Thanks, Eric -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v6 1/2] sparc: fix a building error reported by kbuild
Regards, -Gonglei > -Original Message- > From: linux-crypto-ow...@vger.kernel.org > [mailto:linux-crypto-ow...@vger.kernel.org] On Behalf Of Sam Ravnborg > Sent: Saturday, December 10, 2016 5:59 AM > To: Gonglei (Arei) > Cc: linux-ker...@vger.kernel.org; qemu-de...@nongnu.org; > virtio-...@lists.oasis-open.org; virtualizat...@lists.linux-foundation.org; > linux-crypto@vger.kernel.org; Luonengjun; m...@redhat.com; > stefa...@redhat.com; Huangweidong (C); Wubin (H); xin.z...@intel.com; > Claudio Fontana; herb...@gondor.apana.org.au; pa...@linux.vnet.ibm.com; > da...@davemloft.net; Zhoujian (jay, Euler); Hanweidong (Randy); > arei.gong...@hotmail.com; cornelia.h...@de.ibm.com; Xuquan (Quan Xu); > longpeng; Wanzongshun (Vincent); sparcli...@vger.kernel.org > Subject: Re: [PATCH v6 1/2] sparc: fix a building error reported by kbuild > > Hi Gonglei. > > On Thu, Dec 08, 2016 at 12:37:08PM +0800, Gonglei wrote: > > >> arch/sparc/include/asm/topology_64.h:44:44: > > error: implicit declaration of function 'cpu_data' > > [-Werror=implicit-function-declaration] > > > > #define topology_physical_package_id(cpu) (cpu_data(cpu).proc_id) > >^ > > Let's include cpudata.h in topology_64.h. > > > > Cc: Sam Ravnborg > > Cc: David S. Miller > > Cc: sparcli...@vger.kernel.org > > Suggested-by: Sam Ravnborg > > Signed-off-by: Gonglei > Acked-by: Sam Ravnborg > Thanks. > > --- > > arch/sparc/include/asm/topology_64.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/sparc/include/asm/topology_64.h > b/arch/sparc/include/asm/topology_64.h > > index 7b4898a..2255430 100644 > > --- a/arch/sparc/include/asm/topology_64.h > > +++ b/arch/sparc/include/asm/topology_64.h > > @@ -4,6 +4,7 @@ > > #ifdef CONFIG_NUMA > > > > #include > > +#include > > Nitpick - if you are going to resend this patch, It depends on the maintainer's thought. :) > then please order the two includes in alphabetic order. > > For two includes this looks like bikeshedding, but when we add > more having them in a defined arder prevents merge conflicts. > And makes it readable too. > > We also sometimes order the includes with the longest lines topmost, > and lines with the ame length are ordered alphabetically. > But this is not seen so often. > Regards, -Gonglei -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] [PATCH] siphash: add cryptographically secure hashtable function
On Fri, Dec 09, 2016 at 07:36:59PM +0100, Jason A. Donenfeld wrote:
> SipHash is a 64-bit keyed hash function that is actually a
> cryptographically secure PRF, like HMAC. Except SipHash is super fast,
> and is meant to be used as a hashtable keyed lookup function.
>
> SipHash isn't just some new trendy hash function. It's been around for a
> while, and there really isn't anything that comes remotely close to
> being useful in the way SipHash is. With that said, why do we need this?
>
> There are a variety of attacks known as "hashtable poisoning" in which an
> attacker forms some data such that the hash of that data will be the
> same, and then preceeds to fill up all entries of a hashbucket. This is
> a realistic and well-known denial-of-service vector.
>
> Linux developers already seem to be aware that this is an issue, and
> various places that use hash tables in, say, a network context, use a
> non-cryptographically secure function (usually jhash) and then try to
> twiddle with the key on a time basis (or in many cases just do nothing
> and hope that nobody notices). While this is an admirable attempt at
> solving the problem, it doesn't actually fix it. SipHash fixes it.
>
> (It fixes it in such a sound way that you could even build a stream
> cipher out of SipHash that would resist the modern cryptanalysis.)
>
> There are a modicum of places in the kernel that are vulnerable to
> hashtable poisoning attacks, either via userspace vectors or network
> vectors, and there's not a reliable mechanism inside the kernel at the
> moment to fix it. The first step toward fixing these issues is actually
> getting a secure primitive into the kernel for developers to use. Then
> we can, bit by bit, port things over to it as deemed appropriate.
>
> Dozens of languages are already using this internally for their hash
> tables. Some of the BSDs already use this in their kernels. SipHash is
> a widely known high-speed solution to a widely known problem, and it's
> time we catch-up.
>
> Signed-off-by: Jason A. Donenfeld
> Cc: Jean-Philippe Aumasson
> Cc: Daniel J. Bernstein
> ---
> include/linux/siphash.h | 18 ++
> lib/Makefile| 3 +-
> lib/siphash.c | 163
>
> 3 files changed, 183 insertions(+), 1 deletion(-)
> create mode 100644 include/linux/siphash.h
> create mode 100644 lib/siphash.c
This looks really nice, but we don't usually add stuff into lib/ unless
there is an actual user of the code :)
Have you tried converting any of the existing hash users to use this
instead? If you did that, and it shows a solution for the known
problems with our existing hashes (as you point out above), I doubt
there would be any objection for this patch at all.
Minor coding style nits below:
> @@ -0,0 +1,18 @@
> +/* Copyright (C) 2016 Jason A. Donenfeld
> + *
> + * SipHash: a fast short-input PRF
> + * https://131002.net/siphash/
> + */
> +
> +#ifndef _LINUX_SIPHASH_H
> +#define _LINUX_SIPHASH_H
> +
> +#include
> +
> +enum siphash24_lengths {
> + SIPHASH24_KEY_LEN = 16
> +};
> +
> +uint64_t siphash24(const uint8_t *data, size_t len, const uint8_t
> key[SIPHASH24_KEY_LEN]);
Please use u64 and u8 instead of the userspace uint64_t and uint8_t
types for kernel code. Yes, the ship has probably sailed for trying to
strictly enforce it, but it's a good idea to do where ever possible.
> +
> +#endif /* _LINUX_SIPHASH_H */
> diff --git a/lib/Makefile b/lib/Makefile
> index 50144a3aeebd..d224337b0d01 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -22,7 +22,8 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \
>sha1.o chacha20.o md5.o irq_regs.o argv_split.o \
>flex_proportions.o ratelimit.o show_mem.o \
>is_single_threaded.o plist.o decompress.o kobject_uevent.o \
> - earlycpio.o seq_buf.o nmi_backtrace.o nodemask.o win_minmax.o
> + earlycpio.o seq_buf.o siphash.o \
> + nmi_backtrace.o nodemask.o win_minmax.o
>
> lib-$(CONFIG_MMU) += ioremap.o
> lib-$(CONFIG_SMP) += cpumask.o
> diff --git a/lib/siphash.c b/lib/siphash.c
> new file mode 100644
> index ..022d86f04b9b
> --- /dev/null
> +++ b/lib/siphash.c
> @@ -0,0 +1,163 @@
> +/* Copyright (C) 2015-2016 Jason A. Donenfeld
> + * Copyright (C) 2012-2014 Jean-Philippe Aumasson
>
> + * Copyright (C) 2012-2014 Daniel J. Bernstein
> + *
> + * SipHash: a fast short-input PRF
> + * https://131002.net/siphash/
> + */
Any specific license for this code? It's good to at the least say what
it is. Yes, we know it will default to GPLv2 only as part of the whole
kernel tree, but it's good to be explicit for when someone wants to copy
this code for their own projects...
> +
> +#include
> +#include
> +#include
> +
> +#define ROTL(x,b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b
Don't we have this in kernel.h somewhere? Ah, yeah, it's rol64() in
bitops.h, no need to define it again please.
> +#define U8TO64(p) le64_to_cpu(*(__le64
Re: [PATCH] siphash: add cryptographically secure hashtable function
On 9 December 2016 at 19:36, Jason A. Donenfeld wrote:
> SipHash is a 64-bit keyed hash function that is actually a
> cryptographically secure PRF, like HMAC. Except SipHash is super fast,
> and is meant to be used as a hashtable keyed lookup function.
>
> SipHash isn't just some new trendy hash function. It's been around for a
> while, and there really isn't anything that comes remotely close to
> being useful in the way SipHash is. With that said, why do we need this?
>
> There are a variety of attacks known as "hashtable poisoning" in which an
> attacker forms some data such that the hash of that data will be the
> same, and then preceeds to fill up all entries of a hashbucket. This is
> a realistic and well-known denial-of-service vector.
>
> Linux developers already seem to be aware that this is an issue, and
> various places that use hash tables in, say, a network context, use a
> non-cryptographically secure function (usually jhash) and then try to
> twiddle with the key on a time basis (or in many cases just do nothing
> and hope that nobody notices). While this is an admirable attempt at
> solving the problem, it doesn't actually fix it. SipHash fixes it.
Could you give some more concrete details/examples? Here's the IPv4
hash table from include/net/inet_sock.h / net/ipv4/inet_hashtables.c:
static inline unsigned int __inet_ehashfn(const __be32 laddr,
const __u16 lport,
const __be32 faddr,
const __be16 fport,
u32 initval)
{
return jhash_3words((__force __u32) laddr,
(__force __u32) faddr,
((__u32) lport) << 16 | (__force __u32)fport,
initval);
}
static u32 inet_ehashfn(const struct net *net, const __be32 laddr,
const __u16 lport, const __be32 faddr,
const __be16 fport)
{
static u32 inet_ehash_secret __read_mostly;
net_get_random_once(&inet_ehash_secret, sizeof(inet_ehash_secret));
return __inet_ehashfn(laddr, lport, faddr, fport,
inet_ehash_secret + net_hash_mix(net));
}
There's a 32-bit secret random salt (inet_ehash_secret) which means
that in practice, inet_ehashfn() will select 1 out of 2^32 different
hash functions at random each time you boot the kernel; without
knowing which one it selected, how can a local or remote attacker can
force IPv4 connections/whatever to go into a single hash bucket?
It is not possible to obtain the secret salt directly (except by
reading from kernel memory, in which case you've lost already), nor is
it possible to obtain the result of inet_ehashfn() other than (maybe)
by a timing attack where you somehow need to detect that two
connections went into the same hash bucket and work backwards from
that to figure out how to land more connections into into the same
bucket -- but if they can do that, you've also already lost.
The same pattern is used for IPv6 hashtables and the dentry cache.
I suppose that using a hash function proven to be cryptographically
secure gives a hard guarantee (under some assumptions) that the
salt/key will give enough diversity between the (in the example above)
2^32 different hash functions that you cannot improve your chances of
guessing that two values will map to the same bucket regardless of the
salt/key. However, I am a bit doubtful that using a cryptographically
secure hash function will make much of a difference as long as the
attacker doesn't actually have any way to get the output/result of the
hash function (and given that the hash function isn't completely
trivial, of course).
I am happy to be proven wrong, but you make it sound very easy to
exploit the current situation, so I would just like to ask whether you
have a concrete way to do that?
Vegard
> There are a modicum of places in the kernel that are vulnerable to
> hashtable poisoning attacks, either via userspace vectors or network
> vectors, and there's not a reliable mechanism inside the kernel at the
> moment to fix it. The first step toward fixing these issues is actually
> getting a secure primitive into the kernel for developers to use. Then
> we can, bit by bit, port things over to it as deemed appropriate.
>
> Dozens of languages are already using this internally for their hash
> tables. Some of the BSDs already use this in their kernels. SipHash is
> a widely known high-speed solution to a widely known problem, and it's
> time we catch-up.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
Hi Herbert, On Sat, Dec 10, 2016 at 6:37 AM, Herbert Xu wrote: > As for AEAD we never had a sync interface to begin with and I > don't think I'm going to add one. That's too bad to hear. I hope you'll reconsider. Modern cryptographic design is heading more and more in the direction of using AEADs for interesting things, and having a sync interface would be a lot easier for implementing these protocols. In the same way many protocols need a hash of some data, now protocols often want some particular data encrypted with an AEAD using a particular key and nonce and AD. One protocol that comes to mind is Noise [1]. I know that in my own [currently external to the tree] kernel code, I just forego the use of the crypto API all together, and one of the primary reasons for that is lack of a sync interface for AEADs. When I eventually send this upstream, presumably everyone will want me to use the crypto API, and having a sync AEAD interface would be personally helpful for that. I guess I could always write the sync interface myself, but I imagine you'd prefer having the design control etc. Jason [1] http://noiseprotocol.org/ -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] siphash: add cryptographically secure hashtable function
> There's a 32-bit secret random salt (inet_ehash_secret) which means > that in practice, inet_ehashfn() will select 1 out of 2^32 different > hash functions at random each time you boot the kernel; without > knowing which one it selected, how can a local or remote attacker can > force IPv4 connections/whatever to go into a single hash bucket? By figuring out the salt. The thing is, the timing of hash table lookups *is externally visible*. If I create connections to the target, then see which ones make responses on previous connections slightly slower, I gain information about the salt. I dont't know *where* in the hash table the collissions occur, but I know *which* inputs collide, and that's enough for me to learn something. (I need more connections than the size of the hash table, but even with just one IP source I can use 64K ports on my end times however many the target has open on its end.) With enough information (google "unicity distance") I can recover the entire salt. It's not like I care about the cryptographic strength of the hash; simply trying all 4 billion possible seeds is pretty fast on a 4 GHz processor. Once that happens, I can choose a target connection whose timing I can't observe directly and pack its hash chain without being obvious about it. > I am happy to be proven wrong, but you make it sound very easy to > exploit the current situation, so I would just like to ask whether you > have a concrete way to do that? I don't think anyone's implemented an attack on this particular hash table yet, and the reason it hasn't been urgent is that it's just a mild DoS attack it makes the computer noticeably slower withough disabling it completely. But the general style of attack is well known and has been repeatedly demonstrated. Its practicality is not in question. The only question is whether it's *more* practical that simpler techniques that don't depend on any such algorithmic subtlety like brute-force flooding. But if the history of Internet security has taught us one thing, it's that naively hoping something won't be a problem is doomed. The main issue is performance. IPv6 addresses are big, and although SipHash is fast by the standard of cryptographic hashes, it's far slower than jhash or any other non-cryptographic hash. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kernel-hardening] Re: Remaining crypto API regressions with CONFIG_VMAP_STACK
cc: Viro because I'm talking about iov_iter. On Sat, Dec 10, 2016 at 6:45 AM, Jason A. Donenfeld wrote: > Hi Herbert, > > On Sat, Dec 10, 2016 at 6:37 AM, Herbert Xu > wrote: >> As for AEAD we never had a sync interface to begin with and I >> don't think I'm going to add one. > > That's too bad to hear. I hope you'll reconsider. Modern cryptographic > design is heading more and more in the direction of using AEADs for > interesting things, and having a sync interface would be a lot easier > for implementing these protocols. In the same way many protocols need > a hash of some data, now protocols often want some particular data > encrypted with an AEAD using a particular key and nonce and AD. One > protocol that comes to mind is Noise [1]. > I think that sync vs async has gotten conflated with vectored-vs-nonvectored and the results are unfortunate. There are a lot of users in the tree that are trying to do crypto on very small pieces of data and want to have that data consist of the concatenation of two small buffers and/or want to use primitives that don't have "sync" interfaces. These users are stuck using async interfaces even though using async implementations makes no sense for them. I'd love to see the API restructured a bit to decouple all of these considerations. One approach might be to teach iov_iter about scatterlists. Then, for each primitive, there could be two entry points: 1. A simplified and lower-overhead entry. You pass it an iov_iter (and, depending on what the operation is, an output iov_iter), it does the crypto synchronously, and returns. Operating in-place might be permitted for some primitives. 2. A full-featured async entry. You pass it iov_iters and it requires that the iov_iters be backed by scatterlists in order to operate asynchronously. I see no reason that the decisions to use virtual vs physical addressing or to do vectored vs non-vectored IO should be tied up with asynchronicity. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] crypto: brcm: Add Broadcom SPU driver DT entry.
Hi Rob, [auto build test ERROR on cryptodev/master] [also build test ERROR on v4.9-rc8] [cannot apply to next-20161209] [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/Rob-Rice/crypto-brcm-DT-documentation-for-Broadcom-SPU-driver/20161202-010038 base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master config: arm64-defconfig (attached as .config) compiler: aarch64-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=arm64 All errors (new ones prefixed by >>): >> ERROR: Input tree has errors, aborting (use -f to force output) --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
