obabichevjb opened a new pull request, #16492:
URL: https://github.com/apache/dubbo/pull/16492
## What is the purpose of the change?
Fixes #16491.
`SerializeSecurityConfigurator` auto-allow-lists the classes it can reach
from an exported
interface's signatures. `checkClass` walks **up** into supertypes and
**into** fields and generic
arguments, but never **down** into subtypes. A sealed parameter or return
type therefore gets the
parent trusted and none of the subtypes that actually travel on the wire:
```java
public sealed interface Reply extends Serializable permits Found, NotFound {}
public final class Found implements Reply { /* ... */ }
public final class NotFound implements Reply { /* ... */ }
public interface ReplyService {
Reply lookup(String key); // returns Found or NotFound
}
```
`Reply` and `ReplyService` land in the allow list; `Found` and `NotFound` do
not. In the default
`STRICT` mode the provider then fails to serialize its own response:
```
[Serialization Security] Serialized class Found is not in allow list.
Current mode is `STRICT`, will disallow to deserialize it by default.
```
The consumer sees that wrapped in a `RemotingException`, which makes it read
like a transport
problem rather than a configuration one.
The developer then has to list every subtype by hand and keep that list in
step with the hierarchy —
for a set the compiler already knows in full.
This change walks the subtypes a sealed type permits, so they are trusted
along with the parent.
## Why this does not widen the trust boundary
A sealed type enumerates its subtypes in its own source, and the JVM refuses
to load any subtype
absent from that list. Trusting them grants exactly the set the author
declared and nothing an
attacker can extend. It is a more precise grant than the package-prefix
trust Dubbo already applies
when `trustSerializeClassLevel` is set, which trusts everything under a
package regardless of what
the author declared.
A non-sealed hierarchy is open and cannot be enumerated, so it must keep
behaving as it does today.
`SerializeSecurityConfiguratorSealedTest.testOpenSubclassesAreNotAutoTrusted`
asserts that
distinction directly: a plain interface is trusted, its implementation is
not.
## Brief changelog
- `SerializeSecurityConfigurator.checkClass` walks
`Class.getPermittedSubclasses()` alongside the
existing supertype and field traversal.
- `Class.getPermittedSubclasses()` is Java 17 and `dubbo-common` compiles at
Java 8, so it is
resolved reflectively once into a static `Method` and treated as absent on
older runtimes. An
unresolvable permitted subtype is skipped rather than aborting the walk.
- New `SerializeSecurityConfiguratorSealedTest`, with the sealed fixture
generated by javassist —
the module cannot compile `sealed` source — and gated to Java 17+ runtimes
via
`@EnabledForJreRange`, matching the existing use of that annotation in
this module.
## Verifying this change
- `SerializeSecurityConfiguratorSealedTest` has two cases: sealed subtypes
**are** trusted, and
non-sealed implementations are **not**.
- Reverting the `checkClass` change fails
`testSealedSubclassesAreAutoTrusted` while the non-sealed
control still passes, so the test is not vacuous.
- Full `dubbo-common` suite: 1167 tests, 0 failures, 0 errors. `mvn
spotless:check` passes.
- Run on JDK 8, 11, 17 and 21. On 8 and 11 the sealed case is skipped and
the control passes, so the
Java 8 and 11 unit-test jobs stay green. Compiled with `release 8`, so no
Java 9+ API slipped in.
(JDK 25, also in the CI matrix, was not available locally.)
- Full reactor build with `-DskipTests`: 114 modules, 0 failures. Worth
stating because this touches
`dubbo-common`, so every module downstream of it is affected.
- End-to-end over a real Netty `dubbo://` transport with no allow-list file,
against 3.3.6 and
against a local build of this branch. Three rows move from rejected to
working — a Kotlin
`sealed interface` with a `data class` subtype, the same with a `data
object` subtype, and a
**Java 17 `sealed interface`** — while two control rows are unchanged: a
non-sealed Java interface
stays untrusted, and a Java POJO that is not `Serializable` still fails
the `Serializable` gate.
## Known limitation for Kotlin below jvmTarget 17
Kotlin only emits the JVM `PermittedSubclasses` attribute at `-jvm-target
17` or above; measured on
Kotlin 2.4.20:
| `-jvm-target` | class file | `PermittedSubclasses` |
|---|---|---|
| 1.8 | 52 | absent |
| 11 | 55 | absent |
| 17 | 61 | present |
| 21 | 65 | present |
Below 17 the sealed hierarchy exists only inside `kotlin.Metadata`, whose
payload is protobuf-encoded
and would need `kotlinx-metadata` to read — not worth a core dependency.
Those builds keep behaving
exactly as they do today. The same applies to a JDK 8 or 11 runtime, where
sealed classes do not
exist at all.
## Does this pull request potentially affect one of the following parts?
- Dependencies: no.
- Public API: no.
- Serialization security defaults: yes, in that more classes are
auto-trusted — but only the subtypes
a sealed type already permits, as argued above. Behaviour is unchanged for
non-sealed types, for
JDK 8/11 runtimes and for Kotlin below jvmTarget 17.
## Checklist
- [x] Make sure there is a
[GitHub_issue](https://github.com/apache/dubbo/issues) field for the change. —
#16491
- [x] Write a pull request description that is detailed enough to understand
what the pull request does, how, and why.
- [x] Write necessary unit-test to verify your logic correction. If the new
feature or significant change is committed, please remember to add sample in
[dubbo samples](https://github.com/apache/dubbo-samples) project. —
`SerializeSecurityConfiguratorSealedTest`, covering both the fixed sealed case
and the unchanged non-sealed case.
- [x] Make sure gitHub actions can pass. [Why the workflow is failing and
how to fix it?](../CONTRIBUTING.md)
--
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]
