From 99620550e7a33daf613ec8f4f729b93b772c9730 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 11 Jan 2024 22:55:29 +0800 Subject: [PATCH 1/6] Refactor fe_isnonzero to fe_isnonzero_vartime Signed-off-by: tison --- Cargo.toml | 2 - build.rs | 1 - crypto/curve25519/curve25519.c | 21 +++++----- crypto/mem.c | 70 ---------------------------------- include/ring-core/mem.h | 69 --------------------------------- 5 files changed, 9 insertions(+), 154 deletions(-) delete mode 100644 crypto/mem.c delete mode 100644 include/ring-core/mem.h diff --git a/Cargo.toml b/Cargo.toml index c8ac2a3978..7fc8cdd46a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,7 +94,6 @@ include = [ "crypto/limbs/limbs.c", "crypto/limbs/limbs.h", "crypto/limbs/limbs.inl", - "crypto/mem.c", "crypto/perlasm/arm-xlate.pl", "crypto/perlasm/x86asm.pl", "crypto/perlasm/x86gas.pl", @@ -113,7 +112,6 @@ include = [ "include/ring-core/asm_base.h", "include/ring-core/base.h", "include/ring-core/check.h", - "include/ring-core/mem.h", "include/ring-core/poly1305.h", "include/ring-core/target.h", "include/ring-core/type_check.h", diff --git a/build.rs b/build.rs index f7b94108b7..fd345fbf90 100644 --- a/build.rs +++ b/build.rs @@ -43,7 +43,6 @@ const RING_SRCS: &[(&[&str], &str)] = &[ (&[], "crypto/fipsmodule/ec/gfp_p384.c"), (&[], "crypto/fipsmodule/ec/p256.c"), (&[], "crypto/limbs/limbs.c"), - (&[], "crypto/mem.c"), (&[], "crypto/poly1305/poly1305.c"), (&[AARCH64, ARM, X86_64, X86], "crypto/crypto.c"), diff --git a/crypto/curve25519/curve25519.c b/crypto/curve25519/curve25519.c index c60ee58b3d..ac5a77bab9 100644 --- a/crypto/curve25519/curve25519.c +++ b/crypto/curve25519/curve25519.c @@ -19,8 +19,6 @@ // // The field functions are shared by Ed25519 and X25519 where possible. -#include - #include "internal.h" #include "../internal.h" @@ -397,14 +395,13 @@ static void fe_invert(fe *out, const fe *z) { // return 0 if f == 0 // return 1 if f != 0 -static int fe_isnonzero(const fe_loose *f) { - fe tight; - fe_carry(&tight, f); - uint8_t s[32]; - fe_tobytes(s, &tight); - - static const uint8_t zero[32] = {0}; - return CRYPTO_memcmp(s, zero, sizeof(zero)) != 0; +static int fe_isnonzero_vartime(const fe_loose *f) { + for (unsigned i = 0; i < FE_NUM_LIMBS; i++) { + if (f->v[i] != 0) { + return 1; + } + } + return 0; } // return 1 if f is in {1,3,5,...,q-2} @@ -507,9 +504,9 @@ int x25519_ge_frombytes_vartime(ge_p3 *h, const uint8_t s[32]) { fe_sq_tt(&vxx, &h->X); fe_mul_ttl(&vxx, &vxx, &v); fe_sub(&check, &vxx, &u); - if (fe_isnonzero(&check)) { + if (fe_isnonzero_vartime(&check)) { fe_add(&check, &vxx, &u); - if (fe_isnonzero(&check)) { + if (fe_isnonzero_vartime(&check)) { return 0; } fe_mul_ttt(&h->X, &h->X, &sqrtm1); diff --git a/crypto/mem.c b/crypto/mem.c deleted file mode 100644 index ab4ee95120..0000000000 --- a/crypto/mem.c +++ /dev/null @@ -1,70 +0,0 @@ -/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) - * All rights reserved. - * - * This package is an SSL implementation written - * by Eric Young (eay@cryptsoft.com). - * The implementation was written so as to conform with Netscapes SSL. - * - * This library is free for commercial and non-commercial use as long as - * the following conditions are aheared to. The following conditions - * apply to all code found in this distribution, be it the RC4, RSA, - * lhash, DES, etc., code; not just the SSL code. The SSL documentation - * included with this distribution is covered by the same copyright terms - * except that the holder is Tim Hudson (tjh@cryptsoft.com). - * - * Copyright remains Eric Young's, and as such any Copyright notices in - * the code are not to be removed. - * If this package is used in a product, Eric Young should be given attribution - * as the author of the parts of the library used. - * This can be in the form of a textual message at program startup or - * in documentation (online or textual) provided with the package. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. All advertising materials mentioning features or use of this software - * must display the following acknowledgement: - * "This product includes cryptographic software written by - * Eric Young (eay@cryptsoft.com)" - * The word 'cryptographic' can be left out if the rouines from the library - * being used are not cryptographic related :-). - * 4. If you include any Windows specific code (or a derivative thereof) from - * the apps directory (application code) you must include an acknowledgement: - * "This product includes software written by Tim Hudson (tjh@cryptsoft.com)" - * - * THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - * - * The licence and distribution terms for any publically available version or - * derivative of this code cannot be changed. i.e. this code cannot simply be - * copied and put under another distribution licence - * [including the GNU Public Licence.] */ - -#include -#include "internal.h" - -int CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len) { - const aliasing_uint8_t *a = in_a; - const aliasing_uint8_t *b = in_b; - uint8_t x = 0; - - for (size_t i = 0; i < len; i++) { - x |= a[i] ^ b[i]; - } - - return x; -} diff --git a/include/ring-core/mem.h b/include/ring-core/mem.h deleted file mode 100644 index 303c447e14..0000000000 --- a/include/ring-core/mem.h +++ /dev/null @@ -1,69 +0,0 @@ -/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) - * All rights reserved. - * - * This package is an SSL implementation written - * by Eric Young (eay@cryptsoft.com). - * The implementation was written so as to conform with Netscapes SSL. - * - * This library is free for commercial and non-commercial use as long as - * the following conditions are aheared to. The following conditions - * apply to all code found in this distribution, be it the RC4, RSA, - * lhash, DES, etc., code; not just the SSL code. The SSL documentation - * included with this distribution is covered by the same copyright terms - * except that the holder is Tim Hudson (tjh@cryptsoft.com). - * - * Copyright remains Eric Young's, and as such any Copyright notices in - * the code are not to be removed. - * If this package is used in a product, Eric Young should be given attribution - * as the author of the parts of the library used. - * This can be in the form of a textual message at program startup or - * in documentation (online or textual) provided with the package. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. All advertising materials mentioning features or use of this software - * must display the following acknowledgement: - * "This product includes cryptographic software written by - * Eric Young (eay@cryptsoft.com)" - * The word 'cryptographic' can be left out if the rouines from the library - * being used are not cryptographic related :-). - * 4. If you include any Windows specific code (or a derivative thereof) from - * the apps directory (application code) you must include an acknowledgement: - * "This product includes software written by Tim Hudson (tjh@cryptsoft.com)" - * - * THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - * - * The licence and distribution terms for any publically available version or - * derivative of this code cannot be changed. i.e. this code cannot simply be - * copied and put under another distribution licence - * [including the GNU Public Licence.] */ - -#ifndef OPENSSL_HEADER_MEM_H -#define OPENSSL_HEADER_MEM_H - -#include - -// CRYPTO_memcmp returns zero iff the |len| bytes at |a| and |b| are equal. It -// takes an amount of time dependent on |len|, but independent of the contents -// of |a| and |b|. Unlike memcmp, it cannot be used to put elements into a -// defined order as the return value when a != b is undefined, other than to be -// non-zero. -OPENSSL_EXPORT int CRYPTO_memcmp(const void *a, const void *b, size_t len); - -#endif // OPENSSL_HEADER_MEM_H From eb530d195bb0411da51dd057d3eebd05fbf158d2 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 11 Jan 2024 23:14:25 +0800 Subject: [PATCH 2/6] Update verify_slices_are_equal not use CRYPTO_memcmp Signed-off-by: tison --- build.rs | 1 - src/constant_time.rs | 17 +++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/build.rs b/build.rs index fd345fbf90..4f35f19e58 100644 --- a/build.rs +++ b/build.rs @@ -884,7 +884,6 @@ fn prefix_all_symbols(pp: char, prefix_prefix: &str, prefix: &str) -> String { ]; static SYMBOLS_TO_PREFIX: &[&str] = &[ - "CRYPTO_memcmp", "CRYPTO_poly1305_finish", "CRYPTO_poly1305_finish_neon", "CRYPTO_poly1305_init", diff --git a/src/constant_time.rs b/src/constant_time.rs index 3ac6ad4c21..3bf385c667 100644 --- a/src/constant_time.rs +++ b/src/constant_time.rs @@ -14,27 +14,20 @@ //! Constant-time operations. -use crate::{c, error}; +use crate::error; /// Returns `Ok(())` if `a == b` and `Err(error::Unspecified)` otherwise. /// The comparison of `a` and `b` is done in constant time with respect to the /// contents of each, but NOT in constant time with respect to the lengths of /// `a` and `b`. pub fn verify_slices_are_equal(a: &[u8], b: &[u8]) -> Result<(), error::Unspecified> { - if a.len() != b.len() { - return Err(error::Unspecified); - } - let result = unsafe { CRYPTO_memcmp(a.as_ptr(), b.as_ptr(), a.len()) }; - match result { - 0 => Ok(()), - _ => Err(error::Unspecified), + if a != b { + Err(error::Unspecified) + } else { + Ok(()) } } -prefixed_extern! { - fn CRYPTO_memcmp(a: *const u8, b: *const u8, len: c::size_t) -> c::int; -} - #[cfg(test)] mod tests { use crate::limb::LimbMask; From 6510eb55e7aea915257d649c63fed273d50663ec Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 11 Jan 2024 23:24:29 +0800 Subject: [PATCH 3/6] fixup test Signed-off-by: tison --- crypto/curve25519/curve25519.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crypto/curve25519/curve25519.c b/crypto/curve25519/curve25519.c index ac5a77bab9..a859abec0e 100644 --- a/crypto/curve25519/curve25519.c +++ b/crypto/curve25519/curve25519.c @@ -396,8 +396,13 @@ static void fe_invert(fe *out, const fe *z) { // return 0 if f == 0 // return 1 if f != 0 static int fe_isnonzero_vartime(const fe_loose *f) { - for (unsigned i = 0; i < FE_NUM_LIMBS; i++) { - if (f->v[i] != 0) { + fe tight; + fe_carry(&tight, f); + uint8_t s[32]; + fe_tobytes(s, &tight); + + for (unsigned i = 0; i < 32; i++) { + if (s[i] != 0) { return 1; } } From b9a275c2a0de10d4935d8b079cfe9fa2324e061c Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 13 Jan 2024 18:31:01 +0800 Subject: [PATCH 4/6] revert all previous commits Signed-off-by: tison --- Cargo.toml | 2 + build.rs | 2 + crypto/curve25519/curve25519.c | 16 ++++---- crypto/mem.c | 70 ++++++++++++++++++++++++++++++++++ include/ring-core/mem.h | 69 +++++++++++++++++++++++++++++++++ src/constant_time.rs | 17 ++++++--- 6 files changed, 162 insertions(+), 14 deletions(-) create mode 100644 crypto/mem.c create mode 100644 include/ring-core/mem.h diff --git a/Cargo.toml b/Cargo.toml index 7fc8cdd46a..c8ac2a3978 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ include = [ "crypto/limbs/limbs.c", "crypto/limbs/limbs.h", "crypto/limbs/limbs.inl", + "crypto/mem.c", "crypto/perlasm/arm-xlate.pl", "crypto/perlasm/x86asm.pl", "crypto/perlasm/x86gas.pl", @@ -112,6 +113,7 @@ include = [ "include/ring-core/asm_base.h", "include/ring-core/base.h", "include/ring-core/check.h", + "include/ring-core/mem.h", "include/ring-core/poly1305.h", "include/ring-core/target.h", "include/ring-core/type_check.h", diff --git a/build.rs b/build.rs index 4f35f19e58..f7b94108b7 100644 --- a/build.rs +++ b/build.rs @@ -43,6 +43,7 @@ const RING_SRCS: &[(&[&str], &str)] = &[ (&[], "crypto/fipsmodule/ec/gfp_p384.c"), (&[], "crypto/fipsmodule/ec/p256.c"), (&[], "crypto/limbs/limbs.c"), + (&[], "crypto/mem.c"), (&[], "crypto/poly1305/poly1305.c"), (&[AARCH64, ARM, X86_64, X86], "crypto/crypto.c"), @@ -884,6 +885,7 @@ fn prefix_all_symbols(pp: char, prefix_prefix: &str, prefix: &str) -> String { ]; static SYMBOLS_TO_PREFIX: &[&str] = &[ + "CRYPTO_memcmp", "CRYPTO_poly1305_finish", "CRYPTO_poly1305_finish_neon", "CRYPTO_poly1305_init", diff --git a/crypto/curve25519/curve25519.c b/crypto/curve25519/curve25519.c index a859abec0e..c60ee58b3d 100644 --- a/crypto/curve25519/curve25519.c +++ b/crypto/curve25519/curve25519.c @@ -19,6 +19,8 @@ // // The field functions are shared by Ed25519 and X25519 where possible. +#include + #include "internal.h" #include "../internal.h" @@ -395,18 +397,14 @@ static void fe_invert(fe *out, const fe *z) { // return 0 if f == 0 // return 1 if f != 0 -static int fe_isnonzero_vartime(const fe_loose *f) { +static int fe_isnonzero(const fe_loose *f) { fe tight; fe_carry(&tight, f); uint8_t s[32]; fe_tobytes(s, &tight); - for (unsigned i = 0; i < 32; i++) { - if (s[i] != 0) { - return 1; - } - } - return 0; + static const uint8_t zero[32] = {0}; + return CRYPTO_memcmp(s, zero, sizeof(zero)) != 0; } // return 1 if f is in {1,3,5,...,q-2} @@ -509,9 +507,9 @@ int x25519_ge_frombytes_vartime(ge_p3 *h, const uint8_t s[32]) { fe_sq_tt(&vxx, &h->X); fe_mul_ttl(&vxx, &vxx, &v); fe_sub(&check, &vxx, &u); - if (fe_isnonzero_vartime(&check)) { + if (fe_isnonzero(&check)) { fe_add(&check, &vxx, &u); - if (fe_isnonzero_vartime(&check)) { + if (fe_isnonzero(&check)) { return 0; } fe_mul_ttt(&h->X, &h->X, &sqrtm1); diff --git a/crypto/mem.c b/crypto/mem.c new file mode 100644 index 0000000000..ab4ee95120 --- /dev/null +++ b/crypto/mem.c @@ -0,0 +1,70 @@ +/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) + * All rights reserved. + * + * This package is an SSL implementation written + * by Eric Young (eay@cryptsoft.com). + * The implementation was written so as to conform with Netscapes SSL. + * + * This library is free for commercial and non-commercial use as long as + * the following conditions are aheared to. The following conditions + * apply to all code found in this distribution, be it the RC4, RSA, + * lhash, DES, etc., code; not just the SSL code. The SSL documentation + * included with this distribution is covered by the same copyright terms + * except that the holder is Tim Hudson (tjh@cryptsoft.com). + * + * Copyright remains Eric Young's, and as such any Copyright notices in + * the code are not to be removed. + * If this package is used in a product, Eric Young should be given attribution + * as the author of the parts of the library used. + * This can be in the form of a textual message at program startup or + * in documentation (online or textual) provided with the package. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. All advertising materials mentioning features or use of this software + * must display the following acknowledgement: + * "This product includes cryptographic software written by + * Eric Young (eay@cryptsoft.com)" + * The word 'cryptographic' can be left out if the rouines from the library + * being used are not cryptographic related :-). + * 4. If you include any Windows specific code (or a derivative thereof) from + * the apps directory (application code) you must include an acknowledgement: + * "This product includes software written by Tim Hudson (tjh@cryptsoft.com)" + * + * THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + * + * The licence and distribution terms for any publically available version or + * derivative of this code cannot be changed. i.e. this code cannot simply be + * copied and put under another distribution licence + * [including the GNU Public Licence.] */ + +#include +#include "internal.h" + +int CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len) { + const aliasing_uint8_t *a = in_a; + const aliasing_uint8_t *b = in_b; + uint8_t x = 0; + + for (size_t i = 0; i < len; i++) { + x |= a[i] ^ b[i]; + } + + return x; +} diff --git a/include/ring-core/mem.h b/include/ring-core/mem.h new file mode 100644 index 0000000000..303c447e14 --- /dev/null +++ b/include/ring-core/mem.h @@ -0,0 +1,69 @@ +/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) + * All rights reserved. + * + * This package is an SSL implementation written + * by Eric Young (eay@cryptsoft.com). + * The implementation was written so as to conform with Netscapes SSL. + * + * This library is free for commercial and non-commercial use as long as + * the following conditions are aheared to. The following conditions + * apply to all code found in this distribution, be it the RC4, RSA, + * lhash, DES, etc., code; not just the SSL code. The SSL documentation + * included with this distribution is covered by the same copyright terms + * except that the holder is Tim Hudson (tjh@cryptsoft.com). + * + * Copyright remains Eric Young's, and as such any Copyright notices in + * the code are not to be removed. + * If this package is used in a product, Eric Young should be given attribution + * as the author of the parts of the library used. + * This can be in the form of a textual message at program startup or + * in documentation (online or textual) provided with the package. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. All advertising materials mentioning features or use of this software + * must display the following acknowledgement: + * "This product includes cryptographic software written by + * Eric Young (eay@cryptsoft.com)" + * The word 'cryptographic' can be left out if the rouines from the library + * being used are not cryptographic related :-). + * 4. If you include any Windows specific code (or a derivative thereof) from + * the apps directory (application code) you must include an acknowledgement: + * "This product includes software written by Tim Hudson (tjh@cryptsoft.com)" + * + * THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + * + * The licence and distribution terms for any publically available version or + * derivative of this code cannot be changed. i.e. this code cannot simply be + * copied and put under another distribution licence + * [including the GNU Public Licence.] */ + +#ifndef OPENSSL_HEADER_MEM_H +#define OPENSSL_HEADER_MEM_H + +#include + +// CRYPTO_memcmp returns zero iff the |len| bytes at |a| and |b| are equal. It +// takes an amount of time dependent on |len|, but independent of the contents +// of |a| and |b|. Unlike memcmp, it cannot be used to put elements into a +// defined order as the return value when a != b is undefined, other than to be +// non-zero. +OPENSSL_EXPORT int CRYPTO_memcmp(const void *a, const void *b, size_t len); + +#endif // OPENSSL_HEADER_MEM_H diff --git a/src/constant_time.rs b/src/constant_time.rs index 3bf385c667..3ac6ad4c21 100644 --- a/src/constant_time.rs +++ b/src/constant_time.rs @@ -14,20 +14,27 @@ //! Constant-time operations. -use crate::error; +use crate::{c, error}; /// Returns `Ok(())` if `a == b` and `Err(error::Unspecified)` otherwise. /// The comparison of `a` and `b` is done in constant time with respect to the /// contents of each, but NOT in constant time with respect to the lengths of /// `a` and `b`. pub fn verify_slices_are_equal(a: &[u8], b: &[u8]) -> Result<(), error::Unspecified> { - if a != b { - Err(error::Unspecified) - } else { - Ok(()) + if a.len() != b.len() { + return Err(error::Unspecified); + } + let result = unsafe { CRYPTO_memcmp(a.as_ptr(), b.as_ptr(), a.len()) }; + match result { + 0 => Ok(()), + _ => Err(error::Unspecified), } } +prefixed_extern! { + fn CRYPTO_memcmp(a: *const u8, b: *const u8, len: c::size_t) -> c::int; +} + #[cfg(test)] mod tests { use crate::limb::LimbMask; From e7143af99343ac7ecb213830db1444583a31437b Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 13 Jan 2024 18:35:01 +0800 Subject: [PATCH 5/6] address comment Signed-off-by: tison --- Cargo.toml | 1 - crypto/curve25519/curve25519.c | 11 ++++-- crypto/mem.c | 1 - include/ring-core/mem.h | 69 ---------------------------------- 4 files changed, 7 insertions(+), 75 deletions(-) delete mode 100644 include/ring-core/mem.h diff --git a/Cargo.toml b/Cargo.toml index c8ac2a3978..9707420b9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -113,7 +113,6 @@ include = [ "include/ring-core/asm_base.h", "include/ring-core/base.h", "include/ring-core/check.h", - "include/ring-core/mem.h", "include/ring-core/poly1305.h", "include/ring-core/target.h", "include/ring-core/type_check.h", diff --git a/crypto/curve25519/curve25519.c b/crypto/curve25519/curve25519.c index c60ee58b3d..b0429feb42 100644 --- a/crypto/curve25519/curve25519.c +++ b/crypto/curve25519/curve25519.c @@ -19,8 +19,6 @@ // // The field functions are shared by Ed25519 and X25519 where possible. -#include - #include "internal.h" #include "../internal.h" @@ -403,8 +401,13 @@ static int fe_isnonzero(const fe_loose *f) { uint8_t s[32]; fe_tobytes(s, &tight); - static const uint8_t zero[32] = {0}; - return CRYPTO_memcmp(s, zero, sizeof(zero)) != 0; + for (size_t i = 0; i < sizeof(s); i++) { + if (s[i] != 0) { + return 1; + } + } + + return 0; } // return 1 if f is in {1,3,5,...,q-2} diff --git a/crypto/mem.c b/crypto/mem.c index ab4ee95120..ff2b896539 100644 --- a/crypto/mem.c +++ b/crypto/mem.c @@ -54,7 +54,6 @@ * copied and put under another distribution licence * [including the GNU Public Licence.] */ -#include #include "internal.h" int CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len) { diff --git a/include/ring-core/mem.h b/include/ring-core/mem.h deleted file mode 100644 index 303c447e14..0000000000 --- a/include/ring-core/mem.h +++ /dev/null @@ -1,69 +0,0 @@ -/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) - * All rights reserved. - * - * This package is an SSL implementation written - * by Eric Young (eay@cryptsoft.com). - * The implementation was written so as to conform with Netscapes SSL. - * - * This library is free for commercial and non-commercial use as long as - * the following conditions are aheared to. The following conditions - * apply to all code found in this distribution, be it the RC4, RSA, - * lhash, DES, etc., code; not just the SSL code. The SSL documentation - * included with this distribution is covered by the same copyright terms - * except that the holder is Tim Hudson (tjh@cryptsoft.com). - * - * Copyright remains Eric Young's, and as such any Copyright notices in - * the code are not to be removed. - * If this package is used in a product, Eric Young should be given attribution - * as the author of the parts of the library used. - * This can be in the form of a textual message at program startup or - * in documentation (online or textual) provided with the package. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. All advertising materials mentioning features or use of this software - * must display the following acknowledgement: - * "This product includes cryptographic software written by - * Eric Young (eay@cryptsoft.com)" - * The word 'cryptographic' can be left out if the rouines from the library - * being used are not cryptographic related :-). - * 4. If you include any Windows specific code (or a derivative thereof) from - * the apps directory (application code) you must include an acknowledgement: - * "This product includes software written by Tim Hudson (tjh@cryptsoft.com)" - * - * THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - * - * The licence and distribution terms for any publically available version or - * derivative of this code cannot be changed. i.e. this code cannot simply be - * copied and put under another distribution licence - * [including the GNU Public Licence.] */ - -#ifndef OPENSSL_HEADER_MEM_H -#define OPENSSL_HEADER_MEM_H - -#include - -// CRYPTO_memcmp returns zero iff the |len| bytes at |a| and |b| are equal. It -// takes an amount of time dependent on |len|, but independent of the contents -// of |a| and |b|. Unlike memcmp, it cannot be used to put elements into a -// defined order as the return value when a != b is undefined, other than to be -// non-zero. -OPENSSL_EXPORT int CRYPTO_memcmp(const void *a, const void *b, size_t len); - -#endif // OPENSSL_HEADER_MEM_H From 407599ac3bf581afb93eac7face4a5831116f406 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 21 Feb 2024 22:03:38 +0800 Subject: [PATCH 6/6] fe_isnonzero_vartime is not constant-time fn Signed-off-by: tison --- crypto/curve25519/curve25519.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crypto/curve25519/curve25519.c b/crypto/curve25519/curve25519.c index b0429feb42..f68e8d5bb7 100644 --- a/crypto/curve25519/curve25519.c +++ b/crypto/curve25519/curve25519.c @@ -395,7 +395,7 @@ static void fe_invert(fe *out, const fe *z) { // return 0 if f == 0 // return 1 if f != 0 -static int fe_isnonzero(const fe_loose *f) { +static int fe_isnonzero_vartime(const fe_loose *f) { fe tight; fe_carry(&tight, f); uint8_t s[32]; @@ -510,9 +510,9 @@ int x25519_ge_frombytes_vartime(ge_p3 *h, const uint8_t s[32]) { fe_sq_tt(&vxx, &h->X); fe_mul_ttl(&vxx, &vxx, &v); fe_sub(&check, &vxx, &u); - if (fe_isnonzero(&check)) { + if (fe_isnonzero_vartime(&check)) { fe_add(&check, &vxx, &u); - if (fe_isnonzero(&check)) { + if (fe_isnonzero_vartime(&check)) { return 0; } fe_mul_ttt(&h->X, &h->X, &sqrtm1);