From 149130fd943cef51c20d8eb1e81f0ad7c6e447ef Mon Sep 17 00:00:00 2001 From: Michael Hamburg Date: Wed, 2 Mar 2016 11:00:46 -0800 Subject: [PATCH] working through the TODOs. Correct the sign of the to/from EdDSA conversions (but is it correct for future curves?). SHA-3 now throws exceptions on over-long output --- src/per_curve/crypto.tmpl.c | 2 +- src/per_curve/decaf.tmpl.c | 8 ++++---- src/per_curve/decaf.tmpl.h | 1 - src/per_curve/eddsa.tmpl.c | 11 ++++------- src/public_include/decaf/shake.h | 6 ++++-- src/public_include/decaf/shake.hxx | 29 +++++++++++++++++++---------- src/shake.c | 19 +++++++++++++------ 7 files changed, 45 insertions(+), 31 deletions(-) diff --git a/src/per_curve/crypto.tmpl.c b/src/per_curve/crypto.tmpl.c index a2697a7..8831e21 100644 --- a/src/per_curve/crypto.tmpl.c +++ b/src/per_curve/crypto.tmpl.c @@ -3,7 +3,6 @@ * @brief Example Decaf crypto routines */ -#include "f_field.h" /* for SER_BYTES; FUTURE: find a better way to do this? */ #include #include @@ -11,6 +10,7 @@ #define API_NS(_id) $(c_ns)_##_id #define SCALAR_BITS $(C_NS)_SCALAR_BITS #define SCALAR_BYTES ((SCALAR_BITS + 7)/8) +#define SER_BYTES $(C_NS)_SER_BYTES /* TODO: canonicalize and freeze the STROBE constants in this file * (and STROBE itself for that matter) diff --git a/src/per_curve/decaf.tmpl.c b/src/per_curve/decaf.tmpl.c index 8d92875..39ff384 100644 --- a/src/per_curve/decaf.tmpl.c +++ b/src/per_curve/decaf.tmpl.c @@ -1076,7 +1076,7 @@ void API_NS(point_encode_like_eddsa) ( gf_add( u, x, t ); // x^2 + y^2 gf_add( z, q->y, q->x ); gf_sqr ( y, z); - gf_sub ( y, y, u ); // 2xy + gf_sub ( y, u, y ); // -2xy gf_sub ( z, t, x ); // y^2 - x^2 gf_sqr ( x, q->z ); gf_add ( t, x, x); @@ -1093,7 +1093,7 @@ void API_NS(point_encode_like_eddsa) ( { API_NS(point_double)(q,q); API_NS(point_double)(q,q); - gf_div_qnr(x, q->x); + gf_mul_qnr(x, q->x); gf_copy(y, q->y); gf_copy(z, q->z); } @@ -1106,7 +1106,7 @@ void API_NS(point_encode_like_eddsa) ( gf_add( u, x, t ); gf_add( z, q->y, q->x ); gf_sqr ( y, z); - gf_sub ( y, y, u ); + gf_sub ( y, u, y ); gf_sub ( z, t, x ); gf_sqr ( x, q->z ); gf_add ( t, x, x); @@ -1167,7 +1167,7 @@ decaf_error_t API_NS(point_decode_like_eddsa) ( succ &= gf_isr(p->t,p->x); /* 1/sqrt(num * denom) */ gf_mul(p->x,p->t,p->z); /* sqrt(num / denom) */ - gf_cond_neg(p->x,gf_lobit(p->x)^low); + gf_cond_neg(p->x,~gf_lobit(p->x)^low); gf_copy(p->z,ONE); #if EDDSA_USE_SIGMA_ISOGENY diff --git a/src/per_curve/decaf.tmpl.h b/src/per_curve/decaf.tmpl.h index d718d23..8a62340 100644 --- a/src/per_curve/decaf.tmpl.h +++ b/src/per_curve/decaf.tmpl.h @@ -679,7 +679,6 @@ void $(c_ns)_scalar_destroy ( /** * @brief Overwrite point with zeros. - * @todo Use this internally. */ void $(c_ns)_point_destroy ( $(c_ns)_point_t point diff --git a/src/per_curve/eddsa.tmpl.c b/src/per_curve/eddsa.tmpl.c index 839b9bd..7805897 100644 --- a/src/per_curve/eddsa.tmpl.c +++ b/src/per_curve/eddsa.tmpl.c @@ -23,7 +23,7 @@ #define EDDSA_USE_SIGMA_ISOGENY $(eddsa_sigma_iso) #define COFACTOR $(cofactor) -static void clamp( +static void clamp ( uint8_t secret_scalar_ser[$(C_NS)_EDDSA_PRIVATE_BYTES] ) { /* Blarg */ @@ -78,9 +78,8 @@ void API_NS(eddsa_derive_public_key) ( API_NS(scalar_t) secret_scalar; API_NS(scalar_decode_long)(secret_scalar, secret_scalar_ser, sizeof(secret_scalar_ser)); - /* TODO: write documentation for why (due to isogenies) this needs to be quartered */ - API_NS(scalar_sub)(secret_scalar,API_NS(scalar_zero),secret_scalar); + /* TODO: write documentation for why (due to isogenies) this needs to be quartered/eighthed */ for (unsigned int c = 1; c < COFACTOR/(1+EDDSA_USE_SIGMA_ISOGENY); c <<= 1) { API_NS(scalar_halve)(secret_scalar,secret_scalar); } @@ -149,9 +148,8 @@ void API_NS(eddsa_sign) ( { /* Scalarmul to create the nonce-point */ API_NS(scalar_t) nonce_scalar_2; - API_NS(scalar_sub)(nonce_scalar_2,API_NS(scalar_zero),nonce_scalar); - - for (unsigned int c = 1; c < COFACTOR/(1+EDDSA_USE_SIGMA_ISOGENY); c <<= 1) { + API_NS(scalar_halve)(nonce_scalar_2,nonce_scalar); + for (unsigned int c = 2; c < COFACTOR/(1+EDDSA_USE_SIGMA_ISOGENY); c <<= 1) { API_NS(scalar_halve)(nonce_scalar_2,nonce_scalar_2); } @@ -233,7 +231,6 @@ decaf_error_t API_NS(eddsa_verify) ( &signature[$(C_NS)_EDDSA_PUBLIC_BYTES], $(C_NS)_EDDSA_PRIVATE_BYTES ); - API_NS(scalar_sub)(response_scalar, API_NS(scalar_zero), response_scalar); /* TODO because nega-base point */ #if EDDSA_USE_SIGMA_ISOGENY API_NS(scalar_add)(response_scalar,response_scalar,response_scalar); #endif diff --git a/src/public_include/decaf/shake.h b/src/public_include/decaf/shake.h index 6fdcea6..e949071 100644 --- a/src/public_include/decaf/shake.h +++ b/src/public_include/decaf/shake.h @@ -68,8 +68,10 @@ void decaf_sha3_update ( * @param [inout] sponge The context. * @param [out] out The output data. * @param [in] len The requested output data length in bytes. + * @return DECAF_FAILURE if the sponge has exhausted its output capacity. + * @return DECAF_SUCCESS otherwise. */ -void decaf_sha3_output ( +decaf_error_t decaf_sha3_output ( decaf_keccak_sponge_t sponge, uint8_t * __restrict__ out, size_t len @@ -83,7 +85,7 @@ void decaf_sha3_output ( * @param [out] out The output data. * @param [in] len The requested output data length in bytes. */ -void decaf_sha3_final ( +decaf_error_t decaf_sha3_final ( decaf_keccak_sponge_t sponge, uint8_t * __restrict__ out, size_t len diff --git a/src/public_include/decaf/shake.hxx b/src/public_include/decaf/shake.hxx index 50f1d55..e9ce45e 100644 --- a/src/public_include/decaf/shake.hxx +++ b/src/public_include/decaf/shake.hxx @@ -58,7 +58,9 @@ public: inline SecureBuffer output(size_t len) throw(std::bad_alloc, LengthException) { if (len > max_output_size()) throw LengthException(); SecureBuffer buffer(len); - decaf_sha3_output(sp,buffer.data(),len); + if (DECAF_SUCCESS != decaf_sha3_output(sp,buffer.data(),len)) { + throw LengthException(); + } return buffer; } @@ -66,21 +68,28 @@ public: inline SecureBuffer final(size_t len) throw(std::bad_alloc, LengthException) { if (len > max_output_size()) throw LengthException(); SecureBuffer buffer(len); - decaf_sha3_final(sp,buffer.data(),len); + if (DECAF_SUCCESS != decaf_sha3_final(sp,buffer.data(),len)) { + throw LengthException(); + } return buffer; } - /** - * @brief Output bytes from the sponge. - * @todo make this throw exceptions. + /** @brief Output bytes from the sponge. Throw LengthException if you've + * output too many bytes from a SHA-3 instance. */ inline void output(Buffer b) throw(LengthException) { - decaf_sha3_output(sp,b.data(),b.size()); + if (DECAF_SUCCESS != decaf_sha3_output(sp,b.data(),b.size())) { + throw LengthException(); + } } - /** @brief Output bytes from the sponge and reinitialize it. */ + /** @brief Output bytes from the sponge and reinitialize it. Throw + * LengthException if you've output too many bytes from a SHA3 instance. + */ inline void final(Buffer b) throw(LengthException) { - decaf_sha3_final(sp,b.data(),b.size()); + if (DECAF_SUCCESS != decaf_sha3_final(sp,b.data(),b.size())) { + throw LengthException(); + } } /** @brief Return the sponge's default output size. */ @@ -94,12 +103,12 @@ public: } /** Output the default number of bytes. */ - inline SecureBuffer output() throw(std::bad_alloc) { + inline SecureBuffer output() throw(std::bad_alloc,LengthException) { return output(default_output_size()); } /** Output the default number of bytes, and reset hash. */ - inline SecureBuffer final() throw(std::bad_alloc) { + inline SecureBuffer final() throw(std::bad_alloc,LengthException) { return final(default_output_size()); } diff --git a/src/shake.c b/src/shake.c index fb2c177..8f83b9b 100644 --- a/src/shake.c +++ b/src/shake.c @@ -165,17 +165,22 @@ void decaf_sha3_update ( } } -void decaf_sha3_output ( +decaf_error_t decaf_sha3_output ( decaf_keccak_sponge_t decaf_sponge, uint8_t * __restrict__ out, size_t len ) { + decaf_error_t ret = DECAF_SUCCESS; assert(decaf_sponge->params->position < decaf_sponge->params->rate); assert(decaf_sponge->params->rate < sizeof(decaf_sponge->state)); if (decaf_sponge->params->max_out != 0xFF) { - assert(decaf_sponge->params->client >= len); - decaf_sponge->params->client -= len; + if (decaf_sponge->params->client >= len) { + decaf_sponge->params->client -= len; + } else { + decaf_sponge->params->client = 0; + ret = DECAF_FAILURE; + } } switch (decaf_sponge->params->flags) { @@ -198,7 +203,7 @@ void decaf_sha3_output ( if (cando > len) { memcpy(out, state, len); decaf_sponge->params->position += len; - return; + return ret; } else { memcpy(out, state, cando); dokeccak(decaf_sponge); @@ -206,15 +211,17 @@ void decaf_sha3_output ( out += cando; } } + return ret; } -void decaf_sha3_final ( +decaf_error_t decaf_sha3_final ( decaf_keccak_sponge_t decaf_sponge, uint8_t * __restrict__ out, size_t len ) { - decaf_sha3_output(decaf_sponge,out,len); + decaf_error_t ret = decaf_sha3_output(decaf_sponge,out,len); decaf_sha3_reset(decaf_sponge); + return ret; } void decaf_sha3_reset (