New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment
link

Issue 571231: Implement Curve25519 for TLS

Reported by davidben@chromium.org, Dec 19 2015 Project Member

Issue description

See summary.
 

Comment 1 by bugdroid1@chromium.org, Dec 22 2015

Project Member
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/ab14563022da1d7da13543ee444f13b4e59eb4a5

commit ab14563022da1d7da13543ee444f13b4e59eb4a5
Author: David Benjamin <davidben@chromium.org>
Date: Sat Dec 19 02:15:22 2015

Bundle a copy of golang.org/x/crypto/curve25519 for testing.

Hopefully this can be replaced with a standard library version later.

BUG= 571231 

Change-Id: I61ae1d9d057c6d9e1b92128042109758beccc7ff
Reviewed-on: https://boringssl-review.googlesource.com/6776
Reviewed-by: Adam Langley <agl@google.com>

[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/const_amd64.s
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/cswap_amd64.s
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/curve25519.go
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/curve25519_test.go
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/doc.go
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/freeze_amd64.s
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/ladderstep_amd64.s
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/mont25519_amd64.go
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/mul_amd64.s
[add] http://crrev.com/ab14563022da1d7da13543ee444f13b4e59eb4a5/ssl/test/runner/curve25519/square_amd64.s

Comment 2 by bugdroid1@chromium.org, Dec 22 2015

Project Member
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/cba2b62a85215889285dfbe464f383397538760a

commit cba2b62a85215889285dfbe464f383397538760a
Author: David Benjamin <davidben@chromium.org>
Date: Sat Dec 19 03:13:41 2015

Implement draft-ietf-tls-curve25519-01 in Go.

This injects an interface to abstract between elliptic.Curve and a
byte-oriented curve25519. The C implementation will follow a similar
strategy.

Note that this slightly tweaks the order of operations. The client sees
the server public key before sending its own. To keep the abstraction
simple, ecdhCurve expects to generate a keypair before consuming the
peer's public key. Instead, the client handshake stashes the serialized
peer public value and defers parsing it until it comes time to send
ClientKeyExchange. (This is analogous to what it was doing before where
it stashed the parsed peer public value instead.)

BUG= 571231 

Change-Id: I771bb9aee0dd6903d395c84ec4f2dd7b3e366c75
Reviewed-on: https://boringssl-review.googlesource.com/6777
Reviewed-by: Adam Langley <agl@google.com>

[modify] http://crrev.com/cba2b62a85215889285dfbe464f383397538760a/ssl/test/runner/common.go
[modify] http://crrev.com/cba2b62a85215889285dfbe464f383397538760a/ssl/test/runner/key_agreement.go

Comment 3 by bugdroid1@chromium.org, Dec 22 2015

Project Member

Comment 4 by davidben@chromium.org, Dec 22 2015

Labels: M-50
Targeting M50. The implementation's ready, so that'll land in BoringSSL now, but it won't be in the default list of curves due to timing with holidays and branch point and such.

Comment 5 by bugdroid1@chromium.org, Dec 22 2015

Project Member
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/4298d773794dcfef675aeeacd970bcc8f2fe406d

commit 4298d773794dcfef675aeeacd970bcc8f2fe406d
Author: David Benjamin <davidben@chromium.org>
Date: Sat Dec 19 05:18:25 2015

Implement draft-ietf-tls-curve25519-01 in C.

The new curve is not enabled by default.

As EC_GROUP/EC_POINT is a bit too complex for X25519, this introduces an
SSL_ECDH_METHOD abstraction which wraps just the raw ECDH operation. It
also tidies up some of the curve code which kept converting back and
force between NIDs and curve IDs. Now everything transits as curve IDs
except for API entry points (SSL_set1_curves) which take NIDs. Those
convert immediately and act on curve IDs from then on.

Note that, like the Go implementation, this slightly tweaks the order of
operations. The client sees the server public key before sending its
own. To keep the abstraction simple, SSL_ECDH_METHOD expects to
generate a keypair before consuming the peer's public key. Instead, the
client handshake stashes the serialized peer public value and defers
parsing it until it comes time to send ClientKeyExchange. (This is
analogous to what it was doing before where it stashed the parsed peer
public value instead.)

It still uses TLS 1.2 terminology everywhere, but this abstraction should also
be compatible with TLS 1.3 which unifies (EC)DH-style key exchanges.
(Accordingly, this abstraction intentionally does not handle parsing the
ClientKeyExchange/ServerKeyExchange framing or attempt to handle asynchronous
plain RSA or the authentication bits.)

BUG= 571231 

Change-Id: Iba09dddee5bcdfeb2b70185308e8ab0632717932
Reviewed-on: https://boringssl-review.googlesource.com/6780
Reviewed-by: Adam Langley <agl@google.com>

[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/include/openssl/ssl.h
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/CMakeLists.txt
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/internal.h
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/s3_clnt.c
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/s3_lib.c
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/s3_srvr.c
[add] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/ssl_ecdh.c
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/ssl_lib.c
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/t1_lib.c
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/test/bssl_shim.cc
[modify] http://crrev.com/4298d773794dcfef675aeeacd970bcc8f2fe406d/ssl/test/runner/runner.go

Comment 6 by bugdroid1@chromium.org, Jan 20 2016

Project Member
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/d2f0ce80a2b994c8aa246ac5e64bb0c27eb6a25b

commit d2f0ce80a2b994c8aa246ac5e64bb0c27eb6a25b
Author: David Benjamin <davidben@chromium.org>
Date: Sat Jan 16 00:09:31 2016

Enable X25519 by default in TLS.

BUG= 571231 

Change-Id: I73e39411ccdc817f172c7a94b7f70c448eed938f
Reviewed-on: https://boringssl-review.googlesource.com/6911
Reviewed-by: Adam Langley <agl@google.com>

[modify] http://crrev.com/d2f0ce80a2b994c8aa246ac5e64bb0c27eb6a25b/ssl/t1_lib.c

Comment 7 by bugdroid1@chromium.org, Jan 21 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8dcde7eb270e9e917ebd0ebade754a8351d4db0b

commit 8dcde7eb270e9e917ebd0ebade754a8351d4db0b
Author: davidben <davidben@chromium.org>
Date: Thu Jan 21 04:15:38 2016

Roll src/third_party/boringssl/src afe57cb14..6c22f542f

https://boringssl.googlesource.com/boringssl/+log/afe57cb14d36f70ad4a109fc5e7765d1adc67035..6c22f542f42eccb0de7f43edf27aa1255c7be7e2

As this pulls in a new ECDH curve, add X25519 to histograms.xml. Also fix
OPENSSL_SMALL definition in GYP. direct_dependent_settings doesn't apply to the
current target (oops). It also no longer needs to be exported; no test or
header file depends on it anymore.

BUG= 571231 

Review URL: https://codereview.chromium.org/1608963004

Cr-Commit-Position: refs/heads/master@{#370617}

[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/DEPS
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/BUILD.gn
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/boringssl.gyp
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/boringssl.gypi
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/boringssl_tests.gypi
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/boringssl_unittest.cc
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/err_data.c
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-aarch64/crypto/aes/aesv8-armx64.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-aarch64/crypto/bn/armv8-mont.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-aarch64/crypto/modes/ghashv8-armx64.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-aarch64/crypto/sha/sha1-armv8.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-aarch64/crypto/sha/sha256-armv8.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-aarch64/crypto/sha/sha512-armv8.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/aes/aes-armv4.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/aes/aesv8-armx32.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/aes/bsaes-armv7.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/bn/armv4-mont.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/modes/ghash-armv4.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/modes/ghashv8-armx32.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/sha/sha1-armv4-large.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/sha/sha256-armv4.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/third_party/boringssl/linux-arm/crypto/sha/sha512-armv4.S
[modify] http://crrev.com/8dcde7eb270e9e917ebd0ebade754a8351d4db0b/tools/metrics/histograms/histograms.xml

Comment 8 by davidben@chromium.org, Jan 21 2016

Status: Fixed

Sign in to add a comment