From a49f87179a99abdf9d5a080fcccdc42a398f406e Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Sun, 25 Oct 2020 10:55:53 +0000
Subject: [PATCH] Router: Quick checks of eph. key MSB before Noise DH
Additional checks on ECIES BRR to catch old/buggy routers Detailed logging of
ECIES BRR decrypt fails
---
.../net/i2p/data/i2np/BuildRequestRecord.java | 32 +++++++++++++++---
.../crypto/ratchet/ECIESAEADEngine.java | 33 +++++++++++--------
.../transport/ntcp/InboundEstablishState.java | 21 +++++++-----
3 files changed, 61 insertions(+), 25 deletions(-)
diff --git a/router/java/src/net/i2p/data/i2np/BuildRequestRecord.java b/router/java/src/net/i2p/data/i2np/BuildRequestRecord.java
index 53db1a3ca7..fdea2f388b 100644
--- a/router/java/src/net/i2p/data/i2np/BuildRequestRecord.java
+++ b/router/java/src/net/i2p/data/i2np/BuildRequestRecord.java
@@ -18,6 +18,7 @@ import net.i2p.data.Hash;
import net.i2p.data.PrivateKey;
import net.i2p.data.PublicKey;
import net.i2p.data.SessionKey;
+import net.i2p.util.Log;
import net.i2p.router.RouterContext;
/**
@@ -113,6 +114,8 @@ public class BuildRequestRecord {
/** we show 16 bytes of the peer hash outside the elGamal block */
public static final int PEER_SIZE = 16;
private static final int DEFAULT_EXPIRATION_SECONDS = 10*60;
+ private static final int EC_LEN = EncType.ECIES_X25519.getPubkeyLen();
+ private static final byte[] NULL_KEY = new byte[EC_LEN];
/**
* @return 222 (ElG) or 464 (ECIES) bytes, non-null
@@ -379,15 +382,31 @@ public class BuildRequestRecord {
*/
public BuildRequestRecord(RouterContext ctx, PrivateKey ourKey,
EncryptedBuildRecord encryptedRecord) throws DataFormatException {
+ byte[] encrypted = encryptedRecord.getData();
byte decrypted[];
EncType type = ourKey.getType();
if (type == EncType.ELGAMAL_2048) {
byte preDecrypt[] = new byte[514];
- System.arraycopy(encryptedRecord.getData(), PEER_SIZE, preDecrypt, 1, 256);
- System.arraycopy(encryptedRecord.getData(), PEER_SIZE + 256, preDecrypt, 258, 256);
+ System.arraycopy(encrypted, PEER_SIZE, preDecrypt, 1, 256);
+ System.arraycopy(encrypted, PEER_SIZE + 256, preDecrypt, 258, 256);
decrypted = ctx.elGamalEngine().decrypt(preDecrypt, ourKey);
_isEC = false;
} else if (type == EncType.ECIES_X25519) {
+ // There's several reasons to get bogus-encrypted requests:
+ // very old Java and i2pd routers that don't check the type at all and send ElG,
+ // i2pd treating type 4 like type 1,
+ // and very old i2pd routers like 0.9.32 that have all sorts of bugs.
+ // The following 3 checks weed out about 85% before we get to the DH.
+
+ // fast MSB check for key < 2^255
+ if ((encrypted[PEER_SIZE + EC_LEN - 1] & 0x80) != 0)
+ throw new DataFormatException("Bad PK decrypt fail");
+ // i2pd 0.9.46/47 bug, treating us like type 1
+ if (DataHelper.eq(ourKey.toPublic().getData(), 0, encrypted, PEER_SIZE, EC_LEN))
+ throw new DataFormatException("Our PK decrypt fail");
+ // very old i2pd bug?
+ if (DataHelper.eq(NULL_KEY, 0, encrypted, PEER_SIZE, EC_LEN))
+ throw new DataFormatException("Null PK decrypt fail");
HandshakeState state = null;
try {
KeyFactory kf = TEST ? TESTKF : ctx.commSystem().getXDHFactory();
@@ -396,13 +415,18 @@ public class BuildRequestRecord {
ourKey.toPublic().getData(), 0);
state.start();
decrypted = new byte[LENGTH_EC];
- state.readMessage(encryptedRecord.getData(), PEER_SIZE, EncryptedBuildRecord.LENGTH - PEER_SIZE,
+ state.readMessage(encrypted, PEER_SIZE, EncryptedBuildRecord.LENGTH - PEER_SIZE,
decrypted, 0);
_chachaReplyKey = new SessionKey(state.getChainingKey());
_chachaReplyAD = new byte[32];
System.arraycopy(state.getHandshakeHash(), 0, _chachaReplyAD, 0, 32);
} catch (GeneralSecurityException gse) {
- throw new DataFormatException("decrypt fail", gse);
+ if (state != null) {
+ Log log = ctx.logManager().getLog(BuildRequestRecord.class);
+ if (log.shouldInfo())
+ log.info("ECIES BRR decrypt failure, state at failure:\n" + state);
+ }
+ throw new DataFormatException("ChaCha decrypt fail", gse);
} finally {
if (state != null)
state.destroy();
diff --git a/router/java/src/net/i2p/router/crypto/ratchet/ECIESAEADEngine.java b/router/java/src/net/i2p/router/crypto/ratchet/ECIESAEADEngine.java
index c0e9114aed..de88fee78d 100644
--- a/router/java/src/net/i2p/router/crypto/ratchet/ECIESAEADEngine.java
+++ b/router/java/src/net/i2p/router/crypto/ratchet/ECIESAEADEngine.java
@@ -350,18 +350,6 @@ public final class ECIESAEADEngine {
*/
private CloveSet decryptNewSession(byte data[], PrivateKey targetPrivateKey, RatchetSKM keyManager)
throws DataFormatException {
- HandshakeState state;
- try {
- state = new HandshakeState(HandshakeState.PATTERN_ID_IK, HandshakeState.RESPONDER, _edhThread);
- } catch (GeneralSecurityException gse) {
- throw new IllegalStateException("bad proto", gse);
- }
- state.getLocalKeyPair().setKeys(targetPrivateKey.getData(), 0,
- targetPrivateKey.toPublic().getData(), 0);
- state.start();
- if (_log.shouldDebug())
- _log.debug("State before decrypt new session: " + state);
-
// Elg2
byte[] xx = new byte[KEYLEN];
System.arraycopy(data, 0, xx, 0, KEYLEN);
@@ -373,12 +361,31 @@ public final class ECIESAEADEngine {
if (_log.shouldDebug())
_log.debug("Elg2 decode fail NS");
data[KEYLEN - 1] = xx31;
- state.destroy();
return null;
}
+ // fast MSB check for key < 2^255
+ if ((pk.getData()[KEYLEN - 1] & 0x80) != 0) {
+ if (_log.shouldDebug())
+ _log.debug("Bad PK decode fail NS");
+ data[KEYLEN - 1] = xx31;
+ return null;
+ }
+
// rewrite in place, must restore below on failure
System.arraycopy(pk.getData(), 0, data, 0, KEYLEN);
+ HandshakeState state;
+ try {
+ state = new HandshakeState(HandshakeState.PATTERN_ID_IK, HandshakeState.RESPONDER, _edhThread);
+ } catch (GeneralSecurityException gse) {
+ throw new IllegalStateException("bad proto", gse);
+ }
+ state.getLocalKeyPair().setKeys(targetPrivateKey.getData(), 0,
+ targetPrivateKey.toPublic().getData(), 0);
+ state.start();
+ if (_log.shouldDebug())
+ _log.debug("State before decrypt new session: " + state);
+
int payloadlen = data.length - (KEYLEN + KEYLEN + MACLEN + MACLEN);
byte[] payload = new byte[payloadlen];
try {
diff --git a/router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java b/router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java
index 1bd397cc04..c391b4708b 100644
--- a/router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java
+++ b/router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java
@@ -672,7 +672,6 @@ class InboundEstablishState extends EstablishBase implements NTCP2Payload.Payloa
}
changeState(State.IB_NTCP2_GOT_X);
_received = 0;
-
// replay check using encrypted key
if (!_transport.isHXHIValid(_X)) {
_context.statManager().addRateData("ntcp.replayHXxorBIH", 1);
@@ -680,13 +679,6 @@ class InboundEstablishState extends EstablishBase implements NTCP2Payload.Payloa
return;
}
- try {
- _handshakeState = new HandshakeState(HandshakeState.PATTERN_ID_XK, HandshakeState.RESPONDER, _transport.getXDHFactory());
- } catch (GeneralSecurityException gse) {
- throw new IllegalStateException("bad proto", gse);
- }
- _handshakeState.getLocalKeyPair().setKeys(_transport.getNTCP2StaticPrivkey(), 0,
- _transport.getNTCP2StaticPubkey(), 0);
Hash h = _context.routerHash();
SessionKey bobHash = new SessionKey(h.getData());
// save encrypted data for CBC for msg 2
@@ -696,6 +688,19 @@ class InboundEstablishState extends EstablishBase implements NTCP2Payload.Payloa
fail("Bad msg 1, X = 0");
return;
}
+ // fast MSB check for key < 2^255
+ if ((_X[KEY_SIZE - 1] & 0x80) != 0) {
+ fail("Bad PK msg 1");
+ return;
+ }
+
+ try {
+ _handshakeState = new HandshakeState(HandshakeState.PATTERN_ID_XK, HandshakeState.RESPONDER, _transport.getXDHFactory());
+ } catch (GeneralSecurityException gse) {
+ throw new IllegalStateException("bad proto", gse);
+ }
+ _handshakeState.getLocalKeyPair().setKeys(_transport.getNTCP2StaticPrivkey(), 0,
+ _transport.getNTCP2StaticPubkey(), 0);
byte options[] = new byte[OPTIONS1_SIZE];
try {
_handshakeState.start();
--
GitLab