Closed seeker89 closed 3 years ago
Hi @seeker89
Thats a really good addition, thanks! Ill take a deeper look at this in the next few days, and update you.
I think we will benefit alot from discovering nodes from the api server
Hi @seeker89
Thats a really good addition, thanks! Ill take a deeper look at this in the next few days, and update you.
I think we will benefit alot from discovering nodes from the api server
Thank you @danielsagi - looking forward to a review.
@danielsagi let me know if you'd like any more info!
Happy Monday! Can I help anyhow to make the review easier? @danielsagi Thanks in advance!
Hi @seeker89, Thanks for your patience. I did not have time to sit on this yet, I'm really sorry for the delay, Ill keep you posted this week!
Ping :) @danielsagi
Hi @seeker89 Really sorry for taking too long for this, But I'm with you now. Thank you for your patience! First of all I'll say, I love that you used the kubernetes package I think we should have done that a long time ago (of course until now we needed to do magic api stuff manually, but still).
I have 4 comments:
Right now your implementation would not run with --pod
because of this condition
In order to run when pod scan is enabled, please add that inside FromPodHostDiscovery
inside /kube_hunter/modules/discovery/hosts.py
as well. I also would like for your feature to run automatically on PodScans!, (using the load_incluster_config
) and so the check there should only be for the kubeconfig flag.
because of the minor complexity of this action (mainly dependencies for the current env) I would not implement this under the conf
module, but rather directly inside hosts.py
. (I know it's a huge file, but in the future we plan to split it :) )
I would add more explanation on the help of both arguments, including a reference on --k8s-auto-discover-nodes
to the kubeconfig argument, and vice versa.
Document this feature on the main README :) (under Scanning Options) provide a detailed explanation on when to use this. and the default behaviour if
(optional) because I see us using the --kubeconfig
flag in the future, You can add a new Event
like UserKubeconfigEvent
containing the config loaded object. This one is really optional
Again, I thank you for your time, hope you haven't lost motivation ;) this really is a wonderful feature. I'll give this one a boost.
On a second thought, Regarding my 2nd point. I think this deserves to be in a new kubernetes_client.py
file under kube_hunter/modules/discovery
.
Wdyt?
Hi @seeker89 Really sorry for taking too long for this, But I'm with you now. Thank you for your patience! First of all I'll say, I love that you used the kubernetes package I think we should have done that a long time ago (of course until now we needed to do magic api stuff manually, but still).
I have 4 comments:
- Right now your implementation would not run with
--pod
because of this condition In order to run when pod scan is enabled, please add that insideFromPodHostDiscovery
inside/kube_hunter/modules/discovery/hosts.py
as well. I also would like for your feature to run automatically on PodScans!, (using theload_incluster_config
) and so the check there should only be for the kubeconfig flag.- because of the minor complexity of this action (mainly dependencies for the current env) I would not implement this under the
conf
module, but rather directly insidehosts.py
. (I know it's a huge file, but in the future we plan to split it :) )- I would add more explanation on the help of both arguments, including a reference on
--k8s-auto-discover-nodes
to the kubeconfig argument, and vice versa.- Document this feature on the main README :) (under Scanning Options) provide a detailed explanation on when to use this. and the default behaviour if
- (optional) because I see us using the
--kubeconfig
flag in the future, You can add a newEvent
likeUserKubeconfigEvent
containing the config loaded object. This one is really optionalAgain, I thank you for your time, hope you haven't lost motivation ;) this really is a wonderful feature. I'll give this one a boost.
On a second thought, Regarding my 2nd point. I think this deserves to be in a new
kubernetes_client.py
file underkube_hunter/modules/discovery
. Wdyt?Hi @seeker89 Really sorry for taking too long for this, But I'm with you now. Thank you for your patience! First of all I'll say, I love that you used the kubernetes package I think we should have done that a long time ago (of course until now we needed to do magic api stuff manually, but still). I have 4 comments:
- Right now your implementation would not run with
--pod
because of this condition In order to run when pod scan is enabled, please add that insideFromPodHostDiscovery
inside/kube_hunter/modules/discovery/hosts.py
as well. I also would like for your feature to run automatically on PodScans!, (using theload_incluster_config
) and so the check there should only be for the kubeconfig flag.- because of the minor complexity of this action (mainly dependencies for the current env) I would not implement this under the
conf
module, but rather directly insidehosts.py
. (I know it's a huge file, but in the future we plan to split it :) )- I would add more explanation on the help of both arguments, including a reference on
--k8s-auto-discover-nodes
to the kubeconfig argument, and vice versa.- Document this feature on the main README :) (under Scanning Options) provide a detailed explanation on when to use this. and the default behaviour if
- (optional) because I see us using the
--kubeconfig
flag in the future, You can add a newEvent
likeUserKubeconfigEvent
containing the config loaded object. This one is really optionalAgain, I thank you for your time, hope you haven't lost motivation ;) this really is a wonderful feature. I'll give this one a boost.
Hi @danielsagi I addressed points 1-4, on point 5, it would just be dead code for now, so I'd vote let's add it when it's needed. Let me know if this is good to go now :)
Happy Friday :)
@seeker89 Just ran my tests, everything looks great! Well probably publish a new release for this feature soon, so keep following :)
Description
This PR adds a new feature to
kube-hunter
to automatically list all nodes in akubernetes
cluster, and attempt hunting on all of their advertised addresses.It adds two new flags:
--k8s-auto-discover-nodes
that works like--remote
or--cidr
, but first queries Kubernetes for all nodes--kubeconfig
(optional), if you'd like to use a kubeconfig file, rather than rely on the in-cluster configurationThis is handy in conjunction with
--pod
, when run from a pod (but it will require RBAC to list nodes).All of the discovery logic for different clouds remains unaffected. This is useful when you run your own cluster, and the advertised IPs are routable.
Contribution Guidelines
Please Read through the Contribution Guidelines.
"BEFORE" and "AFTER" output
BEFORE
Feature not available.
AFTER
Contribution checklist
The commits refer to an active issue in the repository.Notes
This is an enhancement, so there was no active issue to reference. If you're happy to go forward with this, I'll add a test for the new function I introduced.