diff --git a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java index b9d811891..4332a3cde 100644 --- a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java +++ b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java @@ -273,6 +273,34 @@ public static Stream authzRequests() { .operation("mqtt:publish") .resource("mqtt:topic:myThing/world") .expectedResult(false) + .build(), + // single character eval not supported by default + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:subscribe") + .resource("mqtt:topic:myThing/#/test/abc") + .expectedResult(false) + .build(), + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:subscribe") + .resource("mqtt:topic:myThing/#/test/???") + .expectedResult(true) + .build() + )), + + Arguments.of("single-character-wildcards-in-resource.yaml", Arrays.asList( + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:subscribe") + .resource("mqtt:topic:myThing/abc/test/a/b") + .expectedResult(true) + .build(), + AuthZRequest.builder() + .thingName("myThing") + .operation("mqtt:subscribe") + .resource("mqtt:topic:myThing/abcd/test/a/b") + .expectedResult(false) .build() )), diff --git a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/single-character-wildcards-in-resource.yaml b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/single-character-wildcards-in-resource.yaml new file mode 100644 index 000000000..bcc31ac1e --- /dev/null +++ b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/single-character-wildcards-in-resource.yaml @@ -0,0 +1,29 @@ +--- +services: + aws.greengrass.Nucleus: + configuration: + runWithDefault: + posixUser: nobody + windowsUser: integ-tester + logging: + level: "DEBUG" + aws.greengrass.clientdevices.Auth: + configuration: + enableSingleCharacterWildcardMatching: true + deviceGroups: + formatVersion: "2021-03-05" + definitions: + myThing: + selectionRule: "thingName: myThing" + policyName: "thingAccessPolicy" + policies: + thingAccessPolicy: + subscribe: + statementDescription: "mqtt subscribe" + operations: + - "mqtt:subscribe" + resources: + - "mqtt:topic:${iot:Connection.Thing.ThingName}/???/test/*" + main: + dependencies: + - aws.greengrass.clientdevices.Auth diff --git a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml index c52a1035e..f7981161a 100644 --- a/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml +++ b/src/integrationtests/resources/com/aws/greengrass/integrationtests/policy/wildcards-in-resource-type.yaml @@ -17,12 +17,18 @@ services: policyName: "thingAccessPolicy" policies: thingAccessPolicy: - policyStatement: + publish: statementDescription: "mqtt publish" operations: - "mqtt:publish" resources: - "mqtt:topic:*/myThing/*" + subscribe: + statementDescription: "mqtt subscribe" + operations: + - "mqtt:subscribe" + resources: + - "mqtt:topic:myThing/#/test/???" main: dependencies: - aws.greengrass.clientdevices.Auth diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java b/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java index 69b079002..b80e35558 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java @@ -170,6 +170,8 @@ private void subscribeToConfigChanges() { private void onConfigurationChanged() { try { cdaConfiguration = CDAConfiguration.from(cdaConfiguration, getConfig()); + // TODO decouple + context.get(PermissionEvaluationUtils.class).setCdaConfiguration(cdaConfiguration); } catch (URISyntaxException e) { serviceErrored(e); } diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java index 624b10c13..8c68d929c 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java @@ -5,15 +5,17 @@ package com.aws.greengrass.clientdevices.auth; -import com.aws.greengrass.authorization.WildcardTrie; +import com.aws.greengrass.clientdevices.auth.configuration.CDAConfiguration; import com.aws.greengrass.clientdevices.auth.configuration.GroupManager; import com.aws.greengrass.clientdevices.auth.configuration.Permission; import com.aws.greengrass.clientdevices.auth.exception.PolicyException; import com.aws.greengrass.clientdevices.auth.session.Session; +import com.aws.greengrass.clientdevices.auth.util.WildcardTrie; import com.aws.greengrass.logging.api.Logger; import com.aws.greengrass.logging.impl.LogManager; import com.aws.greengrass.util.Utils; import lombok.Builder; +import lombok.Setter; import lombok.Value; import org.apache.commons.lang3.StringUtils; @@ -36,6 +38,8 @@ public final class PermissionEvaluationUtils { "Resource is malformed, must be of the form: " + "([a-zA-Z]+):([a-zA-Z]+):" + RESOURCE_NAME_PATTERN.pattern(); private final GroupManager groupManager; + @Setter + private volatile CDAConfiguration cdaConfiguration; /** * Constructor for PermissionEvaluationUtils. @@ -133,10 +137,16 @@ private boolean compareResource(Resource requestResource, String policyResource) if (Objects.equals(requestResource.getResourceStr(), policyResource)) { return true; } + return new WildcardTrie(wildcardOpts()) + .withPattern(policyResource) + .matches(requestResource.getResourceStr()); + } - WildcardTrie wildcardTrie = new WildcardTrie(); - wildcardTrie.add(policyResource); - return wildcardTrie.matchesStandard(requestResource.getResourceStr()); + private WildcardTrie.MatchOptions wildcardOpts() { + CDAConfiguration config = cdaConfiguration; + return WildcardTrie.MatchOptions.builder() + .useSingleCharWildcard(config != null && config.isMatchSingleCharacterWildcard()) + .build(); } private Operation parseOperation(String operationStr) throws PolicyException { diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java b/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java index 3f3a27fd3..69610d27f 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/configuration/CDAConfiguration.java @@ -10,6 +10,8 @@ import com.aws.greengrass.clientdevices.auth.configuration.events.MetricsConfigurationChanged; import com.aws.greengrass.clientdevices.auth.configuration.events.SecurityConfigurationChanged; import com.aws.greengrass.config.Topics; +import com.aws.greengrass.util.Coerce; +import lombok.Builder; import lombok.Getter; import java.net.URISyntaxException; @@ -48,23 +50,19 @@ * | *

*/ +@Builder public final class CDAConfiguration { + public static final String ENABLE_MQTT_WILDCARD_EVALUATION = "enableSingleCharacterWildcardMatching"; + + private final DomainEvents domainEvents; private final RuntimeConfiguration runtime; - private final SecurityConfiguration security; @Getter private final CAConfiguration certificateAuthorityConfiguration; + private final SecurityConfiguration security; private final MetricsConfiguration metricsConfiguration; - private final DomainEvents domainEvents; - - private CDAConfiguration(DomainEvents domainEvents, RuntimeConfiguration runtime, CAConfiguration ca, - SecurityConfiguration security, MetricsConfiguration metricsConfiguration) { - this.domainEvents = domainEvents; - this.runtime = runtime; - this.security = security; - this.certificateAuthorityConfiguration = ca; - this.metricsConfiguration = metricsConfiguration; - } + @Getter + private final boolean matchSingleCharacterWildcard; /** * Creates the CDA (Client Device Auth) Service configuration. And allows it to be available in the context with the @@ -80,9 +78,15 @@ public static CDAConfiguration from(CDAConfiguration existingConfig, Topics topi DomainEvents domainEvents = topics.getContext().get(DomainEvents.class); - CDAConfiguration newConfig = new CDAConfiguration(domainEvents, RuntimeConfiguration.from(runtimeTopics), - CAConfiguration.from(serviceConfiguration), SecurityConfiguration.from(serviceConfiguration), - MetricsConfiguration.from(serviceConfiguration)); + CDAConfiguration newConfig = CDAConfiguration.builder() + .domainEvents(domainEvents) + .runtime(RuntimeConfiguration.from(runtimeTopics)) + .certificateAuthorityConfiguration(CAConfiguration.from(serviceConfiguration)) + .security(SecurityConfiguration.from(serviceConfiguration)) + .metricsConfiguration(MetricsConfiguration.from(serviceConfiguration)) + .matchSingleCharacterWildcard( + Coerce.toBoolean(serviceConfiguration.find(ENABLE_MQTT_WILDCARD_EVALUATION))) + .build(); newConfig.triggerChanges(newConfig, existingConfig); diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java b/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java new file mode 100644 index 000000000..d4446d95b --- /dev/null +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrie.java @@ -0,0 +1,220 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.aws.greengrass.clientdevices.auth.util; + + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import lombok.Builder; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; +import lombok.Value; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.function.Supplier; + +@RequiredArgsConstructor +public class WildcardTrie { + + private static final Function EXCEPTION_UNSUPPORTED_WILDCARD_TYPE = wildcardType -> + new UnsupportedOperationException("wildcard type " + wildcardType.name() + " not supported"); + + private Node root; + + private final MatchOptions opts; + + private static String cleanPattern(@NonNull String s) { + // for example "abc***def" can be reduced to "abc*def" + return s.replaceAll(String.format("\\%s+", WildcardType.GLOB.val), WildcardType.GLOB.val); + } + + public WildcardTrie withPattern(@NonNull String s) { + root = new Node(); + withPattern(root, cleanPattern(s)); + return this; + } + + private Node withPattern(@NonNull Node n, @NonNull String s) { + StringBuilder token = new StringBuilder(s.length()); + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + if (isWildcard(s.charAt(i))) { + // create child node from non-wildcard chars that have been accumulated so far + Node node = token.length() > 0 ? n.children.get(token.toString()) : n; + // create child node for wildcard char itself + WildcardType type = WildcardType.from(c); + node = node.children.get(type.val()); + node.wildcardType = type; + if (i == s.length() - 1) { + // we've reached the last token + return node; + } + return withPattern(node, s.substring(i + 1)); + } else { + token.append(c); + } + } + // use remaining (non-wildcard) chars as last token + if (token.length() > 0) { + return n.children.get(token.toString()); + } else { + return n; + } + } + + private boolean isWildcard(char c) { + WildcardType type = WildcardType.from(c); + if (type == null) { + return false; + } + if (type == WildcardType.SINGLE) { + return opts.useSingleCharWildcard; + } + return true; + } + + public boolean matches(@NonNull String s) { + if (root == null) { + return s.isEmpty(); + } + return matches(root, s); + } + + private boolean matches(@NonNull Node n, @NonNull String s) { + if (n.isTerminal()) { + return n.wildcardType == WildcardType.GLOB || s.isEmpty(); + } + + if (n.isWildcard()) { + switch (n.wildcardType) { + case SINGLE: + return n.children.keySet().stream().anyMatch(token -> { + Node child = n.children.get(token); + // skip over one character for single wildcard + if (child.isWildcard()) { + return !s.isEmpty() && matches(child, s.substring(1)); + } else { + return !s.isEmpty() && s.startsWith(token.substring(0, 1)) && matches(child, s.substring(1)); + } + }); + case GLOB: + return n.children.keySet().stream().anyMatch(token -> { + Node child = n.children.get(token); + if (child.isWildcard()) { + return true;// TODO + } else { + // consume the input string to find a match + return allIndicesOf(s, token).stream() + .anyMatch(tokenIndex -> + matches(child, s.substring(tokenIndex + token.length())) + ); + } + }); + default: + throw EXCEPTION_UNSUPPORTED_WILDCARD_TYPE.apply(n.wildcardType); + } + } + + return n.children.keySet().stream().anyMatch(token -> { + Node child = n.children.get(token); + if (child.isWildcard()) { + switch (child.wildcardType) { + case SINGLE: + // skip past the next character for ? matching + return !s.isEmpty() && matches(child, s.substring(1)); + case GLOB: + // skip past token and figure out retroactively if the glob matched + return matches(child, s); + default: + throw EXCEPTION_UNSUPPORTED_WILDCARD_TYPE.apply(child.wildcardType); + } + } else { + // match found, keep following this trie branch + return s.startsWith(token) && matches(child, s.substring(token.length())); + } + }); + } + + private static List allIndicesOf(@NonNull String s, @NonNull String sub) { + List indices = new ArrayList<>(); + int i = s.indexOf(sub); + while (i >= 0) { + indices.add(i); + i = s.indexOf(sub, i + sub.length()); + } + return indices; + } + + @Value + @Builder + public static class MatchOptions { + boolean useSingleCharWildcard; + } + + enum WildcardType { + GLOB("*"), + SINGLE("?"); + + private final String val; + + WildcardType(@NonNull String val) { + this.val = val; + } + + public static WildcardType from(char c) { + return Arrays.stream(WildcardType.values()) + .filter(t -> t.charVal() == c) + .findFirst() + .orElse(null); + } + + public String val() { + return val; + } + + public char charVal() { + return val.charAt(0); + } + } + + private class Node { + private final Map children = new DefaultHashMap<>(Node::new); + private WildcardType wildcardType; + + public boolean isWildcard() { + return wildcardType != null; + } + + public boolean isTerminal() { + return children.isEmpty(); + } + } + + @SuppressFBWarnings("EQ_DOESNT_OVERRIDE_EQUALS") + private static class DefaultHashMap extends HashMap { + private final transient Supplier defaultVal; + + public DefaultHashMap(Supplier defaultVal) { + super(); + this.defaultVal = defaultVal; + } + + @Override + @SuppressWarnings("unchecked") + public V get(Object key) { + return super.computeIfAbsent((K) key, (k) -> defaultVal.get()); + } + + @Override + public boolean containsKey(Object key) { + return super.get(key) != null; + } + } +} diff --git a/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java b/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java new file mode 100644 index 000000000..a4ddf1f76 --- /dev/null +++ b/src/test/java/com/aws/greengrass/clientdevices/auth/util/WildcardTrieTest.java @@ -0,0 +1,91 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.aws.greengrass.clientdevices.auth.util; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.List; +import java.util.stream.Stream; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.params.provider.Arguments.arguments; + + +class WildcardTrieTest { + + static Stream validMatches() { + return Stream.of( + arguments("foo", singletonList("foo")), + arguments("foo/bar", singletonList("foo/bar")), + // glob wildcard * + arguments("*", asList("foo", "foo/bar", "foo/bar/baz", "$foo/bar", "foo*", "foo?", "***", "???")), + arguments("*test", asList("test", "*test", "**test", "testtest", "test*test")), + arguments("test*", asList("test", "test*", "testA", "test**")), + arguments("test*test", asList("testtest", "testAtest", "test*test")), + arguments("test*test*", asList("testtest", "testtesttesttest", "test*test*")), + arguments("*test*test", asList("Atesttest", "testAtest", "AtestAtest", "testtest", "*test*test")), + arguments("*test*test*", asList("testtest", "*test*test*", "Atesttest", "testAtest", "testtestA", "AtestAtest", "testAtestA", "AtestAtestA")), + // single character wildcard ? + arguments("?", asList("f", "*", "?")), + arguments("??", asList("ff", "**", "??", "*?")), + arguments("?f?", asList("fff", "*f*")), + // glob and single + arguments("?*", asList("?", "*", "a", "??", "**", "ab", "***", "???", "abc")), + arguments("*?", asList("?", "*", "a", "??", "**", "ab", "***", "???", "abc")), + arguments("?*?", asList("??", "**", "aa", "???", "***", "aaa", "????", "****", "aaaa")), + arguments("*?*", asList("?", "*", "a", "??", "**", "ab", "***", "???", "abc")), + arguments("a?*b", asList("acb", "a?b", "a*b", "a?cb")), + arguments("a*?b", asList("acb", "a?b", "a*b", "a?cb")) + ); + } + + @MethodSource("validMatches") + @ParameterizedTest + void GIVEN_trie_with_wildcards_WHEN_valid_matches_provided_THEN_pass(String pattern, List matches) { + WildcardTrie.MatchOptions opts = WildcardTrie.MatchOptions.builder().useSingleCharWildcard(true).build(); + WildcardTrie trie = new WildcardTrie(opts).withPattern(pattern); + matches.forEach(m -> assertTrue(trie.matches(m), + String.format("String \"%s\" did not match the pattern \"%s\"", m, pattern))); + } + + + static Stream invalidMatches() { + return Stream.of( + arguments("foo", singletonList("bar")), + arguments("foo/bar", singletonList("bar/foo")), + // glob wildcard * + arguments("*test", asList("testA", "*testA", "AtestA", "bar")), + arguments("test*", asList("Atest", "Atest*", "AtestA", "Atest**", "bar")), + arguments("test*test", asList("Atesttest", "AtestAtest", "testAtestA", "testbar")), + arguments("test*test*", asList("Atesttest", "AtestAtest", "AtestAtestA", "testbar")), + arguments("*test*test", asList("AtestAtestA", "test*bar", "bartest", "testbar")), + arguments("*test*test*", asList("AtestAbar", "testbar", "bartest")), + // single character wildcard ? + arguments("?", asList("aa", "??", "**")), + arguments("??", asList("aaa", "???", "***")), + arguments("?a?", asList("fff", "aaaa")), + arguments("?f?", asList("ff", "f", "*f", "f*")), + // glob and single + arguments("?*?", asList("a", "?", "*")), + arguments("a?*b", asList("ab", "abc")), + arguments("a*?b", asList("ab", "abc")) + ); + } + + @MethodSource("invalidMatches") + @ParameterizedTest + void GIVEN_trie_with_wildcards_WHEN_invalid_matches_provided_THEN_fail(String pattern, List matches) { + WildcardTrie.MatchOptions opts = WildcardTrie.MatchOptions.builder().useSingleCharWildcard(true).build(); + WildcardTrie trie = new WildcardTrie(opts).withPattern(pattern); + matches.forEach(m -> assertFalse(trie.matches(m), + String.format("String \"%s\" incorrectly matched the pattern \"%s\"", m, pattern))); + } +}