Subject: Re: [Curdle] AD Review of draft-ietf-curdle-gss-keyex-sha2-05

From: Simo Sorce <[email-masked]>
Date: Tue, 01 May 2018 15:08:07 -0400

On Wed, 2018-04-25 at 12:55 -0700, Eric Rescorla wrote:
> On Wed, Apr 25, 2018 at 11:53 AM, Hubert Kario <[email-masked]> wrote:
> 
> > I think you're misunderstanding me. My point is that there are already
> > > documents
> > > which describe how to generate the private and public keys for EC. You
> > > should
> > > be referring to them, not recapitulating their contents here.
> > 
> > proposed in https://github.com/simo5/ietf/pull/24
> 
> 
> I will review this.

Hi Eric,
did you have time to look at this ?
Or do you prefer to get a new draft with these changes ?

Simo.

---

From: Eric Rescorla <[email-masked]>
Date: Fri, 18 May 2018 14:09:46 -0700

On Thu, Apr 26, 2018 at 4:46 AM, Hubert Kario <[email-masked]> wrote:

> On Wednesday, 25 April 2018 21:55:21 CEST Eric Rescorla wrote:
> > > On Friday, 13 April 2018 15:41:37 CEST Eric Rescorla wrote:
> > > > On Thu, Apr 12, 2018 at 11:29 AM, Hubert Kario <[email-masked]>
> > >
>
> > > > > > >      This section defers to [RFC7546] as the source of
> information
> > >
> > > on
> > >
> > > > > GSS-
> > > > >
> > > > > > >      API context establishment operations, Section 3 being the
> > > > > > >      most
> > > > > > >      relevant.  All Security Considerations described in
> [RFC7546]
> > > > >
> > > > > apply
> > > > >
> > > > > > >      here too.
> > > > > >
> > > > > > >      The Client:
> > > > > > This section should be refactored to put all the EC mechanics
> (which
> > > > > > are symmetrical) in one place.
> > > > >
> > > > > I don't think I understand what changes you'd like to see
> > > > >
> > > > > both FFDH and ECDH are symmetrical... both client and server need
> to
> > > > > perform
> > > > > the same operations...
> > > >
> > > > Yes, That's why it's confusing to describe their operations in order
> > >
> > > rather
> > >
> > > > than
> > > > the behavior that a DH peer does and then just the points where they
> are
> > > > inserted in the protocol. Compare, for instance, the TLS 1.3
> > >
> > > specification,
> > >
> > > > where both KeyShare (https://tools.ietf.org/html/d
> > > > raft-ietf-tls-tls13-28#page-53) and
> > > > the DH computations (https://tools.ietf.org/html/d
> > > > raft-ietf-tls-tls13-28#section-7.4) are
> > > > described in an endpoint agnostic manner. DH is inherently
> symmetrical.
> > >
> > > the actions performed in context of GSSAPI-infused key exchange aren't
> >
> > Neither are they in TLS, but we managed to put the DH part in one place.
>
> That's not true:
> https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-7.4
> https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-4.2.8.1
> https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-4.2.8.2

My point was that they're not separated between Client and Server,

> > RFC 7748 says that the verification is optional, I don't see why making
it
> > mandatory is incorrect for SSH. For nist curves, the verification is
about
> > public key, not shared secret, it's just performed in the same step.
>
> I think we're talking past each other. The TLS spec requires that:
>
> - You do public key validation for FFDHE and NIST curves
> - You do output validation for CFRG curves.
>
> Unless I am misreading your text, you are requiring that you also do
output
> validation for the NIST curves. Is that correct? If so, can you provide a
> source for why?

http://www.secg.org/sec1-v2.pdf Section 3.3.1. step 2 of the "Calculate a
> shared secret value as follows" algorithm.
>

Sorry for the delay on this. I've been asking cryptographers. What I
understand
is that this is redundant for these NIST curves as long as you check
the input points, which we do. However, maybe for other curves it is a
problem.
I guess it's OK to leave here.

-Ekr

---

From: Eric Rescorla <[email-masked]>
Date: Fri, 18 May 2018 14:12:05 -0700

These changes look fine, though they only address some of my comments.

---

From: Simo Sorce <[email-masked]>
Date: Mon, 21 May 2018 13:27:26 -0400

> These changes look fine, though they only address some of my comments.

Just for clarity, is the change to describe the whole DH exchange in
one place what you see missing ? Is that a deal breaker ?
Anything else ?

Simo.

> > > On Wed, Apr 25, 2018 at 11:53 AM, Hubert Kario <[email-masked]>
> > 
> > wrote:
> > > 
> > > > > which describe how to generate the private and public keys for EC.
> > 
> > You
> > > > > should

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 29 May 2018 12:27:34 -0700

> one place what you see missing ? Is that a deal breaker ?

That and the repeated text that is the same for each group.

I'm not sure it's a dealbreaker, but it's a real point of concern for me.
Is there
strong WG consensus to retain the current structure, or is this just what
happens
to be there.

-Ekr

---

From: "Mark D. Baushke" <[email-masked]>
Date: Tue, 29 May 2018 13:37:24 -0700

Eric Rescorla <[email-masked]> writes:

> Is there strong WG consensus to retain the current structure, or is
> this just what happens to be there.

I do not believe that the question was raised in the Curdle Working Group.

If you think there should be a different structure, could you provide an
outline of what you would find most useful?

For this draft, I am just another member of the WG.

	-- Mark

---

From: Daniel Migault <[email-masked]>
Date: Tue, 29 May 2018 16:42:36 -0400

Hi,

The structure has not been raised nor discussed in the WG.

Yours,
Daniel

On Tue, May 29, 2018 at 4:37 PM, Mark D. Baushke <[email-masked]> wrote:

> _______________________________________________
> Curdle mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/curdle
>

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 29 May 2018 15:05:24 -0700

> outline of what you would find most useful?

Fair enough. Is this in Github? I can provide a sketchy PR.

-Ekr

---

From: "Salz, Rich" <[email-masked]>
Date: Wed, 30 May 2018 01:33:37 +0000

*   Fair enough. Is this in Github? I can provide a sketchy PR.

And the rationale, obviously.  Although that can (probably should) be PR metadata.

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 29 May 2018 21:09:23 -0700

On Tue, May 29, 2018 at 6:33 PM, Salz, Rich <
rsalz=[email-masked]> wrote:

>
>    - Fair enough. Is this in Github? I can provide a sketchy PR.
>
>
>

The rationale is in my review, but I'm happy to cut and paste from that.

-Ekr

---

From: Hubert Kario <[email-masked]>
Date: Wed, 30 May 2018 12:25:19 +0200

> That and the repeated text that is the same for each group.

the fix for repeated text about groups is already merged to master, for ECDHE: 
https://github.com/simo5/ietf/blob/eb03480d5c49e7340302e4974435f382ba26a911/
draft-ietf-curdle-gss-keyex-sha2.xml#L537-L553
and for FFDHE:
https://github.com/simo5/ietf/blob/eb03480d5c49e7340302e4974435f382ba26a911/
draft-ietf-curdle-gss-keyex-sha2.xml#L148-L164

> > > > > > I think you're misunderstanding me. My point is that there are
> > 
> > already
> > 
> > > > > > > documents
> > > > > > > which describe how to generate the private and public keys for
> > 
> > EC.
> > 
> > > > You
> > > > 
> > > > > > > should

-- 
Regards,
Hubert Kario
Senior Quality Engineer, QE BaseOS Security team
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkyňova 115, 612 00  Brno, Czech Republic

---

From: Simo Sorce <[email-masked]>
Date: Wed, 30 May 2018 08:35:12 -0400

> draft-ietf-curdle-gss-keyex-sha2.xml#L148-L164

Thanks Hubert.

Eric,
it be nice if you could give guidance on how you would like to see the
DH exchange explanation changed, assuming that's the only thing of
concern left for you, feel free to point out anything else as well.

If you prefer to have a new draft submitted, with the changes above, as
a baseline for further discussion, I can submit one.

Simo.

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 30 May 2018 06:24:10 -0700

Sure. I was thinking something like this

5.1.
5.1.1. Generic Procedures
- Generating a DH ephemeral
- Verifying the peer's ephemeral
- Computing the shared key
- Verifying the output
- Computing the key hash and MIC

5.1.2. GSS Key Exchange Steps
Client calls GSS_Init_sec_context()... [5.1.1 step 2]
Server verifies Q_C and calls GSS_Accept_sec_context [5.1.1, step 4]
Server generates ephemeral and computes the shared key [reduced 5.1.1 step
5]
Server call to GSS_Accept_sec_context [5.1.1 step 6]
Client verifies Q_S and computes the shared key [5.1.1 step 7++]
Client calls GSS_VErifyMIC [5.1.1. step 8]

It's hard for me to see if you caught everything else. If you want to
submit a new draft, I can take a look, either before or after this
change.

-Ekr

---

From: Simo Sorce <[email-masked]>
Date: Wed, 30 May 2018 10:24:52 -0400

> submit a new draft, I can take a look, either before or after this

Ok, I guess will try to get the change above in first; to cut on draft
churn.
in the meanwhile if you want to see the current text we are working on
it is available here[1], with commit history (so you can diff [2] for
the previous draft [3] if you wish).

Simo.

[1] https://github.com/simo5/ietf/blob/master/draft-ietf-curdle-gss-key
ex-sha2.xml
[2] https://github.com/simo5/ietf/compare/cbb10832e376e3abab3164f9bb7d2
e759c2bea0b...6057b9920920640f8f193160d00150890adc222b
[3] https://github.com/simo5/ietf/blob/cbb10832e376e3abab3164f9bb7d2e75
9c2bea0b/draft-ietf-curdle-gss-keyex-sha2.xml 

> > > > On Mon, May 21, 2018 at 10:27 AM, Simo Sorce <[email-masked]>
> > 
> > wrote:
> > > > > > These changes look fine, though they only address some of my
> > 
> > comments.
> > > > > 
> > > the fix for repeated text about groups is already merged to master, for
> > 
> > ECDHE:
> > > https://github.com/simo5/ietf/blob/eb03480d5c49e7340302e4974435f3
> > 
> > 82ba26a911/
> > > https://github.com/simo5/ietf/blob/eb03480d5c49e7340302e4974435f3
> > 
> > 82ba26a911/