From 4ef4ae4df58b48fbb5b1292b924a011933f25251 Mon Sep 17 00:00:00 2001
From: zzz <zzz@i2pmail.org>
Date: Mon, 30 Jan 2023 13:21:10 -0500
Subject: [PATCH] Streaming: Refactor sig checking
Save sig OK status in Packet
Fix spot where byte array cache was not used
Do not send NACK 0 for retransmitted SYNs
Remove 1-byte DataHelper.toLong() calls
---
.../streaming/impl/ConnectionManager.java | 22 ++++++++++++++++---
.../impl/ConnectionPacketHandler.java | 4 +++-
.../streaming/impl/MessageInputStream.java | 11 +++++++---
.../net/i2p/client/streaming/impl/Packet.java | 16 ++++++++------
4 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionManager.java b/apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionManager.java
index bb479e0bf4..fbb6703cbf 100644
--- a/apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionManager.java
+++ b/apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionManager.java
@@ -16,6 +16,7 @@ import net.i2p.data.DataHelper;
import net.i2p.data.Destination;
import net.i2p.data.Hash;
import net.i2p.data.SessionKey;
+import net.i2p.util.ByteCache;
import net.i2p.util.ConcurrentHashSet;
import net.i2p.util.ConvertToHash;
import net.i2p.util.LHMCache;
@@ -54,6 +55,7 @@ class ConnectionManager {
/** since 0.9, each manager instantiates its own timer */
private final SimpleTimer2 _timer;
private final Map<Long, Object> _recentlyClosed;
+ private final ByteCache _cache = ByteCache.getInstance(32, 4*1024);
private static final Object DUMMY = new Object();
/** cache of the property to detect changes */
@@ -84,6 +86,8 @@ class ConnectionManager {
/**
* Manage all conns for this session
+ *
+ * @param session the primary session, packets may come in on subsessions also
*/
public ConnectionManager(I2PAppContext context,
I2PSession session,
@@ -237,6 +241,21 @@ class ConnectionManager {
* or if the connection was rejected
*/
public Connection receiveConnection(Packet synPacket) {
+ Destination from = synPacket.getOptionalFrom();
+ if (from == null) {
+ if (_log.shouldWarn())
+ _log.warn("SYN w/o FROM: " + synPacket);
+ return null;
+ }
+ ByteArray ba = _cache.acquire();
+ boolean sigOk = synPacket.verifySignature(_context, ba.getData());
+ _cache.release(ba);
+ if (!sigOk) {
+ if (_log.shouldWarn())
+ _log.warn("Received unsigned / forged SYN apparently from " + from.toBase32() + ": " + synPacket);
+ return null;
+ }
+
boolean reject = false;
int retryAfter = 0;
@@ -261,9 +280,6 @@ class ConnectionManager {
_context.statManager().addRateData("stream.receiveActive", 1);
if (reject) {
- Destination from = synPacket.getOptionalFrom();
- if (from == null)
- return null;
String resp = _defaultOptions.getLimitAction();
if ("drop".equals(resp)) {
// always drop
diff --git a/apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionPacketHandler.java b/apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionPacketHandler.java
index 278f3d653b..692f2a2b2a 100644
--- a/apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionPacketHandler.java
+++ b/apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionPacketHandler.java
@@ -285,6 +285,7 @@ class ConnectionPacketHandler {
if (isSYN && (packet.getSendStreamId() <= 0) ) {
// don't honor the ACK 0 in SYN packets received when the other side
// has obviously not seen our messages
+ // and ignore NACKs
fastAck = false;
} else {
fastAck = ack(con, packet.getAckThrough(), packet.getNacks(), packet, isNew, choke);
@@ -650,8 +651,9 @@ class ConnectionPacketHandler {
packet.isFlagSet(Packet.FLAG_SYNCHRONIZE | Packet.FLAG_CLOSE | Packet.FLAG_SIGNATURE_INCLUDED)) {
// we need a valid signature
SigningPublicKey spk = con.getRemoteSPK();
+ // inbound SYNs are now verifed in ConnectionManager; Packet caches the result
ByteArray ba = _cache.acquire();
- boolean sigOk = packet.verifySignature(_context, spk, null);
+ boolean sigOk = packet.verifySignature(_context, spk, ba.getData());
_cache.release(ba);
if (!sigOk) {
throw new I2PException("Received unsigned / forged packet: " + packet);
diff --git a/apps/streaming/java/src/net/i2p/client/streaming/impl/MessageInputStream.java b/apps/streaming/java/src/net/i2p/client/streaming/impl/MessageInputStream.java
index 9c1cae9981..eed12b4444 100644
--- a/apps/streaming/java/src/net/i2p/client/streaming/impl/MessageInputStream.java
+++ b/apps/streaming/java/src/net/i2p/client/streaming/impl/MessageInputStream.java
@@ -214,9 +214,14 @@ class MessageInputStream extends InputStream {
* Adds the ack-through and nack fields to a packet we are building for transmission
*/
public void updateAcks(PacketLocal packet) {
- synchronized (_dataLock) {
- packet.setAckThrough(_highestBlockId);
- packet.setNacks(locked_getNacks());
+ if (packet.getSendStreamId() > 0 || !packet.isFlagSet(Packet.FLAG_SYNCHRONIZE)) {
+ synchronized (_dataLock) {
+ packet.setAckThrough(_highestBlockId);
+ packet.setNacks(locked_getNacks());
+ }
+ } else {
+ // do not send NACK 0 for retransmitted SYNs
+ packet.setAckThrough(-1);
}
}
diff --git a/apps/streaming/java/src/net/i2p/client/streaming/impl/Packet.java b/apps/streaming/java/src/net/i2p/client/streaming/impl/Packet.java
index 42c0af698a..09fe561515 100644
--- a/apps/streaming/java/src/net/i2p/client/streaming/impl/Packet.java
+++ b/apps/streaming/java/src/net/i2p/client/streaming/impl/Packet.java
@@ -79,6 +79,7 @@ class Packet {
private int _resendDelay;
private int _flags;
private ByteArray _payload;
+ private boolean _sigVerified;
// the next four are set only if the flags say so
protected Signature _optionSignature;
protected Destination _optionFrom;
@@ -466,18 +467,15 @@ class Packet {
cur += 4;
if (_nacks != null) {
// if max win is ever > 255, limit to 255
- DataHelper.toLong(buffer, cur, 1, _nacks.length);
- cur++;
+ buffer[cur++] = (byte) _nacks.length;
for (int i = 0; i < _nacks.length; i++) {
DataHelper.toLong(buffer, cur, 4, _nacks[i]);
cur += 4;
}
} else {
- DataHelper.toLong(buffer, cur, 1, 0);
- cur++;
+ buffer[cur++] = 0;
}
- DataHelper.toLong(buffer, cur, 1, _resendDelay > 0 ? _resendDelay : 0);
- cur++;
+ buffer[cur++] = _resendDelay > 0 ? ((byte) _resendDelay) : 0;
DataHelper.toLong(buffer, cur, 2, _flags);
cur += 2;
@@ -782,6 +780,8 @@ class Packet {
* false otherwise.
*/
public boolean verifySignature(I2PAppContext ctx, SigningPublicKey altSPK, byte buffer[]) {
+ if (_sigVerified)
+ return true;
if (!isFlagSet(FLAG_SIGNATURE_INCLUDED)) return false;
if (_optionSignature == null) return false;
SigningPublicKey spk = _optionFrom != null ? _optionFrom.getSigningPublicKey() : altSPK;
@@ -852,7 +852,9 @@ class Packet {
l.warn("Signature failed on " + toString(), iae);
ok = false;
}
- if (!ok) {
+ if (ok) {
+ _sigVerified = true;
+ } else {
Log l = ctx.logManager().getLog(Packet.class);
if (l.shouldLog(Log.WARN))
l.warn("Signature failed on " + toString() + " using SPK " + spk);
--
GitLab