fix: Ensure that user cannot override reserved labels

[simster7]

Fixes #7215, which could be a potential security vulnerability with users overriding labels set by Argo.

Signed-off-by: Simon Behar simbeh7@gmail.com

Don't bother creating a PR until you've done this:

• Run make pre-commit -B to fix codegen, lint, and commit message problems.

Create your PR as a draft.

• Your PR needs to pass the required checks before it can be approved. If the check is not required (e.g. E2E tests) it
  does not need to pass.
• Once required tests have passed, you can make it "Ready for review".
• Say how how you tested your changes. If you changed the UI, attach screenshots.

Tips:

• If changes were requested, and you've made them, then dismiss the review to get it looked at again.
• Add you organization to USERS.md if you like.
• You can ask for help!

[simster7]

Not super happy with this solution as it only checks CLI submitted labels. However, checking at the WF validation stage is problematic because legitimate reserved labels might be set by Argo sometime before the validation stage, requiring some refactor to correctly validate at that stage.

[agilgur5]

Yes a CLI-only check would indeed be insufficient in many ways. A validation stage check or an informer change check would be most appropriate, but could also be worked around by a user with sufficient privileges.

Therefore, I don't think this approach is doable or that the issue itself is even solveable in k8s. SAs and other identity mechanisms within k8s should be used for this, as those are properly supported authN in k8s.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[terrytangyuan]

@simster7 Still working on this?

[jburke-godaddy]

@simster7 Any chance this is going to re-open?

[tooptoop4]

bump

[agilgur5]

This was linked in #13079, but I think this approach is problematic and the issue itself is unsolvable. Within k8s, any user with appropriate privileges can edit labels.
Argo can do a bunch of workarounds (as mentioned below) to try to prevent this, but those are workarounds at best and not solutions, and should not be relied on for authN purposes. In fact, I would say that any attempt at making this non-overriddeable would give a user a false sense of security and we should not give that intentionally. Trying to make this secure would create various vulnerabilities as well, due to breaking k8s's RBAC model.

As such, I'm going to mark this and the issue as won't fix.

k8s's own RBAC should be used for authN and authZ, which is what Argo's SSO RBAC feature uses. For more fine-grained RBAC, see #6490

[agilgur5]

Yes a CLI-only check would indeed be insufficient in many ways. A validation stage check or an informer change check would be most appropriate, but could also be worked around by a user with sufficient privileges.

Therefore, I don't think this approach is doable or that the issue itself is even solveable in k8s. SAs and other identity mechanisms within k8s should be used for this, as those are properly supported authN in k8s.