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

From: Eric Rescorla <[email-masked]>
Date: Fri, 6 Apr 2018 16:24:27 -0700

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D4544

This document has a huge amount of duplicated material which makes it
very hard to read. Please refactor so that the common material is in
one place.

COMMENTS
>      Due to security concerns with SHA-1 [RFC6194] and with MODP groups
>      with less than 2048 bits [NIST-SP-800-131Ar1]  we propose the use of
>      the SHA-2 based hashes with DH group14, group15, group16, group17 and
>      group18 [RFC3526].  Additionally we add support for key exchange
>      based on Elliptic Curve Diffie Hellman with NIST P-256, P-384 and
>      P-521 as well as X25519 and X448 curves.  Following the rationale of

"the X25519..."

>          +--------------------------+--------------------------------+
>          | Key Exchange Method Name | Implementation Recommendations |
>          +--------------------------+--------------------------------+
>          | gss-group14-sha256-*     | SHOULD/RECOMMENDED             |
>          | gss-group15-sha512-*     | MAY/OPTIONAL                   |
>          | gss-group16-sha512-*     | SHOULD/RECOMMENDED             |

Why are you only specifying SHA-512 with 4096-bit groups. SHA-256 is
still reasonable at that size?

>
>      Each of these methods specifies GSS-API-authenticated Diffie-Hellman
>      key exchange as described in Section 2.1 of [RFC4462]  with SHA-256
>      as HASH, and the group defined in Section 8.2 of [RFC4253] The method
>      name for each method is the concatenation of the string "gss-
>      group14-sha256-" with the Base64 encoding of the MD5 hash [RFC1321]

Why is this MD5? Is there some legacy reason for this? It's not
necessarily bad but it's odd to modern eyes.

>      Each of these methods specifies GSS-API-authenticated Diffie-Hellman
>      key exchange as described in Section 2.1 of [RFC4462]  with SHA-512
>      as HASH, and the group defined in Section 7 of [RFC3526] The method
>      name for each method is the concatenation of the string "gss-
>      group18-sha512-" with the Base64 encoding of the MD5 hash of the
>      ASN.1 DER encoding of the underlying GSS-API mechanism's OID.

These all seem to be boilerplate. is there a way to refactor into a
single paragraph with a table that describes the substitutions?

>      ASN.1 DER encoding of the underlying GSS-API mechanism's OID.
>
>   5.  New Elliptic Curve Diffie-Hellman Key Exchange methods
>
>      In [RFC5656] new SSH key exchange algorithms based on Elliptic Curve
>      Cryptography are introduced.  We reuse much of section 4 to implement

s/implement/define/

>      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.

>            and then y coordinate.  The coordinate coversion MUST preserve
>            leading zero octets.  Thus for nistp521 curve the encoded x
>            coordinate will always have a length of 66 octets while the Q_C
>            representation will be 133 octets long.  This is the
>            uncompressed representation specified in Section 4.3.6 of
>            [ANSI-X9-62-2005].

I have two questions about this:  1. Why are you specifying the
detailed computation of the public key? This seems like you could
defer it to another spec. 2. Why are you specifying uncompressed
representations for NIST curves? We did this in TLS because people
already supported them, but in general they are worse. Is there a
reason here?

>            by 31 zero octets for curve255519 and as an octect of value
>            0x05 followed by 55 zero octets.
>
>            Calculating Q_C as the result of the call to X25519 or X448
>            function, respectively for curve25519 and curve448 key
>            exchange, with parameters d_C and g.

This material all seems to be in RFC 7748 S 6.1 and 6.2.

>
>         For NIST curves, the server verifies that the q_C is not a point
>         at infinity, that both coordinates are in the interval [0, p - 1],
>         where p is the prime associated with the curve of the selected key
>         exchange and that the point lies on the curve (satisfies the curve
>         equation).

You should probably cite to the X9.62 or SP-800 for this procedure.

>         For curve25519, the server verifies that the the high-order bit of
>         the last octet is not set - this prevents distinguishing attacks
>         between implementations that use Montgomery ladder implementation
>         of the curve and ones that use generic elliptic-curve libraries.
>         If the bit is set, the key exchange SHOULD fail.  For curve448 any
>         bit can be set.

