unbridled-41 opened a new pull request, #3112: URL: https://github.com/apache/rocketmq-dashboard/pull/3112
## What is the purpose of the change Follow-up to #2835, as requested by @lizhimins in https://github.com/apache/rocketmq-dashboard/pull/2835#issuecomment-5535499168. The selected DLQ resend lookup (`resendSelectedMessages`) calls `admin.viewMessage(dlqTopic, msgId)` with no address validation. Offset-style msgIds embed a broker address that `MQAdminImpl#viewMessage` decodes and connects to directly (`MessageDecoder.decodeMessageId` builds a literal `InetSocketAddress` from bytes 0-7 of the id — no DNS), so a forged id made the DLQ lookup open a RocketMQ remoting connection to an attacker-chosen `ip:port` — the same primitive #2833 (#2832) closed for the message query path and trace timestamp lookup. ## Brief changelog - Add `isWithinKnownBrokerTopology` to `RocketMQDLQProvider`, mirroring the merged implementation in `RocketMQMessageProvider` (same `decodedBrokerAddr`/`knownBrokerEndpoints` helpers, same warn log on rejection), and apply it in `resendSelectedMessages` before `admin.viewMessage(dlqTopic, msgId)`: - id decodes and the embedded address is outside the selected instance topology → skipped (logged, not resolved); - id decodes but the topology is empty → rejected; - topology verification itself fails (e.g. cluster-info RPC error) → rejected, fail-closed; - id does not decode as an offset id (unique-key form) → unchanged behavior, passes through to `viewMessage`, whose unique-key lookup resolves brokers from the topic route. - No changes to `RocketMQMessageProvider` or any other file; the guard helpers are deliberately scoped to the DLQ provider to keep this follow-up minimal (a shared extraction can be a separate cleanup if maintainers prefer). ## Multi-dimensional verification **Regression tests (new, in `RocketMQDLQProviderTest`):** | test | pins | |---|---| | `resendSelectedMessagesRejectsForgedMsgIdOutsideKnownTopology` | forged offset id (`10.2.3.4:10911`, topology = `172.30.10.100:10911`) → `viewMessage` never invoked, no producer dispatch, `matched=0` | | `resendSelectedMessagesResolvesInTopologyMsgIdNormally` | in-topology id → `viewMessage` invoked once, message resolved and resent, `outcome=SUCCESS` (behavior preservation) | | `resendSelectedMessagesPassesNonOffsetIdsThroughToViewMessage` | unique-key id → passes through to `viewMessage` with zero topology RPCs (behavior preservation) | | `resendSelectedMessagesFailsClosedWhenTopologyCannotBeVerified` | decodable id + `examineBrokerClusterInfo` failure → `viewMessage` never invoked (fail-closed) | **Red/green (tests against the unmodified provider):** | test | unmodified branch | with this fix | |---|---|---| | forged out-of-topology rejected | **FAIL** — `viewMessage` is invoked for the forged address | PASS | | fail-closed on topology failure | **FAIL** — `viewMessage` is invoked | PASS | | in-topology resolves normally | ERROR — strict stubs flags the topology stub as unused, i.e. the old code never consults the topology | PASS | | non-offset id passthrough | PASS | PASS | **Build & suite:** - `mvn -f server/pom.xml -Dtest=RocketMQDLQProviderTest test` — 32 tests pass (28 pre-existing + 4 new). - `mvn -B -ntp clean package -DskipTests` (CI backend-build step, includes checkstyle at validate phase) — SUCCESS. - Full `mvn -B test` on this branch: 1979 tests, single failure is the pre-existing load-sensitive race in `OpenAiCompatibleLlmGatewayTest` already characterized with control runs in #2833 — it fires on unmodified branches too and no code touched by this PR participates in that path. -- 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]
