Cleanup commit to remove @SkipPresubmit and fix nits
This commit removes @SkipPresubmit annotations in IpSecManagerTest to
re-enable Tcp6 tests in presubmit, since b/186608065 is fixed.
This commit also addresses nits in previous code reviews that include:
- Make EspAuthNull, EspAeadCipher, EspCryptCipher, EspCipherNull
final classes
- Rename "INSTANCE" to "sInstance", "IPSEC_MANAGER" to "sIpSecManager"
- Rename protected field "iv" to "mIv"
- Use ArraySet instead of HashSet
- Remove unnecessary "public" and "final" in IpSecAlgorithmImplTest
Bug: 171083832
Test: atest IpSecAlgorithmImplTest
Original-Change: https://android-review.googlesource.com/1729418
Merged-In: Idde90ce5e4fc6f56ad617d5ba9735b1bcc2ce8cd
Change-Id: Idde90ce5e4fc6f56ad617d5ba9735b1bcc2ce8cd
diff --git a/tests/cts/net/src/android/net/cts/IpSecAlgorithmImplTest.java b/tests/cts/net/src/android/net/cts/IpSecAlgorithmImplTest.java
index 55030db..2f29273 100644
--- a/tests/cts/net/src/android/net/cts/IpSecAlgorithmImplTest.java
+++ b/tests/cts/net/src/android/net/cts/IpSecAlgorithmImplTest.java
@@ -86,7 +86,7 @@
InetAddress.parseNumericAddress("2001:db8:1::2");
private static final int REMOTE_PORT = 12345;
- private static final IpSecManager IPSEC_MANAGER =
+ private static final IpSecManager sIpSecManager =
InstrumentationRegistry.getContext().getSystemService(IpSecManager.class);
private static class CheckCryptoImplTest implements TestNetworkRunnable.Test {
@@ -96,7 +96,7 @@
private final EspCipher mEspCipher;
private final EspAuth mEspAuth;
- public CheckCryptoImplTest(
+ CheckCryptoImplTest(
IpSecAlgorithm ipsecEncryptAlgo,
IpSecAlgorithm ipsecAuthAlgo,
IpSecAlgorithm ipsecAeadAlgo,
@@ -149,19 +149,19 @@
}
}
- try (final IpSecManager.SecurityParameterIndex outSpi =
- IPSEC_MANAGER.allocateSecurityParameterIndex(REMOTE_ADDRESS);
- final IpSecManager.SecurityParameterIndex inSpi =
- IPSEC_MANAGER.allocateSecurityParameterIndex(LOCAL_ADDRESS);
- final IpSecTransform outTransform =
+ try (IpSecManager.SecurityParameterIndex outSpi =
+ sIpSecManager.allocateSecurityParameterIndex(REMOTE_ADDRESS);
+ IpSecManager.SecurityParameterIndex inSpi =
+ sIpSecManager.allocateSecurityParameterIndex(LOCAL_ADDRESS);
+ IpSecTransform outTransform =
transformBuilder.buildTransportModeTransform(LOCAL_ADDRESS, outSpi);
- final IpSecTransform inTransform =
+ IpSecTransform inTransform =
transformBuilder.buildTransportModeTransform(REMOTE_ADDRESS, inSpi);
// Bind localSocket to a random available port.
- final DatagramSocket localSocket = new DatagramSocket(0)) {
- IPSEC_MANAGER.applyTransportModeTransform(
+ DatagramSocket localSocket = new DatagramSocket(0)) {
+ sIpSecManager.applyTransportModeTransform(
localSocket, IpSecManager.DIRECTION_IN, inTransform);
- IPSEC_MANAGER.applyTransportModeTransform(
+ sIpSecManager.applyTransportModeTransform(
localSocket, IpSecManager.DIRECTION_OUT, outTransform);
// Send ESP packet
@@ -174,7 +174,7 @@
tunUtils.awaitEspPacket(outSpi.getSpi(), false /* useEncap */);
// Remove transform for good hygiene
- IPSEC_MANAGER.removeTransportModeTransforms(localSocket);
+ sIpSecManager.removeTransportModeTransforms(localSocket);
// Get the kernel-generated ESP payload
final byte[] outEspPayload = new byte[outEspPacket.length - IP6_HDRLEN];
diff --git a/tests/cts/net/src/android/net/cts/IpSecManagerTest.java b/tests/cts/net/src/android/net/cts/IpSecManagerTest.java
index 5f79a3e..5c95aa3 100644
--- a/tests/cts/net/src/android/net/cts/IpSecManagerTest.java
+++ b/tests/cts/net/src/android/net/cts/IpSecManagerTest.java
@@ -74,7 +74,6 @@
import com.android.testutils.DevSdkIgnoreRule;
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
-import com.android.testutils.SkipPresubmit;
import org.junit.Rule;
import org.junit.Test;
@@ -771,7 +770,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesCbcHmacMd5Tcp6() throws Exception {
IpSecAlgorithm crypt = new IpSecAlgorithm(IpSecAlgorithm.CRYPT_AES_CBC, CRYPT_KEY);
IpSecAlgorithm auth = new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_MD5, getKey(128), 96);
@@ -804,7 +802,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesCbcHmacSha1Tcp6() throws Exception {
IpSecAlgorithm crypt = new IpSecAlgorithm(IpSecAlgorithm.CRYPT_AES_CBC, CRYPT_KEY);
IpSecAlgorithm auth = new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_SHA1, getKey(160), 96);
@@ -837,7 +834,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesCbcHmacSha256Tcp6() throws Exception {
IpSecAlgorithm crypt = new IpSecAlgorithm(IpSecAlgorithm.CRYPT_AES_CBC, CRYPT_KEY);
IpSecAlgorithm auth = new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_SHA256, getKey(256), 128);
@@ -870,7 +866,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesCbcHmacSha384Tcp6() throws Exception {
IpSecAlgorithm crypt = new IpSecAlgorithm(IpSecAlgorithm.CRYPT_AES_CBC, CRYPT_KEY);
IpSecAlgorithm auth = new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_SHA384, getKey(384), 192);
@@ -903,7 +898,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesCbcHmacSha512Tcp6() throws Exception {
IpSecAlgorithm crypt = new IpSecAlgorithm(IpSecAlgorithm.CRYPT_AES_CBC, CRYPT_KEY);
IpSecAlgorithm auth = new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_SHA512, getKey(512), 256);
@@ -947,7 +941,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesCtrHmacSha512Tcp6() throws Exception {
assumeTrue(hasIpSecAlgorithm(CRYPT_AES_CTR));
@@ -1002,7 +995,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesCbcAesXCbcTcp6() throws Exception {
assumeTrue(hasIpSecAlgorithm(AUTH_AES_XCBC));
@@ -1043,7 +1035,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesCbcAesCmacTcp6() throws Exception {
assumeTrue(hasIpSecAlgorithm(AUTH_AES_CMAC));
@@ -1082,7 +1073,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesGcm64Tcp6() throws Exception {
IpSecAlgorithm authCrypt =
new IpSecAlgorithm(IpSecAlgorithm.AUTH_CRYPT_AES_GCM, AEAD_KEY, 64);
@@ -1115,7 +1105,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesGcm96Tcp6() throws Exception {
IpSecAlgorithm authCrypt =
new IpSecAlgorithm(IpSecAlgorithm.AUTH_CRYPT_AES_GCM, AEAD_KEY, 96);
@@ -1148,7 +1137,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAesGcm128Tcp6() throws Exception {
IpSecAlgorithm authCrypt =
new IpSecAlgorithm(IpSecAlgorithm.AUTH_CRYPT_AES_GCM, AEAD_KEY, 128);
@@ -1187,7 +1175,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testChaCha20Poly1305Tcp6() throws Exception {
assumeTrue(hasIpSecAlgorithm(AUTH_CRYPT_CHACHA20_POLY1305));
@@ -1463,7 +1450,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testCryptTcp6() throws Exception {
IpSecAlgorithm crypt = new IpSecAlgorithm(IpSecAlgorithm.CRYPT_AES_CBC, CRYPT_KEY);
checkTransform(IPPROTO_TCP, IPV6_LOOPBACK, crypt, null, null, false, 1, false);
@@ -1471,7 +1457,6 @@
}
@Test
- @SkipPresubmit(reason = "b/186608065 - kernel 5.10 regression in TrafficStats with ipsec")
public void testAuthTcp6() throws Exception {
IpSecAlgorithm auth = new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_SHA256, getKey(256), 128);
checkTransform(IPPROTO_TCP, IPV6_LOOPBACK, null, auth, null, false, 1, false);
diff --git a/tests/cts/net/src/android/net/cts/PacketUtils.java b/tests/cts/net/src/android/net/cts/PacketUtils.java
index 9170cff..4d924d1 100644
--- a/tests/cts/net/src/android/net/cts/PacketUtils.java
+++ b/tests/cts/net/src/android/net/cts/PacketUtils.java
@@ -19,6 +19,8 @@
import static android.system.OsConstants.IPPROTO_IPV6;
import static android.system.OsConstants.IPPROTO_UDP;
+import android.util.ArraySet;
+
import com.android.internal.net.ipsec.ike.crypto.AesXCbcImpl;
import java.net.Inet4Address;
@@ -29,7 +31,6 @@
import java.security.GeneralSecurityException;
import java.security.SecureRandom;
import java.util.Arrays;
-import java.util.HashSet;
import java.util.Set;
import javax.crypto.Cipher;
@@ -502,7 +503,7 @@
public final byte[] key;
public final int ivLen;
public final int saltLen;
- protected byte[] iv;
+ protected byte[] mIv;
public EspCipher(String algoName, int blockSize, byte[] key, int ivLen, int saltLen) {
this.algoName = algoName;
@@ -510,11 +511,11 @@
this.key = key;
this.ivLen = ivLen;
this.saltLen = saltLen;
- this.iv = getIv(ivLen);
+ this.mIv = getIv(ivLen);
}
public void updateIv(byte[] iv) {
- this.iv = iv;
+ this.mIv = iv;
}
public static byte[] getPaddedPayload(int nextHeader, byte[] payload, int blockSize) {
@@ -545,19 +546,19 @@
throws GeneralSecurityException;
}
- public static class EspCipherNull extends EspCipher {
+ public static final class EspCipherNull extends EspCipher {
private static final String CRYPT_NULL = "CRYPT_NULL";
private static final int IV_LEN_UNUSED = 0;
private static final byte[] KEY_UNUSED = new byte[0];
- private static final EspCipherNull INSTANCE = new EspCipherNull();
+ private static final EspCipherNull sInstance = new EspCipherNull();
private EspCipherNull() {
super(CRYPT_NULL, ESP_BLK_SIZE, KEY_UNUSED, IV_LEN_UNUSED, SALT_LEN_UNUSED);
}
public static EspCipherNull getInstance() {
- return INSTANCE;
+ return sInstance;
}
@Override
@@ -567,7 +568,7 @@
}
}
- public static class EspCryptCipher extends EspCipher {
+ public static final class EspCryptCipher extends EspCipher {
public EspCryptCipher(String algoName, int blockSize, byte[] key, int ivLen) {
this(algoName, blockSize, key, ivLen, SALT_LEN_UNUSED);
}
@@ -583,7 +584,7 @@
final SecretKeySpec secretKeySpec;
if (AES_CBC.equals(algoName)) {
- ivParameterSpec = new IvParameterSpec(iv);
+ ivParameterSpec = new IvParameterSpec(mIv);
secretKeySpec = new SecretKeySpec(key, algoName);
} else if (AES_CTR.equals(algoName)) {
// Provided key consists of encryption/decryption key plus 4-byte salt. Salt is used
@@ -593,9 +594,9 @@
secretKeySpec = new SecretKeySpec(secretKey, algoName);
final ByteBuffer ivParameterBuffer =
- ByteBuffer.allocate(iv.length + saltLen + AES_CTR_INITIAL_COUNTER.length);
+ ByteBuffer.allocate(mIv.length + saltLen + AES_CTR_INITIAL_COUNTER.length);
ivParameterBuffer.put(salt);
- ivParameterBuffer.put(iv);
+ ivParameterBuffer.put(mIv);
ivParameterBuffer.put(AES_CTR_INITIAL_COUNTER);
ivParameterSpec = new IvParameterSpec(ivParameterBuffer.array());
} else {
@@ -609,15 +610,15 @@
cipher.doFinal(getPaddedPayload(nextHeader, payload, blockSize));
// Build ciphertext
- final ByteBuffer cipherText = ByteBuffer.allocate(iv.length + encrypted.length);
- cipherText.put(iv);
+ final ByteBuffer cipherText = ByteBuffer.allocate(mIv.length + encrypted.length);
+ cipherText.put(mIv);
cipherText.put(encrypted);
return getByteArrayFromBuffer(cipherText);
}
}
- public static class EspAeadCipher extends EspCipher {
+ public static final class EspAeadCipher extends EspCipher {
public final int icvLen;
public EspAeadCipher(
@@ -636,9 +637,9 @@
final SecretKeySpec secretKeySpec = new SecretKeySpec(secretKey, algoName);
- final ByteBuffer ivParameterBuffer = ByteBuffer.allocate(saltLen + iv.length);
+ final ByteBuffer ivParameterBuffer = ByteBuffer.allocate(saltLen + mIv.length);
ivParameterBuffer.put(salt);
- ivParameterBuffer.put(iv);
+ ivParameterBuffer.put(mIv);
final IvParameterSpec ivParameterSpec = new IvParameterSpec(ivParameterBuffer.array());
final ByteBuffer aadBuffer = ByteBuffer.allocate(ESP_HDRLEN);
@@ -654,8 +655,8 @@
// Build ciphertext
final ByteBuffer cipherText =
- ByteBuffer.allocate(iv.length + encryptedTextAndIcv.length);
- cipherText.put(iv);
+ ByteBuffer.allocate(mIv.length + encryptedTextAndIcv.length);
+ cipherText.put(mIv);
cipherText.put(encryptedTextAndIcv);
return getByteArrayFromBuffer(cipherText);
@@ -667,7 +668,7 @@
public final byte[] key;
public final int icvLen;
- private static final Set<String> JCE_SUPPORTED_MACS = new HashSet<>();
+ private static final Set<String> JCE_SUPPORTED_MACS = new ArraySet<>();
static {
JCE_SUPPORTED_MACS.add(HMAC_MD5);
@@ -703,20 +704,20 @@
}
}
- public static class EspAuthNull extends EspAuth {
+ public static final class EspAuthNull extends EspAuth {
private static final String AUTH_NULL = "AUTH_NULL";
private static final int ICV_LEN_UNUSED = 0;
private static final byte[] KEY_UNUSED = new byte[0];
private static final byte[] ICV_EMPTY = new byte[0];
- private static final EspAuthNull INSTANCE = new EspAuthNull();
+ private static final EspAuthNull sInstance = new EspAuthNull();
private EspAuthNull() {
super(AUTH_NULL, KEY_UNUSED, ICV_LEN_UNUSED);
}
public static EspAuthNull getInstance() {
- return INSTANCE;
+ return sInstance;
}
@Override