[JENKINS-16956] Enforce build trigger security #1172
Conversation
…issions, rather than checking the configuring user.
…yet, fall back to running as SYSTEM. Also make a best effort to issue a warning in the log if downstream builds might skipped due to lack of authentication; or if downstream build permissions might not be checked due to legacy behavior.
|
@ikedam you may be interested in this. |
|
core » jenkins-core #426 SUCCESS |
|
core » jenkins-core #428 UNSTABLE |
|
core » jenkins-core #429 UNSTABLE |
|
The change also allows to workaround and close JENKINS-19922 (permission hell for shared projects with triggers, when users or the system add private projects to the trigger's list). |
| @@ -47,6 +47,9 @@ BuildTrigger.NoSuchProject=No such project \u2018{0}\u2019. Did you mean \u2018{ | |||
| BuildTrigger.NoProjectSpecified=No project specified | |||
| BuildTrigger.NotBuildable={0} is not buildable | |||
| BuildTrigger.Triggering=Triggering a new build of {0} | |||
| BuildTrigger.warning_access_control_for_builds_in_glo=Warning: \u2018Access Control for Builds\u2019 in global security configuration is empty, so falling back to legacy behavior of permitting any downstream builds to be triggered | |||
| BuildTrigger.warning_this_build_has_no_associated_aut=Warning: this build has no associated authentication, so build permissions may be lacking, and downstream projects which cannot even be seen by an anonymous user will be silently skipped | |||
| BuildTrigger.warning_you_have_no_plugins_providing_ac=Warning: you have no plugins providing access control for builds, so falling back to legacy behavior of permitting any downstream builds to be triggered | |||
| BuildTrigger.you_have_no_permission_to_build_=You have no permission to build {0} | |||
There was a problem hiding this comment.
It makes sense to change this error message to "User {1} have no permission to build {0}".
There was a problem hiding this comment.
The user name is printed at the start of the build (as of ff1aa0a). The specific project you cannot build is either not known at this stage, because this is a general warning, or is known by Jenkins but cannot be told to the user.
There was a problem hiding this comment.
My objection is no longer true as of the latest commit, so this would now be possible to do. (Ruins existing translations but oh well.)
|
Not possible:
I do not think this PR really changes anything regarding pseudo-triggers, other than removing the weak configuration-time permissions check which did not affect pseudo-triggers at submit time, and removing the permissions check from the form validation that was applied to the UI of pseudo-triggers. Whether they will work or not will depend on what authentication is associated with the upstream project. I discussed the issue in JENKINS-19922; not sure if a change to that belongs in this PR or not. |
| // that has to be treated the same as if there really are downstream projects but the anonymous user cannot see them. | ||
| // For the above two cases, it is OK to suppress the warning when there are no downstream projects, since running as SYSTEM we would be able to see them anyway. | ||
| logger.println(Messages.BuildTrigger_warning_this_build_has_no_associated_aut()); | ||
| auth = Jenkins.ANONYMOUS; |
There was a problem hiding this comment.
For compatibility reasons, don't we need this to be auth = ACL.SYSTEM? Otherwise I think this would cause massive regressions.
There was a problem hiding this comment.
The first two checks should filter away legacy cases for projects without QueueItemAuthenticators.
AFAIK, there is only one public implementation - "Authorize Project" from @ikedam.
This implementation requires the explicit enabling of the JobProperty, so the user should confirm he wants to use specific authorization inside the build.
For this case we can consider the issue as a SECURITY bugfix even if a regression appears
There was a problem hiding this comment.
This last clause is only triggered when you are actually trying to use build authentication, which presumably almost no one in the field is doing yet, because until this PR there was nothing major relying on it (only Copy Artifacts). In this case the authentication is SYSTEM already, and that is the problem: build authentication was configured, yet offered no authentication for this build, so it should not be considered privileged.
|
I started writing my comment here but in the end posted it to https://groups.google.com/d/msg/jenkinsci-dev/KgRSohGjO0o/C9wYu0rf9TAJ |
|
@jglick |
And moving the permissions checks in BuildTrigger from being hardcoded in execute to being overridable in Dependency. This has two benefits: · We can restore the previous form validation logic predicting whether Item.BUILD will be available. · A (true) Trigger could check Item.READ on the upstream project under the authentication that the downstream project would have if it were to be built. (This part is not yet implemented; probably will want to make this replace the current pseudotriggers.)
jglick
changed the title
|
core » jenkins-core #435 UNSTABLE |
|
core » jenkins-core #468 UNSTABLE |
…sions when configuring triggers.
…ince been deleted anyway (configuration-time ACL checks).
…Extension. (It may have a @TestExtension QueueItemAuthenticatorDescriptor if one is needed.)
|
core » jenkins-core #472 UNSTABLE |
|
core » jenkins-core #473 UNSTABLE |
…s displayed in build logs.
|
core » jenkins-core #474 UNSTABLE |
JENKINS-16956: ensure that the user associated a build (or the anonymous user, if unspecified) is able to see and build the downstream project.
Supersedes the placeholder fix of SECURITY-55 (and SECURITY-109): rather than checking permissions at job configuration time, rely on
QueueItemAuthenticators (such as that in theauthorize-projectplugin) to authorize us.Also replaces pseudotriggers with a new true
TriggercheckingItem.READon the upstream project from the perspective of the downstream project’s expected authentication. (Which incidentally supersedesconditional-upstream-trigger-plugin; that implementation was not usable here.)