Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

By default enforce check of clone image via allow-list by image name and tag #4056

Closed
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a8fa090
Extend utils.MatchImage by add utils.MatchImageDynamic and utils.Matc…
6543 Aug 25, 2024
9a59c5b
allow-list trusted- and escalated-plugins by exact match
6543 Aug 26, 2024
553b095
document one change
6543 Aug 26, 2024
95f8b6d
document MatchImageDynamic usage for secret plugin filer
6543 Aug 26, 2024
fa83a29
update privilged plugins to allow list
6543 Aug 26, 2024
5dfed30
lint against clone steps with untrusted images
6543 Aug 26, 2024
1d490b7
could be more
6543 Aug 26, 2024
bcea359
Merge branch 'main' into plugin-secret-exact-filter-if-tag-set
6543 Aug 26, 2024
6f60814
Merge branch 'main' into plugin-secret-exact-filter-if-tag-set
6543 Aug 29, 2024
689f7f8
Merge branch 'main' into plugin-secret-exact-filter-if-tag-set
6543 Aug 31, 2024
0e08318
Merge branch 'main' into plugin-secret-exact-filter-if-tag-set
6543 Aug 31, 2024
82d2103
Merge branch 'main' into plugin-secret-exact-filter-if-tag-set
6543 Aug 31, 2024
b95281b
Apply suggestions from code review
6543 Aug 31, 2024
9336afb
clean
6543 Aug 31, 2024
f81a86e
clean
6543 Aug 31, 2024
9fc5be8
Revert "clean"
6543 Aug 31, 2024
c29a416
clean fix
6543 Aug 31, 2024
1bf9001
Merge branch 'main' into plugin-secret-exact-filter-if-tag-set
6543 Sep 1, 2024
0b357ea
Merge branch 'main' into allow-trusted-and-privileged-plugins-to-be-f…
6543 Sep 1, 2024
8b39ef1
make no-tag mach as of now
6543 Sep 1, 2024
68560af
rename
6543 Sep 1, 2024
184a13b
make trustedClonePlugins an setting for admins
6543 Sep 1, 2024
4501f88
cli lint respect custom trusted clone plugins
6543 Sep 1, 2024
b8a62d8
Update docs/docs/30-administration/10-server-config.md
6543 Sep 1, 2024
bd6aff7
pipeline compiler: init defaultClonePlugin the same and respect 0 if …
6543 Sep 1, 2024
052668a
clean: all about trusted clone
6543 Sep 1, 2024
53a9a3f
clean docs
6543 Sep 1, 2024
c90b4de
clean docs (2)
6543 Sep 1, 2024
1710cb6
add one more info
6543 Sep 1, 2024
54f9d0a
add test
6543 Sep 1, 2024
3d76ba8
Merge branch 'allow-trusted-and-privileged-plugins-to-be-filtered-via…
6543 Sep 1, 2024
d769bc3
Merge branch 'main' into plugin-secret-exact-filter-if-tag-set
6543 Sep 1, 2024
f954bcb
do
6543 Sep 1, 2024
1422752
Update shared/constant/constant.go
6543 Sep 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/docs/20-usage/40-secrets.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ Please be careful when exposing secrets to pull requests. If your repository is

To prevent abusing your secrets from malicious usage, you can limit a secret to a list of plugins. If enabled they are not available to any other plugin (steps without user-defined commands). If you or an attacker defines explicit commands, the secrets will not be available to the container to prevent leaking them.

:::note
if you specify an tag, the filter will also respect it.
just make sure you don't specify the same image without one, this will lead to ignore it again.
:::

![plugins filter](./secrets-plugins-filter.png)

## Adding Secrets
Expand Down
1 change: 1 addition & 0 deletions docs/docs/30-administration/10-server-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ a user can log into Woodpecker, without re-authentication.
> Defaults are defined in [shared/constant/constant.go](https://github.com/woodpecker-ci/woodpecker/blob/main/shared/constant/constant.go)

Docker images to run in privileged mode. Only change if you are sure what you do!
You should specify the tag as images are compared via exact matches.
6543 marked this conversation as resolved.
Show resolved Hide resolved

<!--
### `WOODPECKER_VOLUME`
Expand Down
2 changes: 2 additions & 0 deletions docs/docs/91-migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Some versions need some changes to the server configuration or the pipeline conf

## `next`

- If secret who are filtered by plugin now check against tag if you specify a tag
- Only allow trusted-clone- and escalated-plugins if image name **AND TAG** matches (before the tag was ignored)
- Removed `WOODPECKER_DEV_OAUTH_HOST` and `WOODPECKER_DEV_GITEA_OAUTH_URL` use `WOODPECKER_EXPERT_FORGE_OAUTH_HOST`
- Compatibility mode of deprecated `pipeline:`, `platform:` and `branches:` pipeline config options are now removed and pipeline will now fail if still in use.
- Removed `steps.[name].group` in favor of `steps.[name].depends_on` (see [workflow syntax](./20-usage/20-workflow-syntax.md#depends_on) to learn how to set dependencies)
Expand Down
2 changes: 1 addition & 1 deletion pipeline/frontend/yaml/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (s *Secret) Available(event string, container *yaml_types.Container) error
return fmt.Errorf("secret %q only allowed to be used by plugins by step %q", s.Name, container.Name)
}

if onlyAllowSecretForPlugins && !utils.MatchImage(container.Image, s.AllowedPlugins...) {
if onlyAllowSecretForPlugins && !utils.MatchImageDynamic(container.Image, s.AllowedPlugins...) {
return fmt.Errorf("secret %q is not allowed to be used with image %q by step %q", s.Name, container.Image, container.Name)
}

Expand Down
2 changes: 1 addition & 1 deletion pipeline/frontend/yaml/compiler/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func (c *Compiler) createProcess(container *yaml_types.Container, stepType backe
environment[requested] = secretValue
}

if utils.MatchImage(container.Image, c.escalated...) && container.IsPlugin() {
if utils.MatchImageExact(container.Image, c.escalated...) && container.IsPlugin() {
6543 marked this conversation as resolved.
Show resolved Hide resolved
privileged = true
}

Expand Down
23 changes: 23 additions & 0 deletions pipeline/frontend/yaml/linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
errorTypes "go.woodpecker-ci.org/woodpecker/v2/pipeline/errors/types"
"go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/linter/schema"
"go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/types"
"go.woodpecker-ci.org/woodpecker/v2/pipeline/frontend/yaml/utils"
"go.woodpecker-ci.org/woodpecker/v2/shared/constant"
)

// A Linter lints a pipeline configuration.
Expand Down Expand Up @@ -71,6 +73,10 @@ func (l *Linter) lintFile(config *WorkflowConfig) error {
linterErr = multierr.Append(linterErr, newLinterError("Invalid or missing steps section", config.File, "steps", false))
}

if err := l.lintCloneSteps(config); err != nil {
linterErr = multierr.Append(linterErr, err)
}

if err := l.lintContainers(config, "clone"); err != nil {
linterErr = multierr.Append(linterErr, err)
}
Expand All @@ -94,6 +100,23 @@ func (l *Linter) lintFile(config *WorkflowConfig) error {
return linterErr
}

func (l *Linter) lintCloneSteps(config *WorkflowConfig) error {
if len(config.Workflow.Clone.ContainerList) == 0 {
return nil
}
var linterErr error
for _, container := range config.Workflow.Clone.ContainerList {
if !utils.MatchImageExact(container.Image, constant.TrustedCloneImages...) {
6543 marked this conversation as resolved.
Show resolved Hide resolved
linterErr = multierr.Append(linterErr,
6543 marked this conversation as resolved.
Show resolved Hide resolved
newLinterError(
"Specified clone image does not match allow list, netrc will not be injected",
config.File, fmt.Sprintf("clone.%s", container.Name), true),
)
}
}
return linterErr
}

func (l *Linter) lintContainers(config *WorkflowConfig, area string) error {
var linterErr error

Expand Down
4 changes: 4 additions & 0 deletions pipeline/frontend/yaml/linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ func TestLintErrors(t *testing.T) {
from: "{pipeline: { build: { image: golang, settings: { test: 'true' } } }, when: { branch: main, event: push } }",
want: "Additional property pipeline is not allowed",
},
{
from: "{steps: { build: { image: golang, settings: { test: 'true' } } }, when: { branch: main, event: push }, clone: { git: { image: woodpeckerci/plugin-git:v1.1.0 } } }",
want: "Specified clone image does not match allow list, netrc will not be injected",
},
}

for _, test := range testdata {
Expand Down
2 changes: 1 addition & 1 deletion pipeline/frontend/yaml/types/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,5 +126,5 @@
}

func (c *Container) IsTrustedCloneImage() bool {
return c.IsPlugin() && utils.MatchImage(c.Image, constant.TrustedCloneImages...)
return c.IsPlugin() && utils.MatchImageExact(c.Image, constant.TrustedCloneImages...)

Check warning on line 129 in pipeline/frontend/yaml/types/container.go

View check run for this annotation

Codecov / codecov/patch

pipeline/frontend/yaml/types/container.go#L129

Added line #L129 was not covered by tests
6543 marked this conversation as resolved.
Show resolved Hide resolved
}
41 changes: 40 additions & 1 deletion pipeline/frontend/yaml/utils/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@

package utils

import "github.com/distribution/reference"
import (
"strings"

"github.com/distribution/reference"
)

// trimImage returns the short image name without tag.
func trimImage(name string) string {
Expand Down Expand Up @@ -57,6 +61,41 @@ func MatchImage(from string, to ...string) bool {
return false
}

// MatchImageExact returns true if the image name and tag matches
// an image in the list.
func MatchImageExact(from string, to ...string) bool {
from = expandImage(from)
for _, match := range to {
if from == expandImage(match) {
return true
}
}
return false
}

// MatchImageDynamic check if image is in list based on list.
// If an list entry has a tag specified it only will match if both are the same, else the tag is ignored.
func MatchImageDynamic(from string, to ...string) bool {
fullFrom := expandImage(from)
trimFrom := trimImage(from)
for _, match := range to {
if imageHasTag(match) {
if fullFrom == expandImage(match) {
return true
}
} else {
if trimFrom == trimImage(match) {
return true
}
}
}
return false
}

func imageHasTag(name string) bool {
return strings.Contains(name, ":")
}

// MatchHostname returns true if the image hostname
// matches the specified hostname.
func MatchHostname(image, hostname string) bool {
Expand Down
188 changes: 188 additions & 0 deletions pipeline/frontend/yaml/utils/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ func Test_expandImage(t *testing.T) {
from: "gcr.io/golang:1.0.0",
want: "gcr.io/golang:1.0.0",
},
{
from: "codeberg.org/6543/hello:latest@2c98dce11f78c2b4e40f513ca82f75035eb8cfa4957a6d8eb3f917ecaf77803",
want: "codeberg.org/6543/hello:latest@2c98dce11f78c2b4e40f513ca82f75035eb8cfa4957a6d8eb3f917ecaf77803",
},
// error cases, return input unmodified
{
from: "foo/bar?baz:boo",
Expand All @@ -124,6 +128,57 @@ func Test_expandImage(t *testing.T) {
}
}

func Test_imageHasTag(t *testing.T) {
testdata := []struct {
from string
want bool
}{
{
from: "golang",
want: false,
},
{
from: "golang:latest",
want: true,
},
{
from: "golang:1.0.0",
want: true,
},
{
from: "library/golang",
want: false,
},
{
from: "library/golang:latest",
want: true,
},
{
from: "library/golang:1.0.0",
want: true,
},
{
from: "index.docker.io/library/golang:1.0.0",
want: true,
},
{
from: "gcr.io/golang",
want: false,
},
{
from: "gcr.io/golang:1.0.0",
want: true,
},
{
from: "codeberg.org/6543/hello:latest@2c98dce11f78c2b4e40f513ca82f75035eb8cfa4957a6d8eb3f917ecaf77803",
want: true,
},
}
for _, test := range testdata {
assert.Equal(t, test.want, imageHasTag(test.from))
}
}

func Test_matchImage(t *testing.T) {
testdata := []struct {
from, to string
Expand Down Expand Up @@ -205,6 +260,139 @@ func Test_matchImage(t *testing.T) {
}
}

func Test_matchImageExact(t *testing.T) {
testdata := []struct {
from, to string
want bool
}{
{
from: "golang",
to: "golang",
want: true,
},
{
from: "golang:latest",
to: "golang",
want: true,
},
{
from: "library/golang:latest",
to: "golang",
want: true,
},
{
from: "index.docker.io/library/golang:1.0.0",
to: "golang",
want: false,
},
{
from: "golang",
to: "golang:latest",
want: true,
},
{
from: "library/golang:latest",
to: "library/golang",
want: true,
},
{
from: "gcr.io/golang",
to: "gcr.io/golang",
want: true,
},
{
from: "gcr.io/golang:1.0.0",
to: "gcr.io/golang",
want: false,
},
{
from: "gcr.io/golang:latest",
to: "gcr.io/golang",
want: true,
},
{
from: "gcr.io/golang",
to: "gcr.io/golang:latest",
want: true,
},
{
from: "golang",
to: "library/golang",
want: true,
},
{
from: "golang",
to: "gcr.io/project/golang",
want: false,
},
{
from: "golang",
to: "gcr.io/library/golang",
want: false,
},
{
from: "golang",
to: "gcr.io/golang",
want: false,
},
}
for _, test := range testdata {
if !assert.Equal(t, test.want, MatchImageExact(test.from, test.to)) {
t.Logf("test data: '%s' -> '%s'", test.from, test.to)
}
}
}

func Test_matchImageDynamic(t *testing.T) {
testdata := []struct {
name, from string
to []string
want bool
}{
{
name: "simple compare",
from: "golang",
to: []string{"golang"},
want: true,
},
{
name: "compare non-taged image whit list who tag requirement",
from: "golang",
to: []string{"golang:v3.0"},
want: false,
},
{
name: "compare taged image whit list who tag no requirement",
from: "golang:v3.0",
to: []string{"golang"},
want: true,
},
{
name: "compare taged image whit list who has image with no tag requirement",
from: "golang:1.0",
to: []string{"golang", "golang:2.0"},
want: true,
},
{
name: "compare taged image whit list who only has images with tag requirement",
from: "golang:1.0",
to: []string{"golang:latest", "golang:2.0"},
want: false,
},
{
name: "compare taged image whit list who only has images with tag requirement",
from: "golang:1.0",
to: []string{"golang:latest", "golang:1.0"},
want: true,
},
}
for _, test := range testdata {
if !assert.Equal(t, test.want, MatchImageDynamic(test.from, test.to...)) {
t.Logf("test data: '%s' -> '%s'", test.from, test.to)
}
}
}

func Test_matchHostname(t *testing.T) {
testdata := []struct {
image, hostname string
Expand Down
Loading