Re: [PATCH] crypto: sun4i-ss: support the Security System PRNG
On Tue, Jun 20, 2017 at 01:45:36PM +0200, Corentin Labbe wrote: > On Tue, Jun 20, 2017 at 11:59:47AM +0200, Maxime Ripard wrote: > > Hi, > > > > On Tue, Jun 20, 2017 at 10:58:19AM +0200, Corentin Labbe wrote: > > > The Security System have a PRNG, this patch add support for it via > > > crypto_rng. > > > > This might be a dumb question, but is the CRYPTO_RNG code really > > supposed to be used with PRNG? > > > > Yes, see recently added drivers/crypto/exynos-rng.c It's still not really clear from the commit log (if you're talking about c46ea13f55b6) why and if using the RNG code for a PRNG is a good idea. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature
Re: [kernel-hardening] [PATCH] random: warn when kernel uses unseeded randomness
"Jason A. Donenfeld" writes: > This enables an important dmesg notification about when drivers have > used the crng without it being seeded first. Prior, these errors would > occur silently, and so there hasn't been a great way of diagnosing these > types of bugs for obscure setups. By adding this as a config option, we > can leave it on by default, so that we learn where these issues happen, > in the field, will still allowing some people to turn it off, if they > really know what they're doing and do not want the log entries. > > However, we don't leave it _completely_ by default. An earlier version > of this patch simply had `default y`. I'd really love that, but it turns > out, this problem with unseeded randomness being used is really quite > present and is going to take a long time to fix. Thus, as a compromise > between log-messages-for-all and nobody-knows, this is `default y`, > except it is also `depends on DEBUG_KERNEL`. This will ensure that the > curious see the messages while others don't have to. All the distro kernels I'm aware of have DEBUG_KERNEL=y. Where all includes at least RHEL, SLES, Fedora, Ubuntu & Debian. So it's still essentially default y. Emitting *one* warning by default would be reasonable. That gives users who are interested something to chase, they can then turn on the option to get the full story. Filling the dmesg buffer with repeated warnings is really not helpful. cheers
RE: [PATCH v0] crypto: virtio - Refacotor virtio_crypto driver for new virito crypto services
Ping... any comments? Thanks < -Original Message- < From: Zeng, Xin < Sent: Wednesday, June 7, 2017 2:18 PM < To: linux-ker...@vger.kernel.org; linux-crypto-ow...@vger.kernel.org; < virtio-...@lists.oasis-open.org < Cc: arei.gong...@huawei.com; Zeng, Xin < Subject: [PATCH v0] crypto: virtio - Refacotor virtio_crypto driver for new < virito crypto services < < In current virtio crypto device driver, some common data structures and < implementations that should be used by other virtio crypto algorithms < (e.g. asymmetric crypto algorithms) introduce symmetric crypto algorithms < specific implementations. < This patch refactors these pieces of code so that they can be reused by < other virtio crypto algorithms. < < Signed-off-by: Xin Zeng < --- < drivers/crypto/virtio/virtio_crypto_algs.c | 109 +- < - < drivers/crypto/virtio/virtio_crypto_common.h | 22 +- < drivers/crypto/virtio/virtio_crypto_core.c | 37 ++--- < 3 files changed, 98 insertions(+), 70 deletions(-) < < diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c < b/drivers/crypto/virtio/virtio_crypto_algs.c < index 49defda..5035b0d 100644 < --- a/drivers/crypto/virtio/virtio_crypto_algs.c < +++ b/drivers/crypto/virtio/virtio_crypto_algs.c < @@ -27,12 +27,68 @@ < #include < #include "virtio_crypto_common.h" < < + < +struct virtio_crypto_ablkcipher_ctx { < + struct virtio_crypto *vcrypto; < + struct crypto_tfm *tfm; < + < + struct virtio_crypto_sym_session_info enc_sess_info; < + struct virtio_crypto_sym_session_info dec_sess_info; < +}; < + < +struct virtio_crypto_sym_request { < + struct virtio_crypto_request base; < + < + /* Cipher or aead */ < + uint32_t type; < + struct virtio_crypto_ablkcipher_ctx *ablkcipher_ctx; < + struct ablkcipher_request *ablkcipher_req; < + uint8_t *iv; < + /* Encryption? */ < + bool encrypt; < +}; < + < /* < * The algs_lock protects the below global virtio_crypto_active_devs < * and crypto algorithms registion. < */ < static DEFINE_MUTEX(algs_lock); < static unsigned int virtio_crypto_active_devs; < +static void virtio_crypto_ablkcipher_finalize_req( < + struct virtio_crypto_sym_request *vc_sym_req, < + struct ablkcipher_request *req, < + int err); < + < +static void virtio_crypto_dataq_sym_callback < + (struct virtio_crypto_request *vc_req, int len) < +{ < + struct virtio_crypto_sym_request *vc_sym_req = < + container_of(vc_req, struct virtio_crypto_sym_request, < base); < + struct ablkcipher_request *ablk_req; < + int error; < + < + /* Finish the encrypt or decrypt process */ < + if (vc_sym_req->type == VIRTIO_CRYPTO_SYM_OP_CIPHER) { < + switch (vc_req->status) { < + case VIRTIO_CRYPTO_OK: < + error = 0; < + break; < + case VIRTIO_CRYPTO_INVSESS: < + case VIRTIO_CRYPTO_ERR: < + error = -EINVAL; < + break; < + case VIRTIO_CRYPTO_BADMSG: < + error = -EBADMSG; < + break; < + default: < + error = -EIO; < + break; < + } < + ablk_req = vc_sym_req->ablkcipher_req; < + virtio_crypto_ablkcipher_finalize_req(vc_sym_req, < + ablk_req, error); < + } < +} < < static u64 virtio_crypto_alg_sg_nents_length(struct scatterlist *sg) < { < @@ -286,13 +342,14 @@ static int virtio_crypto_ablkcipher_setkey(struct < crypto_ablkcipher *tfm, < } < < static int < -__virtio_crypto_ablkcipher_do_req(struct virtio_crypto_request *vc_req, < +__virtio_crypto_ablkcipher_do_req(struct virtio_crypto_sym_request < *vc_sym_req, < struct ablkcipher_request *req, < struct data_queue *data_vq) < { < struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req); < + struct virtio_crypto_ablkcipher_ctx *ctx = vc_sym_req- < >ablkcipher_ctx; < + struct virtio_crypto_request *vc_req = &vc_sym_req->base; < unsigned int ivsize = crypto_ablkcipher_ivsize(tfm); < - struct virtio_crypto_ablkcipher_ctx *ctx = vc_req->ablkcipher_ctx; < struct virtio_crypto *vcrypto = ctx->vcrypto; < struct virtio_crypto_op_data_req *req_data; < int src_nents, dst_nents; < @@ -326,9 +383,9 @@ __virtio_crypto_ablkcipher_do_req(struct < virtio_crypto_request *vc_req, < } < < vc_req->req_data = req_data; < - vc_req->type = VIRTIO_CRYPTO_SYM_OP_CIPHER; < + vc_sym_req->type = VIRTIO_CRYPTO_SYM_OP_CIPHER; < /* Head of operation */ < - if (vc_req->encrypt) { < + if (vc_sym_req->encrypt) { < req_data->header.session_id = < cpu_to_le64(ctx->enc_sess_info.session_id); < req_data->header.opcode = < @@ -383,7 +440,7 @@ __virtio_crypto_ablkcipher_do_req
Re: [PATCH] random: warn when kernel uses unseeded randomness
On Tue, Jun 20, 2017 at 5:03 PM, Jason A. Donenfeld wrote: > This enables an important dmesg notification about when drivers have > used the crng without it being seeded first. Prior, these errors would > occur silently, and so there hasn't been a great way of diagnosing these > types of bugs for obscure setups. By adding this as a config option, we > can leave it on by default, so that we learn where these issues happen, > in the field, will still allowing some people to turn it off, if they > really know what they're doing and do not want the log entries. > > However, we don't leave it _completely_ by default. An earlier version > of this patch simply had `default y`. I'd really love that, but it turns > out, this problem with unseeded randomness being used is really quite > present and is going to take a long time to fix. Thus, as a compromise > between log-messages-for-all and nobody-knows, this is `default y`, > except it is also `depends on DEBUG_KERNEL`. This will ensure that the > curious see the messages while others don't have to. This commit log needs updating (default DEBUG_KERNEL, not depends). But otherwise: Reviewed-by: Kees Cook -Kees > > Signed-off-by: Jason A. Donenfeld > Signed-off-by: Theodore Ts'o > --- > Hi Ted, > > This patch is meant to replace d06bfd1989fe97623b32d6df4ffa6e4338c99dc8, > which is currently in your dev tree. It switches from using `default y` > and `depends on DEBUG_KERNEL` to using the more simple `default DEBUG_KERNEL`. > This kind of change I think should satisfy most potential objections, by > being present for those who might find it useful, but invisble for those > who don't want the spam. > > If you'd like to replace the earlier commit with this one, feel free. If > not, that's fine too. > > Jason > > drivers/char/random.c | 15 +-- > lib/Kconfig.debug | 15 +++ > 2 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/random.c b/drivers/char/random.c > index 3853dd4f92e7..fa5bbd5a7ca0 100644 > --- a/drivers/char/random.c > +++ b/drivers/char/random.c > @@ -288,7 +288,6 @@ > #define SEC_XFER_SIZE 512 > #define EXTRACT_SIZE 10 > > -#define DEBUG_RANDOM_BOOT 0 > > #define LONGS(x) (((x) + sizeof(unsigned long) - 1)/sizeof(unsigned long)) > > @@ -1481,7 +1480,7 @@ void get_random_bytes(void *buf, int nbytes) > { > __u8 tmp[CHACHA20_BLOCK_SIZE]; > > -#if DEBUG_RANDOM_BOOT > 0 > +#ifdef CONFIG_WARN_UNSEEDED_RANDOM > if (!crng_ready()) > printk(KERN_NOTICE "random: %pF get_random_bytes called " >"with crng_init = %d\n", (void *) _RET_IP_, crng_init); > @@ -2075,6 +2074,12 @@ u64 get_random_u64(void) > return ret; > #endif > > +#ifdef CONFIG_WARN_UNSEEDED_RANDOM > + if (!crng_ready()) > + printk(KERN_NOTICE "random: %pF get_random_u64 called " > + "with crng_init = %d\n", (void *) _RET_IP_, crng_init); > +#endif > + > batch = &get_cpu_var(batched_entropy_u64); > if (use_lock) > read_lock_irqsave(&batched_entropy_reset_lock, flags); > @@ -2101,6 +2106,12 @@ u32 get_random_u32(void) > if (arch_get_random_int(&ret)) > return ret; > > +#ifdef CONFIG_WARN_UNSEEDED_RANDOM > + if (!crng_ready()) > + printk(KERN_NOTICE "random: %pF get_random_u32 called " > + "with crng_init = %d\n", (void *) _RET_IP_, crng_init); > +#endif > + > batch = &get_cpu_var(batched_entropy_u32); > if (use_lock) > read_lock_irqsave(&batched_entropy_reset_lock, flags); > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index e4587ebe52c7..41cf12288369 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -1209,6 +1209,21 @@ config STACKTRACE > It is also used by various kernel debugging features that require > stack trace generation. > > +config WARN_UNSEEDED_RANDOM > + bool "Warn when kernel uses unseeded randomness" > + default DEBUG_KERNEL > + help > + Some parts of the kernel contain bugs relating to their use of > + cryptographically secure random numbers before it's actually > possible > + to generate those numbers securely. This setting ensures that these > + flaws don't go unnoticed, by enabling a message, should this ever > + occur. This will allow people with obscure setups to know when > things > + are going wrong, so that they might contact developers about fixing > + it. > + > + Say Y here, unless you simply do not care about using unseeded > + randomness and do not want a potential warning message in your logs. > + > config DEBUG_KOBJECT > bool "kobject debugging" > depends on DEBUG_KERNEL > -- > 2.13.1 > -- Kees Cook Pixel Security
[PATCH] random: warn when kernel uses unseeded randomness
This enables an important dmesg notification about when drivers have used the crng without it being seeded first. Prior, these errors would occur silently, and so there hasn't been a great way of diagnosing these types of bugs for obscure setups. By adding this as a config option, we can leave it on by default, so that we learn where these issues happen, in the field, will still allowing some people to turn it off, if they really know what they're doing and do not want the log entries. However, we don't leave it _completely_ by default. An earlier version of this patch simply had `default y`. I'd really love that, but it turns out, this problem with unseeded randomness being used is really quite present and is going to take a long time to fix. Thus, as a compromise between log-messages-for-all and nobody-knows, this is `default y`, except it is also `depends on DEBUG_KERNEL`. This will ensure that the curious see the messages while others don't have to. Signed-off-by: Jason A. Donenfeld Signed-off-by: Theodore Ts'o --- Hi Ted, This patch is meant to replace d06bfd1989fe97623b32d6df4ffa6e4338c99dc8, which is currently in your dev tree. It switches from using `default y` and `depends on DEBUG_KERNEL` to using the more simple `default DEBUG_KERNEL`. This kind of change I think should satisfy most potential objections, by being present for those who might find it useful, but invisble for those who don't want the spam. If you'd like to replace the earlier commit with this one, feel free. If not, that's fine too. Jason drivers/char/random.c | 15 +-- lib/Kconfig.debug | 15 +++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 3853dd4f92e7..fa5bbd5a7ca0 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -288,7 +288,6 @@ #define SEC_XFER_SIZE 512 #define EXTRACT_SIZE 10 -#define DEBUG_RANDOM_BOOT 0 #define LONGS(x) (((x) + sizeof(unsigned long) - 1)/sizeof(unsigned long)) @@ -1481,7 +1480,7 @@ void get_random_bytes(void *buf, int nbytes) { __u8 tmp[CHACHA20_BLOCK_SIZE]; -#if DEBUG_RANDOM_BOOT > 0 +#ifdef CONFIG_WARN_UNSEEDED_RANDOM if (!crng_ready()) printk(KERN_NOTICE "random: %pF get_random_bytes called " "with crng_init = %d\n", (void *) _RET_IP_, crng_init); @@ -2075,6 +2074,12 @@ u64 get_random_u64(void) return ret; #endif +#ifdef CONFIG_WARN_UNSEEDED_RANDOM + if (!crng_ready()) + printk(KERN_NOTICE "random: %pF get_random_u64 called " + "with crng_init = %d\n", (void *) _RET_IP_, crng_init); +#endif + batch = &get_cpu_var(batched_entropy_u64); if (use_lock) read_lock_irqsave(&batched_entropy_reset_lock, flags); @@ -2101,6 +2106,12 @@ u32 get_random_u32(void) if (arch_get_random_int(&ret)) return ret; +#ifdef CONFIG_WARN_UNSEEDED_RANDOM + if (!crng_ready()) + printk(KERN_NOTICE "random: %pF get_random_u32 called " + "with crng_init = %d\n", (void *) _RET_IP_, crng_init); +#endif + batch = &get_cpu_var(batched_entropy_u32); if (use_lock) read_lock_irqsave(&batched_entropy_reset_lock, flags); diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index e4587ebe52c7..41cf12288369 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1209,6 +1209,21 @@ config STACKTRACE It is also used by various kernel debugging features that require stack trace generation. +config WARN_UNSEEDED_RANDOM + bool "Warn when kernel uses unseeded randomness" + default DEBUG_KERNEL + help + Some parts of the kernel contain bugs relating to their use of + cryptographically secure random numbers before it's actually possible + to generate those numbers securely. This setting ensures that these + flaws don't go unnoticed, by enabling a message, should this ever + occur. This will allow people with obscure setups to know when things + are going wrong, so that they might contact developers about fixing + it. + + Say Y here, unless you simply do not care about using unseeded + randomness and do not want a potential warning message in your logs. + config DEBUG_KOBJECT bool "kobject debugging" depends on DEBUG_KERNEL -- 2.13.1
Re: [PATCH] random: silence compiler warnings and fix race
On Wed, Jun 21, 2017 at 1:38 AM, Theodore Ts'o wrote: > The punch was in response to this statement, which I personally found > fairly infuriating: > >>> I more or less agree with you that we should just turn this on for all >>> users and they'll just have to live with the spam and report odd >>> entries, and overtime we'll fix all the violations. Holy cow, please cool it. I think the "or less" part was relevant, as was the subsequent sentence which characterized that sentiment as "hubris". Also, the subsequent email when I made explicit the fact that I was more in agreement with you than you interpreted. So, in case you're really really really not getting the message I'm trying to make so explicitly clear to you: I AGREE WITH YOU. So, no more punching, pretty please? > > There seems to be a fundamental misapprehension that it will be easy > to "fix all the violations". I don't think it will be easy. I agree with you. > But for other > platforms, it really, REALLY isn't that easy to fix. Other platforms will be hard. I agree with you. > So personally, I think I'm going to roll Kees' suggestion into a PATCH and send it to you. You can decide if you want to apply it. I'll be satisfied with whatever you choose and will follow your lead. Jason
Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 11:49:07AM +0200, Jason A. Donenfeld wrote: > Uh, talk about a totally unnecessary punch... In case my last email > wasn't clear, I fully recognize that `default y` is a tad too extreme, > which is why from one of the earliest revisions in this series, I > moved directly to the compromise solution (`depends DEBUG_KERNEL`) > without even waiting for people to complain first. The punch was in response to this statement, which I personally found fairly infuriating: >> I more or less agree with you that we should just turn this on for all >> users and they'll just have to live with the spam and report odd >> entries, and overtime we'll fix all the violations. There seems to be a fundamental misapprehension that it will be easy to "fix all the violations". For certain hardware types, this is not easy, and the "eh, let them get spammed until we get around to fixing it" attitude is precisely what I was pushing back against. There's a certain amount of privilege for those of us who are using x86 systems with built-in hardware random number generators, and cycle counters, where the problem is much easier to solve. But for other platforms, it really, REALLY isn't that easy to fix. One solution that might be acceptable is to simply print a *single* warning, the first time some piece of kernel code tries to get randomness before the CRNG is initialized. And that's it. If it's only a single dmesg line, then we probably don't need to hide it behind a #ifdef. That might satisfy the security-obsessed who want to rub users' noses in the face that their kernel is doing something potentially insecure and there is nothing they can do about it. But since it's also a single line in the syslog, it's not actively annoying. The #ifdef will allow the current behaviour where we suppress duplicates, but we warn for every attempt to get randomness. That we can default to no, since it will only be people who are trying to audit calls to see if the real fix is to switch the call to prandom_u32, because the use case really was't security/crypto sensitive. As I have said before, ultimately I think the only real solution to this whole mess is to allow the bootloader to read entropy from the boot device (or maybe some NVRAM or battery-backed memory), which is then overwritten as early as possible in the boot process with new random data. This is what OpenBSD does, but OpenBSD has a much easier job because they only have to support a small set of architectures. We will need to do this for each bootloader that is used by Linux, which is a pretty large set. But ultimately, it solves *all* the problems, including getting entropy for KASLR, which needs the randomness super-early in the boot process, long before we have any hope of initializing the entropy pool from environmental noise. So personally, I think this focus on trying to warn/spam users is not particularly useful. If we can mute the warnings to those people who want to play whack-a-mole, it's not harmful, but for those who think that futzing with get_random_* calls is the right approach, personally I'm really not convinced. Of course, the kernel is a volunteer project, so ultimately all a maintainer can do is to say no to patches, and not command people to work on things that he or she wishes. I *can* try to pursude people about what the right way to go is, because doing something that involves boot loaders is going to require a huge amount of effort from many people. It's certainly not anything I or anyone else can do by him or herself. - Ted
Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 8:14 PM, Kees Cook wrote: > How about doing this: > >default DEBUG_KERNEL > > Most distro kernel select DEBUG_KERNEL because it unhides a bunch of > other useful configs. Since it doesn't strictly _depend_ on > DEBUG_KERNEL, I think it's probably a mistake to enforce a false > dependency. Using it as a hint for the default seems maybe like a good > middle ground. (And if people can't agree on that, then I guess > "default n"...) I didn't know you could do that with Kconfig. Great idea. I'll make this change and submit a new patch to Ted. Jason
Re: [PATCH] staging: ccree: fix coding style error
On Tue, Jun 20, 2017 at 10:51:58PM +0800, Jhih-Ming Huang wrote: > > Hi, > > This patch fix all coding style error in driver/staging/ccree/ssi_aead.c. Much better. Thanks! regards, dan carpenter
Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 10:50 AM, Sandy Harris wrote: > On Tue, Jun 20, 2017 at 5:49 AM, Jeffrey Walton wrote: >> On Tue, Jun 20, 2017 at 5:36 AM, Theodore Ts'o wrote: >>> On Tue, Jun 20, 2017 at 10:53:35AM +0200, Jason A. Donenfeld wrote: > > Suppressing all messages for all configurations cast a wider net than > necessary. Configurations that could potentially be detected and fixed > likely will go unnoticed. If the problem is not brought to light, then > it won't be fixed. > >> Are there compelling reasons a single dmesg warning cannot be provided? >> >> A single message avoids spamming the logs. It also informs the system >> owner of the problem. An individual or organization can then take >> action based on their risk posture. Finally, it avoids the kernel >> making policy decisions for a user or organization. > > I'd say the best solution is to have no configuration option > specifically for these messages. Always give some, but let > DEBUG_KERNEL control how many. > > If DEBUG_KERNEL is not set, emit exactly one message & ignore any > other errors of this type. On some systems, that message may have to > be ignored, on some it might start an incremental process where one > problem gets fixed only to have another crop up & on some it might > prompt the admin to explore further by compiling with DEBUG_KERNEL. > > If DEBUG_KERNEL is set, emit a message for every error of this type. How about doing this: default DEBUG_KERNEL Most distro kernel select DEBUG_KERNEL because it unhides a bunch of other useful configs. Since it doesn't strictly _depend_ on DEBUG_KERNEL, I think it's probably a mistake to enforce a false dependency. Using it as a hint for the default seems maybe like a good middle ground. (And if people can't agree on that, then I guess "default n"...) -Kees -- Kees Cook Pixel Security
Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 5:49 AM, Jeffrey Walton wrote: > On Tue, Jun 20, 2017 at 5:36 AM, Theodore Ts'o wrote: >> On Tue, Jun 20, 2017 at 10:53:35AM +0200, Jason A. Donenfeld wrote: >>> > Suppressing all messages for all configurations cast a wider net than >>> > necessary. Configurations that could potentially be detected and fixed >>> > likely will go unnoticed. If the problem is not brought to light, then >>> > it won't be fixed. > Are there compelling reasons a single dmesg warning cannot be provided? > > A single message avoids spamming the logs. It also informs the system > owner of the problem. An individual or organization can then take > action based on their risk posture. Finally, it avoids the kernel > making policy decisions for a user or organization. I'd say the best solution is to have no configuration option specifically for these messages. Always give some, but let DEBUG_KERNEL control how many. If DEBUG_KERNEL is not set, emit exactly one message & ignore any other errors of this type. On some systems, that message may have to be ignored, on some it might start an incremental process where one problem gets fixed only to have another crop up & on some it might prompt the admin to explore further by compiling with DEBUG_KERNEL. If DEBUG_KERNEL is set, emit a message for every error of this type.
Re: [PATCH v2 0/3] add support of hardware random generator on MediaTek MT7622
On Tue, 2017-06-20 at 16:59 +0200, Torsten Duwe wrote: > On Tue, Jun 20, 2017 at 10:21:17PM +0800, Sean Wang wrote: > > Hi Herbert, > > > > thanks for effort reviewing on those patches. > > > > By the way, also loop in Torsten > > > > Could you kindly guide me how to determine appropriate > > rng->ops.quality value used by the driver? > > > > I have tested with rngtest on mtk-cir and the result is got as > > the below log shown. If the rngtest always gives the result for > > success rate over 99.8%, can I set the rng->ops.quality 998? > > > > rngtest: starting FIPS tests... > > rngtest: bits received from input: 2032 > > rngtest: FIPS 140-2 successes: 998 > > rngtest: FIPS 140-2 failures: 2 > > No! You'd have to determine the failure threshold of the test and > apply some math to find a lower boundary of your RNG's entropy. > > What the quality is for: your RNG produces bits, but not all of them > are completely independent of each other i.e. not completely random. > So you simply lower the quality rating to express the net entropy > contained in the data stream. > > Torsten > Hi Torsten, Understood, appreciate your quick and clear explanation. For the math, is there existing any well-known or recommended open source software assisting identify the lower boundary of RNG entropy? I think the logic should be common for all RNGs. thanks again Sean Sean
Re: [PATCH] crypto: cavium: make several functions static
From: Colin King Date: Tue, 20 Jun 2017 11:35:50 +0100 > From: Colin Ian King > > The functions cvm_encrypt, cvm_decrypt, cvm_xts_setkey and > cvm_enc_dec_init does not need to be in global scope, so make > them static. > > Signed-off-by: Colin Ian King Acked-by: David S. Miller
Re: [PATCH v2 0/3] add support of hardware random generator on MediaTek MT7622
On Tue, Jun 20, 2017 at 10:21:17PM +0800, Sean Wang wrote: > Hi Herbert, > > thanks for effort reviewing on those patches. > > By the way, also loop in Torsten > > Could you kindly guide me how to determine appropriate > rng->ops.quality value used by the driver? > > I have tested with rngtest on mtk-cir and the result is got as > the below log shown. If the rngtest always gives the result for > success rate over 99.8%, can I set the rng->ops.quality 998? > > rngtest: starting FIPS tests... > rngtest: bits received from input: 2032 > rngtest: FIPS 140-2 successes: 998 > rngtest: FIPS 140-2 failures: 2 No! You'd have to determine the failure threshold of the test and apply some math to find a lower boundary of your RNG's entropy. What the quality is for: your RNG produces bits, but not all of them are completely independent of each other i.e. not completely random. So you simply lower the quality rating to express the net entropy contained in the data stream. Torsten
[PATCH 2/6] staging: ccree: move brace { to previous line for if.
From: Jhih-Ming Hunag Move brace { to previous line for if. Signed-off-by: Jhih-Ming Hunag --- drivers/staging/ccree/ssi_aead.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c index ca3f11f..6bcab5a 100644 --- a/drivers/staging/ccree/ssi_aead.c +++ b/drivers/staging/ccree/ssi_aead.c @@ -1340,8 +1340,7 @@ static int validate_data_size(struct ssi_aead_ctx *ctx, goto data_size_err; if (ctx->cipher_mode == DRV_CIPHER_CCM) break; - if (ctx->cipher_mode == DRV_CIPHER_GCTR) - { + if (ctx->cipher_mode == DRV_CIPHER_GCTR) { if (areq_ctx->plaintext_authenticate_only == true) areq_ctx->is_single_pass = false; break; @@ -1912,8 +1911,7 @@ static int config_gcm_context(struct aead_request *req) { memcpy(req_ctx->gcm_iv_inc1, req->iv, 16); - if (req_ctx->plaintext_authenticate_only == false) - { + if (req_ctx->plaintext_authenticate_only == false) { __be64 temp64; temp64 = cpu_to_be64(req->assoclen * 8); memcpy ( &req_ctx->gcm_len_block.lenA , &temp64, sizeof(temp64) ); -- 2.7.4
[PATCH 3/6] staging: ccree: move '{' to next line for function.
From: Jhih-Ming Hunag Move '{' to next line for function. Signed-off-by: Jhih-Ming Hunag --- drivers/staging/ccree/ssi_aead.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c index 6bcab5a..3d9957f 100644 --- a/drivers/staging/ccree/ssi_aead.c +++ b/drivers/staging/ccree/ssi_aead.c @@ -1542,7 +1542,8 @@ static inline int ssi_aead_ccm( return 0; } -static int config_ccm_adata(struct aead_request *req) { +static int config_ccm_adata(struct aead_request *req) +{ struct crypto_aead *tfm = crypto_aead_reqtfm(req); struct ssi_aead_ctx *ctx = crypto_aead_ctx(tfm); struct aead_req_ctx *req_ctx = aead_request_ctx(req); @@ -1886,7 +1887,8 @@ static inline void ssi_aead_dump_gcm( } #endif -static int config_gcm_context(struct aead_request *req) { +static int config_gcm_context(struct aead_request *req) +{ struct crypto_aead *tfm = crypto_aead_reqtfm(req); struct ssi_aead_ctx *ctx = crypto_aead_ctx(tfm); struct aead_req_ctx *req_ctx = aead_request_ctx(req); -- 2.7.4
[PATCH 1/6] Staging: ccree: add space around comma, brace and operator.
From: Jhih-Ming Hunag Add space around comma, brace, and opertor. Signed-off-by: Jhih-Ming Hunag --- drivers/staging/ccree/ssi_aead.c | 36 ++-- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c index e8936a3..ca3f11f 100644 --- a/drivers/staging/ccree/ssi_aead.c +++ b/drivers/staging/ccree/ssi_aead.c @@ -155,7 +155,7 @@ static int ssi_aead_init(struct crypto_aead *tfm) ctx->auth_mode = ssi_alg->auth_mode; ctx->drvdata = ssi_alg->drvdata; dev = &ctx->drvdata->plat_dev->dev; - crypto_aead_set_reqsize(tfm,sizeof(struct aead_req_ctx)); + crypto_aead_set_reqsize(tfm, sizeof(struct aead_req_ctx)); /* Allocate key buffer, cache line aligned */ ctx->enckey = dma_alloc_coherent(dev, AES_MAX_KEY_SIZE, @@ -663,7 +663,7 @@ static int ssi_aead_setauthsize( CHECK_AND_RETURN_UPON_FIPS_ERROR(); /* Unsupported auth. sizes */ if ((authsize == 0) || - (authsize >crypto_aead_maxauthsize(authenc))) { + (authsize > crypto_aead_maxauthsize(authenc))) { return -ENOTSUPP; } @@ -791,7 +791,7 @@ ssi_aead_process_authenc_data_desc( u32 mlli_nents = areq_ctx->assoc.mlli_nents; if (likely(areq_ctx->is_single_pass == true)) { - if (direct == DRV_CRYPTO_DIRECTION_ENCRYPT){ + if (direct == DRV_CRYPTO_DIRECTION_ENCRYPT) { mlli_addr = areq_ctx->dst.sram_addr; mlli_nents = areq_ctx->dst.mlli_nents; } else { @@ -1566,7 +1566,7 @@ static int config_ccm_adata(struct aead_request *req) { /* taken from crypto/ccm.c */ /* 2 <= L <= 8, so 1 <= L' <= 7. */ if (2 > l || l > 8) { - SSI_LOG_ERR("illegal iv value %X\n",req->iv[0]); + SSI_LOG_ERR("illegal iv value %X\n", req->iv[0]); return -EINVAL; } memcpy(b0, req->iv, AES_BLOCK_SIZE); @@ -1715,7 +1715,7 @@ static inline void ssi_aead_gcm_setup_gctr_desc( set_flow_mode(&desc[idx], S_DIN_to_AES); idx++; - if ((req_ctx->cryptlen != 0) && (req_ctx->plaintext_authenticate_only==false)){ + if ((req_ctx->cryptlen != 0) && (req_ctx->plaintext_authenticate_only == false)) { /* load AES/CTR initial CTR value inc by 2*/ hw_desc_init(&desc[idx]); set_cipher_mode(&desc[idx], DRV_CIPHER_GCTR); @@ -1815,7 +1815,7 @@ static inline int ssi_aead_gcm( //in RFC4543 no data to encrypt. just copy data from src to dest. - if (req_ctx->plaintext_authenticate_only==true){ + if (req_ctx->plaintext_authenticate_only == true) { ssi_aead_process_cipher_data_desc(req, BYPASS, desc, seq_size); ssi_aead_gcm_setup_ghash_desc(req, desc, seq_size); /* process(ghash) assoc data */ @@ -1862,27 +1862,27 @@ static inline void ssi_aead_dump_gcm( ctx->cipher_mode, ctx->authsize, ctx->enc_keylen, req->assoclen, req_ctx->cryptlen ); if ( ctx->enckey != NULL ) { - dump_byte_array("mac key",ctx->enckey, 16); + dump_byte_array("mac key", ctx->enckey, 16); } - dump_byte_array("req->iv",req->iv, AES_BLOCK_SIZE); + dump_byte_array("req->iv", req->iv, AES_BLOCK_SIZE); - dump_byte_array("gcm_iv_inc1",req_ctx->gcm_iv_inc1, AES_BLOCK_SIZE); + dump_byte_array("gcm_iv_inc1", req_ctx->gcm_iv_inc1, AES_BLOCK_SIZE); - dump_byte_array("gcm_iv_inc2",req_ctx->gcm_iv_inc2, AES_BLOCK_SIZE); + dump_byte_array("gcm_iv_inc2", req_ctx->gcm_iv_inc2, AES_BLOCK_SIZE); - dump_byte_array("hkey",req_ctx->hkey, AES_BLOCK_SIZE); + dump_byte_array("hkey", req_ctx->hkey, AES_BLOCK_SIZE); - dump_byte_array("mac_buf",req_ctx->mac_buf, AES_BLOCK_SIZE); + dump_byte_array("mac_buf", req_ctx->mac_buf, AES_BLOCK_SIZE); - dump_byte_array("gcm_len_block",req_ctx->gcm_len_block.lenA, AES_BLOCK_SIZE); + dump_byte_array("gcm_len_block", req_ctx->gcm_len_block.lenA, AES_BLOCK_SIZE); - if (req->src!=NULL && req->cryptlen) { - dump_byte_array("req->src",sg_virt(req->src), req->cryptlen+req->assoclen); + if (req->src != NULL && req->cryptlen) { + dump_byte_array("req->src", sg_virt(req->src), req->cryptlen+req->assoclen); } - if (req->dst!=NULL) { - dump_byte_array("req->dst",sg_virt(req->dst), req->cryptlen+ctx->authsize+req->assoclen); + if (req->dst != NULL) { + dump_byte_array("req->dst", sg_virt(req->dst), req->cryptlen+ctx->authsize+req->assoclen); } } #endif @@ -1959,7 +1959,7 @@ static int ssi_aead_process(struct aead_request *req, enum drv_crypto_direction SSI_LOG_DEBUG("%s
[PATCH 5/6] staging: ccree: remove improper space
From: Jhih-Ming Hunag Remove improper space. Signed-off-by: Jhih-Ming Hunag --- drivers/staging/ccree/ssi_aead.c | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c index 6b9de35..57c7c68 100644 --- a/drivers/staging/ccree/ssi_aead.c +++ b/drivers/staging/ccree/ssi_aead.c @@ -1375,10 +1375,10 @@ static int validate_data_size(struct ssi_aead_ctx *ctx, static unsigned int format_ccm_a0(u8 *pA0Buff, u32 headerSize) { unsigned int len = 0; - if ( headerSize == 0 ) { + if (headerSize == 0) { return 0; } - if ( headerSize < ((1UL << 16) - (1UL << 8) )) { + if (headerSize < ((1UL << 16) - (1UL << 8))) { len = 2; pA0Buff[0] = (headerSize >> 8) & 0xFF; @@ -1588,7 +1588,7 @@ static int config_ccm_adata(struct aead_request *req) req_ctx->ccm_hdr_size = format_ccm_a0 (a0, req->assoclen); memset(req->iv + 15 - req->iv[0], 0, req->iv[0] + 1); - req->iv [15] = 1; + req->iv[15] = 1; memcpy(ctr_count_0, req->iv, AES_BLOCK_SIZE) ; ctr_count_0[15] = 0; @@ -1859,9 +1859,9 @@ static inline void ssi_aead_dump_gcm( } SSI_LOG_DEBUG("cipher_mode %d, authsize %d, enc_keylen %d, assoclen %d, cryptlen %d \n", \ -ctx->cipher_mode, ctx->authsize, ctx->enc_keylen, req->assoclen, req_ctx->cryptlen ); +ctx->cipher_mode, ctx->authsize, ctx->enc_keylen, req->assoclen, req_ctx->cryptlen); - if ( ctx->enckey != NULL ) { + if (ctx->enckey != NULL) { dump_byte_array("mac key", ctx->enckey, 16); } @@ -1916,16 +1916,16 @@ static int config_gcm_context(struct aead_request *req) if (req_ctx->plaintext_authenticate_only == false) { __be64 temp64; temp64 = cpu_to_be64(req->assoclen * 8); - memcpy ( &req_ctx->gcm_len_block.lenA , &temp64, sizeof(temp64) ); + memcpy (&req_ctx->gcm_len_block.lenA, &temp64, sizeof(temp64)); temp64 = cpu_to_be64(cryptlen * 8); - memcpy ( &req_ctx->gcm_len_block.lenC , &temp64, 8 ); + memcpy (&req_ctx->gcm_len_block.lenC, &temp64, 8); } else { //rfc4543=> all data(AAD,IV,Plain) are considered additional data that is nothing is encrypted. __be64 temp64; temp64 = cpu_to_be64((req->assoclen+GCM_BLOCK_RFC4_IV_SIZE+cryptlen) * 8); - memcpy ( &req_ctx->gcm_len_block.lenA , &temp64, sizeof(temp64) ); + memcpy (&req_ctx->gcm_len_block.lenA, &temp64, sizeof(temp64)); temp64 = 0; - memcpy ( &req_ctx->gcm_len_block.lenC , &temp64, 8 ); + memcpy (&req_ctx->gcm_len_block.lenC, &temp64, 8); } return 0; @@ -2001,7 +2001,7 @@ static int ssi_aead_process(struct aead_request *req, enum drv_crypto_direction req->iv = areq_ctx->ctr_iv; areq_ctx->hw_iv_size = CTR_RFC3686_BLOCK_SIZE; } else if ((ctx->cipher_mode == DRV_CIPHER_CCM) || - (ctx->cipher_mode == DRV_CIPHER_GCTR) ) { + (ctx->cipher_mode == DRV_CIPHER_GCTR)) { areq_ctx->hw_iv_size = AES_BLOCK_SIZE; if (areq_ctx->ctr_iv != req->iv) { memcpy(areq_ctx->ctr_iv, req->iv, crypto_aead_ivsize(tfm)); @@ -2082,7 +2082,7 @@ static int ssi_aead_process(struct aead_request *req, enum drv_crypto_direction case DRV_HASH_XCBC_MAC: ssi_aead_xcbc_authenc(req, desc, &seq_len); break; -#if ( SSI_CC_HAS_AES_CCM || SSI_CC_HAS_AES_GCM ) +#if (SSI_CC_HAS_AES_CCM || SSI_CC_HAS_AES_GCM) case DRV_HASH_NULL: #if SSI_CC_HAS_AES_CCM if (ctx->cipher_mode == DRV_CIPHER_CCM) { @@ -2146,7 +2146,7 @@ static int ssi_rfc4309_ccm_encrypt(struct aead_request *req) int rc = -EINVAL; if (!valid_assoclen(req)) { - SSI_LOG_ERR("invalid Assoclen:%u\n", req->assoclen ); + SSI_LOG_ERR("invalid Assoclen:%u\n", req->assoclen); goto out; } @@ -2221,7 +2221,7 @@ static int ssi_rfc4106_gcm_setkey(struct crypto_aead *tfm, const u8 *key, unsign struct ssi_aead_ctx *ctx = crypto_aead_ctx(tfm); int rc = 0; - SSI_LOG_DEBUG("ssi_rfc4106_gcm_setkey() keylen %d, key %p \n", keylen, key ); + SSI_LOG_DEBUG("ssi_rfc4106_gcm_setkey() keylen %d, key %p \n", keylen, key); if (keylen < 4) return -EINVAL; @@ -2239,7 +2239,7 @@ static int ssi_rfc4543_gcm_setkey(struct crypto_aead *tfm, const u8 *key, unsign struct ssi_aead_ctx *ctx = crypto_aead_ctx(tfm); int rc = 0; - SSI_LOG_DEBUG("ssi_rfc4543_gcm_setkey() keylen %d, key %p \n", keylen, key ); + SSI_LOG_DEBUG("
[PATCH 6/6] staging: ccree: move else to follow close brace '}'
From: Jhih-Ming Hunag Move else to follow close brace '}' Signed-off-by: Jhih-Ming Hunag --- drivers/staging/ccree/ssi_aead.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c index 57c7c68..c70e450 100644 --- a/drivers/staging/ccree/ssi_aead.c +++ b/drivers/staging/ccree/ssi_aead.c @@ -1919,8 +1919,7 @@ static int config_gcm_context(struct aead_request *req) memcpy (&req_ctx->gcm_len_block.lenA, &temp64, sizeof(temp64)); temp64 = cpu_to_be64(cryptlen * 8); memcpy (&req_ctx->gcm_len_block.lenC, &temp64, 8); - } - else { //rfc4543=> all data(AAD,IV,Plain) are considered additional data that is nothing is encrypted. + } else { //rfc4543=> all data(AAD,IV,Plain) are considered additional data that is nothing is encrypted. __be64 temp64; temp64 = cpu_to_be64((req->assoclen+GCM_BLOCK_RFC4_IV_SIZE+cryptlen) * 8); memcpy (&req_ctx->gcm_len_block.lenA, &temp64, sizeof(temp64)); -- 2.7.4
[PATCH 4/6] staging: ccree: move * to close variable name instead of type.
From: Jhih-Ming Hunag Move * to close variable name instead of type. Signed-off-by: Jhih-Ming Hunag --- drivers/staging/ccree/ssi_aead.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ccree/ssi_aead.c b/drivers/staging/ccree/ssi_aead.c index 3d9957f..6b9de35 100644 --- a/drivers/staging/ccree/ssi_aead.c +++ b/drivers/staging/ccree/ssi_aead.c @@ -1843,7 +1843,7 @@ static inline int ssi_aead_gcm( #ifdef CC_DEBUG static inline void ssi_aead_dump_gcm( - const char* title, + const char *title, struct aead_request *req) { struct crypto_aead *tfm = crypto_aead_reqtfm(req); -- 2.7.4
[PATCH] staging: ccree: fix coding style error
Hi, This patch fix all coding style error in driver/staging/ccree/ssi_aead.c.
Re: [PATCH v2 0/3] add support of hardware random generator on MediaTek MT7622
Hi Herbert, thanks for effort reviewing on those patches. By the way, also loop in Torsten Could you kindly guide me how to determine appropriate rng->ops.quality value used by the driver? There is less clues since the value is not being set in most drivers. But good value decided would allow feeding fresh entropy data with the hwrng so i would like add it in the next patch. I have tested with rngtest on mtk-cir and the result is got as the below log shown. If the rngtest always gives the result for success rate over 99.8%, can I set the rng->ops.quality 998? rngtest: starting FIPS tests... rngtest: bits received from input: 2032 rngtest: FIPS 140-2 successes: 998 rngtest: FIPS 140-2 failures: 2 rngtest: FIPS 140-2(2001-10-10) Monobit: 1 rngtest: FIPS 140-2(2001-10-10) Poker: 0 rngtest: FIPS 140-2(2001-10-10) Runs: 1 rngtest: FIPS 140-2(2001-10-10) Long run: 0 rngtest: FIPS 140-2(2001-10-10) Continuous run: 0 rngtest: input channel speed: (min=715.902; avg=3846.234; max=3906250.000)Kibits/s rngtest: FIPS tests speed: (min=35.453; avg=37.862; max=38.300)Mibits/s rngtest: Program run time: 5591758 microseconds rngtest: starting FIPS tests... rngtest: bits received from input: 2032 rngtest: FIPS 140-2 successes: 1000 rngtest: FIPS 140-2 failures: 0 rngtest: FIPS 140-2(2001-10-10) Monobit: 0 rngtest: FIPS 140-2(2001-10-10) Poker: 0 rngtest: FIPS 140-2(2001-10-10) Runs: 0 rngtest: FIPS 140-2(2001-10-10) Long run: 0 rngtest: FIPS 140-2(2001-10-10) Continuous run: 0 rngtest: input channel speed: (min=715.588; avg=4424.787; max=4882812.500)Kibits/s rngtest: FIPS tests speed: (min=35.785; avg=37.859; max=38.223)Mibits/s rngtest: Program run time: 4947950 microseconds On Tue, 2017-06-20 at 11:40 +0800, Herbert Xu wrote: > On Mon, Jun 12, 2017 at 11:56:53PM +0800, sean.w...@mediatek.com wrote: > > From: Sean Wang > > > > Changes since v1: > > - update the bindings with the specific "mediatek,mt7622-rng" > > instead of the generic one as "mediatek,generic-rng" > > > > The series add support of hardware RNG on MediaTek MT7622 and > > , runtime PM support and add me as the maintainer for the existing > > and following chipset. > > > > Sean Wang (3): > > dt-bindings: rng: add MediaTek MT7622 Hardware Random Generator > > bindings > > hwrng: mtk - add runtime PM support > > MAINTAINERS: add entry for MediaTek Random Number Generator > > All applied. Thanks.
Re: [PATCH 01/11] Fix coding style of driver/staging/ccree/ssi_aead.c ERROR: space required after that
On Tue, 2017-06-20 at 11:20 +0300, Dan Carpenter wrote: > On Tue, Jun 20, 2017 at 01:19:44PM +0800, Jhih-Ming Huang wrote: [] > > In this series patches, I fix all of the coding style error in > > driver/staging/ccree/ssi_aead.c from 54 errors to 0 error. > > You could put this into the cover letter. When we put this into the > final git log we don't see the series only individual patches. Which sometimes is a pity as the cover letter can contain useful information which can easily be added in a merge.
Re: [PATCH] crypto: sun4i-ss: support the Security System PRNG
On Tue, Jun 20, 2017 at 11:59:47AM +0200, Maxime Ripard wrote: > Hi, > > On Tue, Jun 20, 2017 at 10:58:19AM +0200, Corentin Labbe wrote: > > The Security System have a PRNG, this patch add support for it via > > crypto_rng. > > This might be a dumb question, but is the CRYPTO_RNG code really > supposed to be used with PRNG? > Yes, see recently added drivers/crypto/exynos-rng.c [...] > > --- a/drivers/crypto/sunxi-ss/sun4i-ss.h > > +++ b/drivers/crypto/sunxi-ss/sun4i-ss.h > > @@ -32,6 +32,7 @@ > > #include > > #include > > #include > > +#include > > > > #define SS_CTL0x00 > > #define SS_KEY0 0x04 > > @@ -127,6 +128,9 @@ > > #define SS_RXFIFO_EMP_INT_ENABLE (1 << 2) > > #define SS_TXFIFO_AVA_INT_ENABLE (1 << 0) > > > > +#define SS_SEED_LEN (192 / 8) > > +#define SS_DATA_LEN (160 / 8) > > + > > struct sun4i_ss_ctx { > > void __iomem *base; > > int irq; > > @@ -136,6 +140,7 @@ struct sun4i_ss_ctx { > > struct device *dev; > > struct resource *res; > > spinlock_t slock; /* control the use of the device */ > > + u32 seed[SS_SEED_LEN / 4]; > > Shouldn't you define SS_SEED_LEN in bits, and then use either > BITS_PER_BYTE and BITS_PER_LONG so that it's obvious what you're doing > ? > > And you could also make that variable defined based on the option, > otherwise you'll always allocate that array, even if you're not using > it. I will do that Thanks
[PATCH] crypto: cavium: make several functions static
From: Colin Ian King The functions cvm_encrypt, cvm_decrypt, cvm_xts_setkey and cvm_enc_dec_init does not need to be in global scope, so make them static. Signed-off-by: Colin Ian King --- drivers/crypto/cavium/cpt/cptvf_algs.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/cavium/cpt/cptvf_algs.c b/drivers/crypto/cavium/cpt/cptvf_algs.c index 1b220f3ed017..df21d996db7e 100644 --- a/drivers/crypto/cavium/cpt/cptvf_algs.c +++ b/drivers/crypto/cavium/cpt/cptvf_algs.c @@ -222,17 +222,17 @@ static inline int cvm_enc_dec(struct ablkcipher_request *req, u32 enc) return -EINPROGRESS; } -int cvm_encrypt(struct ablkcipher_request *req) +static int cvm_encrypt(struct ablkcipher_request *req) { return cvm_enc_dec(req, true); } -int cvm_decrypt(struct ablkcipher_request *req) +static int cvm_decrypt(struct ablkcipher_request *req) { return cvm_enc_dec(req, false); } -int cvm_xts_setkey(struct crypto_ablkcipher *cipher, const u8 *key, +static int cvm_xts_setkey(struct crypto_ablkcipher *cipher, const u8 *key, u32 keylen) { struct crypto_tfm *tfm = crypto_ablkcipher_tfm(cipher); @@ -336,7 +336,7 @@ static int cvm_ecb_des3_setkey(struct crypto_ablkcipher *cipher, const u8 *key, return cvm_setkey(cipher, key, keylen, DES3_ECB); } -int cvm_enc_dec_init(struct crypto_tfm *tfm) +static int cvm_enc_dec_init(struct crypto_tfm *tfm) { struct cvm_enc_ctx *ctx = crypto_tfm_ctx(tfm); -- 2.11.0
Re: [PATCH] crypto: sun4i-ss: support the Security System PRNG
Hi, On Tue, Jun 20, 2017 at 10:58:19AM +0200, Corentin Labbe wrote: > The Security System have a PRNG, this patch add support for it via > crypto_rng. This might be a dumb question, but is the CRYPTO_RNG code really supposed to be used with PRNG? > Signed-off-by: Corentin Labbe > --- > drivers/crypto/Kconfig | 8 + > drivers/crypto/sunxi-ss/Makefile| 1 + > drivers/crypto/sunxi-ss/sun4i-ss-core.c | 30 ++ > drivers/crypto/sunxi-ss/sun4i-ss-prng.c | 56 > + > drivers/crypto/sunxi-ss/sun4i-ss.h | 9 ++ > 5 files changed, 104 insertions(+) > create mode 100644 drivers/crypto/sunxi-ss/sun4i-ss-prng.c > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig > index ab82536d64e2..bde0b102eb70 100644 > --- a/drivers/crypto/Kconfig > +++ b/drivers/crypto/Kconfig > @@ -618,6 +618,14 @@ config CRYPTO_DEV_SUN4I_SS > To compile this driver as a module, choose M here: the module > will be called sun4i-ss. > > +config CRYPTO_DEV_SUN4I_SS_PRNG > + bool "Support for Allwinner Security System PRNG" > + depends on CRYPTO_DEV_SUN4I_SS > + select CRYPTO_RNG > + help > + Select this option if you to provides kernel-side support for > + the Pseudo-Random Number Generator found in the Security System. > + > config CRYPTO_DEV_ROCKCHIP > tristate "Rockchip's Cryptographic Engine driver" > depends on OF && ARCH_ROCKCHIP > diff --git a/drivers/crypto/sunxi-ss/Makefile > b/drivers/crypto/sunxi-ss/Makefile > index 8f4c7a273141..ccb893219079 100644 > --- a/drivers/crypto/sunxi-ss/Makefile > +++ b/drivers/crypto/sunxi-ss/Makefile > @@ -1,2 +1,3 @@ > obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sun4i-ss.o > sun4i-ss-y += sun4i-ss-core.o sun4i-ss-hash.o sun4i-ss-cipher.o > +sun4i-ss-$(CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG) += sun4i-ss-prng.o > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c > b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > index 02ad8256e900..d6bb2991c000 100644 > --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c > @@ -213,6 +213,23 @@ static struct sun4i_ss_alg_template ss_algs[] = { > } > } > }, > +#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG > +{ > + .type = CRYPTO_ALG_TYPE_RNG, > + .alg.rng = { > + .base = { > + .cra_name = "stdrng", > + .cra_driver_name= "sun4i_ss_rng", > + .cra_priority = 300, > + .cra_ctxsize= 0, > + .cra_module = THIS_MODULE, > + }, > + .generate = sun4i_ss_prng_generate, > + .seed = sun4i_ss_prng_seed, > + .seedsize = SS_SEED_LEN, > + } > +}, > +#endif > }; > > static int sun4i_ss_probe(struct platform_device *pdev) > @@ -355,6 +372,13 @@ static int sun4i_ss_probe(struct platform_device *pdev) > goto error_alg; > } > break; > + case CRYPTO_ALG_TYPE_RNG: > + err = crypto_register_rng(&ss_algs[i].alg.rng); > + if (err) { > + dev_err(ss->dev, "Fail to register %s\n", > + ss_algs[i].alg.rng.base.cra_name); > + } > + break; > } > } > platform_set_drvdata(pdev, ss); > @@ -369,6 +393,9 @@ static int sun4i_ss_probe(struct platform_device *pdev) > case CRYPTO_ALG_TYPE_AHASH: > crypto_unregister_ahash(&ss_algs[i].alg.hash); > break; > + case CRYPTO_ALG_TYPE_RNG: > + crypto_unregister_rng(&ss_algs[i].alg.rng); > + break; > } > } > if (ss->reset) > @@ -393,6 +420,9 @@ static int sun4i_ss_remove(struct platform_device *pdev) > case CRYPTO_ALG_TYPE_AHASH: > crypto_unregister_ahash(&ss_algs[i].alg.hash); > break; > + case CRYPTO_ALG_TYPE_RNG: > + crypto_unregister_rng(&ss_algs[i].alg.rng); > + break; > } > } > > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c > b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c > new file mode 100644 > index ..3941587def6b > --- /dev/null > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c > @@ -0,0 +1,56 @@ > +#include "sun4i-ss.h" > + > +int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed, > +unsigned int slen) > +{ > + struct sun4i_ss_alg_template *algt; > + struct rng_alg *alg = crypto_rng_alg(tfm); > + > + algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng); > + memcpy(algt->ss->seed, seed, slen); > + > + retur
Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 10:53:35AM +0200, Jason A. Donenfeld wrote: > > Suppressing all messages for all configurations cast a wider net than > > necessary. Configurations that could potentially be detected and fixed > > likely will go unnoticed. If the problem is not brought to light, then > > it won't be fixed. > > I more or less agree with you that we should just turn this on for all > users and they'll just have to live with the spam and report odd > entries, and overtime we'll fix all the violations. Fix all the problems *how*? If you are on an old system which doesn't a hardware random number generator, and which doesn't have a high resolution cycle counter, and may not have a lot of entropy easily harvestable from the environment, there may not be a lot you can do. Sure, you can pretend that the cache (which by the way is usually determinstic) is ***so*** complicated that no one can figure it out, and essentially pretend that you have entropy when you probably don't; that just simply becomes a different way of handwaving and suppressing the warning messages. > But I think there's another camp that would mutiny in the face of this > kind of hubris. Blocking the boot for hours and hours until we have enough entropy to initialize the CRNG is ***not*** an acceptable way of making the warning messages go away. Do that and the users **will** mutiny. It's this sort of attitude which is why Linus has in the past said that security people are sometimes insane - Ted
Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 5:36 AM, Theodore Ts'o wrote: > On Tue, Jun 20, 2017 at 10:53:35AM +0200, Jason A. Donenfeld wrote: >> > Suppressing all messages for all configurations cast a wider net than >> > necessary. Configurations that could potentially be detected and fixed >> > likely will go unnoticed. If the problem is not brought to light, then >> > it won't be fixed. >> >> I more or less agree with you that we should just turn this on for all >> users and they'll just have to live with the spam and report odd >> entries, and overtime we'll fix all the violations. > > Fix all the problems *how*? If you are on an old system which doesn't > a hardware random number generator, and which doesn't have a high > resolution cycle counter, and may not have a lot of entropy easily > harvestable from the environment, there may not be a lot you can do. > Sure, you can pretend that the cache (which by the way is usually > determinstic) is ***so*** complicated that no one can figure it out, > and essentially pretend that you have entropy when you probably don't; > that just simply becomes a different way of handwaving and suppressing > the warning messages. > >> But I think there's another camp that would mutiny in the face of this >> kind of hubris. > > Blocking the boot for hours and hours until we have enough entropy to > initialize the CRNG is ***not*** an acceptable way of making the > warning messages go away. Do that and the users **will** mutiny. > > It's this sort of attitude which is why Linus has in the past said > that security people are sometimes insane I don't believe it has anything to do with insanity. Its sound security engineering. Are there compelling reasons a single dmesg warning cannot be provided? A single message avoids spamming the logs. It also informs the system owner of the problem. An individual or organization can then take action based on their risk posture. Finally, it avoids the kernel making policy decisions for a user or organization. Jeff
Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 11:36 AM, Theodore Ts'o wrote: >> But I think there's another camp that would mutiny in the face of this >> kind of hubris. > > Blocking the boot for hours and hours until we have enough entropy to > initialize the CRNG is ***not*** an acceptable way of making the > warning messages go away. Do that and the users **will** mutiny. > > It's this sort of attitude which is why Linus has in the past said > that security people are sometimes insane Uh, talk about a totally unnecessary punch... In case my last email wasn't clear, I fully recognize that `default y` is a tad too extreme, which is why from one of the earliest revisions in this series, I moved directly to the compromise solution (`depends DEBUG_KERNEL`) without even waiting for people to complain first.
[PATCH v3 7/7] crypto: aes - allow generic AES to be replaced by fixed time AES
On systems where a small memory footprint is important, the generic AES code with its 16 KB of lookup tables and fully unrolled encrypt and decrypt routines may be an unnecessary burden, especially given that modern SoCs often have dedicated instructions for AES. And even if they don't, a time invariant implementation may be preferred over a fast one that may be susceptible to cache timing attacks. So allow the declared dependency of other subsystems on AES to be fulfilled by either the generic table based AES or by the much smaller generic time invariant implementation. Signed-off-by: Ard Biesheuvel --- crypto/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crypto/Kconfig b/crypto/Kconfig index 87d9e03dcb74..dd0bc0d84789 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -899,7 +899,8 @@ config CRYPTO_AES_CORE config CRYPTO_AES tristate - select CRYPTO_AES_GENERIC + select CRYPTO_AES_GENERIC if (CRYPTO_AES=y && CRYPTO_AES_TI != y) || \ +(CRYPTO_AES=m && !CRYPTO_AES_TI) config CRYPTO_AES_GENERIC tristate "Generic table based AES cipher" -- 2.7.4
[PATCH v3 5/7] crypto: aes - repurpose CRYPTO_AES and introduce CRYPTO_AES_GENERIC
Repurpose the Kconfig symbol CRYPTO_AES to signify that a 'select' or 'depends on' relationship on it can be satisfied by any driver that exposes a generic "aes" cipher. The existing generic AES code is now controlled by a new Kconfig symbol CRYPTO_AES_GENERIC, and only dependencies on CRYPTO_AES that truly depend on its exported lookup tables are updated accordingly. Signed-off-by: Ard Biesheuvel --- arch/arm/crypto/Kconfig | 2 +- arch/arm64/crypto/Kconfig | 2 +- crypto/Kconfig| 8 ++-- crypto/Makefile | 2 +- net/sunrpc/Kconfig| 3 ++- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig index fd77aebcb7a9..3a6994ada2d1 100644 --- a/arch/arm/crypto/Kconfig +++ b/arch/arm/crypto/Kconfig @@ -64,7 +64,7 @@ config CRYPTO_SHA512_ARM config CRYPTO_AES_ARM tristate "Scalar AES cipher for ARM" select CRYPTO_ALGAPI - select CRYPTO_AES + select CRYPTO_AES_GENERIC help Use optimized AES assembler routines for ARM platforms. diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig index db55e069c17b..7ffe88267943 100644 --- a/arch/arm64/crypto/Kconfig +++ b/arch/arm64/crypto/Kconfig @@ -43,7 +43,7 @@ config CRYPTO_CRC32_ARM64_CE config CRYPTO_AES_ARM64 tristate "AES core cipher using scalar instructions" - select CRYPTO_AES + select CRYPTO_AES_GENERIC config CRYPTO_AES_ARM64_CE tristate "AES core cipher using ARMv8 Crypto Extensions" diff --git a/crypto/Kconfig b/crypto/Kconfig index 1e6e021fda10..9ae3dade4b2b 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -898,6 +898,10 @@ config CRYPTO_AES_CORE tristate config CRYPTO_AES + tristate + select CRYPTO_AES_GENERIC + +config CRYPTO_AES_GENERIC tristate "AES cipher algorithms" select CRYPTO_ALGAPI select CRYPTO_AES_CORE @@ -940,7 +944,7 @@ config CRYPTO_AES_586 tristate "AES cipher algorithms (i586)" depends on (X86 || UML_X86) && !64BIT select CRYPTO_ALGAPI - select CRYPTO_AES + select CRYPTO_AES_GENERIC help AES cipher algorithms (FIPS-197). AES uses the Rijndael algorithm. @@ -962,7 +966,7 @@ config CRYPTO_AES_X86_64 tristate "AES cipher algorithms (x86_64)" depends on (X86 || UML_X86) && 64BIT select CRYPTO_ALGAPI - select CRYPTO_AES + select CRYPTO_AES_GENERIC help AES cipher algorithms (FIPS-197). AES uses the Rijndael algorithm. diff --git a/crypto/Makefile b/crypto/Makefile index 0979ca461ddb..73395307bcea 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -97,7 +97,7 @@ obj-$(CONFIG_CRYPTO_TWOFISH_COMMON) += twofish_common.o obj-$(CONFIG_CRYPTO_SERPENT) += serpent_generic.o CFLAGS_serpent_generic.o := $(call cc-option,-fsched-pressure) # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79149 obj-$(CONFIG_CRYPTO_AES_CORE) += aes_core.o -obj-$(CONFIG_CRYPTO_AES) += aes_generic.o +obj-$(CONFIG_CRYPTO_AES_GENERIC) += aes_generic.o obj-$(CONFIG_CRYPTO_AES_TI) += aes_ti.o obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia_generic.o obj-$(CONFIG_CRYPTO_CAST_COMMON) += cast_common.o diff --git a/net/sunrpc/Kconfig b/net/sunrpc/Kconfig index ac09ca803296..58aa2ada40b3 100644 --- a/net/sunrpc/Kconfig +++ b/net/sunrpc/Kconfig @@ -19,7 +19,8 @@ config RPCSEC_GSS_KRB5 tristate "Secure RPC: Kerberos V mechanism" depends on SUNRPC && CRYPTO depends on CRYPTO_MD5 && CRYPTO_DES && CRYPTO_CBC && CRYPTO_CTS - depends on CRYPTO_ECB && CRYPTO_HMAC && CRYPTO_SHA1 && CRYPTO_AES + depends on CRYPTO_ECB && CRYPTO_HMAC && CRYPTO_SHA1 + select CRYPTO_AES depends on CRYPTO_ARC4 default y select SUNRPC_GSS -- 2.7.4
[PATCH v3 4/7] crypto: arm64/aes-neon - reuse Sboxes from AES core module
The newly introduced AES core module exposes its Sboxes for the benefit of the fixed time AES driver. Since the arm64 NEON based implementation already depends on the same core module for its key expansion routines, let's use its Sboxes as well, and remove the local copy. Signed-off-by: Ard Biesheuvel --- arch/arm64/crypto/aes-neon.S | 74 +--- 1 file changed, 3 insertions(+), 71 deletions(-) diff --git a/arch/arm64/crypto/aes-neon.S b/arch/arm64/crypto/aes-neon.S index f1e3aa2732f9..2acb5f81dcdb 100644 --- a/arch/arm64/crypto/aes-neon.S +++ b/arch/arm64/crypto/aes-neon.S @@ -32,7 +32,7 @@ /* preload the entire Sbox */ .macro prepare, sbox, shiftrows, temp - adr \temp, \sbox + adr_l \temp, \sbox moviv12.16b, #0x1b ldr q13, \shiftrows ldr q14, .Lror32by8 @@ -44,7 +44,7 @@ /* do preload for encryption */ .macro enc_prepare, ignore0, ignore1, temp - prepare .LForward_Sbox, .LForward_ShiftRows, \temp + prepare crypto_aes_sbox, .LForward_ShiftRows, \temp .endm .macro enc_switch_key, ignore0, ignore1, temp @@ -53,7 +53,7 @@ /* do preload for decryption */ .macro dec_prepare, ignore0, ignore1, temp - prepare .LReverse_Sbox, .LReverse_ShiftRows, \temp + prepare crypto_aes_inv_sbox, .LReverse_ShiftRows, \temp .endm /* apply SubBytes transformation using the the preloaded Sbox */ @@ -274,74 +274,6 @@ .text .align 6 -.LForward_Sbox: - .byte 0x63, 0x7c, 0x77, 0x7b, 0xf2, 0x6b, 0x6f, 0xc5 - .byte 0x30, 0x01, 0x67, 0x2b, 0xfe, 0xd7, 0xab, 0x76 - .byte 0xca, 0x82, 0xc9, 0x7d, 0xfa, 0x59, 0x47, 0xf0 - .byte 0xad, 0xd4, 0xa2, 0xaf, 0x9c, 0xa4, 0x72, 0xc0 - .byte 0xb7, 0xfd, 0x93, 0x26, 0x36, 0x3f, 0xf7, 0xcc - .byte 0x34, 0xa5, 0xe5, 0xf1, 0x71, 0xd8, 0x31, 0x15 - .byte 0x04, 0xc7, 0x23, 0xc3, 0x18, 0x96, 0x05, 0x9a - .byte 0x07, 0x12, 0x80, 0xe2, 0xeb, 0x27, 0xb2, 0x75 - .byte 0x09, 0x83, 0x2c, 0x1a, 0x1b, 0x6e, 0x5a, 0xa0 - .byte 0x52, 0x3b, 0xd6, 0xb3, 0x29, 0xe3, 0x2f, 0x84 - .byte 0x53, 0xd1, 0x00, 0xed, 0x20, 0xfc, 0xb1, 0x5b - .byte 0x6a, 0xcb, 0xbe, 0x39, 0x4a, 0x4c, 0x58, 0xcf - .byte 0xd0, 0xef, 0xaa, 0xfb, 0x43, 0x4d, 0x33, 0x85 - .byte 0x45, 0xf9, 0x02, 0x7f, 0x50, 0x3c, 0x9f, 0xa8 - .byte 0x51, 0xa3, 0x40, 0x8f, 0x92, 0x9d, 0x38, 0xf5 - .byte 0xbc, 0xb6, 0xda, 0x21, 0x10, 0xff, 0xf3, 0xd2 - .byte 0xcd, 0x0c, 0x13, 0xec, 0x5f, 0x97, 0x44, 0x17 - .byte 0xc4, 0xa7, 0x7e, 0x3d, 0x64, 0x5d, 0x19, 0x73 - .byte 0x60, 0x81, 0x4f, 0xdc, 0x22, 0x2a, 0x90, 0x88 - .byte 0x46, 0xee, 0xb8, 0x14, 0xde, 0x5e, 0x0b, 0xdb - .byte 0xe0, 0x32, 0x3a, 0x0a, 0x49, 0x06, 0x24, 0x5c - .byte 0xc2, 0xd3, 0xac, 0x62, 0x91, 0x95, 0xe4, 0x79 - .byte 0xe7, 0xc8, 0x37, 0x6d, 0x8d, 0xd5, 0x4e, 0xa9 - .byte 0x6c, 0x56, 0xf4, 0xea, 0x65, 0x7a, 0xae, 0x08 - .byte 0xba, 0x78, 0x25, 0x2e, 0x1c, 0xa6, 0xb4, 0xc6 - .byte 0xe8, 0xdd, 0x74, 0x1f, 0x4b, 0xbd, 0x8b, 0x8a - .byte 0x70, 0x3e, 0xb5, 0x66, 0x48, 0x03, 0xf6, 0x0e - .byte 0x61, 0x35, 0x57, 0xb9, 0x86, 0xc1, 0x1d, 0x9e - .byte 0xe1, 0xf8, 0x98, 0x11, 0x69, 0xd9, 0x8e, 0x94 - .byte 0x9b, 0x1e, 0x87, 0xe9, 0xce, 0x55, 0x28, 0xdf - .byte 0x8c, 0xa1, 0x89, 0x0d, 0xbf, 0xe6, 0x42, 0x68 - .byte 0x41, 0x99, 0x2d, 0x0f, 0xb0, 0x54, 0xbb, 0x16 - -.LReverse_Sbox: - .byte 0x52, 0x09, 0x6a, 0xd5, 0x30, 0x36, 0xa5, 0x38 - .byte 0xbf, 0x40, 0xa3, 0x9e, 0x81, 0xf3, 0xd7, 0xfb - .byte 0x7c, 0xe3, 0x39, 0x82, 0x9b, 0x2f, 0xff, 0x87 - .byte 0x34, 0x8e, 0x43, 0x44, 0xc4, 0xde, 0xe9, 0xcb - .byte 0x54, 0x7b, 0x94, 0x32, 0xa6, 0xc2, 0x23, 0x3d - .byte 0xee, 0x4c, 0x95, 0x0b, 0x42, 0xfa, 0xc3, 0x4e - .byte 0x08, 0x2e, 0xa1, 0x66, 0x28, 0xd9, 0x24, 0xb2 - .byte 0x76, 0x5b, 0xa2, 0x49, 0x6d, 0x8b, 0xd1, 0x25 - .byte 0x72, 0xf8, 0xf6, 0x64, 0x86, 0x68, 0x98, 0x16 - .byte 0xd4, 0xa4, 0x5c, 0xcc, 0x5d, 0x65, 0xb6, 0x92 - .byte 0x6c, 0x70, 0x48, 0x50, 0xfd, 0xed, 0xb9, 0xda - .byte 0x5e, 0x15, 0x46, 0x57, 0xa7, 0x8d, 0x9d, 0x84 - .byte 0x90, 0xd8, 0xab, 0x00, 0x8c, 0xbc, 0xd3, 0x0a - .byte 0xf7, 0xe4, 0x58, 0x05, 0xb8, 0xb3, 0x45, 0x06 - .byte 0xd0, 0x2c, 0x1e, 0x8f,
[PATCH v3 6/7] crypto: aes - add meaningful help text to the various AES drivers
Remove the duplicated boilerplate help text and add a bit of explanation about the nature of the various AES implementations that exist for various architectures. In particular, highlight the time variant nature of some implementations, and the fact that they can be omitted if required. Signed-off-by: Ard Biesheuvel --- arch/arm/crypto/Kconfig | 16 ++- arch/arm64/crypto/Kconfig | 30 +++- crypto/Kconfig| 144 +++- 3 files changed, 92 insertions(+), 98 deletions(-) diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig index 3a6994ada2d1..d8f3336bfc88 100644 --- a/arch/arm/crypto/Kconfig +++ b/arch/arm/crypto/Kconfig @@ -62,11 +62,23 @@ config CRYPTO_SHA512_ARM using optimized ARM assembler and NEON, when available. config CRYPTO_AES_ARM - tristate "Scalar AES cipher for ARM" + tristate "Table based AES cipher for 32-bit ARM" select CRYPTO_ALGAPI select CRYPTO_AES_GENERIC help - Use optimized AES assembler routines for ARM platforms. + Table based implementation in 32-bit ARM assembler of the FIPS-197 + Advanced Encryption Standard (AES) symmetric cipher algorithm. This + driver reuses the tables exposed by the generic AES driver. + + For CPUs that lack the special ARMv8-CE instructions, this is the + fastest implementation available of the core cipher, but it may be + susceptible to known-plaintext attacks on the key due to the + correlation between the processing time and the input of the first + round. Therefore, it is recommended to also enable the time invariant + NEON based driver below (CRYPTO_AES_ARM_BS), which will supersede + this driver on NEON capable CPUs when using AES in CBC, CTR and XTS + modes. If time invariance is a requirement, this driver should not + be enabled. config CRYPTO_AES_ARM_BS tristate "Bit sliced AES using NEON instructions" diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig index 7ffe88267943..4fb3e519b43f 100644 --- a/arch/arm64/crypto/Kconfig +++ b/arch/arm64/crypto/Kconfig @@ -42,13 +42,37 @@ config CRYPTO_CRC32_ARM64_CE select CRYPTO_HASH config CRYPTO_AES_ARM64 - tristate "AES core cipher using scalar instructions" + tristate "Table based AES cipher for 64-bit ARM" select CRYPTO_AES_GENERIC + help + Table based implementation in 64-bit ARM assembler of the FIPS-197 + Advanced Encryption Standard (AES) symmetric cipher algorithm. This + driver reuses the tables exposed by the generic AES driver. + + For CPUs that lack the special ARMv8-CE instructions, this is the + fastest implementation available of the core cipher, but it may be + susceptible to known-plaintext attacks on the key due to the + correlation between the processing time and the input of the first + round. Therefore, it is recommended to also enable the time invariant + drivers below (CRYPTO_AES_ARM64_NEON_BLK and CRYPTO_AES_ARM64_BS), + which will supersede this driver when using AES in the specific modes + that they implement. If time invariance is a requirement, this driver + should not be enabled. config CRYPTO_AES_ARM64_CE - tristate "AES core cipher using ARMv8 Crypto Extensions" - depends on ARM64 && KERNEL_MODE_NEON + tristate "AES cipher using ARMv8 Crypto Extensions" + depends on KERNEL_MODE_NEON select CRYPTO_ALGAPI + help + Implementation in assembler of the FIPS-197 Advanced Encryption + Standard (AES) symmetric cipher algorithm, using instructions from + ARM's optional ARMv8 Crypto Extensions. This implementation is time + invariant, and is by far the preferred option for CPUs that support + this extension. + + If in doubt, enable as a module: it will be loaded automatically on + CPUs that support it, and supersede other implementations of the AES + cipher. config CRYPTO_AES_ARM64_CE_CCM tristate "AES in CCM mode using ARMv8 Crypto Extensions" diff --git a/crypto/Kconfig b/crypto/Kconfig index 9ae3dade4b2b..87d9e03dcb74 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -902,37 +902,31 @@ config CRYPTO_AES select CRYPTO_AES_GENERIC config CRYPTO_AES_GENERIC - tristate "AES cipher algorithms" + tristate "Generic table based AES cipher" select CRYPTO_ALGAPI select CRYPTO_AES_CORE help - AES cipher algorithms (FIPS-197). AES uses the Rijndael - algorithm. - - Rijndael appears to be consistently a very good performer in - both hardware and software across a wide range of computing - environments regardless of its use in feedback or non-feedback - modes. Its key setup time is excellent, and its key agility is - good. Rijndael's ve
[PATCH v3 3/7] crypto: x86/aes-ni - switch to generic fallback
The time invariant AES-NI implementation is SIMD based, and so it needs a fallback in case the code is called from a context where SIMD is not allowed. On x86, this is really only when executing in the context of an interrupt taken while in kernel mode, since SIMD is allowed in all other cases. There is very little code in the kernel that actually performs AES in interrupt context, and the code that does (mac80211) only does so when running on 802.11 devices that have no support for AES in hardware, and those are rare these days. So switch to the new AES core code as a fallback. It is much smaller, as well as more resistant to cache timing attacks, and removing the dependency allows us to disable the time variant drivers altogether if desired. Signed-off-by: Ard Biesheuvel --- arch/x86/crypto/aesni-intel_glue.c | 4 ++-- crypto/Kconfig | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c index 4a55cdcdc008..1734e6185800 100644 --- a/arch/x86/crypto/aesni-intel_glue.c +++ b/arch/x86/crypto/aesni-intel_glue.c @@ -334,7 +334,7 @@ static void aes_encrypt(struct crypto_tfm *tfm, u8 *dst, const u8 *src) struct crypto_aes_ctx *ctx = aes_ctx(crypto_tfm_ctx(tfm)); if (!irq_fpu_usable()) - crypto_aes_encrypt_x86(ctx, dst, src); + crypto_aes_encrypt(ctx, dst, src); else { kernel_fpu_begin(); aesni_enc(ctx, dst, src); @@ -347,7 +347,7 @@ static void aes_decrypt(struct crypto_tfm *tfm, u8 *dst, const u8 *src) struct crypto_aes_ctx *ctx = aes_ctx(crypto_tfm_ctx(tfm)); if (!irq_fpu_usable()) - crypto_aes_decrypt_x86(ctx, dst, src); + crypto_aes_decrypt(ctx, dst, src); else { kernel_fpu_begin(); aesni_dec(ctx, dst, src); diff --git a/crypto/Kconfig b/crypto/Kconfig index b4edea2aed22..1e6e021fda10 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -984,8 +984,7 @@ config CRYPTO_AES_NI_INTEL tristate "AES cipher algorithms (AES-NI)" depends on X86 select CRYPTO_AEAD - select CRYPTO_AES_X86_64 if 64BIT - select CRYPTO_AES_586 if !64BIT + select CRYPTO_AES_CORE select CRYPTO_ALGAPI select CRYPTO_BLKCIPHER select CRYPTO_GLUE_HELPER_X86 if 64BIT -- 2.7.4
[PATCH v3 2/7] crypto: aes - refactor shared routines into separate core module
In preparation of further refactoring and cleanup of the AES code, move the implementations of crypto_aes_expand_key() and crypto_aes_set_key() into a separate module called aes_core, along with the forward Sbox and some GF(2^8) routines that these routines rely on. Also, introduce crypto_aes_[en|de]crypt() based on the fixed time code, which will be used in future patches by time invariant SIMD drivers that may need to fallback to scalar code in exceptional circumstances. These fallbacks offer a different tradeoff between time invariance and speed, but are generally more appropriate due to the smaller size and cache footprint. Signed-off-by: Ard Biesheuvel --- arch/arm/crypto/Kconfig | 2 +- arch/arm64/crypto/Kconfig | 2 +- crypto/Kconfig| 5 + crypto/Makefile | 1 + crypto/aes_core.c | 333 crypto/aes_generic.c | 178 --- crypto/aes_ti.c | 315 ++ drivers/crypto/Kconfig| 8 +- include/crypto/aes.h | 6 + 9 files changed, 376 insertions(+), 474 deletions(-) diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig index b9adedcc5b2e..fd77aebcb7a9 100644 --- a/arch/arm/crypto/Kconfig +++ b/arch/arm/crypto/Kconfig @@ -73,7 +73,7 @@ config CRYPTO_AES_ARM_BS depends on KERNEL_MODE_NEON select CRYPTO_BLKCIPHER select CRYPTO_SIMD - select CRYPTO_AES + select CRYPTO_AES_CORE help Use a faster and more secure NEON based implementation of AES in CBC, CTR and XTS modes diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig index d92293747d63..db55e069c17b 100644 --- a/arch/arm64/crypto/Kconfig +++ b/arch/arm64/crypto/Kconfig @@ -68,7 +68,7 @@ config CRYPTO_AES_ARM64_NEON_BLK tristate "AES in ECB/CBC/CTR/XTS modes using NEON instructions" depends on ARM64 && KERNEL_MODE_NEON select CRYPTO_BLKCIPHER - select CRYPTO_AES + select CRYPTO_AES_CORE select CRYPTO_SIMD config CRYPTO_CHACHA20_NEON diff --git a/crypto/Kconfig b/crypto/Kconfig index caa770e535a2..b4edea2aed22 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -894,9 +894,13 @@ config CRYPTO_GHASH_CLMUL_NI_INTEL comment "Ciphers" +config CRYPTO_AES_CORE + tristate + config CRYPTO_AES tristate "AES cipher algorithms" select CRYPTO_ALGAPI + select CRYPTO_AES_CORE help AES cipher algorithms (FIPS-197). AES uses the Rijndael algorithm. @@ -917,6 +921,7 @@ config CRYPTO_AES config CRYPTO_AES_TI tristate "Fixed time AES cipher" select CRYPTO_ALGAPI + select CRYPTO_AES_CORE help This is a generic implementation of AES that attempts to eliminate data dependent latencies as much as possible without affecting diff --git a/crypto/Makefile b/crypto/Makefile index d41f0331b085..0979ca461ddb 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -96,6 +96,7 @@ obj-$(CONFIG_CRYPTO_TWOFISH) += twofish_generic.o obj-$(CONFIG_CRYPTO_TWOFISH_COMMON) += twofish_common.o obj-$(CONFIG_CRYPTO_SERPENT) += serpent_generic.o CFLAGS_serpent_generic.o := $(call cc-option,-fsched-pressure) # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79149 +obj-$(CONFIG_CRYPTO_AES_CORE) += aes_core.o obj-$(CONFIG_CRYPTO_AES) += aes_generic.o obj-$(CONFIG_CRYPTO_AES_TI) += aes_ti.o obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia_generic.o diff --git a/crypto/aes_core.c b/crypto/aes_core.c new file mode 100644 index ..d3c8b5eaaf42 --- /dev/null +++ b/crypto/aes_core.c @@ -0,0 +1,333 @@ +/* + * Shared AES primitives for accelerated and generic implementations + * + * Copyright (C) 2017 Linaro Ltd + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include +#include +#include +#include + +static const u8 __cacheline_aligned aes_sbox[] = { + 0x63, 0x7c, 0x77, 0x7b, 0xf2, 0x6b, 0x6f, 0xc5, + 0x30, 0x01, 0x67, 0x2b, 0xfe, 0xd7, 0xab, 0x76, + 0xca, 0x82, 0xc9, 0x7d, 0xfa, 0x59, 0x47, 0xf0, + 0xad, 0xd4, 0xa2, 0xaf, 0x9c, 0xa4, 0x72, 0xc0, + 0xb7, 0xfd, 0x93, 0x26, 0x36, 0x3f, 0xf7, 0xcc, + 0x34, 0xa5, 0xe5, 0xf1, 0x71, 0xd8, 0x31, 0x15, + 0x04, 0xc7, 0x23, 0xc3, 0x18, 0x96, 0x05, 0x9a, + 0x07, 0x12, 0x80, 0xe2, 0xeb, 0x27, 0xb2, 0x75, + 0x09, 0x83, 0x2c, 0x1a, 0x1b, 0x6e, 0x5a, 0xa0, + 0x52, 0x3b, 0xd6, 0xb3, 0x29, 0xe3, 0x2f, 0x84, + 0x53, 0xd1, 0x00, 0xed, 0x20, 0xfc, 0xb1, 0x5b, + 0x6a, 0xcb, 0xbe, 0x39, 0x4a, 0x4c, 0x58, 0xcf, + 0xd0, 0xef, 0xaa, 0xfb, 0x43, 0x4d, 0x33, 0x85, + 0x45, 0xf9, 0x02, 0x7f, 0x50, 0x3c, 0x9f, 0xa8, + 0x51, 0xa3, 0x40, 0x8f, 0x92, 0x9d, 0x38, 0xf5, + 0xbc, 0xb6, 0xda, 0x21, 0x10, 0xff, 0xf3, 0xd2, + 0xcd, 0x0c, 0x13, 0xec, 0x5f, 0x97, 0x44, 0x17, + 0xc4, 0x
[PATCH v3 1/7] drivers/crypto/Kconfig: drop bogus CRYPTO_AES dependencies
In preparation of fine tuning the dependency relations between the accelerated AES drivers and the core support code, let's remove the dependency declarations that are false. None of these modules have link time dependencies on the generic AES code, nor do they declare any AES algos with CRYPTO_ALG_NEED_FALLBACK, so they can function perfectly fine without crypto/aes_generic.o loaded. Signed-off-by: Ard Biesheuvel --- drivers/crypto/Kconfig | 5 - 1 file changed, 5 deletions(-) diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index 0528a62a39a6..7a737c1c669e 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -419,7 +419,6 @@ config CRYPTO_DEV_S5P tristate "Support for Samsung S5PV210/Exynos crypto accelerator" depends on ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST depends on HAS_IOMEM && HAS_DMA - select CRYPTO_AES select CRYPTO_BLKCIPHER help This option allows you to have support for S5P crypto acceleration. @@ -473,7 +472,6 @@ config CRYPTO_DEV_ATMEL_AES tristate "Support for Atmel AES hw accelerator" depends on HAS_DMA depends on ARCH_AT91 || COMPILE_TEST - select CRYPTO_AES select CRYPTO_AEAD select CRYPTO_BLKCIPHER help @@ -591,7 +589,6 @@ config CRYPTO_DEV_SUN4I_SS depends on ARCH_SUNXI && !64BIT select CRYPTO_MD5 select CRYPTO_SHA1 - select CRYPTO_AES select CRYPTO_DES select CRYPTO_BLKCIPHER help @@ -606,7 +603,6 @@ config CRYPTO_DEV_SUN4I_SS config CRYPTO_DEV_ROCKCHIP tristate "Rockchip's Cryptographic Engine driver" depends on OF && ARCH_ROCKCHIP - select CRYPTO_AES select CRYPTO_DES select CRYPTO_MD5 select CRYPTO_SHA1 @@ -622,7 +618,6 @@ config CRYPTO_DEV_MEDIATEK tristate "MediaTek's EIP97 Cryptographic Engine driver" depends on HAS_DMA depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST - select CRYPTO_AES select CRYPTO_AEAD select CRYPTO_BLKCIPHER select CRYPTO_CTR -- 2.7.4
[PATCH v3 0/7] crypto: aes - allow generic AES to be omitted
The generic AES driver uses 16 lookup tables of 1 KB each, and has encryption and decryption routines that are fully unrolled. Given how the dependencies between this code and other drivers are declared in Kconfig files, this code is always pulled into the core kernel, even if it is usually superseded at runtime by accelerated drivers that exist for many architectures. This leaves us with 25 KB of dead code in the kernel, which is negligible in typical environments, but which is actually a big deal for the IoT domain, where every kilobyte counts. Also, the scalar, table based AES routines that exist for ARM, arm64, i586 and x86_64 share the lookup tables with AES generic, and may be invoked occasionally when the time-invariant AES-NI or other special instruction drivers are called in interrupt context, at which time the SIMD register file cannot be used. Pulling 16 KB of code and 9 KB of instructions into the L1s (and evicting what was already there) when a softirq happens to be handled in the context of an interrupt taken from kernel mode (which means no SIMD on x86) is also something that we may like to avoid, by falling back to a much smaller and moderately less performant driver. (Note that arm64 will be updated shortly to supply fallbacks for all SIMD based AES implementations, which will be based on the core routines [if they are accepted].) For the reasons above, this series refactors the way the various AES implementations are wired up, to allow the generic version in crypto/aes_generic.c to be omitted from the build entirely. Patch #1 removes some bogus 'select CRYPTO_AES' statement. Patch #2 introduces CRYPTO_AES_CORE and its implementation crypto/aes_core.c, which contains the existing key expansion routines, and default encrypt and decrypt routines that are not exposed as a crypto_cipher themselves, but can be pulled in by other AES drivers. These routines only depend on the two 256 byte Sboxes Patch #3 switches the fallback in the AES-NI code to the new, generic encrypt and decrypt routines so it no longer depends on the x86 scalar code or [transitively] on AES-generic. Patch #4 removes the local copy of the AES sboxes from the arm64 NEON driver, and switches to the ones exposed by the new AES core module instead. Patch #5 repurposes the CRYPTO_AES Kconfig symbol as an abstract symbol that indicates whether some implementation of AES needs to be available. The existing generic code is now controlled by CRYPTO_AES_GENERIC. Patch #6 updates the Kconfig help text to be more descriptive of what they actually control, rather than duplicating AES's wikipedia entry a number of times. Patch #7 updates the Kconfig logic so CRYPTO_AES_GENERIC can be disabled if any CRYPTO_AES dependencies are satisfied by the fixed time driver. v3: - fix big-endian issue in refactored fixed-time AES driver - improve Kconfig help texts - add patch #4 v2: - repurpose CRYPTO_AES and avoid HAVE_AES/NEED_AES Kconfig symbols - don't factor out tables from AES generic to be reused by per arch drivers, since the space saving is moderate (the generic code only), and the drivers weren't made to be small anyway Ard Biesheuvel (7): drivers/crypto/Kconfig: drop bogus CRYPTO_AES dependencies crypto: aes - refactor shared routines into separate core module crypto: x86/aes-ni - switch to generic fallback crypto: arm64/aes-neon - reuse Sboxes from AES core module crypto: aes - repurpose CRYPTO_AES and introduce CRYPTO_AES_GENERIC crypto: aes - add meaningful help text to the various AES drivers crypto: aes - allow generic AES to be replaced by fixed time AES arch/arm/crypto/Kconfig| 20 +- arch/arm64/crypto/Kconfig | 34 +- arch/arm64/crypto/aes-neon.S | 74 + arch/x86/crypto/aesni-intel_glue.c | 4 +- crypto/Kconfig | 161 -- crypto/Makefile| 3 +- crypto/aes_core.c | 333 crypto/aes_generic.c | 178 --- crypto/aes_ti.c| 315 ++ drivers/crypto/Kconfig | 13 +- include/crypto/aes.h | 6 + net/sunrpc/Kconfig | 3 +- 12 files changed, 486 insertions(+), 658 deletions(-) create mode 100644 crypto/aes_core.c -- 2.7.4
[PATCH] crypto: sun4i-ss: support the Security System PRNG
The Security System have a PRNG, this patch add support for it via crypto_rng. Signed-off-by: Corentin Labbe --- drivers/crypto/Kconfig | 8 + drivers/crypto/sunxi-ss/Makefile| 1 + drivers/crypto/sunxi-ss/sun4i-ss-core.c | 30 ++ drivers/crypto/sunxi-ss/sun4i-ss-prng.c | 56 + drivers/crypto/sunxi-ss/sun4i-ss.h | 9 ++ 5 files changed, 104 insertions(+) create mode 100644 drivers/crypto/sunxi-ss/sun4i-ss-prng.c diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index ab82536d64e2..bde0b102eb70 100644 --- a/drivers/crypto/Kconfig +++ b/drivers/crypto/Kconfig @@ -618,6 +618,14 @@ config CRYPTO_DEV_SUN4I_SS To compile this driver as a module, choose M here: the module will be called sun4i-ss. +config CRYPTO_DEV_SUN4I_SS_PRNG + bool "Support for Allwinner Security System PRNG" + depends on CRYPTO_DEV_SUN4I_SS + select CRYPTO_RNG + help + Select this option if you to provides kernel-side support for + the Pseudo-Random Number Generator found in the Security System. + config CRYPTO_DEV_ROCKCHIP tristate "Rockchip's Cryptographic Engine driver" depends on OF && ARCH_ROCKCHIP diff --git a/drivers/crypto/sunxi-ss/Makefile b/drivers/crypto/sunxi-ss/Makefile index 8f4c7a273141..ccb893219079 100644 --- a/drivers/crypto/sunxi-ss/Makefile +++ b/drivers/crypto/sunxi-ss/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sun4i-ss.o sun4i-ss-y += sun4i-ss-core.o sun4i-ss-hash.o sun4i-ss-cipher.o +sun4i-ss-$(CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG) += sun4i-ss-prng.o diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c index 02ad8256e900..d6bb2991c000 100644 --- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c +++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c @@ -213,6 +213,23 @@ static struct sun4i_ss_alg_template ss_algs[] = { } } }, +#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG +{ + .type = CRYPTO_ALG_TYPE_RNG, + .alg.rng = { + .base = { + .cra_name = "stdrng", + .cra_driver_name= "sun4i_ss_rng", + .cra_priority = 300, + .cra_ctxsize= 0, + .cra_module = THIS_MODULE, + }, + .generate = sun4i_ss_prng_generate, + .seed = sun4i_ss_prng_seed, + .seedsize = SS_SEED_LEN, + } +}, +#endif }; static int sun4i_ss_probe(struct platform_device *pdev) @@ -355,6 +372,13 @@ static int sun4i_ss_probe(struct platform_device *pdev) goto error_alg; } break; + case CRYPTO_ALG_TYPE_RNG: + err = crypto_register_rng(&ss_algs[i].alg.rng); + if (err) { + dev_err(ss->dev, "Fail to register %s\n", + ss_algs[i].alg.rng.base.cra_name); + } + break; } } platform_set_drvdata(pdev, ss); @@ -369,6 +393,9 @@ static int sun4i_ss_probe(struct platform_device *pdev) case CRYPTO_ALG_TYPE_AHASH: crypto_unregister_ahash(&ss_algs[i].alg.hash); break; + case CRYPTO_ALG_TYPE_RNG: + crypto_unregister_rng(&ss_algs[i].alg.rng); + break; } } if (ss->reset) @@ -393,6 +420,9 @@ static int sun4i_ss_remove(struct platform_device *pdev) case CRYPTO_ALG_TYPE_AHASH: crypto_unregister_ahash(&ss_algs[i].alg.hash); break; + case CRYPTO_ALG_TYPE_RNG: + crypto_unregister_rng(&ss_algs[i].alg.rng); + break; } } diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c new file mode 100644 index ..3941587def6b --- /dev/null +++ b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c @@ -0,0 +1,56 @@ +#include "sun4i-ss.h" + +int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed, + unsigned int slen) +{ + struct sun4i_ss_alg_template *algt; + struct rng_alg *alg = crypto_rng_alg(tfm); + + algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng); + memcpy(algt->ss->seed, seed, slen); + + return 0; +} + +int sun4i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src, + unsigned int slen, u8 *dst, unsigned int dlen) +{ + struct sun4i_ss_alg_template *algt; + struct rng_alg *alg = crypto_rng_alg(tfm); + int i; + u32 v; + u32 *
Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 10:33 AM, Jeffrey Walton wrote: > I think it is a bad idea to suppress all messages from a security > engineering point of view. > > Many folks don't run debug kernels. Most of the users who want or need > to know of the issues won't realize its happening. Consider, the > reason we learned of systemd's problems was due to dmesg's. > > Suppressing all messages for all configurations cast a wider net than > necessary. Configurations that could potentially be detected and fixed > likely will go unnoticed. If the problem is not brought to light, then > it won't be fixed. I more or less agree with you that we should just turn this on for all users and they'll just have to live with the spam and report odd entries, and overtime we'll fix all the violations. But I think there's another camp that would mutiny in the face of this kind of hubris. That's why I moved pretty readily toward the compromise position of default y, but depends on DEBUG_KERNEL. My hope was that it'd to an extent satisfy both camps, and also disappoint both camps in an equal way.
Re: [PATCH] hwrng: do not warn when there are no devices
On 20 June 2017 at 00:33, Mike Frysinger wrote: > On Mon, Jun 19, 2017 at 2:43 AM, PrasannaKumar Muralidharan wrote: >> On 19 June 2017 at 11:51, Herbert Xu wrote: >>> On Sun, Jun 18, 2017 at 10:00:17PM -0700, Mike Frysinger wrote: in order to make tpm-rng react in the way you're implying, the TPM subsystem would need to add a notification chain for transitions from none<->some devices, then tpm-rng could subscribe to that, and during those transition points, it would call hwrng_register/hwrng_unregister to make itself visible accordingly to the hwrng subsystem. maybe someone on the TPM side would be interested in writing all that logic, but it sounds excessive for this minor usage. the current tpm-rng driver is *extremely* simple -- it's 3 funcs, each of which are 1 line. >>> >>> It's simple and it's broken, as far as the way it hooks into the >>> hwrng is concerned. >> >> * >> diff --git a/drivers/char/hw_random/tpm-rng.c >> b/drivers/char/hw_random/tpm-rng.c >> index d6d4482..4861b35 100644 >> --- a/drivers/char/hw_random/tpm-rng.c >> +++ b/drivers/char/hw_random/tpm-rng.c >> @@ -22,6 +22,10 @@ >> #include >> >> #define MODULE_NAME "tpm-rng" >> +#define MAX_RETRIES 30 >> + >> +static struct delayed_work check_tpm_work; >> +static int retry_count; >> >> static int tpm_rng_read(struct hwrng *rng, void *data, size_t max, bool >> wait) >> { >> @@ -33,9 +37,27 @@ static struct hwrng tpm_rng = { >> .read = tpm_rng_read, >> }; >> >> +static void check_tpm_presence(struct work_struct *work) >> +{ >> + u8 data = 0; >> + if (tpm_get_random(TPM_ANY_NUM, &data, 1) > 0) { >> + hwrng_register(&tpm_rng); >> + } else { >> + if (retry_count < MAX_RETRIES) { >> + retry_count++; >> + schedule_delayed_work(&check_tpm_work, HZ * 10); >> + } else { >> + pr_err("Could not find any TPM chip, not >> registering rng"); >> + } >> + } >> +} >> + >> static int __init rng_init(void) >> { >> - return hwrng_register(&tpm_rng); >> + INIT_DELAYED_WORK(&check_tpm_work, check_tpm_presence); >> + check_tpm_presence(NULL); >> + >> + return 0; >> } >> module_init(rng_init); >> * >> >> Why not something like this? Patch is completely untested. If this >> idea seems useful I can clean the code but would require help in >> testing. > > first, that's not how deferred device probing works in the kernel. > drivers shouldn't be doing their own sleeping. but we can ignore that > because no amount of delay/retries will work -- TPMs can come & go at > anytime via hotplugging or module loading/unloading. so the only way > to pull it off would be to do something like what i described -- > extending the tpm framework so that it can signal children to come > up/go down. If TPM can come and go then notification or callback is the correct way to handle this case. > imo, standing all of that up is over-engineering and not worth the > effort, so i'm not going to do it. but maybe you can convince some of > the TPM maintainers it's worthwhile. Okay. Thanks, PrasannaKumar
Re: [PATCH] random: silence compiler warnings and fix race
On Tue, Jun 20, 2017 at 4:14 AM, Jason A. Donenfeld wrote: >... > Specifically, I added `depends on DEBUG_KERNEL`. This means that these > useful warnings will only poke other kernel developers. This is probably > exactly what we want. If the various associated developers see a warning > coming from their particular subsystem, they'll be more motivated to > fix it. Ordinary users on distribution kernels shouldn't see the > warnings or the spam at all, since typically users aren't using > DEBUG_KERNEL. I think it is a bad idea to suppress all messages from a security engineering point of view. Many folks don't run debug kernels. Most of the users who want or need to know of the issues won't realize its happening. Consider, the reason we learned of systemd's problems was due to dmesg's. Suppressing all messages for all configurations cast a wider net than necessary. Configurations that could potentially be detected and fixed likely will go unnoticed. If the problem is not brought to light, then it won't be fixed. I feel like the kernel is making policy decisions for some organizations. For those who have hardware that is effectively unfixable, then organization has to decide what to do based on their risk adversity. They may decide to live with the risk, or they may decide to refresh the hardware. However, without information on the issue, they may not even realize they have an actionable item. Jeff
Re: [PATCH 05/11] Fix ERROR: space prohibited after that open parenthesis '('
On Tue, Jun 20, 2017 at 01:21:46PM +0800, Jhih-Ming Huang wrote: > From: Jhih-Ming Hunag > > Fixed "ERROR: space prohibited after that open parenthesis '('". > > Signed-off-by: Jhih-Ming Hunag > --- > drivers/staging/ccree/ssi_aead.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/ccree/ssi_aead.c > b/drivers/staging/ccree/ssi_aead.c > index 6bcab5a..5166874 100644 > --- a/drivers/staging/ccree/ssi_aead.c > +++ b/drivers/staging/ccree/ssi_aead.c > @@ -1375,10 +1375,10 @@ static int validate_data_size(struct ssi_aead_ctx > *ctx, > static unsigned int format_ccm_a0(u8 *pA0Buff, u32 headerSize) > { > unsigned int len = 0; > - if ( headerSize == 0 ) { > + if (headerSize == 0 ) { Remove the other space as well. I looked ahead in the series so I see that you do it later, but it should be done here. regards, dan carpenter
Re: [PATCH 02/11] Fix ERROR: spaces required around that
On Tue, Jun 20, 2017 at 01:20:59PM +0800, Jhih-Ming Huang wrote: > From: Jhih-Ming Hunag > > Fixed 'ERROR: spaces required around that' > You're breaking the patches up in a bad way. This one should be combined with the previous patch. regards, dan carpenter
Re: [PATCH 01/11] Fix coding style of driver/staging/ccree/ssi_aead.c ERROR: space required after that
Subject is wrong. It should be: [PATCH 1/11] Staging: ccree: add spaces blah blah blah On Tue, Jun 20, 2017 at 01:19:44PM +0800, Jhih-Ming Huang wrote: > From: Jhih-Ming Hunag > No need. > In this series patches, I fix all of the coding style error in > driver/staging/ccree/ssi_aead.c from 54 errors to 0 error. You could put this into the cover letter. When we put this into the final git log we don't see the series only individual patches. > > The first patch fixed 'ERROR: space required after that'. > This patch fixes ... regards, dan carpenter
Re: [PATCH] random: silence compiler warnings and fix race
Hey Ted, On Tue, Jun 20, 2017 at 02:03:44AM -0400, Theodore Ts'o wrote: > I actually had set up an earlier version of your patch for on Saturday > while I was in Beijing. (Like Linus, I'm attending the LinuxCon China > conference Monday and Tuesday.) I had even created the signed tag, > I've since respun the commit to reflect your newer patch (see the > random_for_linus_stable tag on random.git) and rebased the dev branch > on top of that. Please take a look and comment. So it looks like you've gone with 4a072c71f49. If that looks good (moving the lock, etc) to you, then great, we're done. If there are still locking objections (are there?), then we'll need to revisit. > but I didn't send the pull request to Linus because I was waiting to > see about how discussions over the locking strategy and the spammy log > messages on PowerPC was going to get resolved. > The other open issue I want to resolve before sending a pull request > this week is whether we want to change the default for > CONFIG_WARN_UNSEEDED_RANDOM so that the answer is 'n'. In the v1 of this patch many moons ago, it was just vanilla, default y, but due to the spamminess, I thought folks would revolt. So I made a change: Specifically, I added `depends on DEBUG_KERNEL`. This means that these useful warnings will only poke other kernel developers. This is probably exactly what we want. If the various associated developers see a warning coming from their particular subsystem, they'll be more motivated to fix it. Ordinary users on distribution kernels shouldn't see the warnings or the spam at all, since typically users aren't using DEBUG_KERNEL. Then, to make things _even less_ annoying to kernel developers, you added a nice patch on top to squelch repeated messages. So, I still think this current strategy is a good one, of default y, but depends on DEBUG_KERNEL. Regards, Jason
Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race
Theodore Ts'o writes: > On Mon, Jun 19, 2017 at 10:57:18PM +0200, Jason A. Donenfeld wrote: >> >> With rc6 already released and rc7 coming up, I'd really appreciate you >> stepping in here and either ACKing the above commit, or giving your >> two cents about it in case I need to roll something different. > > I actually had set up an earlier version of your patch for on Saturday > while I was in Beijing. (Like Linus, I'm attending the LinuxCon China > conference Monday and Tuesday.) I had even created the signed tag, > but I didn't send the pull request to Linus because I was waiting to > see about how discussions over the locking strategy and the spammy log > messages on PowerPC was going to get resolved. > > I've since respun the commit to reflect your newer patch (see the > random_for_linus_stable tag on random.git) and rebased the dev branch > on top of that. Please take a look and comment. > > The other open issue I want to resolve before sending a pull request > this week is whether we want to change the default for > CONFIG_WARN_UNSEEDED_RANDOM so that the answer is 'n'. Yes please. > It *is* spammy for PowerPC, because they aren't getting their CRNG *some* powerpc machines ... > initialized quickly enough, so several userspace processes are getting > fork/exec'ed with an uninitialized CRNG. That being said, it is a > valid warning because it means that the initial stack canary for the > first couple of PowerPC processes are being created without a fully > initialized CRNG, which may mean that an attacker might be able to > circumvent the stack canary on the first couple of processes. So that > could potentially be a real security issue on Power. OTOH, most Power > users aren't going to be able to do anything about the fact the stack > canaries of the system daemons started during early boot don't have > strong randomness, so perhaps we should disable the warning by > default. powerpc supports a wide range of hardware platforms, some of which are 10-15 years old, and don't have a hardware RNG. Is there anything we can do on those machines? Seems like our only option would be to block the boot while some more "entropy" builds up, but that's unlikely to be popular with users. On our newer machines (>= Power8) we have a hardware RNG which we wire up to arch_get_random_seed_long(), so on those machines the warnings would be valid, because they'd indicate a bug. So I think it should be up to arches to decide whether this is turned on via their defconfigs, and the default should be 'n' because a lot of old hardware won't be able to do anything useful with the warnings. cheers