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

Secret.get does has the Secret's contents in plaintext in the statefile #2300

Closed
mike-chen-samsung opened this issue Feb 2, 2023 · 3 comments · Fixed by #2301
Closed

Secret.get does has the Secret's contents in plaintext in the statefile #2300

mike-chen-samsung opened this issue Feb 2, 2023 · 3 comments · Fixed by #2301
Labels
impact/security kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@mike-chen-samsung
Copy link

mike-chen-samsung commented Feb 2, 2023

What happened?

import { Secret } from '@pulumi/kubernetes/core/v1'

Secret.get(...)

If you look at the resource in the statefile, its data will be (encoded) in plaintext under outputs.__inputs.data. This is a security concern! Not only is it unencrypted in state, but it will sometimes show as a diff in pulumi up --diff --refresh

Expected Behavior

As a general rule, I expect that if an outputs.xxx value is encrypted, its corresponding outputs.__inputs.xxx value should be encrypted too

Steps to reproduce

  1. Create a Secret
echo 'apiVersion: v1
kind: Secret
metadata:
  name: mysecret
type: Opaque
data:
  USER_NAME: YWRtaW4=
  PASSWORD: MWYyZDFlMmU2N2Rm' | kubectl apply -f -
  1. In your Pulumi code add Secret.get("mysecret", "default/mysecret")
  2. pulumi stack export > state.json
  3. Find the secret in the statefile and see that outputs.__inputs.data is unencrypted (though outputs.data is properly encrypted)

Output of pulumi about

CLI          
Version      3.53.1
Go Version   go1.19.5
Go Compiler  gc

Plugins
NAME        VERSION
kubernetes  3.21.3

Host     
OS       darwin
Version  11.6
Arch     x86_64

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@mike-chen-samsung mike-chen-samsung added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Feb 2, 2023
@thomas11
Copy link
Contributor

thomas11 commented Feb 3, 2023

Thank you for the report, @mike-chen-samsung, this sounds indeed concerning. A first glance at the code shows that output.__inputs is supposed to be encrypted.

We'll investigate further today.

@thomas11 thomas11 added impact/security and removed needs-triage Needs attention from the triage team labels Feb 3, 2023
@thomas11
Copy link
Contributor

thomas11 commented Feb 3, 2023

I was able to repro this locally with a default Minikube installation, your instructions above (except stack export instead of state export), and this minimal program:

import * as k8s from "@pulumi/kubernetes";
k8s.core.v1.Secret.get("mysecret", "default/mysecret")

thomas11 added a commit that referenced this issue Feb 3, 2023
checkpointObject checks for secrets in the input, but in this case
the input is of Kind=="Secret" but ContainsSecret() is false.

I'm not quite sure yet that this is the best fix but it works in my
local repro.
thomas11 added a commit that referenced this issue Feb 6, 2023
Encrypt secrets in __inputs  #2300

`checkpointObject` checks for secrets in the input, but in this case the input is of Kind=="Secret" and ContainsSecret() is false.

Also encrypt annotations and stringData in the secret kind case.
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Feb 6, 2023
@thomas11
Copy link
Contributor

thomas11 commented Feb 7, 2023

Hi @mike-chen-samsung, the latest pulumi-kubernetes v3.24.0 has this fixed. Thank you again for reporting it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/security kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants