• Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • None

      Some links (e.g. in console output lnks "Building remotely on slaveXYZ") don't preserve protocol and have hardcoded HTTP even if the page is opened via HTTPS. This can be considered as a security risk (e.g. user doesn't notice change of the protocol and later on can open another console output with sensitive data and the data would be sent over unsecure HTTP in this case)

          [JENKINS-16368] Hardcoded protocol in some links

          vjuranek created issue -

          vjuranek added a comment -

          vjuranek added a comment - Fixed in https://github.com/jenkinsci/jenkins/commit/460e508155187918e8c0f4fd0bb66a99cfe78527
          vjuranek made changes -
          Resolution New: Fixed [ 1 ]
          Status Original: Open [ 1 ] New: Resolved [ 5 ]

          kutzi added a comment -

          Might also be worth thinking about using network-path references http://stackoverflow.com/questions/3583103/network-path-reference-uri-scheme-relative-urls

          kutzi added a comment - Might also be worth thinking about using network-path references http://stackoverflow.com/questions/3583103/network-path-reference-uri-scheme-relative-urls

          Jesse Glick added a comment -

          @kutzi I think you mean just simple absolute URIs like /computer/slaveXYZ; a network-path reference would be to a different server using the same protocol (//elsewhere.net/something), which is not needed for anything in Jenkins AFAIK.

          Jesse Glick added a comment - @kutzi I think you mean just simple absolute URIs like /computer/slaveXYZ ; a network-path reference would be to a different server using the same protocol ( //elsewhere.net/something ), which is not needed for anything in Jenkins AFAIK.

          kutzi added a comment - - edited

          Actually, I was thinking about network-path reference as this bug - as I understand it - is that in some places Jenkins#getRootUrl() is used to construct intra-Jenkins links. Indeed, using absolute URIs in this case would be the even better solution.

          I find it quite strange that the value of getRootUrl doesn't always return the same - configured - value, but - based on the 'current' request - something different. But that was already so before this fix, so it doesn't actually worsen the situation.

          kutzi added a comment - - edited Actually, I was thinking about network-path reference as this bug - as I understand it - is that in some places Jenkins#getRootUrl() is used to construct intra-Jenkins links. Indeed, using absolute URIs in this case would be the even better solution. I find it quite strange that the value of getRootUrl doesn't always return the same - configured - value, but - based on the 'current' request - something different. But that was already so before this fix, so it doesn't actually worsen the situation.

          kutzi added a comment -

          Looking at it again, it actually does worsen the situation, as previously I was either getting the configured value or getRootUrlFromRequest() - which is also (kind of) mentioned in the Javadoc.
          Now I have 3 different return values, based on the context.

          kutzi added a comment - Looking at it again, it actually does worsen the situation, as previously I was either getting the configured value or getRootUrlFromRequest() - which is also (kind of) mentioned in the Javadoc. Now I have 3 different return values, based on the context.

          Jesse Glick added a comment -

          The Javadoc could be edited, but I think it is necessary to reconsider the fix, which is just a workaround for missing logic in ConsoleNote.

          My understanding of the bug is that we are talking about links within a single Jenkins master. E.g. https://server/job/stuff/1/console should link to https://computer/someslave not http://computer/someslave even if Jenkins “thinks” it is running as http://computer/. Therefore the link must be a complete URL, so hacking Jenkins.rootUrl is necessary (or the Jenkins admin needs to set the root URL to https://computer/ and accept HTTP -> HTTPS redirection as being the lesser evil).

          The issue here is that AbstractBuildExecution.run calls ModelHyperlinkNote.encodeTo("/computer/someslave", …), and HyperlinkNote.annotate translates /computer/someslave to http://server/computer/someslave (before the current fix). It does this because it has no other way of determining an absolute URI: you could be running Jenkins with a web app context path like http://computer/ci/, or you could be running Jenkins behind a reverse proxy that maps http://localhost:8080/ to http://mycorp.com/ci/, etc., so /computer/someslave cannot be guaranteed to resolve to anything useful. Nor can //computer/someslave, so network-path references are irrelevant.

          The best fix (IMO) would be to produce a relative URI, ../../../computer/someslave in this case, which will not only preserve scheme but would work correctly even in the face of an incorrect Jenkins.rootUrl setting. The difficulty is that HyperlinkNote.annotate now needs to take its Object context and determine its URL relative to the context root; and there seems to be no standard interface for this. For example, Run.getUrl does not implement any method. Perhaps such an interface could be introduced retroactively, in which case contexts assignable to this interface could result in relative URIs (with absolute URLs being used as a fallback).

          Jesse Glick added a comment - The Javadoc could be edited, but I think it is necessary to reconsider the fix, which is just a workaround for missing logic in ConsoleNote . My understanding of the bug is that we are talking about links within a single Jenkins master. E.g. https://server/job/stuff/1/console should link to https://computer/someslave not http://computer/someslave even if Jenkins “thinks” it is running as http://computer/ . Therefore the link must be a complete URL, so hacking Jenkins.rootUrl is necessary (or the Jenkins admin needs to set the root URL to https://computer/ and accept HTTP -> HTTPS redirection as being the lesser evil). The issue here is that AbstractBuildExecution.run calls ModelHyperlinkNote.encodeTo("/computer/someslave", …) , and HyperlinkNote.annotate translates /computer/someslave to http://server/computer/someslave (before the current fix). It does this because it has no other way of determining an absolute URI: you could be running Jenkins with a web app context path like http://computer/ci/ , or you could be running Jenkins behind a reverse proxy that maps http://localhost:8080/ to http://mycorp.com/ci/ , etc., so /computer/someslave cannot be guaranteed to resolve to anything useful. Nor can //computer/someslave , so network-path references are irrelevant. The best fix (IMO) would be to produce a relative URI, ../../../computer/someslave in this case, which will not only preserve scheme but would work correctly even in the face of an incorrect Jenkins.rootUrl setting. The difficulty is that HyperlinkNote.annotate now needs to take its Object context and determine its URL relative to the context root; and there seems to be no standard interface for this. For example, Run.getUrl does not implement any method. Perhaps such an interface could be introduced retroactively, in which case contexts assignable to this interface could result in relative URIs (with absolute URLs being used as a fallback).
          Jesse Glick made changes -
          Resolution Original: Fixed [ 1 ]
          Status Original: Resolved [ 5 ] New: Reopened [ 4 ]

          kutzi added a comment - - edited

          We could have a new method in Jenkins which returns the rootUrl without protocol, so ModelHyperlinkNote could use absolute URIs, like you mentioned.
          Or am I misunderstanding the issue?
          Forget it. I think you already answered this with the proxy example.

          kutzi added a comment - - edited We could have a new method in Jenkins which returns the rootUrl without protocol, so ModelHyperlinkNote could use absolute URIs, like you mentioned. Or am I misunderstanding the issue? Forget it. I think you already answered this with the proxy example.

            kohsuke Kohsuke Kawaguchi
            vjuranek vjuranek
            Votes:
            5 Vote for this issue
            Watchers:
            13 Start watching this issue

              Created:
              Updated:
              Resolved: