Security: do not derive TLS server identity from reverse DNS

CMakeLists.txt

@@ -895,7 +895,6 @@ ENDIF()
 # POSIX-specific checks
 
 CHECK_FUNCTION_EXISTS(getaddrinfo VMIME_HAVE_GETADDRINFO)
-CHECK_FUNCTION_EXISTS(getnameinfo VMIME_HAVE_GETNAMEINFO)
 
 CHECK_FUNCTION_EXISTS(gettid VMIME_HAVE_GETTID)
 CHECK_FUNCTION_EXISTS(syscall VMIME_HAVE_SYSCALL)

cmake/config.hpp.cmake

@@ -73,7 +73,6 @@ typedef unsigned @VMIME_64BIT_TYPE@ vmime_uint64;
 #cmakedefine01 VMIME_HAVE_PTHREAD
 #cmakedefine01 VMIME_HAVE_GETADDRINFO
 #cmakedefine01 VMIME_HAVE_GETADDRINFO_A
-#cmakedefine01 VMIME_HAVE_GETNAMEINFO
 #cmakedefine01 VMIME_HAVE_GETTID
 #cmakedefine01 VMIME_HAVE_SYSCALL
 #cmakedefine01 VMIME_HAVE_SYSCALL_GETTID

src/vmime/net/socket.hpp

@@ -149,9 +149,13 @@ class VMIME_EXPORT socket : public object {
 	  */
 	virtual unsigned int getStatus() const = 0;
 
-	/** Return the hostname of peer this socket is connected to.
+	/** Return the name of the peer this socket is connected to, as it
+	  * was specified when connecting (host name or numeric address).
 	  *
-	  * @return name of the peer, or numeric address if it cannot be found
+	  * No reverse DNS lookup is performed on numeric addresses, as this
+	  * name is used to verify the identity of the server with TLS.
+	  *
+	  * @return name of the peer
 	  */
 	virtual const string getPeerName() const = 0;
 

src/vmime/net/tls/TLSSocket.cpp

@@ -35,6 +35,42 @@ namespace net {
 namespace tls {
 
 
+// static
+bool TLSSocket::isIPAddress(const string& host) {
+
+	// A host name cannot contain ':', so this is an IPv6 address
+	if (host.find(':') != string::npos) {
+		return true;
+	}
+
+	// Check for an IPv4 address in dotted-decimal notation
+	int parts = 0;
+	string::size_type pos = 0;
+
+	while (true) {
+
+		unsigned int value = 0;
+		int digits = 0;
+
+		for ( ; pos < host.length() && host[pos] >= '0' && host[pos] <= '9' ; ++pos, ++digits) {
+			value = value * 10 + static_cast <unsigned int>(host[pos] - '0');
+		}
+
+		if (digits == 0 || digits > 3 || value > 255) {
+			return false;
+		}
+
+		if (++parts == 4 || pos == host.length() || host[pos] != '.') {
+			break;
+		}
+
+		++pos;  // skip '.'
+	}
+
+	return parts == 4 && pos == host.length();
+}
+
+
 } // tls
 } // net
 } // vmime

src/vmime/net/tls/TLSSocket.hpp

@@ -75,6 +75,15 @@ class VMIME_EXPORT TLSSocket : public socket {
 	  * has not been performed yet
 	  */
 	virtual shared_ptr <security::cert::certificateChain> getPeerCertificates() = 0;
+
+	/** Test whether the specified host is an IPv4 or IPv6 address
+	  * rather than a host name. IP addresses must not be sent in
+	  * the Server Name Indication (SNI) extension (RFC 6066).
+	  *
+	  * @param host host name or address
+	  * @return true if the host is an IP address, false otherwise
+	  */
+	static bool isIPAddress(const string& host);
 };
 
 

src/vmime/net/tls/gnutls/TLSSocket_GnuTLS.cpp

