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 not encrypted in state file (even if explicit) #915

Closed
brandonkal opened this issue Dec 10, 2019 · 4 comments
Closed

Secrets not encrypted in state file (even if explicit) #915

brandonkal opened this issue Dec 10, 2019 · 4 comments
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec last-applied-configuration Issues related to the last-applied-configuration annotation resolution/fixed This issue was fixed

Comments

@brandonkal
Copy link

brandonkal commented Dec 10, 2019

Problem description

Secrets do not remain encrypted in the state file even if explicitly marked as a secret. I actually would have expected it to be encrypted in state even without using pulumi.secret().

Errors & Logs

NA

Affected product version(s)

@pulumi/kubernetes 1.4.0
pulumi v1.6.1

Reproducing the issue

  1. Run this pulumi program
import * as k8s from '@pulumi/kubernetes'

const testSecret = new k8s.core.v1.Secret('test-secret', {
  stringData: {
    password: pulumi.secret('supersecret'),
  },
})
  1. Look at the state file. The string "supersecret" is only encrypted as an input. It is visible in plain text in __inputs and annotations.
  2. Add { additionalSecretOutputs: ['stringData'] }. Note that the output remains stored unencrypted in Pulumi state.

Suggestions for a fix

  • Inputs and Outputs to k8s.core.v1.Secret should always be marked as pulumi secrets
  • Explicit secrets should remain secret on output
  • Ensure it is not leaked in annotations
@lblackstone lblackstone self-assigned this Dec 11, 2019
@lblackstone
Copy link
Member

Yes, the stringData field should be automatically encrypted, and I'm fairly certain that was the case last time I looked at this. I'll check into it.

@lblackstone
Copy link
Member

Actually, I wasn't able to reproduce this with the value marked with pulumi.secret().

...
"inputs": {
    "apiVersion": "v1",
    "kind": "Secret",
    "metadata": {
        "annotations": {
            "pulumi.com/autonamed": "true"
        },
        "labels": {
            "app.kubernetes.io/managed-by": "pulumi"
        },
        "name": "test-secret-7svha8ld"
    },
    "stringData": {
        "password": {
            "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
            "ciphertext": "AAABAJdhdKTSwgWqSiVJYzFYjPAYrt6n3dr2vxcGXY0CfWuWye96UVPTIfyi"
        }
    }
},
"outputs": {
    "__initialApiVersion": "v1",
    "__inputs": {
        "apiVersion": "v1",
        "kind": "Secret",
        "metadata": {
            "annotations": {
                "pulumi.com/autonamed": "true"
            },
            "labels": {
                "app.kubernetes.io/managed-by": "pulumi"
            },
            "name": "test-secret-7svha8ld"
        },
        "stringData": {
            "password": {
                "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
                "ciphertext": "AAABAGsAUtOoR8uxIpm6rI+0iF0RalTwT61IV98/Np6glJtsy52OXOBoPdX9"
            }
        }
    },
    "apiVersion": "v1",
    "data": {
        "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
        "ciphertext": "AAABAAo+zqiVzk0Rt3lx/p7RhPumoA69p2dW5RhqoEnvJ3uO36WcQ1LVFEnzQUubw+LaR4MR88cU/THx/N0Nz3fxahbCwmbLLEI4iQr+v1JGhE/l8tzabFd+PUhwqgJ+HeccNzKYw5JwET0U+38pIgYPWEvWS3Lh9h9b8Pw8hfputQa1urzPp4cjuRZTO0W8wK6B96ke1v0="
    },
    "kind": "Secret",
    "metadata": {
        "annotations": {
            "kubectl.kubernetes.io/last-applied-configuration": {
                "4dabf18193072939515e22adb298388d": "1b47061264138c4ac30d75fd1eb44270",
                "ciphertext": "AAABAE7MBg73fWi4VtLylQBDcj5ojG2geGnwBNycEB7W1PmHoolk/Igc5DLEZVaVXL9fefj0iuAWjeBY/6n82VN4ZOgwz53Y23J7snuDUixBBkC+d1bK9kJyJBnvljejMhUviwOYY5zcMcQhNMRqhEJsUBJQeznkNWyQ/VRhVs0baifi1EuZrmz8M7PSw3XFCgH9Ltglo+dqY8T4hy52v126f3BdT594shaStrvIAHDHZsyO+8SnU2Ooy3BBpMayGH9xwjUsNaTt/xQzEJ4/CK6H+p2kJ9OqmCT3vJfVqp4XLUCrsOZdrhFNJ+zVihmbTnUKEbf1xCy0P4Hexo8LeL/XPlMJFXkNjhaJ3ivIbYNGxxkHPkqEshNupQ=="
            },
...

I do see the behavior you're reporting if you pass in a plain value. While the outputs are automatically encrypted, I agree that it's probably worth encrypting the inputs by default as well.

@brandonkal
Copy link
Author

Thanks for checking. With a fresh pulumi stack I am seeing what you are seeing.
If it is done the other way around, it is different:

  1. On a fresh stack, run the code with pulumi up without the pulumi.secret() on the input
  2. Add back the pulumi.secret() and run pulumi up
  3. Observe a state file that looks like this:
    https://gist.github.com/brandonkal/083692eb09f6774736e999a1382f5ac9

So ideally pulumi should check if the json path in the last-applied annotation is now marked as a secret, and encrypt the output.

@lblackstone
Copy link
Member

This should be fixed now that we automatically mark Secret data and stringData fields as Pulumi secrets.

@lblackstone lblackstone added the resolution/fixed This issue was fixed label Jul 28, 2021
@lblackstone lblackstone added the kind/bug Some behavior is incorrect or out of spec label Jul 28, 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
kind/bug Some behavior is incorrect or out of spec last-applied-configuration Issues related to the last-applied-configuration annotation resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

2 participants