I'm not following what this is supposed to do. If you are worried
about this, why don't you just mask off the top bit.

>            For NIST curves, the peers perform point multiplication using
>            d_U and q_V to get point P.
>
>            For NIST curves, peers verify that P is not a point at
>            infinity.  If P is a point at infinity, the key exchange MUST
>            fail.

Why is this text here? It describes the client's behavior.

>            and q_V.  The result of the function is the shared secret.
>
>            For curve25519 and curve448, if all the octets of the shared
>            secret are zero octets, the key exchange MUST fail.
>
>         H = hash(V_C || V_S || I_C || I_S || K_S || Q_C || Q_S || K).

This kind of just comes out of nowhere. You probably want some
prefatory text.

>
>      7.  C verifies that the key Q_S is valid the same way it is done in
>      step 3.  If the key is not valid the key exchange MUST fail.
>
>      8.  C computes the shared secret K and H and verifies that it is
>      valid the same way it is done in step 5.  It then calls

This check only applies to CFRG curves.

>          string    server public host key and certificates (K_S)
>
>      Since this key exchange method does not require the host key to be
>      used for any encryption operations, this message is OPTIONAL.  If the
>      "null" host key algorithm described in Section 5 of [RFC4462] is
>      used, this message MUST NOT be sent.

I am assuming in this situation there is some other form of
authentication?

>          string    I_C, payload of the client's SSH_MSG_KEXINIT
>          string    I_S, payload of the server's SSH_MSG_KEXINIT
>          string    K_S, server's public host key
>          string    Q_C, client's ephemeral public key octet string
>          string    Q_S, server's ephemeral public key octet string
>          mpint     K,   shared secret

The actual equation is way up above this in the document, which is
presumably not great.

>      Each key exchange method is implicitly registered by this document.
>      The IESG is considered to be the owner of all these key exchange
>      methods; this does NOT imply that the IESG is considered to be the
>      owner of the underlying GSS-API mechanism.
>
>   5.2.1.  gss-nistp256-sha256-*

Again, can you refactor this section so it's not so duplicative.

>      the target the user intended.  Some mechanisms implementations (like
>      commonly used krb5 libraries) may use insecure DNS resolution to
>      canonicalize the target name; in these cases spoofing a DNS response
>      that points to an attacker-controlled machine may results in the user
>      silently delegating credentials to the attacker, who can then
>      impersonate the user at will.

Is this something new in this document?

---

From: denis bider <[email-masked]>
Date: Fri, 6 Apr 2018 21:52:50 -0500

Eric:

I'm not an author of this draft, but I can respond with respect to the
following:

> > | gss-group14-sha256-*     | SHOULD/RECOMMENDED
> > | gss-group15-sha512-*     | MAY/OPTIONAL
> > | gss-group16-sha512-*     | SHOULD/RECOMMENDED
>
> Why are you only specifying SHA-512 with 4096-bit groups.
> SHA-256 is still reasonable at that size?

There exist NSA recommendations aimed at "organizations that run classified
or unclassified national security systems (NSS) and vendors that build
products used in NSS."

https://cryptome.org/2016/01/CNSA-Suite-and-Quantum-Computing-FAQ.pdf

These recommendations cover a usage case for software that implements the
above algorithms. These recommendations call for the following minimums:

- Diffie Hellman: 3072-bit or larger

- Hashing: SHA-384 or larger

These recommendations are most effectively met by associating group15 and
group16 with SHA-512.

Otherwise, products that wanted to meet these recommendations would have to
use much larger and more expensive DH groups in order to meet the
SHA-384-or-better requirement.

> Why is this MD5? Is there some legacy reason for this?

Yes, legacy versions of these algorithms use MD5. This has no security
implications and the hash can be statically encoded in the implementation.
The implementation does not have to implement MD5 at run-time.

If a different hash is chosen for new algorithms, MD5 is still in place for
older algorithms, so applications would have to hardcode both the MD5 and
newer hashes. There does not appear to be a compelling reason for this
since there is no security benefit from using a stronger hash.

> Why are you specifying uncompressed representations for NIST curves?

If the author is aiming for consistency with use of ECC in SSH, it would be
consistent to allow, but not require, compressed representation.

