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

ADMINISTER should not imply RUN_SCRIPTS

    Details

    • Similar Issues:

      Description

      Anyone with RUN_SCRIPTS can go to /script and immediately grant themselves other permissions (or disable security altogether, or worse), so it is strictly more powerful than anything else, including ADMINISTER. Yet the impliedBy link normally grants RUN_SCRIPTS automatically to anyone with ADMINISTER, which is backward.

      Less obviously, with UPLOAD_PLUGINS you could do much the same, if you spent a moment writing a custom plugin with a malicious initializer and building an hpi file, then installing it dynamically. This leaves a bit more of an audit trail, but only barely. (Maybe SecurityListener should be notified whenever a script is run or a custom plugin uploaded, and by whom?)

      If (as is common) all these permissions are granted together to administrators and not at all to other users, there is no problem, but the illogical impliedBy relationship together with the relatively innocuous-sounding permission descriptions combine to make even many experienced Jenkins administrators unaware of the risks.
      t
      At a minimum, the descriptions of these three permissions must clearly state what they allow users to do and the implications.

      Next, ADMINISTER should no longer be taken to imply RUN_SCRIPTS or UPLOAD_PLUGINS, since such implications give the misleading impression that these are weaker permissions. They should need to be explicitly granted.

      (There is an unfortunate settings compatibility issue, that authorization strategy configurations which previously granted ADMINISTER explicitly to a user with the expectation that RUN_SCRIPTS would be granted implicitly would after such an update no longer grant RUN_SCRIPTS, since existing strategies typically serialize only granted permissions and make no mention of denied permissions. If popular strategies were fixed to distinguish implicitly from explicitly granted permissions, perhaps they could perform a one-time settings upgrade in which RUN_SCRIPTS were assumed granted to users previously having ADMINISTER.)

      Possibly RUN_SCRIPTS and UPLOAD_PLUGINS should even be made to imply ADMINISTER, as in effect they do.

      Note that it is possible to configure a customized system whereby some users get ADMINISTER but not either of the two other permissions mentioned here, as for example *.ci.cloudbees.com does. One of the changes needed is for the authorization strategy to ignore impliedBy for these cases. With CONFIGURE_UPDATECENTER you could perhaps trick someone with ADMINISTER into installing a "trojan horse" the next time they updated something, though you might be stymied by the signature check. There are other permissions which could be used to compromise a stock system, such as Computer.CONFIGURE (with CommandLauncher), or simply having nonzero executors on the master, so such a lockdown has to extend beyond the ACL unless various other operations in Jenkins are made to perform stricter checks.

        Attachments

          Issue Links

            Activity

            Hide
            jglick Jesse Glick added a comment -

            JENKINS-15484 notes a related problem with impliedBy.

            Show
            jglick Jesse Glick added a comment - JENKINS-15484 notes a related problem with impliedBy .
            Hide
            recampbell Ryan Campbell added a comment -

            Is this still valid?

            Show
            recampbell Ryan Campbell added a comment - Is this still valid?
            Hide
            danielbeck Daniel Beck added a comment -

            The second half is. The first half was mostly addressed in April by fixes in matrix-auth and other plugins.

            Show
            danielbeck Daniel Beck added a comment - The second half is. The first half was mostly addressed in April by fixes in matrix-auth and other plugins.
            Hide
            wfollonier Wadeck Follonier added a comment -

            Problem addressed in scriptler.

            Show
            wfollonier Wadeck Follonier added a comment - Problem addressed in scriptler.
            Hide
            jvz Matt Sicker added a comment -

            I had a thought about this problem: what if we introduced a new top-level permission such as ROOT. Then RUN_SCRIPTS, CONFIGURE_UPDATECENTER, UPLOAD_PLUGINS, and ADMINISTER are implied by ROOT. Migrating permissions from the previous set to the new one might be complicated, though.

            A quick local experiment changing these around only broke 4 integration tests, so either there's not much test coverage, or this idea has some potential.

            Show
            jvz Matt Sicker added a comment - I had a thought about this problem: what if we introduced a new top-level permission such as ROOT. Then RUN_SCRIPTS, CONFIGURE_UPDATECENTER, UPLOAD_PLUGINS, and ADMINISTER are implied by ROOT. Migrating permissions from the previous set to the new one might be complicated, though. A quick local experiment changing these around only broke 4 integration tests, so either there's not much test coverage, or this idea has some potential.
            Hide
            jglick Jesse Glick added a comment -

            Migrating permissions from the previous set to the new one might be complicated, though.

            Indeed. See discussion in JENKINS-17200.

            Show
            jglick Jesse Glick added a comment - Migrating permissions from the previous set to the new one might be complicated, though. Indeed. See discussion in JENKINS-17200 .

              People

              • Assignee:
                Unassigned
                Reporter:
                jglick Jesse Glick
              • Votes:
                1 Vote for this issue
                Watchers:
                8 Start watching this issue

                Dates

                • Created:
                  Updated: