Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/tls: (*tls.Conn).Read() can return (0, nil) #5309

Closed
gopherbot opened this issue Apr 17, 2013 · 9 comments
Closed

crypto/tls: (*tls.Conn).Read() can return (0, nil) #5309

gopherbot opened this issue Apr 17, 2013 · 9 comments
Milestone

Comments

@gopherbot
Copy link

by kballard:

What steps will reproduce the problem?
A program I have that uses crypto/tls receives an unexpected (0, nil) response from
(*tls.Conn).Read().

The program is available at https://github.com/kballard/goirc/tree/tls_bug. It's a
sample program for an IRC library. It connects to Freenode and attempts to log in, but
receives the (0, nil) before the first line of output from the server.

Which operating system are you using?
Occurs on both OS X and Ubuntu 12.04.

Which version are you using?  (run 'go version')
go version devel +d58997478ec6 Mon Apr 08 00:09:35 2013 -0700 darwin/amd64

Please provide any additional information below.
If you uncomment the SSLConfig setting in example.go, which forces it to use RC4, the
problem disappears.
@gopherbot
Copy link
Author

Comment 1 by kballard:

I've attached a tcpdump of the connection.

Attachments:

  1. dump (7779 bytes)

@gopherbot
Copy link
Author

Comment 2 by kballard:

This is actually reproducible using a trivial testcase (although again with Freenode on
the other end).
http://play.golang.org/p/FWTWmjs3uc
    package main
    import (
        "crypto/tls"
        "fmt"
        "io"
    )
    func main() {
        conn, err := tls.Dial("tcp", "chat.freenode.net:6697", nil)
        if err != nil {
            fmt.Println(err)
            return
        }
        io.WriteString(conn, "NICK :goirc")
        io.WriteString(conn, "USER goirc 8 * :goirc")
        buf := make([]byte, 4096)
        for {
            n, err := conn.Read(buf)
            fmt.Println(n, err)
            if err == nil {
                fmt.Println(string(buf[:n]))
            } else {
                break
            }
        }
    }

@gopherbot
Copy link
Author

Comment 3 by kballard:

So far it appears that this requires cipher 0x0035 (TLS_RSA_WITH_AES_256_CBC_SHA) to
reproduce, which is what Freenode is picking. If I force a different cipher, the problem
goes away.
I wrote a sample program which runs both ends of the connection. If I run it with the
default ciphers everything is fine, but if I run it with TLS_RSA_WITH_AES_256_CBC_SHA,
then each written line gets split into 2 pieces on the receiving end, with the first
piece containing 1 byte and the second piece containing the rest of the message. I have
to wonder if maybe longer writes end up starting with a 0-byte piece (or, alternatively,
Freenode's implementation of the cipher produces a 0-byte piece instead of 1-byte piece).
The program is up at http://play.golang.org/p/zMD0fQokCi (requires a server cert/key).

@davecheney
Copy link
Contributor

Comment 4:

Issue #5300 has been merged into this issue.

@davecheney
Copy link
Contributor

Comment 5:

Issue #5300 has been merged into this issue.

@gopherbot
Copy link
Author

Comment 6 by xofyarg:

Sorry for cross post.
Now I quote my question from issue #5300:
> Thank you.
> 
> So what's the suggest action to this problem? Should I enclose the read within a loop?
Or tls package will make some changes to follow the normal read behavior?

@bradfitz
Copy link
Contributor

Comment 7:

Per http://tip.golang.org/pkg/io/#Reader,
"Implementations of Read are discouraged from returning a zero byte count with a nil
error, and callers should treat that situation as a no-op."
This bug should be about crypto/tls not returning (0, nil), but your code should still
handle it just in case, since we haven't always had that io.Reader warning, and you
can't expect all Readers to conform.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 8:

Labels changed: added priority-later, go1.2, removed priority-triage.

@rsc
Copy link
Contributor

rsc commented Sep 6, 2013

Comment 9:

Adam fixed this in May but got the "fixes" syntax wrong.
https://golang.org/cl/8852044

Status changed to Fixed.

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015
@rsc rsc removed the go1.2 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants