Skip to content

add ACL support, fix wildcard routing and QoS >0 crash - #47

Open
durrendal wants to merge 1 commit into
python36:masterfrom
durrendal:acl-qos-wildcard
Open

durrendal wants to merge 1 commit into
python36:masterfrom
durrendal:acl-qos-wildcard

Conversation

@durrendal

@durrendal durrendal commented Feb 13, 2026 •

Copy link
Copy Markdown

This change adds Mosquitto compatible ACL file support (user/topic rules, pattern rules with %u/%c substitution).

Replaces broken topic routing with trie-based index. The broker previously only matched the literal "#" and exact topic string, so subscribers on filters like foo/# or foo/+/bar never received messages. The trie resolves matches in O(topic depth) instead of O(subscription count).

Replaces assert statements in onPubAck, onPubRec, onPubRel, and onPubComp with graceful checks. The asserts crash the broker under normal QoS > 0 operation when work queue state doesn't match expectations due to timing or msgId reuse.

2026-02-12-233438_1366x768_scrot

For color/context, I'm working on building a SaltStack like configuration management tool with nmqtt as the MQTT broker/client in that stack. Adding ACL handling and trie topic parsing will allow me to replace Mosquitto in that setup with a custom nmqtt based broker.

Tangentially, I've also ported nmqtt and its build deps to Alpine Linux as part of that same effort.

This change adds Mosquitto compatible ACL file support (user/topic rules, pattern rules with %u/%c substitution).

Replaces broken topic routing with trie-based index. The broker previously only matched the literal "#" and exact topic string, so subscribers on filters like foo/# or foo/+/bar never received messages. The trie resolves matches in O(topic depth) instead of O(subscription count).

Replaces assert statements in onPubAck, onPubRec, onPubRel, and onPubComp with graceful checks. The asserts crash the broker under normal QoS > 0 operation when work queue state doesn't match expectations due to timing or msgId reuse.
@python36

Copy link
Copy Markdown
Owner

Hi @durrendal,
Thank you for your contribution and for the detailed description of your changes!
Regarding the issues you mentioned (wildcard routing and QoS >0 crashes), we have already fixed them in previous commits.
As for adding ACL support, I don't think it is rational for this project. nmqtt is designed primarily as an educational and research broker, so it isn't meant to compete with fully-featured production solutions like Mosquitto.
However, if you are motivated to continue working on adding ACL into the broker, that would be awesome! To proceed with that, the code will need some adjustments. Below, I will separately add a minimal description of the tests that need to be implemented for this feature.
Let me know what you think, and thank you again for your effort!

# ==============================================================================
# PART 1: TEST ACL CONFIGURATION FILE (acl.conf)
# ==============================================================================
#
# General Default Policy:
# Any request for a topic not explicitly allowed by the rules is DENIED (Deny by default).
#
# --- RULES FOR ANONYMOUS (UNAUTHENTICATED) CLIENTS ---
# Active until the first `user` directive appears
#
# topic read public/#
# topic write public/incoming
# topic readwrite public/chat
# topic deny public/chat/admin_only
# topic deny private/#
# topic deny admin/#
#
# --- USER: alice (regular user) ---
# user alice
# topic deny home/alice/vault/#
# topic readwrite home/alice/#
# topic read sensors/#
# topic write alerts/user
# topic deny secret/#
#
# --- USER: bob (regular user) ---
# user bob
# topic readwrite home/bob/#
# topic read sensors/temperature
# topic deny home/alice/#
#
# --- USER: sensor_device (sensor service account, write-only) ---
# user sensor_device
# topic write sensors/#
# topic write telemetry/+
#
# --- USER: dashboard (monitoring service account, read-only) ---
# user dashboard
# topic read #
# topic deny admin/#
#
# --- USER: admin (full access, including system topics) ---
# user admin
# topic readwrite #
# topic readwrite $SYS/#
#
# --- USER: guest_user (user without rules defined in the file) ---
# user guest_user
# (no rules defined — fallback behavior verification)
#
# --- PATTERNS: %u = username, %c = clientid ---
# pattern readwrite user/%u/#
# pattern write device/%c/status
# pattern read device/%c/commands
# pattern deny device/%c/commands/restricted
#
# ==============================================================================
# PART 2: DETAILED DESCRIPTION OF ALL SCENARIOS AND TEST CASES
# ==============================================================================
#
# ------------------------------------------------------------------------------
# SCENARIO 1: Basic Read and Write Permissions (Read / Write / ReadWrite)
# ------------------------------------------------------------------------------
#
# 1.1. Successful publish and receive (ReadWrite)
# - Client `alice` subscribes to `home/alice/light`.
# - Client `alice` publishes "ON" to `home/alice/light`.
# - Expectations:
#   * Broker returns SUBACK with success code (0x00 / QoS 0).
#   * `alice` successfully receives the message "ON".
#
# 1.2. "Write-only" client attempts to subscribe
# - Client `sensor_device` (has `topic write sensors/#`) attempts to subscribe to `sensors/temp`.
# - Expectations:
#   * Broker returns SUBACK with failure code 0x80 (Failure).
#   * Even if another client publishes to `sensors/temp`, `sensor_device` receives nothing.
#
# 1.3. "Write-only" client successfully delivers message to subscribers
# - Client `alice` (has `topic read sensors/#`) is subscribed to `sensors/temp`.
# - Client `sensor_device` publishes "22.5" to `sensors/temp`.
# - Expectations:
#   * Message is accepted by the broker.
#   * `alice` receives "22.5".
#
# 1.4. "Read-only" client attempts to publish
# - Client `dashboard` (has `topic read #`) attempts to publish to `sensors/temp`.
# - Expectations:
#   * Broker drops the message.
#   * No subscriber (including `dashboard` itself) receives it.
#
# ------------------------------------------------------------------------------
# SCENARIO 2: Access Denial (Deny) and Rule Precedence
# ------------------------------------------------------------------------------
#
# 2.1. Explicit `deny` rule overrides a broader allowed rule with `#`
# - Context: `alice` has `topic read sensors/#`, but has `topic deny secret/#`.
# - Subscription: `alice` subscribes to `sensors/#` or `secret/keys`.
# - Action: `admin` publishes to `secret/keys` and to `sensors/temp`.
# - Expectations:
#   * Message from `sensors/temp` is successfully delivered to `alice`.
#   * Message from `secret/keys` for `alice` is filtered by the broker and NOT delivered.
#   * If `alice` attempts to explicitly subscribe only to `secret/keys`, the broker returns SUBACK 0x80.
#
# 2.2. Access denial to another user's private space
# - Client `bob` attempts to subscribe to `home/alice/bedroom` -> broker returns SUBACK 0x80.
# - Client `bob` attempts to publish to `home/alice/bedroom` -> broker drops the publish.
# - Subscriber `alice` receives nothing.
#
# 2.3. Rule evaluation order (Order of evaluation)
# - Context for `alice`:
#   1. `topic deny home/alice/vault/#`
#   2. `topic readwrite home/alice/#`
# - Action: `alice` attempts to read or write to `home/alice/vault/pin`.
# - Expectations: Denied (SUBACK 0x80 / Drop), as the deny rule is evaluated first.
# - Action: `alice` attempts to read or write to `home/alice/kitchen`.
# - Expectations: Allowed (SUBACK 0x00 / Delivered).
#
# ------------------------------------------------------------------------------
# SCENARIO 3: Topic Wildcards ('+' and '#')
# ------------------------------------------------------------------------------
#
# 3.1. Single-level wildcard `+`
# - Context: `sensor_device` has permission `topic write telemetry/+`.
# - Action 1: Publish to `telemetry/gps` (1 level) -> Allowed, message delivered.
# - Action 2: Publish to `telemetry/gps/speed` (2 levels) -> Denied, message dropped.
# - Action 3: Publish to `telemetry/` (empty level) -> Denied.
#
# 3.2. Multi-level wildcard `#`
# - Context: `alice` has permission `topic readwrite home/alice/#`.
# - Action: Publish to `home/alice/livingroom/tv/power/status` (any depth) -> Allowed.
#
# ------------------------------------------------------------------------------
# SCENARIO 4: Dynamic User and Client Patterns (Pattern %u and %c)
# ------------------------------------------------------------------------------
#
# 4.1. User space isolation by username `%u`
# - Context: `pattern readwrite user/%u/#`.
# - `alice` subscribes to `user/alice/data` -> Allowed (SUBACK 0x00).
# - `alice` publishes to `user/alice/data` -> Receives own message.
# - `bob` subscribes to `user/alice/data` -> Denied (SUBACK 0x80).
# - `bob` publishes to `user/alice/data` -> Denied (message dropped by broker).
# - `bob` subscribes and publishes to their own topic `user/bob/data` -> Allowed.
#
# 4.2. Device isolation by ClientID `%c`
# - Context: `pattern write device/%c/status` and `pattern read device/%c/commands`.
# - Client with ClientID = "ESP_01":
#   * Publishes to `device/ESP_01/status` -> Allowed.
#   * Attempts to publish to `device/ESP_02/status` -> Denied (dropped).
#   * Subscribes to `device/ESP_01/commands` -> Allowed (SUBACK 0x00).
#   * Subscribes to `device/ESP_02/commands` -> Denied (SUBACK 0x80).
#
# 4.3. Pattern edge case: Anonymous client (without username)
# - Context: `pattern readwrite user/%u/#`.
# - Anonymous client attempts to subscribe or publish to `user//data` or `user/anonymous/data`.
# - Expectations: Pattern `%u` must NOT apply to anonymous clients; access is strictly denied.
#
# 4.4. Pattern edge case: Client with auto-generated ClientID
# - Client connects with an empty ClientID (broker generates a random ID, e.g., `auto-7391`).
# - Expectations: `%c` must match specifically the broker-generated ID (`device/auto-7391/status`).
#
# 4.5. Pattern edge case: Special characters in username or ClientID
# - Client has username = "user/hack" or "user+admin".
# - Expectations: Characters `/`, `+`, `#` must not be interpreted as level separators
#   or topic wildcards when substituted into `%u` (protection against ACL injection).
#
# ------------------------------------------------------------------------------
# SCENARIO 5: Multi-subscription in a Single Packet (SUBSCRIBE with Multiple Topics)
# ------------------------------------------------------------------------------
#
# 5.1. SUBSCRIBE packet with mixed permissions
# - Client: `bob`.
# - Action: `bob` sends a single SUBSCRIBE packet with three topics simultaneously:
#   1. `sensors/temperature` (allowed, requested QoS = 1)
#   2. `home/alice/light`    (denied by deny rule, requested QoS = 2)
#   3. `home/bob/light`      (allowed, requested QoS = 0)
# - Expectations:
#   * Broker returns SUBACK with the corresponding array of codes for each topic:
#     Return codes: [0x01, 0x80, 0x00]
#   * Subscriptions to `sensors/temperature` and `home/bob/light` are successfully registered.
#   * Subscription to `home/alice/light` is rejected.
#
# ------------------------------------------------------------------------------
# SCENARIO 6: Broker Behavior on Unauthorized PUBLISH for Different QoS Levels
# ------------------------------------------------------------------------------
#
# 6.1. Unauthorized Publish with QoS 0
# - Unauthorized client sends Publish (QoS 0).
# - Expectations: Broker silently drops the message; no response is sent to the client.
#
# 6.2. Unauthorized Publish with QoS 1 (protection against client queue stall)
# - Unauthorized client sends Publish (QoS 1, packetId = 42) to an unauthorized topic.
# - Expectations:
#   * Broker drops the message body (subscribers do NOT receive it).
#   * Per MQTT 3.1.1 specification, the broker sends `PUBACK (packetId = 42)` to the client,
#     so that the client's inflight message queue does not get blocked.
#
# 6.3. Unauthorized Publish with QoS 2
# - Unauthorized client sends Publish (QoS 2, packetId = 99).
# - Expectations:
#   * Message is not delivered to any subscriber.
#   * Broker correctly completes the handshake (responds with PUBREC / PUBCOMP) or terminates
#     the connection (depending on the chosen security policy).
#
# ------------------------------------------------------------------------------
# SCENARIO 7: Default Fallback Policy / Whitelist
# ------------------------------------------------------------------------------
#
# 7.1. Request for a topic not present in user rules
# - Client `alice` attempts to publish or subscribe to `unconfigured/random/topic`.
# - Expectations: Denied (Deny by default). SUBACK 0x80 / drop publish.
#
# 7.2. User exists in password file but is completely absent from ACL file
# - Client `guest_user` successfully authenticated with username/password.
# - Attempted access to `public/chat` and `private/data`.
# - Expectations:
#   * `guest_user` does NOT have access to private topics.
#   * Has access only to generic rules (if allowed for all) or has no access at all
#     (depending on `allow_unmatched = false` configuration).
#
# ------------------------------------------------------------------------------
# SCENARIO 8: System Topics ($SYS/#)
# ------------------------------------------------------------------------------
#
# 8.1. System topics isolation from general `#` and `+` wildcards
# - Client `dashboard` has permission `topic read #`.
# - Client `dashboard` subscribes to `#`.
# - Broker publishes system metrics to `$SYS/broker/uptime`.
# - Expectations:
#   * Per MQTT specification, subscriptions to `#` or `+` at the root level must NOT
#     match topics starting with `$`.
#   * `dashboard` does NOT receive messages from `$SYS/...`.
#
# 8.2. Access to $SYS only via explicit permission
# - Client `admin` (has explicit rule `topic readwrite $SYS/#`) subscribes to `$SYS/#`.
# - Expectations: `admin` successfully receives system messages.
# - Regular user `alice` attempts to subscribe to `$SYS/#` -> rejected (SUBACK 0x80).
#
# ------------------------------------------------------------------------------
# SCENARIO 9: Anonymous vs Authenticated Users
# ------------------------------------------------------------------------------
#
# 9.1. Anonymous client operation in public topics
# - Anonymous client subscribes to `public/chat` and publishes a message -> Allowed.
#
# 9.2. Anonymous client attempts to publish to a read-only topic
# - Anonymous client attempts to publish to `public/news` (where `topic read public/#` applies) -> Blocked.
#
# 9.3. Anonymous client attempts to access private data
# - Anonymous client attempts to subscribe or publish to `home/alice/#` or `admin/#` ->
#   Rejected (SUBACK 0x80 / message dropped).
#
# ------------------------------------------------------------------------------
# SCENARIO 10: Retained Messages and Last Will and Testament (LWT / Will)
# ------------------------------------------------------------------------------
#
# 10.1. Retained message to an unauthorized topic
# - Client without write permissions sends a message with `retain = true` to `sensors/temperature`.
# - Expectations:
#   * Broker rejects the publication and does NOT store the message in the retained table.
#   * New clients subscribing do not receive this message.
#
# 10.2. Retained message for an unauthorized subscriber
# - A legitimate retained message from admin is stored in topic `secret/keys`.
# - Unauthorized client `bob` sends a subscription to `secret/keys` or `secret/#`.
# - Expectations:
#   * Subscription is rejected (SUBACK 0x80).
#   * Retained message is NOT sent to client `bob`.
#
# 10.3. ACL verification for Will (Last Will and Testament)
# - Client `bob` specifies Will topic `home/alice/alerts` during connection (in CONNECT packet).
# - Connection of `bob` drops unexpectedly.
# - Expectations:
#   * Before sending the Will, the broker checks `bob`'s write permissions for the Will topic.
#   * Since `bob` has no permissions for `home/alice/#`, the broker blocks sending this Will.
#
# ------------------------------------------------------------------------------
# SCENARIO 11: Parsing, Validation, and Configuration Errors
# ------------------------------------------------------------------------------
#
# 11.1. Ignoring comments and blank lines
# - Lines starting with `#` and whitespace-only lines must not cause parsing errors.
#
# 11.2. Whitespace and indentation
# - Lines such as `  topic   readwrite    public/chat  ` must parse correctly.
#
# 11.3. Missing access type in topic directive (default value)
# - A line `topic my/custom/topic` without permission specified should default
#   to `readwrite` permission.
#
# 11.4. Invalid topic syntax in configuration
# - Rule `topic read sensors/#/bad` -> configuration error (`#` character can only be at the end).
# - Broker must emit a clear warning/error when loading config.
#
# 11.5. Missing ACL file
# - If `nmqtt.conf` specifies `acl_file` but the file is not found, the broker must
#   safely report a configuration error and exit (or enter safe mode).
#
# ==============================================================================

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants