Simplify SPAKE2 implementation.

Simplify SPAKE2 implementation.

Currently P224EncryptedKeyExchange uses two pieces of secret: password 
and session-specific key. They are combined and used together as one 
password. It is not really needed and the calling code can do
it when neccessary.

BUG=105214
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114189

[agl, 2011-12-09 23:29:39]

As it appears, this isn't right. Since the API authenticates a channel, a secret
tied to the keys of the channel is absolutely essential and the API reflects
that.

I have heard noises from Wez about changing the API to expose the mutual secret
and thus using it independently of a secure channel. If this change is a step
towards that, then that's ok, but I would like a huge stonking warning in the
comments because the API will be in a bad state during the transition.

LGTM in the latter case, with the warning. No need to round trip the review
again.

[Sergey Ulanov, 2011-12-09 23:58:53]

On 2011/12/09 23:29:39, agl wrote:
> As it appears, this isn't right. Since the API authenticates a channel, a
secret
> tied to the keys of the channel is absolutely essential and the API reflects
> that.
> 
> I have heard noises from Wez about changing the API to expose the mutual
secret
> and thus using it independently of a secure channel.

The plan is to run EKE auth once per session over the signalling channel and
then use the master secret we get from it to authenticate individual channel.
Each channel will be authenticated similar to how chromoting authenticates
client in the current implementation: both ends will send each other hash of
channel-specific key and session key generated with EKE over the signalling
channel.


> If this change is a step
> towards that, then that's ok, but I would like a huge stonking warning in the
> comments because the API will be in a bad state during the transition.

I'm not sure I understand how this change makes this code dangerous.


> 
> LGTM in the latter case, with the warning. No need to round trip the review
> again.

[agl, 2011-12-12 15:13:37]

On Fri, Dec 9, 2011 at 6:58 PM,  <sergeyu@chromium.org> wrote:
> I'm not sure I understand how this change makes this code dangerous.

Because the API still returns a yes/no answer, which is useless
without tying it to a session id. So the partial change (removing the
session id without returning the mutual key) leaves the API in a
weird, transitory state.


Cheers

AGL

[Wez, 2011-12-12 18:21:40]

On 2011/12/12 15:13:37, agl wrote:
> On Fri, Dec 9, 2011 at 6:58 PM,  <mailto:sergeyu@chromium.org> wrote:
> > I'm not sure I understand how this change makes this code dangerous.
> 
> Because the API still returns a yes/no answer, which is useless
> without tying it to a session id. So the partial change (removing the
> session id without returning the mutual key) leaves the API in a
> weird, transitory state.

Fundamentally EKE just generates key data, so the API is useful provided the
caller uses that appropriately; you are worried that callers won't do that?

How would you feel about:
* Replacing "yes/no" with "ready/failed" results.
* Adding GenerateChallenge(secret) and VerifyResponse(response, secret) methods.

The idea being that a single EKE exchange can then authenticate multiple
channels.

[agl, 2011-12-12 18:25:04]

On Mon, Dec 12, 2011 at 1:21 PM,  <wez@chromium.org> wrote:
> Fundamentally EKE just generates key data, so the API is useful provided the
> caller uses that appropriately; you are worried that callers won't do that?
>
> How would you feel about:
> * Replacing "yes/no" with "ready/failed" results.
> * Adding GenerateChallenge(secret) and VerifyResponse(response, secret)
> methods.
>
> The idea being that a single EKE exchange can then authenticate multiple
> channels.

I don't mean to delay you here. It's just that, with this change, the
API returns a yes/no value but doesn't take a session id as a
parameter. That's just wrong. If it's transient, LGTM, so ahead, just
put a warning in a comment about the need to integrate a session id
into the password field.

Later, when you change things to get a key out, you can take away the warning.


Cheers

AGL

[Sergey Ulanov, 2011-12-12 23:49:40]

I've added GetKey() method to this class and updated the comments to reflect the
fact that this class now implements key exchange but not channel authentication.

On 2011/12/12 18:25:04, agl wrote:
> On Mon, Dec 12, 2011 at 1:21 PM,  <mailto:wez@chromium.org> wrote:
> > Fundamentally EKE just generates key data, so the API is useful provided the
> > caller uses that appropriately; you are worried that callers won't do that?
> >
> > How would you feel about:
> > * Replacing "yes/no" with "ready/failed" results.
> > * Adding GenerateChallenge(secret) and VerifyResponse(response, secret)
> > methods.
> >
> > The idea being that a single EKE exchange can then authenticate multiple
> > channels.
> 
> I don't mean to delay you here. It's just that, with this change, the
> API returns a yes/no value but doesn't take a session id as a
> parameter. That's just wrong. If it's transient, LGTM, so ahead, just
> put a warning in a comment about the need to integrate a session id
> into the password field.
> 
> Later, when you change things to get a key out, you can take away the warning.
> 
> 
> Cheers
> 
> AGL

[commit-bot: I haz the power, 2011-12-13 05:24:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/8903001/9001

[commit-bot: I haz the power, 2011-12-13 07:00:21]

Change committed as 114189