Seed secrets (proxy.secretToken, etc) so they don't have to be manually generated #1993

consideRatio · 2021-01-16T15:01:07Z

Wouldn't it be great if we could automatically seed the passwords like proxy.secretToken, hub.cookieSecret, and hub.config.CryptKeeper.keys, so that the user doesn't have to generate and set them manually?

The reason we haven't before is that we had no mechanism to re-use previous generated passwords, but that changed with Helm 3.1 that introduced the lookup function which we can make use of following #1994.

The strategy in this PR is to...

Use an explicitly provided password (backward compatebility!)
Use an existing password in k8s Secret (helm3's new lookup function!)
Use a new randomly generated password

With this, we close #1910.

Action points

Update documentation to reflect we don't have to set proxy.secretToken any more
Update schema.yaml if there are such entries
CI: Provide some extra --set flags to the upgrade steps were we pass proxy.secretToken explicitly so it won't keep failing until next release.
Provide feedback to users in a NOTES.txt note about no longer needing to pass an explicit value for the secrets we now can autogenerate.

Notes for changelog

Note that users should make the upgrade once and then remove what they previously set explicitly in order to avoid a key rotation.

Bonus feature for another PR

Support setting "rotate" or similar to generate a new secret.

manics · 2021-01-21T11:24:59Z

I like it, I'll test it locally if I have time.

I guess we'll need to update the Z2JH docs too?

consideRatio

PR ready for review / merge!

consideRatio · 2021-01-21T17:30:50Z

doc/source/administrator/debug.md

-
-   ```
-   helm upgrade --cleanup-on-fail jhub jupyterhub/jupyterhub -f config.yaml
-   ```


There is an open issue to improve the debugging section, so instead of finding an example with diagnosis etc that wasn't correct any more, I opted to delete the section.

consideRatio · 2021-01-21T17:31:35Z

doc/source/jupyterhub/installation.md

-<!---
-Don't put an example here! People will just copy paste that & that's a
-security issue.
-->


There were significant differences here, so I ended up updating some leading paragraphs as well.

minrk · 2021-01-22T15:42:12Z

jupyterhub/files/hub/jupyterhub_config.py

@@ -372,6 +372,11 @@ def camelCaseify(s):
    c.Spawner.debug = True


+# load seeded secrets
+c.JupyterHub.proxy_auth_token = get_secret_value("JupyterHub.proxy_auth_token")


To clarify: the change here is that these secrets are no longer optional in the chart, and cannot possibly be left unspecified or null, is that correct? The previous logic allowed these to be unspecified, in which case the existing default behavior would occur, but explicitly setting them to null or empty values is not the same thing.

It is correct that we will now always provide a value to these three configuration options, if you search for "assert hack" you will find an assertion I make about that.

Any falsy value for proxy.secretToken, hub.cookieSecret, hub.config.CryptKeeper.keys would lead to automatic generation of a new passwords.

Is this indirectly influencing the behavior of JupyterHub?

get_secret_value("JupyterHub.proxy_auth_token") returns the hub k8s Secret's key's value, which in turn is set by a named template, which in turn has priority logic to:

return any explicitly set value

return the previous k8s Secret's value

return autogenerated value

jupyterhub/templates/NOTES.txt

jupyterhub/templates/hub/secret.yaml

yuvipanda · 2021-01-25T12:26:54Z

This is great!

Resetting / rotating these values is important when you believe they're compromised. Access to proxy secret basically gives you full access to every user's traffic, and access to cookie secret is terrible too. Rotating cookie secret logs everyone out (I think) and that's also very useful.

So as long as these can be set explicitly, I'm <3. But, great that it isn't required!

consideRatio · 2021-01-25T12:35:42Z

Thanks @minrk and @yuvipanda for your feedback!!

Resetting / rotating these values is important when you believe they're compromised.

While it is a bit of a feature creep of this PR, do you have a suggestion on what API to use in order to do this?

Passing -, reset, or rotate as a signal to reset and generate a new secret? A more complicated solution would be to pass reset-<time> or similar which would only reset if it was changed rather than over and over. Doing this, one would not need to first do an upgrade passing - and then a subsequent upgrade passing a blank string again.

yuvipanda · 2021-01-25T13:29:50Z

While it is a bit of a feature creep of this PR, do you have a suggestion on what API to use in order to do this?

If these properties are set explicitly, they should take precedence over the autogenerated values. I think that's the best way to do this. So if we are only autogenerating values when the fields are unset, that's good enough :)

