From d00e11541e8fd2f149b353ab516807bbe23aefec Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Oct 2014 09:39:04 -0700 Subject: [PATCH 1/3] fix for pub key format returned to data-server --- domain-server/src/DomainServer.cpp | 95 +++++++++++++++--------------- 1 file changed, 48 insertions(+), 47 deletions(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 74bb3b6a2b..ec8b0e0ebe 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -619,56 +620,56 @@ bool DomainServer::shouldAllowConnectionFromNode(const QString& username, ALLOWED_USERS_SETTINGS_KEYPATH); static QStringList allowedUsers = allowedUsersVariant ? allowedUsersVariant->toStringList() : QStringList(); + // we always let in a user who is sending a packet from our local socket or from the localhost address + if (senderSockAddr.getAddress() != LimitedNodeList::getInstance()->getLocalSockAddr().getAddress() + && senderSockAddr.getAddress() != QHostAddress::LocalHost) { + return true; + } + if (allowedUsers.count() > 0) { - // this is an agent, we need to ask them to provide us with their signed username to see if they are allowed in - // we always let in a user who is sending a packet from our local socket or from the localhost address - - if (senderSockAddr.getAddress() != LimitedNodeList::getInstance()->getLocalSockAddr().getAddress() - && senderSockAddr.getAddress() != QHostAddress::LocalHost) { - if (allowedUsers.contains(username)) { - // it's possible this user can be allowed to connect, but we need to check their username signature - - QByteArray publicKeyArray = _userPublicKeys.value(username); - if (!publicKeyArray.isEmpty()) { - // if we do have a public key for the user, check for a signature match - - const unsigned char* publicKeyData = reinterpret_cast(publicKeyArray.constData()); - - // first load up the public key into an RSA struct - RSA* rsaPublicKey = d2i_RSAPublicKey(NULL, &publicKeyData, publicKeyArray.size()); - - if (rsaPublicKey) { - QByteArray decryptedArray(RSA_size(rsaPublicKey), 0); - int decryptResult = RSA_public_decrypt(usernameSignature.size(), - reinterpret_cast(usernameSignature.constData()), - reinterpret_cast(decryptedArray.data()), - rsaPublicKey, RSA_PKCS1_PADDING); - - if (decryptResult != -1) { - if (username == decryptedArray) { - qDebug() << "Username signature matches for" << username << "- allowing connection."; - - // free up the public key before we return - RSA_free(rsaPublicKey); - - return true; - } else { - qDebug() << "Username signature did not match for" << username << "- denying connection."; - } - } else { - qDebug() << "Couldn't decrypt user signature for" << username << "- denying connection."; - } - - // free up the public key, we don't need it anymore - RSA_free(rsaPublicKey); - } else { - // we can't let this user in since we couldn't convert their public key to an RSA key we could use - qDebug() << "Couldn't convert data to RSA key for" << username << "- denying connection."; - } - } + if (allowedUsers.contains(username)) { + // it's possible this user can be allowed to connect, but we need to check their username signature - requestUserPublicKey(username); + QByteArray publicKeyArray = _userPublicKeys.value(username); + if (!publicKeyArray.isEmpty()) { + // if we do have a public key for the user, check for a signature match + + const unsigned char* publicKeyData = reinterpret_cast(publicKeyArray.constData()); + + // first load up the public key into an RSA struct + RSA* rsaPublicKey = d2i_RSA_PUBKEY(NULL, &publicKeyData, publicKeyArray.size()); + + if (rsaPublicKey) { + QByteArray decryptedArray(RSA_size(rsaPublicKey), 0); + int decryptResult = RSA_public_decrypt(usernameSignature.size(), + reinterpret_cast(usernameSignature.constData()), + reinterpret_cast(decryptedArray.data()), + rsaPublicKey, RSA_PKCS1_PADDING); + + if (decryptResult != -1) { + if (username == decryptedArray) { + qDebug() << "Username signature matches for" << username << "- allowing connection."; + + // free up the public key before we return + RSA_free(rsaPublicKey); + + return true; + } else { + qDebug() << "Username signature did not match for" << username << "- denying connection."; + } + } else { + qDebug() << "Couldn't decrypt user signature for" << username << "- denying connection."; + } + + // free up the public key, we don't need it anymore + RSA_free(rsaPublicKey); + } else { + // we can't let this user in since we couldn't convert their public key to an RSA key we could use + qDebug() << "Couldn't convert data to RSA key for" << username << "- denying connection."; + } } + + requestUserPublicKey(username); } } else { // since we have no allowed user list, let them all in From 9a842e2202f683ab02f58f89b774bddc5453f28b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Oct 2014 10:52:36 -0700 Subject: [PATCH 2/3] flip domain-server local connect conditional --- domain-server/resources/describe-settings.json | 2 +- domain-server/src/DomainServer.cpp | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/domain-server/resources/describe-settings.json b/domain-server/resources/describe-settings.json index b8bc783aa1..2c33897d07 100644 --- a/domain-server/resources/describe-settings.json +++ b/domain-server/resources/describe-settings.json @@ -64,7 +64,7 @@ "name": "allowed_users", "type": "table", "label": "Allowed Users", - "help": "List the High Fidelity names for people you want to be able to connect to this domain.
An empty list means everyone.
You can always connect from this machine.", + "help": "List the High Fidelity names for people you want to be able to connect to this domain.
An empty list means everyone.
You can always connect from the domain-server machine.", "numbered": false, "columns": [ { diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index ec8b0e0ebe..b655aba25b 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -616,13 +616,13 @@ const QString ALLOWED_USERS_SETTINGS_KEYPATH = "security.allowed_users"; bool DomainServer::shouldAllowConnectionFromNode(const QString& username, const QByteArray& usernameSignature, const HifiSockAddr& senderSockAddr) { - static const QVariant* allowedUsersVariant = valueForKeyPath(_settingsManager.getSettingsMap(), + const QVariant* allowedUsersVariant = valueForKeyPath(_settingsManager.getSettingsMap(), ALLOWED_USERS_SETTINGS_KEYPATH); - static QStringList allowedUsers = allowedUsersVariant ? allowedUsersVariant->toStringList() : QStringList(); + QStringList allowedUsers = allowedUsersVariant ? allowedUsersVariant->toStringList() : QStringList(); // we always let in a user who is sending a packet from our local socket or from the localhost address - if (senderSockAddr.getAddress() != LimitedNodeList::getInstance()->getLocalSockAddr().getAddress() - && senderSockAddr.getAddress() != QHostAddress::LocalHost) { + if (senderSockAddr.getAddress() == LimitedNodeList::getInstance()->getLocalSockAddr().getAddress() + && senderSockAddr.getAddress() == QHostAddress::LocalHost) { return true; } @@ -670,6 +670,8 @@ bool DomainServer::shouldAllowConnectionFromNode(const QString& username, } requestUserPublicKey(username); + } else { + qDebug() << "Connect request denied for user" << username << "not in allowed users list."; } } else { // since we have no allowed user list, let them all in From c89daa30daf972dcb197558fa2b2492f64de3eaf Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 17 Oct 2014 11:07:02 -0700 Subject: [PATCH 3/3] fix another dumb conditional mistake in domain-server --- domain-server/src/DomainServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index b655aba25b..81835fb50a 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -622,7 +622,7 @@ bool DomainServer::shouldAllowConnectionFromNode(const QString& username, // we always let in a user who is sending a packet from our local socket or from the localhost address if (senderSockAddr.getAddress() == LimitedNodeList::getInstance()->getLocalSockAddr().getAddress() - && senderSockAddr.getAddress() == QHostAddress::LocalHost) { + || senderSockAddr.getAddress() == QHostAddress::LocalHost) { return true; }