Use extended Euclidean algorithm for modular inverse #7

swenson · 2013-10-05T04:33:27Z

This roughly doubles the speed of the ed25519 implementation in Python.

For whatever reason, this is faster than the previous squaring method.

Before:

Time generate
2.8231959343

Time create signature
5.24891996384

Time verify signature
7.75262784958

After:

Time generate
1.58998298645

Time create signature
3.2284090519

Time verify signature
4.52029514313

alex · 2013-10-05T04:34:50Z

ed25519.py

+    div = b
+    while True:
+        if div == 0:
+          return u


This should be 4 space indented.

Also, might be easier to write this as while div != 0 and put return u outside the loop. I don't have a strong opinion on that, but it's how I've seen most reference implementations write it.

dstufft · 2013-10-05T04:38:08Z

Important to ask, Are you OK with this code being licensed as in #6 that PR is just waiting on the OK from the other contributor before i merge it.

alex · 2013-10-05T04:40:17Z

ed25519.py

+    while True:
+        if div == 0:
+          return u
+        qq, div, d = d // div, d % div, div


d // div, d %div might be better implemented with divmod(d, div), might save a few cycles.

Ah, I forgot about divmod. I'll double check, but you're probably right.

At least on pypy, divmod actually saves instructions (both div and mod are
implemented in terms of divmod), I assume this is true on CPython as well

On Fri, Oct 4, 2013 at 9:56 PM, Christopher Swenson <
notifications@github.com> wrote:

In ed25519.py:

are both quadratic, so this is likely the fastest way to do this.

See, for example, H. Cohen, A Course in Computational Algebraic Number

Theory, Algorithm 1.3.6

"""

a, b = z, q

if b == 0:

return 1

u = 1

d = a

r = 0

div = b

while True:

if div == 0:

return u

qq, div, d = d // div, d % div, div

Ah, I forgot about divmod. I'll double check, but you're probably right.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/7/files#r6782936
.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

swenson · 2013-10-05T04:54:15Z

@dstufft I left a comment on the license PR -- I'm actually not okay with that license, but mostly because it is contradictory.

* Fix docstring formatting * Use divmod for a speedup * Cleanup 2-spacing * Remove unused variables

swenson · 2013-10-05T04:59:32Z

Updated, now even faster:

Time generate
1.10641002655
Time create signature
2.36265206337
Time verify signature
3.46614480019

alex · 2013-10-05T05:01:03Z

ed25519.py

+    """
+    d, div = z, q
+    if div == 0:
+        return 1


q will never be 0, so we can just drop this case, as far as I can tell.

alex · 2013-10-05T05:02:02Z

Modulo that comment, and the licensing issue, I think this looks good. I want to get another set of eyes on it though.

alex · 2013-10-05T05:25:05Z

This change seems to have caused it to recurse infinetly, and I don't understand why.

alex · 2013-10-05T05:26:35Z

ed25519.py

@@ -92,7 +90,7 @@ def scalarmult(P, e):
    if e == 0:
        return (0, 1)

-    Q = scalarmult(P, e // 2)
+    Q = scalarmult(P, e / 2)


Why this change?

Not sure how that wound up in there. I'll change it back.

alex · 2013-10-05T05:33:36Z

ed25519.py

@@ -39,22 +39,20 @@ def pow2(x, p):
        p -= 1
    return x


pow2 can be deleted now

gnprice · 2013-10-05T06:20:42Z

Great that this is faster! Glad to see more eyes on the crypto algorithms here.

The algorithm looks right, and standard. The one drawback, which is important in some contexts, is that it introduces branches, which means it's vulnerable to timing attacks. This is the reason why DJB didn't use this algorithm for Curve25519, a precursor of this system, and presumably also why he and his collaborators didn't for Ed25519.

The Curve25519 paper explains:

I compute X0 (nQ) = xz p−2 using a straightforward sequence of 254 squarings and 11 multiplications. This is about 7% of the Curve25519 computation. An extended-Euclid inversion of z, randomized to protect against timing attacks, might be faster, but the maximum potential speedup is very small, while the cost in code complexity is large.

http://cr.yp.to/ecdh/curve25519-20060209.pdf

(This version isn't randomized to protect against timing attacks, and I don't think we'd want to implement such a version.) The Ed25519 paper just says it does inversion the same way as in Curve25519.

This is no problem as long as this code is only used for verifying signatures on public data. It's a serious problem for making signatures. We should use this algorithm only if we remove the signing code from this pure-Python fallback, to make sure nobody unwittingly falls into that vulnerability. Or if we use this algorithm for verifying signatures, and another one for making them. Either way, there should also be a comment warning of the danger.

Our existing and previous implementations haven't been carefully secured against branches or loads that depend on potentially secret data -- there might well be such things hidden in the implementation of e.g. long multiplication -- but I believe in principle they could be. At the Python level they're free of such things.

swenson · 2013-10-05T06:24:43Z

@gnprice Great point. We can definitely look into reviving the other inv(z) implementation if we want to guard against that.

alex · 2013-10-05T06:25:54Z

Nice catch @gnprice. I seriously doubt any of this code is timing attack safe (I'm pretty suspicious that you can reliably writing timing attack safe code which runs on CPython or PyPy in general). At a minimum starting to clearly document this would be valuable.

dstufft · 2013-10-05T07:22:14Z

So for the use case of why I wanted to make ed25519 fast verifying signatures is all we needed. However I do think it'd be nice to have the other operations just for completeness sake. I don't understand the PR itself but if it makes sense to have two implementations for different use cases I think that's reasonable?

swenson · 2013-10-05T15:19:02Z

In that case, I think it would make more sense to have a separate file, say, ed25519_safe.py, that had timing-attack resistant implementations of everything.

Or, alternatively, make ed25519.py be the safe one, but also have ed25519_fast.py.

And maybe make a note that if you want it safe and fast, that you should use a C library. :)

alex · 2013-10-05T16:31:40Z

I think before we delve into having multiple APIs, we really need to establish if a) this is remotely timing attack safe now, or b) it has any hope of being so in pure-python.

swenson · 2013-10-05T16:37:58Z

@alex I'm not sure it is timing attack resistant at all right -- the scalarmult function is very sensitive, for instance.

alex · 2013-10-05T16:41:02Z

I'm assuming the word "now" is missing there?

On Sat, Oct 5, 2013 at 9:37 AM, Christopher Swenson <
notifications@github.com> wrote:

@alex https://github.com/alex I'm not sure it is timing attack
resistant at all right -- the scalarmult function is very sensitive, for
instance.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/7#issuecomment-25751871
.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

swenson · 2013-10-05T17:04:26Z

Yup. Right now.
On Oct 5, 2013 9:41 AM, "Alex Gaynor" notifications@github.com wrote:

I'm assuming the word "now" is missing there?

On Sat, Oct 5, 2013 at 9:37 AM, Christopher Swenson <
notifications@github.com> wrote:

@alex https://github.com/alex I'm not sure it is timing attack
resistant at all right -- the scalarmult function is very sensitive, for
instance.

—
Reply to this email directly or view it on GitHub<
https://github.com/pyca/ed25519/pull/7#issuecomment-25751871>
.

"I disapprove of what you say, but I will defend to the death your right
to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/7#issuecomment-25751927
.

alex · 2013-10-05T18:06:13Z

I started documenting this in #11

swenson · 2013-10-05T22:12:24Z

Superceded by homogeneous coordinate version.

Use extended Euclidean algorithm for modular inverse

e353dce

alex reviewed Oct 5, 2013
View reviewed changes

Address @alex's comments:

9aff754

* Fix docstring formatting * Use divmod for a speedup * Cleanup 2-spacing * Remove unused variables

alex reviewed Oct 5, 2013
View reviewed changes

swenson added 2 commits October 4, 2013 22:10

q is never 0, so don't need to check

4a91e6c

Remove comment about quadratic arithmetic in Python

44c90ac

alex reviewed Oct 5, 2013
View reviewed changes

Change back to scalar division

714b598

alex reviewed Oct 5, 2013
View reviewed changes

ed25519.py

@@ -39,22 +39,20 @@ def pow2(x, p):

p -= 1

return x

Copy link

Member

alex Oct 5, 2013

Choose a reason for hiding this comment

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

pow2 can be deleted now

Delete pow2

3efca04

swenson mentioned this pull request Oct 5, 2013

Reuse variables in edwards; use shifts in core loops #9

Closed

swenson closed this Oct 5, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use extended Euclidean algorithm for modular inverse #7

Use extended Euclidean algorithm for modular inverse #7

swenson commented Oct 5, 2013

alex Oct 5, 2013

alex Oct 5, 2013

swenson Oct 5, 2013

dstufft commented Oct 5, 2013

alex Oct 5, 2013

swenson Oct 5, 2013

alex Oct 5, 2013

swenson commented Oct 5, 2013

swenson commented Oct 5, 2013

alex Oct 5, 2013

swenson Oct 5, 2013

alex commented Oct 5, 2013

alex commented Oct 5, 2013

alex Oct 5, 2013

swenson Oct 5, 2013

alex Oct 5, 2013

gnprice commented Oct 5, 2013

swenson commented Oct 5, 2013

alex commented Oct 5, 2013

dstufft commented Oct 5, 2013

swenson commented Oct 5, 2013

alex commented Oct 5, 2013

swenson commented Oct 5, 2013

alex commented Oct 5, 2013

swenson commented Oct 5, 2013

alex commented Oct 5, 2013

swenson commented Oct 5, 2013

Use extended Euclidean algorithm for modular inverse #7

Use extended Euclidean algorithm for modular inverse #7

Conversation

swenson commented Oct 5, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstufft commented Oct 5, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swenson commented Oct 5, 2013

swenson commented Oct 5, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex commented Oct 5, 2013

alex commented Oct 5, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice commented Oct 5, 2013

swenson commented Oct 5, 2013

alex commented Oct 5, 2013

dstufft commented Oct 5, 2013

swenson commented Oct 5, 2013

alex commented Oct 5, 2013

swenson commented Oct 5, 2013

alex commented Oct 5, 2013

swenson commented Oct 5, 2013

alex commented Oct 5, 2013

swenson commented Oct 5, 2013