diff options
Diffstat (limited to 'bundles')
5 files changed, 264 insertions, 84 deletions
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java index a6b9dccc2..fed04f2c7 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/securityadmin/SecurityAdminUnitTests.java @@ -39,6 +39,7 @@ public class SecurityAdminUnitTests extends AbstractBundleTests { private static final PermissionInfo[] RUNTIME_INFOS = new PermissionInfo[] {new PermissionInfo("java.lang.RuntimePermission", "exitVM", null)}; //$NON-NLS-1$ //$NON-NLS-2$ private static final ConditionInfo[] ALLLOCATION_CONDS = new ConditionInfo[] {new ConditionInfo("org.osgi.service.condpermadmin.BundleLocationCondition", new String[] {"*"})}; //$NON-NLS-1$ //$NON-NLS-2$ + private static final ConditionInfo MUT_SAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"MUT_SAT", "true", "false", "true"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ private static final ConditionInfo POST_MUT_SAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"POST_MUT_SAT", "true", "true", "true"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ private static final ConditionInfo POST_MUT_UNSAT = new ConditionInfo("ext.framework.b.TestCondition", new String[] {"POST_MUT_UNSAT", "true", "true", "false"}); //$NON-NLS-1$//$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ @@ -647,6 +648,30 @@ public class SecurityAdminUnitTests extends AbstractBundleTests { testSMPermission(pds, new FilePermission("test", "read"), true); //$NON-NLS-1$ //$NON-NLS-2$ } + public void testMutableConditions() { + installConditionBundle(); + TestCondition.clearConditions(); + + Bundle test1 = installTestBundle(TEST_BUNDLE); + ProtectionDomain pd1 = test1.adapt(ProtectionDomain.class); + ProtectionDomain[] pds = new ProtectionDomain[] {pd1}; + + ConditionalPermissionUpdate update = cpa.newConditionalPermissionUpdate(); + List<ConditionalPermissionInfo> rows = update.getConditionalPermissionInfos(); + rows.add(cpa.newConditionalPermissionInfo(null, new ConditionInfo[] {MUT_SAT}, READONLY_INFOS, ConditionalPermissionInfo.DENY)); + rows.add(cpa.newConditionalPermissionInfo(null, ALLLOCATION_CONDS, READONLY_INFOS, ConditionalPermissionInfo.ALLOW)); + assertTrue("failed to commit", update.commit()); //$NON-NLS-1$); + + testSMPermission(pds, new FilePermission("test", "read"), false); //$NON-NLS-1$ //$NON-NLS-2$ + + TestCondition tc1sat = TestCondition.getTestCondition("MUT_SAT_" + test1.getBundleId()); //$NON-NLS-1$ + + assertNotNull("tc1sat", tc1sat); //$NON-NLS-1$ + + tc1sat.setSatisfied(false); + testSMPermission(pds, new FilePermission("test", "read"), true); //$NON-NLS-1$ //$NON-NLS-2$ + } + public void testAccessControlContext01() { // test single row with signer condition ConditionalPermissionUpdate update = cpa.newConditionalPermissionUpdate(); diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java new file mode 100644 index 000000000..04b87fcf4 --- /dev/null +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/EvaluationCacheKey.java @@ -0,0 +1,45 @@ +/******************************************************************************* + * Copyright (c) 2018 Connexta, LLC and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Connexta, LLC - initial implementation + *******************************************************************************/ +package org.eclipse.osgi.internal.permadmin; + +import java.security.Permission; +import java.util.Objects; + +class EvaluationCacheKey { + + private final Permission permission; + + private final Long bundleId; + + EvaluationCacheKey(BundlePermissions bundlePermissions, Permission permission) { + this.permission = permission; + this.bundleId = bundlePermissions.getBundle().getBundleId(); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + EvaluationCacheKey that = (EvaluationCacheKey) o; + return Objects.equals(bundleId, that.bundleId) && Objects.equals( + permission, + that.permission); + } + + @Override + public int hashCode() { + return Objects.hash(bundleId, permission); + } +} diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityAdmin.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityAdmin.java index e156d6d71..4b992175e 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityAdmin.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityAdmin.java @@ -405,6 +405,7 @@ public final class SecurityAdmin implements PermissionAdmin, ConditionalPermissi permAdminCollections[i].clearPermissionCache(); for (int i = 0; i < condAdminRows.length; i++) condAdminRows[i].clearCaches(); + condAdminTable.clearEvaluationCache(); } EquinoxSecurityManager getSupportedSecurityManager() { diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityRow.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityRow.java index 49431e63e..e6f6f09d6 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityRow.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityRow.java @@ -7,6 +7,7 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Connexta, LLC - performance improvements *******************************************************************************/ package org.eclipse.osgi.internal.permadmin; @@ -33,6 +34,7 @@ public final class SecurityRow implements ConditionalPermissionInfo { private final boolean deny; /* GuardedBy(bundleConditions) */ final Map<BundlePermissions, Condition[]> bundleConditions; + final Object bundleConditionsLock = new Object(); public SecurityRow(SecurityAdmin securityAdmin, String name, ConditionInfo[] conditionInfos, PermissionInfo[] permissionInfos, String decision) { if (permissionInfos == null || permissionInfos.length == 0) @@ -237,62 +239,92 @@ public final class SecurityRow implements ConditionalPermissionInfo { securityAdmin.delete(this, true); } - Condition[] getConditions(Bundle bundle) { - Condition[] conditions = new Condition[conditionInfos.length]; - for (int i = 0; i < conditionInfos.length; i++) { - /* - * TODO: Can we pre-get the Constructors in our own constructor - */ - Class<?> clazz; - try { - clazz = Class.forName(conditionInfos[i].getType()); - } catch (ClassNotFoundException e) { - /* If the class isn't there, we fail */ - return null; + Condition[] getConditions(BundlePermissions bundlePermissions) { + synchronized (bundleConditionsLock) { + Condition[] conditions = null; + if (bundleConditions != null) { + conditions = bundleConditions.get(bundlePermissions); } - Constructor<?> constructor = null; - Method method = null; - try { - method = clazz.getMethod("getCondition", conditionMethodArgs); //$NON-NLS-1$ - if ((method.getModifiers() & Modifier.STATIC) == 0) - method = null; - } catch (NoSuchMethodException e) { - // This is a normal case - } - if (method == null) - try { - constructor = clazz.getConstructor(conditionMethodArgs); - } catch (NoSuchMethodException e) { - // TODO should post a FrameworkEvent of type error here - conditions[i] = Condition.FALSE; - continue; + if (conditions == null) { + conditions = new Condition[conditionInfos.length]; + for (int i = 0; i < conditionInfos.length; i++) { + /* + * TODO: Can we pre-get the Constructors in our own constructor + */ + Class<?> clazz; + try { + clazz = Class.forName(conditionInfos[i].getType()); + } catch (ClassNotFoundException e) { + /* If the class isn't there, we fail */ + return null; + } + Constructor<?> constructor = null; + Method method = getConditionMethod(clazz); + if (method == null) { + constructor = getConditionConstructor(clazz); + if (constructor == null) { + // TODO should post a FrameworkEvent of type error here + conditions[i] = Condition.FALSE; + continue; + } + } + + Object[] args = {bundlePermissions.getBundle(), conditionInfos[i]}; + try { + if (method != null) + conditions[i] = (Condition) method.invoke(null, args); + else + conditions[i] = (Condition) constructor.newInstance(args); + } catch (Exception e) { + // TODO should post a FrameworkEvent of type error here + conditions[i] = Condition.FALSE; + } } + if (bundleConditions != null) { + bundleConditions.put(bundlePermissions, conditions); + } + } + return conditions; + } + } - Object args[] = {bundle, conditionInfos[i]}; - try { - if (method != null) - conditions[i] = (Condition) method.invoke(null, args); - else - conditions[i] = (Condition) constructor.newInstance(args); - } catch (Throwable t) { - // TODO should post a FrameworkEvent of type error here - conditions[i] = Condition.FALSE; + private Method getConditionMethod(Class<?> clazz) { + for (Method checkMethod : clazz.getMethods()) { + if (checkMethod.getName().equals("getCondition") + && (checkMethod.getModifiers() & Modifier.STATIC) == Modifier.STATIC + && checkParameterTypes(checkMethod.getParameterTypes())) { + return checkMethod; } } - return conditions; + return null; + } + + private Constructor<?> getConditionConstructor(Class<?> clazz) { + for (Constructor<?> checkConstructor : clazz.getConstructors()) { + if (checkParameterTypes(checkConstructor.getParameterTypes())) { + return checkConstructor; + } + } + return null; + } + + private boolean checkParameterTypes(Class<?>[] foundTypes) { + if (foundTypes.length != conditionMethodArgs.length) { + return false; + } + + for (int i = 0; i < foundTypes.length; i++) { + if (!foundTypes[i].isAssignableFrom(conditionMethodArgs[i])) { + return false; + } + } + return true; } Decision evaluate(BundlePermissions bundlePermissions, Permission permission) { if (bundleConditions == null || bundlePermissions == null) return evaluatePermission(permission); - Condition[] conditions; - synchronized (bundleConditions) { - conditions = bundleConditions.get(bundlePermissions); - if (conditions == null) { - conditions = getConditions(bundlePermissions.getBundle()); - bundleConditions.put(bundlePermissions, conditions); - } - } + Condition[] conditions = getConditions(bundlePermissions); if (conditions == ABSTAIN_LIST) return DECISION_ABSTAIN; if (conditions == SATISFIED_LIST) @@ -314,7 +346,7 @@ public final class SecurityRow implements ConditionalPermissionInfo { } else { if (!mutable) // this will cause the row to always abstain; mark this to be ignored in future checks - synchronized (bundleConditions) { + synchronized (bundleConditionsLock) { bundleConditions.put(bundlePermissions, ABSTAIN_LIST); } return DECISION_ABSTAIN; @@ -333,7 +365,7 @@ public final class SecurityRow implements ConditionalPermissionInfo { empty &= conditions[i] == null; } if (empty) { - synchronized (bundleConditions) { + synchronized (bundleConditionsLock) { bundleConditions.put(bundlePermissions, SATISFIED_LIST); } } @@ -413,7 +445,7 @@ public final class SecurityRow implements ConditionalPermissionInfo { void clearCaches() { permissionInfoCollection.clearPermissionCache(); if (bundleConditions != null) - synchronized (bundleConditions) { + synchronized (bundleConditionsLock) { bundleConditions.clear(); } } @@ -435,7 +467,7 @@ public final class SecurityRow implements ConditionalPermissionInfo { if (mutable || !condition.isPostponed()) return; // do nothing if (isSatisfied) { - synchronized (row.bundleConditions) { + synchronized (row.bundleConditionsLock) { Condition[] rowConditions = row.bundleConditions.get(bundlePermissions); boolean isEmpty = true; for (int i = 0; i < rowConditions.length; i++) { @@ -448,7 +480,7 @@ public final class SecurityRow implements ConditionalPermissionInfo { row.bundleConditions.put(bundlePermissions, SATISFIED_LIST); } } else { - synchronized (row.bundleConditions) { + synchronized (row.bundleConditionsLock) { row.bundleConditions.put(bundlePermissions, ABSTAIN_LIST); } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java index bdd1c188c..43dc91154 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/permadmin/SecurityTable.java @@ -7,13 +7,18 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Connexta, LLC - evaluation cache implementation *******************************************************************************/ package org.eclipse.osgi.internal.permadmin; import java.security.Permission; import java.security.PermissionCollection; import java.util.Enumeration; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + import org.eclipse.osgi.internal.permadmin.SecurityRow.Decision; +import org.osgi.service.condpermadmin.Condition; public class SecurityTable extends PermissionCollection { private static final long serialVersionUID = -1800193310096318060L; @@ -22,9 +27,14 @@ public class SecurityTable extends PermissionCollection { static final int ABSTAIN = 0x0004; static final int POSTPONED = 0x0008; + private static final int MUTABLE = 0x0016; + private final SecurityRow[] rows; private final SecurityAdmin securityAdmin; + private final transient Map<EvaluationCacheKey, Integer> evaluationCache = + new ConcurrentHashMap<>(10000); + public SecurityTable(SecurityAdmin securityAdmin, SecurityRow[] rows) { if (rows == null) throw new NullPointerException("rows cannot be null!!"); //$NON-NLS-1$ @@ -37,63 +47,130 @@ public class SecurityTable extends PermissionCollection { } int evaluate(BundlePermissions bundlePermissions, Permission permission) { - if (isEmpty()) + if (bundlePermissions == null) { return ABSTAIN; + } + EvaluationCacheKey evaluationCacheKey = new EvaluationCacheKey(bundlePermissions, + permission); + if (isEmpty()) { + evaluationCache.put(evaluationCacheKey, ABSTAIN); + return ABSTAIN; + } + + //can't short-circuit early, so try cache + Integer result = evaluationCache.get(evaluationCacheKey); + boolean hasMutable = false; + if (result != null) { + hasMutable = (result & MUTABLE) == MUTABLE; + if (!hasMutable) { + return result; + } + } + //cache miss or has mutable rows boolean postponed = false; Decision[] results = new Decision[rows.length]; int immediateDecisionIdx = -1; // evaluate each row - for (int i = 0; i < rows.length; i++) { + for (int i = 0; i < rows.length && immediateDecisionIdx == -1; i++) { + if (result == null) { + //check all conditions for any that are mutable, this will turn off the cache + hasMutable |= checkMutable(bundlePermissions, + evaluationCacheKey, + rows[i]); + } try { results[i] = rows[i].evaluate(bundlePermissions, permission); - } catch (Throwable t) { + } catch (Exception e) { // TODO log? results[i] = SecurityRow.DECISION_ABSTAIN; } - if ((results[i].decision & ABSTAIN) != 0) + if ((results[i].decision & ABSTAIN) == ABSTAIN) continue; // ignore this row and continue to next row - if ((results[i].decision & POSTPONED) != 0) { + if ((results[i].decision & POSTPONED) == POSTPONED) { // row is postponed; we can no longer return quickly on a denied decision postponed = true; continue; // continue to next row } - if (!postponed) + if (!postponed) { // no postpones encountered yet; we can return the decision quickly + if (!hasMutable) { + evaluationCache.put(evaluationCacheKey, results[i].decision); + } return results[i].decision; // return GRANTED or DENIED + } // got an immediate answer; but it is after a postponed condition. // no need to process the rest of the rows immediateDecisionIdx = i; - break; } - if (postponed) { - int immediateDecision = immediateDecisionIdx < 0 ? DENIED : results[immediateDecisionIdx].decision; - // iterate over all postponed conditions; - // if they all provide the same decision as the immediate decision then return the immediate decision - boolean allSameDecision = true; - int i = immediateDecisionIdx < 0 ? results.length - 1 : immediateDecisionIdx - 1; - for (; i >= 0 && allSameDecision; i--) { - if (results[i] == null) - continue; - if ((results[i].decision & POSTPONED) != 0) { - if ((results[i].decision & immediateDecision) == 0) - allSameDecision = false; - else - results[i] = SecurityRow.DECISION_ABSTAIN; // we can clear postpones with the same decision as the immediate + Integer immediateDecision = handlePostponedConditions(evaluationCacheKey, + hasMutable, + postponed, + results, + immediateDecisionIdx); + if (immediateDecision != null) + return immediateDecision; + int finalDecision = postponed ? POSTPONED : ABSTAIN; + if (!hasMutable && (finalDecision & POSTPONED) != POSTPONED) { + evaluationCache.put(evaluationCacheKey, finalDecision); + } + return finalDecision; + } + + private boolean checkMutable(BundlePermissions bundlePermissions, + EvaluationCacheKey evaluationCacheKey, SecurityRow row) { + Condition[] conditions = row.getConditions(bundlePermissions); + if (conditions != null) { + for (Condition condition : conditions) { + if (condition.isMutable()) { + evaluationCache.put(evaluationCacheKey, MUTABLE); + return true; } } - if (allSameDecision) - return immediateDecision; - - // we now are forced to postpone; we need to also remember the postponed decisions and - // the immediate decision if there is one. - EquinoxSecurityManager equinoxManager = securityAdmin.getSupportedSecurityManager(); - if (equinoxManager == null) - // TODO this is really an error condition. - // This should never happen. We checked for a supported manager when the row was postponed - return ABSTAIN; - equinoxManager.addConditionsForDomain(results); } - return postponed ? POSTPONED : ABSTAIN; + return false; + } + + private Integer handlePostponedConditions(EvaluationCacheKey evaluationCacheKey, + boolean hasMutable, boolean postponed, Decision[] results, int immediateDecisionIdx) { + if (postponed) { + int immediateDecision = immediateDecisionIdx < 0 ? DENIED : results[immediateDecisionIdx].decision; + // iterate over all postponed conditions; + // if they all provide the same decision as the immediate decision then return the immediate decision + boolean allSameDecision = true; + int i = immediateDecisionIdx < 0 ? results.length - 1 : immediateDecisionIdx - 1; + for (; i >= 0 && allSameDecision; i--) { + if ((results[i].decision & POSTPONED) == POSTPONED) { + if ((results[i].decision & immediateDecision) == 0) + allSameDecision = false; + else + results[i] = SecurityRow.DECISION_ABSTAIN; // we can clear postpones with the same decision as the immediate + } + } + if (allSameDecision) { + if (!hasMutable) { + evaluationCache.put(evaluationCacheKey, immediateDecision); + } + return immediateDecision; + } + + // we now are forced to postpone; we need to also remember the postponed decisions and + // the immediate decision if there is one. + EquinoxSecurityManager equinoxManager = securityAdmin.getSupportedSecurityManager(); + if (equinoxManager == null) { + // TODO this is really an error condition. + // This should never happen. We checked for a supported manager when the row was postponed + if (!hasMutable) { + evaluationCache.put(evaluationCacheKey, ABSTAIN); + } + return ABSTAIN; + } + equinoxManager.addConditionsForDomain(results); + } + return null; + } + + void clearEvaluationCache() { + evaluationCache.clear(); } SecurityRow getRow(int i) { |