Remove execute permission on native .so file

[ddanielr]

Removes the execute permission on the .so native library file created in the java tmp directory.
This permission is not required for java to load the library file successfully.

JLine creates two files in a temp directory location when being used in an interactive application.

-rwx------. jlinenative-3.27.0-862111cc4ad0a980-libjlinenative.so
-rw-------.  jlinenative-3.27.0-862111cc4ad0a980-libjlinenative.so.lck

The .so file has read, write, and execute permissions set on it.

The temp directory path set by java.io.tmpdir is typically /tmp or /var/tmp.
However these directories are commonly set with the noexec flag to conform with security best practices.
example: https://www.stigviewer.com/stig/red_hat_enterprise_linux_8/2023-12-01/finding/V-230513

When the noexec flag is set, JLine is prevented from creating the native lib file and functioning correctly.
However after removing the execute permission from file creation, the native library file was able to be loaded successfully without issue.

[gnodet]

When the noexec flag is set, JLine is prevented from creating the native lib file and functioning correctly. However after removing the execute permission from file creation, the native library file was able to be loaded successfully without issue.

What happens exactly ? My understanding is that the noexec flag simply forbids the execution of the file. It should not affect the fact that the JVM reads the library to load it...

[ctubbsii]

This PR does not fix the problem.

I tested this. The permissions didn't seem to matter. The noexec on the filesystem did matter, though. I tested using a different shared library, which was more readily accessible to me, but I believe the error would likely be the same for JLine. When the filesystem was mounted with noexec, this was the stack trace:

jshell> System.load("/mnt/tmp/libaccumulo.so")
|  Exception java.lang.UnsatisfiedLinkError: /mnt/tmp/libaccumulo.so: /mnt/tmp/libaccumulo.so: failed to map segment from shared object
|        at NativeLibraries.load (Native Method)
|        at NativeLibraries$NativeLibraryImpl.open (NativeLibraries.java:388)
|        at NativeLibraries.loadLibrary (NativeLibraries.java:232)
|        at NativeLibraries.loadLibrary (NativeLibraries.java:174)
|        at ClassLoader.loadLibrary (ClassLoader.java:2394)
|        at Runtime.load0 (Runtime.java:755)
|        at System.load (System.java:1957)
|        at (#4:1)

File Permission	Filesystem Mount Option	Result
744	loop,noexec	Stacktrace
644	loop,noexec	Stacktrace
744	loop	Normal
644	loop	Normal

I read somewhere that dlopen, the system call Java uses to load the library, enforces noexec itself. I couldn't find an authoritative source for that (maybe this?), but that matches the behavior I observed.

Looking at /lib and /usr/lib on my system, it does appear that the convention is to have libraries executable, even if they don't have a main function as an entry point.

So, I don't think this PR is going to do anything to fix the problem. However, I think the comment is wrong... I don't think the executable flag is actually necessary for Java to load the library... it's just convention. The real problem is that /tmp isn't really a suitable location for putting these kinds of files.

All of this leads me to believe that a different solution is warranted. Perhaps $XDG_RUNTIME_DIR/jline/ is a better directory for this file to be extracted and loaded from (that resolves to /run/user/$(id -u)/jline/ on my system)? Or perhaps it is better for jline to create ~/.jline-tmpXXXX directory in the user's home directory? I saw some people attempt to load libraries directly from memory, but I don't think that's the best option, even if it worked, and I'm not sure it does (or should).

Another option is to provide a separate system property like jline.native.tmpdir or a similarly named property, which defaults to the value of java.io.tmpdir, but can be configured independently if the user wishes.

Ultimately, I think the best option (along with a dedicated property for configuring the path separately from java.io.tmpdir) is to make loading the native code optional... if it loading the native library code doesn't work, can JLine be made to fall back a lower-performing operational mode without crashing the application? I'm not actually even sure what the native code is supposed to do, or whether it's really all that important. But it doesn't seem like it should require loading native code to work for most users.

[ddanielr]

When the noexec flag is set, JLine is prevented from creating the native lib file and functioning correctly. However after removing the execute permission from file creation, the native library file was able to be loaded successfully without issue.

What happens exactly ? My understanding is that the noexec flag simply forbids the execution of the file. It should not affect the fact that the JVM reads the library to load it...

So I reviewed my test code and my test conditions weren't correct.
The file creation and setting of the execute permissions does work.

With a tmp directory set with noexec permissions, I can see the .so file get created with execute permissions.
So my previous statement about that part failing is incorrect.

The code returns a log message:

org.jline.nativ.JLineNativeLoader log
WARNING: Failed to load native library: jlinenative-3.27.0-862111cc4ad0a980-libjlinenative.so. osinfo: Linux/x86_64 (caused by: java.lang.UnsatisfiedLinkError: /tmp/jlinenative-3.27.0-862111cc4ad0a980-libjlinenative.so: /tmp/jlinenative-3.27.0-862111cc4ad0a980-libjlinenative.so: failed to map segment from shared object, enable debug logging for stacktrace)

Which seems to be generated here:

jline3/native/src/main/java/org/jline/nativ/JLineNativeLoader.java

Lines 258 to 263 in 1f56fba

	} catch (UnsatisfiedLinkError e) {
	log(
	Level.WARNING,
	"Failed to load native library:" + libPath.getName() + ". osinfo: "
	+ OSInfo.getNativeLibFolderPathForCurrentOS(),
	e);

My original concern was that our application would fall over after jline failed to load the native library. However that failure seems to have been unrelated.
I'm not certain if that's because I'm not triggering a condition that would use the native lib contents, but in my current testing I cannot replicate the failure. We see the warning log message but the application continues to function.

Given that @ctubbsii was able to replicate this with an unrelated library file I agree that the issue is coming up from the underlying system loader so the change in this PR doesn't seem warranted.

[gnodet]

Another option is to provide a separate system property like jline.native.tmpdir or a similarly named property, which defaults to the value of java.io.tmpdir, but can be configured independently if the user wishes.

JLine uses the jline.tmpdir which takes precedence over the java.io.tmpdir when set.

[gnodet]

@ddanielr @ctubbsii thx a lot for taking the time to investigate this issue.
I'll close this one and opened a new one do better document the loading process.

[ctubbsii]

@gnodet I didn't realize a different property already existed! That helps as a workaround. Thanks!

I'm still concerned about the unchecked RuntimeException that gets thrown at

jline3/native/src/main/java/org/jline/nativ/JLineNativeLoader.java

Line 80 in b2749c4

throw new RuntimeException("Unable to load jline native library: " + e.getMessage(), e);

There are several classes that call JLineNativeLoader.initialize() in static initializer blocks. If that throws an exception, then the class fails to be initialized by the classloader and won't be available for use. There is also the ExecTerminalProvider and AbstractPty classes that will rely on the native library, depending on the value of the system properties, org.jline.terminal.pty.fileDescriptorCreationMode and org.jline.terminal.exec.redirectPipeCreationMode. I believe both of these default to reflection,native, and try reflection first. So, that might be why @ddanielr describes only an error message, but things proceeding as expected after that, without an obvious failure. Chances are, one of the classes failed to load from the classloader because the native library could not be loaded in one of the static initializer blocks, but the code path never actually tried to use any of those classes, because everything defaulted to the reflection implementations. At this point, though I'm really just speculating. However, I do think that the load should probably not throw a RuntimeException that falls through the static initializer blocks and prevents those classes from being loaded.