Add kubernetes remote debugging capability #3844
Conversation
naiming-zededa
commented
Apr 6, 2024
•
naiming-zededa
commented
- new edgeview tcp/kube command to handle remote kubernetes kubectl, virtctl operations
- user 'debugging-user' is has RBAC of read-only: 'get', 'list' and 'watch', in user.yaml
- the user.yaml file is copied onto user's laptop in /tmp/download/kube-config-yaml
- edgeview handles the 'kubectl' and 'virtctl' proxy access with read-only operations to remote device kubernetes
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3844 +/- ##
=======================================
Coverage 17.51% 17.51%
=======================================
Files 3 3
Lines 805 805
=======================================
Hits 141 141
Misses 629 629
Partials 35 35 ☔ View full report in Codecov by Sentry. |
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
dad070d
to
4eadafc
Compare
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
4eadafc
to
1342c84
Compare
pkg/edgeview/src/policy.go
Outdated
| if subnet1.Contains(ipa) { | ||
| return true | ||
| } else if subnet2.Contains(ipa) { | ||
| return true | ||
| } | ||
| return false |
There was a problem hiding this comment.
| if subnet1.Contains(ipa) { | |
| return true | |
| } else if subnet2.Contains(ipa) { | |
| return true | |
| } | |
| return false | |
| return subnet1.Contains(ipa) || subnet2.Contains(ipa) |
pkg/edgeview/src/tcp.go
Outdated
| // read content of the encrypted config file | ||
| encryptedData, err := os.ReadFile(tmpCfgFileEnc) | ||
| if err != nil { | ||
| _ = os.Remove(tmpCfgFile) |
There was a problem hiding this comment.
please don't _ errors
There was a problem hiding this comment.
ok, let me handle this.
| @@ -91,6 +91,16 @@ github.com/vishvananda/netns | |||
| # github.com/yusufpapurcu/wmi v1.2.2 | |||
| ## explicit; go 1.16 | |||
| github.com/yusufpapurcu/wmi | |||
| # golang.org/x/crypto v0.14.0 | |||
| ## explicit; go 1.17 | |||
There was a problem hiding this comment.
It doesn't work with go version 1.21?
There was a problem hiding this comment.
didn't try, let me try that.
There was a problem hiding this comment.
well, even i checkout the latest of golang.org/x/crypto, which is v1.22.0, the go is v1.18 after the 'go mod vendor'
pkg/edgeview/src/basics.go
Outdated
| symmetricEncFile="/tmp/download/kube-config-yaml" | ||
|
|
||
| # Read the encrypted symmetric key from file | ||
| encryptedSymKey=$(cat "$symmetricKeyEncFile") |
There was a problem hiding this comment.
Where is encryptedSymKey used?
There was a problem hiding this comment.
maybe left over from change, let me remove.
|
@christoph-zededa are you suggesting not to use 'bash'? |
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
9cee1f0
to
214ed48
Compare
|
I have updated per review comments, except for the go 1.17 thing in the module.txt, since even i tried to update the crypto to the latest, go only moved to 1.18. |
As far as I understand this code saves a shellscript in a golang variable (which makes it hidden from yetus), then writes this to a file and executes it - how often is this function called? I am not sure if we want to be dependent on bash as it is under the GPL. |
i'll use '/bin/sh' instead |
Signed-off-by: Naiming Shen <naiming@Admins-MacBook-Pro-3.local>
Signed-off-by: Naiming Shen <naiming@Admins-MacBook-Pro-3.local>
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
214ed48
to
cc17de6
Compare
Is that it? I thought we were using the specific openssl aes256 format because something we were consuming already produced it that way, or something we are producing to already expects that format. If we are both producing and consuming, why would we use openssl at all? Why wouldn't we just do it all natively in go? |
@shjala @deitch , we are not producing and consuming on the same system in this case. What the workflow is : this file.enc then being transferred to user's laptop, and this 'script' is just a way to let user on laptop to run it, which will automatically using this file.enc and spits out the kubeconfig on the stdout, and this 'script' on user's laptop uses the openssl to decrypt this file.enc with it's SSH private key. |
i took a brief look at this, and at least in the example it's given a 'passphrase' on both sides, do the encrypt/decrypt from that to the text. I think the reason i could not make this work with crypto is that I have a different input on both sides, the encrypt side I have the public key info, and on the decrypt side i have the private key, i doubt this can be handle in this repo. |
Before Milan pointed this out (more likely, after he did but before I realized; on me), I had started an implementation that provides the same thing. It was getting fairly similar and close to what is in that repo. If we decide we need it, I will complete it and hand it over to you. First, let's decide if we will do it at all. |
I got that, but I still think the decryption should be part of the CLI with private key passed as argument, not a fan of dropping shell script to the user system and asking them to execute it. |
Sort of. The purpose of all of this is to get the user a kubeconfig they can use. Someone with a kubeconfig should be able to use it in their own way, whether secured or not, with whatever tools they are used to. Once we provide them with the kubeconfig, our own CLI should be out of the picture. For that matter, I think openssl also should be out of the picture, unless the user normally uses openssl to encrypt/decrypt their kubeconfig. But that would be independent of our flow. Another user might use cfssl, another might use gpg, another an encrypted partition, another a home-grown solution. Either way, from a user perspective (and simplicity of engineering), I think we should get out of the business entirely. "You want access? You need a kubeconfig. Here it is. Have fun. Use it, protect it, stick with whatever flows you normally use." |
|
I'll take the action to remove this encrypt/decrypt, and more confirming to the existing industry practice, and also this kubeConfig is only limited to 'debugginguser' with get/list capabilities only. I'll submit the changes later. |
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
cc17de6
to
53e7f4d
Compare
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
53e7f4d
to
8c7aa49
Compare
|
I have updated this PR without the openssl encrypt/decrypt, simpler solution now. |
pkg/edgeview/src/policy.go
Outdated
| return false | ||
| } | ||
|
|
||
| _, subnet1, err := net.ParseCIDR("10.42.0.0/16") |
There was a problem hiding this comment.
Are these fixed address ranges (same for line 263)? Should these be hard-coded in here?
There was a problem hiding this comment.
those are the default of k3s setup. we don't change that. see the link: https://docs.k3s.io/cli/server in the 'Networking' section of defaults. but for 10.42.0.0/16, I can actually modify this to check the interface prefix of 'cni0' and get from there. for '10.43.0.0/16', we just have to define it here, there is nothing i can gather from the system easily.
pkg/edgeview/src/tcp.go
Outdated
| @@ -75,6 +77,11 @@ const ( | |||
| tcpMaxMappingNUM int = 5 | |||
| tcpIdleTimeoutSec float64 = 1800.0 | |||
| tcpCheckTimeout time.Duration = 300 * time.Second | |||
| tcpKubeEndpoint string = "localhost:6443" | |||
There was a problem hiding this comment.
Are these - localhost and port 6443 - defined elsewhere? Or are we assuming them? We must know them when we deploy the cluster, I assume there is a kubeconfig? Can we inherit them from somewhere rather than hard-coding?
There was a problem hiding this comment.
yes, it's the k3s default setup. although I can change to deduce this from the kubeconfig file we have.
| @@ -75,6 +77,11 @@ const ( | |||
| tcpMaxMappingNUM int = 5 | |||
| tcpIdleTimeoutSec float64 = 1800.0 | |||
| tcpCheckTimeout time.Duration = 300 * time.Second | |||
| tcpKubeEndpoint string = "localhost:6443" | |||
| kubeYamlFile string = "/run/.kube/k3s/user.yaml" | |||
There was a problem hiding this comment.
Can you base it on:
./kubeapi/kubeapi.go: EVEkubeConfigFile = "/run/.kube/k3s/k3s.yaml"
?
This would make it easier to change it from /run/.kube to /run/kube in case we find out that it is inconvenient when debugging to have hidden directories in /run.
There was a problem hiding this comment.
this directory mainly have some secret in the config files, since other PRs also uses this way. We can change later if we all agree.
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
8c7aa49
to
f0cf5b7
Compare
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
f0cf5b7
to
4b9a912
Compare
Signed-off-by: Naiming Shen <naiming@Admins-MacBook-Pro-3.local> Signed-off-by: Naiming Shen <naiming@admins-mbp-3.lan> Signed-off-by: Naiming Shen <naiming@Admins-MacBook-Pro-3.local>
naiming-zededa
force-pushed
the
naiming-ev-kube
branch
from
4b9a912
to
b774a58
Compare
