search

databricks / click

The "Command Line Interactive Controller for Kubernetes"
Apache License 2.0
1.49k stars 84 forks source link

[Bug] Click print "Private key data was invalid" #33

Closed Hevienz closed 2 years ago

Hevienz commented 6 years ago

Click print "Private key data was invalid", but this private key can be used by kubectl and helm.

farcaller commented 6 years ago

Same problem here.

[none] [none] [none] > context kubernetes-admin@kubernetes Private key data was invalid: ()

This is a vanilla kubeconfig generated by kubeadm.

farcaller commented 6 years ago

Specifically, RSAKeyPair::from_der fails in the step 5.i. Can't debug further right now as lldb triggers a hilarious kernel memory leak and osx dies 🤦‍♂️

farcaller commented 6 years ago

Digging deeper, this starts to look like a ring problem to me (maybe related to https://github.com/briansmith/ring/issues/635 ?).

try!(q.verify_less_than(&p)) fails for me (LIMBS_less_than(q, p) returns 0). My pk has q<p though (verified with pycrypto).

farcaller commented 6 years ago

Got sidetracked. It is a ring "feature".

                // XXX: |p < q| is actually OK, it seems, but our implementation
                // of CRT-based moduluar exponentiation used requires that
                // |q > p|. (|p == q| is just wrong.)

kubeadm generated keys have p < q (I looked into a wrong key in the previous comment)

nicklan commented 6 years ago

Ahh yes! We ran into this at some point a long time ago and then moved away from those kinds of keys.

There is a general problem here that the Rust ecosystem has much stricter requirements around keys/certs than most other languages. WebPKI also rejects a number of certs that other languages (i.e. go/python) accept happily.

Trying to fix everything in Click is probably infeasible, but I will be pushing on upstream authors and looking at ways to improve this situation.

For this case, there is a PR out against ring, and once it merges we can update Click's dependencies and you should be good to go. I'll leave this open to remind me to update ring asap.

thedeeno commented 6 years ago

@farcaller The above support just got merged into ring (I referenced the issue). Once they release it and we update the dependency here do you expect this to be fixed?

farcaller commented 6 years ago

I suppose that it will work, but I suggest to wait and see.

sibrahim001982 commented 6 years ago

Still getting the same error.

Any updates on this?

sibrahim001982 commented 6 years ago

This is not only with minikube, same error with kubeadm-bootstraped clusters.