cloudflare / tls-tris

crypto/tls, now with 100% more 1.3. THE API IS NOT STABLE AND DOCUMENTATION IS NOT GUARANTEED.
Other
292 stars 50 forks source link

Apparent bug in TLS 1.0 (upstream) #98

Closed cjpatton closed 6 years ago

cjpatton commented 6 years ago

When TLS 1.0 is negotiated, Conn.Read() appears to only output 1 byte on the first call, then outputs the remainder of the bytes on the next call. This is not an issue in later versions.

This appears to be an issue with the upstream crypto/tls, and not with tris.

Here's some code for reproducing the bug;

package main

import (
    "crypto/tls"
    "crypto/x509"
    "fmt"
    "log"
    "net"
)

var certPEM = `-----BEGIN CERTIFICATE-----
MIIBaTCCAQ6gAwIBAgIQSUo+9uaip3qCW+1EPeHZgDAKBggqhkjOPQQDAjASMRAw
DgYDVQQKEwdBY21lIENvMB4XDTE4MDYxMjIzNDAyNloXDTE5MDYxMjIzNDAyNlow
EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABLf7
fiznPVdc3V5mM3ymswU2/IoJaq/deA6dgdj50ozdYyRiAPjxzcz9zRsZw1apTF/h
yNfiLhV4EE1VrwXcT5OjRjBEMA4GA1UdDwEB/wQEAwIFoDATBgNVHSUEDDAKBggr
BgEFBQcDATAMBgNVHRMBAf8EAjAAMA8GA1UdEQQIMAaHBH8AAAEwCgYIKoZIzj0E
AwIDSQAwRgIhANXG0zmrVtQBK0TNZZoEGMOtSwxmiZzXNe+IjdpxO3TiAiEA5VYx
0CWJq5zqpVXbJMeKVMASo2nrXZoA6NhJvFQ97hw=
-----END CERTIFICATE-----
`

var keyPEM = `-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIMw9DiOfGI1E/XZrrW2huZSjYi0EKwvVjAe+dYtyFsSloAoGCCqGSM49
AwEHoUQDQgAEt/t+LOc9V1zdXmYzfKazBTb8iglqr914Dp2B2PnSjN1jJGIA+PHN
zP3NGxnDVqlMX+HI1+IuFXgQTVWvBdxPkw==
-----END EC PRIVATE KEY-----
`

func main() {
    msg := "hello"

    // Server config
    cert, err := tls.X509KeyPair([]byte(certPEM), []byte(keyPEM))
    if err != nil {
        log.Fatal(err)
    }
    serverConfig := &tls.Config{
        Certificates: []tls.Certificate{cert},
        MaxVersion:   tls.VersionTLS10,
    }

    // Client config
    rootCAs := x509.NewCertPool()
    root, err := x509.ParseCertificate(
        cert.Certificate[len(cert.Certificate)-1])
    if err != nil {
        log.Fatal(err)
    }
    rootCAs.AddCert(root)
    clientConfig := &tls.Config{
        RootCAs:    rootCAs,
        MaxVersion: tls.VersionTLS10,
    }

    // Create a new listener
    ln, err := net.Listen("tcp", "127.0.0.1:0")
    if err != nil {
        ln, err = net.Listen("tcp6", "[::1]:0")
    }
    if err != nil {
        log.Fatal(err)
    }
    defer ln.Close()

        // Set up the server socket.
    srvCh := make(chan *tls.Conn, 1)
    var serr error
    go func() {
        sconn, err := ln.Accept()
        if err != nil {
            serr = err
            srvCh <- nil
            return
        }
        srv := tls.Server(sconn, serverConfig)
        if err := srv.Handshake(); err != nil {
            serr = fmt.Errorf("handshake: %v", err)
            srvCh <- nil
            return
        }
        srvCh <- srv
    }()

        // Set up the client socket.
    cli, err := tls.Dial("tcp", ln.Addr().String(), clientConfig)
    if err != nil {
        log.Fatal(err)
    }
    defer cli.Close()

    srv := <-srvCh
    if srv == nil {
        log.Fatal(serr)
    }

    buf := make([]byte, len(msg))
    cli.Write([]byte(msg))

    n := 0
    m, _ := srv.Read(buf)
    // NOTE Uncommenting the following line makes the test pass.
    //n, _ = srv.Read(buf[1:])
    if string(buf[:m+n]) != msg {
        log.Printf("Server read = %d, buf= %q; want %s",
            m+n, buf, msg)
    }
}
cjpatton commented 6 years ago

Actually, this behavior is intended. From conn.go:

    // SSL 3.0 and TLS 1.0 are susceptible to a chosen-plaintext
    // attack when using block mode ciphers due to predictable IVs.
    // This can be prevented by splitting each Application Data
    // record into two records, effectively randomizing the IV.
    //   
    // http://www.openssl.org/~bodo/tls-cbc.txt
    // https://bugzilla.mozilla.org/show_bug.cgi?id=665814
    // http://www.imperialviolet.org/2012/01/15/beastfollowup.html

    var m int
    if len(b) > 1 && c.vers <= VersionTLS10 {
        if _, ok := c.out.cipher.(cipher.BlockMode); ok { 
            n, err := c.writeRecordLocked(recordTypeApplicationData, b[:1])
            if err != nil {
                return n, c.out.setErrorLocked(err)
            }
            m, b = 1, b[1:]
        }
    }