diff --git a/core/java/src/net/i2p/crypto/CryptixAESEngine.java b/core/java/src/net/i2p/crypto/CryptixAESEngine.java index 25f6b1676..18902adea 100644 --- a/core/java/src/net/i2p/crypto/CryptixAESEngine.java +++ b/core/java/src/net/i2p/crypto/CryptixAESEngine.java @@ -127,6 +127,7 @@ public class CryptixAESEngine extends AESEngine { /** * @param iv 16 bytes + * @param length must be a multiple of 16 (will overrun to next mod 16 if not) */ @Override public void decrypt(byte payload[], int payloadIndex, byte out[], int outIndex, SessionKey sessionKey, byte iv[], int length) { @@ -135,6 +136,7 @@ public class CryptixAESEngine extends AESEngine { /** * @param iv 16 bytes starting at ivOffset + * @param length must be a multiple of 16 (will overrun to next mod 16 if not) */ @Override public void decrypt(byte payload[], int payloadIndex, byte out[], int outIndex, SessionKey sessionKey, byte iv[], int ivOffset, int length) { @@ -169,16 +171,25 @@ public class CryptixAESEngine extends AESEngine { ****/ int numblock = length / 16; - if (length % 16 != 0) numblock++; + if (length % 16 != 0) { + // may not work, it will overrun payload length and could AIOOBE + numblock++; + if (_log.shouldLog(Log.WARN)) + _log.warn("not %16 " + length, new Exception()); + } byte prev[] = SimpleByteCache.acquire(16); byte cur[] = SimpleByteCache.acquire(16); System.arraycopy(iv, ivOffset, prev, 0, 16); for (int x = 0; x < numblock; x++) { - System.arraycopy(payload, payloadIndex + (x * 16), cur, 0, 16); - decryptBlock(payload, payloadIndex + (x * 16), sessionKey, out, outIndex + (x * 16)); - DataHelper.xor(out, outIndex + x * 16, prev, 0, out, outIndex + x * 16, 16); + System.arraycopy(payload, payloadIndex, cur, 0, 16); + decryptBlock(payload, payloadIndex, sessionKey, out, outIndex); + payloadIndex += 16; + //DataHelper.xor(out, outIndex + x * 16, prev, 0, out, outIndex + x * 16, 16); + for (int i = 0; i < 16; i++) { + out[outIndex++] ^= prev[i]; + } iv = prev; // just use IV to switch 'em around prev = cur; cur = iv; diff --git a/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java b/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java index 965db51ed..c25f72649 100644 --- a/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java +++ b/router/java/src/net/i2p/router/transport/udp/PacketBuilder.java @@ -121,8 +121,11 @@ class PacketBuilder { static final int TYPE_SREQ = 52; static final int TYPE_CREAT = 53; - /** we only talk to people of the right version */ + /** we only talk to people of the right version + * Commented out to prevent findbugs noop complaint + * If we ever change this, uncomment below and in UDPPacket static final int PROTOCOL_VERSION = 0; + */ /** if no extended options or rekey data, which we don't support = 37 */ public static final int HEADER_SIZE = UDPPacket.MAC_SIZE + UDPPacket.IV_SIZE + 1 + 4; @@ -1317,7 +1320,7 @@ class PacketBuilder { off += encryptSize; System.arraycopy(iv, 0, data, off, UDPPacket.IV_SIZE); off += UDPPacket.IV_SIZE; - DataHelper.toLong(data, off, 2, encryptSize ^ PROTOCOL_VERSION); + DataHelper.toLong(data, off, 2, encryptSize /* ^ PROTOCOL_VERSION */ ); int hmacOff = packet.getPacket().getOffset(); int hmacLen = encryptSize + UDPPacket.IV_SIZE + 2; diff --git a/router/java/src/net/i2p/router/transport/udp/UDPPacket.java b/router/java/src/net/i2p/router/transport/udp/UDPPacket.java index 958a6d4c9..0b888a098 100644 --- a/router/java/src/net/i2p/router/transport/udp/UDPPacket.java +++ b/router/java/src/net/i2p/router/transport/udp/UDPPacket.java @@ -202,7 +202,7 @@ class UDPPacket implements CDQEntry { off += payloadLength; System.arraycopy(_data, _packet.getOffset() + MAC_SIZE, _validateBuf, off, IV_SIZE); off += IV_SIZE; - DataHelper.toLong(_validateBuf, off, 2, payloadLength ^ PacketBuilder.PROTOCOL_VERSION); + DataHelper.toLong(_validateBuf, off, 2, payloadLength /* ^ PacketBuilder.PROTOCOL_VERSION */ ); off += 2; eq = _context.hmac().verify(macKey, _validateBuf, 0, off, _data, _packet.getOffset(), MAC_SIZE); @@ -241,10 +241,18 @@ class UDPPacket implements CDQEntry { */ public void decrypt(SessionKey cipherKey) { verifyNotReleased(); - Arrays.fill(_ivBuf, (byte)0); System.arraycopy(_data, MAC_SIZE, _ivBuf, 0, IV_SIZE); int len = _packet.getLength(); - _context.aes().decrypt(_data, _packet.getOffset() + MAC_SIZE + IV_SIZE, _data, _packet.getOffset() + MAC_SIZE + IV_SIZE, cipherKey, _ivBuf, len - MAC_SIZE - IV_SIZE); + // As of 0.9.7, ignore padding beyond the last mod 16, + // it could otherwise blow up in decryption. + // This allows for better obfuscation. + // Probably works without this since _data is bigger than necessary, but let's not + // bother decrypting and risk overrun. + int rem = len & 0x0f; + if (rem != 0) + len -= rem; + int off = _packet.getOffset() + MAC_SIZE + IV_SIZE; + _context.aes().decrypt(_data, off, _data, off, cipherKey, _ivBuf, len - MAC_SIZE - IV_SIZE); } /**