]> Cypherpunks repositories - gostls13.git/commitdiff
crypto/tls: support TLS_FALLBACK_SCSV as a server.
authorAdam Langley <agl@golang.org>
Thu, 16 Oct 2014 00:54:04 +0000 (17:54 -0700)
committerAdam Langley <agl@golang.org>
Thu, 16 Oct 2014 00:54:04 +0000 (17:54 -0700)
A new attack on CBC padding in SSLv3 was released yesterday[1]. Go only
supports SSLv3 as a server, not as a client. An easy fix is to change
the default minimum version to TLS 1.0 but that seems a little much
this late in the 1.4 process as it may break some things.

Thus this patch adds server support for TLS_FALLBACK_SCSV[2] -- a
mechanism for solving the fallback problem overall. Chrome has
implemented this since February and Google has urged others to do so in
light of yesterday's news.

With this change, clients can indicate that they are doing a fallback
connection and Go servers will be able to correctly reject them.

[1] http://googleonlinesecurity.blogspot.com/2014/10/this-poodle-bites-exploiting-ssl-30.html
[2] https://tools.ietf.org/html/draft-ietf-tls-downgrade-scsv-00

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/157090043

src/crypto/tls/alert.go
src/crypto/tls/cipher_suites.go
src/crypto/tls/handshake_server.go
src/crypto/tls/handshake_server_test.go
src/crypto/tls/testdata/Server-TLSv11-FallbackSCSV [new file with mode: 0644]

index 0856311e4cba288461c0f83ba9d5c18ba99610fa..3de4834d3f5b83ec3165396788345bd6fe612c0b 100644 (file)
@@ -35,6 +35,7 @@ const (
        alertProtocolVersion        alert = 70
        alertInsufficientSecurity   alert = 71
        alertInternalError          alert = 80
+       alertInappropriateFallback  alert = 86
        alertUserCanceled           alert = 90
        alertNoRenegotiation        alert = 100
 )
@@ -60,6 +61,7 @@ var alertText = map[alert]string{
        alertProtocolVersion:        "protocol version not supported",
        alertInsufficientSecurity:   "insufficient security level",
        alertInternalError:          "internal error",
+       alertInappropriateFallback:  "inappropriate fallback",
        alertUserCanceled:           "user canceled",
        alertNoRenegotiation:        "no renegotiation",
 }
index 39a51459d288fc516248af612c6fabb0a8989258..226e06d68d63acb4b314b38210e747565a85f65c 100644 (file)
@@ -267,4 +267,9 @@ const (
        TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA      uint16 = 0xc014
        TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256   uint16 = 0xc02f
        TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 uint16 = 0xc02b
+
+       // TLS_FALLBACK_SCSV isn't a standard cipher suite but an indicator
+       // that the client is doing version fallback. See
+       // https://tools.ietf.org/html/draft-ietf-tls-downgrade-scsv-00.
+       TLS_FALLBACK_SCSV uint16 = 0x5600
 )
index 520675dfb5adca445c57dd6d1ff4a36307827719..0d907656c6c20cc0698baaaaa2db1ad2cede83b6 100644 (file)
@@ -224,6 +224,18 @@ Curves:
                return false, errors.New("tls: no cipher suite supported by both client and server")
        }
 
+       // See https://tools.ietf.org/html/draft-ietf-tls-downgrade-scsv-00.
+       for _, id := range hs.clientHello.cipherSuites {
+               if id == TLS_FALLBACK_SCSV {
+                       // The client is doing a fallback connection.
+                       if hs.clientHello.vers < c.config.MaxVersion {
+                               c.sendAlert(alertInappropriateFallback)
+                               return false, errors.New("tls: client using inppropriate protocol fallback")
+                       }
+                       break
+               }
+       }
+
        return false, nil
 }
 
