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

Conversation

6543
Copy link
Member

@6543 6543 commented Aug 26, 2024

we currently allow all tags for our clone image. this change it to a an default allow list we maintain.

@6543 6543 added server enhancement improve existing features security breaking will break existing installations if no manual action happens labels Aug 26, 2024
@6543 6543 added this to the 3.0.0 milestone Aug 26, 2024
shared/constant/constant.go Outdated Show resolved Hide resolved
@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Aug 26, 2024

Copy link

codecov bot commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.96%. Comparing base (3c8204a) to head (1422752).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4056   +/-   ##
=======================================
  Coverage   26.96%   26.96%           
=======================================
  Files         394      394           
  Lines       27416    27416           
=======================================
  Hits         7393     7393           
  Misses      19323    19323           
  Partials      700      700           

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

@6543
Copy link
Member Author

6543 commented Aug 31, 2024

broke out the least controversial part: #4069

@qwerty287
Copy link
Contributor

I think what's definitely a good idea is to support tag checks, but do not enforce them. Just like it's for the secrets. Together with removing all privileged plugins by default, wthe admin has the control about this then.

@6543
Copy link
Member Author

6543 commented Aug 31, 2024

it should be enforced in critical stuff ... else users just shoot themself in the foot

@6543
Copy link
Member Author

6543 commented Sep 1, 2024

@lafriks if we want the one or the other as DEFAULT ... we can discust that here.

for the general concept to be able to filter based on tags, i split out a new pull #4074

witch also make it possible to admins to alter trustedClonePlugins as they wish

@6543
Copy link
Member Author

6543 commented Sep 1, 2024

@lafriks I split out ... again the parts that are non breaking and realy usefull: #4074 + #4075

@6543 6543 changed the title Enforce trust check of clone image via allow-list of image name and tag By default enforce check of clone image via allow-list by image name and tag Sep 1, 2024
shared/constant/constant.go Outdated Show resolved Hide resolved
@lafriks
Copy link
Contributor

lafriks commented Sep 1, 2024

Personally I think that allow-list is not the way to go, they takes a lot of resources and are hard to maintain for every instance.

The best solution for this imho would be to have centrally managed vulnerable plugin version list that server would download at specific intervals and would trigger warnings in pipelines that use vulnerable plugin versions and allow woodpecker instance admins to block specific versions. This could be done not only for clone/trusted plugins but other plugins as well.

@6543
Copy link
Member Author

6543 commented Sep 1, 2024

that's an option we could go for ...

@lafriks
Copy link
Contributor

lafriks commented Sep 1, 2024

Also we could have option that would automatically block vulnerable plugin versions if admin chooses to go this way

@6543
Copy link
Member Author

6543 commented Sep 1, 2024

as to my knowledge our clone plugin has no issues ... we can take our time to create a proper solution ...

... the best case would be to be able to have something like a CVE list just for containers ... but I dont know of none

@6543
Copy link
Member Author

6543 commented Sep 1, 2024

or also if you could specify container images if you file CVEs ... I wonder if I could propose some additional flag to the CVE list to specify ''artifacts'' or ''deply methodes'' so tools can check against the usge of them ...

in this way we could then just file normal CVEs for plugins and use the official list to automatically create our own list, else it's also quite some maintinance burden on our side anyway ...

@lafriks
Copy link
Contributor

lafriks commented Sep 1, 2024

I don't think the CVE list is the way to go and I don't think maintaining a list for us would be that hard as currently there haven't even been situations where that is the case and we could add them on issue/PR bases

@qwerty287
Copy link
Contributor

I also think a blocklist is more helpful here. Maintaining a list like this is not that nice

@6543
Copy link
Member Author

6543 commented Sep 3, 2024

ok we can close this in fafour of #4080

@6543 6543 closed this Sep 3, 2024
@6543 6543 deleted the plugin-secret-exact-filter-if-tag-set branch September 3, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking will break existing installations if no manual action happens enhancement improve existing features security server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants