From 9ab77545eecb98ab1c1f12da1b00de47a0aa9429 Mon Sep 17 00:00:00 2001 From: walker0643 <> Date: Sat, 12 May 2018 17:42:55 -0400 Subject: [PATCH] fix ipv6 handling between GUI and barriers/barrierc; zero-fill sockaddr_in(6) structs prior to initializing; update --help output --- src/gui/src/MainWindow.cpp | 10 +-- src/lib/arch/unix/ArchNetworkBSD.cpp | 9 ++- src/lib/arch/win32/ArchNetworkWinsock.cpp | 12 ++-- src/lib/barrier/ClientApp.cpp | 7 +- src/lib/barrier/ServerApp.cpp | 21 ++++-- src/lib/net/NetworkAddress.cpp | 88 +++++++++++------------ 6 files changed, 83 insertions(+), 64 deletions(-) diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index 55a92598..6829c227 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -581,7 +581,7 @@ bool MainWindow::clientArgs(QStringList& args, QString& app) if (m_pCheckBoxAutoConfig->isChecked()) { if (m_pComboServerList->count() != 0) { QString serverIp = m_pComboServerList->currentText(); - args << serverIp + ":" + QString::number(appConfig().port()); + args << "[" + serverIp + "]:" + QString::number(appConfig().port()); return true; } } @@ -595,7 +595,7 @@ bool MainWindow::clientArgs(QStringList& args, QString& app) return false; } - args << m_pLineEditHostname->text() + ":" + QString::number(appConfig().port()); + args << "[" + m_pLineEditHostname->text() + "]:" + QString::number(appConfig().port()); return true; } @@ -637,8 +637,10 @@ QString MainWindow::configFilename() QString MainWindow::address() { - QString i = appConfig().networkInterface(); - return (!i.isEmpty() ? i : "") + ":" + QString::number(appConfig().port()); + QString address = appConfig().networkInterface(); + if (!address.isEmpty()) + address = "[" + address + "]"; + return address + ":" + QString::number(appConfig().port()); } QString MainWindow::appPath(const QString& name) diff --git a/src/lib/arch/unix/ArchNetworkBSD.cpp b/src/lib/arch/unix/ArchNetworkBSD.cpp index 149218c8..2a9259ca 100644 --- a/src/lib/arch/unix/ArchNetworkBSD.cpp +++ b/src/lib/arch/unix/ArchNetworkBSD.cpp @@ -113,6 +113,11 @@ ArchNetworkBSD::newSocket(EAddressFamily family, ESocketType type) } try { setBlockingOnSocket(fd, false); + if (family == kINET6) { + int flag = 0; + if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &flag, sizeof(flag)) != 0) + throwError(errno); + } } catch (...) { close(fd); @@ -647,8 +652,8 @@ ArchNetworkBSD::newAnyAddr(EAddressFamily family) switch (family) { case kINET: { auto* ipAddr = TYPED_ADDR(struct sockaddr_in, addr); + memset(ipAddr, 0, sizeof(struct sockaddr_in)); ipAddr->sin_family = AF_INET; - ipAddr->sin_port = 0; ipAddr->sin_addr.s_addr = INADDR_ANY; addr->m_len = (socklen_t)sizeof(struct sockaddr_in); break; @@ -656,8 +661,8 @@ ArchNetworkBSD::newAnyAddr(EAddressFamily family) case kINET6: { auto* ipAddr = TYPED_ADDR(struct sockaddr_in6, addr); + memset(ipAddr, 0, sizeof(struct sockaddr_in6)); ipAddr->sin6_family = AF_INET6; - ipAddr->sin6_port = 0; memcpy(&ipAddr->sin6_addr, &in6addr_any, sizeof(in6addr_any)); addr->m_len = (socklen_t)sizeof(struct sockaddr_in6); break; diff --git a/src/lib/arch/win32/ArchNetworkWinsock.cpp b/src/lib/arch/win32/ArchNetworkWinsock.cpp index 722c4c58..4bc61d82 100644 --- a/src/lib/arch/win32/ArchNetworkWinsock.cpp +++ b/src/lib/arch/win32/ArchNetworkWinsock.cpp @@ -213,10 +213,10 @@ ArchNetworkWinsock::newSocket(EAddressFamily family, ESocketType type) } try { setBlockingOnSocket(fd, false); - BOOL flag = 0; - int size = sizeof(flag); - if (setsockopt_winsock(fd, IPPROTO_IPV6, IPV6_V6ONLY, &flag, size) == SOCKET_ERROR) { - throwError(getsockerror_winsock()); + if (family == kINET6) { + int flag = 0; + if (setsockopt_winsock(fd, IPPROTO_IPV6, IPV6_V6ONLY, &flag, sizeof(flag)) == SOCKET_ERROR) + throwError(getsockerror_winsock()); } } catch (...) { @@ -685,8 +685,8 @@ ArchNetworkWinsock::newAnyAddr(EAddressFamily family) case kINET: { addr = ArchNetAddressImpl::alloc(sizeof(struct sockaddr_in)); auto* ipAddr = TYPED_ADDR(struct sockaddr_in, addr); + memset(ipAddr, 0, sizeof(struct sockaddr_in)); ipAddr->sin_family = AF_INET; - ipAddr->sin_port = 0; ipAddr->sin_addr.s_addr = INADDR_ANY; break; } @@ -694,8 +694,8 @@ ArchNetworkWinsock::newAnyAddr(EAddressFamily family) case kINET6: { addr = ArchNetAddressImpl::alloc(sizeof(struct sockaddr_in6)); auto* ipAddr = TYPED_ADDR(struct sockaddr_in6, addr); + memset(ipAddr, 0, sizeof(struct sockaddr_in6)); ipAddr->sin6_family = AF_INET6; - ipAddr->sin6_port = 0; memcpy(&ipAddr->sin6_addr, &in6addr_any, sizeof(in6addr_any)); break; } diff --git a/src/lib/barrier/ClientApp.cpp b/src/lib/barrier/ClientApp.cpp index 115edd99..17de18e7 100644 --- a/src/lib/barrier/ClientApp.cpp +++ b/src/lib/barrier/ClientApp.cpp @@ -131,9 +131,10 @@ ClientApp::help() << std::endl << "Default options are marked with a *" << std::endl << std::endl - << "The server address is of the form: [][:]. The hostname" << std::endl - << "must be the address or hostname of the server. The port overrides the" << std::endl - << "default port, " << kDefaultPort << "." << std::endl; + << "The server address is of the form: [][:]. The hostname" << std::endl + << "must be the address or hostname of the server. Placing brackets around" << std::endl + << "an IPv6 address is required when also specifying a port number and " << std::endl + << "optional otherwise. The default port number is " << kDefaultPort << "." << std::endl; LOG((CLOG_PRINT "%s", buffer.str().c_str())); } diff --git a/src/lib/barrier/ServerApp.cpp b/src/lib/barrier/ServerApp.cpp index 25ac0db9..6d8d7cdb 100644 --- a/src/lib/barrier/ServerApp.cpp +++ b/src/lib/barrier/ServerApp.cpp @@ -139,8 +139,9 @@ ServerApp::help() << std::endl << "The argument for --address is of the form: [][:]. The" << std::endl << "hostname must be the address or hostname of an interface on the system." << std::endl - << "The default is to listen on all interfaces. The port overrides the" << std::endl - << "default port, " << kDefaultPort << "." << std::endl + << "Placing brackets around an IPv6 address is required when also specifying " << std::endl + << "a port number and optional otherwise. The default is to listen on all" << std::endl + << "interfaces using port number " << kDefaultPort << "." << std::endl << std::endl << "If no configuration file pathname is provided then the first of the" << std::endl << "following to load successfully sets the configuration:" << std::endl @@ -506,6 +507,16 @@ ServerApp::openServerScreen() return screen; } +static const char* const family_string(IArchNetwork::EAddressFamily family) +{ + if (family == IArchNetwork::kINET) + return "IPv4"; + if (family == IArchNetwork::kINET6) + // assume IPv6 sockets are setup to support IPv4 traffic as well + return "IPv4/IPv6"; + return "Unknown"; +} + bool ServerApp::startServer() { @@ -531,13 +542,15 @@ ServerApp::startServer() double retryTime; ClientListener* listener = NULL; try { - listener = openClientListener(args().m_config->getBarrierAddress()); + auto listenAddress = args().m_config->getBarrierAddress(); + auto family = family_string(ARCH->getAddrFamily(listenAddress.getAddress())); + listener = openClientListener(listenAddress); m_server = openServer(*args().m_config, m_primaryClient); listener->setServer(m_server); m_server->setListener(listener); m_listener = listener; updateStatus(); - LOG((CLOG_NOTE "started server, waiting for clients")); + LOG((CLOG_NOTE "started server (%s), waiting for clients", family)); m_serverState = kStarted; return true; } diff --git a/src/lib/net/NetworkAddress.cpp b/src/lib/net/NetworkAddress.cpp index c395ab03..8d605678 100644 --- a/src/lib/net/NetworkAddress.cpp +++ b/src/lib/net/NetworkAddress.cpp @@ -28,6 +28,45 @@ // NetworkAddress // +static bool parse_address(const std::string& address, std::string& host, int& port) +{ + /* Three cases --- + * brackets: parse inside for host, check end for port as :INTEGER. DONE + * one colon: ipv4 address with port. DONE + * otherwise: all host, no port. DONE + * + * very, very little error checking. depends on address being trimmed before call. + * + * does not override port with a default value if no port was found in address. + */ + + if (address[0] == '[') { + // bracketed host possibly followed by port as :INTEGER + auto endBracket = address.find(']', 1); + if (endBracket == std::string::npos) + return false; + host = address.substr(1, endBracket - 1); + if (endBracket + 1 < address.length()) { + // port follows (or garbage) + if (address[endBracket + 1] != ':') + return false; + port = std::strtol(&address[endBracket + 2], nullptr, 10); + } + } else { + auto colon = address.find(':'); + if (colon != std::string::npos && address.find(':', colon + 1) == std::string::npos) { + // one single colon, must be ipv4 with port + host = address.substr(0, colon); + port = std::strtol(&address[colon + 1], nullptr, 10); + } else { + // no colons (ipv4) or more than one colon (ipv6), both without port + host = address; + } + } + + return true; +} + // name re-resolution adapted from a patch by Brent Priddy. NetworkAddress::NetworkAddress() : @@ -62,50 +101,9 @@ NetworkAddress::NetworkAddress(const String& hostname, int port) : m_hostname(hostname), m_port(port) { - // check for port suffix - String::size_type i = m_hostname.rfind(':'); - if (i != String::npos && i + 1 < m_hostname.size()) { - // found a colon. see if it looks like an IPv6 address. - bool colonNotation = false; - bool dotNotation = false; - bool doubleColon = false; - for (String::size_type j = 0; j < i; ++j) { - if (m_hostname[j] == ':') { - colonNotation = true; - dotNotation = false; - if (m_hostname[j + 1] == ':') { - doubleColon = true; - } - } - else if (m_hostname[j] == '.' && colonNotation) { - dotNotation = true; - } - } - - // port suffix is ambiguous with IPv6 notation if there's - // a double colon and the end of the address is not in dot - // notation. in that case we assume it's not a port suffix. - // the user can replace the double colon with zeros to - // disambiguate. - if ((!doubleColon || dotNotation) && !colonNotation) { - // parse port from hostname - char* end; - const char* chostname = m_hostname.c_str(); - long suffixPort = strtol(chostname + i + 1, &end, 10); - if (end == chostname + i + 1 || *end != '\0') { - throw XSocketAddress(XSocketAddress::kBadPort, - m_hostname, m_port); - } - - // trim port from hostname - m_hostname.erase(i); - - // save port - m_port = static_cast(suffixPort); - } - } - - // check port number + if (!parse_address(hostname, m_hostname, m_port)) + throw XSocketAddress(XSocketAddress::kUnknown, + m_hostname, m_port); checkPort(); } @@ -145,7 +143,7 @@ NetworkAddress::resolve() // if hostname is empty then use wildcard address otherwise look // up the name. if (m_hostname.empty()) { - m_address = ARCH->newAnyAddr(IArchNetwork::kINET); + m_address = ARCH->newAnyAddr(IArchNetwork::kINET6); } else { m_address = ARCH->nameToAddr(m_hostname);