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 central servers behind NAT #954

Merged
merged 3 commits into from
Feb 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

- remove ConsoleWriterFactory (#926)

- add new --serverpublicip option to support central servers behind NAT,
coded by hoffie (#954)


TODO fix crash if settings are changed in ASIO4All during a connection (contained in #796)

Expand Down
19 changes: 19 additions & 0 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ int main ( int argc, char** argv )
QString strRecordingDirName = "";
QString strCentralServer = "";
QString strServerInfo = "";
QString strServerPublicIP = "";
QString strServerListFilter = "";
QString strWelcomeMessage = "";
QString strClientName = "";
Expand Down Expand Up @@ -397,6 +398,20 @@ int main ( int argc, char** argv )
}


// Server Public IP --------------------------------------------------
if ( GetStringArgument ( argc,
argv,
i,
"--serverpublicip", // no short form
"--serverpublicip",
strArgument ) )
{
strServerPublicIP = strArgument;
CommandLineOptions << "--serverpublicip";
continue;
}


// Server info ---------------------------------------------------------
if ( GetStringArgument ( argc,
argv,
Expand Down Expand Up @@ -696,6 +711,7 @@ int main ( int argc, char** argv )
strHTMLStatusFileName,
strCentralServer,
strServerInfo,
strServerPublicIP,
strServerListFilter,
strWelcomeMessage,
strRecordingDirName,
Expand Down Expand Up @@ -814,6 +830,9 @@ QString UsageArguments ( char **argv )
" -u, --numchannels maximum number of channels\n"
" -w, --welcomemessage welcome message on connect\n"
" -z, --startminimized start minimizied\n"
" --serverpublicip specify your public IP address when\n"
Copy link
Member Author

Choose a reason for hiding this comment

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

If you have any other suggestions how the flag should be called, let me know.

" running a slave and your own central server\n"
" behind the same NAT\n"
"\nClient only:\n"
" -M, --mutestream starts the application in muted state\n"
" --mutemyown mute me in my personal mix (headless only)\n"
Expand Down
3 changes: 3 additions & 0 deletions src/protocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ CONNECTION LESS MESSAGES
NOTE: In the PROTMESSID_CLM_SERVER_LIST list, this field will be empty
as only the initial IP address should be used by the client. Where
necessary, that value will contain the server internal address.
When running a central server and a slave server behind the same NAT,
this field is used the other way round: It will contain the public
IP in this case which will be served to clients from the Internet.
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a certain natural flow to that which I like :)



- PROTMESSID_CLM_REGISTER_SERVER_EX: Register a server, providing extended server
Expand Down
2 changes: 2 additions & 0 deletions src/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ CServer::CServer ( const int iNewMaxNumChan,
const QString& strCentralServer,
const QString& strServerInfo,
const QString& strServerListFilter,
const QString& strServerPublicIP,
const QString& strNewWelcomeMessage,
const QString& strRecordingDirName,
const bool bNDisconnectAllClientsOnQuit,
Expand All @@ -245,6 +246,7 @@ CServer::CServer ( const int iNewMaxNumChan,
ServerListManager ( iPortNumber,
strCentralServer,
strServerInfo,
strServerPublicIP,
strServerListFilter,
iNewMaxNumChan,
&ConnLessProtocol ),
Expand Down
1 change: 1 addition & 0 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ class CServer :
const QString& strCentralServer,
const QString& strServerInfo,
const QString& strServerListFilter,
const QString& strServerPublicIP,
const QString& strNewWelcomeMessage,
const QString& strRecordingDirName,
const bool bNDisconnectAllClientsOnQuit,
Expand Down
28 changes: 27 additions & 1 deletion src/serverlist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ CServerListManager::CServerListManager ( const quint16 iNPortNum,
const QString& sNCentServAddr,
const QString& strServerInfo,
const QString& strServerListFilter,
const QString& strServerPublicIP,
const int iNumChannels,
CProtocol* pNConLProt )
: eCentralServerAddressType ( AT_CUSTOM ), // must be AT_CUSTOM for the "no GUI" case
Expand All @@ -41,7 +42,18 @@ CServerListManager::CServerListManager ( const quint16 iNPortNum,
SetCentralServerAddress ( sNCentServAddr );

// set the server internal address, including internal port number
SlaveCurLocalHostAddress = CHostAddress( NetworkUtil::GetLocalAddress().InetAddr, iNPortNum );
QHostAddress qhaServerPublicIP;
Copy link
Member Author

@hoffie hoffie Feb 4, 2021

Choose a reason for hiding this comment

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

Is qhaServerPublicIP the proper naming convention?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There generally isn't consistent Microsoft-style class name initial prefixing of variable names. So these aren't inherently "right" (and I bristle against them) but there are enough type-prefixed names in the code to make them not inherently "wrong", either.

if ( strServerPublicIP == "" )
{
// No user-supplied override via --serverpublicip -> use auto-detection
qhaServerPublicIP = NetworkUtil::GetLocalAddress().InetAddr;
}
else
{
// User-supplied --serverpublicip
qhaServerPublicIP = QHostAddress ( strServerPublicIP );
}
SlaveCurLocalHostAddress = CHostAddress ( qhaServerPublicIP, iNPortNum );

// prepare the server info information
QStringList slServInfoSeparateParams;
Expand Down Expand Up @@ -443,6 +455,20 @@ void CServerListManager::CentralServerQueryServerList ( const CHostAddress& Inet
{
vecServerInfo[iIdx].HostAddr = ServerList[iIdx].LHostAddr;
}
else if ( !NetworkUtil::IsPrivateNetworkIP ( InetAddr.InetAddr ) &&
NetworkUtil::IsPrivateNetworkIP ( vecServerInfo[iIdx].HostAddr.InetAddr ) &&
!NetworkUtil::IsPrivateNetworkIP ( ServerList[iIdx].LHostAddr.InetAddr ) )
{
// We've got a request from a public client, the server
// list's entry's primary address is a private address,
// but it supplied an additional public address using
// --serverpublicip.
// In this case, use the latter.
// This is common when running a central server with slave
// servers behind a NAT and dealing with external, public
// clients.
vecServerInfo[iIdx].HostAddr = ServerList[iIdx].LHostAddr;
}
else
{
// create "send empty message" for all registered servers
Expand Down
1 change: 1 addition & 0 deletions src/serverlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ class CServerListManager : public QObject
const QString& sNCentServAddr,
const QString& strServerInfo,
const QString& strServerListFilter,
const QString& strServerPublicIP,
const int iNumChannels,
CProtocol* pNConLProt );

Expand Down
22 changes: 22 additions & 0 deletions src/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,28 @@ QString NetworkUtil::FixAddress ( const QString& strAddress )
return strAddress.simplified().replace ( " ", "" );
}

// Return whether the given HostAdress is within a private IP range
// as per RFC 1918 & RFC 5735.
bool NetworkUtil::IsPrivateNetworkIP ( const QHostAddress &qhAddr )
Copy link
Member Author

Choose a reason for hiding this comment

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

Is qhAddr the proper naming for a QHostAddress?

{
// https://www.rfc-editor.org/rfc/rfc1918
// https://www.rfc-editor.org/rfc/rfc5735
static QList<QPair<QHostAddress, int>> addresses =
{
QPair<QHostAddress, int> ( QHostAddress ( "10.0.0.0" ), 8 ),
QPair<QHostAddress, int> ( QHostAddress ( "127.0.0.0" ), 8 ),
QPair<QHostAddress, int> ( QHostAddress ( "172.16.0.0" ), 12 ),
QPair<QHostAddress, int> ( QHostAddress ( "192.168.0.0" ), 16 ),
};

foreach ( auto item, addresses ) {
if ( qhAddr.isInSubnet ( item ) ) {
return true;
}
}
return false;
}


// Instrument picture data base ------------------------------------------------
CVector<CInstPictures::CInstPictProps>& CInstPictures::GetTable ( const bool bReGenerateTable )
Expand Down
1 change: 1 addition & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,7 @@ class NetworkUtil
static CHostAddress GetLocalAddress();
static QString GetCentralServerAddress ( const ECSAddType eCentralServerAddressType,
const QString& strCentralServerAddress );
static bool IsPrivateNetworkIP ( const QHostAddress& qhAddr );
};


Expand Down