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

Allow signing of generic webhook provider requests #99

Closed
bigkevmcd opened this issue Dec 7, 2020 · 5 comments · Fixed by #127 or #426
Closed

Allow signing of generic webhook provider requests #99

bigkevmcd opened this issue Dec 7, 2020 · 5 comments · Fixed by #127 or #426
Assignees
Labels
enhancement New feature or request

Comments

@bigkevmcd
Copy link
Contributor

The HTTP endpoint should be able to ensure the provenance of incoming requests from the generic provider.

GitHub and GitLab use a shared-secret mechanism, where the outgoing requests have a header with an HMAC of the request body, the receiver can then sign the body it receives, and confirm the origin of the request.

This should be optional.

One proposal, is to add an additional signature field to the spec:

spec:
 signature:
   secretRef:
       name: hmac

Another option would be to use the presence of an hmac key in the Secret referenced by spec.secretRef.

Some other considerations, sha1 or sha256? What should the header name be?

@stefanprodan
Copy link
Member

I would implement it as GitHub https://developer.github.com/webhooks/securing/

Header: X-Signature
Algorithm : HMAC/SHA1

@bigkevmcd
Copy link
Contributor Author

I'm not sure that PR fixes the issue, that is only about receiving webhooks, not signing outgoing hooks.

@bigkevmcd
Copy link
Contributor Author

I've got a branch that I would need to tidy up the last bit of that would solve the signing part...

@stefanprodan
Copy link
Member

@bigkevmcd you're right, that PR solves ingress not egress. We can reopen.

@stefanprodan stefanprodan reopened this Jan 21, 2021
@stefanprodan
Copy link
Member

stefanprodan commented Sep 29, 2022

I propose we introduce a new alert provider type named generic-hmac (like we did for receivers), that requires a secret with a token key.

Example definition with HMAC and self-signed TLS receiver:

apiVersion: notification.toolkit.fluxcd.io/v1beta1
kind: Provider
metadata:
  name: my-alert-provider
  namespace: default
spec:
  type: generic-hmac
  address: https://my-service.internal/flux-receiver
  secretRef:
    name: my-alert-provider-token
  certSecretRef:
    name: my-alert-provider-ca
---
apiVersion: v1
kind: Secret
metadata:
  name: my-alert-provider-token
  namespace: default
stringData:
  token: <my-hmac-token>
---
apiVersion: v1
kind: Secret
metadata:
  name: my-alert-provider-ca
  namespace: default
stringData:
  caFile: <my-custom-ca>

The HTTP POST will contain a header named X-Signature:

X-Signature: sha256=<hex-digest>

The receiver will lookup the prefix to determine the SHA algorithm, then it will calculate the digest of the body and compare it with the digest in the header.

@makkes makkes self-assigned this Sep 30, 2022
@makkes makkes added the enhancement New feature or request label Sep 30, 2022
makkes added a commit that referenced this issue Sep 30, 2022
This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

closes #99
makkes added a commit that referenced this issue Sep 30, 2022
This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

closes #99

Signed-off-by: Max Jonas Werner <[email protected]>
makkes added a commit that referenced this issue Sep 30, 2022
This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

closes #99

Signed-off-by: Max Jonas Werner <[email protected]>
makkes added a commit that referenced this issue Sep 30, 2022
This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

refs #99

Signed-off-by: Max Jonas Werner <[email protected]>
makkes added a commit that referenced this issue Sep 30, 2022
This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

closes #99

Signed-off-by: Max Jonas Werner <[email protected]>
makkes added a commit that referenced this issue Sep 30, 2022
This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

closes #99

Signed-off-by: Max Jonas Werner <[email protected]>
makkes added a commit that referenced this issue Oct 4, 2022
This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

Any existing `X-Signature` header value set through
a `Provider.spec.secretRef` Secret's `header` field will be
overwritten.

closes #99

Signed-off-by: Max Jonas Werner <[email protected]>
@makkes makkes moved this to In Progress in Maintainers' Focus Oct 5, 2022
@makkes makkes moved this from In Progress to Since Last Dev Meeting in Maintainers' Focus Oct 5, 2022
@pjbgf pjbgf moved this from Since Last Dev Meeting to Done in Maintainers' Focus Oct 6, 2022
gunishmatta added a commit to gunishmatta/notification-controller that referenced this issue Nov 14, 2022
Signed-off-by: gunishmatta <[email protected]>

PR changes

Signed-off-by: gunishmatta <[email protected]>

Update Kubernetes packages to v1.25.0

Signed-off-by: Stefan Prodan <[email protected]>

Fix context cancel defer

Signed-off-by: Philip Laine <[email protected]>

Release v0.25.2

Signed-off-by: Stefan Prodan <[email protected]>

update to new doc links structure

Signed-off-by: Daniel Holbach <[email protected]>

Add .spec.timeout to Provider

Signed-off-by: Somtochi Onyekwere <[email protected]>

Update pkg and controller-runtime

Signed-off-by: Somtochi Onyekwere <[email protected]>

fuzz: Ensure latest base images are used
Latest base image should contain Go 1.18, removing
the need of updating that ourselves, apart from
benefiting from latest changes upstream.

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Reuse go cache from host

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Use Go 1.18 on CI and fix cache path

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Refactor Fuzzers based on Go native fuzzing
The existing fuzzers were converted into the Go native format.
Based on how the code was structured on this project, the fuzzers
can be quite effective, allowing for entire E2E fuzzing in some
cases, but with very low execution cost.

Signed-off-by: Paulo Gomes <[email protected]>

Add finalizers to all the CRDs

Signed-off-by: Somtochi Onyekwere <[email protected]>

Release v0.26.0

Signed-off-by: Stefan Prodan <[email protected]>

api: add custom validation for v1.Duration types

Signed-off-by: Stefan Prodan <[email protected]>

Fix table with git commit status providers

Signed-off-by: Andrey Ivashchenko <[email protected]>

Update dependencies
- k8s.io/* v0.25.2
- controller-runtime v0.13.0
- fluxcd/pkg/runtime v0.19.0

Signed-off-by: Stefan Prodan <[email protected]>

Update Go to 1.19

Signed-off-by: Stefan Prodan <[email protected]>

Dockerfile: Build with Go 1.19

Signed-off-by: Stefan Prodan <[email protected]>

Release v0.27.0

Signed-off-by: Stefan Prodan <[email protected]>

Add "generic-hmac" Provider

This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

Any existing `X-Signature` header value set through
a `Provider.spec.secretRef` Secret's `header` field will be
overwritten.

closes fluxcd#99

Signed-off-by: Max Jonas Werner <[email protected]>

Update dependencies
Includes a fix for CVE-2022-32149 of `golang.org/x/text`

Signed-off-by: Stefan Prodan <[email protected]>

Release v0.28.0

Signed-off-by: Stefan Prodan <[email protected]>

http scheme updates

Signed-off-by: Gunish Matta <[email protected]>

improved tests

Signed-off-by: Gunish Matta <[email protected]>

http scheme updates

Signed-off-by: Gunish Matta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Max Jonas Werner <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Paulo Gomes <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>
gunishmatta added a commit to gunishmatta/notification-controller that referenced this issue Nov 14, 2022
author gunishmatta <[email protected]> 1660541336 +0000
committer gunishmatta <[email protected]> 1668423536 +0000

added flag to disable http

Signed-off-by: gunishmatta <[email protected]>

added flag

Signed-off-by: gunishmatta <[email protected]>

revert mistakenly pushed binaries

Signed-off-by: gunishmatta <[email protected]>

PR changes

Signed-off-by: gunishmatta <[email protected]>

Update Kubernetes packages to v1.25.0

Signed-off-by: Stefan Prodan <[email protected]>

Fix context cancel defer

Signed-off-by: Philip Laine <[email protected]>

Release v0.25.2

Signed-off-by: Stefan Prodan <[email protected]>

update to new doc links structure

Signed-off-by: Daniel Holbach <[email protected]>

Add .spec.timeout to Provider

Signed-off-by: Somtochi Onyekwere <[email protected]>

Update pkg and controller-runtime

Signed-off-by: Somtochi Onyekwere <[email protected]>

fuzz: Ensure latest base images are used
Latest base image should contain Go 1.18, removing
the need of updating that ourselves, apart from
benefiting from latest changes upstream.

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Reuse go cache from host

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Use Go 1.18 on CI and fix cache path

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Refactor Fuzzers based on Go native fuzzing
The existing fuzzers were converted into the Go native format.
Based on how the code was structured on this project, the fuzzers
can be quite effective, allowing for entire E2E fuzzing in some
cases, but with very low execution cost.

Signed-off-by: Paulo Gomes <[email protected]>

Add finalizers to all the CRDs

Signed-off-by: Somtochi Onyekwere <[email protected]>

Release v0.26.0

Signed-off-by: Stefan Prodan <[email protected]>

api: add custom validation for v1.Duration types

Signed-off-by: Stefan Prodan <[email protected]>

Fix table with git commit status providers

Signed-off-by: Andrey Ivashchenko <[email protected]>

Update dependencies
- k8s.io/* v0.25.2
- controller-runtime v0.13.0
- fluxcd/pkg/runtime v0.19.0

Signed-off-by: Stefan Prodan <[email protected]>

Update Go to 1.19

Signed-off-by: Stefan Prodan <[email protected]>

Dockerfile: Build with Go 1.19

Signed-off-by: Stefan Prodan <[email protected]>

Release v0.27.0

Signed-off-by: Stefan Prodan <[email protected]>

Add "generic-hmac" Provider

This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

Any existing `X-Signature` header value set through
a `Provider.spec.secretRef` Secret's `header` field will be
overwritten.

closes fluxcd#99

Signed-off-by: Max Jonas Werner <[email protected]>

Update dependencies
Includes a fix for CVE-2022-32149 of `golang.org/x/text`

Signed-off-by: Stefan Prodan <[email protected]>

Release v0.28.0

Signed-off-by: Stefan Prodan <[email protected]>

http scheme updates

Signed-off-by: Gunish Matta <[email protected]>

improved tests

Signed-off-by: Gunish Matta <[email protected]>

http scheme updates

Signed-off-by: Gunish Matta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Max Jonas Werner <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Paulo Gomes <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>

revert mistakenly pushed binaries

Signed-off-by: gunishmatta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Paulo Gomes <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>
gunishmatta added a commit to gunishmatta/notification-controller that referenced this issue Nov 14, 2022
author gunishmatta <[email protected]> 1660541336 +0000
committer gunishmatta <[email protected]> 1668423536 +0000

added flag to disable http

Signed-off-by: gunishmatta <[email protected]>

added flag

Signed-off-by: gunishmatta <[email protected]>

revert mistakenly pushed binaries

Signed-off-by: gunishmatta <[email protected]>

PR changes

Signed-off-by: gunishmatta <[email protected]>

Update Kubernetes packages to v1.25.0

Signed-off-by: Stefan Prodan <[email protected]>

Fix context cancel defer

Signed-off-by: Philip Laine <[email protected]>

Release v0.25.2

Signed-off-by: Stefan Prodan <[email protected]>

update to new doc links structure

Signed-off-by: Daniel Holbach <[email protected]>

Add .spec.timeout to Provider

Signed-off-by: Somtochi Onyekwere <[email protected]>

Update pkg and controller-runtime

Signed-off-by: Somtochi Onyekwere <[email protected]>

fuzz: Ensure latest base images are used
Latest base image should contain Go 1.18, removing
the need of updating that ourselves, apart from
benefiting from latest changes upstream.

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Reuse go cache from host

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Use Go 1.18 on CI and fix cache path

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Refactor Fuzzers based on Go native fuzzing
The existing fuzzers were converted into the Go native format.
Based on how the code was structured on this project, the fuzzers
can be quite effective, allowing for entire E2E fuzzing in some
cases, but with very low execution cost.

Signed-off-by: Paulo Gomes <[email protected]>

Add finalizers to all the CRDs

Signed-off-by: Somtochi Onyekwere <[email protected]>

Release v0.26.0

Signed-off-by: Stefan Prodan <[email protected]>

api: add custom validation for v1.Duration types

Signed-off-by: Stefan Prodan <[email protected]>

Fix table with git commit status providers

Signed-off-by: Andrey Ivashchenko <[email protected]>

Update dependencies
- k8s.io/* v0.25.2
- controller-runtime v0.13.0
- fluxcd/pkg/runtime v0.19.0

Signed-off-by: Stefan Prodan <[email protected]>

Update Go to 1.19

Signed-off-by: Stefan Prodan <[email protected]>

Dockerfile: Build with Go 1.19

Signed-off-by: Stefan Prodan <[email protected]>

Release v0.27.0

Signed-off-by: Stefan Prodan <[email protected]>

Add "generic-hmac" Provider

This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

Any existing `X-Signature` header value set through
a `Provider.spec.secretRef` Secret's `header` field will be
overwritten.

closes fluxcd#99

Signed-off-by: Max Jonas Werner <[email protected]>

Update dependencies
Includes a fix for CVE-2022-32149 of `golang.org/x/text`

Signed-off-by: Stefan Prodan <[email protected]>

Release v0.28.0

Signed-off-by: Stefan Prodan <[email protected]>

http scheme updates

Signed-off-by: Gunish Matta <[email protected]>

improved tests

Signed-off-by: Gunish Matta <[email protected]>

http scheme updates

Signed-off-by: Gunish Matta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Max Jonas Werner <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Paulo Gomes <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>

revert mistakenly pushed binaries

Signed-off-by: gunishmatta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Paulo Gomes <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>
Signed-off-by: gunishmatta <[email protected]>
gunishmatta added a commit to gunishmatta/notification-controller that referenced this issue Nov 14, 2022
parent 921ebc0
author gunishmatta <[email protected]> 1660541336 +0000
committer gunishmatta <[email protected]> 1668423536 +0000

added flag to disable http

Signed-off-by: gunishmatta <[email protected]>

added flag

Signed-off-by: gunishmatta <[email protected]>

revert mistakenly pushed binaries

Signed-off-by: gunishmatta <[email protected]>

PR changes

Signed-off-by: gunishmatta <[email protected]>

Update Kubernetes packages to v1.25.0

Signed-off-by: Stefan Prodan <[email protected]>

Fix context cancel defer

Signed-off-by: Philip Laine <[email protected]>

Release v0.25.2

Signed-off-by: Stefan Prodan <[email protected]>

update to new doc links structure

Signed-off-by: Daniel Holbach <[email protected]>

Add .spec.timeout to Provider

Signed-off-by: Somtochi Onyekwere <[email protected]>

Update pkg and controller-runtime

Signed-off-by: Somtochi Onyekwere <[email protected]>

fuzz: Ensure latest base images are used
Latest base image should contain Go 1.18, removing
the need of updating that ourselves, apart from
benefiting from latest changes upstream.

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Reuse go cache from host

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Use Go 1.18 on CI and fix cache path

Signed-off-by: Paulo Gomes <[email protected]>

fuzz: Refactor Fuzzers based on Go native fuzzing
The existing fuzzers were converted into the Go native format.
Based on how the code was structured on this project, the fuzzers
can be quite effective, allowing for entire E2E fuzzing in some
cases, but with very low execution cost.

Signed-off-by: Paulo Gomes <[email protected]>

Add finalizers to all the CRDs

Signed-off-by: Somtochi Onyekwere <[email protected]>

Release v0.26.0

Signed-off-by: Stefan Prodan <[email protected]>

api: add custom validation for v1.Duration types

Signed-off-by: Stefan Prodan <[email protected]>

Fix table with git commit status providers

Signed-off-by: Andrey Ivashchenko <[email protected]>

Update dependencies
- k8s.io/* v0.25.2
- controller-runtime v0.13.0
- fluxcd/pkg/runtime v0.19.0

Signed-off-by: Stefan Prodan <[email protected]>

Update Go to 1.19

Signed-off-by: Stefan Prodan <[email protected]>

Dockerfile: Build with Go 1.19

Signed-off-by: Stefan Prodan <[email protected]>

Release v0.27.0

Signed-off-by: Stefan Prodan <[email protected]>

Add "generic-hmac" Provider

This commit adds the "generic-hmac" Provider type for authenticating
webhook requests coming from notification-controller. I extended the
`Forwarder` notifier to accept an optional key used for generating the
HMAC. If the key is nil or empty no HMAC header is generated and the
forwarder behaves as before. If it is provided an `X-Signature` HTTP
header is added to the request carrying the HMAC.

I transformed the `TestForwarder_Post` test into a table-driven test
so that we can use the same setup and testing code for testing HMAC
and non-HMAC forwarder instances.

Any existing `X-Signature` header value set through
a `Provider.spec.secretRef` Secret's `header` field will be
overwritten.

closes fluxcd#99

Signed-off-by: Max Jonas Werner <[email protected]>

Update dependencies
Includes a fix for CVE-2022-32149 of `golang.org/x/text`

Signed-off-by: Stefan Prodan <[email protected]>

Release v0.28.0

Signed-off-by: Stefan Prodan <[email protected]>

http scheme updates

Signed-off-by: Gunish Matta <[email protected]>

improved tests

Signed-off-by: Gunish Matta <[email protected]>

http scheme updates

Signed-off-by: Gunish Matta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Max Jonas Werner <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Paulo Gomes <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>

revert mistakenly pushed binaries

Signed-off-by: gunishmatta <[email protected]>

Update controllers/event_handling_test.go

Co-authored-by: Paulo Gomes <[email protected]>
Signed-off-by: Gunish Matta <[email protected]>
Signed-off-by: gunishmatta <[email protected]>

minor formatter changes

Signed-off-by: gunishmatta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
3 participants