RFR: 8318026: jcmd should provide access to detailed JVM object information [v9]

[Kevin Walls]

On Thu, 28 Mar 2024 14:12:08 GMT, Andrew Dinn <adinn at openjdk.org> wrote:

>>> It looks like the test only inspects a thread and a java object. Perhaps you could add tests for additional VM objects. Maybe grab a frame PC from a thread stack trace.
>> 
>> Yes - added a couple of metadata tests, and a compiled method test, which gets an address from Events info.  We know that should resolve to a compiled method (if we have such an event, so this copes if there are none).
>> 
>> 
>> Also the VM.info and Thread.print runs with the CommandExecutor are now silent.  They are long, and max out the test log file and make it truncate.  That output could mainly be useful if  the regexes can't match items, but the output format should be reproducible in a new run.  Also if we fail to find something we need, like a thread id, it will print the full output it searched.
>
> @kevinjwalls Hi Kevin. Sorry to offer a drive-by comment but I have been watching this thread and I can understand why @tstuefe is expressing his concern about the potential for security issues when it comes to using jcmd to expose JVM internals.
> 
> Exposure of details like a thread/stack/metadata value address or a datum embedded at such a location does look to me like something bad agents might be able to profit from. The danger is not just being able to retrieve specific details of the layout or content of JVM structures themselves, but the opportunity to use that knowledge to upgrade a weak security hole like, say, a memory exposure, into a stronger targeted attack that knows where or to what it might want to apply the crowbar.
> 
> So, first off, please understand that I am not suggesting there *is* a problem here. I am happy to accept your conclusion that your proposed jcmd changes do not expose new data to users who ought not to be able to view either via local or via remote accesses. However, not being an expert when it comes to the jcmd/DCmd implementation, I'm not sure I really understand why that is so from your current explanation. In particular I don't understand what you said about visibility of different commands for local and remote access.
> 
> That confession of my own ignorance is not really of any immediate importance as I have no intention of trying to review this change. However, I still feel it might be useful if you could summarize on this JIRA precisely what security safeguards are currently in place when it comes to running specific jcmds/DCmds and why that means exposure of JVM internal details via jcmd, whether locally or remotely via JMX, will only be to a safe audience of users. That would help any maintainer/developer who refers to this JIRA to follow how those safeguards apply to the proposed commands (in particular it might be of great help to those performing and assessing the correctness of backports). However, it would probably also as a prophylactic to ensure that any future development work does not inadvertently lead to unexpected exposure, which I think is the thing Thomas is more worried about. Indeed, as Thomas suggested, a clear statement of what policies and mechanisms are (or should be) in plac
 e to ensure jcmd content is securely viewable  might be a good starting point for you and/or Thomas to raise follow-up JIRAs if needed to:
> 1. add comments to the code base to ensure devs to not inadvertently add or modify new DCmds ...

Hi @adinn 
Drive-bys are welcome. 8-)

Providing new crowbars to attackers is not something we want to do.

jcmd is protected by the attach api and is not open to other users.  We are trusting in that.  If you can satisfy the attach API connection, you have all the jcmds/DiagnosticCommands available.  No limits.  Likewise you can use other tools to examine abritrary memory in the target process, take a gcore, kill it, etc... 


Why will VM.inspect not be available remotely?

Because this marks the jcmd as "hidden": src/hotspot/share/services/diagnosticCommand.cpp: 
 DCmdFactory::register_DCmdFactory(new DCmdFactoryImpl<VMInspectDCmd>(full_export, true, true /* hidden */ ));
 (The new test for VM.inspect checks it does not appear in the jcmd help output, to ensure it stays hidden.)

In JMX, DiagnosticCommandMBean(Impl) does not expose hidden commands: 

The route for that is DiagnosticCommandImpl.java calling its native getDiagnosticCommands(), that gets into management.cpp's  jmm_GetDiagnosticCommands() calling DCmdFactory::DCmd_list().  In diagnosticFramework.cpp, this iterates DCmdFactories, choosing those which are not hidden.  

Querying over JMX I cannot see or access vmInspect, like I can see vmInfo, gcClassHistogram etc... through the exposed DiagnosticCommandMBean.

That "hidden" flag could be more obvious or better documented perhaps.  Also I don't think we actually say in the DiagnosticCommandMBean API docs that it provides access to the DiagnosticCommand implementations which are also exposed by the jcmd tool.  These concerns could be addressed in a separate PR if desired.

(As well as the hidden flag, we have flags to implement different categories of diagnostic command.  these are not widely used and I think the hidden flag is a better fit, as it keeps it out of the jcmd general help.  This command should be documented in other guides in due course.).

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17655#issuecomment-2025665216