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

Eric Rescorla <ekr@rtfm.com> Fri, 13 April 2018 13:42 UTC

Return-Path: <ekr@rtfm.com>
X-Original-To: curdle@ietfa.amsl.com
Delivered-To: curdle@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 90D10120721 for <curdle@ietfa.amsl.com>; Fri, 13 Apr 2018 06:42:23 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.909
X-Spam-Level:
X-Spam-Status: No, score=-1.909 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, T_DKIMWL_WL_MED=-0.01] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=rtfm-com.20150623.gappssmtp.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2L6aOPOHZd7U for <curdle@ietfa.amsl.com>; Fri, 13 Apr 2018 06:42:19 -0700 (PDT)
Received: from mail-ot0-x22a.google.com (mail-ot0-x22a.google.com [IPv6:2607:f8b0:4003:c0f::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 434971200F1 for <curdle@ietf.org>; Fri, 13 Apr 2018 06:42:19 -0700 (PDT)
Received: by mail-ot0-x22a.google.com with SMTP id 23-v6so9894482otj.0 for <curdle@ietf.org>; Fri, 13 Apr 2018 06:42:19 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rtfm-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=tflGHpnnhIq1/luf3hJ7LBdDwwyIjkaQFotppEWDUAo=; b=KBiL/qJqHMaEjDZcPLQrQeTriYM6bQ97mogGkuofb9GmTnKlBkEH1q+UklujcJd6wf 2TyFlwxMZN3rzuESPciv6IKCShkatPHHjgzRLzXdGqjv92McloMfXkJ3YM1+RvrT9/A8 nWWY+dk1+2eNiINiGPJe5BxUbKQSI5x7OdUutxqddej71BxKXZaHMQ7zWGVQ8RDPjf7P lGcxMqwpa7FJjiVxp6mxuhN4axEdtVyzvfuuwDWbw0aURw8/WG+ep+rUOpOowvNr19it nrxOTGkZMTxRUrivOdVd6BUqZ1pWxvrJWDd1u4yIs6cFIG5VB2iLPmOxuj+EZfuva61U Twhw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=tflGHpnnhIq1/luf3hJ7LBdDwwyIjkaQFotppEWDUAo=; b=Rb4jB6SBgLNBE6s4FgaXyyowCj6uMIZF4fUAE6VC2rSS+mj9Ys+W2W4lhQndCJxj6I 5bkpKBGNRGEArapEZPAUafux4jNsB43E719eJIyiEvFGITMnLuavpK6NCGH64OK58tyZ z1bQ48DqF6bhQKmStxZkFHKTa2kAJoa1ehhAZdizNAFkmjjr/RUiLKUCpEjNQ7fWR4LD XXlWcHtiQoEeTAGeor+Th7XmOkb/lF8Qw0rMZ3TbErCT6w/KyzAhwzHe0W1XPFDJTJpZ lvKh2cAOsIIieQPHBtioQzSTZMBngFoZeYW+cuMO9HfpjGVus0vbB1ipAOxTkI7Fr7Da N6Tg==
X-Gm-Message-State: ALQs6tCbisqEHsPZacLhddszUOpGO2C6ZWKGGrVdxONbBdjkn6fzMVBU CR/jaTb98Hej0ZkFOoCp2UDmHQUg/Rho7tRHHH1OFzyD
X-Google-Smtp-Source: AIpwx48yjpxRsF1ADNlZck1SRaFCkaXzln1UvNQBAx4cXMAOQO/WENNzqpf8bnfPUEfcJlpJtkbUvOKat/B5yvHp9q0=
X-Received: by 2002:a9d:26d4:: with SMTP id i20-v6mr3416983otd.214.1523626938373; Fri, 13 Apr 2018 06:42:18 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.138.18.130 with HTTP; Fri, 13 Apr 2018 06:41:37 -0700 (PDT)
In-Reply-To: <1555475.KUsr8aTfev@pintsize.usersys.redhat.com>
References: <CABcZeBNCUSpGihHz6bPBSALS4-34Tm7W36BCZ_Ev8OQz3KtVag@mail.gmail.com> <1555475.KUsr8aTfev@pintsize.usersys.redhat.com>
From: Eric Rescorla <ekr@rtfm.com>
Date: Fri, 13 Apr 2018 06:41:37 -0700
Message-ID: <CABcZeBP5LRFuH37166YMiXKce-GgJhnji_msYMrac=eQ531AMQ@mail.gmail.com>
To: Hubert Kario <hkario@redhat.com>
Cc: draft-ietf-curdle-gss-keyex-sha2@tools.ietf.org, curdle <curdle@ietf.org>
Content-Type: multipart/alternative; boundary="0000000000006eb2150569bb0a6c"
Archived-At: <https://mailarchive.ietf.org/arch/msg/curdle/ORr6o7igUy-U4-s5GcgZnhTts98>
Subject: Re: [Curdle] AD Review of draft-ietf-curdle-gss-keyex-sha2-05
X-BeenThere: curdle@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: "List for discussion of potential new security area wg." <curdle.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/curdle>, <mailto:curdle-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/curdle/>
List-Post: <mailto:curdle@ietf.org>
List-Help: <mailto:curdle-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/curdle>, <mailto:curdle-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 13 Apr 2018 13:42:23 -0000

On Thu, Apr 12, 2018 at 11:29 AM, Hubert Kario <hkario@redhat.com> wrote:

> Sorry for the delay in replying was swamped with other work, I should have
> no
> problem replying quickly now.
>
> On Saturday, 7 April 2018 01:24:27 CEST Eric Rescorla wrote:
> > 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
> > >          +--------------------------+--------------------------------+
> > >
> > >          | 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?
>
> I think that was already discussed at length - short version: mirroring of
> the
> regular (non-gss) version of those KEX methods
>

Yes, OK.



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

OK.


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

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.


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


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

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.


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

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.



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

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.



> > >         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.
>
> because debugging mismatch in key calculation is harder to do than
> rejection
> of a malformed message
>

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


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



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

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



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

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


> > >      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.
>
> same replay as with FFDHE methods
>

Yes. and I have the same objection.

-Ekr