Merge "Delete unused flag" into main
diff --git a/bpf/headers/include/bpf/BpfRingbuf.h b/bpf/headers/include/bpf/BpfRingbuf.h
index 4bcd259..5fe4ef7 100644
--- a/bpf/headers/include/bpf/BpfRingbuf.h
+++ b/bpf/headers/include/bpf/BpfRingbuf.h
@@ -99,6 +99,7 @@
// 32-bit kernel will just ignore the high-order bits.
std::atomic_uint64_t* mConsumerPos = nullptr;
std::atomic_uint32_t* mProducerPos = nullptr;
+ std::atomic_uint32_t* mLength = nullptr;
// In order to guarantee atomic access in a 32 bit userspace environment, atomic_uint64_t is used
// in addition to std::atomic<T>::is_always_lock_free that guarantees that read / write operations
@@ -247,7 +248,8 @@
// u32 len;
// u32 pg_off;
// };
- uint32_t length = *reinterpret_cast<volatile uint32_t*>(start_ptr);
+ mLength = reinterpret_cast<decltype(mLength)>(start_ptr);
+ uint32_t length = mLength->load(std::memory_order_acquire);
// If the sample isn't committed, we're caught up with the producer.
if (length & BPF_RINGBUF_BUSY_BIT) return count;
diff --git a/networksecurity/service/Android.bp b/networksecurity/service/Android.bp
index 52667ae..a41e6a0 100644
--- a/networksecurity/service/Android.bp
+++ b/networksecurity/service/Android.bp
@@ -32,6 +32,15 @@
"service-connectivity-pre-jarjar",
],
+ static_libs: [
+ "auto_value_annotations",
+ ],
+
+ plugins: [
+ "auto_value_plugin",
+ "auto_annotation_plugin",
+ ],
+
// This is included in service-connectivity which is 30+
// TODO (b/293613362): allow APEXes to have service jars with higher min_sdk than the APEX
// (service-connectivity is only used on 31+) and use 31 here
diff --git a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java
index 16f32c4..f86d127 100644
--- a/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java
+++ b/networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java
@@ -29,6 +29,8 @@
import androidx.annotation.VisibleForTesting;
+import com.android.server.net.ct.DownloadHelper.DownloadStatus;
+
import java.io.IOException;
import java.io.InputStream;
import java.security.GeneralSecurityException;
@@ -147,19 +149,18 @@
}
private void handleMetadataDownloadCompleted(long downloadId) {
- if (!mDownloadHelper.isSuccessful(downloadId)) {
- Log.w(TAG, "Metadata download failed.");
- // TODO: re-attempt download
+ DownloadStatus status = mDownloadHelper.getDownloadStatus(downloadId);
+ if (!status.isSuccessful()) {
+ handleDownloadFailed(status);
return;
}
-
startContentDownload(mDataStore.getProperty(Config.CONTENT_URL_PENDING));
}
private void handleContentDownloadCompleted(long downloadId) {
- if (!mDownloadHelper.isSuccessful(downloadId)) {
- Log.w(TAG, "Content download failed.");
- // TODO: re-attempt download
+ DownloadStatus status = mDownloadHelper.getDownloadStatus(downloadId);
+ if (!status.isSuccessful()) {
+ handleDownloadFailed(status);
return;
}
@@ -202,6 +203,11 @@
}
}
+ private void handleDownloadFailed(DownloadStatus status) {
+ Log.e(TAG, "Content download failed with " + status);
+ // TODO(378626065): Report failure via statsd.
+ }
+
private boolean verify(Uri file, Uri signature) throws IOException, GeneralSecurityException {
if (!mPublicKey.isPresent()) {
throw new InvalidKeyException("Missing public key for signature verification");
diff --git a/networksecurity/service/src/com/android/server/net/ct/DirectoryUtils.java b/networksecurity/service/src/com/android/server/net/ct/DirectoryUtils.java
index e3b4124..ba42a82 100644
--- a/networksecurity/service/src/com/android/server/net/ct/DirectoryUtils.java
+++ b/networksecurity/service/src/com/android/server/net/ct/DirectoryUtils.java
@@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
package com.android.server.net.ct;
import android.annotation.SuppressLint;
@@ -29,16 +30,33 @@
throw new IOException("Unable to make directory " + dir.getCanonicalPath());
}
setWorldReadable(dir);
+ // Needed for the log list file to be accessible.
+ setWorldExecutable(dir);
}
// CT files and directories are readable by all apps.
@SuppressLint("SetWorldReadable")
static void setWorldReadable(File file) throws IOException {
- if (!file.setReadable(true, false)) {
+ if (!file.setReadable(/* readable= */ true, /* ownerOnly= */ false)) {
throw new IOException("Failed to set " + file.getCanonicalPath() + " readable");
}
}
+ // CT directories are executable by all apps, to allow access to the log list by anything on the
+ // device.
+ static void setWorldExecutable(File file) throws IOException {
+ if (!file.isDirectory()) {
+ // Only directories need to be marked as executable to allow for access
+ // to the files inside.
+ // See https://www.redhat.com/en/blog/linux-file-permissions-explained for more details.
+ return;
+ }
+
+ if (!file.setExecutable(/* executable= */ true, /* ownerOnly= */ false)) {
+ throw new IOException("Failed to set " + file.getCanonicalPath() + " executable");
+ }
+ }
+
static boolean removeDir(File dir) {
return deleteContentsAndDir(dir);
}
diff --git a/networksecurity/service/src/com/android/server/net/ct/DownloadHelper.java b/networksecurity/service/src/com/android/server/net/ct/DownloadHelper.java
index cc8c4c0..5748416 100644
--- a/networksecurity/service/src/com/android/server/net/ct/DownloadHelper.java
+++ b/networksecurity/service/src/com/android/server/net/ct/DownloadHelper.java
@@ -24,6 +24,8 @@
import androidx.annotation.VisibleForTesting;
+import com.google.auto.value.AutoValue;
+
/** Class to handle downloads for Certificate Transparency. */
public class DownloadHelper {
@@ -53,25 +55,22 @@
}
/**
- * Returns true if the specified download completed successfully.
+ * Returns the status of the provided download id.
*
* @param downloadId the download.
- * @return true if the download completed successfully.
+ * @return {@link DownloadStatus} of the download.
*/
- public boolean isSuccessful(long downloadId) {
+ public DownloadStatus getDownloadStatus(long downloadId) {
+ DownloadStatus.Builder builder = DownloadStatus.builder().setDownloadId(downloadId);
try (Cursor cursor = mDownloadManager.query(new Query().setFilterById(downloadId))) {
- if (cursor == null) {
- return false;
- }
- if (cursor.moveToFirst()) {
- int status =
- cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS));
- if (DownloadManager.STATUS_SUCCESSFUL == status) {
- return true;
- }
+ if (cursor != null && cursor.moveToFirst()) {
+ builder.setStatus(
+ cursor.getInt(cursor.getColumnIndexOrThrow(DownloadManager.COLUMN_STATUS)));
+ builder.setReason(
+ cursor.getInt(cursor.getColumnIndex(DownloadManager.COLUMN_REASON)));
}
}
- return false;
+ return builder.build();
}
/**
@@ -87,4 +86,58 @@
}
return mDownloadManager.getUriForDownloadedFile(downloadId);
}
+
+ /** A wrapper around the status and reason Ids returned by the {@link DownloadManager}. */
+ @AutoValue
+ public abstract static class DownloadStatus {
+
+ abstract long downloadId();
+
+ abstract int status();
+
+ abstract int reason();
+
+ boolean isSuccessful() {
+ return status() == DownloadManager.STATUS_SUCCESSFUL;
+ }
+
+ boolean isStorageError() {
+ int status = status();
+ int reason = reason();
+ return status == DownloadManager.STATUS_FAILED
+ && (reason == DownloadManager.ERROR_DEVICE_NOT_FOUND
+ || reason == DownloadManager.ERROR_FILE_ERROR
+ || reason == DownloadManager.ERROR_FILE_ALREADY_EXISTS
+ || reason == DownloadManager.ERROR_INSUFFICIENT_SPACE);
+ }
+
+ boolean isHttpError() {
+ int status = status();
+ int reason = reason();
+ return status == DownloadManager.STATUS_FAILED
+ && (reason == DownloadManager.ERROR_HTTP_DATA_ERROR
+ || reason == DownloadManager.ERROR_TOO_MANY_REDIRECTS
+ || reason == DownloadManager.ERROR_UNHANDLED_HTTP_CODE
+ // If an HTTP error occurred, reason will hold the HTTP status code.
+ || (400 <= reason && reason < 600));
+ }
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract Builder setDownloadId(long downloadId);
+
+ abstract Builder setStatus(int status);
+
+ abstract Builder setReason(int reason);
+
+ abstract DownloadStatus build();
+ }
+
+ static Builder builder() {
+ return new AutoValue_DownloadHelper_DownloadStatus.Builder()
+ .setDownloadId(-1)
+ .setStatus(-1)
+ .setReason(-1);
+ }
+ }
}
diff --git a/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java b/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java
index df02446..fb55295 100644
--- a/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java
+++ b/networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java
@@ -31,6 +31,8 @@
import androidx.test.platform.app.InstrumentationRegistry;
+import com.android.server.net.ct.DownloadHelper.DownloadStatus;
+
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -115,11 +117,11 @@
}
@Test
- public void testDownloader_handleMetadataCompleteSuccessful() {
+ public void testDownloader_metadataDownloadSuccess_startContentDownload() {
long metadataId = 123;
mDataStore.setPropertyLong(Config.METADATA_URL_KEY, metadataId);
- when(mDownloadHelper.isSuccessful(metadataId)).thenReturn(true);
-
+ when(mDownloadHelper.getDownloadStatus(metadataId))
+ .thenReturn(makeSuccessfulDownloadStatus(metadataId));
long contentId = 666;
String contentUrl = "http://test-content.org";
mDataStore.setProperty(Config.CONTENT_URL_PENDING, contentUrl);
@@ -132,23 +134,28 @@
}
@Test
- public void testDownloader_handleMetadataCompleteFailed() {
+ public void testDownloader_metadataDownloadFail_doNotStartContentDownload() {
long metadataId = 123;
mDataStore.setPropertyLong(Config.METADATA_URL_KEY, metadataId);
- when(mDownloadHelper.isSuccessful(metadataId)).thenReturn(false);
-
String contentUrl = "http://test-content.org";
mDataStore.setProperty(Config.CONTENT_URL_PENDING, contentUrl);
+ Intent downloadCompleteIntent = makeDownloadCompleteIntent(metadataId);
+ // In all these failure cases we give up on the download.
+ when(mDownloadHelper.getDownloadStatus(metadataId))
+ .thenReturn(
+ makeHttpErrorDownloadStatus(metadataId),
+ makeStorageErrorDownloadStatus(metadataId));
- mCertificateTransparencyDownloader.onReceive(
- mContext, makeDownloadCompleteIntent(metadataId));
+ mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
+ mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
verify(mDownloadHelper, never()).startDownload(contentUrl);
}
@Test
- public void testDownloader_handleContentCompleteInstallSuccessful() throws Exception {
- String version = "666";
+ public void testDownloader_contentDownloadSuccess_installSuccess_updateDataStore()
+ throws Exception {
+ String version = "456";
long contentId = 666;
File logListFile = File.createTempFile("log_list", "json");
Uri contentUri = Uri.fromFile(logListFile);
@@ -157,8 +164,8 @@
Uri metadataUri = Uri.fromFile(metadataFile);
mCertificateTransparencyDownloader.setPublicKey(
Base64.getEncoder().encodeToString(mPublicKey.getEncoded()));
-
- setUpDownloadComplete(version, metadataId, metadataUri, contentId, contentUri);
+ setUpContentDownloadCompleteSuccessful(
+ version, metadataId, metadataUri, contentId, contentUri);
when(mCertificateTransparencyInstaller.install(
eq(Config.COMPATIBILITY_VERSION), any(), eq(version)))
.thenReturn(true);
@@ -166,7 +173,6 @@
assertThat(mDataStore.getProperty(Config.VERSION)).isNull();
assertThat(mDataStore.getProperty(Config.CONTENT_URL)).isNull();
assertThat(mDataStore.getProperty(Config.METADATA_URL)).isNull();
-
mCertificateTransparencyDownloader.onReceive(
mContext, makeDownloadCompleteIntent(contentId));
@@ -178,16 +184,38 @@
}
@Test
- public void testDownloader_handleContentCompleteInstallFails() throws Exception {
- String version = "666";
+ public void testDownloader_contentDownloadFail_doNotInstall() throws Exception {
+ mDataStore.setProperty(Config.VERSION_PENDING, "123");
+ long contentId = 666;
+ Intent downloadCompleteIntent = makeDownloadCompleteIntent(contentId);
+ // In all these failure cases we give up on the download.
+ when(mDownloadHelper.getDownloadStatus(contentId))
+ .thenReturn(
+ makeHttpErrorDownloadStatus(contentId),
+ makeStorageErrorDownloadStatus(contentId));
+
+ mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
+ mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
+ mCertificateTransparencyDownloader.onReceive(mContext, downloadCompleteIntent);
+
+ verify(mCertificateTransparencyInstaller, never()).install(any(), any(), any());
+ assertThat(mDataStore.getProperty(Config.VERSION)).isNull();
+ assertThat(mDataStore.getProperty(Config.CONTENT_URL)).isNull();
+ assertThat(mDataStore.getProperty(Config.METADATA_URL)).isNull();
+ }
+
+ @Test
+ public void testDownloader_contentDownloadSuccess_installFail_doNotUpdateDataStore()
+ throws Exception {
+ String version = "456";
long contentId = 666;
File logListFile = File.createTempFile("log_list", "json");
Uri contentUri = Uri.fromFile(logListFile);
long metadataId = 123;
File metadataFile = sign(logListFile);
Uri metadataUri = Uri.fromFile(metadataFile);
-
- setUpDownloadComplete(version, metadataId, metadataUri, contentId, contentUri);
+ setUpContentDownloadCompleteSuccessful(
+ version, metadataId, metadataUri, contentId, contentUri);
when(mCertificateTransparencyInstaller.install(
eq(Config.COMPATIBILITY_VERSION), any(), eq(version)))
.thenReturn(false);
@@ -201,14 +229,15 @@
}
@Test
- public void testDownloader_handleContentCompleteVerificationFails() throws IOException {
- String version = "666";
+ public void testDownloader_contentDownloadSuccess_verificationFail_doNotInstall()
+ throws IOException {
+ String version = "456";
long contentId = 666;
Uri contentUri = Uri.fromFile(File.createTempFile("log_list", "json"));
long metadataId = 123;
Uri metadataUri = Uri.fromFile(File.createTempFile("log_list-wrong_metadata", "sig"));
-
- setUpDownloadComplete(version, metadataId, metadataUri, contentId, contentUri);
+ setUpContentDownloadCompleteSuccessful(
+ version, metadataId, metadataUri, contentId, contentUri);
mCertificateTransparencyDownloader.onReceive(
mContext, makeDownloadCompleteIntent(contentId));
@@ -221,17 +250,17 @@
}
@Test
- public void testDownloader_handleContentCompleteMissingVerificationPublicKey()
+ public void testDownloader_contentDownloadSuccess_missingVerificationPublicKey_doNotInstall()
throws Exception {
- String version = "666";
+ String version = "456";
long contentId = 666;
File logListFile = File.createTempFile("log_list", "json");
Uri contentUri = Uri.fromFile(logListFile);
long metadataId = 123;
File metadataFile = sign(logListFile);
Uri metadataUri = Uri.fromFile(metadataFile);
-
- setUpDownloadComplete(version, metadataId, metadataUri, contentId, contentUri);
+ setUpContentDownloadCompleteSuccessful(
+ version, metadataId, metadataUri, contentId, contentUri);
mCertificateTransparencyDownloader.onReceive(
mContext, makeDownloadCompleteIntent(contentId));
@@ -248,7 +277,7 @@
.putExtra(DownloadManager.EXTRA_DOWNLOAD_ID, downloadId);
}
- private void setUpDownloadComplete(
+ private void setUpContentDownloadCompleteSuccessful(
String version, long metadataId, Uri metadataUri, long contentId, Uri contentUri)
throws IOException {
mDataStore.setProperty(Config.VERSION_PENDING, version);
@@ -259,10 +288,34 @@
mDataStore.setPropertyLong(Config.CONTENT_URL_KEY, contentId);
mDataStore.setProperty(Config.CONTENT_URL_PENDING, contentUri.toString());
- when(mDownloadHelper.isSuccessful(contentId)).thenReturn(true);
+ when(mDownloadHelper.getDownloadStatus(contentId))
+ .thenReturn(makeSuccessfulDownloadStatus(contentId));
when(mDownloadHelper.getUri(contentId)).thenReturn(contentUri);
}
+ private DownloadStatus makeSuccessfulDownloadStatus(long downloadId) {
+ return DownloadStatus.builder()
+ .setDownloadId(downloadId)
+ .setStatus(DownloadManager.STATUS_SUCCESSFUL)
+ .build();
+ }
+
+ private DownloadStatus makeStorageErrorDownloadStatus(long downloadId) {
+ return DownloadStatus.builder()
+ .setDownloadId(downloadId)
+ .setStatus(DownloadManager.STATUS_FAILED)
+ .setReason(DownloadManager.ERROR_INSUFFICIENT_SPACE)
+ .build();
+ }
+
+ private DownloadStatus makeHttpErrorDownloadStatus(long downloadId) {
+ return DownloadStatus.builder()
+ .setDownloadId(downloadId)
+ .setStatus(DownloadManager.STATUS_FAILED)
+ .setReason(DownloadManager.ERROR_HTTP_DATA_ERROR)
+ .build();
+ }
+
private File sign(File file) throws IOException, GeneralSecurityException {
File signatureFile = File.createTempFile("log_list-metadata", "sig");
Signature signer = Signature.getInstance("SHA256withRSA");
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsInetAddressRecord.java b/service-t/src/com/android/server/connectivity/mdns/MdnsInetAddressRecord.java
index 4399f2d..3eae3c7 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsInetAddressRecord.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsInetAddressRecord.java
@@ -18,8 +18,6 @@
import android.annotation.Nullable;
-import androidx.annotation.VisibleForTesting;
-
import java.io.IOException;
import java.net.Inet4Address;
import java.net.Inet6Address;
@@ -29,7 +27,6 @@
import java.util.Objects;
/** An mDNS "AAAA" or "A" record, which holds an IPv6 or IPv4 address. */
-@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
public class MdnsInetAddressRecord extends MdnsRecord {
@Nullable private Inet6Address inet6Address;
@Nullable private Inet4Address inet4Address;
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsPointerRecord.java b/service-t/src/com/android/server/connectivity/mdns/MdnsPointerRecord.java
index 39bf653..e8f5e71 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsPointerRecord.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsPointerRecord.java
@@ -18,15 +18,12 @@
import android.annotation.Nullable;
-import androidx.annotation.VisibleForTesting;
-
import com.android.net.module.util.DnsUtils;
import java.io.IOException;
import java.util.Arrays;
/** An mDNS "PTR" record, which holds a name (the "pointer"). */
-@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
public class MdnsPointerRecord extends MdnsRecord {
private String[] pointer;
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceRecord.java b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceRecord.java
index fd716d2..907e2ff 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsServiceRecord.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsServiceRecord.java
@@ -18,8 +18,6 @@
import android.annotation.Nullable;
-import androidx.annotation.VisibleForTesting;
-
import com.android.net.module.util.DnsUtils;
import java.io.IOException;
@@ -28,7 +26,6 @@
import java.util.Objects;
/** An mDNS "SRV" record, which contains service information. */
-@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
public class MdnsServiceRecord extends MdnsRecord {
public static final int PROTO_NONE = 0;
public static final int PROTO_TCP = 1;
diff --git a/service-t/src/com/android/server/connectivity/mdns/MdnsTextRecord.java b/service-t/src/com/android/server/connectivity/mdns/MdnsTextRecord.java
index 77d1d7a..2b3ebf9 100644
--- a/service-t/src/com/android/server/connectivity/mdns/MdnsTextRecord.java
+++ b/service-t/src/com/android/server/connectivity/mdns/MdnsTextRecord.java
@@ -18,8 +18,6 @@
import android.annotation.Nullable;
-import androidx.annotation.VisibleForTesting;
-
import com.android.server.connectivity.mdns.MdnsServiceInfo.TextEntry;
import java.io.IOException;
@@ -29,7 +27,6 @@
import java.util.Objects;
/** An mDNS "TXT" record, which contains a list of {@link TextEntry}. */
-@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
public class MdnsTextRecord extends MdnsRecord {
private List<TextEntry> entries;
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
old mode 100755
new mode 100644
index 9015434..0d0f6fc
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -5551,7 +5551,7 @@
}
// Delayed teardown.
- if (nai.isCreated()) {
+ if (nai.isCreated() && !nai.isDestroyed()) {
try {
mNetd.networkSetPermissionForNetwork(nai.network.netId, INetd.PERMISSION_SYSTEM);
} catch (RemoteException e) {
diff --git a/tests/unit/java/com/android/server/connectivityservice/CSDestroyedNetworkTests.kt b/tests/unit/java/com/android/server/connectivityservice/CSDestroyedNetworkTests.kt
index 5c29e3a..b824531 100644
--- a/tests/unit/java/com/android/server/connectivityservice/CSDestroyedNetworkTests.kt
+++ b/tests/unit/java/com/android/server/connectivityservice/CSDestroyedNetworkTests.kt
@@ -27,9 +27,26 @@
import com.android.testutils.TestableNetworkCallback
import org.junit.Test
import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers.anyInt
+import org.mockito.ArgumentMatchers.argThat
+import org.mockito.ArgumentMatchers.eq
+import org.mockito.Mockito.clearInvocations
+import org.mockito.Mockito.inOrder
+import org.mockito.Mockito.never
+import org.mockito.Mockito.verify
private const val LONG_TIMEOUT_MS = 5_000
+private val CAPABILITIES = NetworkCapabilities.Builder()
+ .addTransportType(TRANSPORT_WIFI)
+ .build()
+
+private val REQUEST = NetworkRequest.Builder()
+ .clearCapabilities()
+ .addTransportType(TRANSPORT_WIFI)
+ .build()
+
+
@DevSdkIgnoreRunner.MonitorThreadLeak
@RunWith(DevSdkIgnoreRunner::class)
@SmallTest
@@ -37,29 +54,53 @@
class CSDestroyedNetworkTests : CSTest() {
@Test
fun testDestroyNetworkNotKeptWhenUnvalidated() {
- val nc = NetworkCapabilities.Builder()
- .addTransportType(TRANSPORT_WIFI)
- .build()
-
- val nr = NetworkRequest.Builder()
- .clearCapabilities()
- .addTransportType(TRANSPORT_WIFI)
- .build()
val cbRequest = TestableNetworkCallback()
val cbCallback = TestableNetworkCallback()
- cm.requestNetwork(nr, cbRequest)
- cm.registerNetworkCallback(nr, cbCallback)
+ cm.requestNetwork(REQUEST, cbRequest)
+ cm.registerNetworkCallback(REQUEST, cbCallback)
- val firstAgent = Agent(nc = nc)
+ val firstAgent = Agent(nc = CAPABILITIES)
firstAgent.connect()
cbCallback.expectAvailableCallbacks(firstAgent.network, validated = false)
firstAgent.unregisterAfterReplacement(LONG_TIMEOUT_MS)
- val secondAgent = Agent(nc = nc)
+ val secondAgent = Agent(nc = CAPABILITIES)
secondAgent.connect()
cbCallback.expectAvailableCallbacks(secondAgent.network, validated = false)
cbCallback.expect<Lost>(timeoutMs = 500) { it.network == firstAgent.network }
}
+
+ @Test
+ fun testDestroyNetworkWithDelayedTeardown() {
+ val cbRequest = TestableNetworkCallback()
+ val cbCallback = TestableNetworkCallback()
+ cm.requestNetwork(REQUEST, cbRequest)
+ cm.registerNetworkCallback(REQUEST, cbCallback)
+
+ val firstAgent = Agent(nc = CAPABILITIES)
+ firstAgent.connect()
+ firstAgent.setTeardownDelayMillis(1)
+ cbCallback.expectAvailableCallbacks(firstAgent.network, validated = false)
+
+ clearInvocations(netd)
+ val inOrder = inOrder(netd)
+ firstAgent.unregisterAfterReplacement(LONG_TIMEOUT_MS)
+
+ val secondAgent = Agent(nc = CAPABILITIES)
+ secondAgent.connect()
+ cbCallback.expectAvailableCallbacks(secondAgent.network, validated = false)
+ secondAgent.disconnect()
+
+ cbCallback.expect<Lost>(timeoutMs = 500) { it.network == firstAgent.network }
+ cbCallback.expect<Lost>(timeoutMs = 500) { it.network == secondAgent.network }
+ // onLost is fired before the network is destroyed.
+ waitForIdle()
+
+ inOrder.verify(netd).networkDestroy(eq(firstAgent.network.netId))
+ inOrder.verify(netd).networkCreate(argThat{ it.netId == secondAgent.network.netId })
+ inOrder.verify(netd).networkDestroy(eq(secondAgent.network.netId))
+ verify(netd, never()).networkSetPermissionForNetwork(anyInt(), anyInt())
+ }
}
diff --git a/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt b/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt
index 1f5ee32..9be7d11 100644
--- a/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt
+++ b/tests/unit/java/com/android/server/connectivityservice/base/CSAgentWrapper.kt
@@ -181,6 +181,7 @@
cb.eventuallyExpect<Lost> { it.network == agent.network }
}
+ fun setTeardownDelayMillis(delayMillis: Int) = agent.setTeardownDelayMillis(delayMillis)
fun unregisterAfterReplacement(timeoutMs: Int) = agent.unregisterAfterReplacement(timeoutMs)
fun sendLocalNetworkConfig(lnc: LocalNetworkConfig) = agent.sendLocalNetworkConfig(lnc)
diff --git a/thread/framework/java/android/net/thread/ThreadNetworkController.java b/thread/framework/java/android/net/thread/ThreadNetworkController.java
index 14d22d1..73a6bda 100644
--- a/thread/framework/java/android/net/thread/ThreadNetworkController.java
+++ b/thread/framework/java/android/net/thread/ThreadNetworkController.java
@@ -908,7 +908,6 @@
for (int i = 0; i < channelMaxPowers.size(); i++) {
int channel = channelMaxPowers.keyAt(i);
- int maxPower = channelMaxPowers.get(channel);
if ((channel < ActiveOperationalDataset.CHANNEL_MIN_24_GHZ)
|| (channel > ActiveOperationalDataset.CHANNEL_MAX_24_GHZ)) {
diff --git a/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java b/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java
index ddbef47..aeeed65 100644
--- a/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java
+++ b/thread/tests/integration/src/android/net/thread/BorderRoutingTest.java
@@ -58,6 +58,7 @@
import android.net.thread.utils.InfraNetworkDevice;
import android.net.thread.utils.IntegrationTestUtils;
import android.net.thread.utils.OtDaemonController;
+import android.net.thread.utils.TestTunNetworkUtils;
import android.net.thread.utils.ThreadFeatureCheckerRule;
import android.net.thread.utils.ThreadFeatureCheckerRule.RequiresIpv6MulticastRouting;
import android.net.thread.utils.ThreadFeatureCheckerRule.RequiresSimulationThreadDevice;
@@ -160,6 +161,7 @@
@After
public void tearDown() throws Exception {
mController.setTestNetworkAsUpstreamAndWait(null);
+ TestTunNetworkUtils.tearDownAllInfraNetworks();
mHandlerThread.quitSafely();
mHandlerThread.join();
@@ -226,19 +228,13 @@
FullThreadDevice ftd = mFtds.get(0);
joinNetworkAndWaitForOmr(ftd, DEFAULT_DATASET);
Inet6Address ftdOmr = ftd.getOmrAddress();
- // Create a new infra network and let Thread prefer it
- TestNetworkTracker oldInfraNetworkTracker = mInfraNetworkTracker;
- try {
- setUpInfraNetwork();
- mInfraNetworkReader = newPacketReader(mInfraNetworkTracker.getTestIface(), mHandler);
- startInfraDeviceAndWaitForOnLinkAddr();
+ setUpInfraNetwork();
+ mInfraNetworkReader = newPacketReader(mInfraNetworkTracker.getTestIface(), mHandler);
+ startInfraDeviceAndWaitForOnLinkAddr();
- mInfraDevice.sendEchoRequest(ftd.getOmrAddress());
+ mInfraDevice.sendEchoRequest(ftd.getOmrAddress());
- assertNotNull(pollForIcmpPacketOnInfraNetwork(ICMPV6_ECHO_REPLY_TYPE, ftdOmr));
- } finally {
- runAsShell(MANAGE_TEST_NETWORKS, () -> oldInfraNetworkTracker.teardown());
- }
+ assertNotNull(pollForIcmpPacketOnInfraNetwork(ICMPV6_ECHO_REPLY_TYPE, ftdOmr));
}
@Test
@@ -615,8 +611,6 @@
subscribeMulticastAddressAndWait(ftd, GROUP_ADDR_SCOPE_5);
Inet6Address ftdOmr = ftd.getOmrAddress();
- // Destroy infra link and re-create
- tearDownInfraNetwork();
setUpInfraNetwork();
mInfraNetworkReader = newPacketReader(mInfraNetworkTracker.getTestIface(), mHandler);
startInfraDeviceAndWaitForOnLinkAddr();
@@ -642,8 +636,6 @@
joinNetworkAndWaitForOmr(ftd, DEFAULT_DATASET);
Inet6Address ftdOmr = ftd.getOmrAddress();
- // Destroy infra link and re-create
- tearDownInfraNetwork();
setUpInfraNetwork();
mInfraNetworkReader = newPacketReader(mInfraNetworkTracker.getTestIface(), mHandler);
startInfraDeviceAndWaitForOnLinkAddr();
@@ -683,7 +675,7 @@
@Test
public void nat64_withAilNat64Prefix_threadDevicePingIpv4InfraDevice_outboundPacketIsForwarded()
throws Exception {
- tearDownInfraNetwork();
+ TestTunNetworkUtils.tearDownInfraNetwork(mInfraNetworkTracker);
LinkProperties lp = new LinkProperties();
// NAT64 feature requires the infra network to have an IPv4 default route.
lp.addRoute(
@@ -701,7 +693,7 @@
RouteInfo.RTN_UNICAST,
1500 /* mtu */));
lp.setNat64Prefix(AIL_NAT64_PREFIX);
- mInfraNetworkTracker = IntegrationTestUtils.setUpInfraNetwork(mContext, mController, lp);
+ mInfraNetworkTracker = TestTunNetworkUtils.setUpInfraNetwork(mContext, mController, lp);
mInfraNetworkReader = newPacketReader(mInfraNetworkTracker.getTestIface(), mHandler);
FullThreadDevice ftd = mFtds.get(0);
joinNetworkAndWaitForOmr(ftd, DEFAULT_DATASET);
@@ -717,16 +709,12 @@
}
private void setUpInfraNetwork() throws Exception {
- mInfraNetworkTracker = IntegrationTestUtils.setUpInfraNetwork(mContext, mController);
- }
-
- private void tearDownInfraNetwork() {
- IntegrationTestUtils.tearDownInfraNetwork(mInfraNetworkTracker);
+ mInfraNetworkTracker = TestTunNetworkUtils.setUpInfraNetwork(mContext, mController);
}
private void startInfraDeviceAndWaitForOnLinkAddr() {
mInfraDevice =
- IntegrationTestUtils.startInfraDeviceAndWaitForOnLinkAddr(mInfraNetworkReader);
+ TestTunNetworkUtils.startInfraDeviceAndWaitForOnLinkAddr(mInfraNetworkReader);
}
private void assertInfraLinkMemberOfGroup(Inet6Address address) throws Exception {
diff --git a/thread/tests/integration/src/android/net/thread/InternetAccessTest.kt b/thread/tests/integration/src/android/net/thread/InternetAccessTest.kt
index 162f58e..cb00611 100644
--- a/thread/tests/integration/src/android/net/thread/InternetAccessTest.kt
+++ b/thread/tests/integration/src/android/net/thread/InternetAccessTest.kt
@@ -28,12 +28,10 @@
import android.net.thread.utils.IntegrationTestUtils.joinNetworkAndWaitForOmr
import android.net.thread.utils.IntegrationTestUtils.leaveNetworkAndDisableThread
import android.net.thread.utils.IntegrationTestUtils.newPacketReader
-import android.net.thread.utils.IntegrationTestUtils.setUpInfraNetwork
-import android.net.thread.utils.IntegrationTestUtils.startInfraDeviceAndWaitForOnLinkAddr
-import android.net.thread.utils.IntegrationTestUtils.tearDownInfraNetwork
import android.net.thread.utils.IntegrationTestUtils.waitFor
import android.net.thread.utils.OtDaemonController
import android.net.thread.utils.TestDnsServer
+import android.net.thread.utils.TestTunNetworkUtils
import android.net.thread.utils.TestUdpEchoServer
import android.net.thread.utils.ThreadFeatureCheckerRule
import android.net.thread.utils.ThreadFeatureCheckerRule.RequiresSimulationThreadDevice
@@ -128,11 +126,11 @@
handler = Handler(handlerThread.looper)
ftds = ArrayList()
- infraNetworkTracker = setUpInfraNetwork(context, controller)
+ infraNetworkTracker = TestTunNetworkUtils.setUpInfraNetwork(context, controller)
// Create an infra network device.
infraNetworkReader = newPacketReader(infraNetworkTracker.testIface, handler)
- infraDevice = startInfraDeviceAndWaitForOnLinkAddr(infraNetworkReader)
+ infraDevice = TestTunNetworkUtils.startInfraDeviceAndWaitForOnLinkAddr(infraNetworkReader)
// Create a DNS server
dnsServer = TestDnsServer(infraNetworkReader, DNS_SERVER_ADDR, ANSWER_RECORDS)
@@ -150,7 +148,7 @@
@Throws(Exception::class)
fun tearDown() {
controller.setTestNetworkAsUpstreamAndWait(null)
- tearDownInfraNetwork(infraNetworkTracker)
+ TestTunNetworkUtils.tearDownAllInfraNetworks()
dnsServer.stop()
udpEchoServer.stop()
diff --git a/thread/tests/integration/src/android/net/thread/utils/IntegrationTestUtils.kt b/thread/tests/integration/src/android/net/thread/utils/IntegrationTestUtils.kt
index c3859c1..07d0390 100644
--- a/thread/tests/integration/src/android/net/thread/utils/IntegrationTestUtils.kt
+++ b/thread/tests/integration/src/android/net/thread/utils/IntegrationTestUtils.kt
@@ -649,57 +649,6 @@
)
}
- private fun defaultLinkProperties(): LinkProperties {
- val lp = LinkProperties()
- // TODO: use a fake DNS server
- lp.setDnsServers(listOf(parseNumericAddress("8.8.8.8")))
- // NAT64 feature requires the infra network to have an IPv4 default route.
- lp.addRoute(
- RouteInfo(
- IpPrefix("0.0.0.0/0") /* destination */,
- null /* gateway */,
- null /* iface */,
- RouteInfo.RTN_UNICAST, 1500 /* mtu */
- )
- )
- return lp
- }
-
- @JvmStatic
- @JvmOverloads
- fun startInfraDeviceAndWaitForOnLinkAddr(
- pollPacketReader: PollPacketReader,
- macAddress: MacAddress = MacAddress.fromString("1:2:3:4:5:6")
- ): InfraNetworkDevice {
- val infraDevice = InfraNetworkDevice(macAddress, pollPacketReader)
- infraDevice.runSlaac(Duration.ofSeconds(60))
- requireNotNull(infraDevice.ipv6Addr)
- return infraDevice
- }
-
- @JvmStatic
- @JvmOverloads
- @Throws(java.lang.Exception::class)
- fun setUpInfraNetwork(
- context: Context,
- controller: ThreadNetworkControllerWrapper,
- lp: LinkProperties = defaultLinkProperties()
- ): TestNetworkTracker {
- val infraNetworkTracker: TestNetworkTracker =
- runAsShell(
- MANAGE_TEST_NETWORKS,
- supplier = { initTestNetwork(context, lp, setupTimeoutMs = 5000) })
- val infraNetworkName: String = infraNetworkTracker.testIface.getInterfaceName()
- controller.setTestNetworkAsUpstreamAndWait(infraNetworkName)
-
- return infraNetworkTracker
- }
-
- @JvmStatic
- fun tearDownInfraNetwork(testNetworkTracker: TestNetworkTracker) {
- runAsShell(MANAGE_TEST_NETWORKS) { testNetworkTracker.teardown() }
- }
-
/**
* Stop the ot-daemon by shell command.
*/
diff --git a/thread/tests/integration/src/android/net/thread/utils/TestTunNetworkUtils.kt b/thread/tests/integration/src/android/net/thread/utils/TestTunNetworkUtils.kt
new file mode 100644
index 0000000..1667980
--- /dev/null
+++ b/thread/tests/integration/src/android/net/thread/utils/TestTunNetworkUtils.kt
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2024 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 android.net.thread.utils
+
+import android.Manifest.permission.MANAGE_TEST_NETWORKS
+import android.content.Context
+import android.net.InetAddresses.parseNumericAddress
+import android.net.IpPrefix
+import android.net.LinkProperties
+import android.net.MacAddress
+import android.net.RouteInfo
+import com.android.testutils.PollPacketReader
+import com.android.testutils.TestNetworkTracker
+import com.android.testutils.initTestNetwork
+import com.android.testutils.runAsShell
+import java.time.Duration
+
+object TestTunNetworkUtils {
+ private val networkTrackers = mutableListOf<TestNetworkTracker>()
+
+ @JvmStatic
+ @JvmOverloads
+ fun setUpInfraNetwork(
+ context: Context,
+ controller: ThreadNetworkControllerWrapper,
+ lp: LinkProperties = defaultLinkProperties(),
+ ): TestNetworkTracker {
+ val infraNetworkTracker: TestNetworkTracker =
+ runAsShell(
+ MANAGE_TEST_NETWORKS,
+ supplier = { initTestNetwork(context, lp, setupTimeoutMs = 5000) },
+ )
+ val infraNetworkName: String = infraNetworkTracker.testIface.getInterfaceName()
+ controller.setTestNetworkAsUpstreamAndWait(infraNetworkName)
+ networkTrackers.add(infraNetworkTracker)
+
+ return infraNetworkTracker
+ }
+
+ @JvmStatic
+ fun tearDownInfraNetwork(testNetworkTracker: TestNetworkTracker) {
+ runAsShell(MANAGE_TEST_NETWORKS) { testNetworkTracker.teardown() }
+ }
+
+ @JvmStatic
+ fun tearDownAllInfraNetworks() {
+ networkTrackers.forEach { tearDownInfraNetwork(it) }
+ networkTrackers.clear()
+ }
+
+ @JvmStatic
+ @JvmOverloads
+ fun startInfraDeviceAndWaitForOnLinkAddr(
+ pollPacketReader: PollPacketReader,
+ macAddress: MacAddress = MacAddress.fromString("1:2:3:4:5:6"),
+ ): InfraNetworkDevice {
+ val infraDevice = InfraNetworkDevice(macAddress, pollPacketReader)
+ infraDevice.runSlaac(Duration.ofSeconds(60))
+ requireNotNull(infraDevice.ipv6Addr)
+ return infraDevice
+ }
+
+ private fun defaultLinkProperties(): LinkProperties {
+ val lp = LinkProperties()
+ // TODO: use a fake DNS server
+ lp.setDnsServers(listOf(parseNumericAddress("8.8.8.8")))
+ // NAT64 feature requires the infra network to have an IPv4 default route.
+ lp.addRoute(
+ RouteInfo(
+ IpPrefix("0.0.0.0/0") /* destination */,
+ null /* gateway */,
+ null /* iface */,
+ RouteInfo.RTN_UNICAST,
+ 1500, /* mtu */
+ )
+ )
+ return lp
+ }
+}