Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1662896
| From | Gilad Ben-Yossef <gilad@benyossef.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 3/6] ccree: use constant time memory comparison for macs and tags |
| Date | 2017-06-10 09:50 +0200 |
| Message-ID | <tQJjH-4dB-5@gated-at.bofh.it> (permalink) |
| References | <tQEN3-1mX-1@gated-at.bofh.it> <tQEN3-1mX-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
Thank you Jason,
I think what you are doing is very important.
On Sat, Jun 10, 2017 at 5:59 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Otherwise, we enable several different forgeries via timing attack.
>
> While the C inside this file is nearly incomprehensible, I did notice a
> high volume of "FIPS" and "NIST", which makes this kind of bug slightly
> more embarrassing.
>
The code you are referring to implements, as the function name states,
FIPS power up tests[*].
Specifically, this is the code that compares computed results to known
good results.
As far as I understand the purpose of timing and memory side channel
attacks is to deduce
key material by measurement of time and/or memory usage. However, this
being a FIPS power
up test, the key material is actually part of the source code, so not
much use here.
So, unless I've missed something, I'm going to NAK this one. Your
patch however did inspire me
to look in the ccree driver for other places where not using these
mechanisms is more dangerous,
so thank you for that.
[*] whose implementation inside the driver itself is questionable and
will probably go away as part
of staging clean-ups.
Thanks,
Gilad
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: Gilad Ben-Yossef <gilad@benyossef.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable@vger.kernel.org
> ---
> drivers/staging/ccree/ssi_fips_ll.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/ccree/ssi_fips_ll.c b/drivers/staging/ccree/ssi_fips_ll.c
> index d573574bbb98..3310997d8e3e 100644
> --- a/drivers/staging/ccree/ssi_fips_ll.c
> +++ b/drivers/staging/ccree/ssi_fips_ll.c
> @@ -19,6 +19,7 @@ This file defines the driver FIPS Low Level implmentaion functions,
> that executes the KAT.
> ***************************************************************/
> #include <linux/kernel.h>
> +#include <crypto/algapi.h>
>
> #include "ssi_driver.h"
> #include "ssi_fips_local.h"
> @@ -462,7 +463,7 @@ ssi_cipher_fips_power_up_tests(struct ssi_drvdata *drvdata, void *cpu_addr_buffe
> }
>
> /* compare actual dout to expected */
> - if (memcmp(virt_ctx->dout, cipherData->dataOut, cipherData->dataInSize) != 0)
> + if (crypto_memneq(virt_ctx->dout, cipherData->dataOut, cipherData->dataInSize))
> {
> FIPS_LOG("dout comparison error %d - oprMode=%d, isAes=%d\n", i, cipherData->oprMode, cipherData->isAes);
> FIPS_LOG(" i expected received \n");
> @@ -586,7 +587,7 @@ ssi_cmac_fips_power_up_tests(struct ssi_drvdata *drvdata, void *cpu_addr_buffer,
> }
>
> /* compare actual mac result to expected */
> - if (memcmp(virt_ctx->mac_res, cmac_data->mac_res, cmac_data->mac_res_size) != 0)
> + if (crypto_memneq(virt_ctx->mac_res, cmac_data->mac_res, cmac_data->mac_res_size))
> {
> FIPS_LOG("comparison error %d - digest_size=%d \n", i, cmac_data->mac_res_size);
> FIPS_LOG(" i expected received \n");
> @@ -760,7 +761,7 @@ ssi_hash_fips_power_up_tests(struct ssi_drvdata *drvdata, void *cpu_addr_buffer,
> }
>
> /* compare actual mac result to expected */
> - if (memcmp(virt_ctx->mac_res, hash_data->mac_res, digest_size) != 0)
> + if (crypto_memneq(virt_ctx->mac_res, hash_data->mac_res, digest_size))
> {
> FIPS_LOG("comparison error %d - hash_mode=%d digest_size=%d \n", i, hash_data->hash_mode, digest_size);
> FIPS_LOG(" i expected received \n");
> @@ -1093,7 +1094,7 @@ ssi_hmac_fips_power_up_tests(struct ssi_drvdata *drvdata, void *cpu_addr_buffer,
> }
>
> /* compare actual mac result to expected */
> - if (memcmp(virt_ctx->mac_res, hmac_data->mac_res, digest_size) != 0)
> + if (crypto_memneq(virt_ctx->mac_res, hmac_data->mac_res, digest_size))
> {
> FIPS_LOG("comparison error %d - hash_mode=%d digest_size=%d \n", i, hmac_data->hash_mode, digest_size);
> FIPS_LOG(" i expected received \n");
> @@ -1310,7 +1311,7 @@ ssi_ccm_fips_power_up_tests(struct ssi_drvdata *drvdata, void *cpu_addr_buffer,
> }
>
> /* compare actual dout to expected */
> - if (memcmp(virt_ctx->dout, ccmData->dataOut, ccmData->dataInSize) != 0)
> + if (crypto_memneq(virt_ctx->dout, ccmData->dataOut, ccmData->dataInSize))
> {
> FIPS_LOG("dout comparison error %d - size=%d \n", i, ccmData->dataInSize);
> error = CC_REE_FIPS_ERROR_AESCCM_PUT;
> @@ -1318,7 +1319,7 @@ ssi_ccm_fips_power_up_tests(struct ssi_drvdata *drvdata, void *cpu_addr_buffer,
> }
>
> /* compare actual mac result to expected */
> - if (memcmp(virt_ctx->mac_res, ccmData->macResOut, ccmData->tagSize) != 0)
> + if (crypto_memneq(virt_ctx->mac_res, ccmData->macResOut, ccmData->tagSize))
> {
> FIPS_LOG("mac_res comparison error %d - mac_size=%d \n", i, ccmData->tagSize);
> FIPS_LOG(" i expected received \n");
> @@ -1633,7 +1634,7 @@ ssi_gcm_fips_power_up_tests(struct ssi_drvdata *drvdata, void *cpu_addr_buffer,
>
> if (gcmData->direction == DRV_CRYPTO_DIRECTION_ENCRYPT) {
> /* compare actual dout to expected */
> - if (memcmp(virt_ctx->dout, gcmData->dataOut, gcmData->dataInSize) != 0)
> + if (crypto_memneq(virt_ctx->dout, gcmData->dataOut, gcmData->dataInSize))
> {
> FIPS_LOG("dout comparison error %d - size=%d \n", i, gcmData->dataInSize);
> FIPS_LOG(" i expected received \n");
> @@ -1649,7 +1650,7 @@ ssi_gcm_fips_power_up_tests(struct ssi_drvdata *drvdata, void *cpu_addr_buffer,
> }
>
> /* compare actual mac result to expected */
> - if (memcmp(virt_ctx->mac_res, gcmData->macResOut, gcmData->tagSize) != 0)
> + if (crypto_memneq(virt_ctx->mac_res, gcmData->macResOut, gcmData->tagSize))
> {
> FIPS_LOG("mac_res comparison error %d - mac_size=%d \n", i, gcmData->tagSize);
> FIPS_LOG(" i expected received \n");
> --
> 2.13.1
>
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 0/6] Constant Time Memory Comparisons Are Important "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-10 05:00 +0200
[PATCH 3/6] ccree: use constant time memory comparison for macs and tags "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-10 05:00 +0200
Re: [PATCH 3/6] ccree: use constant time memory comparison for macs and tags Gilad Ben-Yossef <gilad@benyossef.com> - 2017-06-10 09:50 +0200
Re: [PATCH 3/6] ccree: use constant time memory comparison for macs and tags "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-10 13:00 +0200
Re: [PATCH 3/6] ccree: use constant time memory comparison for macs and tags Henrique de Moraes Holschuh <hmh@hmh.eng.br> - 2017-06-10 23:50 +0200
[PATCH 2/6] net/ipv6: use constant time memory comparison for mac "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-10 05:10 +0200
[PATCH 4/6] security/keys: use constant time memory comparison for macs "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-10 05:10 +0200
Re: [kernel-hardening] [PATCH 4/6] security/keys: use constant time memory comparison for macs James Morris <jmorris@namei.org> - 2017-06-14 11:00 +0200
[PATCH 1/6] sunrpc: use constant time memory comparison for mac "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-10 05:10 +0200
[PATCH 5/6] bluetooth/smp: use constant time memory comparison for secret values "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-10 05:10 +0200
Re: [PATCH 5/6] bluetooth/smp: use constant time memory comparison for secret values Marcel Holtmann <marcel@holtmann.org> - 2017-06-10 15:50 +0200
[PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-10 05:10 +0200
Re: [PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs Johannes Berg <johannes@sipsolutions.net> - 2017-06-13 10:30 +0200
Re: [PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-13 15:30 +0200
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important Kalle Valo <kvalo@codeaurora.org> - 2017-06-11 10:20 +0200
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important Kees Cook <keescook@chromium.org> - 2017-06-11 15:40 +0200
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important Emmanuel Grumbach <egrumbach@gmail.com> - 2017-06-11 22:50 +0200
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important Emil Lenngren <emil.lenngren@gmail.com> - 2017-06-11 23:40 +0200
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important Emmanuel Grumbach <egrumbach@gmail.com> - 2017-06-12 07:10 +0200
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important Arend van Spriel <arend.vanspriel@broadcom.com> - 2017-06-12 09:40 +0200
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important Stephan Müller <smueller@chronox.de> - 2017-06-11 23:10 +0200
Re: [PATCH 0/6] Constant Time Memory Comparisons Are Important "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-11 23:30 +0200
[PATCH] rsa-pkcs1pad: use constant time memory comparison for MACs "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-11 23:30 +0200
csiph-web