Fixes after testing

- NSR corruption
- Track SKM key type
- Implement cloning of hybrid keys
- Min size checks
- Session validation fix
- Logging in HandshakeState, probably temporary
- Don't clone destroyed state
- Javadoc fixes
This commit is contained in:
zzz
2025-04-14 13:55:01 -04:00
parent d06b776189
commit c1ca38dca2
5 changed files with 178 additions and 59 deletions

View File

@ -110,7 +110,7 @@ public enum EncType {
/**
* For internal use only (Bob side ciphertext)
* Proposal 169.
* Pubkey 800 bytes; privkey 0
* Pubkey 768 bytes; privkey 0
* @since 0.9.80
*/
MLKEM512_X25519_CT(100008, 768, 0, EncAlgo.ECIES_MLKEM_INT, "EC/None/NoPadding", X25519_SPEC, "0.9.80"),
@ -118,7 +118,7 @@ public enum EncType {
/**
* For internal use only (Bob side ciphertext)
* Proposal 169.
* Pubkey 1184 bytes; privkey 0
* Pubkey 1088 bytes; privkey 0
* @since 0.9.80
*/
MLKEM768_X25519_CT(100009, 1088, 0, EncAlgo.ECIES_MLKEM_INT, "EC/None/NoPadding", X25519_SPEC, "0.9.80"),

View File

@ -28,7 +28,10 @@ import java.util.Arrays;
import javax.crypto.BadPaddingException;
import javax.crypto.ShortBufferException;
import net.i2p.I2PAppContext;
import net.i2p.crypto.KeyFactory;
import net.i2p.util.HexDump;
import net.i2p.util.Log;
/**
* Interface to a Noise handshake.
@ -37,6 +40,7 @@ public class HandshakeState implements Destroyable, Cloneable {
private final SymmetricState symmetric;
private final boolean isInitiator;
private final Log _log;
private DHState localKeyPair;
private DHState localEphemeral;
private DHState localHybrid;
@ -47,6 +51,7 @@ public class HandshakeState implements Destroyable, Cloneable {
private final int requirements;
private int patternIndex;
private boolean wasCloned;
private boolean isDestroyed;
/**
* Enumerated value that indicates that the handshake object
@ -305,6 +310,7 @@ public class HandshakeState implements Destroyable, Cloneable {
*/
public HandshakeState(String patternId, int role, KeyFactory xdh, KeyFactory hdh) throws NoSuchAlgorithmException
{
_log = I2PAppContext.getGlobalContext().logManager().getLog(HandshakeState.class);
this.patternId = patternId;
if (patternId.equals(PATTERN_ID_XK))
pattern = PATTERN_XK;
@ -367,6 +373,7 @@ public class HandshakeState implements Destroyable, Cloneable {
* @since 0.9.44
*/
protected HandshakeState(HandshakeState o) throws CloneNotSupportedException {
_log = I2PAppContext.getGlobalContext().logManager().getLog(HandshakeState.class);
// everything is shallow copied except for symmetric state and keys
// so destroy() doesn't zero them out later
symmetric = o.symmetric.clone();
@ -393,6 +400,23 @@ public class HandshakeState implements Destroyable, Cloneable {
remotePublicKey = o.remotePublicKey.clone();
if (o.remoteEphemeral != null)
remoteEphemeral = o.remoteEphemeral.clone();
if (o.localHybrid != null) {
if (isInitiator) {
// always save Alice's local keys
localHybrid = o.localHybrid.clone();
} else {
if (o.wasCloned) {
// new keys after first time for Bob
localHybrid = o.localHybrid.clone();
} else {
// first time for Bob, use the eph. keys previously generated
localHybrid = o.localHybrid;
o.wasCloned = true;
}
}
}
if (o.remoteHybrid != null)
remoteHybrid = o.remoteHybrid.clone();
action = o.action;
if (action == SPLIT || action == COMPLETE)
throw new CloneNotSupportedException("clone after NSR");
@ -802,8 +826,8 @@ public class HandshakeState implements Destroyable, Cloneable {
if (localHybrid == null)
throw new IllegalStateException("Pattern definition error");
byte[] shared = null;
//System.out.println("State before writing F");
//System.out.println(toString());
if (_log.shouldWarn())
_log.warn("State before writing F: " + this);
if (isInitiator) {
// Only Alice generates a keypair
localHybrid.generateKeyPair();
@ -818,8 +842,8 @@ public class HandshakeState implements Destroyable, Cloneable {
shared = new byte [len];
// this creates the ciphertext and puts it in localHybrid.publicKey
localHybrid.calculate(shared, 0, remoteHybrid);
//System.out.println("State middle of F, len=" + len);
//System.out.println(toString());
if (_log.shouldWarn())
_log.warn("State middle of F: " + this);
}
len = localHybrid.getPublicKeyLength();
macLen = symmetric.getMACLength();
@ -833,8 +857,10 @@ public class HandshakeState implements Destroyable, Cloneable {
symmetric.mixKey(shared, 0, shared.length);
Noise.destroy(shared);
}
//System.out.println("State after F, len=" + len);
//System.out.println(toString());
if (_log.shouldWarn())
_log.warn("Dump:\n" + HexDump.dump(message, messagePosn - (len + macLen), len + macLen));
if (_log.shouldWarn())
_log.warn("State after F, len=" + (len + macLen) + ": " + this);
}
break;
@ -1036,10 +1062,11 @@ public class HandshakeState implements Destroyable, Cloneable {
if (remoteHybrid == null)
throw new IllegalStateException("Pattern definition error");
len = remoteHybrid.getPublicKeyLength();
//System.out.println("State before reading F");
//System.out.println(toString());
//System.out.println("State: F, reading remote eph. key len=" + len);
macLen = symmetric.getMACLength();
if (_log.shouldWarn())
_log.warn("State before reading F, len=" + (len + macLen) + ": " + this);
if (_log.shouldWarn())
_log.warn("Dump:\n" + HexDump.dump(message, messageOffset, len + macLen));
if (space < (len + macLen))
throw new ShortBufferException();
byte[] temp = new byte [len];
@ -1050,8 +1077,8 @@ public class HandshakeState implements Destroyable, Cloneable {
} finally {
Noise.destroy(temp);
}
//System.out.println("State after F");
//System.out.println(toString());
if (_log.shouldWarn())
_log.warn("State after F, len=" + len + ": " + this);
messageOffset += len + macLen;
}
break;
@ -1061,8 +1088,8 @@ public class HandshakeState implements Destroyable, Cloneable {
// DH operation with initiator and responder hybrid keys.
// We are Alice.
mixDH(localHybrid, remoteHybrid);
//System.out.println("State after FF");
//System.out.println(toString());
if (_log.shouldWarn())
_log.warn("State after FF : " + this);
}
break;
@ -1161,7 +1188,8 @@ public class HandshakeState implements Destroyable, Cloneable {
}
@Override
public void destroy() {
public synchronized void destroy() {
isDestroyed = true;
if (symmetric != null)
symmetric.destroy();
if (localKeyPair != null)
@ -1236,6 +1264,8 @@ public class HandshakeState implements Destroyable, Cloneable {
*/
@Override
public synchronized HandshakeState clone() throws CloneNotSupportedException {
if (isDestroyed)
throw new IllegalStateException("destroyed");
return new HandshakeState(this);
}

View File

@ -25,6 +25,7 @@ import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import net.i2p.client.I2PClient;
import net.i2p.crypto.EncType;
import net.i2p.crypto.SessionKeyManager;
import net.i2p.data.DatabaseEntry;
import net.i2p.data.DataHelper;
@ -647,13 +648,14 @@ class ClientConnectionRunner {
_sessionKeyManager = tskm;
}
} else if (hasPQ) {
// fixmeeeeeeeeeeeeeeeeeeeeeee
if (hasEC) {
// ECIES
RatchetSKM rskm1 = new RatchetSKM(_context, dest);
RatchetSKM rskm2 = new RatchetSKM(_context, dest);
// PQ
RatchetSKM rskm2 = new RatchetSKM(_context, dest, EncType.getByCode(pqType));
_sessionKeyManager = new MuxedPQSKM(rskm1, rskm2);
} else {
_sessionKeyManager = new RatchetSKM(_context, dest);
_sessionKeyManager = new RatchetSKM(_context, dest, EncType.getByCode(pqType));
}
} else {
if (hasEC) {

View File

@ -99,6 +99,12 @@ public final class ECIESAEADEngine {
private static final int MIN_NS_MLKEM768_SIZE = EncType.MLKEM768_X25519_INT.getPubkeyLen() + NS_MLKEM_OVERHEAD + DATETIME_SIZE;
// 1568 + 112 + 7 = 1687
private static final int MIN_NS_MLKEM1024_SIZE = EncType.MLKEM1024_X25519_INT.getPubkeyLen() + NS_MLKEM_OVERHEAD + DATETIME_SIZE;
// 856
private static final int MIN_NSR_MLKEM512_SIZE = EncType.MLKEM512_X25519_CT.getPubkeyLen() + MIN_NSR_SIZE + MACLEN;
// 1176
private static final int MIN_NSR_MLKEM768_SIZE = EncType.MLKEM768_X25519_CT.getPubkeyLen() + MIN_NSR_SIZE + MACLEN;
// 1656
private static final int MIN_NSR_MLKEM1024_SIZE = EncType.MLKEM1024_X25519_CT.getPubkeyLen() + MIN_NSR_SIZE + MACLEN;
/**
@ -294,14 +300,22 @@ public final class ECIESAEADEngine {
if (shouldDebug)
_log.debug("Decrypting ES with tag: " + st.toBase64() + " key: " + key + ": " + data.length + " bytes");
decrypted = decryptExistingSession(tag, data, key, targetPrivateKey, keyManager);
} else if (data.length >= MIN_NSR_SIZE) {
if (shouldDebug)
_log.debug("Decrypting NSR with tag: " + st.toBase64() + " key: " + key + ": " + data.length + " bytes");
decrypted = decryptNewSessionReply(tag, data, state, keyManager);
} else {
decrypted = null;
if (_log.shouldWarn())
_log.warn("ECIES decrypt fail, tag found but no state and too small for NSR: " + data.length + " bytes");
// it's important not to attempt decryption for too-short packets,
// because Noise will destroy() the handshake state on failure,
// and we don't clone() on the first one, so it's fatal.
EncType type = targetPrivateKey.getType();
int min = getMinNSRSize(type);
if (data.length >= min) {
if (shouldDebug)
_log.debug("Decrypting NSR with tag: " + st.toBase64() + " key: " + key + ": " + data.length + " bytes");
decrypted = decryptNewSessionReply(tag, data, state, keyManager);
} else {
decrypted = null;
if (_log.shouldWarn())
_log.warn("NSR decrypt fail, tag: " + st.toBase64() + " but packet too small: " + data.length + " bytes, min is " +
min + " on state " + state);
}
}
if (decrypted != null) {
_context.statManager().updateFrequency("crypto.eciesAEAD.decryptExistingSession");
@ -343,24 +357,8 @@ public final class ECIESAEADEngine {
private CloveSet x_decryptSlow(byte data[], PrivateKey targetPrivateKey,
RatchetSKM keyManager) throws DataFormatException {
CloveSet decrypted;
int minns;
switch(targetPrivateKey.getType()) {
case ECIES_X25519:
minns = MIN_NS_SIZE;
break;
case MLKEM512_X25519:
minns = MIN_NS_MLKEM512_SIZE;
break;
case MLKEM768_X25519:
minns = MIN_NS_MLKEM768_SIZE;
break;
case MLKEM1024_X25519:
minns = MIN_NS_MLKEM1024_SIZE;
break;
default:
throw new IllegalArgumentException();
}
EncType type = targetPrivateKey.getType();
int minns = getMinNSSize(type);
boolean isRouter = keyManager.getDestination() == null;
if (data.length >= minns || (isRouter && data.length >= MIN_NS_N_SIZE)) {
if (isRouter)
@ -378,7 +376,8 @@ public final class ECIESAEADEngine {
} else {
decrypted = null;
if (_log.shouldDebug())
_log.debug("ECIES decrypt fail, too small for NS: " + data.length + " bytes");
_log.debug("ECIES decrypt fail, too small for NS: " + data.length + " bytes, min is " +
minns + " for type " + type);
}
return decrypted;
}
@ -433,6 +432,60 @@ public final class ECIESAEADEngine {
}
}
/**
* @since 0.9.80
*/
private static int getMinNSSize(EncType type) {
switch(type) {
case ECIES_X25519:
return MIN_NS_SIZE;
case MLKEM512_X25519:
return MIN_NS_MLKEM512_SIZE;
case MLKEM768_X25519:
return MIN_NS_MLKEM768_SIZE;
case MLKEM1024_X25519:
return MIN_NS_MLKEM1024_SIZE;
default:
throw new IllegalArgumentException("No pattern for " + type);
}
}
/**
* @since 0.9.80
*/
private static int getMinNSRSize(EncType type) {
switch(type) {
case ECIES_X25519:
return MIN_NSR_SIZE;
case MLKEM512_X25519:
return MIN_NSR_MLKEM512_SIZE;
case MLKEM768_X25519:
return MIN_NSR_MLKEM768_SIZE;
case MLKEM1024_X25519:
return MIN_NSR_MLKEM1024_SIZE;
default:
throw new IllegalArgumentException("No pattern for " + type);
}
}
/**
* @since 0.9.80
*/
private static Set<EncType> getEncTypeSet(EncType type) {
switch(type) {
case ECIES_X25519:
return LeaseSetKeys.SET_EC;
case MLKEM512_X25519:
return LeaseSetKeys.SET_PQ1;
case MLKEM768_X25519:
return LeaseSetKeys.SET_PQ2;
case MLKEM1024_X25519:
return LeaseSetKeys.SET_PQ3;
default:
throw new IllegalArgumentException("No pattern for " + type);
}
}
/**
* scenario 1: New Session Message
*
@ -481,7 +534,8 @@ public final class ECIESAEADEngine {
EncType type = targetPrivateKey.getType();
try {
String pattern = getNoisePattern(type);
// Bob does not need a hybrid key factory
// Bob does not need a key factory
//state = new HandshakeState(pattern, HandshakeState.RESPONDER, _edhThread, getHybridKeyFactory(type));
state = new HandshakeState(pattern, HandshakeState.RESPONDER, _edhThread);
} catch (GeneralSecurityException gse) {
throw new IllegalStateException("bad proto", gse);
@ -490,7 +544,7 @@ public final class ECIESAEADEngine {
targetPrivateKey.toPublic().getData(), 0);
state.start();
if (_log.shouldDebug())
_log.debug("State before decrypt new session: " + state);
_log.debug("State before decrypt new session (" + data.length + " bytes) " + state);
int payloadlen = data.length - (KEYLEN + KEYLEN + MACLEN + MACLEN);
DHState hyb = state.getRemoteHybridKeyPair();
@ -504,7 +558,7 @@ public final class ECIESAEADEngine {
// we'll get this a lot on muxed SKM
// logged at INFO in caller
if (_log.shouldDebug())
_log.debug("Decrypt fail NS, state at failure: " + state, gse);
_log.debug("Decrypt fail NS " + data.length + " bytes, state at failure: " + state, gse);
// restore original data for subsequent ElG attempt
System.arraycopy(xx, 0, data, 0, KEYLEN - 1);
data[KEYLEN - 1] = xx31;
@ -569,7 +623,7 @@ public final class ECIESAEADEngine {
state.destroy();
} else {
// tell the SKM
PublicKey alice = new PublicKey(EncType.ECIES_X25519, alicePK);
PublicKey alice = new PublicKey(type, alicePK);
keyManager.createSession(alice, null, state, null);
setResponseTimerNS(alice, pc.cloveSet, keyManager);
}
@ -774,9 +828,16 @@ public final class ECIESAEADEngine {
byte[] encpayloadkey = new byte[32];
_hkdf.calculate(split.k_ba.getData(), ZEROLEN, INFO_6, encpayloadkey);
rcvr.initializeKey(encpayloadkey, 0);
byte[] payload = new byte[data.length - (TAGLEN + KEYLEN + MACLEN + MACLEN)];
int off = TAGLEN + KEYLEN + MACLEN;
int plen = data.length - (TAGLEN + KEYLEN + MACLEN + MACLEN);
if (hyb != null) {
int len = hyb.getPublicKeyLength() + MACLEN;
off += len;
plen -= len;
}
byte[] payload = new byte[plen];
try {
rcvr.decryptWithAd(hash, data, TAGLEN + KEYLEN + MACLEN, payload, 0, payload.length + MACLEN);
rcvr.decryptWithAd(hash, data, off, payload, 0, plen + MACLEN);
} catch (GeneralSecurityException gse) {
if (_log.shouldWarn()) {
_log.warn("Decrypt fail NSR part 2", gse);
@ -821,7 +882,7 @@ public final class ECIESAEADEngine {
}
// tell the SKM
PublicKey bob = new PublicKey(EncType.ECIES_X25519, bobPK);
PublicKey bob = new PublicKey(keyManager.getType(), bobPK);
keyManager.updateSession(bob, oldState, state, null, split);
if (pc == null)
@ -1223,8 +1284,12 @@ public final class ECIESAEADEngine {
byte[] encpayloadkey = new byte[32];
_hkdf.calculate(split.k_ba.getData(), ZEROLEN, INFO_6, encpayloadkey);
sender.initializeKey(encpayloadkey, 0);
int off = TAGLEN + KEYLEN + MACLEN;
if (hyb != null) {
off += hyb.getPublicKeyLength() + MACLEN;
}
try {
sender.encryptWithAd(hash, payload, 0, enc, TAGLEN + KEYLEN + MACLEN, payload.length);
sender.encryptWithAd(hash, payload, 0, enc, off, payload.length);
} catch (GeneralSecurityException gse) {
if (_log.shouldWarn())
_log.warn("Encrypt fail NSR part 2", gse);
@ -1608,7 +1673,7 @@ public final class ECIESAEADEngine {
return;
if (!ls2.isCurrent(Router.CLOCK_FUDGE_FACTOR))
continue;
PublicKey pk = ls2.getEncryptionKey(LeaseSetKeys.SET_EC);
PublicKey pk = ls2.getEncryptionKey(getEncTypeSet(from.getType()));
if (!from.equals(pk))
continue;
if (!ls2.verifySignature())

View File

@ -31,7 +31,6 @@ import net.i2p.data.PrivateKey;
import net.i2p.data.PublicKey;
import net.i2p.data.SessionKey;
import net.i2p.data.SessionTag;
import net.i2p.router.LeaseSetKeys;
import net.i2p.router.RouterContext;
import net.i2p.router.util.DecayingHashSet;
import net.i2p.util.Log;
@ -55,6 +54,7 @@ public class RatchetSKM extends SessionKeyManager implements SessionTagListener
private final HKDF _hkdf;
private final DecayingHashSet _replayFilter;
private final Destination _destination;
private final EncType _type;
/**
* Let outbound session tags sit around for this long before expiring them.
@ -84,7 +84,17 @@ public class RatchetSKM extends SessionKeyManager implements SessionTagListener
* @since 0.9.48
*/
public RatchetSKM(RouterContext context) {
this(context, null);
this(context, null, EncType.ECIES_X25519);
}
/**
* ECIES only.
*
* @param dest null for router's SKM only
* @since 0.9.48
*/
public RatchetSKM(RouterContext context, Destination dest) {
this(context, dest, EncType.ECIES_X25519);
}
/**
@ -92,12 +102,15 @@ public class RatchetSKM extends SessionKeyManager implements SessionTagListener
* client manager.
*
* @param dest null for router's SKM only
* @param type the encryption type
* @since 0.9.80
*/
public RatchetSKM(RouterContext context, Destination dest) {
public RatchetSKM(RouterContext context, Destination dest, EncType type) {
super(context);
_log = context.logManager().getLog(RatchetSKM.class);
_context = context;
_destination = dest;
_type = type;
_outboundSessions = new ConcurrentHashMap<PublicKey, OutboundSession>(64);
_pendingOutboundSessions = new HashMap<PublicKey, List<OutboundSession>>(64);
_inboundTagSets = new ConcurrentHashMap<RatchetSessionTag, RatchetTagSet>(128);
@ -147,6 +160,15 @@ public class RatchetSKM extends SessionKeyManager implements SessionTagListener
return _destination;
}
/**
* The EncType this SKM
*
* @since 0.9.80
*/
public EncType getType() {
return _type;
}
/** RatchetTagSet */
private Set<RatchetTagSet> getRatchetTagSets() {
synchronized (_inboundTagSets) {
@ -202,7 +224,7 @@ public class RatchetSKM extends SessionKeyManager implements SessionTagListener
*/
boolean createSession(PublicKey target, Destination d, HandshakeState state, ReplyCallback callback) {
EncType type = target.getType();
if (!LeaseSetKeys.SET_EC_PQ_ALL.contains(type))
if (type != _type)
throw new IllegalArgumentException("Bad public key type " + type);
OutboundSession sess = new OutboundSession(target, d, null, state, callback);
boolean isInbound = state.getRole() == HandshakeState.RESPONDER;
@ -248,7 +270,7 @@ public class RatchetSKM extends SessionKeyManager implements SessionTagListener
boolean updateSession(PublicKey target, HandshakeState oldState, HandshakeState state,
ReplyCallback callback, SplitKeys split) {
EncType type = target.getType();
if (!LeaseSetKeys.SET_EC_PQ_ALL.contains(type))
if (type != _type)
throw new IllegalArgumentException("Bad public key type " + type);
boolean isInbound = state.getRole() == HandshakeState.RESPONDER;
if (isInbound) {