Precedent in RFC 5656, "Elliptic Curve Algorithm Integration in the Secure
Shell Transport Layer":

   Q is the public key encoded from an elliptic curve point into an
   octet string as defined in Section 2.3.3 of [SEC1
<https://tools.ietf.org/html/rfc5656#ref-SEC1>]; point compression
   MAY be used.

https://tools.ietf.org/html/rfc5656#section-3.1

> > Since this key exchange method does not require the host key
> > to be used for any encryption operations, this message is
> > OPTIONAL.  If the "null" host key algorithm described in
> > Section 5 of [RFC4462] is used, this message MUST NOT be sent.
>

Yes, the purpose of all of these GSS key exchange methods is to allow host
authentication to be performed through some external mechanism via GSSAPI.
Kerberos is a common usage case. When used with these key exchange methods
it can authenticate the host to the client without the use of SSH host keys.

The other issues seem relevant to raise.

On Fri, Apr 6, 2018 at 6:24 PM, Eric Rescorla <[email-masked]> wrote:

> >      the target the user intended..  Some mechanisms implementations
> (like
> _______________________________________________
> Curdle mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/curdle
>
>

---

From: Eric Rescorla <[email-masked]>
Date: Fri, 6 Apr 2018 20:03:15 -0700

On Fri, Apr 6, 2018 at 7:52 PM, denis bider <[email-masked]>
wrote:

> There exist NSA recommendations aimed at "organizations that run
> classified or unclassified national security systems (NSS) and vendors that
> build products used in NSS."
>
> Otherwise, products that wanted to meet these recommendations would have
> to use much larger and more expensive DH groups in order to meet the

My question is why these groups are only specified with SHA-512, and not
SHA-256. I think it's fine to specify them for SHA-512.

-Ekr

---

From: denis bider <[email-masked]>
Date: Fri, 6 Apr 2018 22:07:23 -0500

Well, to avoid algorithm spam. There exist objections to have as many
algorithms as we do, to begin with.

The added value of a SHA256 version when there already exists a SHA512
version for the same DH group appears particularly small.

---

From: Eric Rescorla <[email-masked]>
Date: Sat, 7 Apr 2018 07:19:23 -0700

> version for the same DH group appears particularly small.

The value here would be allowing you to have a 4096-bit DH group without
having to also have a SHA-512 implementation.

I recognize that the NSA recommendations you point to say you should use
SHA-512, however

(a) NSA recommendations don't have any special authority in IETF
(b) they say you should use SHA-512 for *integrity* but assuming I
understand these documents correctly, here H is being used as a KDF, so
it's not clear that the same reasoning applies (especially given that the
document you cite doesn't provide much in the way of reasoning).

-Ekr

---

From: "Salz, Rich" <[email-masked]>
Date: Sat, 7 Apr 2018 14:23:24 +0000

I see no point in adding another hash.  What’s the reason?

---

From: Russ Housley <[email-masked]>
Date: Sat, 7 Apr 2018 11:10:55 -0400

Denis:

> I'm not an author of this draft, but I can respond with respect to the following:
> 
> There exist NSA recommendations aimed at "organizations that run classified or unclassified national security systems (NSS) and vendors that build products used in NSS."
> 
> https://cryptome.org/2016/01/CNSA-Suite-and-Quantum-Computing-FAQ.pdf <https://cryptome.org/2016/01/CNSA-Suite-and-Quantum-Computing-FAQ.pdf>
> 
> These recommendations cover a usage case for software that implements the above algorithms. These recommendations call for the following minimums:
> 
> These recommendations are most effectively met by associating group15 and group16 with SHA-512.
> 
> Otherwise, products that wanted to meet these recommendations would have to use much larger and more expensive DH groups in order to meet the SHA-384-or-better requirement.

I do not quite understand this response.  I can understand a desire to include a ciphersuite that aligns with the NSA guidance so that anyone that needs to follow it can easily do so, but that does not seem to be what you are suggesting.  Looking at page 2 of the document that you cite, a ciphersuite that uses the NIST P-384 curve, SHA-384, and AES-256 is needed.

I would be in favor os adding such a ciphersuite as MAY/OPTIONAL.

Russ

---

From: Mark Baushke <[email-masked]>
Date: Sat, 7 Apr 2018 16:59:08 +0000

> On Apr 7, 2018, at 8:10 AM, Russ Housley <[email-masked]> wrote:
> 
> Denis:
> 

The key exchange ECDH NISTP-384 with SHA-384 is already in RFC 5656 along with ECDSA NISTP-384 with SHA-384 as a public key algorithm.

AES256-CBC is OPTIONAL in RFC4250 and RFC4253 as a symmectric cipher.

AES256-CTR is RECOMMENDED in RFC4344 as a symmectric cipher.

RFC3526 groups are provided for SSH in RFC8268:

group14 with sha256 is SHOULD/RECOMMENDED in RFC8268

group15 with sha512 is OPTIONAL in RFC8268

group16 with sha512 is OPTIONAL in RFC8268

group17 with sha512 is OPTIONAL in RFC8268

group18 with sha512 is OPTIONAL in RFC8268

Should we have decomposed the group with the hash? Or, should we have used shorter key exchange algorithm names? 
Well, perhaps both would have been better, but that should have been done some years ago.

The negotation list is getting a bit long these days and the SSH implementors did not seem to fancy adding SHA256, SHA384, and SHA512 variations of each of the new diffie-hellman groups being added, so the longest one (SHA512) was chosen as it slightly faster to implement on 64-bit architecture machines.

I am given to understand that NIST SP 800-56A revision 3 and FIPS PUB 186-5 may address the use of Finite Field Cryptography safe primes for Diffie-Hellman key exchange sometime this year. (see also URL:
https://csrc.nist.gov/CSRC/media/Publications/sp/800-56a/rev-3/draft/documents/sp800-56ar3-draft.pdf
section 5.5.1 and Appendix E which allow RFC3526 and RFC7919 groups)

For myself, I tend to think that using 2048-bit prime P FFC Diffie-Hellman provable primes that are generated on a bi-weekly basis with Q as a large subgroup of approximately P/2 bits in length and G is a proper generator of q-ordered subgroups. If changed often enough, then there would be no big push to try to precalcualte the majority of the factors of such parameters. Even if we have sufficient sieves to help in factorization, there is still a lot of computation behind breaking a large subgroup. In a Post-Quantum Cryptography world, I tend to trust large FFC calculations over small ECC calculations. Although something better than either would be welcome... always allowing it is desirable to still be able to connect to small embedded devices too.

With RFC4419 and RFC8270 we get safe primes generated with P being a 2048-bits or larger prime. The only downside is that the example calcuation does not always choose a generator g which generates a q-ordered subgroup.

> I would be in favor os adding such a ciphersuite as MAY/OPTIONAL.

I think this may already be present, depending on what you are asking.

For myself, I see no problems with the GSS variations of diffie-hellman using the same groups and hashes as RFC8268.
For gss-group14-sha256-* I support RECOMMENDED to move away from the SHA1 variation. I am okay with
gss-group16-sha512-* as either OPTIONAL or RECOMMENDED.

        -- Mark

---

From: Eric Rescorla <[email-masked]>
Date: Sat, 7 Apr 2018 16:38:21 -0700

> I see no point in adding another hash.  What’s the reason?

So that you can have an implementation that spans a variety of groups with
just SHA-256.

---

From: denis bider <[email-masked]>
Date: Sat, 7 Apr 2018 21:19:22 -0500

I think this misunderstands the scope of the GSS draft.

The GSS draft discusses GSS key exchange methods for SSH. These are not the
main key exchange methods for SSH, they are a *version* of the main key
exchange methods for use with GSS.

Because these are a *version* of the main key exchange methods, the main
value of this draft is that it updates the GSS key exchange methods to be
in line with how the main key exchange methods for SSH have evolved.

As Mark has pointed out, we have already added these key exchange methods
to SSH in RFC 8268:

diffie-hellman-group14-sha256
diffie-hellman-group15-sha512
diffie-hellman-group16-sha512
diffie-hellman-group17-sha512
diffie-hellman-group18-sha512

The whole argument about which groups and which hashes we should have has
already been staged, multiple times, and this is what we arrived at.

The GSS draft is merely mirroring what we have already done for use with
the GSS key exchange methods. It does not make sense for the GSS key
exchange methods to do something entirely different than what the main key
exchange methods do. It makes sense for the GSS key exchange methods to be
consistent with what we have already done for the main ones.

---

From: Eric Rescorla <[email-masked]>
Date: Sun, 8 Apr 2018 06:59:49 -0700

On Sat, Apr 7, 2018 at 7:19 PM, denis bider <[email-masked]>
wrote:

> The GSS draft discusses GSS key exchange methods for SSH. These are not
> the main key exchange methods for SSH, they are a *version* of the main
> key exchange methods for use with GSS.
>

OK. I regret not noticing this when I reviewed the SSH document, but as you
say, it's too late now.

-Ekr

>> So that you can have an implementation that spans a variety of groups
>> with just SHA-256.
>>
>>>
>>>
>>
>>
>

---

From: Simo Sorce <[email-masked]>
Date: Mon, 09 Apr 2018 15:31:58 -0400

Chiming in as one of the Authors, sorry for not responding earlier, I've bene
busy and I will follow up with more answers later on as I ge tmore time to
address them.

> very hard to read. Please refactor so that the common material is in

In general we strived to maintain the same structure as the docuemnt we are
extending. This is why some sections seem repetitive. They are but they also
are consistent with the previous document. We think this is more beneficial
than trying to be more synthetic.

> >      P-521 as well as X25519 and X448 curves.  Following the rationale of

Does this imply it should also be "the NIST P-256, ..." ?

> still reasonable at that size?

I thik this has been sactisfactorily discussed later in the thread.
 
> necessarily bad but it's odd to modern eyes.

Also discussed later, this is just an identifier, and we a re just being
consistent with previous naming which can be hardcoded, no need to implement
MD5.

> single paragraph with a table that describes the substitutions?

See my explanantion above.

> are symmetrical) in one place.

Is there a reason? We wanted to give implementers a clear linear explanation
without having to jump back and forth to reference what is happening at any
given point.

> defer it to another spec.

I think this is because we did not have any good normative reference, but I'll
let Hubert chime in.

> 2. Why are you specifying uncompressed
> representations for NIST curves?

I may be wrong but some compression algorithms have had IPR issues, so this may
derive from a willingness to avoid those, I'll let Hubert chime in here too.

> We did this in TLS because people

I think we wanted it explicitly here, to avoid too much referencing and risk of
errors, but I guess we can reference if you feel strongly about this.
 
> I am assuming in this situation there is some other form of

As explained elswhere, GSS key exchange assumes GSSAPI authentication.

> Is this something new in this document?

No, we just thought important to note, as it was missing in the original
document but it is a know factor to be aware of when dealing with Krb5 which is
de facto the only used GSSAPI mechanism for SHH GSS Key exchanges.

Simo.

-- 
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

---

From: denis bider <[email-masked]>
Date: Tue, 10 Apr 2018 03:02:35 -0500

> Krb5 which is de facto the only used GSSAPI mechanism for
> SHH GSS Key exchanges.

Just a side note: "de facto" are key words there. In practice Kerberos is
the most widely supported GSSAPI mechanism used in SSH. However, GSSAPI key
exchange works equally well with other mechanisms. For example, I believe
we've had someone (either on this list, or on the mostly-zombie SSH list)
mention that they use GSSAPI with X.509 (possibly?) as well as other
mechanisms.

---

From: Simo Sorce <[email-masked]>
Date: Tue, 10 Apr 2018 08:04:35 -0400

> mention that they use GSSAPI with X.509 (possibly?) as well as other

Yes, there are other mechanisms, but the warning applies to the Krb5 mechanism
explicitly.

Simo.
 
> > > >      with less than 2048 bits [NIST-SP-800-131Ar1]  we propose the use
> > 
> > of
> > > >      the SHA-2 based hashes with DH group14, group15, group16, group17
> > 
> > and
> > > >      P-521 as well as X25519 and X448 curves.  Following the rationale
> > 
> > of
> > > 
> > > "the X25519..."
> > 
> > > >      Each of these methods specifies GSS-API-authenticated
> > 
> > Diffie-Hellman
> > > >      key exchange as described in Section 2.1 of [RFC4462]  with
> > 
> > SHA-256
> > > >      as HASH, and the group defined in Section 8.2 of [RFC4253] The
> > 
> > method
> > > >      group14-sha256-" with the Base64 encoding of the MD5 hash
> > 
> > [RFC1321]
> > > 
> > > >      Each of these methods specifies GSS-API-authenticated
> > 
> > Diffie-Hellman
> > > >      key exchange as described in Section 2.1 of [RFC4462]  with
> > 
> > SHA-512
> > > >      as HASH, and the group defined in Section 7 of [RFC3526] The
> > 
> > method
> > > >      In [RFC5656] new SSH key exchange algorithms based on Elliptic
> > 
> > Curve
> > > >      Cryptography are introduced.  We reuse much of section 4 to
> > 
> > implement
> > > 
> > > s/implement/define/
> > > 
> > > 
> > > >      This section defers to [RFC7546] as the source of information on
> > 
> > GSS-
> > > >      relevant.  All Security Considerations described in [RFC7546]
> > 
> > apply
> > > >      here too.
> > > > 
> > > >      The Client:
> > > 
> > > >            and then y coordinate.  The coordinate coversion MUST
> > 
> > preserve
> > > >            coordinate will always have a length of 66 octets while the
> > 
> > Q_C
> > > >         For NIST curves, the server verifies that the q_C is not a
> > 
> > point
> > > >         at infinity, that both coordinates are in the interval [0, p -
> > 
> > 1],
> > > >         where p is the prime associated with the curve of the selected
> > 
> > key
> > > >         exchange and that the point lies on the curve (satisfies the
> > 
> > curve
> > > >         equation).
> > > 
> > > >         For curve25519, the server verifies that the the high-order
> > 
> > bit of
> > > >         the last octet is not set - this prevents distinguishing
> > 
> > attacks
> > > >         between implementations that use Montgomery ladder
> > 
> > implementation
> > > >         of the curve and ones that use generic elliptic-curve
> > 
> > libraries.
> > > >         If the bit is set, the key exchange SHOULD fail.  For curve448
> > 
> > any
> > > >         bit can be set.
> > > 
> > > >            For NIST curves, the peers perform point multiplication
> > 
> > using
> > > >            infinity.  If P is a point at infinity, the key exchange
> > 
> > MUST
> > > >            fail.
> > > 
> > > >      7.  C verifies that the key Q_S is valid the same way it is done
> > 
> > in
> > > >      used for any encryption operations, this message is OPTIONAL.  If
> > 
> > the
> > > >      Each key exchange method is implicitly registered by this
> > 
> > document.
> > > >      the target the user intended.  Some mechanisms implementations
> > 
> > (like
> > > >      canonicalize the target name; in these cases spoofing a DNS
> > 
> > response
> > > >      that points to an attacker-controlled machine may results in the
> > 
> > user

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 10 Apr 2018 08:01:13 -0500

> mention that they use GSSAPI with X.509 (possibly?) as well as other

I had almost replied with a similar note, so thanks -- this prompted
me to actually find a link for the "GSI" mechanism I remember
hearing about:

http://toolkit.globus.org/toolkit/docs/latest-stable/gsic/
and as used in ssh:
http://grid.ncsa.illinois.edu/ssh/

-Ben

---

From: Hubert Kario <[email-masked]>
Date: Thu, 12 Apr 2018 20:29:28 +0200

Sorry for the delay in replying was swamped with other work, I should have no 
problem replying quickly now.

> still reasonable at that size?

I think that was already discussed at length - short version: mirroring of the 
regular (non-gss) version of those KEX methods
 
> necessarily bad but it's odd to modern eyes.

it has no impact on security, it's used only to limit length of the method 
name and because MD5 was always used for that

> single paragraph with a table that describes the substitutions?

while valid point, it follows the style established in RFC 4462

for a programmer familiar with the old gss KEX methods and general IETF 
terminology, the names of the new algorithms alone are sufficient to implement 
them

for a programmer just learning it, it's sufficiently detailed to hand-hold the 
implementation process (and resolve disputes in case of minor differences)

> 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...

> defer it to another spec.

because this allows to verify the sanity of the share when it is being parsed, 
before the cryptographic routines are processing it, limiting the attack area
(and the implementation may want to handle a clearly malformed key share
and a invalid-value malformed key share differently).

> already supported them, but in general they are worse. Is there a

same reason – the uncompressed is the only format that is guaranteed to be 
supported and there is no mechanism in SSH to negotiate the supported formats

e.g. the implementation shipped in Red Hat Enterprise Linux does not support 
compressed representations

since it's also the same (and only) format used in TLS 1.3, it allows better 
cryptographic code reuse

(as Simo pointed out, because the standards allow for compressed 
representation, neither is explicit in expected sizes of serialised point 
representation so they are listed here, also as a simple sanity check to 
perform)

> This material all seems to be in RFC 7748 S 6.1 and 6.2.

we do need local nomenclature for the inputs and outputs though

also, having all the necessary checks in a single document allows for easier 
code review and verification if they are performed.

I opted for verbosity in the name of security. RFC 7748 is already referenced 
in the definitions of the methods.

> You should probably cite to the X9.62 or SP-800 for this procedure.

+1

> about this, why don't you just mask off the top bit.

because debugging mismatch in key calculation is harder to do than rejection 
of a malformed message

> Why is this text here? It describes the client's behavior.

both client and server need to perform that operation
 
> This kind of just comes out of nowhere. You probably want some

good point
 
> This check only applies to CFRG curves.

no, for CFRG curves the invalid value is a point at infinity, for X25519 
invalid value is an all-zero string

so the check if the shared secret is valid must be performed irrespective of 
curve used
 
> I am assuming in this situation there is some other form of

yes, Kerberos is not the sole implementation of GSS API
 
> The actual equation is way up above this in the document, which is

 1. those are equivalent
 2. this approach is inherited from RFC 4462; intention being to quickly show 
    no difference to the underlying inputs (for FFDHE only the hash, prime 
    and generators are changed)
 
> Again, can you refactor this section so it's not so duplicative.

same replay as with FFDHE methods

> Is this something new in this document?

Yes, those are additional security considerations, from discussions with Ben 
on Curve-WG ML.

-- 
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: Eric Rescorla <[email-masked]>
Date: Fri, 13 Apr 2018 06:41:37 -0700

> regular (non-gss) version of those KEX methods

Yes, OK.

> name and because MD5 was always used for that

OK.

> implementation process (and resolve disputes in case of minor differences)

Well, I'm a pretty experienced programmer, and I find it pretty hard to
follow.
It's exactly this kind of boilerplate that leads to confusion.

> both FFDH and ECDH are symmetrical... both client and server need to

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.

> and a invalid-value malformed key share differently).

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.

> since it's also the same (and only) format used in TLS 1.3, it allows

Well, in TLS 1.3 this was an unfortunate concession to backward
compatibility
with a very widely deployed installed base. I'm trying to determine if
that's true
here. As for negotiation, this can be fixed by creating a new code point.

> code review and verification if they are performed.

It's also an opportunity for new mistakes to be made as these documents are
less thoroughly reviewed than RFC 7748, as well as having two normative
specifications for ostensible the same algorithm, which we try not to do.
Please
defer the algorithms to the original sources.

> because debugging mismatch in key calculation is harder to do than

This is explicitly justified as "preventing distinguishing attacks". How
does it do so?

> both client and server need to perform that operation

Yes, that's why it's very confusing to have it in the middle of the
server's operations.

> so the check if the shared secret is valid must be performed irrespective

Hmm... TLS 1.3 does not specify that one must validate the output of the DH
computation. So, the IETF should be consistent on this point. If you think
that
TLS 1.3 is wrong, please explain why.

Second, the specific check you are requiring for the CFRG curves is the
one applicable if you do the recommended DH computations. Here's the
relevant text for TLS 13.

   For X25519 and X448, implementations SHOULD use the approach
   specified in [RFC7748 <https://tools.ietf.org/html/rfc7748>] to
calculate the Diffie-Hellman shared secret.
   Implementations MUST check whether the computed Diffie-Hellman shared
   secret is the all-zero value and abort if so, as described in
   Section 6 of [RFC7748]
<https://tools.ietf.org/html/rfc7748#section-6>.  If implementors use
an alternative
   implementation of these elliptic curves, they SHOULD perform the
   additional checks specified in Section 7 of [RFC7748]
<https://tools.ietf.org/html/rfc7748#section-7>.

>     and generators are changed)

I'll take a look at your revision and see what I think.

> same replay as with FFDHE methods

Yes. and I have the same objection.

-Ekr

---

From: denis bider <[email-masked]>
Date: Fri, 13 Apr 2018 21:51:14 -0500

For the record, I think most of Eric's recommendations here are very
sensible.

On Fri, Apr 13, 2018 at 8:41 AM, Eric Rescorla <[email-masked]> wrote:

> Yes, That's why it's confusing to describe their operations in order
> rather than
> Well, in TLS 1..3 this was an unfortunate concession to backward
> compatibility
>    specified in [RFC7748 <https://tools.ietf.org/html/rfc7748>] to calculate the Diffie-Hellman shared secret.
>    Section 6 of [RFC7748] <https://tools.ietf.org/html/rfc7748#section-6>.  If implementors use an alternative
>    additional checks specified in Section 7 of [RFC7748] <https://tools.ietf.org/html/rfc7748#section-7>.
>
>
>

---

From: Hubert Kario <[email-masked]>
Date: Wed, 25 Apr 2018 20:53:57 +0200

> It's exactly this kind of boilerplate that leads to confusion.

done

> described in an endpoint agnostic manner. DH is inherently symmetrical.

the actions performed in context of GSSAPI-infused key exchange aren't
 
> be referring to them, not recapitulating their contents here.

proposed in https://github.com/simo5/ietf/pull/24
 
> here. As for negotiation, this can be fixed by creating a new code point.

or can be fixed by mandating the most-widely supported format, and if somebody 
is interested in supporting compressed, he or she can propose that extension
 
> defer the algorithms to the original sources.

https://github.com/simo5/ietf/pull/24

> This is explicitly justified as "preventing distinguishing attacks". How

removed

> Yes, that's why it's very confusing to have it in the middle of the

the client part is referencing the server part
 
> <https://tools.ietf.org/html/rfc7748#section-7>.

Section 4.2.8.1:

   Peers MUST validate each other's public key Y by ensuring that 1 < Y
   < p-1.  This check ensures that the remote peer is properly behaved
   and isn't forcing the local system into a small subgroup.

and section 4.2.8.2 of draft-28:

   For the curves secp256r1, secp384r1 and secp521r1, peers MUST
   validate each other's public value Q by ensuring that the point is a
   valid point on the elliptic curve.  The appropriate validation
   procedures are defined in Section 4.3.7 of [X962] and alternatively
   in Section 5.6.2.3 of [KEYAGREEMENT].  This process consists of three
   steps: (1) verify that Q is not the point at infinity (O), (2) verify
   that for Q = (x, y) both integers x and y are in the correct
   interval, (3) ensure that (x, y) is a correct solution to the
   elliptic curve equation.  For these curves, implementers do not need
   to verify membership in the correct subgroup.

seem to me to be more relevant and quite detailed.

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.

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 25 Apr 2018 12:55:21 -0700

On Wed, Apr 25, 2018 at 11:53 AM, Hubert Kario <[email-masked]> wrote:

> > > Sorry for the delay in replying was swamped with other work, I should
> have
> > > no
> > > for a programmer just learning it, it's sufficiently detailed to
> hand-hold
> > > the
> rather

Neither are they in TLS, but we managed to put the DH part in one place.

> proposed in https://github.com/simo5/ietf/pull/24

I will review this.

> is interested in supporting compressed, he or she can propose that

Yes, but then you have both.

> the client part is referencing the server part

Yes, that's why I said you should put all the DH operations in one place.

> > > so the check if the shared secret is valid must be performed
> irrespective
> > > of
> > > curve used
> >

> 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?

-Ekr

---

From: Hubert Kario <[email-masked]>
Date: Thu, 26 Apr 2018 13:46:24 +0200

On Wednesday, 25 April 2018 21:55:21 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-
> > > > 
> > > inserted in the protocol. Compare, for instance, the TLS 1.3
> > 
> > specification,
> > 

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

> > > > > >      7.  C verifies that the key Q_S is valid the same way it is
> > 
> > done
> > 
> > > > in
> > > > 
> > > > > >      8.  C computes the shared secret K and H and verifies that it
> > 
> > is
> > 
> > > > no, for CFRG curves the invalid value is a point at infinity, for
> > 
> > X25519
> > 
> > > Hmm... TLS 1.3 does not specify that one must validate the output of the
> > 
> > DH
> > 
> > > computation. So, the IETF should be consistent on this point. If you
> > 
> > think
> > 
> > > that

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