pulp_webserver: Add support for TLS configuration #325

Spredzy · 2020-06-09T13:26:46Z

Enable HTTPS by default when deploying a new pulp server. One can either
specify the value of the certificate and the key. Or, if none available,
can have the installer generating them.

Support has been added for both nginx and apache.

fixes #6845
fixes #6847

pulpbot · 2020-06-09T13:30:30Z

Attached issue: https://pulp.plan.io/issues/6845

Attached issue: https://pulp.plan.io/issues/6847

CHANGES/6845.feature

roles/pulp_webserver/README.md

roles/pulp_webserver/handlers/main.yml

roles/pulp_webserver/README.md

mdellweg

This is a breaking change, because we (by default) disable the api on port 80. I think, this is the behaviour an end use would want. So can you please add a .removal changelog entry?

Since pulp_webserver_disable_https is False by default, i believe our molecule tests are already executing the new code paths. But i would be glad to see some additional tests, that the api (status page) is really accessible via https and that port 80 is redirected there.

roles/pulp_webserver/tasks/generate_tls_certificates.yml

roles/pulp_webserver/tasks/main.yml

molecule/scenario_resources/tests/test_default.rb

roles/pulp_webserver/README.md

Spredzy · 2020-06-19T10:09:37Z

@mdellweg I have a question regarding your last comment - Not surre I fully grasp it.

This is a breaking change, because we (by default) disable the api on port 80. I think, this is the behaviour an end use would want. So can you please add a .removal changelog entry?

This doesn't enable api on port 80 per se. (Not more that it was already reachable on port 80 before this PR through nginx proxying). So not sure which removal is required here. For user this change "should" be transparent and if they keep reaching http they will be automatically re-routed to https.

Could you provide me with more details on what you meant in the above ? (And thanks for the reviews)

mdellweg · 2020-06-19T11:04:37Z

I believe, you need to adjust the content_origin setting (found in multiple places).

mikedep333

Thank you for working on this! This is a lot of work I can tell, but see comments.

mikedep333 · 2020-06-19T18:38:53Z

molecule/scenario_resources/tests/test_default.rb

+  describe http('http://localhost/pulp/api/v3/status',
+                 ssl_verify: false) do
+      its('status') { should eq 301 }
+    end
+  end
+
+  describe http('http://localhost/pulp/api/v3/status',
+                 ssl_verify: false, max_redirects: 1) do
+      its('status') { should eq 200 }
+      its('body') { should match /database_connection/ }
+    end
+  end
+
+  describe http('https://localhost/pulp/api/v3/status',
+                 ssl_verify: false) do
+      its('status') { should eq 200 }
+      its('body') { should match /database_connection/ }
+    end
+  end


These will fail due to this bug:
https://pulp.plan.io/issues/6586

It's been bugging me for a while, but other devs do not want privileged containers, so it is non-trivial to fix.

roles/pulp_webserver/handlers/main.yml

roles/pulp_webserver/tasks/generate_tls_certificates.yml

roles/pulp_webserver/README.md

roles/pulp_webserver/defaults/main.yml

roles/pulp_webserver/tasks/generate_tls_certificates.yml

mikedep333 · 2020-06-19T19:22:24Z

roles/pulp_webserver/tasks/generate_tls_certificates.yml

+- name: Generate CA key
+  openssl_privatekey:
+    path: '{{ pulp_webserver_tls_folder }}/root.key'


I'm concerned about what happens with these modules if a user runs the installer, replaces the key files on disk in the same filepaths with their desired keys, and then re-runs the installer:
https://docs.ansible.com/ansible/latest/modules/openssl_privatekey_module.html

Please note that the module regenerates private keys if they don’t match the module’s options. In particular, if you provide another passphrase (or specify none), change the keysize, etc., the private key will be regenerated. If you are concerned that this could overwrite your private key, consider using the backup option.

In a general approach, I would say don't do that. If this is what you want to do, use the installer to do it and re-run the installer with pulp_webserver_ssl_cert and pulp_webserver_ssl_key. If you don't yet says you want TLS then yes what you have described will happen. But I feel like it is doing it by design, rightfully.

If you think this is wrong. We can condition this whole block into the conditional non-existence off those 2 files.
And add a pulp_webserver_force_regenerate_certificate or something similar if one really really wants to do it.

I agree. If the keys are managed by ansible, do not even think about messing around by hand.

mikedep333 · 2020-06-19T19:23:59Z

roles/pulp_webserver/tasks/generate_tls_certificates.yml

+- name: Cleanup CSR files
+  file:
+    path: '{{ pulp_webserver_tls_folder }}/{{ item }}'
+    state: absent
+  loop:
+    - root.csr
+    - pulp_webserver.csr


This will fail idempotency with the task that generates them above.

Might be addressed by #325 (comment) based on the option chosen

roles/pulp_webserver/tasks/main.yml

mikedep333 · 2020-06-19T19:36:51Z

@mdellweg @Spredzy The problem we have this that often Pulp needs to return an absolute URL for certain plugins. So content_origin sets the canonical beginning of the URL, including http or https.
https://github.com/pulp/pulp_installer/blob/master/playbooks/example-use/group_vars/all#L18

So if content_origin is one protocol, and the user accesses on the other protocol, they will be redirected to the other at some point.

That said, I thought pulp-api could be accessed at port 80 via the webserver.

mdellweg · 2020-06-22T06:41:46Z

And therefore the user should reconfigure her/his clients to the new protocol, but must reset content_origin, which is a breaking change.

Enable HTTPS by default when deploying a new pulp server. One can either specify the value of the certificate and the key. Or, if none available, can have the installer generating them. Support has been added for both nginx and apache. fixes #6845 fixes #6847

Spredzy · 2020-07-02T09:28:54Z

@mdellweg I have addressed/answered all comments in this PR. I'll let you make this PR cross the finish line. And work out the details. Thanks very much for volunteering for it and hopefully it won't be a nightmare <3

mdellweg · 2020-07-07T11:21:42Z

@mikedep333 To the content_origin problem: Yes, the user is redirected to use https, but if we constantly give him new urls starting with http://, he will make one unencrypted call (probably including credentials) before getting redirected again. Therefore, i think we need to create those addresses with https in mind.

dkliban · 2020-07-08T14:28:34Z

Closing in favor of #356 which is an extension of this work.

Spredzy force-pushed the add_ssl_support branch from 3c1fa87 to 299b0b8 Compare June 9, 2020 13:28

Spredzy mentioned this pull request Jun 9, 2020

Adds self-signing certificate generation #254

Closed

Spredzy force-pushed the add_ssl_support branch 2 times, most recently from 0d10467 to 02c96e7 Compare June 9, 2020 13:48

fao89 requested a review from bmbouter June 9, 2020 14:03

Spredzy force-pushed the add_ssl_support branch 3 times, most recently from 5843eca to a91ca59 Compare June 10, 2020 12:34

bmbouter reviewed Jun 18, 2020

View reviewed changes

CHANGES/6845.feature Outdated Show resolved Hide resolved

bmbouter reviewed Jun 18, 2020

View reviewed changes

roles/pulp_webserver/README.md Outdated Show resolved Hide resolved

bmbouter reviewed Jun 18, 2020

View reviewed changes

roles/pulp_webserver/handlers/main.yml Show resolved Hide resolved

bmbouter reviewed Jun 18, 2020

View reviewed changes

roles/pulp_webserver/README.md Outdated Show resolved Hide resolved

mdellweg reviewed Jun 18, 2020

View reviewed changes

roles/pulp_webserver/tasks/generate_tls_certificates.yml Outdated Show resolved Hide resolved

roles/pulp_webserver/tasks/main.yml Outdated Show resolved Hide resolved

Spredzy force-pushed the add_ssl_support branch 2 times, most recently from 2238b57 to 3740243 Compare June 19, 2020 10:02

mdellweg reviewed Jun 19, 2020

View reviewed changes

molecule/scenario_resources/tests/test_default.rb Show resolved Hide resolved

mdellweg reviewed Jun 19, 2020

View reviewed changes

molecule/scenario_resources/tests/test_default.rb Show resolved Hide resolved

mdellweg reviewed Jun 19, 2020

View reviewed changes

roles/pulp_webserver/README.md Show resolved Hide resolved

Spredzy force-pushed the add_ssl_support branch from 3740243 to 3d14c43 Compare June 19, 2020 10:05

Spredzy force-pushed the add_ssl_support branch 2 times, most recently from be915c8 to 636cf65 Compare June 19, 2020 10:34

mikedep333 requested changes Jun 19, 2020

View reviewed changes

Spredzy force-pushed the add_ssl_support branch 3 times, most recently from 3ac5c42 to f4818ee Compare July 2, 2020 09:18

Spredzy force-pushed the add_ssl_support branch from f4818ee to 8b3e614 Compare July 2, 2020 09:27

mdellweg mentioned this pull request Jul 7, 2020

Add ssl support #356

Merged

dkliban closed this Jul 8, 2020

pulp_webserver: Add support for TLS configuration #325

pulp_webserver: Add support for TLS configuration #325

Spredzy commented Jun 9, 2020

pulpbot commented Jun 9, 2020

mdellweg left a comment

Spredzy commented Jun 19, 2020

mdellweg commented Jun 19, 2020

mikedep333 left a comment

mikedep333 Jun 19, 2020

mikedep333 Jun 19, 2020

Spredzy Jul 2, 2020 •

edited

mdellweg Jul 7, 2020

mikedep333 Jun 19, 2020

Spredzy Jul 2, 2020

mikedep333 commented Jun 19, 2020 •

edited

mdellweg commented Jun 22, 2020

Spredzy commented Jul 2, 2020

mdellweg commented Jul 7, 2020

dkliban commented Jul 8, 2020

pulp_webserver: Add support for TLS configuration #325

pulp_webserver: Add support for TLS configuration #325

Conversation

Spredzy commented Jun 9, 2020

pulpbot commented Jun 9, 2020

mdellweg left a comment

Choose a reason for hiding this comment

Spredzy commented Jun 19, 2020

mdellweg commented Jun 19, 2020

mikedep333 left a comment

Choose a reason for hiding this comment

mikedep333 Jun 19, 2020

Choose a reason for hiding this comment

mikedep333 Jun 19, 2020

Choose a reason for hiding this comment

Spredzy Jul 2, 2020 • edited

Choose a reason for hiding this comment

mdellweg Jul 7, 2020

Choose a reason for hiding this comment

mikedep333 Jun 19, 2020

Choose a reason for hiding this comment

Spredzy Jul 2, 2020

Choose a reason for hiding this comment

mikedep333 commented Jun 19, 2020 • edited

mdellweg commented Jun 22, 2020

Spredzy commented Jul 2, 2020

mdellweg commented Jul 7, 2020

dkliban commented Jul 8, 2020

Spredzy Jul 2, 2020 •

edited

mikedep333 commented Jun 19, 2020 •

edited