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

smtp.PlainAuth susceptible to man-in-the-middle password harvesting [Go 1.8] #22134

Closed
broady opened this Issue Oct 4, 2017 · 6 comments

Comments

Projects
None yet
5 participants
@broady
Member

broady commented Oct 4, 2017

For obvious reasons, it’s a bad idea to send passwords over plaintext network connections.

RFC 4954 requires that, during SMTP, the PLAIN auth scheme must only be used on network connections secured with TLS. The original implementation of smtp.PlainAuth in Go 1.0 enforced this requirement, and it was documented to do so.

In 2013, issue #5184 was filed objecting to the TLS requirement, despite the fact that it is spelled out clearly in RFC 4954. The only possibly legitimate use case raised was using PLAIN auth for connections to localhost, and the suggested fix was to let the server decide: if it advertises that PLAIN auth is OK, believe it. That approach was adopted in CL 8279043 and released in Go 1.1.

Unfortunately, this is exactly wrong. The whole point of the TLS requirement is to make sure not to send the password to the wrong server or to a man-in-the-middle. Instead of implementing this rule, CL 8279043 blindly trusts the server, so that if a man-in-the-middle says "it's OK, you can send me your password," PlainAuth does. And the documentation was not updated to reflect any of this.

The result is that if you set up a man-in-the-middle SMTP server that doesn’t advertise STARTTLS and does advertise that PLAIN auth is OK, the smtp.PlainAuth implementation would send the username and password.

The fix is to restore the original TLS requirement. As a nod to the localhost use case, we then carve out a documented exception for connections made to localhost (defined as “localhost”, “127.0.0.1”, or “::1”).

Fixed in Go 1.8.4 by CL 68023 (4be3fc3).
Fixed in Go 1.9.1 by CL 68210 (39dd6f614474).

Thanks to Stevie Johnstone for reporting this problem.

Update: This has been assigned CVE-2017-15042.

@broady broady added this to the Go1.8.4 milestone Oct 4, 2017

@rsc rsc changed the title from placeholder for security issue to smtp.PlainAuth susceptible to man-in-the-middle password harvesting [Go 1.8] Oct 4, 2017

@rsc rsc closed this Oct 4, 2017

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Oct 5, 2017

Contributor

Update: This has been assigned CVE-2017-15042. (Also added to top comment.)

(Note: This box originally incorrectly said 15041.)

Contributor

rsc commented Oct 5, 2017

Update: This has been assigned CVE-2017-15042. (Also added to top comment.)

(Note: This box originally incorrectly said 15041.)

@KilledKenny

This comment has been minimized.

Show comment
Hide comment
@KilledKenny

KilledKenny Oct 5, 2017

Contributor

@rsc your comment says CVE-2017-15041 but the top comment says CVE-2017-15042

Contributor

KilledKenny commented Oct 5, 2017

@rsc your comment says CVE-2017-15041 but the top comment says CVE-2017-15042

@aronatkins

This comment has been minimized.

Show comment
Hide comment
@aronatkins

aronatkins Oct 6, 2017

@rsc Is there a way to mark SMTP over SSL connections as permitting auth without going through STARTTLS? (connection established using tls.DialWithDialer).

aronatkins commented Oct 6, 2017

@rsc Is there a way to mark SMTP over SSL connections as permitting auth without going through STARTTLS? (connection established using tls.DialWithDialer).

@aronatkins

This comment has been minimized.

Show comment
Hide comment
@aronatkins

aronatkins Oct 6, 2017

Update to my own question in case other folks happen across this: #22166 (comment) notes that the issue with SMTP+SSL will be fixed in Go 1.10.

aronatkins commented Oct 6, 2017

Update to my own question in case other folks happen across this: #22166 (comment) notes that the issue with SMTP+SSL will be fixed in Go 1.10.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Oct 6, 2017

Contributor

@KilledKenny, 15042 is correct, sigh. Thanks. (Just double-checked the assignment mail I received: 15041 is the cmd/go bug. I just got the copy+paste wrong in one of the two places on this bug.) I've updated the "Update:" comment.

Contributor

rsc commented Oct 6, 2017

@KilledKenny, 15042 is correct, sigh. Thanks. (Just double-checked the assignment mail I received: 15041 is the cmd/go bug. I just got the copy+paste wrong in one of the two places on this bug.) I've updated the "Update:" comment.

@hsyan2008

This comment has been minimized.

Show comment
Hide comment
@hsyan2008

hsyan2008 Dec 6, 2017

Go1.9.2 works fine.

hsyan2008 commented Dec 6, 2017

Go1.9.2 works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment