diff --git a/netx/net/sourceforge/jnlp/config/BasicValueValidators.java b/netx/net/sourceforge/jnlp/config/BasicValueValidators.java --- a/netx/net/sourceforge/jnlp/config/BasicValueValidators.java +++ b/netx/net/sourceforge/jnlp/config/BasicValueValidators.java @@ -37,6 +37,8 @@ package net.sourceforge.jnlp.config; +import net.sourceforge.jnlp.runtime.ManifestAttributesChecker; + import java.io.File; import static net.sourceforge.jnlp.runtime.Translator.R; @@ -202,6 +204,90 @@ } /** + * Checks that the value is one of the acceptable single String values + * or an acceptable combination of String values + */ + private static class MultipleStringValueValidator implements ValueValidator { + private final String[] singleOptions; + private final String[] comboOptions; + private final static String DELIMITER = ","; + + public MultipleStringValueValidator(String[] singleOptions, String[] comboOptions) { + this.singleOptions = singleOptions; + this.comboOptions = comboOptions; + } + + @Override + public void validate(Object value) throws IllegalArgumentException { + Object possibleValue = value; + if (!(possibleValue instanceof String)) { + throw new IllegalArgumentException("Must be a string"); + } + + String stringVal = (String) possibleValue; + boolean found = false; + for (String knownVal : singleOptions) { + if (knownVal.equals(stringVal)) { + found = true; + break; + } + } + + if (!found) { + String[] possibleCombo = splitCombination(stringVal); + for (String val : possibleCombo) { + if (comboOptionsContains(val)) { + found = true; + } else { + throw new IllegalArgumentException(); + } + } + } + + if (!found) { + throw new IllegalArgumentException(); + } + } + + private boolean comboOptionsContains(String possibleVal) { + for (String value : comboOptions) { + if (value.equals(possibleVal)) { + return true; + } + } + return false; + } + + public static String[] splitCombination(String val) { + return val.split(DELIMITER); + } + + @Override + public String getPossibleValues() { + String message = "(Values that can be used alone only): " + Arrays.toString(singleOptions) + + " (Values that can be used in combination separated by the delimiter \"" + + DELIMITER + "\" with no space expected ): " + Arrays.toString(comboOptions); + return message; + } + + } + + private static class ManifestAttributeCheckValidator extends MultipleStringValueValidator { + + public ManifestAttributeCheckValidator() { + super(new String[] { + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.ALL.toString(), + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.NONE.toString() + }, new String[] { + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.ALAC.toString(), + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.CODEBASE.toString(), + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.ENTRYPOINT.toString(), + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.PERMISSIONS.toString(), + ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.TRUSTED.toString() + }); + } + } + /** * Checks that the value is a URL */ private static class UrlValidator implements ValueValidator { @@ -262,6 +348,26 @@ } /** + * Returns a {@link ValueValidator} that checks if an object is a string from + * one of the provided single option Strings or a combination from + * the provided combination Strings. + * @param singleValues an array of Strings which are considered valid only by themselves + * @param comboValues an array of Strings which are considered valid in any combination + * with themselves + */ + public static ValueValidator getMultipleStringValidator(String[] singleValues, String[] comboValues) { + return new MultipleStringValueValidator(singleValues, comboValues); + } + + /** + * Returns a {@link ValueValidator} that checks if an object is a string + * from the possible single or combination ManifestAttributeCheck values + */ + public static ValueValidator getManifestAttributeCheckValidator() { + return new ManifestAttributeCheckValidator(); + } + + /** * @return a {@link ValueValidator} that checks if an object represents a * valid url */ diff --git a/netx/net/sourceforge/jnlp/config/Defaults.java b/netx/net/sourceforge/jnlp/config/Defaults.java --- a/netx/net/sourceforge/jnlp/config/Defaults.java +++ b/netx/net/sourceforge/jnlp/config/Defaults.java @@ -43,6 +43,8 @@ import net.sourceforge.jnlp.ShortcutDesc; import static net.sourceforge.jnlp.config.PathsAndFiles.*; import net.sourceforge.jnlp.runtime.JNLPProxySelector; +import net.sourceforge.jnlp.runtime.ManifestAttributesChecker; + import static net.sourceforge.jnlp.runtime.Translator.R; /** @@ -409,8 +411,8 @@ //enable manifest-attributes checks { DeploymentConfiguration.KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK, - BasicValueValidators.getBooleanValidator(), - String.valueOf(true) + BasicValueValidators.getManifestAttributeCheckValidator(), + String.valueOf("ALL") } }; diff --git a/netx/net/sourceforge/jnlp/resources/Messages.properties b/netx/net/sourceforge/jnlp/resources/Messages.properties --- a/netx/net/sourceforge/jnlp/resources/Messages.properties +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties @@ -95,6 +95,10 @@ and<br/> <a href="http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/no_redeploy.html"> \ Preventing the Repurposing of an Application</a> +MACDisabledMessage=Manifest attribute checks are disabled. +MACCheckSkipped=check on {0} skipped because property of deployment.manifest.attributes.check \ +was not set to ALL or includes {1} in the combination of options + # LS - Severity LSMinor=Minor LSFatal=Fatal diff --git a/netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java b/netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java --- a/netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java +++ b/netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java @@ -37,7 +37,9 @@ package net.sourceforge.jnlp.runtime; import java.net.URL; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; import net.sourceforge.jnlp.ExtensionDesc; @@ -59,13 +61,14 @@ import net.sourceforge.jnlp.util.UrlUtils; import net.sourceforge.jnlp.util.logging.OutputController; +import static net.sourceforge.jnlp.runtime.Translator.R; + public class ManifestAttributesChecker { private final SecurityDesc security; private final JNLPFile file; private final SigningState signing; private final SecurityDelegate securityDelegate; - public static final String MANIFEST_CHECK_DISABLED_MESSAGE = "Manifest attribute checks are disabled."; public ManifestAttributesChecker(final SecurityDesc security, final JNLPFile file, final SigningState signing, final SecurityDelegate securityDelegate) throws LaunchException { @@ -75,24 +78,66 @@ this.securityDelegate = securityDelegate; } + public enum MANIFEST_ATTRIBUTES_CHECK { + ALL, + NONE, + PERMISSIONS, + CODEBASE, + TRUSTED, + ALAC, + ENTRYPOINT + } + void checkAll() throws LaunchException { - if (isCheckEnabled()) { - checkPermissionsAttribute(); - checkTrustedOnlyAttribute(); - checkCodebaseAttribute(); - checkPermissionsAttribute(); - checkApplicationLibraryAllowableCodebaseAttribute(); - checkEntryPoint(); + List<MANIFEST_ATTRIBUTES_CHECK> attributesCheck = getAttributesCheck(); + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.NONE)) { + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, R("MACDisabledMessage")); } else { - OutputController.getLogger().log(OutputController.Level.WARNING_ALL, MANIFEST_CHECK_DISABLED_MESSAGE); + + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.TRUSTED) || + attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALL)) { + checkTrustedOnlyAttribute(); + } else { + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, R("MACCheckSkipped", "Trusted-Only", "TRUSTED")); + } + + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.CODEBASE) || + attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALL)) { + checkCodebaseAttribute(); + } else { + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, R("MACCheckSkipped", "Codebase", "CODEBASE")); + } + + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.PERMISSIONS) || + attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALL)) { + checkPermissionsAttribute(); + } else { + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, R("MACCheckSkipped", "Permissions", "PERMISSIONS")); + } + + if (attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALAC) || + attributesCheck.contains(MANIFEST_ATTRIBUTES_CHECK.ALL)) { + checkApplicationLibraryAllowableCodebaseAttribute(); + } else { + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, R("MACCheckSkipped", "Application Library Allowable Codebase", "ALAC")); + } + } } - public static boolean isCheckEnabled() { + public static List<MANIFEST_ATTRIBUTES_CHECK> getAttributesCheck() { final String deploymentProperty = JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK); - return Boolean.parseBoolean(deploymentProperty); + String[] attributesCheck = deploymentProperty.split(","); + List<MANIFEST_ATTRIBUTES_CHECK> manifestAttributesCheckList = new ArrayList<>(); + for (String attribute : attributesCheck) { + for (MANIFEST_ATTRIBUTES_CHECK manifestAttribute : MANIFEST_ATTRIBUTES_CHECK.values()) { + if (manifestAttribute.toString().equals(attribute)) { + manifestAttributesCheckList.add(manifestAttribute); + } + } + } + return manifestAttributesCheckList; } - /* * http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/manifest.html#entry_pt */ @@ -159,16 +204,16 @@ || (isSandboxed && SecurityDesc.SANDBOX_PERMISSIONS.equals(desc)); final String signedMsg; if (isFullySigned && !isSandboxed) { - signedMsg = Translator.R("STOAsignedMsgFully"); + signedMsg = R("STOAsignedMsgFully"); } else if (isFullySigned && isSandboxed) { - signedMsg = Translator.R("STOAsignedMsgAndSandbox"); + signedMsg = R("STOAsignedMsgAndSandbox"); } else { - signedMsg = Translator.R("STOAsignedMsgPartiall"); + signedMsg = R("STOAsignedMsgPartiall"); } OutputController.getLogger().log(OutputController.Level.MESSAGE_DEBUG, "Trusted Only manifest attribute is \"true\". " + signedMsg + " and requests permission level: " + securityType); if (!(isFullySigned && requestsCorrectPermissions)) { - throw new LaunchException(Translator.R("STrustedOnlyAttributeFailure", signedMsg, securityType)); + throw new LaunchException(R("STrustedOnlyAttributeFailure", signedMsg, securityType)); } } @@ -177,30 +222,30 @@ */ private void checkCodebaseAttribute() throws LaunchException { if (file.getCodeBase() == null || file.getCodeBase().getProtocol().equals("file")) { - OutputController.getLogger().log(OutputController.Level.WARNING_ALL, Translator.R("CBCheckFile")); + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, R("CBCheckFile")); return; } final Object securityType = security.getSecurityType(); final URL codebase = UrlUtils.guessCodeBase(file); final ClasspathMatchers codebaseAtt = file.getManifestsAttributes().getCodebase(); if (codebaseAtt == null) { - OutputController.getLogger().log(OutputController.Level.WARNING_ALL, Translator.R("CBCheckNoEntry")); + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, R("CBCheckNoEntry")); return; } if (securityType.equals(SecurityDesc.SANDBOX_PERMISSIONS)) { if (codebaseAtt.matches(codebase)) { - OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, Translator.R("CBCheckUnsignedPass")); + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, R("CBCheckUnsignedPass")); } else { - OutputController.getLogger().log(OutputController.Level.ERROR_ALL, Translator.R("CBCheckUnsignedFail")); + OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("CBCheckUnsignedFail")); } } else { if (codebaseAtt.matches(codebase)) { - OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, Translator.R("CBCheckOkSignedOk")); + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, R("CBCheckOkSignedOk")); } else { if (file instanceof PluginBridge) { - throw new LaunchException(Translator.R("CBCheckSignedAppletDontMatchException", file.getManifestsAttributes().getCodebase().toString(), codebase)); + throw new LaunchException(R("CBCheckSignedAppletDontMatchException", file.getManifestsAttributes().getCodebase().toString(), codebase)); } else { - OutputController.getLogger().log(OutputController.Level.ERROR_ALL, Translator.R("CBCheckSignedFail")); + OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("CBCheckSignedFail")); } } } diff --git a/tests/netx/unit/net/sourceforge/jnlp/config/BasicValueValidatorsTests.java b/tests/netx/unit/net/sourceforge/jnlp/config/BasicValueValidatorsTests.java --- a/tests/netx/unit/net/sourceforge/jnlp/config/BasicValueValidatorsTests.java +++ b/tests/netx/unit/net/sourceforge/jnlp/config/BasicValueValidatorsTests.java @@ -130,4 +130,55 @@ Assert.assertTrue(ex instanceof IllegalArgumentException); } } + + @Test + public void testMultipleStringValueValidator() { + String[] singleValues = { + "SINGLE", "ONLY", "NONE" + }; + String[] multipleValues = { + "MULTIPLE", "COMBO", "ONE", "TWO", "THREE" + }; + ValueValidator multipleValidator = BasicValueValidators.getMultipleStringValidator(singleValues, multipleValues); + + multipleValidator.validate("SINGLE"); + multipleValidator.validate("ONLY"); + multipleValidator.validate("NONE"); + multipleValidator.validate("MULTIPLE"); + multipleValidator.validate("MULTIPLE,COMBO,TWO"); + multipleValidator.validate("THREE,ONE,COMBO,MULTIPLE"); + } + + @Test(expected = IllegalArgumentException.class) + public void testMultipleStringValueValidatorCantMixSingleAndComboValues() { + String[] singleValues = { + "SINGLE", "ONLY", "NONE" + }; + String[] multipleValues = { + "MULTIPLE", "COMBO", "ONE", "TWO", "THREE" + }; + ValueValidator multipleValidator = BasicValueValidators.getMultipleStringValidator(singleValues, multipleValues); + + multipleValidator.validate("SINGLE,COMBO"); + } + + @Test + public void testManifestAttributeCheckValidator() { + ValueValidator multipleValidator = BasicValueValidators.getManifestAttributeCheckValidator(); + + multipleValidator.validate("ALL"); + multipleValidator.validate("PERMISSIONS"); + multipleValidator.validate("NONE"); + multipleValidator.validate("ALAC"); + multipleValidator.validate("CODEBASE"); + multipleValidator.validate("CODEBASE,ALAC,PERMISSIONS"); + multipleValidator.validate("TRUSTED,ALAC,CODEBASE"); + } + + @Test(expected = IllegalArgumentException.class) + public void testManifestAttributeCheckValidatorCantMixSingleAndComboValues() { + ValueValidator multipleValidator = BasicValueValidators.getManifestAttributeCheckValidator(); + + multipleValidator.validate("ALL,CODEBASE,NONE"); + } } diff --git a/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java b/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java --- a/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java +++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java @@ -39,6 +39,7 @@ import java.io.File; import java.net.URL; import java.util.Arrays; +import java.util.List; import java.util.Locale; import java.util.jar.Attributes; import java.util.jar.Manifest; @@ -60,14 +61,14 @@ public class JNLPFileTest extends NoStdOutErrTest { private static AppletSecurityLevel level; - private static boolean attCheckValue; + private static List<ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK> attCheckValue; @BeforeClass public static void setPermissions() { level = AppletStartupSecuritySettings.getInstance().getSecurityLevel(); - attCheckValue = ManifestAttributesChecker.isCheckEnabled(); + attCheckValue = ManifestAttributesChecker.getAttributesCheck(); JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_SECURITY_LEVEL, AppletSecurityLevel.ALLOW_UNSIGNED.toChars()); - JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK, String.valueOf(true)); + JNLPRuntime.getConfiguration().setProperty(DeploymentConfiguration.KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK, String.valueOf(ManifestAttributesChecker.MANIFEST_ATTRIBUTES_CHECK.ALL)); } @AfterClass diff --git a/tests/reproducers/custom/PartiallySignedAppletManifestSpecifiesSandbox/testcases/PartiallySignedAppletManifestSpecifiesSandboxTests.java b/tests/reproducers/custom/PartiallySignedAppletManifestSpecifiesSandbox/testcases/PartiallySignedAppletManifestSpecifiesSandboxTests.java --- a/tests/reproducers/custom/PartiallySignedAppletManifestSpecifiesSandbox/testcases/PartiallySignedAppletManifestSpecifiesSandboxTests.java +++ b/tests/reproducers/custom/PartiallySignedAppletManifestSpecifiesSandbox/testcases/PartiallySignedAppletManifestSpecifiesSandboxTests.java @@ -36,6 +36,10 @@ */ import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; + import net.sourceforge.jnlp.ProcessResult; import net.sourceforge.jnlp.annotations.Bug; import net.sourceforge.jnlp.annotations.NeedsDisplay; @@ -44,6 +48,10 @@ import net.sourceforge.jnlp.browsertesting.Browsers; import net.sourceforge.jnlp.closinglisteners.AutoOkClosingListener; +import net.sourceforge.jnlp.config.PathsAndFiles; +import net.sourceforge.jnlp.util.FileUtils; +import org.junit.AfterClass; +import org.junit.BeforeClass; import org.junit.Test; public class PartiallySignedAppletManifestSpecifiesSandboxTests extends BrowserTest { @@ -56,6 +64,52 @@ private static final String STACKTRACE_NOT_GRANT_PERMISSIONS_TYPE = "Cannot grant permissions to unsigned jars"; private static final String USER_HOME = System.getProperty("user.home"); + private static File deployFile; + private static String attributesCheck; + private static String securityLevel; + + @BeforeClass + public static void setupDeploymentProperties() throws IOException { + deployFile = PathsAndFiles.USER_DEPLOYMENT_FILE.getFile(); + String properties = FileUtils.loadFileAsString(deployFile); + + for (String line : properties.split("\n")) { + if (line.contains("deployment.manifest.attribute.check")) { + attributesCheck = line; + properties = properties.replace(line, "deployment.manifest.attributes.check=PERMISSIONS\n"); + } + if (line.contains("deployment.security.level")) { + securityLevel = line; + properties = properties.replace(line, "deployment.security.level=ALLOW_UNSIGNED\n"); + } + } + if (attributesCheck == null) { + properties += "deployment.manifest.attributes.check=PERMISSIONS\n"; + } + if (securityLevel == null) { + properties += "deployment.security.level=ALLOW_UNSIGNED\n"; + } + + FileUtils.saveFile(properties, deployFile); + } + + @AfterClass + public static void setbackDeploymentProperties() throws IOException { + String properties = FileUtils.loadFileAsString(deployFile); + if (attributesCheck != null) { + properties = properties.replace("deployment.manifest.attributes.check=PERMISSIONS\n", attributesCheck); + } else { + properties = properties.replace("deployment.manifest.attributes.check=PERMISSIONS\n", ""); + } + + if (securityLevel != null) { + properties = properties.replace("deployment.security.level=ALLOW_UNSIGNED\n", securityLevel); + } else { + properties = properties.replace("deployment.security.level=ALLOW_UNSIGNED\n", ""); + } + FileUtils.saveFile(properties, deployFile); + } + @Test @NeedsDisplay @TestInBrowsers(testIn={Browsers.one}) diff --git a/tests/reproducers/signed/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java b/tests/reproducers/signed/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java --- a/tests/reproducers/signed/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java +++ b/tests/reproducers/signed/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java @@ -36,6 +36,11 @@ */ import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.util.List; + import net.sourceforge.jnlp.ProcessResult; import net.sourceforge.jnlp.annotations.Bug; import net.sourceforge.jnlp.annotations.NeedsDisplay; @@ -44,6 +49,11 @@ import net.sourceforge.jnlp.browsertesting.Browsers; import net.sourceforge.jnlp.closinglisteners.AutoOkClosingListener; +import net.sourceforge.jnlp.config.PathsAndFiles; +import net.sourceforge.jnlp.runtime.ManifestAttributesChecker; +import net.sourceforge.jnlp.util.FileUtils; +import org.junit.AfterClass; +import org.junit.BeforeClass; import org.junit.Test; public class SignedAppletManifestSpecifiesSandboxTests extends BrowserTest { @@ -56,6 +66,39 @@ private static final String JNLP_EXPECTED_STDOUT = "Initialization Error"; private static final String JNLP_EXPECTED_STDERR = "net.sourceforge.jnlp.LaunchException"; + private static File deployFile; + private static String attributesCheck; + + @BeforeClass + public static void setupDeploymentProperties() throws IOException { + deployFile = PathsAndFiles.USER_DEPLOYMENT_FILE.getFile(); + String properties = FileUtils.loadFileAsString(deployFile); + + for (String line : properties.split("\n")) { + if (line.contains("deployment.manifest.attribute.check")) { + attributesCheck = line; + properties = properties.replace(line, "deployment.manifest.attributes.check=PERMISSIONS\n"); + } + } + if (attributesCheck == null) { + properties += "deployment.manifest.attributes.check=PERMISSIONS\n"; + } + + FileUtils.saveFile(properties, deployFile); + } + + @AfterClass + public static void setbackDeploymentProperties() throws IOException { + String properties = FileUtils.loadFileAsString(deployFile); + if (attributesCheck != null) { + properties = properties.replace("deployment.manifest.attributes.check=PERMISSIONS\n", attributesCheck); + } else { + properties = properties.replace("deployment.manifest.attributes.check=PERMISSIONS\n", ""); + } + + FileUtils.saveFile(properties, deployFile); + } + @Test @NeedsDisplay @TestInBrowsers(testIn={Browsers.one})