From 20b71e6f07370923027ac3741d5d0466d177bfdb Mon Sep 17 00:00:00 2001 From: evitayan Date: Tue, 7 Apr 2020 21:48:32 -0700 Subject: Extract IKE SPI in separate file before creating SPI generator This commit moves IkeSecurityParameterIndex out of IkeSessionStateMachine as a preparation of building IKE SPI generator Bug: 148689509 Test: atest FrameworksIkeTests Change-Id: Ica3ba9c8812b103b927bc59961f384bea2bf0973 --- .../net/ipsec/ike/IkeSessionStateMachine.java | 97 +--------------- .../android/internal/net/ipsec/ike/SaRecord.java | 2 +- .../net/ipsec/ike/message/IkeSaPayload.java | 2 +- .../ipsec/ike/utils/IkeSecurityParameterIndex.java | 122 +++++++++++++++++++++ .../net/ipsec/ike/IkeSessionStateMachineTest.java | 2 +- .../internal/net/ipsec/ike/SaRecordTest.java | 2 +- 6 files changed, 127 insertions(+), 100 deletions(-) create mode 100644 src/java/com/android/internal/net/ipsec/ike/utils/IkeSecurityParameterIndex.java diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java index f18c71f9..32915d2b 100644 --- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java +++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java @@ -77,7 +77,6 @@ import android.os.SystemClock; import android.system.ErrnoException; import android.system.Os; import android.system.OsConstants; -import android.util.CloseGuard; import android.util.LongSparseArray; import android.util.Pair; import android.util.SparseArray; @@ -128,6 +127,7 @@ import com.android.internal.net.ipsec.ike.message.IkeSaPayload.IkeProposal; import com.android.internal.net.ipsec.ike.message.IkeTsPayload; import com.android.internal.net.ipsec.ike.message.IkeVendorPayload; import com.android.internal.net.ipsec.ike.utils.IkeAlarmReceiver; +import com.android.internal.net.ipsec.ike.utils.IkeSecurityParameterIndex; import com.android.internal.net.ipsec.ike.utils.Retransmitter; import com.android.internal.util.State; @@ -140,7 +140,6 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.security.GeneralSecurityException; -import java.security.SecureRandom; import java.security.cert.TrustAnchor; import java.security.cert.X509Certificate; import java.util.ArrayList; @@ -731,100 +730,6 @@ public class IkeSessionStateMachine extends AbstractSessionStateMachine { mTempFailureReceived = false; } } - /** - * This class represents a reserved IKE SPI. - * - *

This class is created to avoid assigning same SPI to the same address. - * - *

Objects of this type are used to track reserved IKE SPI to avoid SPI collision. They can - * be obtained by calling {@link #allocateSecurityParameterIndex()} and must be released by - * calling {@link #close()} when they are no longer needed. - * - *

This class follows the pattern of {@link IpSecManager.SecurityParameterIndex}. - * - *

TODO: Move this class to a central place, like IkeManager. - */ - public static final class IkeSecurityParameterIndex implements AutoCloseable { - // Remember assigned IKE SPIs to avoid SPI collision. - private static final Set> sAssignedIkeSpis = new HashSet<>(); - private static final int MAX_ASSIGN_IKE_SPI_ATTEMPTS = 100; - private static final SecureRandom IKE_SPI_RANDOM = new SecureRandom(); - - private final InetAddress mSourceAddress; - private final long mSpi; - private final CloseGuard mCloseGuard = new CloseGuard(); - - private IkeSecurityParameterIndex(InetAddress sourceAddress, long spi) { - mSourceAddress = sourceAddress; - mSpi = spi; - mCloseGuard.open("close"); - } - - /** - * Get a new IKE SPI and maintain the reservation. - * - * @return an instance of IkeSecurityParameterIndex. - */ - public static IkeSecurityParameterIndex allocateSecurityParameterIndex( - InetAddress sourceAddress) throws IOException { - // TODO: Create specific Exception for SPI assigning error. - - for (int i = 0; i < MAX_ASSIGN_IKE_SPI_ATTEMPTS; i++) { - long spi = IKE_SPI_RANDOM.nextLong(); - // Zero value can only be used in the IKE responder SPI field of an IKE INIT - // request. - if (spi != 0L - && sAssignedIkeSpis.add(new Pair(sourceAddress, spi))) { - return new IkeSecurityParameterIndex(sourceAddress, spi); - } - } - - throw new IOException("Failed to generate IKE SPI."); - } - - /** - * Get a new IKE SPI and maintain the reservation. - * - * @return an instance of IkeSecurityParameterIndex. - */ - public static IkeSecurityParameterIndex allocateSecurityParameterIndex( - InetAddress sourceAddress, long requestedSpi) throws IOException { - if (sAssignedIkeSpis.add(new Pair(sourceAddress, requestedSpi))) { - return new IkeSecurityParameterIndex(sourceAddress, requestedSpi); - } - - throw new IOException( - "Failed to generate IKE SPI for " - + requestedSpi - + " with source address " - + sourceAddress.getHostAddress()); - } - - /** - * Get the underlying SPI held by this object. - * - * @return the underlying IKE SPI. - */ - public long getSpi() { - return mSpi; - } - - /** Release an SPI that was previously reserved. */ - @Override - public void close() { - sAssignedIkeSpis.remove(new Pair(mSourceAddress, mSpi)); - mCloseGuard.close(); - } - - /** Check that the IkeSecurityParameterIndex was closed properly. */ - @Override - protected void finalize() throws Throwable { - if (mCloseGuard != null) { - mCloseGuard.warnIfOpen(); - } - close(); - } - } // TODO: Add methods for building and validating general Informational packet. diff --git a/src/java/com/android/internal/net/ipsec/ike/SaRecord.java b/src/java/com/android/internal/net/ipsec/ike/SaRecord.java index f24813c4..a0b585fc 100644 --- a/src/java/com/android/internal/net/ipsec/ike/SaRecord.java +++ b/src/java/com/android/internal/net/ipsec/ike/SaRecord.java @@ -33,7 +33,6 @@ import android.util.CloseGuard; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.ChildLocalRequest; import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.LocalRequest; -import com.android.internal.net.ipsec.ike.IkeSessionStateMachine.IkeSecurityParameterIndex; import com.android.internal.net.ipsec.ike.crypto.IkeCipher; import com.android.internal.net.ipsec.ike.crypto.IkeMacIntegrity; import com.android.internal.net.ipsec.ike.crypto.IkeMacPrf; @@ -42,6 +41,7 @@ import com.android.internal.net.ipsec.ike.message.IkeMessage; import com.android.internal.net.ipsec.ike.message.IkeMessage.DecodeResultPartial; import com.android.internal.net.ipsec.ike.message.IkeNoncePayload; import com.android.internal.net.ipsec.ike.message.IkePayload; +import com.android.internal.net.ipsec.ike.utils.IkeSecurityParameterIndex; import java.io.IOException; import java.net.Inet4Address; diff --git a/src/java/com/android/internal/net/ipsec/ike/message/IkeSaPayload.java b/src/java/com/android/internal/net/ipsec/ike/message/IkeSaPayload.java index a4a60e6a..532ee7ff 100644 --- a/src/java/com/android/internal/net/ipsec/ike/message/IkeSaPayload.java +++ b/src/java/com/android/internal/net/ipsec/ike/message/IkeSaPayload.java @@ -36,9 +36,9 @@ import android.util.ArraySet; import android.util.Pair; import com.android.internal.annotations.VisibleForTesting; -import com.android.internal.net.ipsec.ike.IkeSessionStateMachine.IkeSecurityParameterIndex; import com.android.internal.net.ipsec.ike.exceptions.InvalidSyntaxException; import com.android.internal.net.ipsec.ike.exceptions.NoValidProposalChosenException; +import com.android.internal.net.ipsec.ike.utils.IkeSecurityParameterIndex; import java.io.IOException; import java.lang.annotation.Retention; diff --git a/src/java/com/android/internal/net/ipsec/ike/utils/IkeSecurityParameterIndex.java b/src/java/com/android/internal/net/ipsec/ike/utils/IkeSecurityParameterIndex.java new file mode 100644 index 00000000..7f72e117 --- /dev/null +++ b/src/java/com/android/internal/net/ipsec/ike/utils/IkeSecurityParameterIndex.java @@ -0,0 +1,122 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.internal.net.ipsec.ike.utils; + +import android.util.CloseGuard; +import android.util.Pair; + +import java.io.IOException; +import java.net.InetAddress; +import java.security.SecureRandom; +import java.util.HashSet; +import java.util.Set; + +/** + * This class represents a reserved IKE SPI. + * + *

This class is created to avoid assigning same SPI to the same address. + * + *

Objects of this type are used to track reserved IKE SPI to avoid SPI collision. They can be + * obtained by calling {@link #allocateSecurityParameterIndex()} and must be released by calling + * {@link #close()} when they are no longer needed. + * + *

This class MUST only be called from IKE worker thread. Methods that allocate and close IKE + * SPI resource are not thread safe. + * + *

This class follows the pattern of {@link IpSecManager.SecurityParameterIndex}. + */ +public final class IkeSecurityParameterIndex implements AutoCloseable { + // Remember assigned IKE SPIs to avoid SPI collision. + private static final Set> sAssignedIkeSpis = new HashSet<>(); + private static final int MAX_ASSIGN_IKE_SPI_ATTEMPTS = 100; + private static final SecureRandom IKE_SPI_RANDOM = new SecureRandom(); + + private final InetAddress mSourceAddress; + private final long mSpi; + private final CloseGuard mCloseGuard = new CloseGuard(); + + private IkeSecurityParameterIndex(InetAddress sourceAddress, long spi) { + mSourceAddress = sourceAddress; + mSpi = spi; + mCloseGuard.open("close"); + } + + /** + * Get a new IKE SPI and maintain the reservation. + * + * @return an instance of IkeSecurityParameterIndex. + */ + public static IkeSecurityParameterIndex allocateSecurityParameterIndex( + InetAddress sourceAddress) throws IOException { + // TODO: Create specific Exception for SPI assigning error. + + for (int i = 0; i < MAX_ASSIGN_IKE_SPI_ATTEMPTS; i++) { + long spi = IKE_SPI_RANDOM.nextLong(); + // Zero value can only be used in the IKE responder SPI field of an IKE INIT + // request. + if (spi != 0L + && sAssignedIkeSpis.add(new Pair(sourceAddress, spi))) { + return new IkeSecurityParameterIndex(sourceAddress, spi); + } + } + + throw new IOException("Failed to generate IKE SPI."); + } + + /** + * Get a new IKE SPI and maintain the reservation. + * + * @return an instance of IkeSecurityParameterIndex. + */ + public static IkeSecurityParameterIndex allocateSecurityParameterIndex( + InetAddress sourceAddress, long requestedSpi) throws IOException { + if (sAssignedIkeSpis.add(new Pair(sourceAddress, requestedSpi))) { + return new IkeSecurityParameterIndex(sourceAddress, requestedSpi); + } + + throw new IOException( + "Failed to generate IKE SPI for " + + requestedSpi + + " with source address " + + sourceAddress.getHostAddress()); + } + + /** + * Get the underlying SPI held by this object. + * + * @return the underlying IKE SPI. + */ + public long getSpi() { + return mSpi; + } + + /** Release an SPI that was previously reserved. */ + @Override + public void close() { + sAssignedIkeSpis.remove(new Pair(mSourceAddress, mSpi)); + mCloseGuard.close(); + } + + /** Check that the IkeSecurityParameterIndex was closed properly. */ + @Override + protected void finalize() throws Throwable { + if (mCloseGuard != null) { + mCloseGuard.warnIfOpen(); + } + close(); + } +} diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java index c7fb0ba2..34d4b2fc 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachineTest.java @@ -108,7 +108,6 @@ import com.android.internal.net.ipsec.ike.ChildSessionStateMachineFactory.ChildS import com.android.internal.net.ipsec.ike.ChildSessionStateMachineFactory.IChildSessionFactoryHelper; import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.ChildLocalRequest; import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.IkeLocalRequest; -import com.android.internal.net.ipsec.ike.IkeSessionStateMachine.IkeSecurityParameterIndex; import com.android.internal.net.ipsec.ike.IkeSessionStateMachine.ReceivedIkePacket; import com.android.internal.net.ipsec.ike.SaRecord.ISaRecordHelper; import com.android.internal.net.ipsec.ike.SaRecord.IkeSaRecord; @@ -155,6 +154,7 @@ import com.android.internal.net.ipsec.ike.message.IkeTsPayload; import com.android.internal.net.ipsec.ike.testutils.CertUtils; import com.android.internal.net.ipsec.ike.testutils.MockIpSecTestUtils; import com.android.internal.net.ipsec.ike.utils.IkeAlarmReceiver; +import com.android.internal.net.ipsec.ike.utils.IkeSecurityParameterIndex; import com.android.internal.net.ipsec.ike.utils.Retransmitter; import com.android.internal.net.ipsec.ike.utils.Retransmitter.IBackoffTimeoutCalculator; import com.android.internal.net.ipsec.ike.utils.State; diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/SaRecordTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/SaRecordTest.java index 501437b7..0f6bc421 100644 --- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/SaRecordTest.java +++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/SaRecordTest.java @@ -38,7 +38,6 @@ import android.net.ipsec.ike.SaProposal; import com.android.internal.net.TestUtils; import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.ChildLocalRequest; import com.android.internal.net.ipsec.ike.IkeLocalRequestScheduler.LocalRequest; -import com.android.internal.net.ipsec.ike.IkeSessionStateMachine.IkeSecurityParameterIndex; import com.android.internal.net.ipsec.ike.SaRecord.ChildSaRecord; import com.android.internal.net.ipsec.ike.SaRecord.ChildSaRecordConfig; import com.android.internal.net.ipsec.ike.SaRecord.IIpSecTransformHelper; @@ -54,6 +53,7 @@ import com.android.internal.net.ipsec.ike.message.IkeSaPayload.EncryptionTransfo import com.android.internal.net.ipsec.ike.message.IkeSaPayload.IntegrityTransform; import com.android.internal.net.ipsec.ike.message.IkeSaPayload.PrfTransform; import com.android.internal.net.ipsec.ike.testutils.MockIpSecTestUtils; +import com.android.internal.net.ipsec.ike.utils.IkeSecurityParameterIndex; import com.android.server.IpSecService; import org.junit.Before; -- cgit v1.2.3 From 1152eaf9c0ba8e0d63a8b233aa86621509e1e83d Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Mon, 13 Apr 2020 12:39:24 -0700 Subject: Make EapAkaPrimeConfigs have the correct methodType. This change adds a private constructor to EapAkaConfig so that EapAkaPrimeConfig instances will be created with the correct methodType value. EapAkaPrimeConfig currently calls a super-constructor to EapAkaConfig, which passes a methodType for EAP-AKA to EapUiccConfig. This causes EapAkaPrimeConfig to have a method type for EAP-AKA, which is incorrect. Bug: 153684470 Test: atest FrameworksIkeTests CtsIkeTestCases Change-Id: Ie27ee45163567651ac5680e637cc1a267c5ef7f3 --- src/java/android/net/eap/EapSessionConfig.java | 9 +++++++-- .../iketests/src/java/android/net/eap/EapSessionConfigTest.java | 3 +++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/java/android/net/eap/EapSessionConfig.java b/src/java/android/net/eap/EapSessionConfig.java index 7a7ee8f6..0deb1576 100644 --- a/src/java/android/net/eap/EapSessionConfig.java +++ b/src/java/android/net/eap/EapSessionConfig.java @@ -303,7 +303,12 @@ public final class EapSessionConfig { /** @hide */ @VisibleForTesting public EapAkaConfig(int subId, @UiccAppType int apptype) { - super(EAP_TYPE_AKA, subId, apptype); + this(EAP_TYPE_AKA, subId, apptype); + } + + /** @hide */ + EapAkaConfig(int methodType, int subId, @UiccAppType int apptype) { + super(methodType, subId, apptype); } } @@ -323,7 +328,7 @@ public final class EapSessionConfig { @UiccAppType int apptype, @NonNull String networkName, boolean allowMismatchedNetworkNames) { - super(subId, apptype); + super(EAP_TYPE_AKA_PRIME, subId, apptype); if (networkName == null) { throw new IllegalArgumentException("NetworkName was null"); diff --git a/tests/iketests/src/java/android/net/eap/EapSessionConfigTest.java b/tests/iketests/src/java/android/net/eap/EapSessionConfigTest.java index eed32e3e..cfa6cbfc 100644 --- a/tests/iketests/src/java/android/net/eap/EapSessionConfigTest.java +++ b/tests/iketests/src/java/android/net/eap/EapSessionConfigTest.java @@ -72,6 +72,7 @@ public class EapSessionConfigTest { assertArrayEquals(DEFAULT_IDENTITY, result.eapIdentity); EapMethodConfig eapMethodConfig = result.eapConfigs.get(EAP_TYPE_AKA); + assertEquals(EAP_TYPE_AKA, eapMethodConfig.methodType); EapAkaConfig eapAkaConfig = (EapAkaConfig) eapMethodConfig; assertEquals(SUB_ID, eapAkaConfig.subId); assertEquals(APPTYPE_USIM, eapAkaConfig.apptype); @@ -87,6 +88,7 @@ public class EapSessionConfigTest { assertEquals(DEFAULT_IDENTITY, result.eapIdentity); EapMethodConfig eapMethodConfig = result.eapConfigs.get(EAP_TYPE_AKA_PRIME); + assertEquals(EAP_TYPE_AKA_PRIME, eapMethodConfig.methodType); EapAkaPrimeConfig eapAkaPrimeConfig = (EapAkaPrimeConfig) eapMethodConfig; assertEquals(SUB_ID, eapAkaPrimeConfig.subId); assertEquals(APPTYPE_USIM, eapAkaPrimeConfig.apptype); @@ -100,6 +102,7 @@ public class EapSessionConfigTest { new EapSessionConfig.Builder().setEapMsChapV2Config(USERNAME, PASSWORD).build(); EapMsChapV2Config config = (EapMsChapV2Config) result.eapConfigs.get(EAP_TYPE_MSCHAP_V2); + assertEquals(EAP_TYPE_MSCHAP_V2, config.methodType); assertEquals(USERNAME, config.username); assertEquals(PASSWORD, config.password); } -- cgit v1.2.3