crypto/tls: cannot send TLS_FALLBACK_SCSV

[SlyMarbo]

version: go1.4.1

Because the ClientHello's cipher suites are constructed in tls.Conn.clientHandshake() (crypto/tls/handshake_client.go:31) from the cipherSuites slice defined at crypto/tls/cipher_suites.go:69, which doesn't contain an entry for TLS_FALLBACK_SCSV, it's impossible for the package to send the SCSV.

The relevant code is as follows:

65      possibleCipherSuites := c.config.cipherSuites()
66      hello.cipherSuites = make([]uint16, 0, len(possibleCipherSuites))
67  
68  NextCipherSuite:
69      for _, suiteId := range possibleCipherSuites {
70          for _, suite := range cipherSuites {
71              if suite.id != suiteId {
72                  continue
73              }
74              // Don't advertise TLS 1.2-only cipher suites unless
75              // we're attempting TLS 1.2.
76              if hello.vers < VersionTLS12 && suite.flags&suiteTLS12 != 0 {
77                  continue
78              }
79              hello.cipherSuites = append(hello.cipherSuites, suiteId)
80              continue NextCipherSuite
81          }
82      }

Since TLS_FALLBACK_SCSV is not in cipherSuites, line 72 above will skip over any entries of TLS_FALLBACK_SCSV in the tls.Config. The following patch would resolve the issue:

--- a/src/crypto/tls/handshake_client.go
+++ b/src/crypto/tls/handshake_client.go
@@ -67,6 +67,10 @@ func (c *Conn) clientHandshake() error {

 NextCipherSuite:
        for _, suiteId := range possibleCipherSuites {
+               if suiteId == TLS_FALLBACK_SCSV {
+                       hello.cipherSuites = append(hello.cipherSuites, suiteId)
+                       continue
+               }
                for _, suite := range cipherSuites {
                        if suite.id != suiteId {
                                continue

[bradfitz]

Another one for @agl.

[agl]

The Go client doesn't do fallback so doesn't need to send TLS_FALLBACK_SCSV. Why do you think that you need to send it?

[SlyMarbo]

I wanted to check whether my server is handling TLS_FALLBACK_SCSV correctly and using crypto/tls seemed the easiest option. It's not the most important feature but it would be nice.

[agl]

On the fence about whether we want to support this, but dropping for 1.5 at least.

You would probably be better off constructing a little ClientHello message yourself for this sort of testing.

[agl]

I'm going to call this one: I don't think that supporting this option is worthwhile in the standard library. The use-case is pretty obscure and there are lots of testing needs that can only be met by crafting ClientHellos already.