linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Weimer <fw@deneb.enyo.de>
To: Daniel Borkmann <borkmann@iogearbox.net>
Cc: gregkh@linuxfoundation.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] lib: memcmp_nta: add timing-attack secure memcmp
Date: Tue, 12 Feb 2013 11:23:29 +0100	[thread overview]
Message-ID: <87y5et7rvi.fsf@mid.deneb.enyo.de> (raw)
In-Reply-To: <5119777E.6030005@iogearbox.net> (Daniel Borkmann's message of "Mon, 11 Feb 2013 23:58:06 +0100")

* Daniel Borkmann:

> On 02/11/2013 08:00 PM, Florian Weimer wrote:
>> * Daniel Borkmann:
>
> Thanks for your feedback, Florian!
>
>>> + * memcmp_nta - memcmp that is secure against timing attacks
>>
>> It's not providing an ordering, so it should not have "cmp" in the
>> name.
>
> I agree. What would you suggest? Probably, it would make sense to
> integrate this into the Linux crypto API and name it sth like ...
>
>   crypto_mem_verify(const void *,const void *,__kernel_size_t)
>
> ... which returns:
>
>   == 0 - mem regions equal each other
>   != 0 - mem regions do not equal each other

crypto_mem_equal or crypto_mem_equals should be fine.  Or anything
else which matches an existing function name with similar function.

>>> +	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
>>> +		res |= (*su1 ^ *su2);
>>
>> The compiler could still short-circuit this loop.  Unlikely at
>> present, but this looks like a maintenance hazard.
>
> So then better we leave out '|' as a possible candidate and rewrite it as:
>
> +	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
> +		res += (*su1 ^ *su2);

That will cause false matches for long inputs.

If we had only four platforms to support, I would write this function
in assembler because it will be considerably easier to read.

      reply	other threads:[~2013-02-12 10:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1360528614.git.dborkman@redhat.com>
2013-02-10 22:00 ` [PATCH] lib: memcmp_nta: add timing-attack secure memcmp Daniel Borkmann
2013-02-10 23:24   ` Joe Perches
2013-02-10 23:30     ` Daniel Borkmann
2013-02-10 23:50       ` Greg KH
2013-02-11  8:19         ` Daniel Borkmann
2013-02-11 18:37   ` Andy Lutomirski
2013-02-11 19:39     ` Daniel Borkmann
2013-02-11 19:00   ` Florian Weimer
2013-02-11 22:58     ` Daniel Borkmann
2013-02-12 10:23       ` Florian Weimer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y5et7rvi.fsf@mid.deneb.enyo.de \
    --to=fw@deneb.enyo.de \
    --cc=akpm@linux-foundation.org \
    --cc=borkmann@iogearbox.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).