Allow OpenStack SSL certs + keys to take path or content

[ljfranklin]

The goal of this PR is to allow the existing OpenStack provider fields cacert_file, cert, and key to take either a path to the file (existing behavior) or the contents of the file itself (new functionality). This will make it easier to interact with Terraform in CI via environment variables.

[ljfranklin]

Before I dive into this, does this seem like a reasonable change to make? Seems like it would only require changing a couple lines here and adding some tests.

[jtopjian]

That's a good question. I know other fields, such as SSH keys and such, have been modified over time to take the contents of a file rather than a file path, so this would align with those changes.

Is there precedence elsewhere in Terraform for using SSL cert contents?

[ljfranklin]

@jtopjian the aws_iam_server_certificate resource only takes the body and suggests you use the file() helper to read from disk if you want. Same with the private_key field in the provisioner. Seems like always taking the contents of a file is the most flexible as you can fall back to using the file() helper if needed.

Given there's a bit of precedence but we still need to maintain backwards compatibility, sounds like I can move ahead with updating the OpenStack provider to take either the contents of the SSL certs or a path to the cert. Please let me know if that doesn't seem right.

[ljfranklin]

@jtopjian was hoping to get a little guidance on writing acceptance tests in Terraform. I'm trying to write a few tests against the OpenStack provider directly. For example, given a provider option cacert_file which contains a filepath, I should be able to configure the provider (TestProvider_caCertFile). While the tests shown in this PR pass, they fail in Travis as the acceptance test environment variables are not present.

Is there a way to mark a test as an acceptance test? I tried to create a test using resource.TestCase, but this harness didn't seem to lend itself to modifying the provider options on the fly for each test case, e.g. setting provider's cacert_file to a filepath in one test and the string contents in the next. Appreciate the help!

[jtopjian]

@ljfranklin Good question. I did a cursory look but I'm not entirely sure. I'm going to ping @stack72 or @mitchellh and see if they know off-hand rather than us spin our wheels :)

[ljfranklin]

@jtopjian @stack72 @mitchellh gentle bump on the above testing framework question :)

[ljfranklin]

@jtopjian I ended up just skipping the provider tests that need to hit the OpenStack API unless TF_ACC was set, let me know if there's a better solution. I think this is ready for a review now, let me know if something looks amiss!

[jtopjian]

@ljfranklin Sorry for the late reply, I've been traveling for work.

This all sounds good to me. I want to set up a dev environment to verify the acceptance tests, though. I'll try to get to that within the next week.

[jtopjian]

@ljfranklin I had some time to test this out today.

I had to review #6279 to remember how to do it. I configured a test OpenStack environment to use the demo certificates that can be found here: https://github.com/OpenVPN/openvpn/tree/master/sample/sample-keys

Once I did that, I was able to compile a terraform binary based on this PR and successfully ran some acceptance tests. So that's good news!

However, I ran into problems with the acceptance tests that you made, using the certs that you included:

[root@packstack-standard openstack(keystone_demo)]# TF_ACC=1 go test -v ./ -run=TestAccProvider
=== RUN   TestAccProvider_caCertFile
--- FAIL: TestAccProvider_caCertFile (0.11s)
        provider_test.go:105: Expected error to contain 'x509', but it did not: Post https://example.com:5000/v3/auth/tokens: remote error: tls: unknown certificate authority
=== RUN   TestAccProvider_caCertString
--- FAIL: TestAccProvider_caCertString (0.06s)
        provider_test.go:133: Expected error to contain 'x509', but it did not: Post https://example.com:5000/v3/auth/tokens: remote error: tls: unknown certificate authority
=== RUN   TestAccProvider_clientCertFile
--- FAIL: TestAccProvider_clientCertFile (0.06s)
        provider_test.go:172: Unexpected err when specifying OpenStack Client keypair by file: Post https://example.com:5000/v3/auth/tokens: x509: certificate signed by unknown authority
=== RUN   TestAccProvider_clientCertString
--- FAIL: TestAccProvider_clientCertString (0.03s)
        provider_test.go:206: Unexpected err when specifying OpenStack Client keypair by contents: Post https://example.com:5000/v3/auth/tokens: x509: certificate signed by unknown authority
FAIL

I tried using the certs obtained from the link above, but since the tests are not checking the CA file, they still fail.

Perhaps I ran something incorrectly?

Here's what I did to set up my test environment:

Using the cert link above, I saved the following files locally: server.key, server.crt, ca.crt, client.crt, and client.key.

I then configured apache like so:

<VirtualHost *:35357>
  ServerName packstack-standard.cloud.cybera.ca

  ## Vhost docroot
  DocumentRoot "/var/www/cgi-bin/keystone"

  SSLEngine on
  SSLVerifyDepth 1
  SSLCertificateFile  /etc/ssl/certs/server.crt
  SSLCertificateKeyFile   /etc/ssl/certs/server.key
  SSLCACertificateFile /etc/ssl/certs/ca.crt
  SSLVerifyClient require

  ## Directories, there should at least be a declaration for /var/www/cgi-bin/keystone

  <Directory "/var/www/cgi-bin/keystone">
    Options Indexes FollowSymLinks MultiViews
    AllowOverride None
    Require all granted
  </Directory>

  ## Logging
  ErrorLog "/var/log/httpd/keystone_wsgi_admin_error.log"
  ServerSignature Off
  CustomLog "/var/log/httpd/keystone_wsgi_admin_access.log" combined
  WSGIApplicationGroup %{GLOBAL}
  WSGIDaemonProcess keystone_admin display-name=keystone-admin group=keystone processes=8 threads=8 user=keystone
  WSGIProcessGroup keystone_admin
  WSGIScriptAlias / "/var/www/cgi-bin/keystone/keystone-admin"
  WSGIPassAuthorization On
</VirtualHost>

and

<VirtualHost *:5000>
  ServerName packstack-standard.cloud.cybera.ca

  ## Vhost docroot
  DocumentRoot "/var/www/cgi-bin/keystone"

  SSLEngine on
  SSLVerifyDepth 1
  SSLCertificateFile  /etc/ssl/certs/server.crt
  SSLCertificateKeyFile   /etc/ssl/certs/server.key
  SSLCACertificateFile /etc/ssl/certs/ca.crt
  SSLVerifyClient require


  ## Directories, there should at least be a declaration for /var/www/cgi-bin/keystone

  <Directory "/var/www/cgi-bin/keystone">
    Options Indexes FollowSymLinks MultiViews
    AllowOverride None
    Require all granted
  </Directory>

  ## Logging
  ErrorLog "/var/log/httpd/keystone_wsgi_main_error.log"
  ServerSignature Off
  CustomLog "/var/log/httpd/keystone_wsgi_main_access.log" combined
  WSGIApplicationGroup %{GLOBAL}
  WSGIDaemonProcess keystone_main display-name=keystone-main group=keystone processes=8 threads=8 user=keystone
  WSGIProcessGroup keystone_main
  WSGIScriptAlias / "/var/www/cgi-bin/keystone/keystone-public"
  WSGIPassAuthorization On
</VirtualHost>

The Keystone catalog and all OpenStack components were then updated to use https for the above Keystone services. The server hostname was also modified, etc etc.

On the client side, specifying OS_KEY as client.key and OS_CERT as client.crt, general OpenStack connectivity worked.

Configuring Terraform like so also worked:

provider "openstack" {
  key = "/etc/ssl/certs/client.key"
  cert = "/etc/ssl/certs/client.crt"
  cacert_file = "/etc/ssl/certs/ca.crt"
}

resource "openstack_compute_servergroup_v2" "test" {
  name = "test"
  policies = ["anti-affinity"]
}

With all of the above said, given the complexity required to set up a test environment to run these tests, I think a conditional should be added to the tests in addition to TF_ACC (such as OS_RUN_SSL_TESTS or something).

[ljfranklin]

@jtopjian thanks for fighting through that environment setup! I just pushed an update with the changes below.

I think the "TestAccProvider_caCertString" and "TestAccProvider_caCertFile" tests are failing due to a slightly different error string, possibly OS dependent. On Linux I got "x509: certificate signed by unknown authority" but your error is "remote error: tls: unknown certificate authority". I changed my test to check for "certificate" rather than "x509" to sidestep the issue but I'm open to other ideas that are less brittle.

I added the "OS_CLIENT_SSL_TESTS" variable to skip the client SSL tests as the environment setup is a pain. I left the "TestAccProvider_caCert" tests without this skip as they are more around client-side validation rather than special server-side validation and should pass in a vanilla environment.

For the failing "TestAccProvider_clientCert" tests, I think you just need to set "OS_CACERT" environment variable before running those tests and the provider should pick it up. I didn't require that env variable in the test as you could have an OpenStack with Server certs signed by a legit CA, but use a different private CA to sign your client certs.

[jtopjian]

I think the "TestAccProvider_caCertString" and "TestAccProvider_caCertFile" tests are failing due to a slightly different error string, possibly OS dependent.

Just an aside: my test environment is CentOS.

Would it be better to write the test so that the entire certificate chain passes? This would include a cert, key, and ca file. If any of the three aren't valid certificates, then the test wouldn't pass. I think this would end up being the existing TestAccProvider_clientCertFile with the inclusion of a CA file and the other two tests could be removed. Or is there another reason for the TestAccProvider_caCertFile and TestAccProvider_caCertString tests?

I added the "OS_CLIENT_SSL_TESTS" variable to skip the client SSL tests as the environment setup is a pain. I left the "TestAccProvider_caCert" tests without this skip as they are more around client-side validation rather than special server-side validation and should pass in a vanilla environment.

Thanks! And agreed.

For the failing "TestAccProvider_clientCert" tests, I think you just need to set "OS_CACERT" environment variable before running those tests and the provider should pick it up. I didn't require that env variable in the test as you could have an OpenStack with Server certs signed by a legit CA, but use a different private CA to sign your client certs.

You're right - I was switching back and forth between the demo certs I downloaded and the ones you included and the CA was not set up correctly for the former.

However, with the cert/key that you included, I'm getting certificate validation errors that, I think, a CA file would resolve. Are you able to run the tests with just the cert/key included in this PR?

[ljfranklin]

@jtopjian pushed a couple more changes.

Would it be better to write the test so that the entire certificate chain passes?

I kinda like leaving them as separate tests as they are two separate features. One has the TF CLI verify the cert from the server, the other is a server-side authentication feature. That said, I think the negative test was more trouble than it was worth so I removed the fake certs and require that the user pass the real CA cert.

However, with the cert/key that you included...

The included cert and key were only used for the negative CA tests, so I just removed them. Whatever certs and CA you have nginx configured with should be fine.

[jtopjian]

Thank you for your patience with this!

I think the last modification to make would be to make the two caCert tests run only on the OS_CLIENT_SSL_TESTS conditional. Without that set, it assumes the testing environment is configured both with https endpoints and uses a intermediate CA file.

Then the comment you made here:

// Steps for configuring OpenStack with client SSL validation are here:
// https://github.com/hashicorp/terraform/pull/6279#issuecomment-219020144

Can be moved to the top of the series of tests and explain that these tests are only valid for test environments that are configured with https endpoints which require a valid SSL chain consisting of a cert, key, and intermediate cert.

Does that make sense? I really apologize for, more or less, grilling the testing part of this PR. I just want to make sure that the tests are correct yet only run under the right condition. Neither Packstack nor Devstack (which are used to test the OpenStack support) are able to set up this kind of SSL environment out of the box. :/

[ljfranklin]

@jtopjian no worries, appreciate you sticking with it! Changed the var to "OS_SSL_TESTS" and it is now being checked for all four tests.

[jtopjian]

Looks good! I'll merge this in the next day or so. I'll be offline traveling and I don't want to merge and disappear :)

[jtopjian]

@ljfranklin Thank you for your patience and help with this!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.