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

Secrets in plain text in Kubernetes resource outputs.__inputs #734

Closed
lukehoban opened this issue Aug 22, 2019 · 2 comments · Fixed by #741
Closed

Secrets in plain text in Kubernetes resource outputs.__inputs #734

lukehoban opened this issue Aug 22, 2019 · 2 comments · Fixed by #741
Assignees
Labels
impact/security last-applied-configuration Issues related to the last-applied-configuration annotation p1 A bug severe enough to be the next item assigned to an engineer
Milestone

Comments

@lukehoban
Copy link
Member

If I deploy a resource with secrets, they are encypted in the inputs and in the main part of outputs, but are in plain text in outputs.__inputs for Kubernetes resources.

The RandomPassword is correct:

            {
                "urn": "urn:pulumi:dev::pulumi-onprem-kubernetes::random:index/randomPassword:RandomPassword::local_keys",
                "custom": true,
                "id": "none",
                "type": "random:index/randomPassword:RandomPassword",
                "inputs": {
                    "__defaults": [
                        "lower",
                        "minLower",
                        "minNumeric",
                        "minSpecial",
                        "minUpper",
                        "number",
                        "special",
                        "upper"
                    ],
                    "length": 32,
                    "lower": true,
                    "minLower": 0,
                    "minNumeric": 0,
                    "minSpecial": 0,
                    "minUpper": 0,
                    "number": true,
                    "special": true,
                    "upper": true
                },
                "outputs": {
                    "id": "none",
                    "length": 32,
                    "lower": true,
                    "minLower": 0,
                    "minNumeric": 0,
                    "minSpecial": 0,
                    "minUpper": 0,
                    "number": true,
                    "result": {
                        "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
                        "ciphertext": "AAABAOKIe8B1gv2hRu5iF9A95EsZZKNG02V2mJXgoJek1uI/5B4HAedE4ykdkpy2QP4/RFBmOqk6IztdVGtGkNMouKnbN7I="
                    },
                    "special": true,
                    "upper": true
                },
                "parent": "urn:pulumi:dev::pulumi-onprem-kubernetes::pulumi:pulumi:Stack::pulumi-onprem-kubernetes-dev",
                "provider": "urn:pulumi:dev::pulumi-onprem-kubernetes::pulumi:providers:random::default_1_0_0::3a886140-bc7d-4800-ae11-fae5225f9724",
                "propertyDependencies": {
                    "length": null
                },
                "additionalSecretOutputs": [
                    "result"
                ],
                "customTimeouts": {
                    "Create": 0,
                    "Update": 0,
                    "Delete": 0
                }
            },

The Kubernetes ConfigMap is correct on inputs, but leaks plaintext on outputs:

{
                "urn": "urn:pulumi:dev::pulumi-onprem-kubernetes::kubernetes:core/v1:ConfigMap::localkeys",
                "custom": true,
                "id": "default/localkeys-l489ixie",
                "type": "kubernetes:core/v1:ConfigMap",
                "inputs": {
                    "apiVersion": "v1",
                    "binaryData": {
                        "localkeys": {
                            "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
                            "ciphertext": "AAABAB1t+Pn+/8cKwNWwwhKl7i6Jk0rlyqKq4zir0IkyEHXiutXHmmLDBXyMLflre77pjV46HMTuX9UjbXwA1rBPl9XYApgWWJUx+1fE"
                        }
                    },
                    "kind": "ConfigMap",
                    "metadata": {
                        "annotations": {
                            "pulumi.com/autonamed": "true"
                        },
                        "labels": {
                            "app.kubernetes.io/managed-by": "pulumi"
                        },
                        "name": "localkeys-l489ixie"
                    }
                },
                "outputs": {
                    "__inputs": {
                        "apiVersion": "v1",
                        "binaryData": {
                            "localkeys": "Z0c8TjcxTT97XWZ2MjNsbkMjYjUySU8jVDlxe2xASDQ="
                        },
                        "kind": "ConfigMap",
                        "metadata": {
                            "annotations": {
                                "pulumi.com/autonamed": "true"
                            },
                            "labels": {
                                "app.kubernetes.io/managed-by": "pulumi"
                            },
                            "name": "localkeys-l489ixie"
                        }
                    },
                    "apiVersion": "v1",
                    "binaryData": {
                        "localkeys": {
                            "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
                            "ciphertext": "AAABAAiYWGCKChU5Xv7Ac+L8PnKUj1jmXm60LeQNEsfc0fcZirouVcsQclozREj9QVs5e0s3Sf0jV10z+Z3WRYO9t1t0+Rk6rUGRNS7F"
                        }
                    },
                    "kind": "ConfigMap",
                    "metadata": {
                        "annotations": {
                            "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"v1\",\"binaryData\":{\"localkeys\":\"Z0c8TjcxTT97XWZ2MjNsbkMjYjUySU8jVDlxe2xASDQ=\"},\"kind\":\"ConfigMap\",\"metadata\":{\"annotations\":{\"pulumi.com/autonamed\":\"true\"},\"labels\":{\"app.kubernetes.io/managed-by\":\"pulumi\"},\"name\":\"localkeys-l489ixie\"}}\n",
                            "pulumi.com/autonamed": "true"
                        },
                        "creationTimestamp": "2019-08-22T21:11:05Z",
                        "labels": {
                            "app.kubernetes.io/managed-by": "pulumi"
                        },
                        "name": "localkeys-l489ixie",
                        "namespace": "default",
                        "resourceVersion": "660664",
                        "selfLink": "/api/v1/namespaces/default/configmaps/localkeys-l489ixie",
                        "uid": "5ac70e0b-c521-11e9-a83d-025000000001"
                    }
                },
                "parent": "urn:pulumi:dev::pulumi-onprem-kubernetes::pulumi:pulumi:Stack::pulumi-onprem-kubernetes-dev",
                "dependencies": [
                    "urn:pulumi:dev::pulumi-onprem-kubernetes::random:index/randomPassword:RandomPassword::local_keys"
                ],
                "provider": "urn:pulumi:dev::pulumi-onprem-kubernetes::pulumi:providers:kubernetes::default_1_0_0_beta_1::28220046-6f28-4153-b97a-bda442b83beb",
                "propertyDependencies": {
                    "apiVersion": null,
                    "binaryData": [
                        "urn:pulumi:dev::pulumi-onprem-kubernetes::random:index/randomPassword:RandomPassword::local_keys"
                    ],
                    "kind": null
                },
                "customTimeouts": {
                    "Create": 0,
                    "Update": 0,
                    "Delete": 0
                }
            }
@lblackstone
Copy link
Member

We're also leaking the secret through .outputs.metadata.annotations["kubectl.kubernetes.io/last-applied-configuration"

@lblackstone lblackstone assigned ellismg and unassigned lblackstone Aug 23, 2019
@lblackstone
Copy link
Member

Matt is taking the lead on this for now since he's more familiar with the changes required to fix.

ellismg added a commit that referenced this issue Aug 26, 2019
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
coresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object iself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevent members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

Fixes #734
ellismg added a commit that referenced this issue Aug 26, 2019
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
coresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object iself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevent members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

Fixes #734
ellismg added a commit that referenced this issue Aug 26, 2019
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
coresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object iself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevent members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

Fixes #734
ellismg added a commit that referenced this issue Aug 27, 2019
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
coresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object iself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevent members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

Fixes #734
ellismg added a commit that referenced this issue Aug 27, 2019
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
coresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object iself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevent members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

In addition, this adds code to mark `data` as secret on
`k8s.core.v1.Secret` if `stringData` is a secret (the API Server
base64 encodes the `stringData` bag into `data` and so we should
logically flow the secretness).

Fixes #734
ellismg added a commit that referenced this issue Aug 27, 2019
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
coresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object iself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevent members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

In addition, this adds code to mark `data` as secret on
`k8s.core.v1.Secret` if `stringData` is a secret (the API Server
base64 encodes the `stringData` bag into `data` and so we should
logically flow the secretness).

Fixes #734
ellismg added a commit that referenced this issue Aug 28, 2019
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
coresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object iself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevent members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

In addition, this adds code to mark `data` as secret on
`k8s.core.v1.Secret` if `stringData` is a secret (the API Server
base64 encodes the `stringData` bag into `data` and so we should
logically flow the secretness).

Fixes #734
@lukehoban lukehoban assigned lblackstone and unassigned ellismg Aug 28, 2019
lblackstone pushed a commit that referenced this issue Aug 28, 2019
In order to support tracking secretness that flows from inputs to
outputs when using providers that do not understand secrets directly,
the engine takes any input that is secret and if there is a
corresponding output with the same name, marks it as a secret. This
works in common cases, but does not work for Kubernetes for two key
reasons:

1. The provider retains a copy of the inputs for a resource on an
object called `__inputs` inside the state object. It uses this during
Diff for reasons that are un-interesting to this PR.

2. The provider JSON stringifies the inputs and stores them as an
annotation on the object itself, as `kubectl` would.

These two decisions mean that if a secret value is used as an input to
a k8s resource, we will persist the plaintext value in the state
file, since the engine has no idea to look at `__inputs` or
`lastAppliedConfig`.

This change updates the provider to be able to handle secrets. The
engine will now pass any secret inputs as strongly typed secrets. The
provider will use this information to ensure that the relevant members
in the `__inputs` bag are marked as secrets as well as ensuring that
if there are any inputs that are secret, all of
`lastAppliedConfig` (which is a stringified JSON object) is marked as
a secret as well.

An integration test confirms this behavior by stringifying the state
and ensuring that our secret values do not end up in it (which will
catch cases where we may copy this data to other places as well).

In addition, this adds code to mark `data` as secret on
`k8s.core.v1.Secret` if `stringData` is a secret (the API Server
base64 encodes the `stringData` bag into `data` and so we should
logically flow the secretness).

Fixes #734
@infin8x infin8x added the p1 A bug severe enough to be the next item assigned to an engineer label Jul 10, 2021
@lblackstone lblackstone added the last-applied-configuration Issues related to the last-applied-configuration annotation label Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/security last-applied-configuration Issues related to the last-applied-configuration annotation p1 A bug severe enough to be the next item assigned to an engineer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants