Skip to content

Commit

Permalink
crypto/tls: support TLS_FALLBACK_SCSV as a server.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
agl committed Oct 16, 2014
1 parent 3c40ee0 commit e5624ed
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 2 deletions.
2 changes: 2 additions & 0 deletions src/crypto/tls/alert.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const (
alertProtocolVersion alert = 70
alertInsufficientSecurity alert = 71
alertInternalError alert = 80
alertInappropriateFallback alert = 86
alertUserCanceled alert = 90
alertNoRenegotiation alert = 100
)
Expand All @@ -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",
}
Expand Down
5 changes: 5 additions & 0 deletions src/crypto/tls/cipher_suites.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
12 changes: 12 additions & 0 deletions src/crypto/tls/handshake_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
27 changes: 25 additions & 2 deletions src/crypto/tls/handshake_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions src/crypto/tls/testdata/Server-TLSv11-FallbackSCSV
Original file line number Diff line number Diff line change
@@ -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|

0 comments on commit e5624ed

Please sign in to comment.