[PATCH] dhcp6: Fix uninitialized variable build warning
by Mat Martineau
Commit 17f79c855e61 added a variable 'r' to dhcp6_client_rx_message()
that holds either a new dhcp6_state or an error (negative value). 'r'
was not set in several client states, but it appears that the unit tests
only cover cases where it is set.
It appears that the new code at the end of dhcp6_client_rx_message() is
only intended for the cases where 'r' was set, so initialize 'r' to the
current client->state and rely on the existing state check to preserve
old behavior.
---
Denis, I don't have enough background knowledge about DHCPv6 to be 100%
sure this patch does the right thing. Is my guess correct about the
intended use of the new code at the end of dhcp6_client_rx_message()?
ell/dhcp6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ell/dhcp6.c b/ell/dhcp6.c
index d41cdde..b0c6438 100644
--- a/ell/dhcp6.c
+++ b/ell/dhcp6.c
@@ -1148,7 +1148,7 @@ static void dhcp6_client_rx_message(const void *data, size_t len,
{
struct l_dhcp6_client *client = userdata;
const struct dhcp6_message *message = data;
- int r;
+ int r = client->state;
CLIENT_DEBUG("");
--
2.28.0
1 year, 2 months
[PATCH 0/2] drop RC4 support
by Ard Biesheuvel
RC4 support in libell is based on the ecb(arc4) skcipher exposed by the
Linux crypto API, which is an odd beast given that it does not
distinguish between the key and the IV, and therefore does not fit the
skcipher API very well.
Now that work is underway in the linux-crypto kernel community to
implement chained requests for skciphers, we are running into problems
with this: the state that needs to be kept between skcipher requests to
implement chaining is currently kept in the TFM object (which holds the
key) in the case of ecb(arc4), and moving this into the request object
(to match the way chaining is implemented for other skcipher modes) may
enable key reuse (and thus IV reuse in the ARC4 case), which amount to
catastrophic failure for any stream cipher.
So in order to address this in a safe and robust manner, we intend to
retire the ecb(arc4) skcipher entirely on the Linux side. This obviously
requires work in the userland side as well, hence this series.
Ard Biesheuvel (2):
tls: remove support for RC4 cipher suites
cipher: remove obsolete arc4 support
ell/cipher.c | 8 +--
ell/cipher.h | 4 +-
ell/tls-suites.c | 41 ++------------
unit/test-cipher.c | 56 --------------------
unit/test-tls.c | 3 +-
5 files changed, 11 insertions(+), 101 deletions(-)
--
2.20.1
1 year, 2 months
[PATCHv2] Fix side channel leak in l_ecc_scalar_new
by Daniel DE ALMEIDA BRAGA
A secure (constant time and memory access) memory comparaison function
is needed to handle secret data. Namely, l_ecc_scalar tends to be
instantiated with secret, and _vli_cmp leak "how much" different it is
from the prime p.
The secure memcmp is defined in ell/util.h to be available to projects
using ELL.
---
ell/ecc-private.h | 33 +++++++++++++++++++++
ell/ecc.c | 2 +-
ell/util.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 107 insertions(+), 1 deletion(-)
diff --git a/ell/ecc-private.h b/ell/ecc-private.h
index 70d9607..a700983 100644
--- a/ell/ecc-private.h
+++ b/ell/ecc-private.h
@@ -24,6 +24,7 @@
#include <stdint.h>
#include "ecc.h"
+#include "util.h"
struct l_ecc_curve;
@@ -49,6 +50,38 @@ struct l_ecc_scalar {
const struct l_ecc_curve *curve;
};
+/*
+ * Performs a secure memory comparison of two uint64_t buffers of size bytes
+ * representing an integer. Blobs are ordered in little endian. It returns
+ * a negative, zero or positif value if a < b, a == b or a > b respectively.
+ */
+static inline int l_secure_memcmp_64(const uint64_t *a, const uint64_t *b,
+ size_t size)
+{
+ uint64_t aa_64, bb_64;
+
+ int res = 0, mask;
+
+ int i = 0;
+
+ if (size) {
+ /*
+ * Arrays store blobs in LE, we will process each blob as a byte array
+ * of size 8 using l_secure_memcmp. We need to make sure to feed a BE
+ * byte array to avoid unexpected behavior on different architectures.
+ */
+ do {
+ aa_64 = L_CPU_TO_BE64(a[i]);
+ bb_64 = L_CPU_TO_BE64(b[i]);
+ mask = l_secure_memcmp(&aa_64, &bb_64, 8);
+ res = (mask & res) | mask;
+ i++;
+ } while(i != size);
+ }
+
+ return res;
+}
+
void _ecc_be2native(uint64_t *dest, const uint64_t *bytes,
unsigned int ndigits);
diff --git a/ell/ecc.c b/ell/ecc.c
index 250f7e8..c69057f 100644
--- a/ell/ecc.c
+++ b/ell/ecc.c
@@ -603,7 +603,7 @@ LIB_EXPORT struct l_ecc_scalar *l_ecc_scalar_new(
_ecc_be2native(c->c, buf, curve->ndigits);
if (!vli_is_zero_or_one(c->c, curve->ndigits) &&
- _vli_cmp(c->c, curve->n, curve->ndigits) < 0)
+ l_secure_memcmp_64(curve->n, c->c, curve->ndigits) > 0)
return c;
l_ecc_scalar_free(c);
diff --git a/ell/util.h b/ell/util.h
index 6e88505..d87ec2e 100644
--- a/ell/util.h
+++ b/ell/util.h
@@ -315,6 +315,79 @@ const char *l_util_get_debugfs_path(void);
while (__result == -1L && errno == EINTR); \
__result; }))
+/*
+ * Taken from https://github.com/chmike/cst_time_memcmp, adding a volatile to
+ * ensure the compiler does not try to optimize the constant time behavior.
+ * The code has been modified to add comments and project specific code
+ * styling.
+ * This specific piece of code is subject to the following copyright:
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2015 Christophe Meessen
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * This function performs a secure memory comparison of two buffers of size
+ * bytes, representing an integer (byte order is big endian). It returns
+ * a negative, zero or positif value if a < b, a == b or a > b respectively.
+ */
+static inline int l_secure_memcmp(const void *a, const void *b,
+ size_t size)
+{
+ const volatile uint8_t *aa = a;
+ const volatile uint8_t *bb = b;
+ int res = 0, diff, mask;
+ /*
+ * We will compare all bytes, starting with the less significant. When we
+ * find a non-zero difference, we update the result accordingly.
+ */
+ if (size > 0) {
+ do {
+ --size;
+ diff = aa[size] - bb[size];
+ /*
+ * The following couple of lines can be summarized as a constant
+ * time/memory access version of:
+ * if (diff != 0) res = diff;
+ *
+ * From the previous operation, we know that diff is in [-255, 255]
+ * The following figure show the possible value of mask, based on
+ * different cases of diff:
+ *
+ * diff | diff-1 | ~diff | ((diff-1) & ~diff) | mask
+ * -------|------------|------------|--------------------|------
+ * < 0 | 0xFFFFFFXX | 0x000000YY | 0x000000ZZ | 0
+ * == 0 | 0xFFFFFFFF | 0xFFFFFFFF | 0xFFFFFFFF | 0xF..F
+ * > 0 | 0x000000XX | 0xFFFFFFYY | 0x000000ZZ | 0
+ *
+ * Hence, the mask allows to keep res when diff == 0, and to set
+ * res to diff otherwise.
+ */
+ mask = (((diff - 1) & ~diff) >> 8);
+ res = (res & mask) | diff;
+ } while (size != 0);
+ }
+
+ return res;
+}
+
#ifdef __cplusplus
}
#endif
--
2.25.4
1 year, 2 months