Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for micro-ecc (WIP) #228

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[submodule "ext/micro-ecc"]
path = ext/micro-ecc
url = https://github.com/kmackay/micro-ecc.git
14 changes: 8 additions & 6 deletions Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,16 @@ HEADERS:=dtls.h hmac.h dtls_debug.h dtls_config.h uthash.h numeric.h crypto.h gl
netq.h alert.h utlist.h dtls_prng.h peer.h state.h dtls_time.h session.h \
tinydtls.h dtls_mutex.h
PKG_CONFIG_FILES:=tinydtls.pc
__micro_ecc_path:=ext/micro-ecc
MICRO_ECC_FILES:=$(addprefix $(__micro_ecc_path)/, uECC.h uECC.c uECC_vli.h types.h) $(wildcard $(__micro_ecc_path)/*.inc)
CFLAGS:=-Wall -pedantic -std=c99 -DSHA2_USE_INTTYPES_H @CFLAGS@ \
@WARNING_CFLAGS@ $(EXTRA_CFLAGS)
CPPFLAGS:=@CPPFLAGS@ -DDTLS_CHECK_CONTENTTYPE -I$(top_srcdir)
SUBDIRS:=tests tests/unit-tests doc platform-specific sha2 aes ecc
SUBDIRS:=tests tests/unit-tests doc platform-specific sha2 aes
DISTSUBDIRS:=$(SUBDIRS)
DISTDIR=$(top_builddir)/$(package)
FILES:=Makefile.in configure configure.ac dtls_config.h.in \
Makefile.tinydtls $(SOURCES) $(HEADERS)
Makefile.tinydtls tinydtls.pc.in ar-lib $(SOURCES) $(HEADERS) $(MICRO_ECC_FILES)
LIB:=libtinydtls
LIBS:=$(LIB).a
ifeq ("@ENABLE_SHARED@", "1")
Expand Down Expand Up @@ -87,9 +89,6 @@ dirs: $(SUBDIRS)

dtls_prng.o:: $(wildcard platform-specific/dtls_prng_*.c)

$(SUB_OBJECTS)::
$(MAKE) -C $(@D) $(@F)

$(LIB).so: $(OBJECTS)
$(LINK.c) $(LDFLAGS) -shared $^ -o $@

Expand All @@ -104,6 +103,9 @@ clean:
done
endif # WITH_CONTIKI

# Override several warnings for uECC.o
ext/micro-ecc/uECC.o:: CFLAGS+=-Wno-pedantic -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations

doc:
$(MAKE) -C doc

Expand All @@ -113,7 +115,7 @@ distclean: clean

dist: $(FILES) $(DISTSUBDIRS)
test -d $(DISTDIR) || mkdir $(DISTDIR)
cp $(FILES) $(DISTDIR)
cp --parents $(FILES) $(DISTDIR)
for dir in $(DISTSUBDIRS); do \
$(MAKE) -C $$dir dist; \
done
Expand Down
19 changes: 15 additions & 4 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,13 @@ AC_ARG_WITH(ecc,
[AS_HELP_STRING([--without-ecc],[disable support for TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8])],
[],
[AC_DEFINE(DTLS_ECC, 1, [Define to 1 if building with ECC support.])
OPT_OBJS="${OPT_OBJS} ecc/ecc.o"
OPT_OBJS="${OPT_OBJS} ext/micro-ecc/uECC.o"
dnl remove all ECC except for secp256r1
AC_DEFINE([uECC_SUPPORTS_secp160r1], [0], [Define to 1 if building with uECC curve secp160r1])
AC_DEFINE([uECC_SUPPORTS_secp192r1], [0], [Define to 1 if building with uECC curve secp192r1])
AC_DEFINE([uECC_SUPPORTS_secp224r1], [0], [Define to 1 if building with uECC curve secp224r1])
AC_DEFINE([uECC_SUPPORTS_secp256r1], [1], [Define to 1 if building with uECC curve secp256r1])
AC_DEFINE([uECC_SUPPORTS_secp256k1], [0], [Define to 1 if building with uECC curve secp256k1])
DTLS_ECC=1])

AC_ARG_WITH(psk,
Expand Down Expand Up @@ -116,7 +122,13 @@ AC_SUBST(NDEBUG)
AC_SUBST(DTLS_ECC)
AC_SUBST(DTLS_PSK)
AC_SUBST(ENABLE_SHARED)
AC_SUBST(AR)
AC_SUBST(AC)

AC_SUBST(uECC_SUPPORTS_secp160r1)
AC_SUBST(uECC_SUPPORTS_secp192r1)
AC_SUBST(uECC_SUPPORTS_secp224r1)
AC_SUBST(uECC_SUPPORTS_secp256r1)
AC_SUBST(uECC_SUPPORTS_secp256k1)

# Checks for header files.
AC_CHECK_HEADERS([assert.h arpa/inet.h fcntl.h inttypes.h netdb.h netinet/in.h stddef.h stdint.h stdlib.h string.h strings.h sys/param.h sys/socket.h unistd.h])
Expand Down Expand Up @@ -148,6 +160,5 @@ AC_CONFIG_FILES([Makefile
platform-specific/Makefile
tinydtls.pc
sha2/Makefile
aes/Makefile
ecc/Makefile])
aes/Makefile])
AC_OUTPUT
204 changes: 155 additions & 49 deletions crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,17 @@
#else
#define assert(x)
#endif
#include <inttypes.h>

#include "global.h"
#include "dtls_debug.h"
#include "numeric.h"
#include "dtls.h"
#include "crypto.h"
#include "ccm.h"
#include "ecc/ecc.h"
#ifdef DTLS_ECC
#include "ext/micro-ecc/uECC.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ext/micro-ecc" is part of the include path, maybe it's simpler to use "uECC.h"?

Copy link
Contributor

@mrdeep1 mrdeep1 Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is my preference to always include the package information path in the #include statement, to prevent name clashes (unlikely with uECC.h, but with possible with names like global.h etc.).

So, I would recommend that the include path includes ext/, and use #include <micro-ecc/uECC.h>.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pain is, that "micro-ecc/uECC.c" uses:

#include "uECC.h"
#include "uECC_vli.h"

and "micro-ecc/uECC_vli.h" uses:

#include "uECC.h"
#include "types.h"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything that is getting built external to micro-ecc will need the equivalent of "micro-ecc/uECC.h" to get at the function definitions. For the actual micro-ecc code compilation, I assume that it is done in the micro-ecc/ directory, in which case just "uECCH.h" without a path is fine.

If you declare #include "" , the compiler first searches in current directory of source file and if not found,
continues to search in any included -I directories and then the default include directories.

If you declare #include <> , the compiler searches directly in any included -I directories and then the default
include directories.

$ cpp -v shows the include search directory order.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I've checked it and at least the cmake build doesn't need to extend the include path with "ext/micro-ecc".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ext/micro-ecc" is part of the include path,

I mixed it up, it's only part of "FILES".

#endif /* DTLS_ECC */
#include "dtls_prng.h"
#include "netq.h"

Expand Down Expand Up @@ -356,6 +359,10 @@ dtls_psk_pre_master_secret(unsigned char *key, size_t keylen,
#endif /* DTLS_PSK */

#ifdef DTLS_ECC
#ifdef uECC_SUPPORTS_secp256r1
const dtls_ecdh_curve default_curve = TLS_EXT_ELLIPTIC_CURVES_SECP256R1;
#endif /* uECC_SUPPORTS_secp256r1 */

static void dtls_ec_key_to_uint32(const unsigned char *key, size_t key_size,
uint32_t *result) {
int i;
Expand Down Expand Up @@ -432,69 +439,148 @@ int dtls_ec_key_asn1_from_uint32(const uint32_t *key, size_t key_size,
return key_size + 2;
}

static int get_uecc_curve(dtls_ecdh_curve curve, uECC_Curve *result) {
struct {
dtls_ecdh_curve curve;
uECC_Curve uecc_curve;
} known_curves[] = {
#if uECC_SUPPORTS_secp256r1
{ TLS_EXT_ELLIPTIC_CURVES_SECP256R1, uECC_secp256r1() },
#endif /* uECC_SUPPORTS_secp256r1 */
};
unsigned int index;

for (index = 0; index < sizeof(known_curves)/sizeof(known_curves[0]); index++) {
if (known_curves[index].curve == curve) {
*result = known_curves[index].uecc_curve;
return 1;
}
}
return 0;
}

int dtls_ecdh_pre_master_secret2(const unsigned char *priv_key,
const unsigned char *pub_key,
size_t key_size,
dtls_ecdh_curve curve,
unsigned char *result,
size_t result_len) {
uECC_Curve uecc_curve;
if (!get_uecc_curve(curve, &uecc_curve)) {
dtls_warn("curve %" PRIu16 " not supported\n", curve);
return -1;
}

if (result_len < key_size) {
return -1;
}

if (!uECC_valid_public_key(pub_key, uecc_curve)) {
dtls_warn("invalid public key\n");
}

if (!uECC_shared_secret(pub_key, priv_key, result, uecc_curve)) {
dtls_warn("cannot generate ECDH shared secret\n");
return 0;
}

return key_size;
}

int dtls_ecdh_pre_master_secret(unsigned char *priv_key,
unsigned char *pub_key_x,
unsigned char *pub_key_y,
size_t key_size,
unsigned char *result,
size_t result_len) {
uint32_t priv[8];
uint32_t pub_x[8];
uint32_t pub_y[8];
uint32_t result_x[8];
uint32_t result_y[8];
const dtls_ecdh_curve curve = default_curve;
uint8_t pub_key[2 * DTLS_EC_KEY_SIZE];

assert(key_size == sizeof(priv));
if (result_len < key_size) {
return -1;
}

dtls_ec_key_to_uint32(priv_key, key_size, priv);
dtls_ec_key_to_uint32(pub_key_x, key_size, pub_x);
dtls_ec_key_to_uint32(pub_key_y, key_size, pub_y);

ecc_ecdh(pub_x, pub_y, priv, result_x, result_y);

dtls_ec_key_from_uint32(result_x, key_size, result);
return key_size;
memcpy(pub_key, pub_key_x, DTLS_EC_KEY_SIZE);
memcpy(pub_key + DTLS_EC_KEY_SIZE, pub_key_y, DTLS_EC_KEY_SIZE);
return dtls_ecdh_pre_master_secret2(priv_key, pub_key, key_size, curve,
result, result_len);
}

void
dtls_ecdsa_generate_key(unsigned char *priv_key,
unsigned char *pub_key_x,
unsigned char *pub_key_y,
size_t key_size) {
uint32_t priv[8];
uint32_t pub_x[8];
uint32_t pub_y[8];
const dtls_ecdh_curve curve = default_curve;
uint8_t pub_key[2 * DTLS_EC_KEY_SIZE];

int res = dtls_ecdsa_generate_key2(priv_key, pub_key, key_size, curve);
if (res > 0) {
memcpy(pub_key_x, pub_key, res);
memcpy(pub_key_y, pub_key + res, res);
}
}

do {
dtls_prng((unsigned char *)priv, key_size);
} while (!ecc_is_valid_key(priv));
int
dtls_ecdsa_generate_key2(unsigned char *priv_key,
unsigned char *pub_key,
size_t key_size,
dtls_ecdh_curve curve) {
uECC_Curve uecc_curve;
if (!get_uecc_curve(curve, &uecc_curve)) {
dtls_warn("curve %" PRIu16 " not supported\n", curve);
return -1;
}

ecc_gen_pub_key(priv, pub_x, pub_y);
assert(key_size >= (unsigned int)uECC_curve_private_key_size(uecc_curve));
assert(2 * key_size >= (unsigned int)uECC_curve_public_key_size(uecc_curve));

dtls_ec_key_from_uint32(priv, key_size, priv_key);
dtls_ec_key_from_uint32(pub_x, key_size, pub_key_x);
dtls_ec_key_from_uint32(pub_y, key_size, pub_key_y);
if (!uECC_make_key(pub_key, priv_key, uecc_curve)
|| !uECC_valid_public_key(pub_key, uecc_curve)) {
dtls_crit("cannot generate ECC key pair\n");
return 0;
}
return uECC_curve_private_key_size(uecc_curve);
}

/* rfc4492#section-5.4 */
void
dtls_ecdsa_create_sig_hash(const unsigned char *priv_key, size_t key_size,
const unsigned char *sign_hash, size_t sign_hash_size,
uint32_t point_r[9], uint32_t point_s[9]) {
int ret;
uint32_t priv[8];
uint32_t hash[8];
uint32_t randv[8];

dtls_ec_key_to_uint32(priv_key, key_size, priv);
dtls_ec_key_to_uint32(sign_hash, sign_hash_size, hash);
do {
dtls_prng((unsigned char *)randv, key_size);
ret = ecc_ecdsa_sign(priv, hash, randv, point_r, point_s);
} while (ret);
const dtls_ecdh_curve curve = default_curve;
dtls_ecdsa_create_sig_hash2(priv_key, key_size,
sign_hash, sign_hash_size,
curve, point_r, point_s);

}

int
dtls_ecdsa_create_sig_hash2(const unsigned char *priv_key, size_t key_size,
const unsigned char *sign_hash, size_t sign_hash_size,
dtls_ecdh_curve curve,
uint32_t point_r[9], uint32_t point_s[9]) {
uint8_t sign[2 * DTLS_EC_KEY_SIZE];
uECC_Curve uecc_curve;
int curve_size;
if (!get_uecc_curve(curve, &uecc_curve)) {
dtls_warn("curve %" PRIu16 " not supported\n", curve);
return -1;
}

curve_size = uECC_curve_private_key_size(uecc_curve);

assert(key_size >= (unsigned int)curve_size);
assert(sign_hash_size >= (unsigned int)curve_size);
assert(sizeof(sign) >= 2 * (unsigned int)curve_size);
if (!uECC_sign(priv_key, sign_hash, sign_hash_size, sign, uecc_curve)) {
dtls_warn("cannot create signature\n");
return -1;
}

dtls_ec_key_to_uint32(sign, curve_size, point_r);
dtls_ec_key_to_uint32(sign + curve_size, curve_size, point_s);
return 2 * curve_size;
}

void
Expand Down Expand Up @@ -522,19 +608,39 @@ dtls_ecdsa_verify_sig_hash(const unsigned char *pub_key_x,
const unsigned char *pub_key_y, size_t key_size,
const unsigned char *sign_hash, size_t sign_hash_size,
unsigned char *result_r, unsigned char *result_s) {
uint32_t pub_x[8];
uint32_t pub_y[8];
uint32_t hash[8];
uint32_t point_r[8];
uint32_t point_s[8];

dtls_ec_key_to_uint32(pub_key_x, key_size, pub_x);
dtls_ec_key_to_uint32(pub_key_y, key_size, pub_y);
dtls_ec_key_to_uint32(result_r, key_size, point_r);
dtls_ec_key_to_uint32(result_s, key_size, point_s);
dtls_ec_key_to_uint32(sign_hash, sign_hash_size, hash);

return ecc_ecdsa_validate(pub_x, pub_y, hash, point_r, point_s);
const dtls_ecdh_curve curve = default_curve;
uint8_t pub_key[2 * DTLS_EC_KEY_SIZE];
memcpy(pub_key, pub_key_x, key_size);
memcpy(pub_key + key_size, pub_key_y, key_size);
return dtls_ecdsa_verify_sig_hash2(pub_key, key_size,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, the size of pub_key isn't that of the passed in key_size.

sign_hash, sign_hash_size,
curve,
result_r, result_s);
}

int
dtls_ecdsa_verify_sig_hash2(const unsigned char *pub_key, size_t key_size,
const unsigned char *sign_hash, size_t sign_hash_size,
dtls_ecdh_curve curve,
unsigned char *result_r, unsigned char *result_s) {
uint8_t sign[2 * DTLS_EC_KEY_SIZE];
uECC_Curve uecc_curve;
int curve_size;
if (!get_uecc_curve(curve, &uecc_curve)) {
dtls_warn("curve %" PRIu16 " not supported\n", curve);
return -1;
}
(void)result_r;
(void)result_s;

curve_size = uECC_curve_public_key_size(uecc_curve);

assert(key_size == (unsigned int)curve_size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That fires on my interop test. Adding

dtls_warn("%d != %d\n", (int) key_size, curve_size);

I get

Feb 26 16:46:45 WARN 32 != 64
tinydtls-server: crypto.c:638: dtls_ecdsa_verify_sig_hash2: Assertion `key_size == (unsigned int)curve_size' failed.

AFAIK, the key-size is passed in as sizeof(config->keyx.ecdsa.other_pub_x) in dtls.c, line 2589

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside the assert, the first interop-tests with Californium are successful.
Great!

assert(sizeof(sign) >= (unsigned int)curve_size);

/* clear sign to avoid maybe-unitialized warning */
memset(sign, 0, sizeof(sign));
return uECC_verify(pub_key, sign_hash, sign_hash_size, sign, uecc_curve);
}

int
Expand Down
Loading
Loading