index 580fbc0bfbdb1e6fb8e61acc9336b9bc9bdfd7c4..0338af457ee5f8bedccc9b24c646499fe32857be 100644 (file)
@@ -260,6 +260,9 @@ type serverTest struct {
        // expectAlert, if true, indicates that a fatal alert should be returned
        // when handshaking with the server.
        expectAlert bool
+       // expectHandshakeErrorIncluding, when not empty, contains a string
+       // that must be a substring of the error resulting from the handshake.
+       expectHandshakeErrorIncluding string
        // validate, if not nil, is a function that will be called with the
        // ConnectionState of the resulting connection. It returns false if the
        // ConnectionState is unacceptable.
@@ -362,9 +365,17 @@ func (test *serverTest) run(t *testing.T, write bool) {
        server := Server(serverConn, config)
        connStateChan := make(chan ConnectionState, 1)
        go func() {
-               if _, err := server.Write([]byte("hello, world\n")); err != nil {
+               var err error
+               if _, err = server.Write([]byte("hello, world\n")); err != nil {
                        t.Logf("Error from Server.Write: %s", err)
                }
+               if len(test.expectHandshakeErrorIncluding) > 0 {
+                       if err == nil {
+                               t.Errorf("Error expected, but no error returned")
+                       } else if s := err.Error(); !strings.Contains(s, test.expectHandshakeErrorIncluding) {
+                               t.Errorf("Error expected containing '%s' but got '%s'", test.expectHandshakeErrorIncluding, s)
+                       }
+               }
                server.Close()
                serverConn.Close()
                connStateChan <- server.ConnectionState()
@@ -429,7 +440,9 @@ func (test *serverTest) run(t *testing.T, write bool) {
                recordingConn.Close()
                if len(recordingConn.flows) < 3 {
                        childProcess.Stdout.(*bytes.Buffer).WriteTo(os.Stdout)
-                       t.Fatalf("Handshake failed")
+                       if len(test.expectHandshakeErrorIncluding) == 0 {
+                               t.Fatalf("Handshake failed")
+                       }
                }
                recordingConn.WriteTo(out)
                fmt.Printf("Wrote %s\n", path)
@@ -702,6 +715,16 @@ func TestResumptionDisabled(t *testing.T) {
        // file for ResumeDisabled does not include a resumption handshake.
 }
 
+func TestFallbackSCSV(t *testing.T) {
+       test := &serverTest{
+               name: "FallbackSCSV",
+               // OpenSSL 1.0.1j is needed for the -fallback_scsv option.
+               command: []string{"openssl", "s_client", "-fallback_scsv"},
+               expectHandshakeErrorIncluding: "inppropriate protocol fallback",
+       }
+       runServerTestTLS11(t, test)
+}
+
 // cert.pem and key.pem were generated with generate_cert.go
 // Thus, they have no ExtKeyUsage fields and trigger an error
 // when verification is turned on.
diff --git a/src/crypto/tls/testdata/Server-TLSv11-FallbackSCSV b/src/crypto/tls/testdata/Server-TLSv11-FallbackSCSV
new file mode 100644 (file)
index 0000000..2d8dfbc
--- /dev/null
@@ -0,0 +1,17 @@
+>>> Flow 1 (client to server)
+00000000  16 03 01 00 d4 01 00 00  d0 03 02 74 2d da 6d 98  |...........t-.m.|
+00000010  ad 3e a5 ec 90 ea d1 5b  f0 e0 a7 45 33 d9 5e 8d  |.>.....[...E3.^.|
+00000020  0f 1d 01 16 6d 00 31 65  ed 50 88 00 00 5e c0 14  |....m.1e.P...^..|
+00000030  c0 0a 00 39 00 38 00 88  00 87 c0 0f c0 05 00 35  |...9.8.........5|
+00000040  00 84 c0 13 c0 09 00 33  00 32 00 9a 00 99 00 45  |.......3.2.....E|
+00000050  00 44 c0 0e c0 04 00 2f  00 96 00 41 00 07 c0 11  |.D...../...A....|
+00000060  c0 07 c0 0c c0 02 00 05  00 04 c0 12 c0 08 00 16  |................|
+00000070  00 13 c0 0d c0 03 00 0a  00 15 00 12 00 09 00 14  |................|
+00000080  00 11 00 08 00 06 00 03  00 ff 56 00 01 00 00 49  |..........V....I|
+00000090  00 0b 00 04 03 00 01 02  00 0a 00 34 00 32 00 0e  |...........4.2..|
+000000a0  00 0d 00 19 00 0b 00 0c  00 18 00 09 00 0a 00 16  |................|
+000000b0  00 17 00 08 00 06 00 07  00 14 00 15 00 04 00 05  |................|
+000000c0  00 12 00 13 00 01 00 02  00 03 00 0f 00 10 00 11  |................|
+000000d0  00 23 00 00 00 0f 00 01  01                       |.#.......|
+>>> Flow 2 (server to client)
+00000000  15 03 02 00 02 02 56                              |......V|