From e95b7c7f0e05939e1e23b914539cbdb18c7da5ca Mon Sep 17 00:00:00 2001 From: Michael Hamburg Date: Thu, 19 Nov 2015 13:36:22 -0800 Subject: [PATCH] made scalar inverse WARN_UNUSED and made it throw. Small fix to sagetest. Changed some places that assumed that success is true, in case I want to adopt the proposal that success is 0 --- Makefile | 2 +- src/public_include/decaf/decaf_255.h | 4 +-- src/public_include/decaf/decaf_255.hxx | 37 +++++++++++++++++--------- src/public_include/decaf/decaf_448.h | 2 +- src/public_include/decaf/decaf_448.hxx | 34 +++++++++++++++-------- test/test_decaf.cxx | 3 ++- test/test_decaf.sage | 3 ++- 7 files changed, 56 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index b9221f7..b04d5ff 100644 --- a/Makefile +++ b/Makefile @@ -206,7 +206,7 @@ $(BUILD_ASM)/%.s: test/%.cxx $(HEADERSXX) sage: $(BUILDPYS) sagetest: sage lib - LD_LIBRARY_PATH=$(BUILD_LIB) sage $(BUILD_PY)/test_decaf.sage + $(SAGE) $(BUILD_PY)/test_decaf.sage $(BUILDPYS): $(SAGES) $(BUILD_OBJ)/timestamp cp -f $(SAGES) $(BUILD_PY)/ diff --git a/src/public_include/decaf/decaf_255.h b/src/public_include/decaf/decaf_255.h index fa6d939..1054b09 100644 --- a/src/public_include/decaf/decaf_255.h +++ b/src/public_include/decaf/decaf_255.h @@ -161,12 +161,12 @@ void decaf_255_scalar_mul ( * @brief Invert a scalar. When passed zero, return 0. The input and output may alias. * @param [in] a A scalar. * @param [out] out 1/a. - * @return DECAF_TRUE The input is nonzero. + * @return DECAF_SUCCESS The input is nonzero. */ decaf_bool_t decaf_255_scalar_invert ( decaf_255_scalar_t out, const decaf_255_scalar_t a -) API_VIS NONNULL2 NOINLINE; +) API_VIS WARN_UNUSED NONNULL2 NOINLINE; /** * @brief Copy a scalar. The scalars may use the same memory, in which diff --git a/src/public_include/decaf/decaf_255.hxx b/src/public_include/decaf/decaf_255.hxx index 87dad30..c1ca053 100644 --- a/src/public_include/decaf/decaf_255.hxx +++ b/src/public_include/decaf/decaf_255.hxx @@ -164,13 +164,19 @@ public: inline Scalar operator- () const NOEXCEPT { Scalar r((NOINIT())); decaf_255_scalar_sub(r.s,decaf_255_scalar_zero,s); return r; } /** @brief Invert with Fermat's Little Theorem (slow!). If *this == 0, return 0. */ - inline Scalar inverse() const NOEXCEPT { Scalar r; decaf_255_scalar_invert(r.s,s); return r; } + inline Scalar inverse() const throw(CryptoException) { + Scalar r; + if (DECAF_SUCCESS != decaf_255_scalar_invert(r.s,s)) { + throw CryptoException(); + } + return r; + } /** @brief Divide by inverting q. If q == 0, return 0. */ - inline Scalar operator/ (const Scalar &q) const NOEXCEPT { return *this * q.inverse(); } + inline Scalar operator/ (const Scalar &q) const throw(CryptoException) { return *this * q.inverse(); } /** @brief Divide by inverting q. If q == 0, return 0. */ - inline Scalar &operator/=(const Scalar &q) NOEXCEPT { return *this *= q.inverse(); } + inline Scalar &operator/=(const Scalar &q) throw(CryptoException) { return *this *= q.inverse(); } /** @brief Compare in constant time */ inline bool operator!=(const Scalar &q) const NOEXCEPT { return !(*this == q); } @@ -245,8 +251,12 @@ public: * or was the identity and allow_identity was DECAF_FALSE. */ inline explicit Point(const FixedBlock &buffer, decaf_bool_t allow_identity=DECAF_TRUE) - throw(CryptoException) { if (!decode(*this,buffer,allow_identity)) throw CryptoException(); } - + throw(CryptoException) { + if (DECAF_SUCCESS != decode(*this,buffer,allow_identity)) { + throw CryptoException(); + } + } + /** * @brief Initialize from C++ fixed-length byte string. * The all-zero string maps to the identity. @@ -335,13 +345,13 @@ public: inline Point &operator*=(const Scalar &s) NOEXCEPT { decaf_255_point_scalarmul(p,p,s.s); return *this; } /** @brief Multiply by s.inverse(). If s=0, maps to the identity. */ - inline Point operator/ (const Scalar &s) const NOEXCEPT { return (*this) * s.inverse(); } + inline Point operator/ (const Scalar &s) const throw(CryptoException) { return (*this) * s.inverse(); } /** @brief Multiply by s.inverse(). If s=0, maps to the identity. */ - inline Point &operator/=(const Scalar &s) NOEXCEPT { return (*this) *= s.inverse(); } + inline Point &operator/=(const Scalar &s) throw(CryptoException) { return (*this) *= s.inverse(); } /** @brief Validate / sanity check */ - inline bool validate() const NOEXCEPT { return !!decaf_255_point_valid(p); } + inline bool validate() const NOEXCEPT { return DECAF_SUCCESS == decaf_255_point_valid(p); } /** @brief Double-scalar multiply, equivalent to q*qs + r*rs but faster. */ static inline Point double_scalarmul ( @@ -403,12 +413,12 @@ public: if (buf.size() < HASH_BYTES) { ret &= decaf_memeq(&buf2[buf.size()], &buf2[HASH_BYTES], HASH_BYTES - buf.size()); } - if (ret) { + if (DECAF_SUCCESS == ret) { /* TODO: make this constant time?? */ memcpy(buf.data(),buf2,(buf.size() < HASH_BYTES) ? buf.size() : HASH_BYTES); } decaf_bzero(buf2,sizeof(buf2)); - return !!ret; + return DECAF_SUCCESS == ret; } /** @brief Steganographically encode this */ @@ -510,7 +520,7 @@ public: inline Point operator* (const Scalar &s) const NOEXCEPT { Point r; decaf_255_precomputed_scalarmul(r.p,get(),s.s); return r; } /** @brief Multiply by s.inverse(). If s=0, maps to the identity. */ - inline Point operator/ (const Scalar &s) const NOEXCEPT { return (*this) * s.inverse(); } + inline Point operator/ (const Scalar &s) const throw(CryptoException) { return (*this) * s.inverse(); } /** @brief Return the table for the base point. */ static inline const Precomputed base() NOEXCEPT { return Precomputed(); } @@ -535,8 +545,11 @@ inline SecureBuffer IsoEd25519::Scalar::direct_scalarmul ( decaf_bool_t short_circuit ) const throw(CryptoException) { SecureBuffer out(IsoEd25519::Point::SER_BYTES); - if (!decaf_255_direct_scalarmul(out.data(), in.data(), s, allow_identity, short_circuit)) + if (DECAF_SUCCESS != + decaf_255_direct_scalarmul(out.data(), in.data(), s, allow_identity, short_circuit) + ) { throw CryptoException(); + } return out; } /** endcond */ diff --git a/src/public_include/decaf/decaf_448.h b/src/public_include/decaf/decaf_448.h index bcf7c91..001aaed 100644 --- a/src/public_include/decaf/decaf_448.h +++ b/src/public_include/decaf/decaf_448.h @@ -164,7 +164,7 @@ void decaf_448_scalar_mul ( decaf_bool_t decaf_448_scalar_invert ( decaf_448_scalar_t out, const decaf_448_scalar_t a -) API_VIS NONNULL2 NOINLINE; +) API_VIS WARN_UNUSED NONNULL2 NOINLINE; /** * @brief Copy a scalar. The scalars may use the same memory, in which diff --git a/src/public_include/decaf/decaf_448.hxx b/src/public_include/decaf/decaf_448.hxx index fcb3566..484bcc2 100644 --- a/src/public_include/decaf/decaf_448.hxx +++ b/src/public_include/decaf/decaf_448.hxx @@ -163,13 +163,19 @@ public: inline Scalar operator- () const NOEXCEPT { Scalar r((NOINIT())); decaf_448_scalar_sub(r.s,decaf_448_scalar_zero,s); return r; } /** @brief Invert with Fermat's Little Theorem (slow!). If *this == 0, return 0. */ - inline Scalar inverse() const NOEXCEPT { Scalar r; decaf_448_scalar_invert(r.s,s); return r; } + inline Scalar inverse() const throw(CryptoException) { + Scalar r; + if (DECAF_SUCCESS != decaf_448_scalar_invert(r.s,s)) { + throw CryptoException(); + } + return r; + } /** @brief Divide by inverting q. If q == 0, return 0. */ - inline Scalar operator/ (const Scalar &q) const NOEXCEPT { return *this * q.inverse(); } + inline Scalar operator/ (const Scalar &q) const throw(CryptoException) { return *this * q.inverse(); } /** @brief Divide by inverting q. If q == 0, return 0. */ - inline Scalar &operator/=(const Scalar &q) NOEXCEPT { return *this *= q.inverse(); } + inline Scalar &operator/=(const Scalar &q) throw(CryptoException) { return *this *= q.inverse(); } /** @brief Compare in constant time */ inline bool operator!=(const Scalar &q) const NOEXCEPT { return !(*this == q); } @@ -190,8 +196,11 @@ public: decaf_bool_t short_circuit=DECAF_TRUE ) const throw(CryptoException) { SecureBuffer out(/*FIXME Point::*/SER_BYTES); - if (!decaf_448_direct_scalarmul(out.data(), in.data(), s, allow_identity, short_circuit)) + if (DECAF_SUCCESS != + decaf_448_direct_scalarmul(out.data(), in.data(), s, allow_identity, short_circuit) + ) { throw CryptoException(); + } return out; } }; @@ -247,7 +256,9 @@ public: * or was the identity and allow_identity was DECAF_FALSE. */ inline explicit Point(const FixedBlock &buffer, decaf_bool_t allow_identity=DECAF_TRUE) - throw(CryptoException) { if (!decode(*this,buffer,allow_identity)) throw CryptoException(); } + throw(CryptoException) { + if (DECAF_SUCCESS != decode(*this,buffer,allow_identity)) throw CryptoException(); + } /** * @brief Initialize from C++ fixed-length byte string. @@ -346,13 +357,13 @@ public: inline Point &operator*=(const Scalar &s) NOEXCEPT { decaf_448_point_scalarmul(p,p,s.s); return *this; } /** @brief Multiply by s.inverse(). If s=0, maps to the identity. */ - inline Point operator/ (const Scalar &s) const NOEXCEPT { return (*this) * s.inverse(); } + inline Point operator/ (const Scalar &s) const throw(CryptoException) { return (*this) * s.inverse(); } /** @brief Multiply by s.inverse(). If s=0, maps to the identity. */ - inline Point &operator/=(const Scalar &s) NOEXCEPT { return (*this) *= s.inverse(); } + inline Point &operator/=(const Scalar &s) throw(CryptoException) { return (*this) *= s.inverse(); } /** @brief Validate / sanity check */ - inline bool validate() const NOEXCEPT { return !!decaf_448_point_valid(p); } + inline bool validate() const NOEXCEPT { return DECAF_SUCCESS == decaf_448_point_valid(p); } /** @brief Double-scalar multiply, equivalent to q*qs + r*rs but faster. */ static inline Point double_scalarmul ( @@ -417,14 +428,15 @@ public: ret = decaf_448_invert_elligator_nonuniform(buf2, p, hint); } if (buf.size() < HASH_BYTES) { + // FIXME: this &= will fail if success becomes 0 ret &= decaf_memeq(&buf2[buf.size()], &buf2[HASH_BYTES], HASH_BYTES - buf.size()); } - if (ret) { + if (DECAF_SUCCESS == ret) { /* TODO: make this constant time?? */ memcpy(buf.data(),buf2,(buf.size() < HASH_BYTES) ? buf.size() : HASH_BYTES); } decaf_bzero(buf2,sizeof(buf2)); - return !!ret; + return (ret == DECAF_SUCCESS); } /** @brief Steganographically encode this */ @@ -525,7 +537,7 @@ public: inline Point operator* (const Scalar &s) const NOEXCEPT { Point r; decaf_448_precomputed_scalarmul(r.p,get(),s.s); return r; } /** @brief Multiply by s.inverse(). If s=0, maps to the identity. */ - inline Point operator/ (const Scalar &s) const NOEXCEPT { return (*this) * s.inverse(); } + inline Point operator/ (const Scalar &s) const throw(CryptoException) { return (*this) * s.inverse(); } /** @brief Return the table for the base point. */ static inline const Precomputed base() NOEXCEPT { return Precomputed(); } diff --git a/test/test_decaf.cxx b/test/test_decaf.cxx index 147bb1d..ca49830 100644 --- a/test/test_decaf.cxx +++ b/test/test_decaf.cxx @@ -158,7 +158,8 @@ static void test_arithmetic() { if (i%20) continue; if (y!=0) arith_check(test,x,y,z,x*y/y,x,"invert"); - arith_check(test,x,y,z,x/0,0,"invert0"); + // TODO: negative test, but this throws an exception + //arith_check(test,x,y,z,x/0,0,"invert0"); } } diff --git a/test/test_decaf.sage b/test/test_decaf.sage index cb13320..c2e7439 100644 --- a/test/test_decaf.sage +++ b/test/test_decaf.sage @@ -1,7 +1,7 @@ from ctypes import * from base64 import * -DECAF = CDLL("libdecaf.so") +DECAF = CDLL("build/lib/libdecaf.so") F = GF(2^448-2^224-1) d = -39081 @@ -16,6 +16,7 @@ passing = True # TODO: pathological cases # TODO: Elligator # TODO: double scalar mul +# TODO: Curve25519 def random_array(length): answer = "".join([chr(randint(0,255)) for i in xrange(length)])