Allow sensitive values to be used in preconfigured webhooks without showing up in the execution context

[willgorman]

Issue Summary:

Preconfigured webhooks may contain sensitive values (like credentials to authenticate with the webhook endpoint) with an expectation of privacy since those values are not displayed in Deck when adding the stage to a pipeline. However, as mentioned here those values still are available in the execution context.

Feature Area:

Webhook stages

Description:

It would be helpful to be able to keep the values from the preconfigured webhook stage that aren't needed in Deck from being returned. This could be the default behavior, but in cases where the values aren't sensitive it can be valuable to still have them available for troubleshooting. Another option would be to allow the preconfigured stage to declare what values are sensitive:

      customHeaders:
        X-Api-Key:
          - <newrelic-api-key>
        Content-Type:
          - application/json
      # Proposed change
      sensitiveHeaders:
        - X-Api-Key
      sensitivePayload: true

(I'm assuming that only headers or the payload would be expected to contain sensitive data)

Additional details:

Relates to spinnaker/orca#1329

[lwander]

I think this deserves a broader discussion around handling secrets between spinnaker & a trigger source, as well integrating this with Spinnaker's authz to avoid having the wrong users read/edit a shared secret.

I know @ezimanyi has some thoughts about this, especially with the new, more generic CI integrations he's working on.

[ezimanyi]

My thinking had been more related to improving the security of inbound webhooks. Right now, the only authentication mechanism we support is adding a pre-shared expected key in the payload; I was considering augmenting that, possibly by adding a specific authentication-related field and only storing a hash of the expected value in Spinnaker.

On the outbound side, we of course can't get away with just storing a hash as we'll need the actual secret to send along, so the goal should be limiting the exposure of that secret as much as possible. Ideally there would be some kind of secret management that could handle access to these secrets---but absent that it seems that identifying in the webhook which ones are sensitive and avoiding including those in the execution context seems reasonable.

[spinnakerbot]

This issue hasn't been updated in 90 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue is tagged as 'to-be-closed' and hasn't been updated in 45 days, so we are closing it. You can always reopen this issue if needed.

[ghostsquad]

can this issue be reopened?

[ezimanyi]

I'll re-open and tag this as sig/security to see if they have any thoughts, though definitely can't comment on whether they will prioritize this.

[bcornils]

Hi @ezimanyi Thanks for filing the issue. Unfortunately, no one is available to work on this at the moment, but we welcome contributions from our community. We strongly encourage you to discuss your plans here or on the SIG's slack channel before you start working on the issue.

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[ghostsquad]

@spinnakerbot remove-label stale

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[ghostsquad]

@spinnakerbot remove-label stale

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[iiro]

@spinnakerbot remove-label stale

…

On Wed, Sep 30, 2020 at 11:00 PM spinnakerbot ***@***.***> wrote: This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment: @spinnakerbot <https://github.com/spinnakerbot> remove-label stale — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2787 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQ4T55VI2QCH3OVNOT5WFLSIOMAJANCNFSM4FABP56A> .

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[qrnik]

@spinnakerbot <https://github.com/spinnakerbot> remove-label stale

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[dcchambers]

@spinnakerbot remove-label to-be-closed

[dcchambers]

@spinnakerbot remove-label stale

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[jervi]

@spinnakerbot remove-label stale

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[dcchambers]

@spinnakerbot remove-label to-be-closed

[dcchambers]

@spinnakerbot remove-label stale

[spinnakerbot]

This issue hasn't been updated in 45 days, so we are tagging it as 'stale'. If you want to remove this label, comment:

@spinnakerbot remove-label stale

[spinnakerbot]

This issue is tagged as 'stale' and hasn't been updated in 45 days, so we are tagging it as 'to-be-closed'. It will be closed in 45 days unless updates are made. If you want to remove this label, comment:

@spinnakerbot remove-label to-be-closed

[spinnakerbot]

This issue is tagged as 'to-be-closed' and hasn't been updated in 45 days, so we are closing it. You can always reopen this issue if needed.

[Lisenish]

@ezimanyi @bcornils Can we reopen it again? I think this is still actual (and maybe also it makes sense to froze it so bot doesn't auto-close it?)

[vjda]

I've created this PR spinnaker/orca#4615 to address part of this issue. It would be great if any maintainer could take a look at it.