[
https://issues.apache.org/jira/browse/GEODE-10521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18042863#comment-18042863
]
ASF subversion and git services commented on GEODE-10521:
---------------------------------------------------------
Commit 87e564238d35d75faf06e0ded0a74a9e48a30b08 in geode's branch
refs/heads/develop from Jinwoo Hwang
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=87e564238d ]
[GEODE-10522] Security : Eliminate Reflection in VMStats50 to Remove
--add-opens Requirement (#7957)
* GEODE-10522: Eliminate reflection in VMStats50 to remove --add-opens
requirement
Replace reflection-based access to platform MXBean methods with direct
interface casting, eliminating the need for
--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED JVM flag.
Key Changes:
- Replaced Method.invoke() with direct calls to com.sun.management interfaces
- Removed setAccessible(true) calls that required module opening
- Updated to use OperatingSystemMXBean and UnixOperatingSystemMXBean directly
- Removed COM_SUN_MANAGEMENT_INTERNAL_OPEN flag from MemberJvmOptions
- Removed unused ClassPathLoader import
- Improved code clarity and type safety
Benefits:
- Completes Java Platform Module System (JPMS) compliance initiative
- Eliminates last remaining --add-opens flag requirement
- Improves security posture (no module violations)
- Better performance (no reflection overhead)
- Simpler, more maintainable code
Testing:
- All VMStats tests pass
- Tested without module flags
- Uses public, documented APIs from exported com.sun.management package
This completes the module compliance initiative:
- GEODE-10519: Eliminated java.base/java.lang opening
- GEODE-10520: Eliminated sun.nio.ch export
- GEODE-10521: Eliminated java.base/java.nio opening
- GEODE-10522: Eliminated jdk.management/com.sun.management.internal opening
(this commit)
Apache Geode now requires ZERO module flags to run on Java 17+.
* Apply code formatting to VMStats50
- Fix import ordering (move com.sun.management imports after java.util imports)
- Remove trailing whitespace
- Apply consistent formatting throughout
* Address reviewer feedback: Add null check and improve error message
- Add null check for platformOsBean before calling getAvailableProcessors()
- Enhance error message to clarify impact on statistics vs core functionality
- Both changes suggested by @sboorlagadda in PR review
* Remove SUN_NIO_CH_EXPORT reference from JAVA_11_OPTIONS
- Fix compilation error after merging GEODE-10520 changes
- SUN_NIO_CH_EXPORT constant was removed but still referenced in list
* Fix duplicate JAVA_NIO_OPEN and missing JAVA_LANG_OPEN
- Remove duplicate JAVA_NIO_OPEN definition
- Add missing JAVA_LANG_OPEN constant
- Fix comment to correctly reference UnsafeThreadLocal for JAVA_LANG_OPEN
> Eliminate Reflection-Based Access to java.nio.Buffer Internals in
> AddressableMemoryManager
> ------------------------------------------------------------------------------------------
>
> Key: GEODE-10521
> URL: https://issues.apache.org/jira/browse/GEODE-10521
> Project: Geode
> Issue Type: Improvement
> Reporter: Jinwoo Hwang
> Assignee: Jinwoo Hwang
> Priority: Major
> Fix For: 2.1.0
>
>
> h2. Problem Description
> h3. Current State
> The {{AddressableMemoryManager}} class in {{geode-core}} currently uses
> reflection to access private internal APIs of
> {{{}java.nio.DirectByteBuffer{}}}:
> # {{DirectByteBuffer.address()}} method - to retrieve the native memory
> address from a DirectByteBuffer
> # {{DirectByteBuffer(long, int)}} constructor - to create a DirectByteBuffer
> wrapping a native memory address
> This reflection-based approach requires calling {{setAccessible(true)}} on
> both the method and constructor, which violates Java's module encapsulation
> rules introduced in Java 9 (JEP 260, 261).
> h3. Required JVM Flag
> To allow this reflection at runtime, Apache Geode currently requires the
> following JVM flag:
> {code:java}
> --add-opens=java.base/java.nio=ALL-UNNAMED
> {code}
> This flag is hardcoded in {{MemberJvmOptions.JAVA_11_OPTIONS}} as
> {{JAVA_NIO_OPEN}} and automatically added to all Geode member JVMs running on
> Java 11 and later.
> h3. Code Evidence
> Current implementation in {{{}AddressableMemoryManager.java{}}}:
> {code:java}
> // Reflection-based access requiring --add-opens
> private static volatile Method dbbAddressMethod = null;
> private static volatile Constructor dbbCtor = null;
> public static long getDirectByteBufferAddress(ByteBuffer bb) {
> Method m = dbbAddressMethod;
> if (m == null) {
> m = dbbClass.getDeclaredMethod("address");
> m.setAccessible(true); // REQUIRES --add-opens
> dbbAddressMethod = m;
> }
> return (Long) m.invoke(bb);
> }
> static ByteBuffer createDirectByteBuffer(long address, int size) {
> Constructor ctor = dbbCtor;
> if (ctor == null) {
> ctor = dbbClass.getDeclaredConstructor(long.class, int.class);
> ctor.setAccessible(true); // REQUIRES --add-opens
> dbbCtor = ctor;
> }
> return (ByteBuffer) ctor.newInstance(address, size);
> }
> {code}
> h2. Business Impact
> h3. Security and Compliance Issues
> h4. Java Platform Security Model Violations
> The use of {{--add-opens}} flags represents a security concern and technical
> debt:
> * {*}Strong Encapsulation Bypass{*}: The flag explicitly breaks Java's
> module system encapsulation, which was designed to protect internal
> implementation details
> * {*}Security Manager Conflicts{*}: In environments with security managers
> or restricted permissions, these flags may be blocked or flagged as security
> violations
> * {*}Audit Trail Issues{*}: Security audits and compliance scans flag
> {{--add-opens}} usage as potential vulnerabilities
> * {*}Attack Surface{*}: Opening internal packages to ALL-UNNAMED increases
> the attack surface by allowing any code to access these internals
> h4. Deployment and Operations Impact
> The required JVM flag creates operational friction:
> * {*}Container Security Policies{*}: Many container platforms (Kubernetes,
> Docker Enterprise) have security policies that restrict or audit JVM flags
> * {*}Cloud Platform Restrictions{*}: Some cloud platforms (AWS Lambda, Azure
> Functions, GCP Cloud Run) restrict or monitor JVM flags
> * {*}Enterprise Security Policies{*}: Enterprise security teams often
> require justification and approval for {{--add-opens}} flags
> * {*}Deployment Complexity{*}: Administrators must remember to include the
> flag in all deployment configurations
> h4. Forward Compatibility Risk
> Java's module system evolution poses risks:
> * {*}JEP 403 (Java 17+){*}: Strongly encapsulates JDK internals by default,
> with warnings issued for illegal reflective access
> * {*}Future Java Versions{*}: The Java platform is moving toward stricter
> enforcement of module boundaries
> * {*}Deprecation Path{*}: {{--add-opens}} may become deprecated or
> restricted in future Java releases
> * {*}Migration Cost{*}: The longer this technical debt remains, the more
> costly it becomes to remediate
> h3. Technical Impact
> h4. Off-Heap Memory Management
> {{AddressableMemoryManager}} is a critical component in Geode's off-heap
> memory management:
> * {*}Purpose{*}: Manages direct ByteBuffer access to off-heap memory regions
> * {*}Performance Critical{*}: Used in hot paths for off-heap data access
> * {*}Memory Safety{*}: Provides abstraction layer over native memory
> addresses
> * {*}Zero-Copy Operations{*}: Enables wrapping native memory addresses as
> ByteBuffers without copying data
> Current usage patterns:
> {code:java}
> // Get address from a DirectByteBuffer
> long address = AddressableMemoryManager.getDirectByteBufferAddress(buffer);
> // Create a ByteBuffer wrapping a native memory address
> ByteBuffer wrapper = AddressableMemoryManager.createDirectByteBuffer(address,
> size);
> {code}
> h4. Dependency Chain
> The {{java.nio}} module opening affects multiple components:
> * {{{}geode-core{}}}: Direct usage in {{AddressableMemoryManager}}
> * {{{}geode-gfsh{}}}: Automatic flag injection via {{MemberJvmOptions}}
> * All Geode members: Every server, locator, and client requires the flag
> * Test infrastructure: All integration and unit tests must include the flag
> h2. Benefits of Remediation
> h3. Security and Compliance
> * {*}Eliminates Security Audit Findings{*}: Removes {{--add-opens}} flag
> from security scan results
> * {*}Improves Security Posture{*}: Operates within Java's intended security
> model
> * {*}Simplifies Compliance{*}: Reduces justification burden for security
> teams
> * {*}Future-Proof Security{*}: Aligns with Java platform security evolution
> h3. Operational Excellence
> * {*}Simplified Deployment{*}: One less JVM flag to configure and document
> * {*}Cloud-Native Friendly{*}: Compatible with restrictive cloud platform
> policies
> * {*}Container Security{*}: Passes stricter container security policies
> without exceptions
> * {*}Reduced Configuration Complexity{*}: Fewer deployment-specific
> configurations to maintain
> h3. Java Platform Compatibility
> * {*}Java 17+ Ready{*}: Fully compatible with JEP 403 strong encapsulation
> * {*}Java 21 Forward Compatible{*}: No deprecation warnings or compatibility
> issues
> * {*}Virtual Threads Compatible{*}: Works correctly with Project Loom
> virtual threads
> * {*}GraalVM Native Image Ready{*}: Eliminates reflection that complicates
> native image generation
> h3. Developer Experience
> * {*}Cleaner Code{*}: Less reflection boilerplate and error handling
> * {*}Better IDE Support{*}: IDEs can analyze code without reflection
> indirection
> * {*}Easier Testing{*}: Tests don't require special JVM flag configurations
> * {*}Reduced Maintenance{*}: Eliminates reflection-related failure modes
> h3. Performance Considerations
> * {*}Faster Reflection{*}: Field offset caching is more efficient than
> method reflection
> * {*}JIT Optimization{*}: Direct Unsafe field access optimizes better than
> reflective method invocation
> * {*}Reduced Overhead{*}: Eliminates {{invoke()}} overhead on every call
> * {*}Better Inlining{*}: JIT compiler can inline field access more
> aggressively than reflection
> h2. Module System Context
> This issue is part of a broader initiative to eliminate all JVM module system
> violations in Apache Geode:
> h3. Related Work
> * {*}GEODE-10519{*}: Eliminated
> {{--add-opens=java.base/java.lang=ALL-UNNAMED}} ({{{}UnsafeThreadLocal{}}})
> * {*}GEODE-10520{*}: Eliminated
> {{--add-exports=java.base/sun.nio.ch=ALL-UNNAMED}} ({{{}DirectBuffer{}}})
> * {*}GEODE-10521{*}: This issue - Eliminate
> {{--add-opens=java.base/java.nio=ALL-UNNAMED}}
> ({{{}AddressableMemoryManager{}}})
> * {*}Future{*}:
> {{--add-opens=java.base/com.sun.management.internal=ALL-UNNAMED}}
> ({{{}VMStats50{}}})
> h3. Strategic Goal
> The ultimate goal is to run Apache Geode on Java 17, and 21 without requiring
> any {{-{-}add-opens{-}}} or {{-add-exports}} flags, ensuring:
> * Full compliance with Java module system
> * Maximum security and encapsulation
> * Forward compatibility with future Java releases
> * Cloud-native and container-friendly deployment
> h2. Affected Versions
> * All Geode versions running on Java 17 and later
> * Particularly impacts Java 17+ deployments where strong encapsulation
> warnings appear
> h2. References
> * [JEP 260: Encapsulate Most Internal APIs|https://openjdk.org/jeps/260]
> * [JEP 261: Module System|https://openjdk.org/jeps/261]
> * [JEP 403: Strongly Encapsulate JDK Internals|https://openjdk.org/jeps/403]
> * Java Platform Module System (JPMS) specification
> h2. Additional Context
> h3. Why Reflection Was Used Initially
> The original implementation used reflection because:
> # {{DirectByteBuffer}} constructor is package-private (not accessible)
> # {{DirectByteBuffer.address()}} method is package-private (not accessible)
> # Public {{ByteBuffer}} API doesn't expose native memory address
> # No public API exists to wrap an arbitrary memory address as a ByteBuffer
> This was a pragmatic solution when Java 9 was new and the module system
> implications were less understood.
> h3. Off-Heap Memory Architecture
> Geode's off-heap memory management is critical for large heap scenarios:
> * Reduces garbage collection pressure by keeping data off-heap
> * Enables multi-gigabyte caches without GC pauses
> * Provides direct memory access for serialization/deserialization
> * Supports memory-mapped file operations
> {{AddressableMemoryManager}} is a cornerstone of this architecture, making
> its security and compatibility critical.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
