Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: single character wildcard matching #437

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,34 @@ public static Stream<Arguments> 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()
)),

Expand Down
Original file line number Diff line number Diff line change
@@ -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/*"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the combination of single level ? and multi level * valid in the same level and should we test for it? Ex: mqtt:topic:myThing/???* or mqtt:topic:myThing/*???

Copy link
Member Author

@jcosentino11 jcosentino11 Sep 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Evaluation here is more regex-like than mqtt: * doesn't know anything about topic levels.

There should be test cases for this in WildcardTrieTest already (not all are passing quite yet)

main:
dependencies:
- aws.greengrass.clientdevices.Auth
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -48,23 +50,19 @@
* |
* </p>
*/
@Builder

Check notice

Code scanning / CodeQL

Use of default toString() Note

Default toString(): MetricsConfiguration inherits toString() from Object, and so is not suitable for printing.
Default toString(): SecurityConfiguration inherits toString() from Object, and so is not suitable for printing.
Default toString(): CAConfiguration inherits toString() from Object, and so is not suitable for printing.
Default toString(): RuntimeConfiguration inherits toString() from Object, and so is not suitable for printing.
Default toString(): DomainEvents inherits toString() from Object, and so is not suitable for printing.
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
Expand All @@ -80,9 +78,15 @@

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);

Expand Down
Loading
Loading