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

QueueItemAuthenticator fallback behavior cleanup

      Currently Queue.Item.authenticate and Tasks.getAuthenticationOf check any configured QueueItemAuthenticator instances for a specific answer, then fall back to Queue.Task.getDefaultAuthentication, which for compatibility is normally SYSTEM. This behavior makes sense if there are no configured authenticators (the default case), or if there is an authenticator which returns a non-null authentication for every task.

      The problematic case is when there is a configured authenticator yet it returns null for the given task or item. This could happen if for example ProjectQueueItemAuthenticator were configured yet a user created a new job and did not add a AuthorizeProjectProperty. Now the job runs as SYSTEM, with full privileges, whereas we would really want it to run as ANONYMOUS, with no special privileges.

      BuildTrigger and ReverseBuildTrigger already specially handle this case by substituting ANONYMOUS for SYSTEM while they run (and in the former case printing a warning to the build console). But this does not extend to other plugin or core access control checks.

      In particular, Computer.BUILD is checked in two places (on Item) when deciding whether to build a job on a particular slave. You would expect to be able to control access to sensitive slaves by selectively granting BUILD only to certain people, and then requiring them to associate themselves with jobs they wish to build on those slaves. But this scheme cannot work if they can simply omit any authentication on the job and still have the build be authorized. (Jenkins Enterprise by CloudBees has a feature using a different scheme for the same use case.)

      It would be better if the callers of QueueItemAuthenticator.authenticate fell back to getDefaultAuthentication but replaced SYSTEM with ANONYMOUS in case there were at least one authenticator consulted. Then the replacement of SYSTEM with ANONYMOUS could be removed from both BuildTrigger and ReverseBuildTrigger. BuildTrigger.warning_you_have_no_plugins_providing_ac and BuildTrigger.warning_access_control_for_builds_in_glo could be left as is, or made more generic and moved into Run.execute; BuildTrigger.warning_this_build_has_no_associated_aut should be made more generic and moved into Run.execute (and Run.running_as_ printed with anonymous).

          [JENKINS-22949] QueueItemAuthenticator fallback behavior cleanup

          Jesse Glick created issue -

          Jesse Glick added a comment -

          See ada4a21 and 63aba90 (Computer.BUILD) in 1.520.

          Jesse Glick added a comment - See ada4a21 and 63aba90 ( Computer.BUILD ) in 1.520.
          Jesse Glick made changes -
          Link New: This issue is blocking JENKINS-16956 [ JENKINS-16956 ]
          Jesse Glick made changes -
          Link New: This issue is blocking JENKINS-18285 [ JENKINS-18285 ]

          Oleg Nenashev added a comment - - edited

          I think that the default user should be configurable from the global security page.
          SYSTEM should be default (with scary red/yellow warning, of course)
          Otherwise, it's hard to predict all possible regressions

          Oleg Nenashev added a comment - - edited I think that the default user should be configurable from the global security page. SYSTEM should be default (with scary red/yellow warning, of course) Otherwise, it's hard to predict all possible regressions

          Jesse Glick added a comment -

          The only possible regressions are to people already using the Authorize Project plugin. Currently there are 155 installations, presumably some of them not actually having configured it yet.

          Jesse Glick added a comment - The only possible regressions are to people already using the Authorize Project plugin. Currently there are 155 installations, presumably some of them not actually having configured it yet.

          Oleg Nenashev added a comment -

          We use an internal plugin on "eval" servers with 1.532.x+, other users may use such plugins as well.
          Actually, I'd definitely use the "Anonymous" user as a default fallback, but I don't think that the regression injection is a good approach in general.

          Oleg Nenashev added a comment - We use an internal plugin on "eval" servers with 1.532.x+, other users may use such plugins as well. Actually, I'd definitely use the "Anonymous" user as a default fallback, but I don't think that the regression injection is a good approach in general.

          Jesse Glick added a comment -

          I would consider a system property or similar to provide a temporary workaround to the probably small number of people already using the plugin who really need the current (insecure) behavior until something else if fixed or reconfigured. But I do not think it should be a UI option since it does not make sense going forward.

          Jesse Glick added a comment - I would consider a system property or similar to provide a temporary workaround to the probably small number of people already using the plugin who really need the current (insecure) behavior until something else if fixed or reconfigured. But I do not think it should be a UI option since it does not make sense going forward.

          Oleg Nenashev added a comment -

          Yes, a system property would be a convenient solution.

          Oleg Nenashev added a comment - Yes, a system property would be a convenient solution.
          Jesse Glick made changes -
          Link New: This issue is blocking JENKINS-22460 [ JENKINS-22460 ]

            jglick Jesse Glick
            jglick Jesse Glick
            Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: