byte-buddy J9 attachment only checks for self-owned directories

[FelixMarxIBM]

Hi @raphw ,
we noticed that the attachment on Linux only allows for attachments against directories owned by the current user.
So far we used the normal attachment of the J9 or Semeru JDK and on Java 10+ this has always worked with the root user, even though it is mentioned in the openj9 documentation that this should only work with the current user,

Would it be possible to remove the same user check in VirtualMachine.java?

For testing purpose in our smoke-test environment we added this very basic workaround that disables the same-user check:
instana@9c4990e

We are trying to find out in which environments this limitation is actually true.

[raphw]

I remember that this is implemented in HotSpot. I assume that this requirement is taken from there but is not implemented. Disabling it by a property seems like a reasonable option as I think that this might be implemented in the future since it is a security concern. I add it. Did you reach out to the J9 team to see if this is planned for the future?

[FelixMarxIBM]

Milestone should most likely be 1.14.15 as 1.12.14 is already released.
I did reach out to the J9 developers and will keep you updated.

[FelixMarxIBM]

I had some discussion with the J9 developers and the handling is comparable to Hotspot based JVMs. On Java versions up to including 1.9, the attacher requires the same user that also started the to-be-attached JVM.
On Java 10+, this limitation does not apply and root can also attach to these JVMs.
I did test this with Hotspot and Semeru based JVMs.

We have to be a bit smarter here and only omit the file check if we are not root (or not in the root group).
Unfortunatly the attachInfo file does not tell us which java version is used here, so even if we could read it, we would not know upfront if the target JVM is java 9 or below and will not allow us to attach.

In our code, we do know if we are root or not and we also know the version of the target as we do a java -version before an attachment, but I guess it will be hard to move any such logic into byte-buddy directly.
I guess the easiest solution will be to add a switch to turn this check off and let the outer application handle the errors.

A normal use case is for the same user and there the check does make sense.

[raphw]

I remember implementing this by extracting the logic from a Java 8 version of J9 and I did not revisit thereafter. If I remember correctly, HotSpot checks the chmod of the checked file, but chances are that a root-owned file is today accepted as you say.

As you said, I think it's hard to discover this from the process. I introduced an argument now that allows to controll this per process id. So if the executing process has knowledge of this, it can supply the override as an argument.

[FelixMarxIBM]

Thanks, I will give this a try in our smoke-test environment
f04d254
01ed75b

[FelixMarxIBM]

I noticed that you are not forwarding the ignoreUser as part of the
public static VirtualMachine attach(String processId, boolean ignoreUser) method
01ed75b#diff-f796555defc948cb31030ac3933918deee757e89f8788700db985f035dcfa212R1674-R1678

[FelixMarxIBM]

At least for us the current version won't work as we typically just call public static VirtualMachine attach(String processId) via reflection, so the ignoreUser flag would still be false.

[FelixMarxIBM]

When testing this, there is a timeout

Attaching using net.bytebuddy.agent.VirtualMachine$ForOpenJ9
Attach using net.bytebuddy.agent.VirtualMachine$ForOpenJ9
trying to look into /proc/2235675/root/tmp/.com_ibm_tools_attach/2235675
java.net.SocketTimeoutException: Accept timed out
	at java.base/java.net.PlainSocketImpl.socketAccept(Native Method)
	at java.base/java.net.AbstractPlainSocketImpl.accept(AbstractPlainSocketImpl.java:474)
	at java.base/java.net.ServerSocket.implAccept(ServerSocket.java:565)
	at java.base/java.net.ServerSocket.accept(ServerSocket.java:533)
	at net.bytebuddy.agent.VirtualMachine$ForOpenJ9.attach(VirtualMachine.java:1832)
	at net.bytebuddy.agent.VirtualMachine$ForOpenJ9.attach(VirtualMachine.java:1675)
	at net.bytebuddy.agent.VirtualMachine$ForOpenJ9.attach(VirtualMachine.java:1663)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:572)
	at com.instana.agent.loader.AgentLoaderAttach.attach(AgentLoaderAttach.java:398)
	at com.instana.agent.loader.AgentLoaderAttach.run(AgentLoaderAttach.java:150)
	at com.instana.agent.loader.AgentLoaderAttach.parseArgsAndRun(AgentLoaderAttach.java:100)
	at com.instana.agent.loader.AgentLoaderAttach.main(AgentLoaderAttach.java:83)

I suppose because we need to issue the following command
https://github.com/eclipse-openj9/openj9/blob/264f2752c6125e2a6a4d9a4e1a99d3faa4cdc3a6/jcl/src/java.base/share/classes/openj9/internal/tools/attach/target/Reply.java#L88-L90

[FelixMarxIBM]

Okay, on J9, the check for isFileOwnedByUid excludes userid 0
https://github.com/eclipse-openj9/openj9/blob/master/jcl/src/java.base/share/classes/openj9/internal/tools/attach/target/CommonDirectory.java#L424-L426

	/**
	 * Check if the file is owned by the UID.  Note that UID 0 "owns" all files.
	 * @param f File or directory
	 * @param myUid user UID. 
	 * @return true if the uid owns the file or uid == 0.
	 */
	public static boolean isFileOwnedByUid(File f, long myUid) {
		return (0 == myUid) || (myUid == CommonDirectory.getFileOwner(f.getAbsolutePath()));
	}

[raphw]

Ok, so we need to check if the owners are unequal and change it unless the current id is 0.

I can look into it, but I'd appreciate at PR of course.

[FelixMarxIBM]

I created #1631 to solve the problem.
In our smoke test, the attachment from an agent running as root (and Hotspot) against a Semeru Java 11 running as user now works.

[FelixMarxIBM]

@raphw would be great if you can do another release of byte-buddy and then we can close this issue 🥺

[raphw]

Just triggered a release.

[FelixMarxIBM]

We can close this issue. Our smoke tests pass with the 1.14.15 release. Thank you very much for the quick release.