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

net/smtp: NewClient should initialize tls based on the incoming connection type #22166

Closed
aronatkins opened this issue Oct 6, 2017 · 7 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Milestone

Comments

@aronatkins
Copy link

What version of Go are you using (go version)?

1.9.1

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/aron/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/ts/s940qvdj5vj1czr9qh07fvtw0000gn/T/go-build463641693=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

Attempted to perform SMTP PLAIN authentication over an SSL connection (without STARTTLS).

Code resembles:

conn, err := tls.Dial("tcp", "hostname:port", tlsConfig)
client, err := smtp.NewClient(conn, "hostname")
auth := smtp.PlainAuth(identity, "username", "password", "hostname")
err := client.Auth(&auth)

What did you expect to see?

Successful authentication and mail-sending.

What did you see instead?

Authentication failed with an "unencrypted connection" error.

This approach worked before #22134/#22133

smtp.NewClient could look at its incoming net.Conn and if it is an *tls.Conn, initialize Client.tls as true.

@ianlancetaylor
Copy link
Contributor

I believe this was just fixed for the future 1.10 release by https://golang.org/cl/68470. It will not be fixed for 1.9. Closing. Please comment if you disagree.

@aronatkins
Copy link
Author

@ianlancetaylor I have a local workaround which installs a PlainAuth implementation without the TLS check when the connection is already known to be encrypted.

The 1.9.1 release closed the MITM vulnerability but broke a long-valid SMTP use-case. If 1.9.1 were still open, I would argue for this fix to be included. Probably doesn't justify a release on its own, but should be noted as a known problem in the 1.9.1 release notes. Folks using SMTP+SSL will need a similar workaround to mine.

@ianlancetaylor ianlancetaylor added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 6, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.9.2 milestone Oct 6, 2017
@ianlancetaylor
Copy link
Contributor

OK, reopening to consider backporting https://golang.org/cl/68470 to 1.9.2.

@ccbrown
Copy link

ccbrown commented Oct 12, 2017

+1 to backporting to 1.9.2. We got a bit more than we bargained for when we upgraded for the security fixes, and all our servers are now unable to send emails.

Fortunately it's easy to create a PlainAuth function that bypasses the TLS check:

type plainAuthOverTLSConn struct {
	smtp.Auth
}

func PlainAuthOverTLSConn(identity, username, password, host string) smtp.Auth {
	return &plainAuthOverTLSConn{smtp.PlainAuth(identity, username, password, host)}
}

func (a *plainAuthOverTLSConn) Start(server *smtp.ServerInfo) (string, []byte, error) {
	server.TLS = true
	return a.Auth.Start(server)
}

@ianlancetaylor
Copy link
Contributor

I am now in favor of backporting to 1.9.2.

@ianlancetaylor ianlancetaylor added release-blocker and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 13, 2017
@rsc
Copy link
Contributor

rsc commented Oct 13, 2017

CL 68470 OK for Go 1.9.2.

@rsc rsc added the CherryPickApproved Used during the release process for point releases label Oct 14, 2017
@rsc
Copy link
Contributor

rsc commented Oct 26, 2017

go1.9.2 has been packaged and includes:

The release is posted at golang.org/dl.

— golang.org/x/build/cmd/releasebot, Oct 26 21:09:21 UTC

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants