Project

Profile

Help

Story #2416

closed

Generate random SECRET_KEY for Django as part of setup workflow

Added by dkliban@redhat.com about 8 years ago. Updated almost 5 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
Start date:
Due date:
% Done:

100%

Estimated time:
Platform Release:
Groomed:
Yes
Sprint Candidate:
Yes
Tags:
Sprint:
Sprint 21
Quarter:

Description

The setup workflow for a new deployment of Pulp platform needs to generate a random SECRET_KEY[0] for the Django app serving the API.

This should not be done by the RPM installation automatically, so that pre-built images can be re-used with their own config and especially their own secrets.

[0] https://docs.djangoproject.com/en/1.8/ref/settings/#std:setting-SECRET_KEY

Actions #1

Updated by mhrivnak over 7 years ago

This should be done at the same time as other related secret-generation, like the RSA key. Jeff did some work to separate that stuff out from the install process, because when building an image that you expect to re-use (container or VM), you don't necessarily want a secret re-used.

@jortel, can you elaborate on what you did and how this might fit in?

Actions #2

Updated by jortel@redhat.com over 7 years ago

Mainly moved the key generation to a separate script[0] users run during installation.

The django documentation calls this a SECRET_KEY but it really seems to be a secret (str) not an actual key. Unlike this secret, the RSA keys can be (and often are) shared between pulp installations. Not sure a script is a good fit for this. Perhaps just generate SECRET_KEY when the settings.py is imported?

[0] https://github.com/pulp/pulp/blob/master/server/bin/pulp-gen-key-pair

Actions #3

Updated by bmbouter over 7 years ago

I'm wondering if this issue should be about doing this at pip install time instead of rpm install time. The rpm install calls the pip install so if we put it there we would have the secret generated for both install types. What do you all think?

Actions #4

Updated by bmbouter over 7 years ago

wrote:

The django documentation calls this a SECRET_KEY but it really seems to be a secret (str) not an actual key. Unlike this secret, the RSA keys can be (and often are) shared between pulp installations. Not sure a script is a good fit for this. Perhaps just generate SECRET_KEY when the settings.py is imported?

I believe the SECRET_KEY needs to be shared across all machines accessing the same Django db, so that would be all Pulp boxes in the cluster. Since pip is run on each box, we need to ensure that all machines get the same string value for that. This is like all the Pulp settings in server.yaml so we probably should read the value of SECRET_KEY out of server.yaml which also holds other shared secrets.

I'm not exactly sure how or when we should set the value of that field in server.yaml.

As an aside, maybe server.yaml should be on the clustered filesystem so that users don't have to propagate settings across so many machines?

Actions #5

Updated by mhrivnak over 7 years ago

  • Subject changed from Generate random SECRET_KEY for Django as part of pulp-server.rpm install to Generate random SECRET_KEY for Django as part of setup workflow
  • Description updated (diff)

The need to share config across machines has been around a long time, and I don't think we need to do anything special for it. Let's leave our config in /etc/ and make it easy to manage with config management.

And to reiterate, automatically generating secrets at the time of software installation, whether by rpm or pip, is insecure. It needs to be done at deployment time, which may be different than install time for deployment modes that use pre-baked images. That includes containerized deployments as well as VMs. More concretely:

If we generate it at RPM install time while creating an image, the secret is baked into the image. Everyone using the image will have the same secret. Not good.

If we generate it at pip install time, then the secret is baked into the RPM itself. Everyone installing that RPM would have the same secret. This is worse.

I'm updating the title and description accordingly so it matches our approach to generating other secrets.

Actions #6

Updated by mhrivnak over 7 years ago

  • Sprint Candidate changed from No to Yes
Actions #7

Updated by bmbouter over 7 years ago

mhrivnak Is the idea that it's part of the documentation for the user to create and set this string in the conf file?

We don't have an installation page currently, maybe this would create one? That would be good overall I think.

I think I understand this story better now. I think what this story would do would be to delete this line from here[0] and have it added as a config section here[1] right? The settings from[1] are automatically overlaid onto the Django settings[0] already.

[0]: https://github.com/pulp/pulp/blob/253ae59c9d80a4698c47a0608f96e5d81a62f0d7/platform/pulpcore/app/settings.py#L25
[1]: https://github.com/pulp/pulp/blob/b86677f069c03643380d98bc51cd1291fdfa3289/platform/etc/pulp/server.yaml

As an aside, is the SECRET_KEY field required for dev installs? If so, we should also add a step to Ansible for the dev setup to randomly generate this field. This is also related to if vagrant is symlinking or copying the server.yaml file for dev installs. If every time the Ansible dev setup runs it modifies the installed server.yaml it can't be under version control.

Actions #8

Updated by mhrivnak over 7 years ago

Thanks for the help!

bmbouter wrote:

mhrivnak Is the idea that it's part of the documentation for the user to create and set this string in the conf file?

That seems reasonable. I'll add a checklist item about it.

We don't have an installation page currently, maybe this would create one? That would be good overall I think.

Agreed. Those steps are currently captured here:

http://docs.pulpproject.org/user-guide/installation/f23-.html#server

But breaking that out to a dedicated section or page would be a good idea. Although that doesn't necessarily need to happen with this task.

I think I understand this story better now. I think what this story would do would be to delete this line from here[0] and have it added as a config section here[1] right? The settings from[1] are automatically overlaid onto the Django settings[0] already.

[0]: https://github.com/pulp/pulp/blob/253ae59c9d80a4698c47a0608f96e5d81a62f0d7/platform/pulpcore/app/settings.py#L25
[1]: https://github.com/pulp/pulp/blob/b86677f069c03643380d98bc51cd1291fdfa3289/platform/etc/pulp/server.yaml

Something like that. Deleting it may or may not be the best thing. It's behaviorally just like database settings. The user needs to provide them in the config file, and we need to load them into django's settings.

As an aside, is the SECRET_KEY field required for dev installs? If so, we should also add a step to Ansible for the dev setup to randomly generate this field. This is also related to if vagrant is symlinking or copying the server.yaml file for dev installs. If every time the Ansible dev setup runs it modifies the installed server.yaml it can't be under version control.

django documents it as required, so yes this is a great point. I'll add it as a checklist item.

Being able to modify the settings file in your dev env without modifying the checked-in copy is a general need we have. Devs want to change log levels, try out a different DB, try a different message broker, reproduce bugs with specific config values, etc. We should figure that out, but not necessarily as part of this work.

Actions #9

Updated by pcreech over 7 years ago

  • Groomed changed from No to Yes
Actions #10

Updated by mhrivnak over 7 years ago

  • Sprint/Milestone set to 40
Actions #11

Updated by bizhang over 7 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to bizhang

Added by werwty over 7 years ago

Revision ee2f1911 | View on GitHub

Copy server.yaml into correct location, generate django secret_key

re #2416 https://pulp.plan.io/issues/2416

Added by werwty over 7 years ago

Revision 3179575c | View on GitHub

Remove SECRET_KEY from Django settings

closes #2416 https://pulp.plan.io/issues/2416

Added by werwty over 7 years ago

Revision 3179575c | View on GitHub

Remove SECRET_KEY from Django settings

closes #2416 https://pulp.plan.io/issues/2416

Actions #13

Updated by werwty over 7 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100
Actions #14

Updated by bmbouter over 6 years ago

  • Sprint set to Sprint 21
Actions #15

Updated by bmbouter over 6 years ago

  • Sprint/Milestone deleted (40)
Actions #16

Updated by daviddavis over 5 years ago

  • Sprint/Milestone set to 3.0.0
Actions #17

Updated by bmbouter over 5 years ago

  • Tags deleted (Pulp 3)
Actions #18

Updated by bmbouter almost 5 years ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE

Also available in: Atom PDF