Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-65582

Illegal reflective access by jenkins.slaves.StandardOutputSwapper$ChannelSwapper to constructor java.io.FileDescriptor(int)

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Minor Minor
    • core
    • Jenkins 2.263.4
      RH7 Linux, no container (slaves and master)
      remoting 4.5
      openJDK jdk-11.0.9.1+1 on slaves and master
      (openJDK jdk8u275-b01 for crosscheck on one slave)

      On start of a slave using openJDK jdk-11.0.9.1+1 on slaves and master I get in the log:

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

      <===[JENKINS REMOTING CAPACITY]===>channel started
      Remoting version: 4.5
      This is a Unix agent
      WARNING: An illegal reflective access operation has occurred
      WARNING: Illegal reflective access by jenkins.slaves.StandardOutputSwapper$ChannelSwapper to constructor java.io.FileDescriptor(int)
      WARNING: Please consider reporting this to the maintainers of jenkins.slaves.StandardOutputSwapper$ChannelSwapper
      WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
      WARNING: All illegal access operations will be denied in a future release
      Evacuated stdout
      [StartupTrigger] - Scanning jobs for node ullteb106

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

      The bold warning makes me a little bit nervous....

      All these warnings are not present, if I use openJDK jdk8u275-b01 instead. (Everything else unchanged)

      There is a similar report referring to Windows and (Sun) JDK9: https://issues.jenkins.io/browse/JENKINS-46631
      And on JDK11, but referring to "after installing the selenium plugin" https://issues.jenkins.io/browse/JENKINS-64831

      (We DON'T have this plugin installed)

      I'm running Jenkins with JDK11, because in my understanding the switch is pending; I'm using openJDK because Sun/Oracle would need a license in my understanding.

          [JENKINS-65582] Illegal reflective access by jenkins.slaves.StandardOutputSwapper$ChannelSwapper to constructor java.io.FileDescriptor(int)

          Basil Crow added a comment - - edited

          The code in question could be reworked to call jnr-posix's JavaLibCHelper.toFileDescriptor, but that also prints a reflective access warning as described in jnr/jnr-posix#110. The comments to that issue describe one possible workaround (using JNI), but really the root of the problem is that this use case just isn't supported by the Java Platform. StandardOutputSwapper itself only seems to exist as Kohsuke's solution to a stream integrity problem encountered years ago. It might be worth reconsidering that use case and possibly redesigning the subsystem so that this problem (and therefore the use case for doing file descriptor manipulation from Java) is avoided.

          Basil Crow added a comment - - edited The code in question could be reworked to call jnr-posix 's JavaLibCHelper.toFileDescriptor , but that also prints a reflective access warning as described in jnr/jnr-posix#110 . The comments to that issue describe one possible workaround (using JNI), but really the root of the problem is that this use case just isn't supported by the Java Platform. StandardOutputSwapper itself only seems to exist as Kohsuke's solution to a stream integrity problem encountered years ago. It might be worth reconsidering that use case and possibly redesigning the subsystem so that this problem (and therefore the use case for doing file descriptor manipulation from Java) is avoided.

          Jesse Glick added a comment -

          I gave some suggestions in JENKINS-64831.

          Jesse Glick added a comment - I gave some suggestions in JENKINS-64831 .

          Mark Symons added a comment -

          Issue still affect 2.346.1 using Remoting version: 4.13.2

          Mark Symons added a comment - Issue still affect 2.346.1 using Remoting version: 4.13.2

          Niels Kristian Jensen added a comment - - edited

          Also seen with Remoting version: 4.13.2, from the log:

          openjdk 11.0.15 2022-04-19
          OpenJDK Runtime Environment (build 11.0.15+10-Ubuntu-0ubuntu0.18.04.1)
          OpenJDK 64-Bit Server VM (build 11.0.15+10-Ubuntu-0ubuntu0.18.04.1, mixed mode, sharing)

          running master "Jenkins 2.346.1"

          Tim noted that the logs are harmless - perhaps this is just to be ignored?

          Niels Kristian Jensen added a comment - - edited Also seen with Remoting version: 4.13.2, from the log: openjdk 11.0.15 2022-04-19 OpenJDK Runtime Environment (build 11.0.15+10-Ubuntu-0ubuntu0.18.04.1) OpenJDK 64-Bit Server VM (build 11.0.15+10-Ubuntu-0ubuntu0.18.04.1, mixed mode, sharing) running master "Jenkins 2.346.1" Tim noted that the logs are harmless - perhaps this is just to be ignored?

          Basil Crow added a comment -

          Following up on jglick's comment in JENKINS-64831:

          I suppose this could be dealt with by having the stdio mode use a slightly more complicated protocol (new Channel.Mode?) whereby each packet is printed as text-ish with a distinctive or even random prefix and a trailing newline. Thus any unrelated messages printed to stdout would just be ignored.

          I suppose this is possible and could even be a variant of the existing Channel.Mode.TEXT mode that is split up into line-based frames with a prefix for each line. One downside to that scheme is that Base64-encoding the data would decrease efficiency.

          One question I have is whether anything else besides SSHLauncher (from SSH Build Agents) is using standard input and standard output as the streams behind a ClassicCommandTransport. That is the only use case I am aware of and it seems like the primary one. If so, perhaps an alternative would be an SSH-specific solution that creates another SSH channel for Remoting I/O, leaving the primary channel as-is for standard input and standard output. The dedicated Remoting SSH channel would effectively function as its own socket whose input and output streams would back the ClassicCommandTransport. Since Windows now supports AF_UNIX domain sockets, OpenSSH supports Unix domain socket forwarding as of 6.7, and Java 17 has native support for UnixDomainSocketAddress, perhaps we could create a Unix domain socket on the controller, forward it to the agent over SSH (not sure if Trilead supports this yet), and pass the socket path to Remoting, which would create its ClassicCommandTransport using the input and output of that socket instead of standard input and standard output (which would remain untouched). Thus we would not need to write our own framing protocol but could rely on the built-in multiplexing offered by the SSH protocol itself.

          Basil Crow added a comment - Following up on jglick 's comment in JENKINS-64831 : I suppose this could be dealt with by having the stdio mode use a slightly more complicated protocol (new Channel.Mode ?) whereby each packet is printed as text-ish with a distinctive or even random prefix and a trailing newline. Thus any unrelated messages printed to stdout would just be ignored. I suppose this is possible and could even be a variant of the existing Channel.Mode.TEXT mode that is split up into line-based frames with a prefix for each line. One downside to that scheme is that Base64-encoding the data would decrease efficiency. One question I have is whether anything else besides SSHLauncher (from SSH Build Agents) is using standard input and standard output as the streams behind a ClassicCommandTransport . That is the only use case I am aware of and it seems like the primary one. If so, perhaps an alternative would be an SSH-specific solution that creates another SSH channel for Remoting I/O, leaving the primary channel as-is for standard input and standard output. The dedicated Remoting SSH channel would effectively function as its own socket whose input and output streams would back the ClassicCommandTransport . Since Windows now supports AF_UNIX domain sockets, OpenSSH supports Unix domain socket forwarding as of 6.7, and Java 17 has native support for UnixDomainSocketAddress , perhaps we could create a Unix domain socket on the controller, forward it to the agent over SSH (not sure if Trilead supports this yet), and pass the socket path to Remoting, which would create its ClassicCommandTransport using the input and output of that socket instead of standard input and standard output (which would remain untouched). Thus we would not need to write our own framing protocol but could rely on the built-in multiplexing offered by the SSH protocol itself.

          Jesse Glick added a comment -

          whether anything else besides SSHLauncher (from SSH Build Agents) is using standard input and standard output

          https://github.com/jenkinsci/command-launcher-plugin/blob/669d7ccb7c317ad2ba51a77dd5b2b30c88891d85/src/main/java/hudson/slaves/CommandLauncher.java#L170 I think; presumably less common than SSHLauncher.

          perhaps we could create a Unix domain socket on the controller […]

          I would not advise spending so much time on this, especially not if it winds up making agent launchers harder to understand. I think we could simply delete StandardOutputSwapper and deprecate or delete StandardOutputStream with few real consequences. Launcher already calls System.setOut which ought to redirect any output from Java code, so the only problem would be stray text sent to stdout by native code—which we would really like to get rid of from the agent JVM anyway.

          Base64-encoding the data would decrease efficiency

          FWIW I did not suggest that. Would suffice for each packet-ish to emit some magic byte sequence, then packet length, then raw (binary) packet, then newline. The reader can then consume data starting with the magic sequence up the length (including any interior 0x0A) and discard other lines that look like text. Whether that is practical as a Mode depends on whether the mode translation stream (analogous to BinarySafeStream) is being wrapped around a reasonably buffered output, or whether Command.writeTo is going to be writing one byte at a time. ClassicCommandTransport is actually not ideal for this purpose, but in fact that transport is not used any more since the two sides would negotiate up to ChunkedCommandTransport. The related classes (esp. ChunkHeader actually already implement most of what we need and it might just be possible to make ChunkedInputStream.readHeader tolerate textual junk. But again, probably not worth the effort.

          Jesse Glick added a comment - whether anything else besides SSHLauncher (from SSH Build Agents) is using standard input and standard output https://github.com/jenkinsci/command-launcher-plugin/blob/669d7ccb7c317ad2ba51a77dd5b2b30c88891d85/src/main/java/hudson/slaves/CommandLauncher.java#L170 I think; presumably less common than SSHLauncher . perhaps we could create a Unix domain socket on the controller […] I would not advise spending so much time on this, especially not if it winds up making agent launchers harder to understand. I think we could simply delete StandardOutputSwapper and deprecate or delete StandardOutputStream with few real consequences. Launcher already calls System.setOut which ought to redirect any output from Java code, so the only problem would be stray text sent to stdout by native code—which we would really like to get rid of from the agent JVM anyway. Base64-encoding the data would decrease efficiency FWIW I did not suggest that. Would suffice for each packet-ish to emit some magic byte sequence, then packet length, then raw (binary) packet, then newline. The reader can then consume data starting with the magic sequence up the length (including any interior 0x0A) and discard other lines that look like text. Whether that is practical as a Mode depends on whether the mode translation stream (analogous to BinarySafeStream ) is being wrapped around a reasonably buffered output, or whether Command.writeTo is going to be writing one byte at a time. ClassicCommandTransport is actually not ideal for this purpose, but in fact that transport is not used any more since the two sides would negotiate up to ChunkedCommandTransport . The related classes (esp. ChunkHeader actually already implement most of what we need and it might just be possible to make ChunkedInputStream.readHeader tolerate textual junk. But again, probably not worth the effort.

          Basil Crow added a comment -

          I would not advise spending so much time on this, especially not if it winds up making agent launchers harder to understand.

          I do not see why it would make agent launchers harder to understand, but I do not plan on spending any time on it at all.

          Basil Crow added a comment - I would not advise spending so much time on this, especially not if it winds up making agent launchers harder to understand. I do not see why it would make agent launchers harder to understand, but I do not plan on spending any time on it at all.

          David Borin added a comment -

          Am I the only one bothered by the legacy "slave" references all over the place?

          David Borin added a comment - Am I the only one bothered by the legacy "slave" references all over the place?

          dborin - I think so. Not an issue for me at least.

          Niels Kristian Jensen added a comment - dborin - I think so. Not an issue for me at least.

          Jesse Glick added a comment -

          Note that this can already be disabled by starting the controller with -Djenkins.slaves.StandardOutputSwapper.disabled=true. Doing so may also avoid ever loading JNA in the agent JVM (if nothing else in core/plugins requests it), which would save a 2Mb file transfer for new cloud agents.

          Another thing to deprecate (more): Channel.getUnderlyingOutput

          Jesse Glick added a comment - Note that this can already be disabled by starting the controller with -Djenkins.slaves.StandardOutputSwapper.disabled=true . Doing so may also avoid ever loading JNA in the agent JVM (if nothing else in core/plugins requests it), which would save a 2Mb file transfer for new cloud agents. Another thing to deprecate (more): Channel.getUnderlyingOutput

            Unassigned Unassigned
            martinjost Martin Jost
            Votes:
            6 Vote for this issue
            Watchers:
            13 Start watching this issue

              Created:
              Updated: