From a8831e89ff7a1af077bac1335e458b2f5aa4c25a Mon Sep 17 00:00:00 2001 From: David Kelly Date: Thu, 16 Feb 2017 09:50:56 -0700 Subject: [PATCH] Ban only by machine fingerprint, when possible --- .../src/DomainServerSettingsManager.cpp | 104 +++++++++--------- 1 file changed, 51 insertions(+), 53 deletions(-) diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index 23f663817d..661a6213b8 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -30,6 +30,7 @@ #include #include #include //for KillAvatarReason +#include #include "DomainServerNodeData.h" const QString SETTINGS_DESCRIPTION_RELATIVE_PATH = "/resources/describe-settings.json"; @@ -439,7 +440,7 @@ bool DomainServerSettingsManager::unpackPermissionsForKeypath(const QString& key foreach (QVariant permsHash, permissionsList) { NodePermissionsPointer perms { new NodePermissions(permsHash.toMap()) }; QString id = perms->getID(); - + NodePermissionsKey idKey = perms->getKey(); if (mapPointer->contains(idKey)) { @@ -484,7 +485,7 @@ void DomainServerSettingsManager::unpackPermissions() { // make sure that this permission row is for a non-empty hardware if (perms->getKey().first.isEmpty()) { _macPermissions.remove(perms->getKey()); - + // we removed a row from the MAC permissions, we'll need a re-pack needPack = true; } @@ -555,7 +556,7 @@ void DomainServerSettingsManager::unpackPermissions() { QList> permissionsSets; permissionsSets << _standardAgentPermissions.get() << _agentPermissions.get() << _groupPermissions.get() << _groupForbiddens.get() - << _ipPermissions.get() << _macPermissions.get() + << _ipPermissions.get() << _macPermissions.get() << _machineFingerprintPermissions.get(); foreach (auto permissionSet, permissionsSets) { @@ -668,71 +669,68 @@ void DomainServerSettingsManager::processNodeKickRequestPacket(QSharedPointerclear(NodePermissions::Permission::canConnectToDomain); } else { - // otherwise we apply the kick to the IP from active socket for this node and the MAC address - - // remove connect permissions for the IP (falling back to the public socket if not yet active) - auto& kickAddress = matchingNode->getActiveSocket() - ? matchingNode->getActiveSocket()->getAddress() - : matchingNode->getPublicSocket().getAddress(); - - // probably isLoopback covers it, as whenever I try to ban an agent on same machine as the domain-server - // it is always 127.0.0.1, but looking at the public and local addresses just to be sure - // TODO: soon we will have feedback (in the form of a message to the client) after we kick. When we - // do, we will have a success flag, and perhaps a reason for failure. For now, just don't do it. - if (kickAddress == limitedNodeList->getPublicSockAddr().getAddress() || - kickAddress == limitedNodeList->getLocalSockAddr().getAddress() || - kickAddress.isLoopback() ) { - qWarning() << "attempt to kick node running on same machine as domain server, ignoring KickRequest"; - return; - } - NodePermissionsKey ipAddressKey(kickAddress.toString(), QUuid()); - - // check if there were already permissions for the IP - bool hadIPPermissions = hasPermissionsForIP(kickAddress); - - // grab or create permissions for the given IP address - auto ipPermissions = _ipPermissions[ipAddressKey]; - - if (!hadIPPermissions || ipPermissions->can(NodePermissions::Permission::canConnectToDomain)) { - newPermissions = true; - - ipPermissions->clear(NodePermissions::Permission::canConnectToDomain); - } - - // potentially remove connect permissions for the MAC address and machine fingerprint + // remove connect permissions for the machine fingerprint DomainServerNodeData* nodeData = static_cast(matchingNode->getLinkedData()); if (nodeData) { - // mac address first - NodePermissionsKey macAddressKey(nodeData->getHardwareAddress(), 0); + // get this machine's fingerprint + auto domainServerFingerprint = FingerprintUtils::getMachineFingerprint(); - bool hadMACPermissions = hasPermissionsForMAC(nodeData->getHardwareAddress()); - - auto macPermissions = _macPermissions[macAddressKey]; - - if (!hadMACPermissions || macPermissions->can(NodePermissions::Permission::canConnectToDomain)) { - newPermissions = true; - - macPermissions->clear(NodePermissions::Permission::canConnectToDomain); + if (nodeData->getMachineFingerprint() == domainServerFingerprint) { + qWarning() << "attempt to kick node running on same machine as domain server (by fingerprint), ignoring KickRequest"; + return; } - - // now for machine fingerprint NodePermissionsKey machineFingerprintKey(nodeData->getMachineFingerprint().toString(), 0); - + + // check if there were already permissions for the fingerprint bool hadFingerprintPermissions = hasPermissionsForMachineFingerprint(nodeData->getMachineFingerprint()); - + + // grab or create permissions for the given fingerprint auto fingerprintPermissions = _machineFingerprintPermissions[machineFingerprintKey]; - + + // write them if (!hadFingerprintPermissions || fingerprintPermissions->can(NodePermissions::Permission::canConnectToDomain)) { newPermissions = true; fingerprintPermissions->clear(NodePermissions::Permission::canConnectToDomain); } + } else { + // if no node data, all we can do is IP address + auto& kickAddress = matchingNode->getActiveSocket() + ? matchingNode->getActiveSocket()->getAddress() + : matchingNode->getPublicSocket().getAddress(); + + // probably isLoopback covers it, as whenever I try to ban an agent on same machine as the domain-server + // it is always 127.0.0.1, but looking at the public and local addresses just to be sure + // TODO: soon we will have feedback (in the form of a message to the client) after we kick. When we + // do, we will have a success flag, and perhaps a reason for failure. For now, just don't do it. + if (kickAddress == limitedNodeList->getPublicSockAddr().getAddress() || + kickAddress == limitedNodeList->getLocalSockAddr().getAddress() || + kickAddress.isLoopback() ) { + qWarning() << "attempt to kick node running on same machine as domain server, ignoring KickRequest"; + return; + } + + + NodePermissionsKey ipAddressKey(kickAddress.toString(), QUuid()); + + // check if there were already permissions for the IP + bool hadIPPermissions = hasPermissionsForIP(kickAddress); + + // grab or create permissions for the given IP address + auto ipPermissions = _ipPermissions[ipAddressKey]; + + if (!hadIPPermissions || ipPermissions->can(NodePermissions::Permission::canConnectToDomain)) { + newPermissions = true; + + ipPermissions->clear(NodePermissions::Permission::canConnectToDomain); + } } } - // if we are here, then we kicked them, so send the KillAvatar message to everyone + + // if we are here, then we kicked them, so send the KillAvatar message auto packet = NLPacket::create(PacketType::KillAvatar, NUM_BYTES_RFC4122_UUID + sizeof(KillAvatarReason), true); packet->write(nodeUUID.toRfc4122()); packet->writePrimitive(KillAvatarReason::NoReason); - + // send to avatar mixer, it sends the kill to everyone else limitedNodeList->broadcastToNodes(std::move(packet), NodeSet() << NodeType::AvatarMixer); @@ -743,7 +741,7 @@ void DomainServerSettingsManager::processNodeKickRequestPacket(QSharedPointer