From bfbda18d4810e0d706aa8408071a77d2e1ae31c6 Mon Sep 17 00:00:00 2001
From: Nathan Harold <nharold@google.com>
Date: Wed, 23 Aug 2017 13:46:33 -0700
Subject: [PATCH] Input Validation for IpSecService

All of the input to IpSecService over the Binder
interface needs to be validated both for sanity
and for safety.

-Sanity check all the parameters coming from binder.
-Added setters for IpSecConfig to decouple the test
 from the IpSecManager. This was needed because the
 input validation caused the tests to fail due to a
 null parameter that was previously un-tested.
-Added the mode flag to the IpSecConfig bundle this
 oversight was found during testing.
-Expose the getResourceId() methods for testing in
 UdpEncapsulationSocket, SecurityParameterIndex, and
 IpSecTransform classes.
-Remove the unneeded getIpSecConfig() from
 IpSecTransform: unneeded now that we can synthesize
 configs.

Bug: 38397094
Test: runtest frameworks-net
Change-Id: I5241fc7fbfa9816d54219acd8d81a9f7eef10dd4
---
 .../com/android/server/IpSecServiceTest.java  | 49 +++++++++++++------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/tests/net/java/com/android/server/IpSecServiceTest.java b/tests/net/java/com/android/server/IpSecServiceTest.java
index 23fee286b8..4d37982526 100644
--- a/tests/net/java/com/android/server/IpSecServiceTest.java
+++ b/tests/net/java/com/android/server/IpSecServiceTest.java
@@ -25,7 +25,6 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyLong;
-import static org.mockito.Matchers.anyObject;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
@@ -297,24 +296,23 @@ public class IpSecServiceTest {
         IpSecAlgorithm authAlgo =
                 new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_SHA256, AUTH_KEY, AUTH_KEY.length * 8);
 
-        InetAddress localAddr = InetAddress.getByAddress(new byte[] {127, 0, 0, 1});
-
+        InetAddress remoteAddr = InetAddress.getByName("8.8.4.4");
         /** Allocate and add SPI records in the IpSecService through IpSecManager interface. */
         IpSecManager.SecurityParameterIndex outSpi =
-                ipSecManager.reserveSecurityParameterIndex(IpSecTransform.DIRECTION_OUT, localAddr);
+                ipSecManager.reserveSecurityParameterIndex(
+                        IpSecTransform.DIRECTION_OUT, remoteAddr);
         IpSecManager.SecurityParameterIndex inSpi =
-                ipSecManager.reserveSecurityParameterIndex(IpSecTransform.DIRECTION_IN, localAddr);
+                ipSecManager.reserveSecurityParameterIndex(IpSecTransform.DIRECTION_IN, remoteAddr);
 
-        IpSecConfig ipSecConfig =
-                new IpSecTransform.Builder(mMockContext)
-                        .setSpi(IpSecTransform.DIRECTION_OUT, outSpi)
-                        .setSpi(IpSecTransform.DIRECTION_IN, inSpi)
-                        .setEncryption(IpSecTransform.DIRECTION_OUT, encryptAlgo)
-                        .setAuthentication(IpSecTransform.DIRECTION_OUT, authAlgo)
-                        .setEncryption(IpSecTransform.DIRECTION_IN, encryptAlgo)
-                        .setAuthentication(IpSecTransform.DIRECTION_IN, authAlgo)
-                        .getIpSecConfig();
-        return ipSecConfig;
+        IpSecConfig config = new IpSecConfig();
+        config.setSpiResourceId(IpSecTransform.DIRECTION_IN, inSpi.getResourceId());
+        config.setSpiResourceId(IpSecTransform.DIRECTION_OUT, outSpi.getResourceId());
+        config.setEncryption(IpSecTransform.DIRECTION_OUT, encryptAlgo);
+        config.setAuthentication(IpSecTransform.DIRECTION_OUT, authAlgo);
+        config.setEncryption(IpSecTransform.DIRECTION_IN, encryptAlgo);
+        config.setAuthentication(IpSecTransform.DIRECTION_IN, authAlgo);
+        config.setRemoteAddress(remoteAddr.getHostName());
+        return config;
     }
 
     @Test
@@ -432,4 +430,25 @@ public class IpSecServiceTest {
 
         verify(mMockNetd).ipSecRemoveTransportModeTransform(pfd.getFileDescriptor());
     }
+
+    @Test
+    public void testValidateIpAddresses() throws Exception {
+        String[] invalidAddresses =
+                new String[] {"www.google.com", "::", "2001::/64", "0.0.0.0", ""};
+        for (String address : invalidAddresses) {
+            try {
+                IpSecSpiResponse spiResp =
+                        mIpSecService.reserveSecurityParameterIndex(
+                                IpSecTransform.DIRECTION_OUT, address, DROID_SPI, new Binder());
+                fail("Invalid address was passed through IpSecService validation: " + address);
+            } catch (IllegalArgumentException e) {
+            } catch (Exception e) {
+                fail(
+                        "Invalid InetAddress was not caught in validation: "
+                                + address
+                                + ", Exception: "
+                                + e);
+            }
+        }
+    }
 }