Why we cannot use environment with settings?

[janydoe]

Ref: #3909

To reduce the amount of the same type of code, I use a simple plugin to deploy docker in swarm.

That's why I need settings, secrets, and an environment. Can we enable this behavior in the settings? For example for the trusted plugins???

steps:
  - name: deploy-to-staging
    image: ${ORG_REGISTRY}/drone_plugins/docker_stack
    pull: true
    settings:
      name: mailer
    secrets:
      - secret_key
    environment:
      API_IMAGE: ${ORG_REGISTRY}/${CI_REPO}:${CI_COMMIT_SHA:0:8}

At the moment i force to use 2.6.0

[qwerty287]

Sorry for letting you waiting so long for an answer, but I'll try to explain it:

Plugins can have access to special, possibly dangerous features without additional validations (some of them like docker-buildx are set as privilieged: true by default). However, you can use environment variables to control the behaviour of the plugin and this could allow executing what you want, not what the plugin is built for. In the worst case, this can allow taking over the agent. Plugins should only be used with the settings because then the plugin authors can control what's configurable and therefore prevent dangerous options.

What you can do now:

1. Manually mark your repo as trusted and set privileged: true on the step.
2. Ask the plugin authors to include a setting to control the variable if it can't be abused.

I hope this helps you.

[zc-devs]

As GHSA-3wf2-2pq4-4rvc

1. apparently is not so high severity,
2. but Disallow to set arbitrary environments for plugins #3909 is a huge breaking,
3. and the next major is coming,

I would revert #3909 for 2.7.1. Optionally, Remove all default privileged plugins (#3918) (it has lower impact, because it's an instance setting, instead of pipeline config).

---

Also there are the secrets, which you still can use in the plugins and they work similar to the envs, seems.

[lafriks]

I would say that secrets should have the same limitations as env.

My suggestion would be remove secrets section at all, allow back environment with denylist limitations of envs that could be abused (system envs and woodpecker envs) that would not be allowed to use for plugins. This way we can have more control and better UX for users

[qwerty287]

I would revert #3909 for 2.7.1.

Would be fine for me, but only with a warning about this issue.

Also there are the secrets, which you still can use in the plugins and they work similar to the envs, seems.
I would say that secrets should have the same limitations as env.

Indeed, secrets should be blocked for plugins too. (Whether we drop it completely, also for normal steps, is another question.)

allow back environment with denylist limitations of envs that could be abused

This list would literally contain every env var. We don't know which plugins are given this privilege and which features they allow. If it's possible to set any env var you want, there might be a plugin that contains an application and this app can do dangerous things if an env var is set.

[zc-devs]

Offtopic

First off, from application design and spirit of plugins I agree, that environment and secrets should be removed from plugin syntax.

---

Activity around both of the issues reminds me

Don't get me wrong: this is OK for urgent time like it was.

But some time is passed and we have a better understanding of the issues. So, what do we have today?

Some issues are fixed/hacked, some aren't.
GHSA-3wf2-2pq4-4rvc has not been fixed, because of secrets, but there is breaking change in a minor release. So, to be consistent, in 2.7.1 either remove secrets or bring back envs. Points for later was given in #3959 (reply in thread).

---

If it's possible to set any env var you want, there might be a plugin that contains an application and this app can do dangerous things if an env var is set

Because plugins can expose secrets that otherwise would be protected by plugin functionality

It is hard to understand without the examples, TBH. And details of the issues would be revealed ... when, BTW?

info will be published later once we got adoption of the update

Do you have a statistic?

At the moment i force to use 2.6.0

---

But let me go back to the examples. I started to write some in reply to xoxys. But almost all examples crash against

If someone has the rights to change env vars in a CI config, the one can also change the entire step or add a new one. In this new step, everything can be executed again.

So, you can execute arbitrary code, right? What is this "dangerous" thing then?

can lead to a host takeover

How? If isolation doesn't work. Why? Because you run container in privileged mode. How?

1. Trusted repo with privileged step. This is not a case of this issues/patches.
2. Not trusted repo, but some piece of code runs with implicit privileged. Bingo!

So, GHSA-3wf2-2pq4-4rvc has high severity only in case when some privileged plugin can be executed.

That is funny sad, how the root cause is defended to be alive for another year.

Also, that is why I proposed to remove all default privileged plugins in 2.7.1.

[qwerty287]

there is breaking change in a minor release

We said that it's fine because it was never documented that you can use env vars together with settings.

So, GHSA-3wf2-2pq4-4rvc has high severity only in case when some privileged plugin can be executed.

Yes, and this is the core "problem" here.

That is funny sad, #3918.

However, we don't know which plugins users will put in there (yes, it's their responsibility). Now, let's look at public instances like codeberg CI - if they remove all plugins from this list, nobody is able to build multiarch docker images anymore.

Plugins that are given extended privileges without an admin explicitly enabling the trusted setting must be as restrictive as possible to prevent anyone being able to abuse a plugin.

The defaults are only one part of the root cause, not the root cause itself. Yes, having a default there is bad, but you can disable it if you don't need it. But what if you need it?

The plugins should only be allowed together with the options the plugin authors want to allow. This means: Only allow settings. Env vars or secrets (which are env vars in the end too) could lead to different behaviors.

Also, that is why I proposed to remove all default privileged plugins in 2.7.1.

That's a bit problematic as it's a breaking change and thus incompatible with semver (but I get your point and would agree to it).

Unprivileged plugins if abused can also expose secrets so this is the risk not only for plugins that require privileged permissions

Yes, that's another issue.

---

So, to sum this up again what I'd do now (GitHub is really bad at coming to a conclusion somehow…):

• if you think it's necessary, allow environment again in 2.7.1, but only with a big warning
• remove default privileged plugins in 3.0
• disallow both environment and secrets in 3.0

I can prepare PRs for those not done yet, but I'd like to have this approved first.

[lafriks]

I don't see how releasing 2.7.1 would help as they can still use 2.6.0 and won't be able to use that undocumented feature in 3+ anyway

[zc-devs]

Unprivileged plugins if abused can also expose secrets so this is the risk not only for plugins that require privileged permissions

This crashes about xoxys's comment.

Consider example:

 steps:
   - name: test-plugin
     image: my-plugin
     environment:
       settings:
         token:
           from_secret: secret_token

I dont have to abuse plugins:

 steps:
   - name: test-plugin
     image: my-plugin
     environment:
       settings:
         token:
           from_secret: secret_token
  - name: test-step
    image: alpine
    environment:
      SECRET:
        from_secret: test_secret
    commands:
      - echo $$SECRET | sed -e 's/\(.\)/\1\ /g'
      - # or send this token to wherever I want

We said that it's fine because it was never documented that you can use env vars together with settings.

Yeah. My bad, sorry.

Edit: However, does schema serves for documentation purposes?
Edit 2: While de jure it is correct, de facto we can observe broken pipelines. And the point was not fixed issue && broken pipelines, which should be transformed to disjunction.

if they remove all plugins from this list, nobody is able to build multiarch docker images anymore

Why not, if you have nodes on this architectures. Sure, it would be more difficult, but not impossible.

The defaults are only one part of the root cause, not the root cause itself

Yes. And this is the first part I would start from.

Yes, having a default there is bad, but you can disable it if you don't need it

@qwerty287, @lafriks can I call it "insecure by default"? or "security time bomb issue by default"?

Edit: To be honest, doesn't impact me, cause I use Kubernetes and can set up baseline or restricted PSS. But this doesn't decline previous "by default" statement. Also, don't know if Docker users have some feature like Kubernetes does.

But what if you need it?

Then enable :) Xoxys sad multiple times.

The plugins should only be allowed together with the options the plugin authors want to allow ...

I agreed on that.

if you think it's necessary, allow environment again in 2.7.1, but only with a big warning

I want it to be consistent. As Disallow secrets for plugins #4027 should be backported, you go first approach from #3959 (comment). That is fine for me.
Edit: ⬆️ it's not clear which way #4027 is going.
I want it to be consistent: (fix the issue and remove secrets from plugins syntax) or (leave the issue and bring back envs). As per #4027 (comment) it could be the first approach for 3.0 and the second for 2.7.

[lafriks]

Unprivileged plugins if abused can also expose secrets so this is the risk not only for plugins that require privileged permissions
I dont have to abuse plugins:

 steps:
   - name: test-plugin
     image: my-plugin
     environment:
       settings:
         token:
           from_secret: secret_token
  - name: test-step
    image: alpine
    environment:
      SECRET:
        from_secret: test_secret
    commands:
      - echo $$SECRET | sed -e 's/\(.\)/\1\ /g'
      - # or send this token to wherever I want

Actually you won't be able to abuse it directly if secret is set to be allowed to be used for specific plugin. But if we allow plugins to have env/secrets set we can abuse to have "trusted" plugin to get secret and abuse plugin to expose this secret

[zc-devs]

if secret is set to be allowed to be used for specific plugin

Screenshot 2024-08-11
You are right. Thank you for explanation.

[qwerty287]

I actually see a problem there I wasn't aware of that it's completely blocked to use env and settings…

[qwerty287]

#4027 should address this. If you're not fine with it please comment there.

[qwerty287]

This is possible again now.