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

JNLP3-connect protocol format does not handle encrypted cookies that happen to contain a newline

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed (View Workflow)
    • Priority: Critical
    • Resolution: Fixed
    • Component/s: remoting
    • Labels:
      None
    • Similar Issues:

      Description

      Found this while comparing JNLP3-connect with JNLP4-connect under my stress test harness.

      The protocol finishes negotiation on the server side by sending a new cookie. The cookie is random bytes which have been encrypted using the cipher.

      The bytes are sent using as a `println(encryptedString)`

      The client then reads this using a `readLine` before switching over to the encrypted stream.

      Unfortunately if the encrypted string format of the new cookie contains a `\n` then basically the protocol is screwed up and the client will be stuck in an infinite loop trying to initialize the `ChannelBuilder` by detecting the preambles... which is akin to waiting on an infinite number of monkeys with typewriters to produce the complete works of Shakespeare.

      The symptom will be a JNLP client that never connects... with luck some of the remoting operations on the server side will time out (I'm looking at you Ping) and the connection will be closed, and a subsequent retry will hopefully "get lucky". Alternatively restarting the JNLP client will cause a new cookie to be created and we get to play roulette again.

      We can analyse the predicted probability of getting this type of failure.

      There are 32 random bytes of the cookie that get encoded as a hex string of length 64.

      A good cipher should result in something that appears close enough to a uniformly random distribution. Thus we have 64 bytes of random values in the range 0-255 and we want the probability that none of those 64 bytes are `\n`, or `(254/255)^64` which is about 78% of the time.

      Basically 22% of connections will fall victim to this bug... which matches the observed frequency of failure in my tests

        Attachments

          Activity

          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Stephen Connolly
          Path:
          src/main/java/org/jenkinsci/remoting/engine/JnlpServer3Handshake.java
          http://jenkins-ci.org/commit/remoting/e70d881fb619172460095e60a1ec88935c287a08
          Log:
          [FIXED JENKINS-37140] Roll the dice again if we are unlucky

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/org/jenkinsci/remoting/engine/JnlpServer3Handshake.java http://jenkins-ci.org/commit/remoting/e70d881fb619172460095e60a1ec88935c287a08 Log: [FIXED JENKINS-37140] Roll the dice again if we are unlucky
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Stephen Connolly
          Path:
          src/main/java/org/jenkinsci/remoting/engine/JnlpServer3Handshake.java
          http://jenkins-ci.org/commit/remoting/7fbd009ddfb9a2944e27907f2f687030acdecb9d
          Log:
          Merge pull request #95 from jenkinsci/jenkins-37140

          [FIXED JENKINS-37140] Roll the dice again if we are unlucky

          Compare: https://github.com/jenkinsci/remoting/compare/fe2feffc943b...7fbd009ddfb9

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: src/main/java/org/jenkinsci/remoting/engine/JnlpServer3Handshake.java http://jenkins-ci.org/commit/remoting/7fbd009ddfb9a2944e27907f2f687030acdecb9d Log: Merge pull request #95 from jenkinsci/jenkins-37140 [FIXED JENKINS-37140] Roll the dice again if we are unlucky Compare: https://github.com/jenkinsci/remoting/compare/fe2feffc943b...7fbd009ddfb9

            People

            • Assignee:
              stephenconnolly Stephen Connolly
              Reporter:
              stephenconnolly Stephen Connolly
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: