search

containers / netavark

Container network stack
Apache License 2.0
512 stars 81 forks source link

DHCP Proxy Does Not Report Option 12 (Hostname) #676

Open apsoyka opened 1 year ago

apsoyka commented 1 year ago

Currently, the DHCP proxy does not report option 12 (client hostname) to the DHCP server when requesting a DHCP lease. This means that routers which function also as a DNS server (most consumer routers), and use that information in order to create an A record pointing to the DHCP client, will not function.

This issue does not affect the core DHCP proxy functionality, but instead represents a quality-of-life issue.

I would be willing to submit a PR for this, but I'm not familiar with the codebase; could someone please point me in the direction of the necessary changes?

Luap99 commented 1 year ago

It should not be that complicated, however as of of today netavark does not know the host name of a container so the first step would be to send the Hostname from podman to netavark. The type for podman is defined here: https://github.com/containers/common/blob/536bbe5e7e21d8e2087eee9615f3b3c995955840/libnetwork/types/network.go#L233 Then you need to make podman set this new struct field with the correct value, likely here: https://github.com/containers/podman/blob/911be1cbcb0ad654a8e10666262bdcee50ee54e0/libpod/networking_common.go#L47

Change the type here in netavark to read the new field. Then add it to the grpc type so you can send it to the dhcp-proxy and then you need to figure out how to set this field in for the dhcp call. I checked at it seems the our DHCP library seems to support the Hostname option so it should be possible.

That said I definitely wouldn't call this easy which of course depends on your expertise. The proxy code is definitely not very "clean", so figuring out how it works is not as trivial as it should be.

agorgl commented 9 months ago

Waiting for this too!

SilverBut commented 9 months ago

Hi @Luap99, I have an implementation on my dev branch. Should I make a PR?

But I'm changing the proto, and maybe this feature should only be enabled if explicitly asked. Please suggest some idea.

Luap99 commented 9 months ago

Hi @Luap99, I have an implementation on my dev branch. Should I make a PR?

But I'm changing the proto, and maybe this feature should only be enabled if explicitly asked. Please suggest some idea.

I don't think you need to change the proto at all, there is already a host_name field which should be used. What is missing right now is that podman does not send us the real container hostname so we would need to fix this first.

I have no problem sending the hostname to the dhcp server by default, a hostname is not a secret.

SilverBut commented 9 months ago

@Luap99 Yes, missing container hostname is the problem. Actually the protobuf is changed to provide the container name, as a backup solution if hostname is missing.

Another problem is, DHCP server can actually send a hostname to client. What's worse(better?) is that popular implementations including systemd-networkd and dhcpcd will accept it. But I didn't find related structure about how (and when) to set hostname from netavark. Should this be implemented as well? Will someone want this feature?

Aaron-Rumpler commented 4 months ago

@Luap99 and @SilverBut Any updates on this?

Luap99 commented 4 months ago

No, this is waiting for someone to propose a PR. I don't have time for it.

SilverBut commented 4 months ago

Seems my dev branch have not updated for a while. Let me see what I can do.

briankoco commented 3 months ago

Using the container name as the dhcp_host_name seems like a reasonable default absent an explicit hostname request. From a podman perspective, interpreting the container name as a DNS hostname also seems reasonable, at least per this comment: https://github.com/containers/common/blob/536bbe5e7e21d8e2087eee9615f3b3c995955840/libnetwork/types/network.go#L236

... as well as the fact that multiple containers in a podman network can DNS resolve each other via their container names by default