Committed by
Brian O'Connor
ONOS-3521: SM-ONOS performance improvement
Change-Id: I8643187f2ceb35f8e0701d9e7ddb10098f05b244
Showing
7 changed files
with
105 additions
and
25 deletions
| ... | @@ -16,26 +16,102 @@ | ... | @@ -16,26 +16,102 @@ |
| 16 | 16 | ||
| 17 | package org.onosproject.security; | 17 | package org.onosproject.security; |
| 18 | 18 | ||
| 19 | +import java.security.AccessController; | ||
| 20 | +import java.security.AccessControlContext; | ||
| 21 | +import java.security.PrivilegedAction; | ||
| 22 | +import java.security.ProtectionDomain; | ||
| 19 | 23 | ||
| 20 | import com.google.common.annotations.Beta; | 24 | import com.google.common.annotations.Beta; |
| 25 | +import com.google.common.cache.Cache; | ||
| 26 | +import com.google.common.cache.CacheBuilder; | ||
| 27 | + | ||
| 28 | +import java.lang.reflect.Field; | ||
| 29 | +import java.util.concurrent.ExecutionException; | ||
| 30 | +import java.util.concurrent.TimeUnit; | ||
| 21 | 31 | ||
| 22 | /** | 32 | /** |
| 23 | * Aids SM-ONOS to perform API-level permission checking. | 33 | * Aids SM-ONOS to perform API-level permission checking. |
| 24 | */ | 34 | */ |
| 25 | @Beta | 35 | @Beta |
| 26 | public final class AppGuard { | 36 | public final class AppGuard { |
| 27 | - | ||
| 28 | private AppGuard() { | 37 | private AppGuard() { |
| 29 | } | 38 | } |
| 30 | 39 | ||
| 31 | /** | 40 | /** |
| 32 | * Checks if the caller has the required permission only when security-mode is enabled. | 41 | * Checks if the caller has the required permission only when security-mode is enabled. |
| 42 | + * | ||
| 33 | * @param permission permission to be checked | 43 | * @param permission permission to be checked |
| 34 | */ | 44 | */ |
| 35 | public static void checkPermission(AppPermission.Type permission) { | 45 | public static void checkPermission(AppPermission.Type permission) { |
| 46 | + | ||
| 36 | SecurityManager sm = System.getSecurityManager(); | 47 | SecurityManager sm = System.getSecurityManager(); |
| 37 | - if (sm != null) { | 48 | + if (sm == null) { |
| 38 | - System.getSecurityManager().checkPermission(new AppPermission(permission)); | 49 | + return; |
| 50 | + } | ||
| 51 | + | ||
| 52 | + Object result = AccessController.doPrivileged((PrivilegedAction<Object>) () -> { | ||
| 53 | + int contextHash = 0; | ||
| 54 | + AccessControlContext context = AccessController.getContext(); | ||
| 55 | + Field f = null; | ||
| 56 | + try { | ||
| 57 | + f = context.getClass().getDeclaredField("context"); | ||
| 58 | + | ||
| 59 | + f.setAccessible(true); | ||
| 60 | + ProtectionDomain[] domain = (ProtectionDomain[]) f.get(context); | ||
| 61 | + for (ProtectionDomain pd : domain) { | ||
| 62 | + if (pd.getCodeSource() != null) { | ||
| 63 | + contextHash = contextHash ^ pd.getCodeSource().getLocation().hashCode(); | ||
| 64 | + } else { | ||
| 65 | + return null; | ||
| 66 | + } | ||
| 67 | + } | ||
| 68 | + return contextHash; | ||
| 69 | + } catch (NoSuchFieldException e) { | ||
| 70 | + return null; | ||
| 71 | + } catch (IllegalAccessException e) { | ||
| 72 | + return null; | ||
| 73 | + } | ||
| 74 | + }); | ||
| 75 | + | ||
| 76 | + if (result == null) { | ||
| 77 | + sm.checkPermission(new AppPermission(permission)); | ||
| 78 | + } else { | ||
| 79 | + AppPermission perm = new AppPermission(permission); | ||
| 80 | + int hash = ((int) result) ^ perm.hashCode(); | ||
| 81 | + PermissionCheckCache.getInstance().checkCache(hash, perm); | ||
| 39 | } | 82 | } |
| 40 | } | 83 | } |
| 84 | + | ||
| 85 | + | ||
| 86 | + private static final class PermissionCheckCache { | ||
| 87 | + | ||
| 88 | + private static final Cache<Integer, Boolean> CACHE = CacheBuilder.newBuilder() | ||
| 89 | + .maximumSize(1000) | ||
| 90 | + .expireAfterAccess(10, TimeUnit.MINUTES) | ||
| 91 | + .build(); | ||
| 92 | + | ||
| 93 | + private PermissionCheckCache() { | ||
| 94 | + } | ||
| 95 | + | ||
| 96 | + private static class SingletonHelper { | ||
| 97 | + private static final PermissionCheckCache INSTANCE = new PermissionCheckCache(); | ||
| 98 | + } | ||
| 99 | + | ||
| 100 | + public static PermissionCheckCache getInstance() { | ||
| 101 | + return SingletonHelper.INSTANCE; | ||
| 102 | + } | ||
| 103 | + | ||
| 104 | + public static void checkCache(int key, AppPermission perm) { | ||
| 105 | + try { | ||
| 106 | + CACHE.get(key, () -> { | ||
| 107 | + System.getSecurityManager().checkPermission(perm); | ||
| 108 | + return true; | ||
| 109 | + }); | ||
| 110 | + } catch (ExecutionException e) { | ||
| 111 | + System.getSecurityManager().checkPermission(perm); | ||
| 112 | + } | ||
| 113 | + } | ||
| 114 | + } | ||
| 115 | + | ||
| 41 | } | 116 | } |
| 117 | + | ... | ... |
| ... | @@ -74,6 +74,4 @@ public interface SecurityAdminService { | ... | @@ -74,6 +74,4 @@ public interface SecurityAdminService { |
| 74 | * @return Map of list of permissions sorted by permission type | 74 | * @return Map of list of permissions sorted by permission type |
| 75 | */ | 75 | */ |
| 76 | Map<Integer, List<Permission>> getPrintableRequestedPermissions(ApplicationId appId); | 76 | Map<Integer, List<Permission>> getPrintableRequestedPermissions(ApplicationId appId); |
| 77 | - | ||
| 78 | - | ||
| 79 | } | 77 | } | ... | ... |
| ... | @@ -54,11 +54,12 @@ import org.onosproject.net.topology.TopologyService; | ... | @@ -54,11 +54,12 @@ import org.onosproject.net.topology.TopologyService; |
| 54 | import org.onosproject.security.SecurityAdminService; | 54 | import org.onosproject.security.SecurityAdminService; |
| 55 | import org.onosproject.store.service.StorageAdminService; | 55 | import org.onosproject.store.service.StorageAdminService; |
| 56 | import org.onosproject.store.service.StorageService; | 56 | import org.onosproject.store.service.StorageService; |
| 57 | -import org.osgi.framework.BundlePermission; | ||
| 58 | -import org.osgi.framework.CapabilityPermission; | ||
| 59 | import org.osgi.framework.ServicePermission; | 57 | import org.osgi.framework.ServicePermission; |
| 60 | -import org.osgi.framework.PackagePermission; | 58 | +import org.osgi.framework.AdminPermission; |
| 61 | import org.osgi.framework.AdaptPermission; | 59 | import org.osgi.framework.AdaptPermission; |
| 60 | +import org.osgi.framework.CapabilityPermission; | ||
| 61 | +import org.osgi.framework.BundlePermission; | ||
| 62 | +import org.osgi.framework.PackagePermission; | ||
| 62 | import org.osgi.service.cm.ConfigurationPermission; | 63 | import org.osgi.service.cm.ConfigurationPermission; |
| 63 | 64 | ||
| 64 | import javax.net.ssl.SSLPermission; | 65 | import javax.net.ssl.SSLPermission; |
| ... | @@ -68,6 +69,7 @@ import javax.security.auth.kerberos.DelegationPermission; | ... | @@ -68,6 +69,7 @@ import javax.security.auth.kerberos.DelegationPermission; |
| 68 | import javax.sound.sampled.AudioPermission; | 69 | import javax.sound.sampled.AudioPermission; |
| 69 | import java.io.FilePermission; | 70 | import java.io.FilePermission; |
| 70 | import java.io.SerializablePermission; | 71 | import java.io.SerializablePermission; |
| 72 | +import java.lang.reflect.ReflectPermission; | ||
| 71 | import java.net.NetPermission; | 73 | import java.net.NetPermission; |
| 72 | import java.net.SocketPermission; | 74 | import java.net.SocketPermission; |
| 73 | import java.security.Permissions; | 75 | import java.security.Permissions; |
| ... | @@ -159,6 +161,7 @@ public final class DefaultPolicyBuilder { | ... | @@ -159,6 +161,7 @@ public final class DefaultPolicyBuilder { |
| 159 | permSet.add(new PackagePermission("*", PackagePermission.IMPORT)); | 161 | permSet.add(new PackagePermission("*", PackagePermission.IMPORT)); |
| 160 | permSet.add(new AdaptPermission("*", AdaptPermission.ADAPT)); | 162 | permSet.add(new AdaptPermission("*", AdaptPermission.ADAPT)); |
| 161 | permSet.add(new ConfigurationPermission("*", ConfigurationPermission.CONFIGURE)); | 163 | permSet.add(new ConfigurationPermission("*", ConfigurationPermission.CONFIGURE)); |
| 164 | + permSet.add(new AdminPermission("*", AdminPermission.METADATA)); | ||
| 162 | return permSet; | 165 | return permSet; |
| 163 | } | 166 | } |
| 164 | 167 | ||
| ... | @@ -359,6 +362,12 @@ public final class DefaultPolicyBuilder { | ... | @@ -359,6 +362,12 @@ public final class DefaultPolicyBuilder { |
| 359 | } else if (permission instanceof ServicePermission) { | 362 | } else if (permission instanceof ServicePermission) { |
| 360 | return new org.onosproject.security.Permission( | 363 | return new org.onosproject.security.Permission( |
| 361 | ServicePermission.class.getName(), permission.getName(), permission.getActions()); | 364 | ServicePermission.class.getName(), permission.getName(), permission.getActions()); |
| 365 | + } else if (permission instanceof AdminPermission) { | ||
| 366 | + return new org.onosproject.security.Permission( | ||
| 367 | + AdminPermission.class.getName(), permission.getName(), permission.getActions()); | ||
| 368 | + } else if (permission instanceof ConfigurationPermission) { | ||
| 369 | + return new org.onosproject.security.Permission( | ||
| 370 | + ConfigurationPermission.class.getName(), permission.getName(), permission.getActions()); | ||
| 362 | } | 371 | } |
| 363 | return null; | 372 | return null; |
| 364 | } | 373 | } |
| ... | @@ -416,10 +425,16 @@ public final class DefaultPolicyBuilder { | ... | @@ -416,10 +425,16 @@ public final class DefaultPolicyBuilder { |
| 416 | return new PackagePermission(name, actions); | 425 | return new PackagePermission(name, actions); |
| 417 | } else if (ServicePermission.class.getName().equals(classname)) { | 426 | } else if (ServicePermission.class.getName().equals(classname)) { |
| 418 | return new ServicePermission(name, actions); | 427 | return new ServicePermission(name, actions); |
| 428 | + } else if (AdminPermission.class.getName().equals(classname)) { | ||
| 429 | + return new AdminPermission(name, actions); | ||
| 430 | + } else if (ConfigurationPermission.class.getName().equals(classname)) { | ||
| 431 | + return new ConfigurationPermission(name, actions); | ||
| 432 | + } else if (ReflectPermission.class.getName().equals(classname)) { | ||
| 433 | + return new ReflectPermission(name, actions); | ||
| 419 | } | 434 | } |
| 420 | 435 | ||
| 421 | //AllPermission, SecurityPermission, UnresolvedPermission | 436 | //AllPermission, SecurityPermission, UnresolvedPermission |
| 422 | - //AWTPermission, AdminPermission(osgi), ReflectPermission not allowed | 437 | + //AWTPermission, ReflectPermission not allowed |
| 423 | return null; | 438 | return null; |
| 424 | 439 | ||
| 425 | } | 440 | } |
| ... | @@ -444,5 +459,4 @@ public final class DefaultPolicyBuilder { | ... | @@ -444,5 +459,4 @@ public final class DefaultPolicyBuilder { |
| 444 | } | 459 | } |
| 445 | return permissions; | 460 | return permissions; |
| 446 | } | 461 | } |
| 447 | -} | 462 | +} |
| 448 | - | ||
| ... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
| ... | @@ -92,16 +92,12 @@ public class SecurityModeManager implements SecurityAdminService { | ... | @@ -92,16 +92,12 @@ public class SecurityModeManager implements SecurityAdminService { |
| 92 | 92 | ||
| 93 | private PermissionAdmin permissionAdmin = getPermissionAdmin(); | 93 | private PermissionAdmin permissionAdmin = getPermissionAdmin(); |
| 94 | 94 | ||
| 95 | - | ||
| 96 | @Activate | 95 | @Activate |
| 97 | public void activate() { | 96 | public void activate() { |
| 98 | 97 | ||
| 99 | eventDispatcher.addSink(SecurityModeEvent.class, listenerRegistry); | 98 | eventDispatcher.addSink(SecurityModeEvent.class, listenerRegistry); |
| 100 | - // add Listeners | ||
| 101 | logReaderService.addLogListener(securityLogListener); | 99 | logReaderService.addLogListener(securityLogListener); |
| 102 | 100 | ||
| 103 | - store.setDelegate(delegate); | ||
| 104 | - | ||
| 105 | if (System.getSecurityManager() == null) { | 101 | if (System.getSecurityManager() == null) { |
| 106 | log.warn("J2EE security manager is disabled."); | 102 | log.warn("J2EE security manager is disabled."); |
| 107 | deactivate(); | 103 | deactivate(); |
| ... | @@ -112,6 +108,7 @@ public class SecurityModeManager implements SecurityAdminService { | ... | @@ -112,6 +108,7 @@ public class SecurityModeManager implements SecurityAdminService { |
| 112 | deactivate(); | 108 | deactivate(); |
| 113 | return; | 109 | return; |
| 114 | } | 110 | } |
| 111 | + store.setDelegate(delegate); | ||
| 115 | 112 | ||
| 116 | log.info("Security-Mode Started"); | 113 | log.info("Security-Mode Started"); |
| 117 | } | 114 | } |
| ... | @@ -302,4 +299,6 @@ public class SecurityModeManager implements SecurityAdminService { | ... | @@ -302,4 +299,6 @@ public class SecurityModeManager implements SecurityAdminService { |
| 302 | return FrameworkUtil.getBundle(this.getClass()).getBundleContext(); | 299 | return FrameworkUtil.getBundle(this.getClass()).getBundleContext(); |
| 303 | 300 | ||
| 304 | } | 301 | } |
| 302 | + | ||
| 303 | + | ||
| 305 | } | 304 | } |
| ... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
| ... | @@ -303,14 +303,4 @@ public class DistributedSecurityModeStore | ... | @@ -303,14 +303,4 @@ public class DistributedSecurityModeStore |
| 303 | } | 303 | } |
| 304 | return locations; | 304 | return locations; |
| 305 | } | 305 | } |
| 306 | - | ||
| 307 | - @Override | ||
| 308 | - public void setDelegate(SecurityModeStoreDelegate delegate) { | ||
| 309 | - super.setDelegate(delegate); | ||
| 310 | - } | ||
| 311 | - | ||
| 312 | - @Override | ||
| 313 | - public void unsetDelegate(SecurityModeStoreDelegate delegate) { | ||
| 314 | - super.setDelegate(delegate); | ||
| 315 | - } | ||
| 316 | } | 306 | } | ... | ... |
| ... | @@ -101,4 +101,5 @@ public interface SecurityModeStore extends Store<SecurityModeEvent, SecurityMode | ... | @@ -101,4 +101,5 @@ public interface SecurityModeStore extends Store<SecurityModeEvent, SecurityMode |
| 101 | * @param permissionSet array of PermissionInfo | 101 | * @param permissionSet array of PermissionInfo |
| 102 | */ | 102 | */ |
| 103 | void acceptPolicy(ApplicationId appId, Set<Permission> permissionSet); | 103 | void acceptPolicy(ApplicationId appId, Set<Permission> permissionSet); |
| 104 | + | ||
| 104 | } | 105 | } |
| ... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
| ... | @@ -186,6 +186,7 @@ import org.onosproject.net.resource.link.LinkResourceRequest; | ... | @@ -186,6 +186,7 @@ import org.onosproject.net.resource.link.LinkResourceRequest; |
| 186 | import org.onosproject.net.resource.link.MplsLabel; | 186 | import org.onosproject.net.resource.link.MplsLabel; |
| 187 | import org.onosproject.net.resource.link.MplsLabelResourceAllocation; | 187 | import org.onosproject.net.resource.link.MplsLabelResourceAllocation; |
| 188 | import org.onosproject.net.resource.link.MplsLabelResourceRequest; | 188 | import org.onosproject.net.resource.link.MplsLabelResourceRequest; |
| 189 | +import org.onosproject.security.Permission; | ||
| 189 | import org.onosproject.store.Timestamp; | 190 | import org.onosproject.store.Timestamp; |
| 190 | import org.onosproject.store.service.MapEvent; | 191 | import org.onosproject.store.service.MapEvent; |
| 191 | import org.onosproject.store.service.SetEvent; | 192 | import org.onosproject.store.service.SetEvent; |
| ... | @@ -286,6 +287,7 @@ public final class KryoNamespaces { | ... | @@ -286,6 +287,7 @@ public final class KryoNamespaces { |
| 286 | ApplicationState.class, | 287 | ApplicationState.class, |
| 287 | ApplicationRole.class, | 288 | ApplicationRole.class, |
| 288 | DefaultApplication.class, | 289 | DefaultApplication.class, |
| 290 | + Permission.class, | ||
| 289 | Device.Type.class, | 291 | Device.Type.class, |
| 290 | Port.Type.class, | 292 | Port.Type.class, |
| 291 | ChassisId.class, | 293 | ChassisId.class, | ... | ... |
-
Please register or login to post a comment