From b0733d72bad76bc5d8df2f4a7792ebb2539ebdc8 Mon Sep 17 00:00:00 2001 From: Peter Doornbosch Date: Fri, 10 Jan 2025 18:55:26 +0100 Subject: [PATCH] use secure hash for ConnectionSource, the key in the connection registry hashtable --- .../buildlogic.java-common-conventions.gradle | 3 + core/build.gradle | 3 +- core/src/main/java/module-info.java | 1 + .../core/server/impl/ConnectionSource.java | 7 +- .../impl/ServerConnectionRegistryImpl.java | 27 ++++-- .../java/tech/kwik/core/util/SecureHash.java | 36 ++++++++ .../tech/kwik/core/util/SecureHashTest.java | 82 +++++++++++++++++++ 7 files changed, 148 insertions(+), 11 deletions(-) create mode 100644 core/src/main/java/tech/kwik/core/util/SecureHash.java create mode 100644 core/src/test/java/tech/kwik/core/util/SecureHashTest.java diff --git a/buildSrc/src/main/groovy/buildlogic.java-common-conventions.gradle b/buildSrc/src/main/groovy/buildlogic.java-common-conventions.gradle index 47a8fea9..db883434 100644 --- a/buildSrc/src/main/groovy/buildlogic.java-common-conventions.gradle +++ b/buildSrc/src/main/groovy/buildlogic.java-common-conventions.gradle @@ -22,6 +22,9 @@ javadoc { } repositories { + flatDir { + dirs "$rootDir/localdeps" + } mavenCentral() } diff --git a/core/build.gradle b/core/build.gradle index fdad43df..98880c07 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -2,12 +2,13 @@ plugins { id 'buildlogic.java-common-conventions' } - dependencies { implementation group: 'tech.kwik', name: 'agent15', version: "$agent15_version" // https://mvnrepository.com/artifact/at.favre.lib/hkdf implementation group: 'at.favre.lib', name: 'hkdf', version: '2.0.0' + + implementation group: 'io.whitfin', name: 'siphash', version: '2.0.0-auto-module' } task includeVersion { diff --git a/core/src/main/java/module-info.java b/core/src/main/java/module-info.java index 9e98f049..d393492a 100644 --- a/core/src/main/java/module-info.java +++ b/core/src/main/java/module-info.java @@ -1,6 +1,7 @@ module tech.kwik.core { requires tech.kwik.agent15; requires at.favre.lib.hkdf; + requires io.whitfin.siphash; exports tech.kwik.core; exports tech.kwik.core.concurrent; diff --git a/core/src/main/java/tech/kwik/core/server/impl/ConnectionSource.java b/core/src/main/java/tech/kwik/core/server/impl/ConnectionSource.java index b88bc7b6..f1e645c4 100644 --- a/core/src/main/java/tech/kwik/core/server/impl/ConnectionSource.java +++ b/core/src/main/java/tech/kwik/core/server/impl/ConnectionSource.java @@ -19,20 +19,23 @@ package tech.kwik.core.server.impl; import tech.kwik.core.util.Bytes; +import tech.kwik.core.util.SecureHash; import java.util.Arrays; public class ConnectionSource { private final byte[] dcid; + private final int hashCode; - public ConnectionSource(byte[] dcid) { + public ConnectionSource(byte[] dcid, SecureHash secureHash) { this.dcid = dcid; + hashCode = secureHash.generateHashCode(dcid); } @Override public int hashCode() { - return Arrays.hashCode(dcid); + return hashCode; } @Override diff --git a/core/src/main/java/tech/kwik/core/server/impl/ServerConnectionRegistryImpl.java b/core/src/main/java/tech/kwik/core/server/impl/ServerConnectionRegistryImpl.java index 3b143990..871c72fa 100644 --- a/core/src/main/java/tech/kwik/core/server/impl/ServerConnectionRegistryImpl.java +++ b/core/src/main/java/tech/kwik/core/server/impl/ServerConnectionRegistryImpl.java @@ -21,8 +21,10 @@ import tech.kwik.core.log.Logger; import tech.kwik.core.server.ServerConnectionRegistry; import tech.kwik.core.util.Bytes; +import tech.kwik.core.util.SecureHash; import java.net.InetSocketAddress; +import java.security.SecureRandom; import java.time.Duration; import java.util.Comparator; import java.util.List; @@ -43,6 +45,7 @@ public class ServerConnectionRegistryImpl implements ServerConnectionRegistry { private final Map currentConnections; private final Lock checkEmptyLock; private final Condition allConnectionsClosed; + private final SecureHash secureHash; ServerConnectionRegistryImpl(Logger log) { this.log = log; @@ -50,24 +53,32 @@ public class ServerConnectionRegistryImpl implements ServerConnectionRegistry { checkEmptyLock = new ReentrantLock(); allConnectionsClosed = checkEmptyLock.newCondition(); + + byte[] seed = new byte[16]; + new SecureRandom().nextBytes(seed); + secureHash = new SecureHash(seed); } @Override public void registerConnection(ServerConnectionProxy connection, byte[] connectionId) { - currentConnections.put(new ConnectionSource(connectionId), connection); + currentConnections.put(connectionSource(connectionId), connection); + } + + private ConnectionSource connectionSource(byte[] connectionId) { + return new ConnectionSource(connectionId, secureHash); } @Override public void deregisterConnection(ServerConnectionProxy connection, byte[] connectionId) { - currentConnections.remove(new ConnectionSource(connectionId)); + currentConnections.remove(connectionSource(connectionId)); checkAllConnectionsClosed(); } @Override public void registerAdditionalConnectionId(byte[] currentConnectionId, byte[] newConnectionId) { - ServerConnectionProxy connection = currentConnections.get(new ConnectionSource(currentConnectionId)); + ServerConnectionProxy connection = currentConnections.get(connectionSource(currentConnectionId)); if (connection != null) { - currentConnections.put(new ConnectionSource(newConnectionId), connection); + currentConnections.put(connectionSource(newConnectionId), connection); } else { log.error("Cannot add additional cid to non-existing connection " + Bytes.bytesToHex(currentConnectionId)); @@ -76,21 +87,21 @@ public void registerAdditionalConnectionId(byte[] currentConnectionId, byte[] ne @Override public void deregisterConnectionId(byte[] connectionId) { - currentConnections.remove(new ConnectionSource(connectionId)); + currentConnections.remove(connectionSource(connectionId)); checkAllConnectionsClosed(); } Optional isExistingConnection(InetSocketAddress clientAddress, byte[] dcid) { - return Optional.ofNullable(currentConnections.get(new ConnectionSource(dcid))); + return Optional.ofNullable(currentConnections.get(connectionSource(dcid))); } ServerConnectionProxy removeConnection(ServerConnectionImpl connection) { // Remove the entry this is registered with the original dcid - ServerConnectionProxy removed = currentConnections.remove(new ConnectionSource(connection.getOriginalDestinationConnectionId())); + ServerConnectionProxy removed = currentConnections.remove(connectionSource(connection.getOriginalDestinationConnectionId())); // Remove all entries that are registered with the active cids List removedConnections = connection.getActiveConnectionIds().stream() - .map(cid -> new ConnectionSource(cid)) + .map(cid -> connectionSource(cid)) .map(cs -> currentConnections.remove(cs)) .filter(Objects::nonNull) .collect(Collectors.toList()); diff --git a/core/src/main/java/tech/kwik/core/util/SecureHash.java b/core/src/main/java/tech/kwik/core/util/SecureHash.java new file mode 100644 index 00000000..6e3ff7c5 --- /dev/null +++ b/core/src/main/java/tech/kwik/core/util/SecureHash.java @@ -0,0 +1,36 @@ +/* + * Copyright © 2025 Peter Doornbosch + * + * This file is part of Kwik, an implementation of the QUIC protocol in Java. + * + * Kwik is free software: you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * Kwik is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for + * more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ +package tech.kwik.core.util; + +import io.whitfin.siphash.SipHasher; +import io.whitfin.siphash.SipHasherContainer; + +public class SecureHash { + + private final SipHasherContainer container; + + public SecureHash(byte[] key) { + container = SipHasher.container(key); + } + + public int generateHashCode(byte[] dcid) { + long longHash = container.hash(dcid); + return (int)(longHash ^ (longHash >>> 32)); + } +} diff --git a/core/src/test/java/tech/kwik/core/util/SecureHashTest.java b/core/src/test/java/tech/kwik/core/util/SecureHashTest.java new file mode 100644 index 00000000..15a9324c --- /dev/null +++ b/core/src/test/java/tech/kwik/core/util/SecureHashTest.java @@ -0,0 +1,82 @@ +/* + * Copyright © 2025 Peter Doornbosch + * + * This file is part of Kwik, an implementation of the QUIC protocol in Java. + * + * Kwik is free software: you can redistribute it and/or modify it under + * the terms of the GNU Lesser General Public License as published by the + * Free Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * Kwik is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for + * more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + */ +package tech.kwik.core.util; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import tech.kwik.core.test.ByteUtils; + +import java.security.SecureRandom; + +import static org.assertj.core.api.Assertions.assertThat; + + +class SecureHashTest { + + private SecureHash secureHash; + + @BeforeEach + void setUp() { + byte[] key = "1234567890123456".getBytes(); + secureHash = new SecureHash(key); + } + + @Test + void generatingHashForSameInputShouldReturnSameHash() { + // Given + byte[] input = ByteUtils.hexToBytes("cd8330cac6107e88"); + + // When + int hash1 = secureHash.generateHashCode(input); + int hash2 = secureHash.generateHashCode(input); + + // Then + assertThat(hash1).isEqualTo(hash2); + } + + @Test + void generatingHashForDifferentInputsShouldReturnDifferentHashes() { + // Given + byte[] input1 = ByteUtils.hexToBytes("cd8330cac6107e88"); + byte[] input2 = ByteUtils.hexToBytes("07e89cd8330cac61"); + // When + int hash1 = secureHash.generateHashCode(input1); + int hash2 = secureHash.generateHashCode(input2); + + // Then + assertThat(hash1).isNotEqualTo(hash2); + } + + @Test + void generatingHashForSameInputButDifferentSeedsShouldReturnDifferentHash() { + // Given + byte[] key = new byte[16]; + new SecureRandom().nextBytes(key); + SecureHash secureHash2 = new SecureHash(key); + + byte[] input = ByteUtils.hexToBytes("cd8330cac6107e88"); + + // When + int hash1 = secureHash.generateHashCode(input); + int hash2 = secureHash2.generateHashCode(input); + + // Then + assertThat(hash1).isNotEqualTo(hash2); + } +} \ No newline at end of file