HADOOP-10565. Support IP ranges (CIDR) in proxyuser.hosts. (Contributed by Benoy Antony)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1605987 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -480,6 +480,9 @@ Release 2.5.0 - UNRELEASED
     HADOOP-10754. Reenable several HA ZooKeeper-related tests on Windows.
     (cnauroth)
 
+    HADOOP-10565. Support IP ranges (CIDR) in proxyuser.hosts. (Benoy Antony
+    via Arpit Agarwal)
+
   OPTIMIZATIONS
 
   BUG FIXES 

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/DefaultImpersonationProvider.java

@@ -18,8 +18,6 @@
 
 package org.apache.hadoop.security.authorize;
 
-import java.net.InetAddress;
-import java.net.UnknownHostException;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
@@ -28,7 +26,7 @@ import java.util.regex.Pattern;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.security.UserGroupInformation;
-import org.apache.hadoop.util.StringUtils;
+import org.apache.hadoop.util.MachineList;
 
 import com.google.common.annotations.VisibleForTesting;
 
@@ -46,8 +44,8 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
   // acl and list of hosts per proxyuser
   private Map<String, AccessControlList> proxyUserAcl = 
     new HashMap<String, AccessControlList>();
-  private Map<String, Collection<String>> proxyHosts = 
+  private static Map<String, MachineList> proxyHosts = 
-    new HashMap<String, Collection<String>>();
+    new HashMap<String, MachineList>();
   private Configuration conf;
 
   @Override
@@ -70,7 +68,7 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
     allMatchKeys = conf.getValByRegex(CONF_HADOOP_PROXYUSER_RE_HOSTS);
     for(Entry<String, String> entry : allMatchKeys.entrySet()) {
       proxyHosts.put(entry.getKey(),
-          StringUtils.getTrimmedStringCollection(entry.getValue()));
+          new MachineList(entry.getValue()));
     }
   }
 
@@ -95,27 +93,10 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
           + " is not allowed to impersonate " + user.getUserName());
     }
 
-    boolean ipAuthorized = false;
+    MachineList MachineList = proxyHosts.get(
-    Collection<String> ipList = proxyHosts.get(
         getProxySuperuserIpConfKey(realUser.getShortUserName()));
 
-    if (isWildcardList(ipList)) {
+    if(!MachineList.includes(remoteAddress)) {
-      ipAuthorized = true;
-    } else if (ipList != null && !ipList.isEmpty()) {
-      for (String allowedHost : ipList) {
-        InetAddress hostAddr;
-        try {
-          hostAddr = InetAddress.getByName(allowedHost);
-        } catch (UnknownHostException e) {
-          continue;
-        }
-        if (hostAddr.getHostAddress().equals(remoteAddress)) {
-          // Authorization is successful
-          ipAuthorized = true;
-        }
-      }
-    }
-    if(!ipAuthorized) {
       throw new AuthorizationException("Unauthorized connection for super-user: "
           + realUser.getUserName() + " from IP " + remoteAddress);
     }
@@ -129,16 +110,6 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
     return key;
   }
   
-  /**
-   * Return true if the configuration specifies the special configuration value
-   * "*", indicating that any group or host list is allowed to use this configuration.
-   */
-  private boolean isWildcardList(Collection<String> list) {
-    return (list != null) &&
-    (list.size() == 1) &&
-    (list.contains("*"));
-  }
-  
   /**
    * Returns configuration key for effective usergroups allowed for a superuser
    * 
@@ -180,6 +151,12 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
 
   @VisibleForTesting
   public Map<String, Collection<String>> getProxyHosts() {
-    return proxyHosts;
+    Map<String, Collection<String>> tmpProxyHosts = 
+        new HashMap<String, Collection<String>>();
+    for (Map.Entry<String, MachineList> proxyHostEntry :proxyHosts.entrySet()) {
+      tmpProxyHosts.put(proxyHostEntry.getKey(), 
+          proxyHostEntry.getValue().getCollection());
+    }
+    return tmpProxyHosts;
   }
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/MachineList.java

@@ -0,0 +1,203 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.hadoop.util;
+
+import java.net.InetAddress;
+
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.commons.net.util.SubnetUtils;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.net.InetAddresses;
+
+/**
+ * Container class which holds a list of ip/host addresses and 
+ * answers membership queries.
+ * .
+ * Accepts list of ip addresses, ip addreses in CIDR format and/or 
+ * host addresses.
+ */
+
+public class MachineList {
+  
+  public static final Log LOG = LogFactory.getLog(MachineList.class);
+
+  /**
+   * InetAddressFactory is used to obtain InetAddress from host.
+   * This class makes it easy to simulate host to ip mappings during testing.
+   *
+   */
+  public static class InetAddressFactory {
+
+    static final InetAddressFactory S_INSTANCE = new InetAddressFactory();
+
+    public InetAddress getByName (String host) throws UnknownHostException {
+      return InetAddress.getByName(host);
+    }
+  }
+
+  private final boolean all;
+  private final Set<String> ipAddresses;
+  private final List<SubnetUtils.SubnetInfo> cidrAddresses;
+  private final Set<String> hostNames;
+  private final InetAddressFactory addressFactory;
+
+  /**
+   * 
+   * @param hostEntries comma separated ip/cidr/host addresses
+   */
+  public MachineList(String hostEntries) {
+    this(StringUtils.getTrimmedStringCollection(hostEntries),
+        InetAddressFactory.S_INSTANCE);
+  }
+
+  /**
+   * Accepts a collection of ip/cidr/host addresses
+   * 
+   * @param hostEntries
+   * @param addressFactory addressFactory to convert host to InetAddress
+   */
+  public MachineList(Collection<String> hostEntries, InetAddressFactory addressFactory) {
+    this.addressFactory = addressFactory;
+    if (hostEntries != null) {
+      if ((hostEntries.size() == 1) && (hostEntries.contains("*"))) {
+        all = true; 
+        ipAddresses = null; 
+        hostNames = null; 
+        cidrAddresses = null; 
+      } else {
+        all = false;
+        Set<String> ips = new HashSet<String>();
+        List<SubnetUtils.SubnetInfo> cidrs = new LinkedList<SubnetUtils.SubnetInfo>();
+        Set<String> hosts = new HashSet<String>();
+        for (String hostEntry : hostEntries) {
+          //ip address range
+          if (hostEntry.indexOf("/") > -1) {
+            try {
+              SubnetUtils subnet = new SubnetUtils(hostEntry);
+              subnet.setInclusiveHostCount(true);
+              cidrs.add(subnet.getInfo());
+            } catch (IllegalArgumentException e) {
+              LOG.warn("Invalid CIDR syntax : " + hostEntry);
+              throw e;
+            }
+          } else if (InetAddresses.isInetAddress(hostEntry)) { //ip address
+            ips.add(hostEntry);
+          } else { //hostname
+            hosts.add(hostEntry);
+          }
+        }
+        ipAddresses = (ips.size() > 0) ? ips : null;
+        cidrAddresses = (cidrs.size() > 0) ? cidrs : null;
+        hostNames = (hosts.size() > 0) ? hosts : null;
+      }
+    } else {
+      all = false; 
+      ipAddresses = null;
+      hostNames = null; 
+      cidrAddresses = null; 
+    }
+  }
+  /**
+   * Accepts an ip address and return true if ipAddress is in the list
+   * @param ipAddress
+   * @return true if ipAddress is part of the list
+   */
+  public boolean includes(String ipAddress) {
+    
+    if (all) {
+      return true;
+    }
+    
+    //check in the set of ipAddresses
+    if ((ipAddresses != null) && ipAddresses.contains(ipAddress)) {
+      return true;
+    }
+    
+    //iterate through the ip ranges for inclusion
+    if (cidrAddresses != null) {
+      for(SubnetUtils.SubnetInfo cidrAddress : cidrAddresses) {
+        if(cidrAddress.isInRange(ipAddress)) {
+          return true;
+        }
+      }
+    }
+    
+    //check if the ipAddress matches one of hostnames
+    if (hostNames != null) {
+      //convert given ipAddress to hostname and look for a match
+      InetAddress hostAddr;
+      try {
+        hostAddr = addressFactory.getByName(ipAddress);
+        if ((hostAddr != null) && hostNames.contains(hostAddr.getCanonicalHostName())) {
+          return true;
+        }
+      } catch (UnknownHostException e) {
+        //ignore the exception and proceed to resolve the list of hosts
+      }
+
+      //loop through host addresses and convert them to ip and look for a match
+      for (String host : hostNames) {
+        try {
+          hostAddr = addressFactory.getByName(host);
+        } catch (UnknownHostException e) {
+          continue;
+        }
+        if (hostAddr.getHostAddress().equals(ipAddress)) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
+  /**
+   * returns the contents of the MachineList as a Collection<String>
+   * This can be used for testing 
+   * @return contents of the MachineList
+   */
+  @VisibleForTesting
+  public Collection<String> getCollection() {
+    Collection<String> list = new ArrayList<String>();
+    if (all) {
+      list.add("*"); 
+    } else {
+      if (ipAddresses != null) {
+        list.addAll(ipAddresses);
+      }
+      if (hostNames != null) {
+        list.addAll(hostNames);
+      }
+      if (cidrAddresses != null) {
+        for(SubnetUtils.SubnetInfo cidrAddress : cidrAddresses) {
+          list.add(cidrAddress.getCidrSignature());
+        }
+      }
+    }
+    return list;
+  }
+}

hadoop-common-project/hadoop-common/src/site/apt/SecureMode.apt.vm

@@ -236,6 +236,25 @@ KVNO Timestamp         Principal
   </property>
 ----
 
+  The <<<hadoop.proxyuser.${superuser}.hosts>>> accepts list of ip addresses,
+  ip address ranges in CIDR format and/or host names.
+  
+  For example, by specifying as below in core-site.xml,
+  user named <<<oozie>>> accessing from hosts in the range 
+  10.222.0.0-15 and 10.113.221.221
+  can impersonate any user belonging to any group.
+  
+  ----
+  <property>
+    <name>hadoop.proxyuser.oozie.hosts</name>
+    <value>10.222.0.0/16,10.113.221.221</value>
+  </property>
+  <property>
+    <name>hadoop.proxyuser.oozie.groups</name>
+    <value>*</value>
+  </property>
+----
+
 ** Secure DataNode
 
   Because the data transfer protocol of DataNode

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestProxyUsers.java

@@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.security.SecureRandom;
 import java.util.Arrays;
 import java.util.Collection;
 
@@ -50,6 +51,7 @@ public class TestProxyUsers {
   private static final String[] SUDO_GROUP_NAMES =
     new String[] { "sudo_proxied_user" };
   private static final String PROXY_IP = "1.2.3.4";
+  private static final String PROXY_IP_RANGE = "10.222.0.0/16,10.113.221.221";
 
   /**
    * Test the netgroups (groups in ACL rules that start with @)
@@ -295,6 +297,29 @@ public class TestProxyUsers {
     assertNotAuthorized(proxyUserUgi, "1.2.3.5");
   }
   
+  @Test
+  public void testIPRange() {
+    Configuration conf = new Configuration();
+    conf.set(
+        DefaultImpersonationProvider.getProxySuperuserGroupConfKey(REAL_USER_NAME),
+        "*");
+    conf.set(
+        DefaultImpersonationProvider.getProxySuperuserIpConfKey(REAL_USER_NAME),
+        PROXY_IP_RANGE);
+    ProxyUsers.refreshSuperUserGroupsConfiguration(conf);
+
+    // First try proxying a group that's allowed
+    UserGroupInformation realUserUgi = UserGroupInformation
+        .createRemoteUser(REAL_USER_NAME);
+    UserGroupInformation proxyUserUgi = UserGroupInformation.createProxyUserForTesting(
+        PROXY_USER_NAME, realUserUgi, GROUP_NAMES);
+
+    // From good IP
+    assertAuthorized(proxyUserUgi, "10.222.0.0");
+    // From bad IP
+    assertNotAuthorized(proxyUserUgi, "10.221.0.0");
+  }
+
   @Test
   public void testWithDuplicateProxyGroups() throws Exception {
     Configuration conf = new Configuration();
@@ -431,4 +456,71 @@ public class TestProxyUsers {
       return null;
     }
   }
+  
+  public static void loadTest(String ipString, int testRange) {
+    Configuration conf = new Configuration();
+    conf.set(
+        DefaultImpersonationProvider.getProxySuperuserGroupConfKey(REAL_USER_NAME),
+        StringUtils.join(",", Arrays.asList(GROUP_NAMES)));
+
+    conf.set(
+        DefaultImpersonationProvider.getProxySuperuserIpConfKey(REAL_USER_NAME),
+        ipString
+        );
+    ProxyUsers.refreshSuperUserGroupsConfiguration(conf);
+
+
+    // First try proxying a group that's allowed
+    UserGroupInformation realUserUgi = UserGroupInformation
+        .createRemoteUser(REAL_USER_NAME);
+    UserGroupInformation proxyUserUgi = UserGroupInformation.createProxyUserForTesting(
+        PROXY_USER_NAME, realUserUgi, GROUP_NAMES);
+
+    long startTime = System.nanoTime();
+    SecureRandom sr = new SecureRandom();
+    for (int i=1; i < 1000000; i++){
+      try {
+        ProxyUsers.authorize(proxyUserUgi,  "1.2.3."+ sr.nextInt(testRange));
+       } catch (AuthorizationException e) {
+      }
+    }
+    long stopTime = System.nanoTime();
+    long elapsedTime = stopTime - startTime;
+    System.out.println(elapsedTime/1000000 + " ms");
+  }
+  
+  /**
+   * invokes the load Test
+   * A few sample invocations  are as below
+   * TestProxyUsers ip 128 256
+   * TestProxyUsers range 1.2.3.0/25 256
+   * TestProxyUsers ip 4 8
+   * TestProxyUsers range 1.2.3.0/30 8
+   * @param args
+   */
+  public static void main (String[] args){
+    String ipValues = null;
+
+    if (args.length != 3 || (!args[0].equals("ip") && !args[0].equals("range"))) {
+      System.out.println("Invalid invocation. The right syntax is ip/range <numberofIps/cidr> <testRange>");
+    }
+    else {
+      if (args[0].equals("ip")){
+        int numberOfIps =  Integer.parseInt(args[1]);
+        StringBuilder sb = new StringBuilder();
+        for (int i=0; i < numberOfIps; i++){
+          sb.append("1.2.3."+ i + ",");
+        }
+        ipValues = sb.toString();
+      }
+      else if (args[0].equals("range")){
+        ipValues = args[1];
+      }
+
+      int testRange = Integer.parseInt(args[2]);
+
+      loadTest(ipValues, testRange);
+    }
+  }
+
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestMachineList.java

@@ -0,0 +1,291 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 org.apache.hadoop.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Collection;
+
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class TestMachineList {
+  private static String IP_LIST = "10.119.103.110,10.119.103.112,10.119.103.114";
+  private static String IP_LIST_SPACES = 
+    " 10.119.103.110 , 10.119.103.112,10.119.103.114 ,10.119.103.110, ";
+  private static String CIDR_LIST = "10.222.0.0/16,10.241.23.0/24";
+  private static String CIDR_LIST1 = "10.222.0.0/16";
+  private static String CIDR_LIST2 = "10.241.23.0/24";
+  private static String INVALID_CIDR = "10.241/24";
+  private static String IP_CIDR_LIST =
+    "10.222.0.0/16,10.119.103.110,10.119.103.112,10.119.103.114,10.241.23.0/24";
+  private static String HOST_LIST = "host1,host4";
+  private static String HOSTNAME_IP_CIDR_LIST =
+    "host1,10.222.0.0/16,10.119.103.110,10.119.103.112,10.119.103.114,10.241.23.0/24,host4,";
+
+  @Test
+  public void testWildCard() {
+    //create MachineList with a list of of IPs
+    MachineList ml = new MachineList("*");
+
+    //test for inclusion with any IP
+    assertTrue(ml.includes("10.119.103.112"));
+    assertTrue(ml.includes("1.2.3.4"));
+  }
+
+  @Test
+  public void testIPList() {
+    //create MachineList with a list of of IPs
+    MachineList ml = new MachineList(IP_LIST);
+
+    //test for inclusion with an known IP
+    assertTrue(ml.includes("10.119.103.112"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("10.119.103.111"));
+  }
+
+  @Test
+  public void testIPListSpaces() {
+    //create MachineList with a ip string which has duplicate ip and spaces
+    MachineList ml = new MachineList(IP_LIST_SPACES);
+
+    //test for inclusion with an known IP
+    assertTrue(ml.includes("10.119.103.112"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("10.119.103.111"));
+  }
+
+  @Test
+  public void testStaticIPHostNameList()throws UnknownHostException {
+    //create MachineList with a list of of Hostnames
+    InetAddress addressHost1 = InetAddress.getByName("1.2.3.1");
+    InetAddress addressHost4 = InetAddress.getByName("1.2.3.4");
+
+    MachineList.InetAddressFactory addressFactory = 
+      Mockito.mock(MachineList.InetAddressFactory.class);
+    Mockito.when(addressFactory.getByName("host1")).thenReturn(addressHost1);
+    Mockito.when(addressFactory.getByName("host4")).thenReturn(addressHost4);
+
+    MachineList ml = new MachineList(
+        StringUtils.getTrimmedStringCollection(HOST_LIST), addressFactory);
+
+    //test for inclusion with an known IP
+    assertTrue(ml.includes("1.2.3.4"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("1.2.3.5"));
+  }
+
+  @Test
+  public void testHostNames() throws UnknownHostException {
+    //create MachineList with a list of of Hostnames
+    InetAddress addressHost1 = InetAddress.getByName("1.2.3.1");
+    InetAddress addressHost4 = InetAddress.getByName("1.2.3.4");
+    InetAddress addressMockHost4 = Mockito.mock(InetAddress.class);
+    Mockito.when(addressMockHost4.getCanonicalHostName()).thenReturn("differentName");
+
+    InetAddress addressMockHost5 = Mockito.mock(InetAddress.class);
+    Mockito.when(addressMockHost5.getCanonicalHostName()).thenReturn("host5");
+
+    MachineList.InetAddressFactory addressFactory = 
+      Mockito.mock(MachineList.InetAddressFactory.class);
+    Mockito.when(addressFactory.getByName("1.2.3.4")).thenReturn(addressMockHost4);
+    Mockito.when(addressFactory.getByName("1.2.3.5")).thenReturn(addressMockHost5);
+    Mockito.when(addressFactory.getByName("host1")).thenReturn(addressHost1);
+    Mockito.when(addressFactory.getByName("host4")).thenReturn(addressHost4);
+
+    MachineList ml = new MachineList(
+        StringUtils.getTrimmedStringCollection(HOST_LIST), addressFactory );
+
+    //test for inclusion with an known IP
+    assertTrue(ml.includes("1.2.3.4"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("1.2.3.5"));
+  }
+
+  @Test
+  public void testHostNamesReverserIpMatch() throws UnknownHostException {
+    //create MachineList with a list of of Hostnames
+    InetAddress addressHost1 = InetAddress.getByName("1.2.3.1");
+    InetAddress addressHost4 = InetAddress.getByName("1.2.3.4");
+    InetAddress addressMockHost4 =  Mockito.mock(InetAddress.class);
+    Mockito.when(addressMockHost4.getCanonicalHostName()).thenReturn("host4");
+
+    InetAddress addressMockHost5 =  Mockito.mock(InetAddress.class);
+    Mockito.when(addressMockHost5.getCanonicalHostName()).thenReturn("host5");
+
+    MachineList.InetAddressFactory addressFactory = 
+      Mockito.mock(MachineList.InetAddressFactory.class);
+    Mockito.when(addressFactory.getByName("1.2.3.4")).thenReturn(addressMockHost4);
+    Mockito.when(addressFactory.getByName("1.2.3.5")).thenReturn(addressMockHost5);
+    Mockito.when(addressFactory.getByName("host1")).thenReturn(addressHost1);
+    Mockito.when(addressFactory.getByName("host4")).thenReturn(addressHost4);
+
+    MachineList ml = new MachineList(
+        StringUtils.getTrimmedStringCollection(HOST_LIST), addressFactory );
+
+    //test for inclusion with an known IP
+    assertTrue(ml.includes("1.2.3.4"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("1.2.3.5"));
+  }
+
+  @Test
+  public void testCIDRs() {
+    //create MachineList with a list of of ip ranges specified in CIDR format
+    MachineList ml = new MachineList(CIDR_LIST);
+
+    //test for inclusion/exclusion 
+    assertFalse(ml.includes("10.221.255.255"));
+    assertTrue(ml.includes("10.222.0.0")); 
+    assertTrue(ml.includes("10.222.0.1"));
+    assertTrue(ml.includes("10.222.0.255"));
+    assertTrue(ml.includes("10.222.255.0"));
+    assertTrue(ml.includes("10.222.255.254"));
+    assertTrue(ml.includes("10.222.255.255"));
+    assertFalse(ml.includes("10.223.0.0"));
+
+    assertTrue(ml.includes("10.241.23.0"));
+    assertTrue(ml.includes("10.241.23.1"));
+    assertTrue(ml.includes("10.241.23.254"));
+    assertTrue(ml.includes("10.241.23.255"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("10.119.103.111"));
+
+  }
+
+  @Test
+  public void testCIDRWith16bitmask() {
+    //create MachineList with a list of of ip ranges specified in CIDR format
+    MachineList ml = new MachineList(CIDR_LIST1);
+
+    //test for inclusion/exclusion 
+    assertFalse(ml.includes("10.221.255.255"));
+    assertTrue(ml.includes("10.222.0.0")); 
+    assertTrue(ml.includes("10.222.0.1"));
+    assertTrue(ml.includes("10.222.0.255"));
+    assertTrue(ml.includes("10.222.255.0"));
+    assertTrue(ml.includes("10.222.255.254"));
+    assertTrue(ml.includes("10.222.255.255"));
+    assertFalse(ml.includes("10.223.0.0"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("10.119.103.111"));
+  }
+
+  @Test
+  public void testCIDRWith8BitMask() {
+    //create MachineList with a list of of ip ranges specified in CIDR format
+    MachineList ml = new MachineList(CIDR_LIST2);
+
+    //test for inclusion/exclusion  
+    assertFalse(ml.includes("10.241.22.255"));
+    assertTrue(ml.includes("10.241.23.0")); 
+    assertTrue(ml.includes("10.241.23.1"));
+    assertTrue(ml.includes("10.241.23.254"));
+    assertTrue(ml.includes("10.241.23.255")); 
+    assertFalse(ml.includes("10.241.24.0"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("10.119.103.111"));
+  }
+
+  //test invalid cidr
+  @Test
+  public void testInvalidCIDR() {
+    //create MachineList with an Invalid CIDR
+    try {
+    new MachineList(INVALID_CIDR);
+    fail("Expected IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      //expected Exception
+    } catch (Throwable t) {
+      fail ("Expected only IllegalArgumentException");
+    }
+  }
+  //
+  @Test
+  public void testIPandCIDRs() {
+    //create MachineList with a list of of ip ranges and ip addresses
+    MachineList ml = new MachineList(IP_CIDR_LIST);
+
+    //test for inclusion with an known IP
+    assertTrue(ml.includes("10.119.103.112"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("10.119.103.111"));
+
+    //CIDR Ranges
+    assertFalse(ml.includes("10.221.255.255"));
+    assertTrue(ml.includes("10.222.0.0")); 
+    assertTrue(ml.includes("10.222.255.255"));
+    assertFalse(ml.includes("10.223.0.0"));
+
+    assertFalse(ml.includes("10.241.22.255"));
+    assertTrue(ml.includes("10.241.23.0")); 
+    assertTrue(ml.includes("10.241.23.255")); 
+    assertFalse(ml.includes("10.241.24.0"));
+  }
+
+  @Test
+  public void testHostNameIPandCIDRs() {
+    //create MachineList with a mix of ip addresses , hostnames and ip ranges
+    MachineList ml = new MachineList(HOSTNAME_IP_CIDR_LIST);
+
+    //test for inclusion with an known IP
+    assertTrue(ml.includes("10.119.103.112"));
+
+    //test for exclusion with an unknown IP
+    assertFalse(ml.includes("10.119.103.111"));
+
+    //CIDR Ranges
+    assertFalse(ml.includes("10.221.255.255"));
+    assertTrue(ml.includes("10.222.0.0")); 
+    assertTrue(ml.includes("10.222.255.255"));
+    assertFalse(ml.includes("10.223.0.0"));
+
+    assertFalse(ml.includes("10.241.22.255"));
+    assertTrue(ml.includes("10.241.23.0")); 
+    assertTrue(ml.includes("10.241.23.255")); 
+    assertFalse(ml.includes("10.241.24.0"));
+  }
+
+  @Test
+  public void testGetCollection() {
+    //create MachineList with a mix of ip addresses , hostnames and ip ranges
+    MachineList ml = new MachineList(HOSTNAME_IP_CIDR_LIST);
+
+    Collection<String> col = ml.getCollection();
+    //test getCollectionton to return the full collection
+    assertEquals(7,ml.getCollection().size());
+
+    for (String item:StringUtils.getTrimmedStringCollection(HOSTNAME_IP_CIDR_LIST)) {
+      assertTrue(col.contains(item));
+    }
+  }
+}