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

Question regarding used port #106

Closed
SimonBosse opened this issue Aug 15, 2022 · 11 comments · Fixed by #109
Closed

Question regarding used port #106

SimonBosse opened this issue Aug 15, 2022 · 11 comments · Fixed by #109

Comments

@SimonBosse
Copy link

Hi,

I am using cri-dockerd as CRI for kubernetes. I have set --container-runtime=remote --container-runtime-endpoint=unix:///var/run/cri-dockerd.sock as flags for kubelet. I have noticed that there is a port (>30000) used by cri-dockerd which is bound to 0.0.0.0 and I am asking me for what this port is used and if there is a possibility to bind it to localhost.

Thanks for you answer

@afbjorklund
Copy link
Contributor

afbjorklund commented Aug 16, 2022

This is the streaming server that was added, it is binding to an empty "tcp" address (?) for some reason.

https://github.com/Mirantis/cri-dockerd/blob/v0.2.5/streaming/server.go#L239

Originally, it came from here:


But there are not many details.

Taken from here: https://github.com/kubernetes/kubernetes/blob/v1.23.9/pkg/kubelet/cri/streaming/server.go

@afbjorklund

This comment was marked as outdated.

@SimonBosse
Copy link
Author

Thanks for the information. Does this mean that if I want the streaming server to bind to another address I have to change the source code and then compile it?

@afbjorklund

This comment was marked as outdated.

@evol262
Copy link
Contributor

evol262 commented Aug 17, 2022

This is the streaming server that was added, it is binding to an empty "tcp" address (?) for some reason.

To give a little more information, the streaming server uses GRPC (streaming) for some portforwarding, kubectl exec ... instead of round-tripping over HTTP1.1

But there are not many details.

That commit was mostly "refactor", in the "don't import massive upstream k8s modules to get some consts" and "don't require the entire k8s klog logging implementation", etc. It could have been a better commit message, just that there was a giant ton of things to rip out.

I have noticed that there is a port (>30000) used by cri-dockerd which is bound to 0.0.0.0 and I am asking me for what this port is used and if there is a possibility to bind it to localhost.

0.0.0.0 listens to all addresses on all interfaces, including localhost (similarly, :: listens to all addresses including ipv6). Localhost should be included in this.

It seems to be missing this line, during the copy/paste:

        config.Addr = net.JoinHostPort("localhost", "0")

This is actually somewhat different, and it will only listen on localhost then. Docker doesn't really support the "streaming" part of the CRI spec anyway (the Docker API handles it somewhat differently), but explicitly binding this to localhost rather than 0.0.0.0 || :: will be a slight behavior change which would have side effects on any GPRC client which is trying to reach across network namespaces or other.

If this is changed, it should still default to "all interfaces", and should be a slice of addresses to listen to instead.

Prior to that, though, @SimonBosse is it actually not listening to localhost, or is this curiosity? 0.0.0.0 || :: will absolutely bind to lo, 127.0.0.1 (which is always on lo), and any other address your machine has.

@evol262
Copy link
Contributor

evol262 commented Aug 17, 2022

For historical background, the implementation in the upstream codebase actually set this by calling a non-exported method. The initial handover from upstream was great, but a little raw, so we're implementing what's public from the CRI spec. At the time, listening to "all interfaces" seemed like an ok tradeoff in being a little less rigid, but it's unclear at the moment whether anyone's actually using that functionality. Hyrum's Law says that someone probably is, but we wouldn't know until it was changed.

@afbjorklund
Copy link
Contributor

afbjorklund commented Aug 17, 2022

Mine was listening to all interfaces (*), including v6 (and localhost). Thanks for the clarification, it should be alright then ?

@evol262
Copy link
Contributor

evol262 commented Aug 17, 2022

Mine was listening to all interfaces (*), including v6 (and localhost). Thanks for the clarification, it should be alright then ?

That's my question for OP ;) It should be alright in the sense that all interfaces will also include localhost, so there's no need to explicit rebind there.

But "listen to all addresses" is a safety valve which not everyone may want. Just from a basic server hardening perspective, may as well make which addresses configurable as part of #102, and change the default to localhost then, with a way to easily change it which is not "recompile"

@SimonBosse
Copy link
Author

Hardening was the reason why my question came up. My intention was to find a possibility to only bind it to localhost for security reasons.

@SimonBosse
Copy link
Author

SimonBosse commented Aug 17, 2022

If I understand it right, there is no possibility to bind only to localhost at the moment without changing the source code, is this right?

@evol262
Copy link
Contributor

evol262 commented Aug 17, 2022

Well, I wasn't planning on releasing a 0.2.6, but #105 absolutely needs a fix. As long as that's happening, this can also be added as a flag (single address for now)

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 a pull request may close this issue.

3 participants