consideRatio · 2021-01-25T20:35:26Z

If these properties are set explicitly, they should take precedence over the autogenerated values. I think that's the best way to do this. So if we are only autogenerating values when the fields are unset, that's good enough :)

Okay yeah then we are good to go - that is what this PR currently does.

…rets" This reverts commit 78b6d75.

consideRatio · 2021-01-26T13:49:37Z

I added a randHex function and made the generated length 64 hex characters, which aligns with documentation in JupyterHub for cookie secret and how we previously instructed users to generate their passwords.

That was the last itch I had with regards to this PR.

minrk · 2021-01-27T09:15:03Z

Awesome! So great to have this resolved.

jupyterhub/zero-to-jupyterhub-k8s#1993 Merge pull request #1993 from consideRatio/pr/seed-secrets

consideRatio · 2021-01-27T09:38:07Z

Wieeee thanks for review / merge @minrk @yuvipanda @manics! 🎉 ❤️

Followup fixes to seed secrets PR (#1993)

With jupyterhub/zero-to-jupyterhub-k8s#1993, these are no longer required to be explicitly set. We remove both just for staging hubs - doing so for production hubs will cause everyone to be logged out (cookieSecret) and temporarily disconnected (proxy.secretToken) so we aren't doing that yet

yuvipanda · 2021-05-05T08:36:17Z

jupyterhub/templates/hub/_helpers-passwords.tpl

+    {{- $result := "" }}
+    {{- range $i := until 100 }}
+        {{- if lt (len $result) . }}
+            {{- $rand_list := randAlphaNum . | splitList "" -}}


For those curious - randAlphaNum is cryptographically safe too - http://masterminds.github.io/sprig/strings.html.

yuvipanda · 2021-05-05T08:56:20Z

Also, you can rotate these secrets with k -n <namespace> delete secret hub and then doing a redeploy. New keys!

Follow-up to berkeley-dsep-infra/datahub#2381. I verified that this doesn't *actually* rotate the secrets themeselves - just removes them from git. This just reduces the amount of secret data we store here. See jupyterhub/zero-to-jupyterhub-k8s#1993 for more info on what is happening here.

Thanks to jupyterhub/zero-to-jupyterhub-k8s#1993, proxy.secretToken is automatically generated :)

consideRatio marked this pull request as draft January 16, 2021 15:01

consideRatio force-pushed the pr/seed-secrets branch 6 times, most recently from 776bafd to dbe8403 Compare January 16, 2021 15:30

consideRatio added the enhancement label Jan 16, 2021

consideRatio marked this pull request as ready for review January 20, 2021 18:57

consideRatio requested review from minrk, manics and yuvipanda January 20, 2021 18:58

consideRatio force-pushed the pr/seed-secrets branch from dbe8403 to d6eca86 Compare January 20, 2021 18:59

consideRatio changed the title ~~Seed secrets so they don't have to be manually generated~~ Seed secrets (proxy.secretToken, etc) so they don't have to be manually generated Jan 20, 2021

consideRatio force-pushed the pr/seed-secrets branch from d6eca86 to f068a19 Compare January 21, 2021 11:43

consideRatio added 2 commits January 21, 2021 17:04

Try seed secrets

698675e

docs: proxy.secretToken / hub.cookieSret are now autogenerated

8ce16b3

consideRatio force-pushed the pr/seed-secrets branch 2 times, most recently from ddf3e1a to abdd456 Compare January 21, 2021 16:42

consideRatio added 2 commits January 21, 2021 17:51

ci: help upgrade-from steps work during transition

227d7fc

Give feedback to users explicitly setting autogeneratable secrets

78b6d75

consideRatio force-pushed the pr/seed-secrets branch from abdd456 to 78b6d75 Compare January 21, 2021 16:51

ci: let helm diff show secrets

fc3f27e

consideRatio force-pushed the pr/seed-secrets branch from 3aa39ca to fc3f27e Compare January 21, 2021 17:18

consideRatio commented Jan 21, 2021

View reviewed changes

consideRatio force-pushed the pr/seed-secrets branch from 4f17b51 to f96f255 Compare January 21, 2021 17:33

ci: split steps for improved readability of logs

4517d02

consideRatio force-pushed the pr/seed-secrets branch from f96f255 to 4517d02 Compare January 21, 2021 17:34

minrk reviewed Jan 22, 2021

View reviewed changes

minrk reviewed Jan 25, 2021

View reviewed changes

jupyterhub/templates/NOTES.txt Outdated Show resolved Hide resolved

minrk reviewed Jan 25, 2021

View reviewed changes

jupyterhub/templates/hub/secret.yaml Outdated Show resolved Hide resolved

consideRatio and others added 2 commits January 26, 2021 12:48

Avoid duplicated load of potentially seeded secrets

a2d0c23

Revert "Give feedback to users explicitly setting autogeneratable sec…

b8c60ef

…rets" This reverts commit 78b6d75.

consideRatio force-pushed the pr/seed-secrets branch from 25bf8a0 to b8c60ef Compare January 26, 2021 11:49

consideRatio and others added 2 commits January 26, 2021 13:17

Update assertion message and comments in hub Secret for readability

b94fec5

Add randHex named template based on randAlphaNum

3c98c3e

consideRatio requested a review from minrk January 26, 2021 13:53

minrk merged commit 0a2a397 into jupyterhub:master Jan 27, 2021

consideRatio pushed a commit to jupyterhub/helm-chart that referenced this pull request Jan 27, 2021

[jupyterhub] Automatic update for commit 0.11.1-n073.h0a2a397d

a33388c

jupyterhub/zero-to-jupyterhub-k8s#1993 Merge pull request #1993 from consideRatio/pr/seed-secrets

consideRatio mentioned this pull request Jan 27, 2021

Allow extraFiles to be injected to hub / singleuser pods and automatically load config in /usr/local/etc/jupyterhub_config.d #2006

Merged

minrk added a commit that referenced this pull request Jan 29, 2021

Merge pull request #2016 from consideRatio/pr/fix-seed-secrets

8b63e21

Followup fixes to seed secrets PR (#1993)

consideRatio mentioned this pull request Jan 29, 2021

Automate process of building api tokens dask/helm-chart#132

Closed

yuvipanda mentioned this pull request May 5, 2021

Remove explicit cookieSecret and proxy.secretToken for staging berkeley-dsep-infra/datahub#2381

Merged

yuvipanda reviewed May 5, 2021

View reviewed changes

yuvipanda mentioned this pull request May 5, 2021

Remove proxy & cookie tokens from prod hubs berkeley-dsep-infra/datahub#2382

Merged

yuvipanda added a commit to yuvipanda/pilot-hubs that referenced this pull request May 11, 2021

Don't explicitly generate proxy.secretToken

c753aa3

Thanks to jupyterhub/zero-to-jupyterhub-k8s#1993, proxy.secretToken is automatically generated :)

yuvipanda mentioned this pull request May 12, 2021

Don't explicitly generate proxy.secretToken 2i2c-org/infrastructure#396

Merged

This was referenced Jul 13, 2021

Autogenerate and set passwords for JupyterHub's registered services' api_tokens #2308

Closed

Automatically seed the SSH key needed if unspecified yuvipanda/jupyterhub-ssh#38

Closed

Seed secrets (proxy.secretToken, etc) so they don't have to be manually generated #1993

Seed secrets (proxy.secretToken, etc) so they don't have to be manually generated #1993

Uh oh!

Conversation

consideRatio commented Jan 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Action points

Notes for changelog

Bonus feature for another PR

Uh oh!

manics commented Jan 21, 2021

Uh oh!

consideRatio left a comment

Choose a reason for hiding this comment

Uh oh!

consideRatio Jan 21, 2021

Choose a reason for hiding this comment

Uh oh!

consideRatio Jan 21, 2021

Choose a reason for hiding this comment

Uh oh!

minrk Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

consideRatio Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

consideRatio Jan 26, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yuvipanda commented Jan 25, 2021

Uh oh!

consideRatio commented Jan 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuvipanda commented Jan 25, 2021

Uh oh!

consideRatio commented Jan 25, 2021

Uh oh!

consideRatio commented Jan 26, 2021

Uh oh!

minrk commented Jan 27, 2021

Uh oh!

consideRatio commented Jan 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuvipanda May 5, 2021

Choose a reason for hiding this comment

Uh oh!

yuvipanda commented May 5, 2021

Uh oh!

Uh oh!

consideRatio commented Jan 16, 2021 •

edited

Loading

consideRatio commented Jan 25, 2021 •

edited

Loading

consideRatio commented Jan 27, 2021 •

edited

Loading