Remove all default 3rd party privileged plugins

[6543]

As woodpecker do assume a lot of trust to those plugins, if we have no influence we should not by default have them as trusted.

#4056 will future enhance the security model of it,

close #4053

[qwerty287]

Can you remove all? Including our owns. If you don't use the buildx plugin, it's a bad idea to have it privileged by default. Users should know when they enable something that impacts the security.

[woodpecker-bot]

Tearing down https://woodpecker-ci-woodpecker-pr-3918.surge.sh

[codecov]

Codecov Report

Attention: Patch coverage is 55.55556% with 12 lines in your changes missing coverage. Please review.

Project coverage is 26.91%. Comparing base (42e7d71) to head (a17f334).
Report is 25 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/server/setup.go	0.00%	5 Missing ⚠️
pipeline/frontend/yaml/linter/option.go	0.00%	4 Missing ⚠️
pipeline/frontend/yaml/linter/linter.go	81.25%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3918      +/-   ##
==========================================
+ Coverage   26.82%   26.91%   +0.08%     
==========================================
  Files         394      394              
  Lines       27341    27380      +39     
==========================================
+ Hits         7335     7368      +33     
- Misses      19308    19312       +4     
- Partials      698      700       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[6543]

well now we can argue about tradeoffs: harden but make it inconfinient and harder for beginners, or more confinient and beginner freandly but not hardened.

about the security implications, if it whould be that bad to have privileged plugin we should go and delete our buildx anyway ...
e.g. what I try to say instead of fearing that buildx might do things wrong, improve and harden the buildx plugin itselve (we manage it)

[xoxys]

I second the comment from @qwerty287

There are a LOT of other ways to build container images that don't require privileged containers, so not everyone needs this plugin out of the box. However, there are still valid reasons to use buildkit, but I would not escalate any plugin by default. This should be a decision made by the server admin, and the admin should know the impact before enabling it globally. Setting one config option more (is a simple list value) is not inconvenient IMO.

[lafriks]

This PR deals with risk that 3rd-party plugins are trusted by default, I think we should keep our plugin buildx trusted for now by default until there is better plugin for building docker images that does not require special privileges

[xoxys]

Ok then I would like to block this PR until we discussed it further. For me, this doesn't make sense, the buildx plugin is a high risk plugin and as the next release will be a breaking release anyway we should drop it now all together.

[zc-devs]

better

Too abstract.

plugin for building docker images that does not require special privileges

They all require special privileges.
But if we are talking about privileged alone, then there is a choice:
https://github.com/woodpecker-ci/plugin-kaniko
https://github.com/maltegrosse/woodpecker-buildah
https://codeberg.org/Taywee/woodpecker-buildah/

[lafriks]

That's my point that there currently are no good options that would be compatible with docker buildx and would not require special privileges. For me it does not make sense from a security perspective to keep plugins/docker etc in trusted list as they are external plugins and could be abused if someone pushes malicious image to them (this is the risk this PR solves). Later on we can discuss if we need to remove plugin buildx also but keeping this blocked just because of that does not make the situation better but worse imho

[xoxys]

We have 3:2 votes for removing all default trusted plugins. I don't see why we should keep it? It's pretty simple to enable it server-wide again if (and only if) a server admin is aware of the impact. There are way more high impact changes in the upcoming v3 than this one.

Later on

Later on, we would need yet another major release to change it.

[lafriks]

But I think most instances will still need to add it anyway as nowadays containers are pretty much a way to go for deployment, so I don't really think this will improve anything apart to make 90% (wild guess and might be exaggeration 😁) this change that they will find out mostly by builds not working without really improving security as with our current limitations set by server there is not really a known way of abusing buildx plugin

[pat-s]

I am fine with this change going into 3.0 IF we better communicate what needs to be done to get the functionality back for internal instances.
We already communicated the removal of the environment: section in plugins badly lately and caused a lot of trouble.
Such PRs should IMO also include changes to the docs and refs to other PRs which update the docs there (e.g. here to an update in the buildx plugin which explains this change vice-versa).
-> I am blocking this PR as well until we get better at cross-updating all places which are affected by such changes instead of only changing the core codebase.

