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

@gopherbot gopherbot commented Apr 17, 2013

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

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented Apr 17, 2013

Comment 1 by kballard:

I've attached a tcpdump of the connection.

Attachments:

  1. dump (7779 bytes)
@gopherbot

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented Apr 17, 2013

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

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented Apr 18, 2013

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

This comment has been minimized.

Copy link
Contributor

@davecheney davecheney commented Apr 25, 2013

Comment 4:

Issue #5300 has been merged into this issue.

@davecheney

This comment has been minimized.

Copy link
Contributor

@davecheney davecheney commented Apr 26, 2013

Comment 5:

Issue #5300 has been merged into this issue.

@gopherbot

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented Apr 26, 2013

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

This comment has been minimized.

Copy link
Contributor

@bradfitz bradfitz commented Jul 22, 2013

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

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jul 30, 2013

Comment 8:

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

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc 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.

@gopherbot gopherbot added fixed labels Sep 6, 2013
@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
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.