pjfanning opened a new pull request, #3505:
URL: https://github.com/apache/pekko/pull/3505
### Motivation
Three sites parse HOCON that arrived in a message, using the default parse
options:
| Site | Reached by |
| --- | --- |
| `ClusterMessageSerializer:302` — `deserializeInitJoin` | `InitJoin`, from
a node that has not joined |
| `ClusterMessageSerializer:313` — `deserializeInitJoinAck` | `InitJoinAck`,
from a claimed seed node |
| `MiscMessageSerializer:549` — `deserializeConfig` | any `Config` payload
from an associated peer |
HOCON `include` directives are resolved by the *parser*, not by `resolve()`,
so they take
effect as soon as the message is deserialized — before `ClusterDaemon` forms
any opinion
about the sender. `InitJoin` is accepted in the `uninitialized` state
(`ClusterDaemon.scala:501`), so for that one the sender need not be a
cluster member at all.
That gives a peer a blind SSRF primitive from inside the node (`include
url("http://…";)`
reaches addresses the sender cannot) and a forced read of local files and
classpath
resources.
I checked the behaviour against config 1.4.6 rather than assuming it, and
the result changed
the shape of the fix. A plain `ConfigIncluder` is **not** enough — the
parser falls back to
its own handling, which does read the resource, for any typed form the
includer does not
implement:
```
include form defaults plain ConfigIncluder all four interfaces
include file(...) reads reads blocked
include required(file) reads reads blocked
include url(...) reads reads blocked
include classpath(...) reads reads blocked
include "..." no-op blocked blocked
```
(Bare `include "..."` is already inert here: `SimpleIncluder` resolves it
relative to the
including source, and a string has none.)
### Modification
Add `org.apache.pekko.serialization.WireConfig` (`@InternalApi`), which
parses with
`ConfigParseOptions.defaults().setIncluder(...)` where the includer resolves
every form to an
empty object. It implements `ConfigIncluder`, `ConfigIncluderFile`,
`ConfigIncluderURL` and
`ConfigIncluderClasspath` — all four are needed, per the table above. Route
the three sites
through it.
Every serializer writes config with `ConfigRenderOptions.concise`
(`MiscMessageSerializer:172`,
`ClusterMessageSerializer:401`), which renders JSON and cannot produce an
`include`, so a
well-behaved sender loses nothing and no rolling-upgrade path is affected.
Includes are dropped silently rather than logged. `deserializeConfig` is on
the ordinary
per-message path, so a peer-triggerable log line there is its own small
flooding surface, and
the drop is already fail-safe. Happy to add a `LogMarker.Security` debug
line if reviewers
would rather have the signal.
### Result
Deserializing a message no longer reads local files or classpath resources,
or issues
outbound requests, on behalf of the sender.
### Tests
- `sbt "actor-tests/testOnly org.apache.pekko.serialization.WireConfigSpec"`
— 7 passed: ordinary
HOCON and concise-rendered config still parse; `file`, `required(file)`,
`url` and `classpath`
includes all resolve to nothing; and one test asserts the *default* parser
does resolve all
three, so the others are testing the change rather than an inert directive
- `sbt "cluster/testOnly
org.apache.pekko.cluster.protobuf.ClusterMessageSerializerSpec"` — 10 passed,
including a new test covering both `InitJoin` and `InitJoinAck`
- `sbt "remote/testOnly
org.apache.pekko.remote.serialization.MiscMessageSerializerSpec"` — 104 passed,
including a new test for the `Config` payload
- Both new module tests were checked to discriminate by reverting the
production change and
re-running: each fails with `true did not equal false`
- `sbt "actor/mimaReportBinaryIssues" "remote/mimaReportBinaryIssues"
"cluster/mimaReportBinaryIssues"` — no issues
- `sbt scalafmtAll headerCreateAll` — no changes
### References
None. `WireConfig.scala` is new code and carries the standard ASF header.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
