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

Setting SNI breaks STARTTLS #231

Closed
RichardSteele opened this issue Nov 18, 2019 · 2 comments
Closed

Setting SNI breaks STARTTLS #231

RichardSteele opened this issue Nov 18, 2019 · 2 comments

Comments

@RichardSteele
Copy link
Contributor

@RichardSteele RichardSteele commented Nov 18, 2019

The changes caused by 0fcf45b break upgrading a plain text connection to an encrypted connection!

TLSSocket_OpenSSL::createSSLHandle throws an exception if it doesn't know the server's address:

void TLSSocket_OpenSSL::createSSLHandle() {
if (m_wrapped->isConnected()) {
if (m_address.empty()) {
throw exceptions::tls_exception("Unknown host name, will not be able to set SNI");
}

The address is made known only in TLSSocket_OpenSSL::connect:

void TLSSocket_OpenSSL::connect(const string& address, const port_t port) {
try {
m_wrapped->connect(address, port);
m_address = address;

This function, however, isn't called if you start with a plain connection and upgrade to an encrypted connection via STARTTLS because an existing, already connected socket gets wrapped. I noticed this when using a SMTP connection:

void SMTPConnection::startTLS() {
try {
sendRequest(SMTPCommand::STARTTLS());
shared_ptr <SMTPResponse> resp = readResponse();
if (resp->getCode() != 220) {
throw SMTPCommandError(
"STARTTLS", resp->getText(), resp->getCode(), resp->getEnhancedCode()
);
}
shared_ptr <tls::TLSSession> tlsSession = tls::TLSSession::create(
getTransport()->getCertificateVerifier(),
getTransport()->getSession()->getTLSProperties()
);
shared_ptr <tls::TLSSocket> tlsSocket = tlsSession->getSocket(m_socket);
tlsSocket->handshake();
m_socket = tlsSocket;

@RichardSteele

This comment has been minimized.

Copy link
Contributor Author

@RichardSteele RichardSteele commented Nov 18, 2019

Using socket::getPeerName() instead of m_address might be a solution.

RichardSteele added a commit to RichardSteele/vmime that referenced this issue Nov 18, 2019
@RichardSteele RichardSteele changed the title Setting SNI (OpenSSL backend) breaks STARTTLS Setting SNI breaks STARTTLS Nov 18, 2019
vincent-richard added a commit that referenced this issue Nov 18, 2019
Fix #231: SNI breaks STARTTLS
@vincent-richard

This comment has been minimized.

Copy link
Member

@vincent-richard vincent-richard commented Nov 18, 2019

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.