unbridled-41 opened a new pull request, #2833:
URL: https://github.com/apache/rocketmq-dashboard/pull/2833
## What is the purpose of the change
Fixes #2832.
PR #2339 added a broker-topology guard (`validatedBrokerAddr`) for offset
message ids, but it only covered the fallback path (`viewMessageByOffsetId`).
The primary lookup path — `adminExt.viewMessage(topic, msgId)` in
`queryByMsgId` and `resolveMessageStoreTimestamp` — runs **before** the
fallback with no validation. Since `MQAdminImpl.viewMessage` decodes the broker
address embedded in a 32-char offset id and opens a RocketMQ remoting
connection to it, a forged msgId still made Studio connect to an
attacker-chosen `ip:port` (reader-accessible via `GET /api/messages` and `GET
/api/messages/{msgId}/trace`), which is the exact primitive `ToolAccessPolicy`
deny-lists for readers.
## Brief changelog
- Add `isWithinKnownBrokerTopology`: decode the msgId and validate the
embedded broker address against the instance topology with the existing
`validatedBrokerAddr` helper before invoking `adminExt.viewMessage(topic,
msgId)`.
- Apply the guard in both `queryByMsgId` and `resolveMessageStoreTimestamp`.
- Ids that do not decode as offset ids are unaffected: `MQAdminImpl` falls
back to its unique-key lookup, which resolves brokers from the topic route and
is not steerable by the id.
- Strengthen the two guard tests to verify `adminExt.viewMessage` is never
invoked for out-of-topology ids (previously they stubbed `viewMessage` to
throw, which masked the primary-path connection), and add a positive-path test
proving in-topology ids still resolve through `viewMessage`.
## Verifying this change
- `mvn -f server/pom.xml -Dtest=RocketMQMessageProviderTest test` — 32 tests
pass.
- Red/green check: with the production change stashed, the strengthened
`queryByMsgIdRejectsDecodedBrokerOutsideKnownTopology` and
`getMessageTraceDoesNotUseDecodedBrokerOutsideKnownTopology` both fail (the
primary path does invoke `viewMessage` for an out-of-topology address); with
the fix they pass.
- Full `mvn test` on `server`: 1789 tests, only pre-existing unrelated flake
in `OpenAiCompatibleLlmGatewayTest` (passes when its class is run in isolation,
fails identically on the unmodified branch).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
