ONOS-4507,ONOS-4774, ONOS-4775, ONOS-4776 + some minor fixes

Change-Id: I9eaf17b03899074d4b63e01e920fada6797158a0

@@ -45,6 +45,12 @@ public class ReviewCommand extends AbstractShellCommand {
             required = false, multiValued = false)
     String accept = null;
 
+
+    public static final String ANSI_RESET = "\u001B[0m";
+    public static final String ANSI_RED = "\u001B[31m";
+    public static final String ANSI_GREEN = "\u001B[32m";
+    public static final String ANSI_YELLOW = "\u001B[33m";
+
     @Override
     protected void execute() {
         ApplicationAdminService applicationAdminService = get(ApplicationAdminService.class);
@@ -86,38 +92,64 @@ public class ReviewCommand extends AbstractShellCommand {
         print("");
 
     }
+
+    /**
+     * TYPES.
+     * 0 - APP_PERM
+     * 1 - ADMIN SERVICE
+     * 2 - NB_SERVICE
+     * 3 - SB_SERVICE
+     * 4 - CLI_SERVICE
+     * 5 - ETC_SERVICE
+     * 6 - CRITICAL PERMISSIONS
+     * 7 - ETC
+     **/
     private void printMap(Map<Integer, List<Permission>> assortedMap) {
-        for (Integer type : assortedMap.keySet()) {
-            switch (type) {
-                case 0:
-                    for (Permission perm: assortedMap.get(0)) {
-                        print("\t[APP PERMISSION] " + perm.getName());
-                    }
-                    break;
-                case 1:
-                    for (Permission perm: assortedMap.get(1)) {
-                        print("\t[NB-ADMIN SERVICE] " + perm.getName() + "(" + perm.getActions() + ")");
-                    }
-                    break;
-                case 2:
-                    for (Permission perm: assortedMap.get(2)) {
-                        print("\t[NB SERVICE] " + perm.getName() + "(" + perm.getActions() + ")");
-                    }
-                    break;
-                case 3:
-                    for (Permission perm: assortedMap.get(3)) {
-                        print("\t[Other SERVICE] " + perm.getName() + "(" + perm.getActions() + ")");
-                    }
-                    break;
-                case 4:
+
+        for (Permission perm: assortedMap.get(0)) { // APP PERM
+            if (perm.getName().contains("WRITE")) {
+                printYellow("\t[APP PERMISSION] " + perm.getName());
+            } else {
+                printGreen("\t[APP PERMISSION] " + perm.getName());
+            }
+        }
+
         for (Permission perm: assortedMap.get(4)) {
-                        print("\t[Other] " + perm.getClass().getSimpleName() +
+            printGreen("\t[CLI SERVICE] " + perm.getName() + "(" + perm.getActions() + ")");
+        }
+
+        for (Permission perm: assortedMap.get(5)) {
+            printYellow("\t[Other SERVICE] " + perm.getName() + "(" + perm.getActions() + ")");
+        }
+
+        for (Permission perm: assortedMap.get(7)) {
+            printYellow("\t[Other] " + perm.getClass().getSimpleName() +
                                 " " + perm.getName() + " (" + perm.getActions() + ")");
         }
-                    break;
-                default:
-                    break;
+
+        for (Permission perm: assortedMap.get(1)) { // ADMIN SERVICES
+            printRed("\t[NB-ADMIN SERVICE] " + perm.getName() + "(" + perm.getActions() + ")");
+        }
+
+        for (Permission perm: assortedMap.get(3)) { // ADMIN SERVICES
+            printRed("\t[SB SERVICE] " + perm.getName() + "(" + perm.getActions() + ")");
+        }
+
+        for (Permission perm: assortedMap.get(6)) { // CRITICAL SERVICES
+            printRed("\t[CRITICAL PERMISSION] " + perm.getClass().getSimpleName() +
+                             " " + perm.getName() + " (" + perm.getActions() + ")");
         }
     }
+
+    private void printRed(String format, Object... args) {
+        print(ANSI_RED + String.format(format, args) + ANSI_RESET);
+    }
+
+    private void printYellow(String format, Object... args) {
+        print(ANSI_YELLOW + String.format(format, args) + ANSI_RESET);
+    }
+
+    private void printGreen(String format, Object... args) {
+        print(ANSI_GREEN + String.format(format, args) + ANSI_RESET);
     }
 }

@@ -82,7 +82,8 @@ public class AppPermission extends BasicPermission {
         TUNNEL_WRITE,
         TUNNEL_EVENT,
         UI_READ,
-        UI_WRITE
+        UI_WRITE,
+        ADMIN
     }
 
     protected Type type;

@@ -55,6 +55,7 @@ import static org.onosproject.app.ApplicationEvent.Type.APP_DEACTIVATED;
 import static org.onosproject.app.ApplicationEvent.Type.APP_INSTALLED;
 import static org.onosproject.app.ApplicationEvent.Type.APP_UNINSTALLED;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.onosproject.security.AppPermission.Type.APP_READ;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -151,6 +152,7 @@ public class ApplicationManager
 
     @Override
     public Application install(InputStream appDescStream) {
+        checkPermission(ADMIN);
         checkNotNull(appDescStream, "Application archive stream cannot be null");
         Application app = store.create(appDescStream);
         SecurityUtil.register(app.id());
@@ -159,12 +161,14 @@ public class ApplicationManager
 
     @Override
     public void uninstall(ApplicationId appId) {
+        checkPermission(ADMIN);
         checkNotNull(appId, APP_ID_NULL);
         updateStoreAndWaitForNotificationHandling(appId, store::remove);
     }
 
     @Override
     public void activate(ApplicationId appId) {
+        checkPermission(ADMIN);
         checkNotNull(appId, APP_ID_NULL);
         if (!SecurityUtil.isAppSecured(appId)) {
             return;
@@ -174,12 +178,14 @@ public class ApplicationManager
 
     @Override
     public void deactivate(ApplicationId appId) {
+        checkPermission(ADMIN);
         checkNotNull(appId, APP_ID_NULL);
         updateStoreAndWaitForNotificationHandling(appId, store::deactivate);
     }
 
     @Override
     public void setPermissions(ApplicationId appId, Set<Permission> permissions) {
+        checkPermission(ADMIN);
         checkNotNull(appId, APP_ID_NULL);
         checkNotNull(permissions, "Permissions cannot be null");
         store.setPermissions(appId, permissions);

@@ -58,6 +58,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.onosproject.security.AppPermission.Type.CLUSTER_READ;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -135,6 +136,7 @@ public class ClusterManager
 
     @Override
     public void markFullyStarted(boolean started) {
+        checkPermission(ADMIN);
         store.markFullyStarted(started);
     }
 
@@ -146,6 +148,7 @@ public class ClusterManager
 
     @Override
     public void formCluster(Set<ControllerNode> nodes) {
+        checkPermission(ADMIN);
         checkNotNull(nodes, "Nodes cannot be null");
         checkArgument(!nodes.isEmpty(), "Nodes cannot be empty");
 
@@ -163,6 +166,7 @@ public class ClusterManager
 
     @Override
     public ControllerNode addNode(NodeId nodeId, IpAddress ip, int tcpPort) {
+        checkPermission(ADMIN);
         checkNotNull(nodeId, INSTANCE_ID_NULL);
         checkNotNull(ip, "IP address cannot be null");
         checkArgument(tcpPort > 5000, "TCP port must be > 5000");
@@ -171,6 +175,7 @@ public class ClusterManager
 
     @Override
     public void removeNode(NodeId nodeId) {
+        checkPermission(ADMIN);
         checkNotNull(nodeId, INSTANCE_ID_NULL);
         store.removeNode(nodeId);
     }

@@ -46,6 +46,7 @@ import java.util.Enumeration;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.onosproject.security.AppPermission.Type.CLUSTER_READ;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -103,6 +104,7 @@ public class ClusterMetadataManager
 
     @Override
     public void setClusterMetadata(ClusterMetadata metadata) {
+        checkPermission(ADMIN);
         checkNotNull(metadata, "Cluster metadata cannot be null");
         ClusterMetadataProvider primaryProvider = getPrimaryProvider();
         if (primaryProvider == null) {

@@ -15,6 +15,8 @@
  */
 package org.onosproject.cluster.impl;
 
+import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.slf4j.LoggerFactory.getLogger;
 
 import java.util.Map;
@@ -100,16 +102,19 @@ public class LeadershipManager
 
     @Override
     public boolean transferLeadership(String topic, NodeId to) {
+        checkPermission(ADMIN);
         return store.moveLeadership(topic, to);
     }
 
     @Override
     public void unregister(NodeId nodeId) {
+        checkPermission(ADMIN);
         store.removeRegistration(nodeId);
     }
 
     @Override
     public boolean promoteToTopOfCandidateList(String topic, NodeId nodeId) {
+        checkPermission(ADMIN);
         return store.makeTopCandidate(topic, nodeId);
     }
 }

@@ -64,6 +64,7 @@ import static org.onlab.metrics.MetricsUtil.startTimer;
 import static org.onlab.metrics.MetricsUtil.stopTimer;
 import static org.onosproject.net.MastershipRole.MASTER;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.onosproject.security.AppPermission.Type.CLUSTER_READ;
 import static org.onosproject.security.AppPermission.Type.CLUSTER_WRITE;
 import static org.slf4j.LoggerFactory.getLogger;
@@ -119,6 +120,7 @@ public class MastershipManager
 
     @Override
     public CompletableFuture<Void> setRole(NodeId nodeId, DeviceId deviceId, MastershipRole role) {
+        checkPermission(ADMIN);
         checkNotNull(nodeId, NODE_ID_NULL);
         checkNotNull(deviceId, DEVICE_ID_NULL);
         checkNotNull(role, ROLE_NULL);
@@ -207,6 +209,7 @@ public class MastershipManager
 
     @Override
     public void balanceRoles() {
+        checkPermission(ADMIN);
         List<ControllerNode> nodes = newArrayList(clusterService.getNodes());
         Map<ControllerNode, Set<DeviceId>> controllerDevices = new HashMap<>();
         int deviceCount = 0;

@@ -25,6 +25,7 @@ import static org.onosproject.net.MastershipRole.STANDBY;
 import static org.onosproject.net.optical.device.OchPortHelper.ochPortDescription;
 import static org.onosproject.net.optical.device.OduCltPortHelper.oduCltPortDescription;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.onosproject.security.AppPermission.Type.DEVICE_READ;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -247,6 +248,7 @@ public class DeviceManager
 
     @Override
     public void removeDevice(DeviceId deviceId) {
+        checkPermission(ADMIN);
         checkNotNull(deviceId, DEVICE_ID_NULL);
         DeviceEvent event = store.removeDevice(deviceId);
         if (event != null) {
@@ -258,6 +260,7 @@ public class DeviceManager
     @Override
     public void changePortState(DeviceId deviceId, PortNumber portNumber,
                                 boolean enable) {
+        checkPermission(ADMIN);
         checkNotNull(deviceId, DEVICE_ID_NULL);
         checkNotNull(deviceId, PORT_NUMBER_NULL);
         DeviceProvider provider = getProvider(deviceId);

@@ -85,11 +85,13 @@ public class DriverManager extends DefaultDriverProvider implements DriverAdminS
 
     @Override
     public Set<DriverProvider> getProviders() {
+        checkPermission(ADMIN);
         return ImmutableSet.copyOf(providers);
     }
 
     @Override
     public void registerProvider(DriverProvider provider) {
+        checkPermission(ADMIN);
         provider.getDrivers().forEach(driver -> {
             Driver d = addDriver(driver);
             driverByKey.put(key(driver.manufacturer(),
@@ -101,6 +103,7 @@ public class DriverManager extends DefaultDriverProvider implements DriverAdminS
 
     @Override
     public void unregisterProvider(DriverProvider provider) {
+        checkPermission(ADMIN);
         provider.getDrivers().forEach(driver -> {
             removeDriver(driver);
             driverByKey.remove(key(driver.manufacturer(),

@@ -191,6 +191,7 @@ public class HostManager
 
     @Override
     public void removeHost(HostId hostId) {
+        checkPermission(ADMIN);
         checkNotNull(hostId, HOST_ID_NULL);
         store.removeHost(hostId);
     }

@@ -37,6 +37,7 @@ import java.util.Collection;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.onosproject.security.AppPermission.Type.DEVICE_KEY_READ;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -71,12 +72,14 @@ public class DeviceKeyManager extends AbstractListenerManager<DeviceKeyEvent, De
 
     @Override
     public void addKey(DeviceKey deviceKey) {
+        checkPermission(ADMIN);
         checkNotNull(deviceKey, "Device key cannot be null");
         store.createOrUpdateDeviceKey(deviceKey);
     }
 
     @Override
     public void removeKey(DeviceKeyId deviceKeyId) {
+        checkPermission(ADMIN);
         checkNotNull(deviceKeyId, "Device key identifier cannot be null");
         store.deleteDeviceKey(deviceKeyId);
     }

@@ -182,6 +182,7 @@ public class LinkManager
 
     @Override
     public void removeLinks(ConnectPoint connectPoint) {
+        checkPermission(ADMIN);
         if (deviceService.getRole(connectPoint.deviceId()) != MastershipRole.MASTER) {
             return;
         }
@@ -190,6 +191,7 @@ public class LinkManager
 
     @Override
     public void removeLinks(DeviceId deviceId) {
+        checkPermission(ADMIN);
         if (deviceService.getRole(deviceId) != MastershipRole.MASTER) {
             return;
         }
@@ -198,6 +200,7 @@ public class LinkManager
 
     @Override
     public void removeLink(ConnectPoint src, ConnectPoint dst) {
+        checkPermission(ADMIN);
         post(store.removeLink(src, dst));
     }
 

@@ -42,6 +42,7 @@ import java.util.Set;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableList.of;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.slf4j.LoggerFactory.getLogger;
 import static org.onosproject.security.AppGuard.checkPermission;
 import static org.onosproject.security.AppPermission.Type.REGION_READ;
@@ -85,6 +86,7 @@ public class RegionManager extends AbstractListenerManager<RegionEvent, RegionLi
     @Override
     public Region createRegion(RegionId regionId, String name, Region.Type type,
                                List<Set<NodeId>> masterNodeIds) {
+        checkPermission(ADMIN);
         checkNotNull(regionId, REGION_ID_NULL);
         checkNotNull(name, NAME_NULL);
         checkNotNull(name, REGION_TYPE_NULL);
@@ -94,6 +96,7 @@ public class RegionManager extends AbstractListenerManager<RegionEvent, RegionLi
     @Override
     public Region updateRegion(RegionId regionId, String name, Region.Type type,
                                List<Set<NodeId>> masterNodeIds) {
+        checkPermission(ADMIN);
         checkNotNull(regionId, REGION_ID_NULL);
         checkNotNull(name, NAME_NULL);
         checkNotNull(name, REGION_TYPE_NULL);
@@ -102,12 +105,14 @@ public class RegionManager extends AbstractListenerManager<RegionEvent, RegionLi
 
     @Override
     public void removeRegion(RegionId regionId) {
+        checkPermission(ADMIN);
         checkNotNull(regionId, REGION_ID_NULL);
         store.removeRegion(regionId);
     }
 
     @Override
     public void addDevices(RegionId regionId, Collection<DeviceId> deviceIds) {
+        checkPermission(ADMIN);
         checkNotNull(regionId, REGION_ID_NULL);
         checkNotNull(deviceIds, DEVICE_IDS_NULL);
         checkState(!deviceIds.isEmpty(), DEVICE_IDS_EMPTY);
@@ -116,6 +121,7 @@ public class RegionManager extends AbstractListenerManager<RegionEvent, RegionLi
 
     @Override
     public void removeDevices(RegionId regionId, Collection<DeviceId> deviceIds) {
+        checkPermission(ADMIN);
         checkNotNull(regionId, REGION_ID_NULL);
         checkNotNull(deviceIds, DEVICE_IDS_NULL);
         checkState(!deviceIds.isEmpty(), DEVICE_IDS_EMPTY);

@@ -46,6 +46,7 @@ import java.util.stream.Collectors;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.onosproject.security.AppPermission.Type.RESOURCE_WRITE;
 import static org.onosproject.security.AppPermission.Type.RESOURCE_READ;
 import static org.slf4j.LoggerFactory.getLogger;
@@ -109,6 +110,7 @@ public final class ResourceManager extends AbstractListenerManager<ResourceEvent
 
     @Override
     public boolean release(ResourceConsumer consumer) {
+        checkPermission(RESOURCE_WRITE);
         checkNotNull(consumer);
 
         Collection<ResourceAllocation> allocations = getResourceAllocations(consumer);
@@ -201,6 +203,7 @@ public final class ResourceManager extends AbstractListenerManager<ResourceEvent
 
     @Override
     public boolean register(List<Resource> resources) {
+        checkPermission(ADMIN);
         checkNotNull(resources);
 
         return store.register(resources);
@@ -208,6 +211,7 @@ public final class ResourceManager extends AbstractListenerManager<ResourceEvent
 
     @Override
     public boolean unregister(List<ResourceId> ids) {
+        checkPermission(ADMIN);
         checkNotNull(ids);
 
         return store.unregister(ids);

@@ -38,15 +38,18 @@ import org.onosproject.security.store.SecurityModeListener;
 import org.onosproject.security.store.SecurityModeStore;
 import org.onosproject.security.store.SecurityModeStoreDelegate;
 import org.osgi.framework.BundleContext;
+import org.osgi.framework.FrameworkEvent;
 import org.osgi.framework.FrameworkUtil;
 import org.osgi.framework.ServicePermission;
-import org.osgi.service.log.LogEntry;
-import org.osgi.service.log.LogListener;
-import org.osgi.service.log.LogReaderService;
+import org.osgi.framework.FrameworkListener;
 import org.osgi.service.permissionadmin.PermissionInfo;
 
+import java.io.FilePermission;
+import java.lang.reflect.ReflectPermission;
+import java.net.SocketPermission;
 import java.security.AccessControlException;
 import java.security.Permission;
+import java.security.SecurityPermission;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
@@ -76,9 +79,6 @@ public class SecurityModeManager implements SecurityAdminService {
     protected ApplicationAdminService appAdminService;
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected LogReaderService logReaderService;
-
-    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected EventDeliveryService eventDispatcher;
 
     private final Logger log = getLogger(getClass());
@@ -88,7 +88,7 @@ public class SecurityModeManager implements SecurityAdminService {
 
     private final SecurityModeStoreDelegate delegate = new InternalStoreDelegate();
 
-    private SecurityLogListener securityLogListener = new SecurityLogListener();
+    private SecurityEventListener securityEventListener = new SecurityEventListener();
 
     private PermissionAdmin permissionAdmin = getPermissionAdmin();
 
@@ -96,7 +96,7 @@ public class SecurityModeManager implements SecurityAdminService {
     public void activate() {
 
         eventDispatcher.addSink(SecurityModeEvent.class, listenerRegistry);
-        logReaderService.addLogListener(securityLogListener);
+        getBundleContext().addFrameworkListener(new SecurityEventListener());
 
         if (System.getSecurityManager() == null) {
             log.warn("J2EE security manager is disabled.");
@@ -116,7 +116,7 @@ public class SecurityModeManager implements SecurityAdminService {
     @Deactivate
     public void deactivate() {
         eventDispatcher.removeSink(SecurityModeEvent.class);
-        logReaderService.removeLogListener(securityLogListener);
+        getBundleContext().removeFrameworkListener(securityEventListener);
         store.unsetDelegate(delegate);
         log.info("Stopped");
 
@@ -169,29 +169,34 @@ public class SecurityModeManager implements SecurityAdminService {
                 DefaultPolicyBuilder.convertToJavaPermissions(store.getRequestedPermissions(appId)));
     }
 
-    private class SecurityLogListener implements LogListener {
+    private class SecurityEventListener implements FrameworkListener {
         @Override
-        public void logged(LogEntry entry) {
-            if (entry.getException() != null &&
-                    entry.getException() instanceof AccessControlException) {
-                String location = entry.getBundle().getLocation();
-                Permission javaPerm =
-                        ((AccessControlException) entry.getException()).getPermission();
-                org.onosproject.security.Permission permission = DefaultPolicyBuilder.getOnosPermission(javaPerm);
-                if (permission == null) {
-                    log.warn("Unsupported permission requested.");
+        public void frameworkEvent(FrameworkEvent event) {
+            if (event.getType() != FrameworkEvent.ERROR) {
+                return;
+            }
+            Throwable throwable = event.getThrowable();
+            if (throwable == null || !(throwable instanceof AccessControlException)) {
                 return;
             }
-                store.getApplicationIds(location).stream().filter(
+            String bundleLocation = event.getBundle().getLocation();
+            Permission nativePerm = ((AccessControlException) throwable).getPermission();
+            org.onosproject.security.Permission onosPerm = DefaultPolicyBuilder.getOnosPermission(nativePerm);
+
+            if (onosPerm == null) {
+                log.warn("Unsupported permission requested: " + nativePerm.toString());
+                return;
+            }
+
+            store.getApplicationIds(bundleLocation).stream().filter(
                     appId -> store.isSecured(appId) &&
                             appAdminService.getState(appId) == ApplicationState.ACTIVE).forEach(appId -> {
-                    store.requestPermission(appId, permission);
+                store.requestPermission(appId, onosPerm);
                 print("[POLICY VIOLATION] APP: %s / Bundle: %s / Permission: %s ",
-                            appId.name(), location, permission.toString());
+                      appId.name(), bundleLocation, onosPerm.toString());
             });
         }
     }
-    }
 
     private class InternalStoreDelegate implements SecurityModeStoreDelegate {
         @Override
@@ -213,32 +218,59 @@ public class SecurityModeManager implements SecurityAdminService {
      * 0 - APP_PERM
      * 1 - ADMIN SERVICE
      * 2 - NB_SERVICE
-     * 3 - ETC_SERVICE
-     * 4 - ETC
+     * 3 - SB_SERVICE
+     * 4 - CLI_SERVICE
+     * 5 - ETC_SERVICE
+     * 6 - CRITICAL PERMISSIONS
+     * 7 - ETC
      * @param perms
      */
-    private Map<Integer, List<Permission>> getPrintablePermissionMap(List<Permission> perms) {
+    private Map<Integer, List<Permission>> getPrintablePermissionMap(Set<Permission> perms) {
         ConcurrentHashMap<Integer, List<Permission>> sortedMap = new ConcurrentHashMap<>();
         sortedMap.put(0, new ArrayList());
         sortedMap.put(1, new ArrayList());
         sortedMap.put(2, new ArrayList());
         sortedMap.put(3, new ArrayList());
         sortedMap.put(4, new ArrayList());
+        sortedMap.put(5, new ArrayList());
+        sortedMap.put(6, new ArrayList());
+        sortedMap.put(7, new ArrayList());
+
         for (Permission perm : perms) {
-            if (perm instanceof ServicePermission) {
-                if (DefaultPolicyBuilder.getNBServiceList().contains(perm.getName())) {
-                    if (perm.getName().contains("Admin")) {
+            if (perm instanceof AppPermission) {
+                sortedMap.get(0).add(perm);
+            } else if (perm instanceof ServicePermission) {
+                String permName = perm.getName().trim();
+                if (DefaultPolicyBuilder.getNBServiceList().contains(permName)) { // ONOS NB SERVICES
+                    if (permName.contains("Admin")) {
                         sortedMap.get(1).add(perm);
                     } else {
                         sortedMap.get(2).add(perm);
                     }
-                } else {
+                } else if (permName.contains("org.onosproject") && permName.contains("Provider")) { //ONOS SB SERVICES
                     sortedMap.get(3).add(perm);
+                } else if (DefaultPolicyBuilder.getCliServiceList().contains(permName)) { //CLI SERVICES
+                    sortedMap.get(4).add(perm);
+                } else if (permName.contains("Security")) { //CRITICAL SERVICES
+                    sortedMap.get(6).add(perm);
+                } else {
+                    sortedMap.get(5).add(perm);
                 }
-            } else if (perm instanceof AppPermission) {
-                sortedMap.get(0).add(perm);
+            } else if (perm instanceof RuntimePermission || perm instanceof SocketPermission ||
+                    perm instanceof FilePermission || perm instanceof SecurityPermission ||
+                    perm instanceof ReflectPermission) { // CRITICAL PERMISSIONS
+                sortedMap.get(6).add(perm);
             } else {
-                sortedMap.get(4).add(perm);
+                boolean isDefault = false;
+                for (Permission dPerm : DefaultPolicyBuilder.getDefaultPerms()) {
+                    if (perm.implies(dPerm)) {
+                        isDefault = true;
+                        break;
+                    }
+                }
+                if (!isDefault) {
+                    sortedMap.get(7).add(perm);
+                }
             }
         }
         return sortedMap;
@@ -261,13 +293,13 @@ public class SecurityModeManager implements SecurityAdminService {
 
 
 
-    private List<Permission> getMaximumPermissions(ApplicationId appId) {
+    private Set<Permission> getMaximumPermissions(ApplicationId appId) {
         Application app = appAdminService.getApplication(appId);
         if (app == null) {
             print("Unknown application.");
             return null;
         }
-        List<Permission> appPerms;
+        Set<Permission> appPerms;
         switch (app.role()) {
             case ADMIN:
                 appPerms = DefaultPolicyBuilder.getAdminApplicationPermissions(app.permissions());
@@ -300,5 +332,4 @@ public class SecurityModeManager implements SecurityAdminService {
 
     }
 
-
 }
\ No newline at end of file

@@ -93,12 +93,10 @@ public class DistributedSecurityModeStore
             .register(KryoNamespaces.API)
             .register(SecurityModeState.class)
             .register(SecurityInfo.class)
-            .register(Permission.class)
             .build());
 
     private static final KryoNamespace.Builder VIOLATION_SERIALIZER = KryoNamespace.newBuilder()
-            .register(KryoNamespaces.API)
-            .register(Permission.class);
+            .register(KryoNamespaces.API);
 
     @Activate
     public void activate() {

@@ -74,6 +74,8 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static org.onlab.util.Tools.get;
 import static org.onlab.util.Tools.isNullOrEmpty;
 import static org.onosproject.net.topology.TopologyEvent.Type.TOPOLOGY_CHANGED;
+import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
@@ -316,11 +318,13 @@ public class DistributedTopologyStore
 
     @Override
     public void setDefaultLinkWeight(LinkWeight linkWeight) {
+        checkPermission(ADMIN);
         DefaultTopology.setDefaultLinkWeight(linkWeight);
     }
 
     @Override
     public void setDefaultGraphPathSearch(GraphPathSearch<TopologyVertex, TopologyEdge> graphPathSearch) {
+        checkPermission(ADMIN);
         DefaultTopology.setDefaultGraphPathSearch(graphPathSearch);
     }
 

@@ -16,6 +16,7 @@
 
 package org.onosproject.store.primitives.impl;
 
+import static org.onosproject.security.AppPermission.Type.ADMIN;
 import static org.slf4j.LoggerFactory.getLogger;
 
 import java.io.File;
@@ -152,6 +153,7 @@ public class PartitionManager extends AbstractListenerManager<PartitionEvent, Pa
 
     @Override
     public List<PartitionInfo> partitionInfo() {
+        checkPermission(ADMIN);
         return partitions.values()
                          .stream()
                          .flatMap(x -> Tools.stream(x.info()))
@@ -177,6 +179,7 @@ public class PartitionManager extends AbstractListenerManager<PartitionEvent, Pa
 
     @Override
     public List<PartitionClientInfo> partitionClientInfo() {
+        checkPermission(ADMIN);
         return partitions.values()
                          .stream()
                          .map(StoragePartition::client)

@@ -172,11 +172,13 @@ public class StorageManager implements StorageService, StorageAdminService {
 
     @Override
     public List<MapInfo> getMapInfo() {
+        checkPermission(ADMIN);
         return listMapInfo(federatedPrimitiveCreator);
     }
 
     @Override
     public Map<String, Long> getCounters() {
+        checkPermission(ADMIN);
         Map<String, Long> counters = Maps.newConcurrentMap();
         federatedPrimitiveCreator.getAsyncAtomicCounterNames()
                .forEach(name -> counters.put(name,
@@ -186,11 +188,13 @@ public class StorageManager implements StorageService, StorageAdminService {
 
     @Override
     public List<PartitionInfo> getPartitionInfo() {
+        checkPermission(ADMIN);
         return partitionAdminService.partitionInfo();
     }
 
     @Override
     public Collection<TransactionId> getPendingTransactions() {
+        checkPermission(ADMIN);
         return Futures.getUnchecked(transactions.keySet());
     }