From 1b0234a5ada2bdb2c0788eb274f84039302e3c98 Mon Sep 17 00:00:00 2001 From: Hai Shalom Date: Fri, 26 Jul 2019 10:52:05 -0700 Subject: [Passpoint] OSU system exception when the URL is HTTP Added checks that the OSU URL is a HTTPS type, and that openConnection returns an object of HTTPS type before casting it. Added additional tests. Add addtional error logs when the OSU cert fails verifications. Bug: 138444946 Test: atest OsuServerConnectionTest Test: Verify class cast exception with HTTP URL before the change and a handled error when the change is applied. Change-Id: I85b88988f056efd993e19ba157df1d3987b60b27 --- .../server/wifi/hotspot2/OsuServerConnection.java | 61 ++++++++++++++++++---- .../wifi/hotspot2/OsuServerConnectionTest.java | 32 +++++++++--- 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/service/java/com/android/server/wifi/hotspot2/OsuServerConnection.java b/service/java/com/android/server/wifi/hotspot2/OsuServerConnection.java index c748ca1ac..94f584f72 100644 --- a/service/java/com/android/server/wifi/hotspot2/OsuServerConnection.java +++ b/service/java/com/android/server/wifi/hotspot2/OsuServerConnection.java @@ -43,6 +43,7 @@ import java.io.IOException; import java.io.InputStream; import java.net.HttpURLConnection; import java.net.URL; +import java.net.URLConnection; import java.security.KeyManagementException; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; @@ -163,7 +164,7 @@ public class OsuServerConnection { */ public boolean connect(@NonNull URL url, @NonNull Network network) { if (url == null) { - Log.e(TAG, "url is null"); + Log.e(TAG, "URL is null"); return false; } if (network == null) { @@ -171,6 +172,14 @@ public class OsuServerConnection { return false; } + String protocol = url.getProtocol(); + // According to section 7.5.1 OSU operational requirements, in HS2.0 R3 specification, + // the URL must be HTTPS. Enforce it here. + if (!TextUtils.equals(protocol, "https")) { + Log.e(TAG, "OSU server URL must be HTTPS"); + return false; + } + mHandler.post(() -> performTlsConnection(url, network)); return true; } @@ -271,13 +280,37 @@ public class OsuServerConnection { mNetwork = network; mUrl = url; - HttpsURLConnection urlConnection; + URLConnection urlConnection; + HttpsURLConnection httpsURLConnection; + + try { + urlConnection = mNetwork.openConnection(mUrl); + } catch (IOException e) { + Log.e(TAG, "Unable to establish a URL connection: " + e); + if (mOsuServerCallbacks != null) { + mOsuServerCallbacks.onServerConnectionStatus( + mOsuServerCallbacks.getSessionId(), + false); + } + return; + } + + if (urlConnection instanceof HttpsURLConnection) { + httpsURLConnection = (HttpsURLConnection) urlConnection; + } else { + Log.e(TAG, "Invalid URL connection"); + if (mOsuServerCallbacks != null) { + mOsuServerCallbacks.onServerConnectionStatus(mOsuServerCallbacks.getSessionId(), + false); + } + return; + } + try { - urlConnection = (HttpsURLConnection) mNetwork.openConnection(mUrl); - urlConnection.setSSLSocketFactory(mSocketFactory); - urlConnection.setConnectTimeout(HttpsServiceConnection.DEFAULT_TIMEOUT_MS); - urlConnection.setReadTimeout(HttpsServiceConnection.DEFAULT_TIMEOUT_MS); - urlConnection.connect(); + httpsURLConnection.setSSLSocketFactory(mSocketFactory); + httpsURLConnection.setConnectTimeout(HttpsServiceConnection.DEFAULT_TIMEOUT_MS); + httpsURLConnection.setReadTimeout(HttpsServiceConnection.DEFAULT_TIMEOUT_MS); + httpsURLConnection.connect(); } catch (IOException e) { Log.e(TAG, "Unable to establish a URL connection: " + e); if (mOsuServerCallbacks != null) { @@ -286,7 +319,7 @@ public class OsuServerConnection { } return; } - mUrlConnection = urlConnection; + mUrlConnection = httpsURLConnection; if (mOsuServerCallbacks != null) { mOsuServerCallbacks.onServerConnectionStatus(mOsuServerCallbacks.getSessionId(), true); } @@ -572,9 +605,15 @@ public class OsuServerConnection { (SSLSocket) null); certsValid = true; } catch (CertificateException e) { - Log.e(TAG, "Unable to validate certs " + e); - if (mVerboseLoggingEnabled) { - e.printStackTrace(); + Log.e(TAG, "Certificate validation failure: " + e); + int i = 0; + for (X509Certificate cert : chain) { + // Provide some more details about the invalid certificate + Log.e(TAG, "Cert " + i + " details: " + cert.getSubjectDN()); + Log.e(TAG, "Not before: " + cert.getNotBefore() + ", not after: " + + cert.getNotAfter()); + Log.e(TAG, "Cert " + i + " issuer: " + cert.getIssuerDN()); + i++; } } if (mOsuServerCallbacks != null) { diff --git a/tests/wifitests/src/com/android/server/wifi/hotspot2/OsuServerConnectionTest.java b/tests/wifitests/src/com/android/server/wifi/hotspot2/OsuServerConnectionTest.java index c5baac721..843caf1a9 100644 --- a/tests/wifitests/src/com/android/server/wifi/hotspot2/OsuServerConnectionTest.java +++ b/tests/wifitests/src/com/android/server/wifi/hotspot2/OsuServerConnectionTest.java @@ -81,7 +81,8 @@ import javax.net.ssl.X509TrustManager; */ @SmallTest public class OsuServerConnectionTest { - private static final String TEST_VALID_URL = "http://www.google.com"; + private static final String TEST_VALID_URL = "https://www.google.com"; + private static final String TEST_INVALID_URL = "http://www.google.com"; private static final String AUTH_TYPE = "ECDHE_RSA"; private static final String PROVIDER_NAME_VALID = "Boingo"; private static final String PROVIDER_NAME_INVALID = "Boingo1"; @@ -90,11 +91,10 @@ public class OsuServerConnectionTest { private TestLooper mLooper = new TestLooper(); private OsuServerConnection mOsuServerConnection; - private URL mValidServerUrl; + private URL mServerUrl; private List> mProviderIdentities = new ArrayList<>(); private ArgumentCaptor mTrustManagerCaptor = ArgumentCaptor.forClass(TrustManager[].class); - private Map> mTrustCertsInfo = new HashMap<>(); @Mock PasspointProvisioner.OsuServerCallbacks mOsuServerCallbacks; @@ -114,7 +114,7 @@ public class OsuServerConnectionTest { mOsuServerConnection = new OsuServerConnection(mLooper.getLooper()); mOsuServerConnection.enableVerboseLogging(ENABLE_VERBOSE_LOGGING); mProviderIdentities.add(Pair.create(Locale.US, PROVIDER_NAME_VALID)); - mValidServerUrl = new URL(TEST_VALID_URL); + mServerUrl = new URL(TEST_VALID_URL); when(mWfaKeyStore.get()).thenReturn(mKeyStore); when(mOsuServerCallbacks.getSessionId()).thenReturn(TEST_SESSION_ID); when(mNetwork.openConnection(any(URL.class))).thenReturn(mUrlConnection); @@ -185,7 +185,7 @@ public class OsuServerConnectionTest { mOsuServerConnection.setEventCallback(mOsuServerCallbacks); assertTrue(mOsuServerConnection.canValidateServer()); - assertTrue(mOsuServerConnection.connect(mValidServerUrl, mNetwork)); + assertTrue(mOsuServerConnection.connect(mServerUrl, mNetwork)); mLooper.dispatchAll(); @@ -203,7 +203,7 @@ public class OsuServerConnectionTest { mOsuServerConnection.setEventCallback(mOsuServerCallbacks); assertTrue(mOsuServerConnection.canValidateServer()); - assertTrue(mOsuServerConnection.connect(mValidServerUrl, mNetwork)); + assertTrue(mOsuServerConnection.connect(mServerUrl, mNetwork)); mLooper.dispatchAll(); @@ -216,13 +216,16 @@ public class OsuServerConnectionTest { @Test public void verifyInitAndConnectCertValidationFailure() throws Exception { establishServerConnection(); + List certificateList = PasspointProvisioningTestUtil.getOsuCertsForTest(); + X509Certificate[] certificates = new X509Certificate[1]; + certificates[0] = certificateList.get(0); TrustManager[] trustManagers = mTrustManagerCaptor.getValue(); X509TrustManager trustManager = (X509TrustManager) trustManagers[0]; doThrow(new CertificateException()).when(mDelegate) .getTrustedChainForServer(any(X509Certificate[].class), anyString(), (Socket) isNull()); - trustManager.checkServerTrusted(new X509Certificate[1], AUTH_TYPE); + trustManager.checkServerTrusted(certificates, AUTH_TYPE); verify(mOsuServerCallbacks).onServerValidationStatus(anyInt(), eq(false)); } @@ -475,13 +478,26 @@ public class OsuServerConnectionTest { } } + /** + * Verifies initialization and opening URL connection failure for an HTTP URL (not HTTPS) + */ + @Test + public void verifyInitAndNetworkOpenURLConnectionFailedWithHttpUrl() throws Exception { + mServerUrl = new URL(TEST_INVALID_URL); + mOsuServerConnection.init(mTlsContext, mDelegate); + mOsuServerConnection.setEventCallback(mOsuServerCallbacks); + + assertTrue(mOsuServerConnection.canValidateServer()); + assertFalse(mOsuServerConnection.connect(mServerUrl, mNetwork)); + } + private void establishServerConnection() throws Exception { mOsuServerConnection.init(mTlsContext, mDelegate); mOsuServerConnection.setEventCallback(mOsuServerCallbacks); verify(mTlsContext).init(isNull(), mTrustManagerCaptor.capture(), isNull()); assertTrue(mOsuServerConnection.canValidateServer()); - assertTrue(mOsuServerConnection.connect(mValidServerUrl, mNetwork)); + assertTrue(mOsuServerConnection.connect(mServerUrl, mNetwork)); mLooper.dispatchAll(); verify(mOsuServerCallbacks).onServerConnectionStatus(anyInt(), eq(true)); -- cgit v1.2.3