GitLab CI gives the possibility to enable debug job logging, which makes usage of shell's debugging features.
For all shells that GitLab CI supports, it means that the content of variables will be exposed. Therefore from the very beginning we've been warning about security implications and that enabling this feature when job output is public is dangerous - in the docs we can see:
Warning: Enabling debug tracing can have severe security implications. The output will contain the content of all your variables and any other secrets! The output will be uploaded to the GitLab server and made visible in job logs!
(...)
Before enabling this, you should ensure jobs are visible to team members only. You should also erase all generated job logs before making them visible again.
Some time ago we've made it possible to fully disable this feature. But this option have two limitations:
It must be set on the Runner side, so only Runner administrator is able to disable it.
It affects all jobs handled by the configured Runner.
And sometimes it's very useful to be able to get this debug output, especially on the proper target environment. So in many cases runner administrators will probably leave it undefined.
The proposal here is to update GitLab and make the job page of every job that uses CI_DEBUG_TRACE=true configured on GitLab side (so in the group or project CI/CD Variables settings or through .gitlab-ci.yml file) accessible only for people who have developer or higher permissions in the project. We could even consider making it accessible for only the maintainers or higher (who already have access to the secrets defined in the variables). But this would make the CI_DEBUG_TRACE feature less usable for a regular developer who is probably the most interested in using it (to debug the changes implemented in the CI/CD configuration and scripting).
Having this hidden from the wide public, we could also show some big and noticeable warning at the top of the page, repeating the warning from the documentation (and specifying that because of that access to this job page is limited).
Anyway, with this change, in the case when the user haven't noticed the warning in the docs or haven't fully understand the implications, the security risk of revealing the secrets will be highly reduced.
Of course the same >=developer or >=maintainer access should be required also for the API access for such jobs.
Proposal
Update access controls so that only Developers and above can access the job page for jobs where CI_DEBUG_TRACE=true is configured on GitLab side (via group or project CI/CD Variables settings or the .gitlab-ci.yml file).
Availability & Testing
Unit test changes - required, please add tests for all new implemented logics
Integration test changes - required, please add test for different user access with correct expected response from UI and API.
I've marked it as confidential, to because of the security label and to not increase the knowledge of CI_DEBUG_TRACE being able to reveal some secrets. We already document it, but I think it's reasonable to not increase this awareness. Especially that the CI_DEBUG_TRACE variable could be added by an attacker to a template, that is included in other projects (so getting access to one popular source of templates may affect multiple less popular projects)...
@thaoyeager@darbyfrey Do you think this could be handled in near future? The change doesn't seem huge - we have most blocks like authorization and job assigned variables access available, and it's just a matter of putting all of this together.
With more and more people joining the team, I expect situations like this one - https://gitlab.com/gitlab-com/gl-security/secops/operations/-/issues/757 - to happen more often. Having a decent solution for reducing the impact of such unintentional secrets revealing, for both us and our customers, feels important.
Thank you for creating this issue and collaborating in the discussion so far, @tmaczukin! Unnfortunately given some more critical Security issues raised recently (the last couple ~S1 ~P1 Security incidents CI had are related), we'll be addressing them first. https://gitlab.com/gitlab-org/gitlab/-/issues/221070 has more details on the collaboration between Appsec and Engineering on this.
This is a very interesting and important issue. The question that I would like to ask @tmaczukin is whether it is possible to set CI_DEBUG_TRACE as a runner variable, and this way GitLab would not know about the fact that a runner sends a debug trace.
Probably the most simple fix here is to check if CI_DEBUG_TRACE is set on the GitLab side, this might be the quick fix, but in order to fix that properly I think that the runner should send this information the it is a detailed trace.
Let me set weight 2 for the time being, for the quick fix, we can always bump it once we know what the proper fix might be.
The question that I would like to ask @tmaczukin is whether it is possible to set CI_DEBUG_TRACE as a runner variable, and this way GitLab would not know about the fact that a runner sends a debug trace.
Yes, it is. However: it would be not wise.
And I don't think it's popular. You usually want to use it in context of a specific job for a specific change, to debug why things are failing. While setting this in Runner configuration, makes it forced for all jobs handled by such [[runners]] runner... so visible for all and always. It's really hard to imagine that it could be done "by mistake", and I'd really like to believe, that such configuration - if exists - is made consciously, being aware of the security concerns.
Anyway, I fully agree with this:
Probably the most simple fix here is to check if CI_DEBUG_TRACE is set on the GitLab side, this might be the quick fix, but in order to fix that properly I think that the runner should send this information the it is a detailed trace.
Let's start with checking this on GitLab side and rise required permissions then.
In next step we can create another job metadata field like trace_debug boolean (not sure where to put it, since ci_builds is already too big...) that would be set to true on detection of CI_DEBUG_TRACE and the permission would act depending on this field.
And as the third step, we could extend the Runner API for the trace update or trace patch, to send information about trace_debug=(true|false) depending on what's detected on the Runner side (since at the end the decision about enabling debug trace is happening in Runner). This could then cover all possible cases where CI_DEBUG_TRACE can be set
As discussed today in an RCA concerning credential leakage, there's a point that we should keep in mind: protecting job logs only when CI_DEBUG_TRACE is set to true will most likely be insufficient.
Although the CI_DEBUG_TRACE flag has proven to be problematic if not used with caution, it's not the only way to leak secrets. The more abstract problem seems to be that we let Maintainers make mistakes.
Allowing Maintainers to modify artifacts whose output ultimately lands in a job's log, which is a normal thing for Maintainers to do, forces us to deal with all infinite ways in which sensitive variables could possibly be leaked: CI_DEBUG_TRACE, echo, env and more. This is, I argue, impractical.
Instead of approaching this problem in a case-by-case basis we could:
Fix CI/CD variable masking, so that there are no restrictions w/r/t format, length or content. How to best do this is to be defined, but it is clear, that there are plenty scenarios where CI/CD variables should be masked even if they are too complex in format (service accounts)
We could make job logs, or CI/CD logs in general, a first citizen w/r/t to permissions, allowing users to set the visibility of these artifacts to "public/internal/private" or define what access level should be able to view them "reporter/developer/maintainer/owner"
Both the above would keep a Maintainer from making mistakes and thus leading to credential leakage.
If we fix variable masking, no amount of echo, env, etc will circumvent variable masking, as even complex variables that are unprotected due to their complexity and length today will be masked.
If we make job logs, or CI/CD logs in general, a first citizen w/r/t to permissions, Maintainers can make mistakes w/r/t what they've echoed or not, or even forget to mask a variable, but at least it won't be out in the open. Internal leaks are another beast, but I argue that taking this approach also helps in that case, as non-members of a project won't be able to see job log output in case of leakage.
Fix CI/CD variable masking, so that there are no restrictions w/r/t format, length or content. How to best do this is to be defined, but it is clear, that there are plenty scenarios where CI/CD variables should be masked even if they are too complex in format (service accounts)
I agree variable masking is a clean solution to protect user from accidentally leaking sensitive information. The way GitHub is handling this masked variables is by enforcing masked variable requirements as follow
Be in a single line.Be at least 8 characters long.Not be a predefined or custom environment variable.Consist only of characters from the Base64 alphabet (RFC4648). In GitLab 12.2 and newer, @ and : are also valid values.
I am thinking the reason for these requirements is it is difficult to mask infinite variation of sensitive information. But the direction is clear and could be a selling point of our CI/CD service, if we can develop a CI/CD that would automatically mask all the sensitive info without their effort that'd be great.
We could make job logs, or CI/CD logs in general, a first citizen w/r/t to permissions, allowing users to set the visibility of these artifacts to "public/internal/private" or define what access level should be able to view them "reporter/developer/maintainer/owner"
For now, GitLab allows Public and Private job logs, so we should be good if user set the artifacts to private only, what we are working on here is the ones that user chose to publish their CI/CD logs
For now, GitLab allows Public and Private job logs, so we should be good if user set the artifacts to private only, what we are working on here is the ones that user chose to publish their CI/CD logs
@rchan-gitlab that's great! I do wonder, however, what the access scheme looks like, i.e. in case "artifacts are private" is set to true, do all members of the group get access to the logs? This helps mitigate the risk of external parties gaining access to mistakenly disclosed variables, but an internal leak is still possible.
Turning the check-box into a drop-down minimum level access required to access CI/CD artifacts that allows to users to choose between reporter/ developer/ maintainer/ owner could be valuable to us and our customers.
IMO we should put this up for discussion w/r/t to our company policy:
Making gitlab-com artifacts (pipelines and logs) private by default? The potential risk to us currently is too great and our tooling as well as approaches to prevent such scenarios clearly insufficient.
I also argue that although external parties may derive value from Wikis, Repos and others where we document our processes, job logs, especially in company-related projects, provide zero value to external parties besides malicious users or attackers.
I also argue that although external parties may derive value from Wikis, Repos and others where we document our processes, job logs, especially in company-related projects, provide zero value to external parties besides malicious users or attackers.
They are useful for users debugging a build, a test or anything where the crash is documented in the job log. For open source projects those users won't necessarily be members of the project.
I do wonder, however, what the access scheme looks like, i.e. in case "artifacts are private" is set to true, do all members of the group get access to the logs? This helps mitigate the risk of external parties gaining access to mistakenly disclosed variables, but an internal leak is still possible.
With artifacts set to private, members with Reporter or above role would be able to access the job logs
@jdsalaro You should open new issues for those two concerns (removing the limitations on variable masking), improve the logs permission handling. That way we can prioritize and evaluate them independently from this.
If we restrict access to the job page, wouldn't the logs still leak via artifacts? i.e. someone will be able to download the artifacts and access the jobs.
The long term solution could be to introduce per-job visibility settings.
For the short-term solution, maybe we could implement a new condition in the policy (for debug jobs), and prevent access to builds and/or artifacts for debug jobs? Instead of restricting access to the build page via a before action. WDYT @grzesiek?
@fabiopitino Any thoughts on this? Looks like this may have to push this to ~"candidate::13.6" but I'm also wary of leaving a security issue with priority2severity2 too far past its due date. 13.6 would mean Nov 17th completion, which is 3 months after what the current due date reads.
Probably the most simple fix here is to check if CI_DEBUG_TRACE is set on the GitLab side, this might be the quick fix, but in order to fix that properly I think that the runner should send this information the it is a detailed trace.
@cheryl.li Moving this issue to ~"candidate::13.6" and out of %13.4 since it is moved back to workflowplanning breakdown; if we can get the approach figured out it might be possible to move this issue into %13.5.
@marcel.amirault / @thaoyeager, I think we may want a message here so that users aren't wondering why they are denied. The default for access denied is a 404 but this is for cases where we may not even want other users to know the resource exists. In this case users can still see the job in the job list page already since they have read_build access we just do not want them seeing the debug logs.
So far, I used the existing forbidden template and added the message:
You must be a developer or higher for the associated project to view the build details if debug trace is enabled. You can disable the debug trace by unsetting 'CI_DEBUG_TRACE=true' wherever variables are set for this job.
@allison.browne What you have here is already pretty good, but I think we can give it a slight touchup.
I don't think you need a GitLab admin either, just a project maintainer?
How about:
You don't have permission to access this page (Alternative -> You cannot access this page)
You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings.
If you need to view this job log, a project maintainer must add you to the project with developer permissions or higher.
@fabiopitino, @matteeyah, I assume we wouldn't want to forbid the user every place where we use :read_build, for instance, it still seems safe to expose artifacts and code coverage reports in this case? The artifacts should not expose the variables.
There's a slight chance that build artifacts might contain variable information - e.g. the build script dumps some of the variable information inside of the artifact. This is an unlikely scenario, I just wanted to voice that this possibility does exist.
There's a slight chance that build artifacts might contain variable information
@matteeyah I think this is regardless of the CI_DEBUG_TRACE problem, right? CI_DEBUG_TRACE=true enables a more verbose trace coming from the Runner. Whatever the user does with the script (like dumping variables into artifacts) goes beyond this security issue and will be a problem even if CI_DEBUG_TRACE is disabled.
If we restrict artifacts download we could potentially fail a pipeline that depends on that artifact if permissions suddenly change:
build:needs:-project:org/the-projectjob:the-dependency# this job used CI_DEBUG_TRACE=trueartifacts:true
The build job would normally work for a user with developer access but if someone sets CI_DEBUG_TRACE=true for that job then user of build job would need maintainer access.
@allison.browne I think it's important to keep the UX consistent. If in the UI we show a 403 page for the Job page, we are essentially restricting access to the whole job. If we display the 403 message instead of the job log we would restrict only the log, keeping all the other details available. Same UX IMO should exist with the API.
@allison.browne I think JobsController#raw is another one we should restrict as it renders the raw/unprocessed trace coming from the Runner. But perhaps we don't need to restrict JobsController#show as job logs are requested separately by frontend via JobsController#trace.
Thanks, @fabiopitino, I agree, I think that makes sense to only restrict the trace/log and not the whole job as was initially specified. In that case, I think introducing a new Ability like :read_build_trace makes sense.
@dcouture Is there anything in this issue preventing us from opening it? I don't see the keep confidential label, but maybe there was some information that was shared in the discussion and should be kept confidential (and I don't see it).
This issue was replaced with #290955 (closed) and few other issues, which all are already handled, closed and opened to public