From 812d805a488597cb9ab6b375869bc6748fd7fa94 Mon Sep 17 00:00:00 2001 From: Valerie Peng Date: Fri, 9 Sep 2022 00:30:54 +0000 Subject: [PATCH] 6447816: Provider filtering (getProviders) is not working with OR'd conditions Reviewed-by: weijun --- .../share/classes/java/security/Security.java | 343 +++++++----------- .../security/Security/ProviderFiltering.java | 156 ++++++++ 2 files changed, 295 insertions(+), 204 deletions(-) create mode 100644 test/jdk/java/security/Security/ProviderFiltering.java diff --git a/src/java.base/share/classes/java/security/Security.java b/src/java.base/share/classes/java/security/Security.java index 247612ebb21..d3359290acc 100644 --- a/src/java.base/share/classes/java/security/Security.java +++ b/src/java.base/share/classes/java/security/Security.java @@ -519,14 +519,20 @@ public final class Security { public static Provider[] getProviders(String filter) { String key; String value; + int index = filter.indexOf(':'); - if (index == -1) { - key = filter; + if (index == -1) { // . only + key = filter.trim(); value = ""; } else { - key = filter.substring(0, index); - value = filter.substring(index + 1); + // . : + key = filter.substring(0, index).trim(); + value = filter.substring(index + 1).trim(); + // ensure value is not empty here; rest will be checked in Criteria + if (value.isEmpty()) { + throw new InvalidParameterException("Invalid filter"); + } } Hashtable hashtableFilter = new Hashtable<>(1); @@ -591,42 +597,31 @@ public final class Security { // Get all installed providers first. // Then only return those providers who satisfy the selection criteria. Provider[] allProviders = Security.getProviders(); - Set keySet = filter.keySet(); - LinkedHashSet candidates = new LinkedHashSet<>(5); + Set> entries = filter.entrySet(); - // Returns all installed providers - // if the selection criteria is null. - if ((keySet == null) || (allProviders == null)) { + if (allProviders == null || allProviders.length == 0) { + return null; + } else if (entries == null) { + // return all installed providers if the selection criteria is null return allProviders; + } else if (entries.isEmpty()) { + // return null if the selection criteria is empty; this is to match + // earlier behavior + return null; } - boolean firstSearch = true; + LinkedList candidates = + new LinkedList<>(Arrays.asList(allProviders)); // For each selection criterion, remove providers // which don't satisfy the criterion from the candidate set. - for (String key : keySet) { - String value = filter.get(key); - - LinkedHashSet newCandidates = getAllQualifyingCandidates(key, value, - allProviders); - if (firstSearch) { - candidates = newCandidates; - firstSearch = false; + for (var e : entries) { + Criteria cr = new Criteria(e.getKey(), e.getValue()); + candidates.removeIf(p -> !cr.isCriterionSatisfied(p)); + if (candidates.isEmpty()) { + return null; } - - if (!newCandidates.isEmpty()) { - // For each provider in the candidates set, if it - // isn't in the newCandidate set, we should remove - // it from the candidate set. - candidates.removeIf(prov -> !newCandidates.contains(prov)); - } else { - candidates = null; - break; - } - } - - if (candidates == null || candidates.isEmpty()) - return null; + }; return candidates.toArray(new Provider[0]); } @@ -822,194 +817,134 @@ public final class Security { } } - /* - * Returns all providers who satisfy the specified - * criterion. - */ - private static LinkedHashSet getAllQualifyingCandidates( - String filterKey, - String filterValue, - Provider[] allProviders) { - String[] filterComponents = getFilterComponents(filterKey, - filterValue); + private static class Criteria { + private final String serviceName; + private final String algName; + private final String attrName; + private final String attrValue; - // The first component is the service name. - // The second is the algorithm name. - // If the third isn't null, that is the attribute name. - String serviceName = filterComponents[0]; - String algName = filterComponents[1]; - String attrName = filterComponents[2]; + Criteria(String key, String value) throws InvalidParameterException { - return getProvidersNotUsingCache(serviceName, algName, attrName, - filterValue, allProviders); - } - - private static LinkedHashSet getProvidersNotUsingCache( - String serviceName, - String algName, - String attrName, - String filterValue, - Provider[] allProviders) { - LinkedHashSet candidates = new LinkedHashSet<>(5); - for (int i = 0; i < allProviders.length; i++) { - if (isCriterionSatisfied(allProviders[i], serviceName, - algName, - attrName, filterValue)) { - candidates.add(allProviders[i]); + int snEndIndex = key.indexOf('.'); + if (snEndIndex <= 0) { + // There must be a dot in the filter, and the dot + // shouldn't be at the beginning of this string. + throw new InvalidParameterException("Invalid filter"); } - } - return candidates; - } - /* - * Returns {@code true} if the given provider satisfies - * the selection criterion key:value. - */ - private static boolean isCriterionSatisfied(Provider prov, - String serviceName, - String algName, - String attrName, - String filterValue) { - String key = serviceName + '.' + algName; + serviceName = key.substring(0, snEndIndex); + attrValue = value; - if (attrName != null) { - key += ' ' + attrName; - } - // Check whether the provider has a property - // whose key is the same as the given key. - String propValue = getProviderProperty(key, prov); - - if (propValue == null) { - // Check whether we have an alias instead - // of a standard name in the key. - String standardName = getProviderProperty("Alg.Alias." + - serviceName + "." + - algName, - prov); - if (standardName != null) { - key = serviceName + "." + standardName; - - if (attrName != null) { - key += ' ' + attrName; + if (value.isEmpty()) { + // value is empty. So the key should be in the format of + // .. + algName = key.substring(snEndIndex + 1); + attrName = null; + } else { + // value is non-empty. So the key must be in the format + // of .(one or more + // spaces) + int algEndIndex = key.indexOf(' ', snEndIndex); + if (algEndIndex == -1) { + throw new InvalidParameterException + ("Invalid filter - need algorithm name"); + } + algName = key.substring(snEndIndex + 1, algEndIndex); + attrName = key.substring(algEndIndex + 1).trim(); + if (attrName.isEmpty()) { + throw new InvalidParameterException + ("Invalid filter - need attribute name"); + } else if (isCompositeValue() && attrValue.indexOf('|') != -1) { + throw new InvalidParameterException + ("Invalid filter - composite values unsupported"); } - - propValue = getProviderProperty(key, prov); } + // check required values + if (serviceName.isEmpty() || algName.isEmpty()) { + throw new InvalidParameterException + ("Invalid filter - need service and algorithm"); + } + } + + // returns true when this criteria contains a standard attribute + // whose value may be composite, i.e. multiple values separated by "|" + private boolean isCompositeValue() { + return (attrName != null && + (attrName.equalsIgnoreCase("SupportedKeyClasses") || + attrName.equalsIgnoreCase("SupportedPaddings") || + attrName.equalsIgnoreCase("SupportedModes") || + attrName.equalsIgnoreCase("SupportedKeyFormats"))); + } + + /* + * Returns {@code true} if the given provider satisfies + * the selection criterion key:value. + */ + private boolean isCriterionSatisfied(Provider prov) { + // Constructed key have ONLY 1 space between algName and attrName + String key = serviceName + '.' + algName + + (attrName != null ? (' ' + attrName) : ""); + + // Check whether the provider has a property + // whose key is the same as the given key. + String propValue = getProviderProperty(key, prov); + if (propValue == null) { - // The provider doesn't have the given - // key in its property list. - return false; - } - } + // Check whether we have an alias instead + // of a standard name in the key. + String standardName = getProviderProperty("Alg.Alias." + + serviceName + "." + algName, prov); + if (standardName != null) { + key = serviceName + "." + standardName + + (attrName != null ? ' ' + attrName : ""); + propValue = getProviderProperty(key, prov); + } - // If the key is in the format of: - // ., - // there is no need to check the value. - - if (attrName == null) { - return true; - } - - // If we get here, the key must be in the - // format of . . - if (isStandardAttr(attrName)) { - return isConstraintSatisfied(attrName, filterValue, propValue); - } else { - return filterValue.equalsIgnoreCase(propValue); - } - } - - /* - * Returns {@code true} if the attribute is a standard attribute; - * otherwise, returns {@code false}. - */ - private static boolean isStandardAttr(String attribute) { - // For now, we just have two standard attributes: - // KeySize and ImplementedIn. - if (attribute.equalsIgnoreCase("KeySize")) - return true; - - return attribute.equalsIgnoreCase("ImplementedIn"); - } - - /* - * Returns {@code true} if the requested attribute value is supported; - * otherwise, returns {@code false}. - */ - private static boolean isConstraintSatisfied(String attribute, - String value, - String prop) { - // For KeySize, prop is the max key size the - // provider supports for a specific .. - if (attribute.equalsIgnoreCase("KeySize")) { - int requestedSize = Integer.parseInt(value); - int maxSize = Integer.parseInt(prop); - return requestedSize <= maxSize; - } - - // For Type, prop is the type of the implementation - // for a specific .. - if (attribute.equalsIgnoreCase("ImplementedIn")) { - return value.equalsIgnoreCase(prop); - } - - return false; - } - - static String[] getFilterComponents(String filterKey, String filterValue) { - int algIndex = filterKey.indexOf('.'); - - if (algIndex < 0) { - // There must be a dot in the filter, and the dot - // shouldn't be at the beginning of this string. - throw new InvalidParameterException("Invalid filter"); - } - - String serviceName = filterKey.substring(0, algIndex); - String algName; - String attrName = null; - - if (filterValue.isEmpty()) { - // The filterValue is an empty string. So the filterKey - // should be in the format of .. - algName = filterKey.substring(algIndex + 1).trim(); - if (algName.isEmpty()) { - // There must be an algorithm or type name. - throw new InvalidParameterException("Invalid filter"); - } - } else { - // The filterValue is a non-empty string. So the filterKey must be - // in the format of - // . - int attrIndex = filterKey.indexOf(' '); - - if (attrIndex == -1) { - // There is no attribute name in the filter. - throw new InvalidParameterException("Invalid filter"); - } else { - attrName = filterKey.substring(attrIndex + 1).trim(); - if (attrName.isEmpty()) { - // There is no attribute name in the filter. - throw new InvalidParameterException("Invalid filter"); + if (propValue == null) { + // The provider doesn't have the given + // key in its property list. + return false; } } - // There must be an algorithm name in the filter. - if ((attrIndex < algIndex) || - (algIndex == attrIndex - 1)) { - throw new InvalidParameterException("Invalid filter"); + // If the key is in the format of: + // ., + // there is no need to check the value. + if (attrName == null) { + return true; + } + + // If we get here, the key must be in the + // format of . . + + // Check the "Java Security Standard Algorithm Names" guide for the + // list of supported Service Attributes + + // For KeySize, prop is the max key size the provider supports + // for a specific .. + if (attrName.equalsIgnoreCase("KeySize")) { + int requestedSize = Integer.parseInt(attrValue); + int maxSize = Integer.parseInt(propValue); + return requestedSize <= maxSize; + } + + // Handle attributes with composite values + if (isCompositeValue()) { + String attrValue2 = attrValue.toUpperCase(Locale.ENGLISH); + propValue = propValue.toUpperCase(Locale.ENGLISH); + + // match value to the property components + String[] propComponents = propValue.split("\\|"); + for (String pc : propComponents) { + if (attrValue2.equals(pc)) return true; + } + return false; } else { - algName = filterKey.substring(algIndex + 1, attrIndex); + // direct string compare (ignore case) + return attrValue.equalsIgnoreCase(propValue); } } - - String[] result = new String[3]; - result[0] = serviceName; - result[1] = algName; - result[2] = attrName; - - return result; } /** diff --git a/test/jdk/java/security/Security/ProviderFiltering.java b/test/jdk/java/security/Security/ProviderFiltering.java new file mode 100644 index 00000000000..f0ed1c2ea6b --- /dev/null +++ b/test/jdk/java/security/Security/ProviderFiltering.java @@ -0,0 +1,156 @@ +/* + * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @library /test/lib + * @bug 6447816 + * @summary Check that provider service matching/filtering is done correctly + * @run main/othervm ProviderFiltering + */ +import jdk.test.lib.Utils; +import java.util.*; +import java.security.*; + +public class ProviderFiltering { + + private static void doit(Object filter, String... expectedPNs) { + System.out.println("Filter: " + filter); + System.out.println("Expected Provider(s): " + + (expectedPNs.length > 0 ? Arrays.toString(expectedPNs) : + "")); + Provider ps[]; + if (filter instanceof String filterStr) { + ps = Security.getProviders(filterStr); + } else if (filter instanceof Map filterMap) { + ps = Security.getProviders(filterMap); + } else { + throw new RuntimeException("Error: unknown input type: " + filter); + } + + if (ps == null) { + if (expectedPNs.length != 0) { + throw new RuntimeException("Fail: expected provider(s) " + + "not found"); + } + } else { + if (ps.length == expectedPNs.length) { + // check the provider names + for (int i = 0; i < ps.length; i++) { + if (!ps[i].getName().equals(expectedPNs[i])) { + throw new RuntimeException("Fail: provider name " + + "mismatch at index " + i + ", got " + + ps[i].getName()); + } + } + } else { + throw new RuntimeException("Fail: # of providers mismatch"); + } + } + System.out.println("=> Passed"); + } + + + public static void main(String[] args) + throws NoSuchAlgorithmException { + // test filter parsing + String[] invalidFilters = { "", "Cipher.", ".RC2 ", "Cipher.RC2 :", + "Cipher.RC2 a: ", "Cipher.RC2 :b", + "Cipher.RC2 SupportedKeyClasses:a|b" + }; + for (String i : invalidFilters) { + System.out.println("Testing IPE for :" + i); + Utils.runAndCheckException(()-> Security.getProviders(i), + InvalidParameterException.class); + } + + String p = "SUN"; + + // test alias + doit("Signature.NONEwithDSA", p); + + String sigService = "Signature.SHA256withDSA"; + // javadoc allows extra spaces in between + String key = sigService + " SupportedKeyClasses"; + String valComp1 = "java.security.interfaces.DSAPublicKey"; + String valComp2 = "java.security.interfaces.DSAPrivateKey"; + String valComp2CN = valComp2.substring(valComp2.lastIndexOf('.') + 1); + + // test using String filter + doit(key + ":" + valComp1, p); + doit(key + ":" + valComp2, p); + // current impl does matching on individual attribute value for + // attributes with composite values; no partial match + doit(key + ":" + valComp2CN); + + // repeat above tests using filter Map + Map filters = new HashMap<>(); + // match existing behavior; return null if empty filter map + doit(filters); + filters.put(key, valComp1); + doit(filters, p); + filters.put(key, valComp2); + doit(filters, p); + filters.put(key, valComp2CN); + doit(filters); + + // try non-attribute filters + filters.clear(); + filters.put(sigService, ""); + doit(filters, p); + filters.put("Cipher.RC2", ""); + doit(filters); + + // test against a custom provider and attribute + filters.clear(); + String customKey = "customAttr"; + String customValue = "customValue"; + String pName = "TestProv"; + Provider testProv = new TestProvider(pName, sigService, customKey, + customValue); + Security.insertProviderAt(testProv, 1); + // should find both TestProv and SUN and in this order + doit(sigService, pName, "SUN"); + filters.put(sigService, ""); + doit(filters, pName, "SUN"); + + String specAttr = sigService + " " + customKey + ":" + customValue; + // should find only TestProv + doit(specAttr, pName); + filters.put(sigService + " " + customKey, customValue); + doit(filters, pName); + + // should find no provider now that TestProv is removed + Security.removeProvider(pName); + doit(specAttr); + } + + private static class TestProvider extends Provider { + TestProvider(String name, String service, String attrKey, + String attrValue) { + super(name, "0.0", "Not for use in production systems!"); + put(service, "a.b.c"); + put(service + " " + attrKey, attrValue); + } + } +}