I can see why it should be not enabled/trusted by default and having a focus on security is surely not a bad thing. At the same time, having a focus often comes with inconvenience as, this is not just an issue of WP or CI in general.

[6543]

WOODPECKER_ESCALATE='' did not work in the past ... now it does
WOODPECKER_ESCALATE=' ' would have done it too...

also we clearly indicate via an lint error when the pipelines are still in usage, so users migrating from drone or with old config don't debug hours and hours

[6543]

@xoxys is your concern addressed?

[6543]

@pat-s added the linter error so users get notified first hand

[qwerty287]

This leaves out important parts. With privileged mode, you can do a lot more than just accessing docker.

[6543]

well could you suggest a better wording please

[xoxys]

I'll merge this pull and we can discusst the rest on

Why do you make a poll if you then decide on personal preferences? However, I dont have anything to add to the discussion anymore.

[6543]

Why do you make a poll ...

that is currently +3 & -2 vs +3 & -3 ... so that's no clear answer at all ...

addressed

[qwerty287]

It's fine to merge this for me as these should be removed in any case. We can discuss removing buildx in the other pr too and I also still think we should remove all

[Daniel451]

@6543 I am late to the party and since I also just very recently started using Woodpecker, I would opt to not vote anyways and leave this to maintainers / long-term users.

However, I wanted to share something important about that: the one reason I migrated to Woodpecker (except being open-source) is: simplicity. It looked nice, setup was easy, and migration was easy.

I cannot speak for other / new users and increasing security is always a good idea but I would really like to see Woodpecker keeping setup, configuration, ... as easy as possible. In my use-case, for example, I am running the agent on a separated server that literally runs the Woodpecker agents only, thus security there is not a big concern. Easy setup & configuration, on the other hand, is something that will always impact all users.

Just my 2 cents :)

[xoxys]

As long as the dedicated nodes used as agent runner are not isolated from the rest of you network and disconnected from the internet, security should be a concern.

We were talking about full node takeover incl. root access here. Thus setting a single env var to activate privileged access if and only if it is needed is not complex or hard to achieve.

[Daniel451]

@xoxys thank you for the input. I agree, as stated before:

increasing security is always a good idea

...that security should always be of concern. However, I believe that the impact of increased security concerns a subset of Woodpeckers users only whereas any change that impacts "ease of setup" or "ease of use" will, by definition, always impact all users.

setting a single env var to activate privileged access if and only if it is needed is not complex or hard to achieve.

I agree that solutions like a single env var can be considered very "lightweight" w.r.t. configuration and yes, this would not render documentation, setup, or use "too complex" all of the sudden. I just like to remind about complexity in situations like that. For example, harbor (the private docker registry) also started off quite simple and nowadays you do not even have a docker compose file for that thing anymore but an actual install script, which creates like a 200 loc compose file out of other config files because they added one var here, one service there, and at some point it reached the level of complexity that a setup script was needed.

Back to topic: I agree that there would be solutions where impact on ease of setup / use are minimal in this case.

As long as the dedicated nodes used as agent runner are not isolated from the rest of you network and disconnected from the internet, security should be a concern.
We were talking about full node takeover incl. root access here.

I am in slight disagreement here. I am working at a non-profit and even in this environment we at least had time to limit the agent's network access to docker hub for pulling 3rd party stuff, our woodpecker server, and our private docker registry. The host itself runs nothing but the agent itself. The CI pipeline is used by our staff only. Thus it would take one of our own developers with bad intentions (or not checking a 3rd party base image) to even introduce an attacker and even then the attack surface is quite limited, as root on the agent's host grants you access to nothing (and for ease of use for our devs woodpecker server and our private docker registry are, of course, reachable via internet, thus if someone wanted to brute-force or ddos or ... they could do that already without taking over the agent's host).

[6543]

Well normal abuse is only about resource abuse ... but asap you can extract secrets or take over a whole host we should have good defaults in place.

And nothing beside building docker is affected ... I get that it's inconvinient :/

And I dont have a better option atm.

As soon as we can do the same without having to forward an docker socket into an step, the problem will be solved for all users, so we should focus on that