[FIXED] LeafNode: Publish permissions checked against pre-transform subject - #8530
[FIXED] LeafNode: Publish permissions checked against pre-transform subject#8530fbarth-ifr wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d42f99814e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| subjectToCheck, _ := getGWRoutedSubjectOrSelf(subject) | ||
| subjectToCheck = sub.importTargetSubject(subjectToCheck) |
There was a problem hiding this comment.
When a leaf shadow subscription applies a stream-import transform to a gateway-routed subject—for example, an import from > to local.>—the header path transforms the complete subject and emits local._GR_.<hashes>.foo, while this permission path strips _GR_.<hashes>. first and checks local.foo. The leaf can therefore authorize a different subject than it places on the wire; the receiving hub may reject that wire subject and close the leaf connection, and local deny rules are evaluated against the wrong value. Apply the import transform first, then strip a gateway prefix only if the resulting wire subject still has one.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This finding is real. I changed the order in which transformation and gateway-strip are applied and added a regression test.
…ubject A shadow subscription created by a stream import delivers the import's post-transform subject to the leafnode. The publish permission check in deliverMsg() was performed against the exporting account's subject instead, which is never sent on the wire. As a result, the pre-transform subject had to be added to the allow list for messages to be delivered at all, which grants the leafnode publish permissions beyond what is intended to travel over the connection. Extract the remap that msgHeaderForRouteOrLeaf() already performs into importTargetSubject() and use it for the permission check, so that both call sites derive the delivered subject the same way. The extracted logic is unchanged. Resolves nats-io#4608 Signed-off-by: Florian Barth <florian.barth@foerstergroup.com>
d42f998 to
d971835
Compare
1 participant
Resolves #4608
Problem
When a message is delivered to a leafnode, it may have undergone a subject transformation. Today, the code performs the publish permissions check against the original pre-transform subject, which is never sent to the leafnode.
This bug requires users to include both the pre- and post-transform subjects in the allow list for leafnodes to receive messages, which is a security concern because it widens the scope of allowed subjects beyond what is actually intended to travel through the leafnode connection.
Fix
The fix is to extract the logic from
msgHeaderForRouteOrLeaf(), which already correctly handles the post-transform subject, and use it indeliverMsg()for the publish permissions check when delivering messages to a leafnode. By extracting the pre-existing logic into its own function without changing semantics, we ensure it stays consistent between both call sites and always uses the post-transform subject for the check.Backwards Compatibility
This fix only removes the need to include the pre-transform subject in the allow list. It does not break existing setups, since such setups previously had to allow both the pre- and post-transform subjects for leafnodes to receive messages at all. Allowing only the post-transform subject results in the leaf server rejecting the message silently, apart from a debug log. Allowing only the pre-transform subject lets the message leave the leaf server, but the hub then rejects it with a
Publish Violationbefore closing the connection.The same holds for gateway routed subjects. The check now uses exactly the subject that goes on the wire, which is the subject the hub evaluates as well. Any setup in which a message actually reached the hub must therefore already allow that subject.
Regression Test
TestLeafNodePermsWithImportSubjectTransformverifies that a message with a transformed subjectTestLeafNodePermsWithImportSubjectTransformGatewayRoutedcovers a gateway routed subject. It verifies that the message