Timing Attacks

14 views
Skip to first unread message

Adam Sjøgren

unread,
Jul 17, 2010, 6:48:59 PM7/17/10
to openi...@googlegroups.com
Hi.

Recently some discussion of timing attacks against OpenID
implementations has surfaced:

* http://lists.openid.net/pipermail/openid-security/2010-July/001156.html

I wonder if the implementation in Net::OpenID isn't susceptible as well.

I have pushed a preliminary and untested patch to my fork of the
libnet-openid-perl repository on github:

* http://github.com/asjo/libnet-openid-perl/commit/fd251ddaa5da72129fe715873beff6821ae23bcd

I am by no means well versed in security matters, and I have mostly been
monkeying around with the Net::OpenID code, so caveat emptor.

What are your thoughts?

Best regards,

Adam

--
"Halleluja og hurlumhej" Adam Sjøgren
as...@koldfront.dk

Brad Fitzpatrick

unread,
Aug 4, 2010, 1:32:31 AM8/4/10
to openi...@googlegroups.com, Martin Atkins, Tatsuhiko Miyagawa
Martin, Tatsuhiko,

Could one of you look at this?



--
You received this message because you are subscribed to the Google Groups "Net::OpenID for Perl" group.
To post to this group, send email to openi...@googlegroups.com.
To unsubscribe from this group, send email to openid-perl...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/openid-perl?hl=en.


Martin Atkins

unread,
Aug 8, 2010, 11:47:15 PM8/8/10
to openi...@googlegroups.com
On 07/17/2010 03:48 PM, Adam Sj�gren wrote:
> Hi.
>
> Recently some discussion of timing attacks against OpenID
> implementations has surfaced:
>
> * http://lists.openid.net/pipermail/openid-security/2010-July/001156.html
>
> I wonder if the implementation in Net::OpenID isn't susceptible as well.
>
> I have pushed a preliminary and untested patch to my fork of the
> libnet-openid-perl repository on github:
>
> * http://github.com/asjo/libnet-openid-perl/commit/fd251ddaa5da72129fe715873beff6821ae23bcd
>
> I am by no means well versed in security matters, and I have mostly been
> monkeying around with the Net::OpenID code, so caveat emptor.
>
> What are your thoughts?
>

In your commit message you include some benchmark output, but it looks
like the difference between your "same" and "different" tests didn't
really change that much between the original code and your new
implementation. Are you sure your code is working as expected?

Also, am I reading it right that after your change it now takes ~38s to
compare the strings ten times? That seems like a pretty big performance hit.


Adam Sjøgren

unread,
Aug 9, 2010, 5:24:01 AM8/9/10
to openi...@googlegroups.com
On Sun, 08 Aug 2010 20:47:15 -0700, Martin wrote:

> On 07/17/2010 03:48 PM, Adam Sjøgren wrote:

>> What are your thoughts?

> In your commit message you include some benchmark output, but it looks
> like the difference between your "same" and "different" tests didn't
> really change that much between the original code and your new
> implementation.

The difference between different (11441647.60/s) and same
(10040160.64/s) was 1401486.96/s, i.e. ~12.2%.

The difference afterwards was ~0.

> Are you sure your code is working as expected?

No. As I said, I am no security expert.

I know the code works for me, in that my homegrown OpenID provider lets
me login, and my test-application that acts as a client also works.

But I wouldn't conclude from that, that the patch solves the problem.

I have tried to implement straightforwardly the suggested solution in
the referenced article, and I have hooked it in where it looked
appropriate, but I may very well have made mistakes.

> Also, am I reading it right that after your change it now takes ~38s
> to compare the strings ten times? That seems like a pretty big
> performance hit.

The code _is_ comparing two strings of size 10 million, by doing a
substr() of both strings 10 million times. That has to be slower than
the built-in comparison, which, I guess, is optimized.

Just doing a single substr on each position of a 10 million char string
takes quite a while:

$ perl -MBenchmark -e '$x="x" x 10000000; Benchmark::timethese(10, { substr=>sub { $l=length($x); for($i=0; $i<$l; $i++) { substr($x, $i, 1) } } });'
Benchmark: timing 10 iterations of substr...
substr: 23 wallclock secs (23.14 usr + 0.01 sys = 23.15 CPU) @ 0.43/s (n=10)

Since the new function is used to compare signatures, which aren't that
long, I don't think performance is much of a problem.


Best regards,

Adam

--
"Emacs is like a laser guided missile. It only has to Adam Sjøgren
be slight mis-configured to ruin your whole day." as...@koldfront.dk

Adam Sjøgren

unread,
Aug 9, 2010, 5:42:55 AM8/9/10
to openi...@googlegroups.com
I have added some simple tests of whether timing_indep_eq() returns the
same as 'eq':

* http://github.com/asjo/libnet-openid-perl/commit/1dba1ac8cfe43687a5c429ae643fb2da3425a451


Best regards,

Adam

--
"It's been a long, slow collision" Adam Sjøgren
as...@koldfront.dk

Martin Atkins

unread,
Aug 9, 2010, 1:00:33 PM8/9/10
to openi...@googlegroups.com
On 08/09/2010 02:24 AM, Adam Sj�gren wrote:
> On Sun, 08 Aug 2010 20:47:15 -0700, Martin wrote:
>
>> On 07/17/2010 03:48 PM, Adam Sj�gren wrote:
>
>>> What are your thoughts?
>
>> In your commit message you include some benchmark output, but it looks
>> like the difference between your "same" and "different" tests didn't
>> really change that much between the original code and your new
>> implementation.
>
> The difference between different (11441647.60/s) and same
> (10040160.64/s) was 1401486.96/s, i.e. ~12.2%.
>
> The difference afterwards was ~0.
>

Oh, okay. I was reading the wrong part of the output, apparently. Sorry.
The formatting of your commit message in github's commit page threw me
off a little but with fresh eyes I see what you're doing.

Looks good, then.

>
>> Also, am I reading it right that after your change it now takes ~38s
>> to compare the strings ten times? That seems like a pretty big
>> performance hit.
>
> The code _is_ comparing two strings of size 10 million, by doing a
> substr() of both strings 10 million times. That has to be slower than
> the built-in comparison, which, I guess, is optimized.
>
> Just doing a single substr on each position of a 10 million char string
> takes quite a while:
>
> $ perl -MBenchmark -e '$x="x" x 10000000; Benchmark::timethese(10, { substr=>sub { $l=length($x); for($i=0; $i<$l; $i++) { substr($x, $i, 1) } } });'
> Benchmark: timing 10 iterations of substr...
> substr: 23 wallclock secs (23.14 usr + 0.01 sys = 23.15 CPU) @ 0.43/s (n=10)
>
> Since the new function is used to compare signatures, which aren't that
> long, I don't think performance is much of a problem.
>

Right. The performance penalty makes sense in light of how it's
implemented. In practice we'll be comparing against a known value which
is much shorter than that (20 bytes, according to the code) and I notice
that your timing_indep_eq function returns early if the input string is
not the same length as the expected string.

Reply all
Reply to author
Forward
0 new messages