triggers: Enable TriggerBindings to validate requests (e.g. GitHub)

Expected Behavior

Events received by the EventListener addressable service should only be handled if the requester and request are authenticated.

We can use GitHub as an initial use case.

From @khrm :

Github computes the hash of payload using the above token and then pass the computed hash to the header. And on triggers side, if we want to be sure, we compute the hash of payload using that token(needs to be stored) and match it with the one received in the header.

Actual Behavior

Currently, nothing does this.

We have discussed doing this via header matching:

As per the WG discussion, add a http field with an underlying headersMatch field (as showcased in the proposal) to the TriggerBindingSpec. The deployment could intelligently map the received events to the correct TriggerBinding(s), but simply iterating over the bindings is also good as a first pass.

However as mentioned above, more is required than just header matching.

Additional Info

@akihikokuroda included some example code at https://github.com/tektoncd/triggers/issues/78#issuecomment-523676106 to implement this.

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 1
Comments: 20 (11 by maintainers)

Commits related to this issue

Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources,... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources,... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. Before the creation of resources,... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to khrm/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to tektoncd/triggers by khrm 5 years ago
Enable TriggerBindings to validate requests This PR resolves the issue #45. It assumes that a task has been defined which can validate requests. That task will receive header and payload as params. B... — committed to openshift/tektoncd-triggers by khrm 5 years ago

Most upvoted comments

As far as the manifestation, I don’t have particular feelings one way or the other. It seems like the PipelineRun reconciler is where the only place where Conditions are resolved into TaskRuns. Looking at this briefly, it seems like we would have to recreate some of this, which is less than ideal, but maybe not too bad? Otherwise, we could have a condition section that allows users to spec out PodTemplates/Tasks/TaskRuns or something of the sort to be explicit like @akihikokuroda mentioned.

vtereso on Aug 23, 2019

@khrm I thought it might not fit. I might not understand the conditional correctly. I’ll look it, again.

akihikokuroda on Aug 22, 2019

@khrm I’ve tried to group together issues related to this: https://github.com/tektoncd/triggers/issues/78#issuecomment-523956330

It sounds like for this issue our next step will be to create a design for how we express authenticating the caller.

bobcatfish on Aug 22, 2019

If I understand @dlorenc use case, it’s about securing webhook? I don’t think we can validate token which is set up for webhook using just regex.

@vtereso what do you think about expanding this issue to be about authenticating against GitHub including hashing the payload ?

bobcatfish on Aug 22, 2019

I assign myself to work on this. Yet to start. Just following the discussions atm.

khrm on Aug 6, 2019