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

Add support for IMAP STARTTLS #183

Merged
merged 1 commit into from
May 21, 2019
Merged

Conversation

seththeriault
Copy link
Contributor

This is a quick-and-dirty attempt to add STARTTLS support for the IMAP protocol ( see #120 ) inspired by a recent blog post by @siebenmann. It definitely needs some review.

@CLAassistant
Copy link

CLAassistant commented May 21, 2019

CLA assistant check
All committers have signed the CLA.

@mcpherrinm
Copy link
Contributor

Thank you! At first glance this looks reasonable, though I'm not very familiar with IMAP.

Are there some servers "in the real world" that you know of which we can test this against?

@seththeriault
Copy link
Contributor Author

seththeriault commented May 21, 2019

Sure, these servers support STARTTLS:

imap.1and1.com
imap.o2online.de (even shows a cert chain verification error)
outlook.office365.com
imap.dreamhost.com

"net"
)

func dumpAuthTLSFromIMAP(dialer Dialer, address string, config *tls.Config) (*tls.ConnectionState, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the original naming here was probably taken from the FTP code, which sends an AUTH TLS message as part of the FTP STARTTLS protocol (explicit FTPS). Would probably make more sense if we called these something else now, like maybe dumpTLSConnStateFromFTP and dumpTLSConnStateFromIMAP.

@csstaub csstaub merged commit 75258cf into square:master May 21, 2019
@csstaub
Copy link
Contributor

csstaub commented May 21, 2019

Thank you for your contribution @seththeriault! I updated the method names and merged this into master. We'll try to cut a release with this soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants