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

kubeadm swallows errors when CA key isn't in RSA format #1210

Closed
juicemia opened this issue Nov 3, 2018 · 3 comments · Fixed by kubernetes/kubernetes#70611
Closed

kubeadm swallows errors when CA key isn't in RSA format #1210

juicemia opened this issue Nov 3, 2018 · 3 comments · Fixed by kubernetes/kubernetes#70611
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@juicemia
Copy link

juicemia commented Nov 3, 2018

What happened: When trying to deploy Kubernetes using kubeadm v1.12.1, kubeadm failed to generate certificates, causing it to fail at a later step.

After passing -v10 to get all the logs I could I found that it was expecting all the certificates to have been pre-made, even though I only added the certificate and key for the certificate authority. So I dug into the code, and in pkiutil.TryLoadKeyFromDisk I found this:

	// Allow RSA format only
	var key *rsa.PrivateKey
	switch k := privKey.(type) {
	case *rsa.PrivateKey:
		key = k
	default:
		return nil, fmt.Errorf("the private key file %s isn't in RSA format", privateKeyPath)
	}

It turns out all the PKI I've been generating for things like etcd has been ECDSA. So I found the issue I was having, but I still couldn't figure out why kubeadm wasn't failing right away. I took a look at the caller of that function:

			// Try and load a CA Key
			caKey, err = pkiutil.TryLoadKeyFromDisk(ic.CertificatesDir, ca.BaseName)
			if err != nil {
				// If there's no CA key, make sure every certificate exists.
				for _, leaf := range leaves {
					cl := certKeyLocation{
						pkiDir:   ic.CertificatesDir,
						baseName: leaf.BaseName,
						uxName:   leaf.Name,
					}
					if err := validateSignedCertWithCA(cl, caCert); err != nil {
						return fmt.Errorf("could not load expected certificate %q or validate the existence of key %q for it: %v", leaf.Name, ca.Name, err)
					}
				}
				// CACert exists and all clients exist, continue to next CA.
				continue
			}

It looks like that original error is swallowed.

What you expected to happen: Immediate failure when keys are in the wrong format.

How to reproduce it (as minimally and precisely as possible): Use an ECDSA cert/key pair and run kubeadm init.

Anything else we need to know?:

Environment:

  • Kubernetes version (use kubectl version): v1.12.1
  • Cloud provider or hardware configuration: OpenStack
  • OS (e.g. from /etc/os-release): Ubuntu 18.04
  • Kernel (e.g. uname -a): 4.15.0-34-generic discuss the meaning of kubeadm init --api-advertise-addresses #37-Ubuntu SMP Mon Aug 27 15:21:48 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Install tools: kubeadm
  • Others:

/kind bug

@yagonobre
Copy link
Member

yagonobre commented Nov 3, 2018

@juicemia thanks for create the issue on the right repo, can you close the kubernetes/kubernetes#70594?

I'm working on a fix.

@yagonobre
Copy link
Member

yagonobre commented Nov 3, 2018

This was fixed on kubernetes/kubernetes#70331, but due to a wrong error wrap on pkihelper if you try to create a cluster with invalid ca it will result in a panic.
/priority important-soon

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Nov 3, 2018
@neolit123
Copy link
Member

the fix from @yagonobre seems good.

ECDSA isn't supported by kubeadm yet, here is the tracking issue:
#984

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants