Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the CA certificate from bundle #111

Merged
merged 6 commits into from
Dec 2, 2021

Conversation

rgl
Copy link
Contributor

@rgl rgl commented Oct 14, 2021

Description

See #105

I also took the opportunity to:

  1. use a different csr for the tinkerbell ca (this allows us to clearly identify the ca and server certificates)
  2. switch from rsa to ecdsa (its equally safer and its generally faster)
  3. modify tls-gen to also copy the ca certificate to the workflow directory (this simplifies the compose file)
  4. Configure the vagrant environment to trust the tinkerbell CA.
  5. Configure the vagrant environment root and vagrant accounts to automatically login into the registry.

Why is this needed

See #105

Fixes: #105

How Has This Been Tested?

Tested locally by starting Tinkerbell with compose and vagrant.

How are existing users impacted? What migration steps/scripts do we need?

Should not be impacted.

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

@rgl rgl force-pushed the rgl-remove-ca-from-crt-bundle branch 4 times, most recently from 939f492 to 7826b82 Compare October 16, 2021 06:13
@jacobweinstock
Copy link
Member

hmmm....I wasn't able to get this PR working. Let me get some notes together for what I ran into.

@rgl rgl force-pushed the rgl-remove-ca-from-crt-bundle branch from 7826b82 to f6755e9 Compare October 19, 2021 18:00
@rgl
Copy link
Contributor Author

rgl commented Oct 19, 2021

Oh, the last commit had broken the build. It should work now. Can you please re-run it?

@jacobweinstock
Copy link
Member

Hey @rgl, thanks for the update. vagrant up is now working for me. I apologize but I'm not fully understanding what this PR or #105 are trying to resolve.

This branch and the existing mainline branch both behave the same for me in terms of pushing to the local container registry and curl https://192.168.50.4/v2/_catalog.

I get the following output from both this PR and main.

vagrant@ubuntu2004:~$ docker tag hello-world 192.168.50.4/hello-world
vagrant@ubuntu2004:~$ docker push 192.168.50.4/hello-world
Using default tag: latest
The push refers to repository [192.168.50.4/hello-world]
Get "https://192.168.50.4/v2/": x509: certificate signed by unknown authority
---
vagrant@ubuntu2004:~$ curl https://192.168.50.4/v2/_catalog
curl: (60) SSL certificate problem: unable to get local issuer certificate
More details here: https://curl.haxx.se/docs/sslcerts.html

curl failed to verify the legitimacy of the server and therefore could not
establish a secure connection to it. To learn more about this situation and
how to fix it, please visit the web page mentioned above.
---
vagrant@ubuntu2004:~$ curl https://192.168.50.4/v2/_catalog -k
{"errors":[{"code":"UNAUTHORIZED","message":"authentication required","detail":[{"Type":"registry","Class":"","Name":"catalog","Action":"*"}]}]}
---

Also, for this PR the initial vagrant up is successful but shows this:

==> provisioner: Running provisioner: Trust the Tinkerbell CA (shell)...
    provisioner: Running: script: Trust the Tinkerbell CA
    provisioner: /tmp/vagrant-shell: line 2: docker-compose: command not found
    provisioner: Updating certificates in /etc/ssl/certs...
    provisioner: 0 added, 0 removed; done.
    provisioner: Running hooks in /etc/ca-certificates/update.d...
    provisioner: done.

@rgl rgl force-pushed the rgl-remove-ca-from-crt-bundle branch from f6755e9 to 748246e Compare October 21, 2021 11:07
@rgl
Copy link
Contributor Author

rgl commented Oct 21, 2021

@jacobweinstock, I've now finally actually used sandbox for the first time, and was able to fix it.

This now works:

$ vagrant ssh provisioner

$ docker tag bash:4.4 192.168.50.4/hello-world

$ docker push 192.168.50.4/hello-world
Using default tag: latest
The push refers to repository [192.168.50.4/hello-world]
d0c293cc0edd: Pushed 
9f708c6c10e5: Pushed 
e2eb06d8af82: Pushed 
latest: digest: sha256:f290434b66bde9a4155f882710dab774c557acca4b3faccad7c53327afaebd1a size: 946

$ curl https://192.168.50.4/v2/_catalog
{"errors":[{"code":"UNAUTHORIZED","message":"authentication required","detail":[{"Type":"registry","Class":"","Name":"catalog","Action":"*"}]}]}

Let me known if it works for you.

@jacobweinstock
Copy link
Member

Hey @rgl, thanks! this is working for me. Would it be possible to move this setup out of the Vagrantfile so that other infrastructure providers like Terraform can consume it? Finding a way to allow docker-compose only consumers to use this would also be ideal. Maybe a new directory for host scripts? Just a brainstorming idea.

I apologize that it's not well documented but the design philosophy here is:

"Vagrant and Terraform are now only responsible for standing up infrastructure and then running docker-compose, not for running any glue scripts. The compose calls single-shot services to do all the glue required to get a fully functional Tinkerbell stack." ref: here

@rgl
Copy link
Contributor Author

rgl commented Oct 29, 2021

Hey @rgl, thanks! this is working for me. Would it be possible to move this setup out of the Vagrantfile so that other infrastructure providers like Terraform can consume it? Finding a way to allow docker-compose only consumers to use this would also be ideal. Maybe a new directory for host scripts? Just a brainstorming idea.

You mean to move it to a script file at deploy/host/trust-tinkerbell-ca-and-restart-docker.sh?

Note that the script will modify the host configuration using sudo, and it assumes the host is running Ubuntu and the systemd docker service exists. If that's fine, I can do that.

I apologize that it's not well documented but the design philosophy here is:

"Vagrant and Terraform are now only responsible for standing up infrastructure and then running docker-compose, not for running any glue scripts. The compose calls single-shot services to do all the glue required to get a fully functional Tinkerbell stack." ref: here

Can docker-compose.yml run scripts in the host?

@rgl
Copy link
Contributor Author

rgl commented Nov 4, 2021

@jacobweinstock, ping? :-)

@jacobweinstock
Copy link
Member

jacobweinstock commented Nov 16, 2021

Hey @rgl, really sorry for the delayed response.

You mean to move it to a script file at deploy/host/trust-tinkerbell-ca-and-restart-docker.sh?
That's an option. What do you think? The design idea for this repo is to keep concise as possible layers between what handles

  • infrastructure stand up
    • vagrant, terraform, etc
  • Tinkerbell stand up
    • the glue required to get a fully functional Tinkerbell stack

I don't know if there's a clean way to handle this. The code added to the Vagrantfile isnt portable to Terraform right now. So Terraform users arent able to take advantage of this TLS update.

a server certificate must not include its CA certificate.

the CA certificate must already be installed/trusted by the clients.

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
this allows us to clearly identify the ca and server certificates

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
its equally safer and its generally faster

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
this simplifies the compose file

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
@rgl rgl force-pushed the rgl-remove-ca-from-crt-bundle branch 2 times, most recently from 234c0dc to 0b2d4c1 Compare November 18, 2021 07:26
Signed-off-by: Rui Lopes <rgl@ruilopes.com>
@rgl rgl force-pushed the rgl-remove-ca-from-crt-bundle branch from 0b2d4c1 to 78c7f94 Compare November 18, 2021 07:36
@rgl
Copy link
Contributor Author

rgl commented Nov 18, 2021

@jacobweinstock, I've refactored the code into deploy/compose/tls/trust.sh which is now shared between Vagrant and Terraform.

Please check it again.

Copy link
Member

@jacobweinstock jacobweinstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience and pushing this through!

@jacobweinstock jacobweinstock merged commit 1e0157f into tinkerbell:main Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TLS server certificate must not contain the CA certificate
2 participants