• Icon: New Feature New Feature
    • Resolution: Unresolved
    • Icon: Major Major
    • github-oauth-plugin
    • None

      Background

      We are operating Jenkins in an enterprise where we have Slave to Master control in place.

      Unfortunately this plugin doesn't allow mapping GitHub users to Agent/* permissions (aside from matrix auth, but that causes us to lose Committer Authorisation Strategy which we love).

      So we are currently are forced to make agent users into Jenkins Admins, which undermines Slave to Master controls.

      The Feature

      We would like to add a new configuration, agentUserNames.

      This would behave in the same way as adminUserNames does now (example), however only for Agent/* permission checks.

      This means that any usernames declared in agentUserNames will short-circuit the ACL checking function if it is for an Agent/* permission.

      A Question

      As this is currently blocking us, we are planning to implement the above feature in a fork of this plugin. We wanted to ask early on, whether it sounds like an OK solution and something we could contribute via a PR when we are done?

       

          [JENKINS-63421] Agent/Connect and other Agent permissions

          Sam Gleske added a comment - - edited

          JENKINS-27844 outlines the planned rewrite of repository permissions.

          I would like it to be a cross between project matrix auth and the committer authorization strategy which allows one to arbitrarily choose any permission depending on the user's role in the repository.

          Sam Gleske added a comment - - edited JENKINS-27844 outlines the planned rewrite of repository permissions. I would like it to be a cross between project matrix auth and the committer authorization strategy which allows one to arbitrarily choose any permission depending on the user's role in the repository.

          Andreas Nygard added a comment - - edited

          Hi sag47, so if I play back my understanding, the idea is:

          • Extend the existing Github committer strategy to support opt-in Matrix-based permissions
            • It will behave exactly the same as Github committer strategy does today if no matrix-auth permissions are configured (since matrix auth will be opt-in), keeping this feature backward compatible
            • By extending the existing strategy, we provide less choices to the user and so keep the UX simple
          • If the opt-in Matrix-based or Project Matrix-based permissions are used, they are additive
            • This keeps behaviour consistent with the additive-approach taken in the Matrix Authorization plugin

          Are you aware of any work that has been undertaken or underway on this, in order for my team to leverage existing bits?

          Cheers

          Andreas Nygard added a comment - - edited Hi sag47 , so if I play back my understanding, the idea is: Extend the existing Github committer strategy to support opt-in Matrix-based permissions It will behave exactly the same as Github committer strategy does today if no matrix-auth permissions are configured (since matrix auth will be opt-in), keeping this feature backward compatible By extending the existing strategy, we provide less choices to the user and so keep the UX simple If the opt-in Matrix-based or Project Matrix-based permissions are used, they are additive This keeps behaviour consistent with the additive-approach taken in the Matrix Authorization plugin Are you aware of any work that has been undertaken or underway on this, in order for my team to leverage existing bits? Cheers

          Sam Gleske added a comment - - edited

          No work has been done beyond brainstorming as far as I'm aware.

          • They are not additive.  It would replace the existing system but functionally work the same.
          • The configuration for upgrading Jenkins infra would be migrated in a way that permissions are preserved.
          • The default permissions on a new Jenkins instance (configured in  matrix-like style) should be  the same as it always has been.
          • The intent is to  extend support for all permissions dynamically because different plugins can provide their own custom permissions (e.g. extended read plugin).

          Sam Gleske added a comment - - edited No work has been done beyond brainstorming as far as I'm aware. They are not additive.  It would replace the existing system but functionally work the same. The configuration for upgrading Jenkins infra would be migrated in a way that permissions are preserved. The default permissions on a new Jenkins instance (configured in  matrix-like style) should be  the same as it always has been. The intent is to  extend support for all permissions dynamically because different plugins can provide their own custom permissions (e.g. extended read plugin).

          Jim Klimov added a comment - - edited

          Hello, as a user of Swarm plugin I am also interested in this subject - since if we enable Github Auth plugin for the team members, we seem to lose access for locally defined accounts that swarm clients log into Jenkins controller with.

          It seems clumsy to have do define account(s) for the swarm client(s) on github for dynamic connections from the internal build farm. I did not check yet if a dedicated set of github login + pass/token suffice for the swarm client on a worker (and if that works - probably such account should be outside the Github organization so it has no rights there and in Jenkins jobs by accident), or if it should also have to have access to the internet and hop to github login forms and back...

          Is it possible to (develop extensions to) mix "local database" accounts with Github ones, so we can have the benefits of both? This idea does seem compatible with proposal here, to allow Github auth strategy to set the Agent/* permissions to certain accounts (like the Matrix strategy can), with an extension that such accounts might not be defined in Github but rather locally.

          UPDATE: It was indeed possible to reconnect the swarm clients with a new github account not attached to any organization, after I generated a token for it with "read:user" (and "user:email") permissions, and also "read:org" seemed to be required - auth failed on a first token without it). Note that API auth directly by password no longer works on Github side since last year, only tokens. On an upside, this allows to issue (and pull back) many tokens for many build agents using the same account.

          On Jenkins side, I also had to switch from GitHub strategy to a manually made somewhat similar matrix (for our project's static list of GH organizations and accounts, though I am not sure the matrix setup can go into such detail as github-auth can - e.g. considering PR authors, collaborators, contributors... as special groups), and allowed that new account the permissions needed for Swarm client (Overall/Read and some of Agent/* ones). I was not able to authenticate the swarm client with an account defined only locally with a password, although this account is otherwise known and manageable in Jenkins UI.

          I have also double-checked that with the worker isolated by firewall so it can not access the Internet and can only talk to the Jenkins controller (in the same subnet), authentication still works - so it is not the client that has to go to github and back to confirm the account. Cool on that side

          Jim Klimov added a comment - - edited Hello, as a user of Swarm plugin I am also interested in this subject - since if we enable Github Auth plugin for the team members, we seem to lose access for locally defined accounts that swarm clients log into Jenkins controller with. It seems clumsy to have do define account(s) for the swarm client(s) on github for dynamic connections from the internal build farm. I did not check yet if a dedicated set of github login + pass/token suffice for the swarm client on a worker (and if that works - probably such account should be outside the Github organization so it has no rights there and in Jenkins jobs by accident), or if it should also have to have access to the internet and hop to github login forms and back... Is it possible to (develop extensions to) mix "local database" accounts with Github ones, so we can have the benefits of both? This idea does seem compatible with proposal here, to allow Github auth strategy to set the Agent/* permissions to certain accounts (like the Matrix strategy can), with an extension that such accounts might not be defined in Github but rather locally. UPDATE: It was indeed possible to reconnect the swarm clients with a new github account not attached to any organization, after I generated a token for it with "read:user" (and "user:email") permissions, and also "read:org" seemed to be required - auth failed on a first token without it). Note that API auth directly by password no longer works on Github side since last year, only tokens. On an upside, this allows to issue (and pull back) many tokens for many build agents using the same account. On Jenkins side, I also had to switch from GitHub strategy to a manually made somewhat similar matrix (for our project's static list of GH organizations and accounts, though I am not sure the matrix setup can go into such detail as github-auth can - e.g. considering PR authors, collaborators, contributors... as special groups), and allowed that new account the permissions needed for Swarm client (Overall/Read and some of Agent/* ones). I was not able to authenticate the swarm client with an account defined only locally with a password, although this account is otherwise known and manageable in Jenkins UI. I have also double-checked that with the worker isolated by firewall so it can not access the Internet and can only talk to the Jenkins controller (in the same subnet), authentication still works - so it is not the client that has to go to github and back to confirm the account. Cool on that side

          Andreas Nygard added a comment - - edited

          Hi sag47 , we are picking up this story now after some delay. I'm updating you below on where we are at, and what our thinking is for the next step.

          Implementation challenges

          I made an attempt at implementing the hybrid solution ("if matrix role is defined for user, use that, else, use github derived role"). From an implementation perspective, I have not succeeded. It requires nesting the UI widgets of the 2 plugins:

          From a maintainability perspective, I would like to treat the above two widgets as black boxes. Our feature should just compose the underlying features.

          The correct approach appears to be to use jelly include in order to compose the page:

          • Matrix widget: <st:include page="config.jelly" class="hudson.security.GlobalMatrixAuthorizationStrategy" />
          • GH widget: <st:include page="config.jelly" class="org.jenkinsci.plugins.GithubAuthorizationStrategy" />

          However I receive a runtime exception when attempting to render the page using the above approaches (I don't have it on hand sorry, I have since moved to a new approach). I suspect that the jelly framework is not very great at encapsulating UI components to allow this kind of composition, and I'll be required to jump through hoops to plumb it all together.

          Rethinking the approach

          Let's come back to the original problem: this plugin has a security vulnerability. A malicious user could compromise the credential on an agent, and use it to become an admin on the master. This is because agent->master accounts currently require full admin. This is especially problematic in enterprise environments, where agents can be operated by other teams, so it is trivial for them to obtain the agent credential. Jenkins has made steps to address this with Slave to Master Access Control, but that approach is undermined by the flaw in this plugin.

          In order to make it easy for people to do the right thing, I feel that the original suggestion of adding a new configuration, agentUserNames, is the better approach. This is because it specifically addresses the security problem, in an opinionated way. The alternate suggestion of giving users a super-generic matrix-hybrid permission system also allows the problem to be solved, but it does not present the security flaw. Most users will probably keep doing what they are doing if we offer the hybrid solution.

          Therefore, I think the original suggestion is more pragmatic, solving the security flaw, and making it obvious to users that the flaw exists.

          TL;DR

          We are going with the original approach suggested in this ticket, since it is much easier to implement, and leads to better security outcomes for the community.

          Andreas Nygard added a comment - - edited Hi sag47 , we are picking up this story now after some delay. I'm updating you below on where we are at, and what our thinking is for the next step. Implementation challenges I made an attempt at implementing the hybrid solution ("if matrix role is defined for user, use that, else, use github derived role"). From an implementation perspective, I have not succeeded. It requires nesting the UI widgets of the 2 plugins: https://github.com/jenkinsci/matrix-auth-plugin https://github.com/jenkinsci/github-oauth-plugin From a maintainability perspective, I would like to treat the above two widgets as black boxes. Our feature should just compose the underlying features. The correct approach appears to be to use jelly include in order to compose the page: Matrix widget: <st:include page="config.jelly" class="hudson.security.GlobalMatrixAuthorizationStrategy" /> GH widget: <st:include page="config.jelly" class="org.jenkinsci.plugins.GithubAuthorizationStrategy" /> However I receive a runtime exception when attempting to render the page using the above approaches (I don't have it on hand sorry, I have since moved to a new approach). I suspect that the jelly framework is not very great at encapsulating UI components to allow this kind of composition, and I'll be required to jump through hoops to plumb it all together. Rethinking the approach Let's come back to the original problem: this plugin has a security vulnerability. A malicious user could compromise the credential on an agent, and use it to become an admin on the master. This is because agent->master accounts currently require full admin. This is especially problematic in enterprise environments, where agents can be operated by other teams, so it is trivial for them to obtain the agent credential. Jenkins has made steps to address this with Slave to Master Access Control , but that approach is undermined by the flaw in this plugin. In order to make it easy for people to do the right thing , I feel that the original suggestion of adding a new configuration, agentUserNames , is the better approach. This is because it specifically addresses the security problem, in an opinionated way. The alternate suggestion of giving users a super-generic matrix-hybrid permission system also allows the problem to be solved, but it does not present the security flaw. Most users will probably keep doing what they are doing if we offer the hybrid solution. Therefore, I think the original suggestion is more pragmatic, solving the security flaw, and making it obvious to users that the flaw exists. TL;DR We are going with the original approach suggested in this ticket, since it is much easier to implement, and leads to better security outcomes for the community.

          Andreas Nygard added a comment - - edited

          We have implemented the feature, which simply adds this new setting for reduced-permission agent accounts. Hope to raise a PR soon.

          Andreas Nygard added a comment - - edited We have implemented the feature, which simply adds this new setting for reduced-permission agent accounts. Hope to raise a PR soon.

          Jesse Glick added a comment -

          enterprise environments, where agents can be operated by other teams

          I have a feeling this is replacing one problem with one another.

          The recommended setup for Jenkins is to use a Cloud to provision agents from elastic infrastructure on demand, so that there is nothing to manage (except for the initial Cloud setup, which would be done by an administrator). Of course there are cases where you need static agents, like that one Mac Mini in the closet, but the idea is that these should be a small minority that the controller administration team can handle. This feature, however, seems targeted to environments where a broad list of people from various teams (who are not trusted to administer the whole controller) are expected to be connecting agents from all over the place.

          If you come up with a list of GitHub usernames who should all be allowed general Computer/* permissions (as I think the proposed change does), then they will all be permitted to disconnect, reconfigure, and reconnect each other’s agents. Suppose Team A had set up a folder based on one GitHub repo or org, defined some deployment credentials in it, attached a static agent to Jenkins, and started running builds on it. Now someone from Team B could quietly come along and reconfigure that agent to point to infrastructure they control and from which they could freely copy source code, credentials, and anything else used during the course of Team A’s builds. If that is considered an accepted risk, it should certainly be emphasized in the configuration GUI for the authorization strategy.

          A possible alternative design would be to let anyone who already has Job/Configure (say) to be allowed to create static agents but mark it somehow (not in agent config.xml) as owned by them or their team, and then block any builds from running on such an agent that are not also associated with them or their team.

          CC danielbeck in case he wants to weigh in, or knows someone who would.

          Jesse Glick added a comment - enterprise environments, where agents can be operated by other teams I have a feeling this is replacing one problem with one another. The recommended setup for Jenkins is to use a Cloud to provision agents from elastic infrastructure on demand, so that there is nothing to manage (except for the initial Cloud setup, which would be done by an administrator). Of course there are cases where you need static agents, like that one Mac Mini in the closet, but the idea is that these should be a small minority that the controller administration team can handle. This feature, however, seems targeted to environments where a broad list of people from various teams (who are not trusted to administer the whole controller) are expected to be connecting agents from all over the place. If you come up with a list of GitHub usernames who should all be allowed general Computer/* permissions (as I think the proposed change does), then they will all be permitted to disconnect, reconfigure, and reconnect each other’s agents. Suppose Team A had set up a folder based on one GitHub repo or org, defined some deployment credentials in it, attached a static agent to Jenkins, and started running builds on it. Now someone from Team B could quietly come along and reconfigure that agent to point to infrastructure they control and from which they could freely copy source code, credentials, and anything else used during the course of Team A’s builds. If that is considered an accepted risk, it should certainly be emphasized in the configuration GUI for the authorization strategy. A possible alternative design would be to let anyone who already has Job/Configure (say) to be allowed to create static agents but mark it somehow (not in agent config.xml ) as owned by them or their team, and then block any builds from running on such an agent that are not also associated with them or their team. CC danielbeck in case he wants to weigh in, or knows someone who would.

          That's true, so to summarise:

          1. This proposal will prevent regular users from escalating themselves to admin
          2. This proposal will not prevent agent owners in a multi-owner agent pool from hijacking each other's agents

          The security impact of point #1 is quite high, as the attacker needs only be a regular user of the system.

          The security impact of #2 is lower, as the attacker must be part of an agent-owner team.

          To avoid scope creep, can we look at pulling #2 into a separate problem? I believe #1 will provide a material benefit for security. As a future step, #2 could introduce an attribute to the agent definition, when an agent will declare a whitelist of user/group(s) allowed to operate on its behalf.

          Andreas Nygard added a comment - That's true, so to summarise: This proposal will prevent regular users from escalating themselves to admin This proposal will not prevent agent owners in a multi-owner agent pool from hijacking each other's agents The security impact of point #1 is quite high, as the attacker needs only be a regular user of the system. The security impact of #2 is lower, as the attacker must be part of an agent-owner team. To avoid scope creep, can we look at pulling #2 into a separate problem? I believe #1 will provide a material benefit for security. As a future step, #2 could introduce an attribute to the agent definition, when an agent will declare a whitelist of user/group(s) allowed to operate on its behalf.

            nygardan Andreas Nygard
            nygardan Andreas Nygard
            Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated: