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: return error from SendMail when required AUTH not available #22145

Closed
stevenjohnstone opened this issue Oct 5, 2017 · 8 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@stevenjohnstone
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

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)?

linux amd64

What did you do?

Ran a MITM between a SendMail client and an SMTP server which supports TLS. MITM removes TLS negotiation, can see and tamper with messages in the clear.

What did you expect to see?

SendMail should fail or have some feedback mechanism to get an okay to proceed without TLS.

What did you see instead?

SendMail sends mail in the clear to the MITM.

@stevenjohnstone stevenjohnstone changed the title net/smpt SendMail vulnerable to MITM attacks net/smtp SendMail vulnerable to MITM attacks Oct 5, 2017
@stevenjohnstone
Copy link
Contributor Author

Follow up to #22133 and #22134 where PLAIN auth will only be used when TLS is negotiated. There's a wider problem though that the email data can be read/modified by a MITM who removes offers of TLS from the server.

@as
Copy link
Contributor

as commented Oct 5, 2017

If some adversary removes offers of TLS from the wire, then TLS is not negotiated. If TLS is not negotiated, PLAIN auth will not be used (upon resolution of the aforementioned issues). Given those constraints, what formal notion of security is being violated in addition to the two existing issues?

@stevenjohnstone
Copy link
Contributor Author

@as the recent fixes cover harvesting of plaintext credentials, which is an improvement.

However, if SendMail is configured to use auth other than PLAIN, then it'll send data (the email) in plaintext when TLS negotiation is deliberately removed by the MITM. The email can be read or modified by the attacker.

@rsc
Copy link
Contributor

rsc commented Oct 5, 2017

To paraphrase, @stevenjohnstone's report here is about the fact that a MITM can sit in the middle and downgrade to non-TLS and then snoop any mail being sent. As of yesterday, passwords will no longer be sent, but the mail messages still are (and always have been).

I think it would be interesting to consider changing SendMail for Go 1.10 to force the use of authentication when an auth implementation is passed to SendMail.

That is, right now, SendMail does basically:

if server allows auth {
    if auth != nil {
        use auth
    }
}

If the auth is smtp.PlainAuth and it is invoked, it will reject non-TLS sessions. But the MITM can just suggest not using it, and SendMail plays along (and always has).

It could be that we should change this code to be:

if auth != nil {
    if server does not allow auth {
       return error // cannot authenticate
    }
    use auth
}

That way, if you pass smtp.PlainAuth, which requires TLS, to SendMail, then SendMail would be guaranteed to use it, in effect requiring TLS.

The thing that this would break is using SendMail with servers that may or may not permit authentication and the caller can't predict which. I don't see how that situation would reasonably arise in practice, though. It just seems odd: "here, send this mail, and who knows if you need a password but if you do here it is."

If the user supplies a password for a particular server, I think it's reasonable for SendMail to interpret that as a direction that the mail is intended for that server and only that server, and to take steps (namely make sure it uses the authentication) to ensure that.

@rsc rsc changed the title net/smtp SendMail vulnerable to MITM attacks net/smtp: enable SendMail to require TLS Oct 5, 2017
@as
Copy link
Contributor

as commented Oct 5, 2017

That makes sense. Are there any other stdlib packages that may have more issues like this? It seems like historically most of the attention goes into crypto/* . SMTP wasn't a very popular package but godoc still shows ~1.7k imports.

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 5, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Oct 5, 2017
@rsc
Copy link
Contributor

rsc commented Nov 22, 2017

Too late for go 1.10 but I'd still be interested to see this in Go 1.11 if anyone wants to send a CL. Thanks.

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/79776 mentions this issue: net/mail: enable SendMail to require TLS

@johnnyluo
Copy link
Contributor

given the go1.11 tree is open can someone help to kick off the try bot on https://golang.org/cl/79776

@bradfitz bradfitz changed the title net/smtp: enable SendMail to require TLS net/smtp: return error from SendMail when required AUTH not available Feb 13, 2018
@golang golang locked and limited conversation to collaborators Feb 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants