Use TLS authentication for cluster-api libvirt connection #296

bison · 2018-09-20T20:25:14Z

This enables TLS authentication for the connection from the in-cluster cluster-api components to the host libvirtd. It includes a script under hack/ to generate the necessary TLS assets, and includes Terraform changes to conditionally render a secret containing the client credentials.

@crawford mentioned that it might be best to hold off on this until the asset generation is rewritten, but I wanted to get something up for reference. If this needs to be put on hold, that's fine. I can rebase when ready.

openshift-ci-robot · 2018-09-20T20:25:39Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bison
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: staebler

If they are not already assigned, you can assign the PR to them by writing /assign @staebler in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

bison · 2018-09-20T21:29:20Z

/test golint
/test go-vet

wking

Overall approach looks good to me.

wking · 2018-09-20T22:26:48Z

Documentation/dev/libvirt-howto.md

 ```

-Note that authentication is not currently supported, but should be soon.
+You can omit the `--network` flag if you're using the defualt


"defualt" -> "default"

wking · 2018-09-20T22:28:06Z

Documentation/dev/libvirt-howto.md

@@ -74,45 +83,7 @@ On Debian based distros, modify `/etc/default/libvirtd` and set:
 libvirtd_opts="--listen"
 ```

-Next, restart libvirt: `systemctl restart libvirtd`
-
-Finally, if you have a firewall, you may have to allow connections from the IP


Where's this going? Don't we still need to open up the firewall to let these connections through?

We shouldn't need this any of this if we're deriving the libvirt URL from the cluster network. Rules will already be added by libvirt to allow traffic to the bridge / gateway from the nodes.

wking · 2018-09-20T22:29:51Z

examples/libvirt.yaml

-  # and that the cluster-api controller pod will be able to connect
-  # to.  Often 192.168.122.1 is the default for the virbr0 interface.
-  uri: qemu+tcp://192.168.122.1/system
+  uri: qemu:///system


Without the IP address, how does the cluster API controller talk to libvirtd?

See below. It's talking via the gateway / bridge now.

wking · 2018-09-20T22:34:25Z

installer/pkg/config-generator/generator.go

+		return "", err
+	}
+
+	uri := fmt.Sprintf("qemu+tls://%s/system?pkipath=%s", gateway.String(), libvirtPKIPath)


Ah, this is why you can drop the IP from the config. I'm a bit nervous about assuming QEMU. Won't that break non-Linux libvirt even more (#201). Can we stick with the IP-form in the config and drop the libvirtURI helper?

Hmm, I'd really like to stick with something close to this. The current default in the config is kind of a hack. It's just relying on a popularly configured default network that is totally unrelated to the infrastructure being defined for the cluster.

What if I parse the URL that's in the config here and generate this a little more carefully, keeping the driver and other options?

Changed this to be a bit smarter and added some tests.

mrogers950 · 2018-09-25T20:44:02Z

hack/libvirt-ca/main.go

+
+	cfg := &tls.CertCfg{
+		KeyUsages:    x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+		ExtKeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},


Does libvirt explicitly require a client auth EKU on the server cert?

Oops. No. Fixed.

bison · 2018-09-27T14:01:18Z

/test e2e-aws-smoke

Adds a script to help with one-time generation of TLS asserts for libvirt.

Renders a secret containing pre-generated libvirt TLS credentials to be used by the cluster-api components.

Generates the libvirt URI used by the cluster-api based on the cluster network. This lets the installer talk to libvirt over the local socket, while the cluster-api uses a TLS connection.

Bumps the machine-api-operator image to include support for mounting the libvirt credentials secret.

Updates the documentation for the libvirt development setup to include TLS authentication between the cluster-api and the host libvirtd.

bison · 2018-09-27T19:20:03Z

@wking / @crawford: What do you guys think of this at this point?

crawford · 2018-09-27T19:28:18Z

@bison I think it is not an immediate requirement, so we are deferring the review for now while we finish up some of the large work. We'll follow up once we have some spare cycles.

openshift-bot · 2018-09-30T11:28:30Z

@bison: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sallyom · 2018-10-08T19:43:06Z

@bison I have a WIP PR [1] for handling cloud creds (aws and openstack) and also have stubbed out something for libvirt creds. The cloud-creds secret is generated in tectonic svc, not bootkube svc. After you rebase this PR, we can see if it makes sense to include the libvirt creds secret generation with this cloud-creds secret PR er not. [1] #427

crawford · 2018-12-03T23:47:34Z

This looks abandoned so I'm going to close this out. @bison feel free to reopen this after you've rebased and incorporated the functionality in #427.

/close

openshift-ci-robot · 2018-12-03T23:47:35Z

@crawford: Closed this PR.

In response to this:

This looks abandoned so I'm going to close this out. @bison feel free to reopen this after you've rebased and incorporated the functionality in #427.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot requested review from aaronlevy and smarterclayton September 20, 2018 20:25

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 20, 2018

bison mentioned this pull request Sep 20, 2018

Add volume mount for libvirt TLS assets openshift/machine-api-operator#66

Closed

steveej mentioned this pull request Sep 20, 2018

docs/libvirt-howto: add faq & troubleshooting #297

Merged

wking reviewed Sep 20, 2018

View reviewed changes

mrogers950 reviewed Sep 25, 2018

View reviewed changes

openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 26, 2018

bison mentioned this pull request Sep 27, 2018

add terraform to provision packet machine openshift/cluster-api-provider-libvirt#22

Merged

bison added 2 commits September 27, 2018 13:39

hack: Add libvirt-ca script

6efb823

Adds a script to help with one-time generation of TLS asserts for libvirt.

steps/assets: Render libvirt-certs secret

7dba3b1

Renders a secret containing pre-generated libvirt TLS credentials to be used by the cluster-api components.

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 27, 2018

bison added 3 commits September 27, 2018 14:12

installer/config-generator: Generate libvirt URI from network

ae7a5de

Generates the libvirt URI used by the cluster-api based on the cluster network. This lets the installer talk to libvirt over the local socket, while the cluster-api uses a TLS connection.

modules/bootkube: Bump machine-api-operator image

15194a3

Bumps the machine-api-operator image to include support for mounting the libvirt credentials secret.

Documentation: Update libvirt-howto to include TLS auth

f8316ad

Updates the documentation for the libvirt development setup to include TLS authentication between the cluster-api and the host libvirtd.

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 30, 2018

wking mentioned this pull request Oct 7, 2018

add cloud-creds-secret #427

Merged

openshift-ci-robot closed this Dec 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use TLS authentication for cluster-api libvirt connection #296

Use TLS authentication for cluster-api libvirt connection #296

bison commented Sep 20, 2018

openshift-ci-robot commented Sep 20, 2018

bison commented Sep 20, 2018

wking left a comment

wking Sep 20, 2018

bison Sep 26, 2018

wking Sep 20, 2018

bison Sep 25, 2018

wking Sep 20, 2018

bison Sep 25, 2018

wking Sep 20, 2018

bison Sep 25, 2018

bison Sep 26, 2018

mrogers950 Sep 25, 2018

bison Sep 26, 2018

bison commented Sep 27, 2018

bison commented Sep 27, 2018

crawford commented Sep 27, 2018

openshift-bot commented Sep 30, 2018

sallyom commented Oct 8, 2018

crawford commented Dec 3, 2018

openshift-ci-robot commented Dec 3, 2018

Use TLS authentication for cluster-api libvirt connection #296

Use TLS authentication for cluster-api libvirt connection #296

Conversation

bison commented Sep 20, 2018

openshift-ci-robot commented Sep 20, 2018

bison commented Sep 20, 2018

wking left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bison commented Sep 27, 2018

bison commented Sep 27, 2018

crawford commented Sep 27, 2018

openshift-bot commented Sep 30, 2018

sallyom commented Oct 8, 2018

crawford commented Dec 3, 2018

openshift-ci-robot commented Dec 3, 2018