From 7df8381b8ff76bb801cf9afa6962ddf1a7429c69 Mon Sep 17 00:00:00 2001 From: CalDescent Date: Sat, 1 Jan 2022 21:09:48 +0000 Subject: [PATCH] Don't allow a row to be added to ArbitraryPeers (or the message to be rebroadcast) if an entry already exists with the same hash and host/ip. This avoids duplicate entries from the same host/ip with differing ports. This can occur due to some requests using ephemeral port numbers. Ideally we would filter these out altogether, but this at least acts as a safety net to prevent a very cluttered db and associated "broadcast storm". The main tradeoff here is that multiple nodes on the same IP address will be recorded as a single entry. This doesn't seem like it will be a major limitation, because one of them will remain available. --- .../arbitrary/ArbitraryDataManager.java | 6 ++- .../repository/ArbitraryRepository.java | 2 + .../hsqldb/HSQLDBArbitraryRepository.java | 31 ++++++++++++ .../test/arbitrary/ArbitraryPeerTests.java | 50 ++++++++++++++++--- 4 files changed, 80 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataManager.java b/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataManager.java index 8aa039e6..38e8783e 100644 --- a/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataManager.java +++ b/src/main/java/org/qortal/controller/arbitrary/ArbitraryDataManager.java @@ -386,9 +386,11 @@ public class ArbitraryDataManager extends Thread { try (final Repository repository = RepositoryManager.getRepository()) { for (byte[] signature : signatures) { - // Check if a record already exists for this hash/peer combination + // Check if a record already exists for this hash/host combination + // The port is not checked here - only the host/ip - in order to avoid duplicates + // from filling up the db due to dynamic/ephemeral ports ArbitraryPeerData existingEntry = repository.getArbitraryRepository() - .getArbitraryPeerDataForSignatureAndPeer(signature, peer.getPeerData().getAddress().toString()); + .getArbitraryPeerDataForSignatureAndHost(signature, peer.getPeerData().getAddress().getHost()); if (existingEntry == null) { // We haven't got a record of this mapping yet, so add it diff --git a/src/main/java/org/qortal/repository/ArbitraryRepository.java b/src/main/java/org/qortal/repository/ArbitraryRepository.java index fdfb0c6e..ba3ba1d8 100644 --- a/src/main/java/org/qortal/repository/ArbitraryRepository.java +++ b/src/main/java/org/qortal/repository/ArbitraryRepository.java @@ -33,6 +33,8 @@ public interface ArbitraryRepository { public ArbitraryPeerData getArbitraryPeerDataForSignatureAndPeer(byte[] signature, String peerAddress) throws DataException; + public ArbitraryPeerData getArbitraryPeerDataForSignatureAndHost(byte[] signature, String host) throws DataException; + public void save(ArbitraryPeerData arbitraryPeerData) throws DataException; public void delete(ArbitraryPeerData arbitraryPeerData) throws DataException; diff --git a/src/main/java/org/qortal/repository/hsqldb/HSQLDBArbitraryRepository.java b/src/main/java/org/qortal/repository/hsqldb/HSQLDBArbitraryRepository.java index 0ecb14c5..992d4f2d 100644 --- a/src/main/java/org/qortal/repository/hsqldb/HSQLDBArbitraryRepository.java +++ b/src/main/java/org/qortal/repository/hsqldb/HSQLDBArbitraryRepository.java @@ -498,6 +498,37 @@ public class HSQLDBArbitraryRepository implements ArbitraryRepository { } } + public ArbitraryPeerData getArbitraryPeerDataForSignatureAndHost(byte[] signature, String host) throws DataException { + // Hash the signature so it fits within 32 bytes + byte[] hashedSignature = Crypto.digest(signature); + + // Create a host wildcard string which allows any port + String hostWildcard = String.format("%s:%%", host); + + String sql = "SELECT hash, peer_address, successes, failures, last_attempted, last_retrieved " + + "FROM ArbitraryPeers " + + "WHERE hash = ? AND peer_address LIKE ?"; + + try (ResultSet resultSet = this.repository.checkedExecute(sql, hashedSignature, hostWildcard)) { + if (resultSet == null) + return null; + + byte[] hash = resultSet.getBytes(1); + String peerAddr = resultSet.getString(2); + Integer successes = resultSet.getInt(3); + Integer failures = resultSet.getInt(4); + Long lastAttempted = resultSet.getLong(5); + Long lastRetrieved = resultSet.getLong(6); + + ArbitraryPeerData arbitraryPeerData = new ArbitraryPeerData(hash, peerAddr, successes, failures, + lastAttempted, lastRetrieved); + + return arbitraryPeerData; + } catch (SQLException e) { + throw new DataException("Unable to fetch arbitrary peer data from repository", e); + } + } + @Override public void save(ArbitraryPeerData arbitraryPeerData) throws DataException { HSQLDBSaver saveHelper = new HSQLDBSaver("ArbitraryPeers"); diff --git a/src/test/java/org/qortal/test/arbitrary/ArbitraryPeerTests.java b/src/test/java/org/qortal/test/arbitrary/ArbitraryPeerTests.java index 16eb17bb..05751ffd 100644 --- a/src/test/java/org/qortal/test/arbitrary/ArbitraryPeerTests.java +++ b/src/test/java/org/qortal/test/arbitrary/ArbitraryPeerTests.java @@ -29,13 +29,14 @@ public class ArbitraryPeerTests extends Common { try (final Repository repository = RepositoryManager.getRepository()) { String peerAddress = "127.0.0.1:12392"; + String host = peerAddress.split(":")[0]; // Create random bytes to represent a signature byte[] signature = new byte[64]; new Random().nextBytes(signature); // Make sure we don't have an entry for this hash/peer combination - assertNull(repository.getArbitraryRepository().getArbitraryPeerDataForSignatureAndPeer(signature, peerAddress)); + assertNull(repository.getArbitraryRepository().getArbitraryPeerDataForSignatureAndHost(signature, host)); // Now add this mapping to the db Peer peer = new Peer(new PeerData(PeerAddress.fromString(peerAddress))); @@ -44,8 +45,8 @@ public class ArbitraryPeerTests extends Common { // We should now have an entry for this hash/peer combination ArbitraryPeerData retrievedArbitraryPeerData = repository.getArbitraryRepository() - .getArbitraryPeerDataForSignatureAndPeer(signature, peerAddress); - assertNotNull(arbitraryPeerData); + .getArbitraryPeerDataForSignatureAndHost(signature, host); + assertNotNull(retrievedArbitraryPeerData); // .. and its data should match what was saved assertArrayEquals(Crypto.digest(signature), retrievedArbitraryPeerData.getHash()); @@ -59,13 +60,14 @@ public class ArbitraryPeerTests extends Common { try (final Repository repository = RepositoryManager.getRepository()) { String peerAddress = "127.0.0.1:12392"; + String host = peerAddress.split(":")[0]; // Create random bytes to represent a signature byte[] signature = new byte[64]; new Random().nextBytes(signature); // Make sure we don't have an entry for this hash/peer combination - assertNull(repository.getArbitraryRepository().getArbitraryPeerDataForSignatureAndPeer(signature, peerAddress)); + assertNull(repository.getArbitraryRepository().getArbitraryPeerDataForSignatureAndHost(signature, host)); // Now add this mapping to the db Peer peer = new Peer(new PeerData(PeerAddress.fromString(peerAddress))); @@ -74,8 +76,8 @@ public class ArbitraryPeerTests extends Common { // We should now have an entry for this hash/peer combination ArbitraryPeerData retrievedArbitraryPeerData = repository.getArbitraryRepository() - .getArbitraryPeerDataForSignatureAndPeer(signature, peerAddress); - assertNotNull(arbitraryPeerData); + .getArbitraryPeerDataForSignatureAndHost(signature, host); + assertNotNull(retrievedArbitraryPeerData); // .. and its data should match what was saved assertArrayEquals(Crypto.digest(signature), retrievedArbitraryPeerData.getHash()); @@ -97,7 +99,7 @@ public class ArbitraryPeerTests extends Common { // Retrieve data once again ArbitraryPeerData updatedArbitraryPeerData = repository.getArbitraryRepository() - .getArbitraryPeerDataForSignatureAndPeer(signature, peerAddress); + .getArbitraryPeerDataForSignatureAndHost(signature, host); assertNotNull(updatedArbitraryPeerData); // Check the values @@ -112,4 +114,38 @@ public class ArbitraryPeerTests extends Common { assertTrue(NTP.getTime() - updatedArbitraryPeerData.getLastAttempted() < 1000); } } + + @Test + public void testDuplicatePeerHost() throws DataException { + try (final Repository repository = RepositoryManager.getRepository()) { + + String peerAddress1 = "127.0.0.1:12392"; + String peerAddress2 = "127.0.0.1:62392"; + String host1 = peerAddress1.split(":")[0]; + String host2 = peerAddress2.split(":")[0]; + + // Create random bytes to represent a signature + byte[] signature = new byte[64]; + new Random().nextBytes(signature); + + // Make sure we don't have an entry for these hash/peer combinations + assertNull(repository.getArbitraryRepository().getArbitraryPeerDataForSignatureAndHost(signature, host1)); + assertNull(repository.getArbitraryRepository().getArbitraryPeerDataForSignatureAndHost(signature, host2)); + + // Now add this mapping to the db + Peer peer = new Peer(new PeerData(PeerAddress.fromString(peerAddress1))); + ArbitraryPeerData arbitraryPeerData = new ArbitraryPeerData(signature, peer); + repository.getArbitraryRepository().save(arbitraryPeerData); + + // We should now have an entry for this hash/peer combination + ArbitraryPeerData retrievedArbitraryPeerData = repository.getArbitraryRepository() + .getArbitraryPeerDataForSignatureAndHost(signature, host1); + assertNotNull(retrievedArbitraryPeerData); + + // And we should also have an entry for the similar peerAddress string with a matching host + ArbitraryPeerData retrievedArbitraryPeerData2 = repository.getArbitraryRepository() + .getArbitraryPeerDataForSignatureAndHost(signature, host2); + assertNotNull(retrievedArbitraryPeerData2); + } + } }