Project

Profile

Help

Story #2416

Generate random SECRET_KEY for Django as part of setup workflow

Added by dkliban@redhat.com about 3 years ago. Updated 7 months ago.

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

100%

Platform Release:
Blocks Release:
Backwards Incompatible:
No
Groomed:
Yes
Sprint Candidate:
Yes
Tags:
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 21

Description

The setup workflow for a new deployment of Pulp platform needs to generate a random SECRET_KEY0 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


Checklist

Associated revisions

Revision ee2f1911 View on GitHub
Added by werwty over 2 years ago

Copy server.yaml into correct location, generate django secret_key

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

Revision 3179575c View on GitHub
Added by werwty over 2 years ago

Remove SECRET_KEY from Django settings

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

Revision 3179575c View on GitHub
Added by werwty over 2 years ago

Remove SECRET_KEY from Django settings

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

Revision 3179575c View on GitHub
Added by werwty over 2 years ago

Remove SECRET_KEY from Django settings

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

History

#1 Updated by mhrivnak over 2 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?

#2 Updated by jortel@redhat.com over 2 years ago

Mainly moved the key generation to a separate script0 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

#3 Updated by bmbouter over 2 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?

#4 Updated by bmbouter over 2 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?

#5 Updated by mhrivnak over 2 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.

#6 Updated by mhrivnak over 2 years ago

  • Sprint Candidate changed from No to Yes

#7 Updated by bmbouter over 2 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 here0 and have it added as a config section here1 right? The settings from1 are automatically overlaid onto the Django settings0 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.

#8 Updated by mhrivnak over 2 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 here0 and have it added as a config section here1 right? The settings from1 are automatically overlaid onto the Django settings0 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.

#9 Updated by pcreech over 2 years ago

  • Groomed changed from No to Yes

#10 Updated by mhrivnak over 2 years ago

  • Sprint/Milestone set to 40

#11 Updated by bizhang over 2 years ago

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

#13 Updated by werwty over 2 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#14 Updated by bmbouter over 1 year ago

  • Sprint set to Sprint 21

#15 Updated by bmbouter over 1 year ago

  • Sprint/Milestone deleted (40)

#16 Updated by daviddavis 7 months ago

  • Sprint/Milestone set to 3.0

#17 Updated by bmbouter 7 months ago

  • Tags deleted (Pulp 3)

Please register to edit this issue

Also available in: Atom PDF