From 7a4ff6447df79de973ff13751bea3bde689cfcd0 Mon Sep 17 00:00:00 2001 From: Kurt Marcinkiewicz Date: Mon, 5 Mar 2018 14:45:04 -0800 Subject: [PATCH 1/2] Allow specifying a network for SNTP time sync Permits syncing over a specific network instead of the default for the process. This was causing an issue with Android Wear devices paired with iOS where the default network is bluetooth (see b/32663274). This CL is in support of ag/3776564 Bug: 32663274 Test: adb shell am instrument -e class android.net.SntpClientTest -w \ com.android.frameworks.coretests/android.support.test.runner.AndroidJUnitRunner (cherry-pick of pi-dev Ic9fc169cf75457810d4992121d85d7642e350b90) Merged-In: I339c77063c72a9d76a5c4cb17052e20fb6e045a6 Merged-In: I8dfd1cad99c63efdc14c174c19f094a61cdfc44f Change-Id: I44df66688292b144ec7dfcdd9ae5d82489f82774 --- core/java/android/net/SntpClient.java | 16 +++++++--- core/java/android/util/NtpTrustedTime.java | 19 ++++++++++-- .../src/android/net/SntpClientTest.java | 31 +++++++++++++------ .../BandwidthEnforcementTestService.java | 12 +++++-- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/core/java/android/net/SntpClient.java b/core/java/android/net/SntpClient.java index ffc735c93aef1..66cdc99150a59 100644 --- a/core/java/android/net/SntpClient.java +++ b/core/java/android/net/SntpClient.java @@ -80,25 +80,27 @@ public class SntpClient { * * @param host host name of the server. * @param timeout network timeout in milliseconds. + * @param network network over which to send the request. * @return true if the transaction was successful. */ - public boolean requestTime(String host, int timeout) { + public boolean requestTime(String host, int timeout, Network network) { InetAddress address = null; try { - address = InetAddress.getByName(host); + address = network.getByName(host); } catch (Exception e) { EventLogTags.writeNtpFailure(host, e.toString()); if (DBG) Log.d(TAG, "request time failed: " + e); return false; } - return requestTime(address, NTP_PORT, timeout); + return requestTime(address, NTP_PORT, timeout, network); } - public boolean requestTime(InetAddress address, int port, int timeout) { + public boolean requestTime(InetAddress address, int port, int timeout, Network network) { DatagramSocket socket = null; final int oldTag = TrafficStats.getAndSetThreadStatsTag(TrafficStats.TAG_SYSTEM_NTP); try { socket = new DatagramSocket(); + network.bindSocket(socket); socket.setSoTimeout(timeout); byte[] buffer = new byte[NTP_PACKET_SIZE]; DatagramPacket request = new DatagramPacket(buffer, buffer.length, address, port); @@ -168,6 +170,12 @@ public class SntpClient { return true; } + @Deprecated + public boolean requestTime(String host, int timeout) { + Log.w(TAG, "Shame on you for calling the hidden API requestTime()!"); + return false; + } + /** * Returns the time computed from the NTP transaction. * diff --git a/core/java/android/util/NtpTrustedTime.java b/core/java/android/util/NtpTrustedTime.java index ed2d3c60fcd04..30d7b6c0786cc 100644 --- a/core/java/android/util/NtpTrustedTime.java +++ b/core/java/android/util/NtpTrustedTime.java @@ -20,6 +20,7 @@ import android.content.ContentResolver; import android.content.Context; import android.content.res.Resources; import android.net.ConnectivityManager; +import android.net.Network; import android.net.NetworkInfo; import android.net.SntpClient; import android.os.SystemClock; @@ -80,6 +81,18 @@ public class NtpTrustedTime implements TrustedTime { @Override public boolean forceRefresh() { + // We can't do this at initialization time: ConnectivityService might not be running yet. + synchronized (this) { + if (mCM == null) { + mCM = sContext.getSystemService(ConnectivityManager.class); + } + } + + final Network network = mCM == null ? null : mCM.getActiveNetwork(); + return forceRefresh(network); + } + + public boolean forceRefresh(Network network) { if (TextUtils.isEmpty(mServer)) { // missing server, so no trusted time available return false; @@ -88,11 +101,11 @@ public class NtpTrustedTime implements TrustedTime { // We can't do this at initialization time: ConnectivityService might not be running yet. synchronized (this) { if (mCM == null) { - mCM = (ConnectivityManager) sContext.getSystemService(Context.CONNECTIVITY_SERVICE); + mCM = sContext.getSystemService(ConnectivityManager.class); } } - final NetworkInfo ni = mCM == null ? null : mCM.getActiveNetworkInfo(); + final NetworkInfo ni = mCM == null ? null : mCM.getNetworkInfo(network); if (ni == null || !ni.isConnected()) { if (LOGD) Log.d(TAG, "forceRefresh: no connectivity"); return false; @@ -101,7 +114,7 @@ public class NtpTrustedTime implements TrustedTime { if (LOGD) Log.d(TAG, "forceRefresh() from cache miss"); final SntpClient client = new SntpClient(); - if (client.requestTime(mServer, (int) mTimeout)) { + if (client.requestTime(mServer, (int) mTimeout, network)) { mHasCache = true; mCachedNtpTime = client.getNtpTime(); mCachedNtpElapsedRealtime = client.getNtpTimeReference(); diff --git a/core/tests/coretests/src/android/net/SntpClientTest.java b/core/tests/coretests/src/android/net/SntpClientTest.java index 8b8cf671e723f..63d4080f64750 100644 --- a/core/tests/coretests/src/android/net/SntpClientTest.java +++ b/core/tests/coretests/src/android/net/SntpClientTest.java @@ -16,7 +16,8 @@ package android.net; -import android.net.SntpClient; +import android.content.Context; +import android.test.AndroidTestCase; import android.util.Log; import libcore.util.HexEncoding; @@ -26,10 +27,9 @@ import java.net.DatagramSocket; import java.net.InetAddress; import java.net.SocketException; import java.util.Arrays; -import junit.framework.TestCase; -public class SntpClientTest extends TestCase { +public class SntpClientTest extends AndroidTestCase { private static final String TAG = "SntpClientTest"; private static final int ORIGINATE_TIME_OFFSET = 24; @@ -61,20 +61,29 @@ public class SntpClientTest extends TestCase { private final SntpTestServer mServer = new SntpTestServer(); private final SntpClient mClient = new SntpClient(); + private Network mNetwork; + + @Override + protected void setUp() throws Exception { + super.setUp(); + ConnectivityManager mCM = getContext().getSystemService(ConnectivityManager.class); + mNetwork = mCM.getActiveNetwork(); + } + public void testBasicWorkingSntpClientQuery() throws Exception { mServer.setServerReply(HexEncoding.decode(WORKING_VERSION4.toCharArray(), false)); - assertTrue(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500)); + assertTrue(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500, mNetwork)); assertEquals(1, mServer.numRequestsReceived()); assertEquals(1, mServer.numRepliesSent()); } public void testDnsResolutionFailure() throws Exception { - assertFalse(mClient.requestTime("ntp.server.doesnotexist.example", 5000)); + assertFalse(mClient.requestTime("ntp.server.doesnotexist.example", 5000, mNetwork)); } public void testTimeoutFailure() throws Exception { mServer.clearServerReply(); - assertFalse(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500)); + assertFalse(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500, mNetwork)); assertEquals(1, mServer.numRequestsReceived()); assertEquals(0, mServer.numRepliesSent()); } @@ -83,7 +92,7 @@ public class SntpClientTest extends TestCase { final byte[] reply = HexEncoding.decode(WORKING_VERSION4.toCharArray(), false); reply[0] |= (byte) 0xc0; mServer.setServerReply(reply); - assertFalse(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500)); + assertFalse(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500, mNetwork)); assertEquals(1, mServer.numRequestsReceived()); assertEquals(1, mServer.numRepliesSent()); } @@ -95,7 +104,8 @@ public class SntpClientTest extends TestCase { reply[0] &= (byte) 0xf8; reply[0] |= (byte) i; mServer.setServerReply(reply); - final boolean rval = mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500); + final boolean rval = mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500, + mNetwork); switch (i) { case NTP_MODE_SERVER: case NTP_MODE_BROADCAST: @@ -119,7 +129,8 @@ public class SntpClientTest extends TestCase { final String logMsg = "stratum: " + i; reply[1] = (byte) i; mServer.setServerReply(reply); - final boolean rval = mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500); + final boolean rval = mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500, + mNetwork); if (STRATUM_MIN <= i && i <= STRATUM_MAX) { assertTrue(logMsg, rval); } else { @@ -134,7 +145,7 @@ public class SntpClientTest extends TestCase { final byte[] reply = HexEncoding.decode(WORKING_VERSION4.toCharArray(), false); Arrays.fill(reply, TRANSMIT_TIME_OFFSET, TRANSMIT_TIME_OFFSET + 8, (byte) 0x00); mServer.setServerReply(reply); - assertFalse(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500)); + assertFalse(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500, mNetwork)); assertEquals(1, mServer.numRequestsReceived()); assertEquals(1, mServer.numRepliesSent()); } diff --git a/tests/BandwidthTests/src/com/android/tests/bandwidthenforcement/BandwidthEnforcementTestService.java b/tests/BandwidthTests/src/com/android/tests/bandwidthenforcement/BandwidthEnforcementTestService.java index a2427f514e171..35f1e585931bd 100644 --- a/tests/BandwidthTests/src/com/android/tests/bandwidthenforcement/BandwidthEnforcementTestService.java +++ b/tests/BandwidthTests/src/com/android/tests/bandwidthenforcement/BandwidthEnforcementTestService.java @@ -16,7 +16,10 @@ package com.android.tests.bandwidthenforcement; import android.app.IntentService; +import android.content.Context; import android.content.Intent; +import android.net.ConnectivityManager; +import android.net.Network; import android.net.SntpClient; import android.os.Environment; import android.util.Log; @@ -55,7 +58,7 @@ public class BandwidthEnforcementTestService extends IntentService { String outputFile = intent.getStringExtra(OUTPUT_FILE); dumpResult("testUrlConnection", testUrlConnection(), outputFile); dumpResult("testUrlConnectionv6", testUrlConnectionv6(), outputFile); - dumpResult("testSntp", testSntp(), outputFile); + dumpResult("testSntp", testSntp(getApplicationContext()), outputFile); dumpResult("testDns", testDns(), outputFile); } @@ -138,9 +141,12 @@ public class BandwidthEnforcementTestService extends IntentService { * Tests to connect via sntp. * @return true if it was able to connect, false otherwise. */ - public static boolean testSntp() { + public static boolean testSntp(Context context) { final SntpClient client = new SntpClient(); - if (client.requestTime("0.pool.ntp.org", 10000)) { + final ConnectivityManager mCM = context.getSystemService(ConnectivityManager.class); + final Network network = mCM.getActiveNetwork(); + + if (client.requestTime("0.pool.ntp.org", 10000, network)) { return true; } return false; From 6657d42602a25264ccbf70ce2eb36f3974518139 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 16 May 2018 17:29:32 +0900 Subject: [PATCH 2/2] Fix SntpClientTest failures with no active network The last change is using getActiveNetwork to run the queries, which results in an NPE if there is no network on the device. Using network 0 causes no socket tagging and allows queries to the local test SNTP server to go through as previously. Also migrating to JUnit4 and moving mServer and mClient initializers to setUp(). Test: SntpClientTest now passes Change-Id: Ieb0a5d247129bcad89c1add0e9c1c504f516e2a9 --- .../src/android/net/SntpClientTest.java | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/core/tests/coretests/src/android/net/SntpClientTest.java b/core/tests/coretests/src/android/net/SntpClientTest.java index 63d4080f64750..d72738c6e32c3 100644 --- a/core/tests/coretests/src/android/net/SntpClientTest.java +++ b/core/tests/coretests/src/android/net/SntpClientTest.java @@ -16,11 +16,19 @@ package android.net; -import android.content.Context; -import android.test.AndroidTestCase; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertTrue; + +import android.support.test.runner.AndroidJUnit4; import android.util.Log; + import libcore.util.HexEncoding; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + import java.io.IOException; import java.net.DatagramPacket; import java.net.DatagramSocket; @@ -28,8 +36,8 @@ import java.net.InetAddress; import java.net.SocketException; import java.util.Arrays; - -public class SntpClientTest extends AndroidTestCase { +@RunWith(AndroidJUnit4.class) +public class SntpClientTest { private static final String TAG = "SntpClientTest"; private static final int ORIGINATE_TIME_OFFSET = 24; @@ -58,18 +66,19 @@ public class SntpClientTest extends AndroidTestCase { "d9ca945194bd3fff" + "d9ca945194bd4001"; - private final SntpTestServer mServer = new SntpTestServer(); - private final SntpClient mClient = new SntpClient(); - + private SntpTestServer mServer; + private SntpClient mClient; private Network mNetwork; - @Override - protected void setUp() throws Exception { - super.setUp(); - ConnectivityManager mCM = getContext().getSystemService(ConnectivityManager.class); - mNetwork = mCM.getActiveNetwork(); + @Before + public void setUp() throws Exception { + // NETID_UNSET allows the test to run, with a loopback server, even w/o external networking + mNetwork = new Network(ConnectivityManager.NETID_UNSET); + mServer = new SntpTestServer(); + mClient = new SntpClient(); } + @Test public void testBasicWorkingSntpClientQuery() throws Exception { mServer.setServerReply(HexEncoding.decode(WORKING_VERSION4.toCharArray(), false)); assertTrue(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500, mNetwork)); @@ -77,10 +86,12 @@ public class SntpClientTest extends AndroidTestCase { assertEquals(1, mServer.numRepliesSent()); } + @Test public void testDnsResolutionFailure() throws Exception { assertFalse(mClient.requestTime("ntp.server.doesnotexist.example", 5000, mNetwork)); } + @Test public void testTimeoutFailure() throws Exception { mServer.clearServerReply(); assertFalse(mClient.requestTime(mServer.getAddress(), mServer.getPort(), 500, mNetwork)); @@ -88,6 +99,7 @@ public class SntpClientTest extends AndroidTestCase { assertEquals(0, mServer.numRepliesSent()); } + @Test public void testIgnoreLeapNoSync() throws Exception { final byte[] reply = HexEncoding.decode(WORKING_VERSION4.toCharArray(), false); reply[0] |= (byte) 0xc0; @@ -97,6 +109,7 @@ public class SntpClientTest extends AndroidTestCase { assertEquals(1, mServer.numRepliesSent()); } + @Test public void testAcceptOnlyServerAndBroadcastModes() throws Exception { final byte[] reply = HexEncoding.decode(WORKING_VERSION4.toCharArray(), false); for (int i = 0; i <= 7; i++) { @@ -120,6 +133,7 @@ public class SntpClientTest extends AndroidTestCase { } } + @Test public void testAcceptableStrataOnly() throws Exception { final int STRATUM_MIN = 1; final int STRATUM_MAX = 15; @@ -141,6 +155,7 @@ public class SntpClientTest extends AndroidTestCase { } } + @Test public void testZeroTransmitTime() throws Exception { final byte[] reply = HexEncoding.decode(WORKING_VERSION4.toCharArray(), false); Arrays.fill(reply, TRANSMIT_TIME_OFFSET, TRANSMIT_TIME_OFFSET + 8, (byte) 0x00);