@@ -318,8 +318,11 @@ void TLSSocket_GnuTLS::handshake() {
 	// Start handshaking process
 	try {
 		string peerName = getPeerName();
-
-		gnutls_server_name_set(*m_session->m_gnutlsSession, GNUTLS_NAME_DNS, peerName.c_str(), peerName.size());
+
+		// IP addresses are not permitted in SNI (RFC 6066)
+		if (!isIPAddress(peerName)) {
+			gnutls_server_name_set(*m_session->m_gnutlsSession, GNUTLS_NAME_DNS, peerName.c_str(), peerName.size());
+		}
 
 		while (true) {
 

src/vmime/net/tls/openssl/TLSSocket_OpenSSL.cpp

@@ -168,7 +168,12 @@ void TLSSocket_OpenSSL::createSSLHandle() {
 		}
 
 		SSL_set_bio(m_ssl, sockBio, sockBio);
-		SSL_set_tlsext_host_name(m_ssl, peerName.c_str());
+
+		// IP addresses are not permitted in SNI (RFC 6066)
+		if (!isIPAddress(peerName)) {
+			SSL_set_tlsext_host_name(m_ssl, peerName.c_str());
+		}
+
 		SSL_set_connect_state(m_ssl);
 		SSL_set_mode(m_ssl, SSL_MODE_AUTO_RETRY | SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
 

src/vmime/platforms/posix/posixSocket.cpp

@@ -541,35 +541,6 @@ void posixSocket::disconnect() {
 }
 
 
-static bool isNumericAddress(const char* address) {
-
-#if VMIME_HAVE_GETADDRINFO
-
-	struct addrinfo hint, *info = NULL;
-	memset(&hint, 0, sizeof(hint));
-
-	hint.ai_family = AF_UNSPEC;
-	hint.ai_flags = AI_NUMERICHOST;
-
-	if (getaddrinfo(address, 0, &hint, &info) == 0) {
-
-		freeaddrinfo(info);
-		return true;
-
-	} else {
-
-		return false;
-	}
-
-#else
-
-	return inet_addr(address) != INADDR_NONE;
-
-#endif
-
-}
-
-
 const string posixSocket::getPeerAddress() const {
 
 	// Get address of connected peer
@@ -601,53 +572,9 @@ const string posixSocket::getPeerAddress() const {
 
 const string posixSocket::getPeerName() const {
 
-	// Get address of connected peer
-	sockaddr_storage peer;
-	socklen_t peerLen = sizeof(peer);
-
-	if (getpeername(m_desc, reinterpret_cast <sockaddr*>(&peer), &peerLen) != 0) {
-		throwSocketError(errno);
-	}
-
-	// If server address as specified when connecting is a numeric
-	// address, try to get a host name for it
-	if (isNumericAddress(m_serverAddress.c_str())) {
-
-#if VMIME_HAVE_GETNAMEINFO
-
-		char host[NI_MAXHOST + 1];
-		char service[NI_MAXSERV + 1];
-
-		if (getnameinfo(reinterpret_cast <sockaddr *>(&peer), peerLen,
-				host, sizeof(host), service, sizeof(service),
-				/* flags */ NI_NAMEREQD) == 0) {
-
-			return string(host);
-		}
-
-#else
-
-		struct hostent *hp;
-		const void* addr;
-		socklen_t addrLen;
-
-		if (peer.ss_family == AF_INET6) {
-			addr = &reinterpret_cast <sockaddr_in6*>(&peer)->sin6_addr;
-			addrLen = sizeof(in6_addr);
-		} else {
-			addr = &reinterpret_cast <sockaddr_in*>(&peer)->sin_addr;
-			addrLen = sizeof(in_addr);
-		}
-
-		if ((hp = gethostbyaddr(addr, addrLen, peer.ss_family)) != NULL) {
-
-			return string(hp->h_name);
-		}
-
-#endif
-
-	}
-
+	// Do not try to get a host name for a numeric address: the peer name is
+	// used as the TLS reference identity, and it must not be derived from an
+	// insecure source such as a reverse DNS lookup
 	return m_serverAddress;
 }
 

src/vmime/platforms/windows/windowsSocket.cpp

@@ -173,26 +173,6 @@ void windowsSocket::disconnect() {
 }
 
 
-static bool isNumericAddress(const char* address) {
-
-	struct addrinfo hint, *info = NULL;
-	memset(&hint, 0, sizeof(hint));
-
-	hint.ai_family = AF_UNSPEC;
-	hint.ai_flags = AI_NUMERICHOST;
-
-	if (getaddrinfo(address, 0, &hint, &info) == 0) {
-
-		freeaddrinfo(info);
-		return true;
-
-	} else {
-
-		return false;
-	}
-}
-
-
 const string windowsSocket::getPeerAddress() const {
 
 	// Get address of connected peer
@@ -218,27 +198,9 @@ const string windowsSocket::getPeerAddress() const {
 
 const string windowsSocket::getPeerName() const {
 
-	// Get address of connected peer
-	sockaddr_storage peer;
-	socklen_t peerLen = sizeof(peer);
-
-	getpeername(m_desc, reinterpret_cast <sockaddr*>(&peer), &peerLen);
-
-	// If server address as specified when connecting is a numeric
-	// address, try to get a host name for it
-	if (isNumericAddress(m_serverAddress.c_str())) {
-
-		char host[NI_MAXHOST + 1];
-		char service[NI_MAXSERV + 1];
-
-		if (getnameinfo(reinterpret_cast <sockaddr *>(&peer), peerLen,
-				host, sizeof(host), service, sizeof(service),
-				/* flags */ NI_NAMEREQD) == 0) {
-
-			return string(host);
-		}
-	}
-
+	// Do not try to get a host name for a numeric address: the peer name is
+	// used as the TLS reference identity, and it must not be derived from an
+	// insecure source such as a reverse DNS lookup
 	return m_serverAddress;
 }
 

src/vmime/security/cert/openssl/X509Certificate_OpenSSL.cpp

@@ -433,6 +433,14 @@ bool X509Certificate_OpenSSL::verifyHostName(
 	std::vector <std::string>* nonMatchingNames
 ) const {
 
+	// An IP address must match an IP address in subject alternative names
+	// (X509_check_ip_asc() returns -2 if hostname is not an IP address)
+	const int ipMatch = X509_check_ip_asc(m_data->cert, hostname.c_str(), 0);
+
+	if (ipMatch != -2) {
+		return ipMatch == 1;
+	}
+
 	// First, check subject common name against hostname
 	char CNBuffer[1024];
 	CNBuffer[sizeof(CNBuffer) - 1] = '\0';

tests/net/tls/TLSSocketTest.cpp

@@ -0,0 +1,82 @@
+//
+// VMime library (http://www.vmime.org)
+// Copyright (C) 2002 Vincent Richard <vincent@vmime.org>
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License as
+// published by the Free Software Foundation; either version 3 of
+// the License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+// General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+//
+// Linking this library statically or dynamically with other modules is making
+// a combined work based on this library.  Thus, the terms and conditions of
+// the GNU General Public License cover the whole combination.
+//
+
+#include "tests/testUtils.hpp"
+
+
+#if VMIME_HAVE_MESSAGING_FEATURES && VMIME_HAVE_TLS_SUPPORT
+
+
+#include "vmime/net/tls/TLSSocket.hpp"
+
+
+using vmime::net::tls::TLSSocket;
+
+
+VMIME_TEST_SUITE_BEGIN(TLSSocketTest)
+
+	VMIME_TEST_LIST_BEGIN
+		VMIME_TEST(testIsIPAddress_IPv4)
+		VMIME_TEST(testIsIPAddress_IPv6)
+		VMIME_TEST(testIsIPAddress_HostName)
+		VMIME_TEST(testIsIPAddress_Invalid)
+	VMIME_TEST_LIST_END
+
+
+	void testIsIPAddress_IPv4() {
+
+		VASSERT_TRUE("1", TLSSocket::isIPAddress("192.0.2.1"));
+		VASSERT_TRUE("2", TLSSocket::isIPAddress("0.0.0.0"));
+		VASSERT_TRUE("3", TLSSocket::isIPAddress("255.255.255.255"));
+	}
+
+	void testIsIPAddress_IPv6() {
+
+		VASSERT_TRUE("1", TLSSocket::isIPAddress("2001:db8::1"));
+		VASSERT_TRUE("2", TLSSocket::isIPAddress("::1"));
+		VASSERT_TRUE("3", TLSSocket::isIPAddress("::ffff:192.0.2.1"));
+	}
+
+	void testIsIPAddress_HostName() {
+
+		VASSERT_FALSE("1", TLSSocket::isIPAddress("mail.vmime.org"));
+		VASSERT_FALSE("2", TLSSocket::isIPAddress("localhost"));
+		VASSERT_FALSE("3", TLSSocket::isIPAddress("192.vmime.org"));
+		VASSERT_FALSE("4", TLSSocket::isIPAddress("192.0.2.1.vmime.org"));
+	}
+
+	void testIsIPAddress_Invalid() {
+
+		VASSERT_FALSE("1", TLSSocket::isIPAddress(""));
+		VASSERT_FALSE("2", TLSSocket::isIPAddress("192.0.2"));
+		VASSERT_FALSE("3", TLSSocket::isIPAddress("192.0.2.1.5"));
+		VASSERT_FALSE("4", TLSSocket::isIPAddress("192.0.2.1."));
+		VASSERT_FALSE("5", TLSSocket::isIPAddress("192..2.1"));
+		VASSERT_FALSE("6", TLSSocket::isIPAddress("192.0.2.256"));
+		VASSERT_FALSE("7", TLSSocket::isIPAddress("192.0.2.0001"));
+	}
+
+VMIME_TEST_SUITE_END
+
+
+#endif // VMIME_HAVE_MESSAGING_FEATURES && VMIME_HAVE_TLS_SUPPORT