Allow rotating the password non-interactively #11094

nicklasfrahm · 2022-10-20T07:54:44Z

Signed-off-by: Nicklas Frahm nilfr@vestas.com

Description

Fixes #11083. Also fixes some deprecation warnings within the passphrase package.

Checklist

I have tested this locally using:

export PULUMI_CONFIG_PASSPHRASE=test
pulumi stack init -s test
echo "hello" | pulumi stack change-secrets-provider passphrase
export PULUMI_CONFIG_PASSPHRASE=hello
echo "test" | pulumi stack change-secrets-provider passphrase

I have run make changelog and committed the changelog/pending/<file> documenting my change

Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

Signed-off-by: Nicklas Frahm <nilfr@vestas.com>

github-actions · 2022-10-20T07:55:10Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

pulumi-bot · 2022-10-20T07:55:20Z

Changelog

[uncommitted] (2022-10-21)

Features

[cli] Allow rotating the passphrase non-interactively

RobbieMcKinstry · 2022-10-20T15:00:28Z

/run-acceptance-tests
Please view the results of the acceptance tests Here

pkg/secrets/passphrase/manager.go

RobbieMcKinstry

LGTM! One small change requested, otherwise I'm good with it! :)

RobbieMcKinstry · 2022-10-20T15:13:39Z

Also, I just want to say thanks, @nicklasfrahm. I appreciate you taking the time to fix this issue (especially so quickly after it was just opened)! 🎉

github-actions · 2022-10-21T09:32:44Z

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

/run-acceptance-tests - used to test run the acceptance tests for the project
/run-codegen - used to test the Pull Request against downstream codegen
/run-docs-gen - used to test the Pull Request against documentation generation

nicklasfrahm · 2022-10-21T09:35:26Z

@RobbieMcKinstry Could you give this another look?

Frassle · 2022-10-24T08:29:44Z

bors merge

11094: Allow rotating the password non-interactively r=Frassle a=nicklasfrahm Signed-off-by: Nicklas Frahm <nilfr@vestas.com>  # Description Fixes #11083. Also fixes some deprecation warnings within the `passphrase` package. ## Checklist  - [x] I have tested this locally using: ```bash export PULUMI_CONFIG_PASSPHRASE=test pulumi stack init -s test echo "hello" | pulumi stack change-secrets-provider passphrase export PULUMI_CONFIG_PASSPHRASE=hello echo "test" | pulumi stack change-secrets-provider passphrase ```  - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change  - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version  Co-authored-by: Nicklas Frahm <nilfr@vestas.com>

bors · 2022-10-24T09:22:43Z

Build failed:

bors-ok

Frassle · 2022-10-24T09:26:03Z

bors retry

11094: Allow rotating the password non-interactively r=Frassle a=nicklasfrahm Signed-off-by: Nicklas Frahm <nilfr@vestas.com>  # Description Fixes #11083. Also fixes some deprecation warnings within the `passphrase` package. ## Checklist  - [x] I have tested this locally using: ```bash export PULUMI_CONFIG_PASSPHRASE=test pulumi stack init -s test echo "hello" | pulumi stack change-secrets-provider passphrase export PULUMI_CONFIG_PASSPHRASE=hello echo "test" | pulumi stack change-secrets-provider passphrase ```  - [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change  - [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version  Co-authored-by: Nicklas Frahm <nilfr@vestas.com>

bors · 2022-10-24T10:43:34Z

Build failed:

bors-ok

Frassle · 2022-10-24T10:55:28Z

bors retry

bors · 2022-10-24T12:10:04Z

Build succeeded:

bors-ok

same-id · 2023-11-29T16:21:46Z

I don't think this solution is good enough.
Consider this:

First run:

pulumi stack init -s test

No problem, stack created

Second run:

export PULUMI_CONFIG_PASSPHRASE=test
pulumi stack change-secrets-provider passphrase

No problem, secret newly created

Third run:

export PULUMI_CONFIG_PASSPHRASE=test
pulumi stack change-secrets-provider passphrase

passphrase had been set to the empty string.

The rotation must be more explicit.

I suggest:

Introducing PULUMI_CONFIG_ROTATE_PASSPHRASE, PULUMI_CONFIG_ROTATE_PASSPHRASE_FILE, PULUMI_CONFIG_ROTATE_PASSPHRASE_STDIN
If none provided will previously and explicitely fail with:
passphrase rotation requires an interactive terminal, or through the use of PULUMI_CONFIG_ROTATE_PASSPHRASE_STDIN=TRUE to read from stdin, PULUMI_CONFIG_ROTATE_PASSPHRASE to specify the new passphrase or PULUMI_CONFIG_ROTATE_PASSPHRASE_FILE to specify the new passphrase using a file.

nicklasfrahm · 2023-11-29T16:35:02Z

What if we failed it, if the passphrase is empty? I also feel like it would be nice to do some of this via flags rather than ENV vars. What do you think?

Something like, --allow-empty-passphrase?

Frassle · 2023-11-29T16:37:36Z

I'd really like to disallow empty passphrase in general but last time we did that users complained that it broke their workflows, very https://xkcd.com/1172/ feeling to that.

But we could probably disallow empty passphrases on stdin, I don't even think we need to have an opt-out to allow empty passphrases.

nicklasfrahm · 2023-11-29T16:50:37Z

I generally prefer strongly discouraging these things and allowing opt-outs for the 1% of cases that are not the user's but the environment's fault. Phase of moon, etc.

same-id · 2023-11-29T17:01:14Z

For context, I reached here through a miraculous turn of events that took me forever to debug

//nolint:paralleltest // mutates environment variables
func TestChangeSecretsProviderToPassphraseNonInteractively(t *testing.T) {
	e := ptesting.NewEnvironment(t)
	e.Passphrase = "correct horse battery staple"
	defer func() {
		if !t.Failed() {
			e.DeleteEnvironment()
		}
	}()

	const stackName = "imulup"

	integration.CreateBasicPulumiRepo(e)

	e.ImportDirectory("integration/stack_outputs/nodejs")

	e.SetBackend(e.LocalURL())
	e.RunCommand(
		"pulumi", "stack", "init", stackName,
	)

	e.RunCommand("pulumi", "stack", "change-secrets-provider", "passphrase")

	e.RunCommand("pulumi", "up", "--non-interactive", "--yes", "--skip-preview")
}

This fails on pulumi up with:

STDERR: error: getting stack configuration: get stack secrets manager: incorrect passphrase

Why?

I think this code is incorrect:

        // If we're setting the secrets provider to the same provider then do a rotation.
	rotateProvider := secretsProvider == currentProjectStack.SecretsProvider ||
		// passphrase doesn't get saved to stack state, so if we're changing to passphrase see if
		// the current secrets provider is empty
		((secretsProvider == "passphrase") && (currentProjectStack.SecretsProvider == ""))

Specifically:

(secretsProvider == "passphrase") && (currentProjectStack.SecretsProvider == ""))

Since it marks this is a rotation then secret provider actually being set in the first ever change-secrets-provider is actually from stdin (empty string) and not correct horse battery staple from env var.
But then up runs with correct horse battery staple?

same-id · 2023-11-29T17:33:44Z

With my suggestion, if passphrase rotation is set, It would have failed in
pulumi stack change-secrets-provider passphrase with

error: Existing passphrase rotation requires either an interactive terminal, to rotate non-interactively set PULUMI_CONFIG_ROTATE_PASSPHRASE_STDIN=TRUE to read from stdin, PULUMI_CONFIG_ROTATE_PASSPHRASE to specify the new passphrase or PULUMI_CONFIG_ROTATE_PASSPHRASE_FILE to specify the new passphrase from a file.

or at least

error: Existing passphrase rotation requires either an interactive session or provide the --stdin flag to the change-secrets-provider command.

Now I see that pulumi stack change-secrets-provider passphrase fails with "Existing passphrase rotation" which allows my to catch the actual bug.

Also pulumi stack change-secrets-provider passphrase --stdin is more explicit and you can see from miles away that it will not use the envvar but will prefer stdin.

WDYT?

nicklasfrahm · 2023-11-29T18:08:42Z

I like the option with the flag, because it is a lot less typing than all those environment variables, so it does have a certain simplicity to it.

Frassle · 2023-11-29T20:26:39Z

Adding a flag now would be a breaking change, its much more palatable to go with erroring on empty here, still a break change but a more manageable and understandable one.

same-id · 2023-11-29T20:48:39Z

I think that it's still has to be explicit when read from stdin.

echo hi | pulumi stack change-secrets-provider passphrase <- may not really use "hi" as passphrase
...
echo bye | pulumi stack change-secrets-provider passphrase

First one maybe not even read the passphrase from stdin - depends if it was a rotation or not.

same-id · 2023-11-29T20:56:10Z

Spinned up a PR to show how it will look like:

#14698

Regardless, I do agree empty passphrases are useless, might as well not encrypt at all

feat(cli): allow rotating the passphrase non-interactively

3f0e41f

Signed-off-by: Nicklas Frahm <nilfr@vestas.com>

RobbieMcKinstry self-requested a review October 20, 2022 15:00

RobbieMcKinstry reviewed Oct 20, 2022

View reviewed changes

pkg/secrets/passphrase/manager.go Outdated Show resolved Hide resolved

RobbieMcKinstry reviewed Oct 20, 2022

View reviewed changes

RobbieMcKinstry mentioned this pull request Oct 20, 2022

Allow STDIN to change passphrase #11083

Closed

feat(cli): use scanner to read os.Stdin

3a06c7a

Frassle approved these changes Oct 24, 2022

View reviewed changes

bors bot merged commit 4870ecd into pulumi:master Oct 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow rotating the password non-interactively #11094

Allow rotating the password non-interactively #11094

nicklasfrahm commented Oct 20, 2022

github-actions bot commented Oct 20, 2022

pulumi-bot commented Oct 20, 2022 •

edited

RobbieMcKinstry commented Oct 20, 2022 •

edited by pulumi-bot

RobbieMcKinstry left a comment

RobbieMcKinstry commented Oct 20, 2022

github-actions bot commented Oct 21, 2022

nicklasfrahm commented Oct 21, 2022

Frassle commented Oct 24, 2022

bors bot commented Oct 24, 2022

Frassle commented Oct 24, 2022

bors bot commented Oct 24, 2022

Frassle commented Oct 24, 2022

bors bot commented Oct 24, 2022

same-id commented Nov 29, 2023 •

edited

nicklasfrahm commented Nov 29, 2023 •

edited

Frassle commented Nov 29, 2023

nicklasfrahm commented Nov 29, 2023 •

edited

same-id commented Nov 29, 2023

same-id commented Nov 29, 2023 •

edited

nicklasfrahm commented Nov 29, 2023

Frassle commented Nov 29, 2023

same-id commented Nov 29, 2023 •

edited

same-id commented Nov 29, 2023

Allow rotating the password non-interactively #11094

Allow rotating the password non-interactively #11094

Conversation

nicklasfrahm commented Oct 20, 2022

Description

Checklist

github-actions bot commented Oct 20, 2022

pulumi-bot commented Oct 20, 2022 • edited

Changelog

[uncommitted] (2022-10-21)

Features

RobbieMcKinstry commented Oct 20, 2022 • edited by pulumi-bot

RobbieMcKinstry left a comment

Choose a reason for hiding this comment

RobbieMcKinstry commented Oct 20, 2022

github-actions bot commented Oct 21, 2022

nicklasfrahm commented Oct 21, 2022

Frassle commented Oct 24, 2022

bors bot commented Oct 24, 2022

Frassle commented Oct 24, 2022

bors bot commented Oct 24, 2022

Frassle commented Oct 24, 2022

bors bot commented Oct 24, 2022

same-id commented Nov 29, 2023 • edited

nicklasfrahm commented Nov 29, 2023 • edited

Frassle commented Nov 29, 2023

nicklasfrahm commented Nov 29, 2023 • edited

same-id commented Nov 29, 2023

same-id commented Nov 29, 2023 • edited

nicklasfrahm commented Nov 29, 2023

Frassle commented Nov 29, 2023

same-id commented Nov 29, 2023 • edited

same-id commented Nov 29, 2023

pulumi-bot commented Oct 20, 2022 •

edited

RobbieMcKinstry commented Oct 20, 2022 •

edited by pulumi-bot

same-id commented Nov 29, 2023 •

edited

nicklasfrahm commented Nov 29, 2023 •

edited

nicklasfrahm commented Nov 29, 2023 •

edited

same-id commented Nov 29, 2023 •

edited

same-id commented Nov 29, 2023 •

edited