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

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

Closed
AnastasiyaSoyka opened this issue Apr 19, 2023 · 17 comments · Fixed by #1130
Closed

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

AnastasiyaSoyka opened this issue Apr 19, 2023 · 17 comments · Fixed by #1130
Labels

Comments

@AnastasiyaSoyka
Copy link
Contributor

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
Copy link
Member

Luap99 commented Apr 20, 2023

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
Copy link
Contributor

agorgl commented Dec 2, 2023

Waiting for this too!

@SilverBut
Copy link
Contributor

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
Copy link
Member

Luap99 commented Dec 7, 2023

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
Copy link
Contributor

@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
Copy link

@Luap99 and @SilverBut Any updates on this?

@Luap99
Copy link
Member

Luap99 commented Apr 29, 2024

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

@SilverBut
Copy link
Contributor

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

@briankoco
Copy link

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

@gtjoseph
Copy link
Contributor

@SilverBut Are you still working on this? I just started converting containers to macvlan/dhcp and was surprised that a name was not sent. This has caused me to have to write scripts to update DNS when containers are created. Does the code in your dev branch work with container name as hostname? If so, that's a great start. I you don't have time to work on it would you mind if I took that code and created a PR for it?

@gtjoseph
Copy link
Contributor

I have a working implementation for passing the hostname from podman to netavark that I'm about to push up PRs for. Since the subject of defaulting to container name came up I want to clarify that the way I have it implemented, the default hostname is left to podman to specify rather than netavark. The reason is that if you don't specify --hostname in your podman run command, podman currently defaults the hostname to the first 12 characters of the container ID and that's what the container's /etc/hostname is set to. If we default it to container name in netavark, the container and the dhcp lease/ddns entry will disagree.

I am going to ask the podman folks if it would be reasonable to change its hostname default from the short ID to the container name with characters not in the set [0-9a-zA-Z.-] removed but that might be a big deal to users. There's also the issue that if no hostname is specified and the container isn't running in a private namespace, the hostname gets set to the hostname of the host which we certainly do NOT want passed in a DHCP request.

In any case, I believe that the responsibility of determining the hostname should rest with podman and that if no hostname is passed to netavark, netavark should simply not send hostname in the DHCP request.

@Luap99 does this make sense to you?

@Luap99
Copy link
Member

Luap99 commented Nov 25, 2024

I have a working implementation for passing the hostname from podman to netavark that I'm about to push up PRs for. Since the subject of defaulting to container name came up I want to clarify that the way I have it implemented, the default hostname is left to podman to specify rather than netavark. The reason is that if you don't specify --hostname in your podman run command, podman currently defaults the hostname to the first 12 characters of the container ID and that's what the container's /etc/hostname is set to. If we default it to container name in netavark, the container and the dhcp lease/ddns entry will disagree.

In any case, I believe that the responsibility of determining the hostname should rest with podman and that if no hostname is passed to netavark, netavark should simply not send hostname in the DHCP request.

@Luap99 does this make sense to you?

yes that seems good

I am going to ask the podman folks if it would be reasonable to change its hostname default from the short ID to the container name with characters not in the set [0-9a-zA-Z.-] removed but that might be a big deal to users. There's also the issue that if no hostname is specified and the container isn't running in a private namespace, the hostname gets set to the hostname of the host which we certainly do NOT want passed in a DHCP request.

I don't think we want to change the defaults, it is the same with docker AFAIK. On the other side with pods we default to the pod name as hostname so so there is certainly the case to be made to make this consistent.
We can have such discussion on the podman repo if you like, we netavark maintainers are a subset of podman maintainers to be clear.

The namespace issue can happen but I find it very unlikely that someone wants uts=host while also using a private network with DHCP. Ultimately we would still request a lease for a different mac address so I don't think it is that bad and in the end easy to call a user configuration issue.

@gtjoseph
Copy link
Contributor

I don't think we want to change the defaults, it is the same with docker AFAIK. On the other side with pods we default to the pod name as hostname so so there is certainly the case to be made to make this consistent.

Ultimately what I decided to do was add a use_container_name_as_hostname option to the CONTAINERS table in containers.conf. If not set (the default) then the behavior is as it is today (short ID if hostname isn't specified). If it is set, then the container's name (with characters not in the set [0-9a-zA-Z.-] removed) will be used as the hostname if it's not explicitly set. I'll have the PRs for podman, common and netavark up today up today so we can discuss in the appropriate repo.

The namespace issue can happen but I find it very unlikely that someone wants uts=host while also using a private network with DHCP. Ultimately we would still request a lease for a different mac address so I don't think it is that bad and in the end easy to call a user configuration issue.

Yeah it's a very corner case but since it can do damage to someone's environment I set the hostname passed to netavark to an empty string when not in a private UTS namespace. The container itself still gets the host's hostname as it is today.

@SilverBut
Copy link
Contributor

Hi @gtjoseph , sorry for late reply. Of course you can take any part of my branch if it's helpful, but seems your implementation is well completed. I have abandoned this feature due to some infra reasons. For now I'm sending additional container information with Option 82.

But I advise against using hostname as client id by default. A option might be better. Problem mentioned in the comment is because some DHCP implements won't always respect reservation due to existing leases. But making client id non-unique could cause additional risk, like container name reused on different host, which is a common use case. According to RFC2131:

The 'client identifier' chosen by a DHCP client MUST be unique to that client within the subnet to which the client is attached.

Even if DHCP server accepts this by assigning same IP for both client, the routing convergence time will be much longer, and such failure is not easy to debug.

@gtjoseph
Copy link
Contributor

@SilverBut What would you think about using the first 16 characters of the container ID as the client id? This should give enough uniqueness to prevent issues and will remain constant for the life of the container.

@SilverBut
Copy link
Contributor

I think most DHCP server will be fine as long as the client id is unique across device in same pool.

As for the length, I happen to run both kea and isc-dhcp, so I can get the lease database. Among ~10k leases (including those under reclaim time):

  • 7% have no client id because some weird old IoT devices only use mac address and never provide a client identifier.
  • Another 18% have more than 16+1 byte client id (some of them are servers running systemd-networkd, which generates a ff:${18-byte-dhcpv6-duid-info}). Maximum length is 37+1 byte and I have no idea about the reason.
  • All other clients simply use a 6+1 byte client id 01:${mac_addr}.

All length works well for both server. I'm not sure how Microsoft DHCP works but I believe it won't complain for a longer client identifier.

@gtjoseph
Copy link
Contributor

After discussion with @Luap99 I changed it to send all 64 bytes of the container ID as the client ID. RFC2132 section 9.14 doesn't mention a maximum length but it does mention setting it to a FQDN as an example so I think we're safe with 65 (1 byte for the type plus 64 byte client id). I tested with dnsmasq and a Mikrotik router with no issues. If any implementations were going to have issues, it would have been those two. :)

gtjoseph added a commit to gtjoseph/netavark that referenced this issue Nov 29, 2024
…t id

Currently, since a container's hostname isn't sent to netavark, we can't
include it in DHCP requests when using a macvlan network and DHCP IPAM
driver. Therefore, if DNS records need to be added for a container, it needs
to be done by external scripts.  Another issue is that since every time a
container starts it gets a new random mac address and since, in the absense
of a client id, the mac address is used by DHCP servers to keep track of
leases, it's likely that the container will get a new ip address on restart.
If the container is providing an exposed service, even just ssh, clients may
have the old address cached and may not be able to reach the container.

This commit accepts the container's hostname on incoming JSON setup commands
and adds it to the DHCP client configuration so an Option 12
(Hostname) parameter can be sent in DHCP messages. This should allow DDNS
updates of DNS resords. This is dependent on corresponding commits in Podman
and common to actually pass the host name of course.  This commit also sets
the Option 61 (Client identifier) to the container's ID.  Since this stays
constant across container restarts, it's likely the DHCP server will use the
existing lease and return the same IP address.

* Added "container_hostname" to network::types::NetworkOptions. The setup
  command will set this from the incoming JSON and pass it through ultimately
  to dhcp_service::DhcpV4Service via macvlan_dhcp.get_dhcp_lease().

* Added "container_id" to proxy.proto so it can also be passed to
  dhcp_service::DhcpV4Service and used to set the client id.

* Added tests to test-dhcp/002-setup.bats to check that hostname is passed
  correctly.

Fixes: containers#676
Signed-off-by: George Joseph <[email protected]>
gtjoseph added a commit to gtjoseph/netavark that referenced this issue Nov 30, 2024
…t id

Currently, since a container's hostname isn't sent to netavark, we can't
include it in DHCP requests when using a macvlan network and DHCP IPAM
driver. Therefore, if DNS records need to be added for a container, it needs
to be done by external scripts.  Another issue is that since every time a
container starts it gets a new random mac address and since, in the absense
of a client id, the mac address is used by DHCP servers to keep track of
leases, it's likely that the container will get a new ip address on restart.
If the container is providing an exposed service, even just ssh, clients may
have the old address cached and may not be able to reach the container.

This commit accepts the container's hostname on incoming JSON setup commands
and adds it to the DHCP client configuration so an Option 12
(Hostname) parameter can be sent in DHCP messages. This should allow DDNS
updates of DNS resords. This is dependent on corresponding commits in Podman
and common to actually pass the host name of course.  This commit also sets
the Option 61 (Client identifier) to the container's ID.  Since this stays
constant across container restarts, it's likely the DHCP server will use the
existing lease and return the same IP address.

* Added "container_hostname" to network::types::NetworkOptions. The setup
  command will set this from the incoming JSON and pass it through ultimately
  to dhcp_service::DhcpV4Service via macvlan_dhcp.get_dhcp_lease().

* Added "container_id" to proxy.proto so it can also be passed to
  dhcp_service::DhcpV4Service and used to set the client id.

* Added tests to test-dhcp/002-setup.bats to check that hostname is passed
  correctly.

Fixes: containers#676
Signed-off-by: George Joseph <[email protected]>
gtjoseph added a commit to gtjoseph/netavark that referenced this issue Nov 30, 2024
…t id

Currently, since a container's hostname isn't sent to netavark, we can't
include it in DHCP requests when using a macvlan network and DHCP IPAM
driver. Therefore, if DNS records need to be added for a container, it needs
to be done by external scripts.  Another issue is that since every time a
container starts it gets a new random mac address and since, in the absense
of a client id, the mac address is used by DHCP servers to keep track of
leases, it's likely that the container will get a new ip address on restart.
If the container is providing an exposed service, even just ssh, clients may
have the old address cached and may not be able to reach the container.

This commit accepts the container's hostname on incoming JSON setup commands
and adds it to the DHCP client configuration so an Option 12
(Hostname) parameter can be sent in DHCP messages. This should allow DDNS
updates of DNS resords. This is dependent on corresponding commits in Podman
and common to actually pass the host name of course.  This commit also sets
the Option 61 (Client identifier) to the container's ID.  Since this stays
constant across container restarts, it's likely the DHCP server will use the
existing lease and return the same IP address.

* Added "container_hostname" to network::types::NetworkOptions. The setup
  command will set this from the incoming JSON and pass it through ultimately
  to dhcp_service::DhcpV4Service via macvlan_dhcp.get_dhcp_lease().

* Added "container_id" to proxy.proto so it can also be passed to
  dhcp_service::DhcpV4Service and used to set the client id.

* Added tests to test-dhcp/002-setup.bats to check that hostname is passed
  correctly.

Fixes: containers#676
Signed-off-by: George Joseph <[email protected]>
gtjoseph added a commit to gtjoseph/netavark that referenced this issue Nov 30, 2024
…t id

Currently, since a container's hostname isn't sent to netavark, we can't
include it in DHCP requests when using a macvlan network and DHCP IPAM
driver. Therefore, if DNS records need to be added for a container, it needs
to be done by external scripts.  Another issue is that since every time a
container starts it gets a new random mac address and since, in the absense
of a client id, the mac address is used by DHCP servers to keep track of
leases, it's likely that the container will get a new ip address on restart.
If the container is providing an exposed service, even just ssh, clients may
have the old address cached and may not be able to reach the container.

This commit accepts the container's hostname on incoming JSON setup commands
and adds it to the DHCP client configuration so an Option 12
(Hostname) parameter can be sent in DHCP messages. This should allow DDNS
updates of DNS resords. This is dependent on corresponding commits in Podman
and common to actually pass the host name of course.  This commit also sets
the Option 61 (Client identifier) to the container's ID.  Since this stays
constant across container restarts, it's likely the DHCP server will use the
existing lease and return the same IP address.

* Added "container_hostname" to network::types::NetworkOptions. The setup
  command will set this from the incoming JSON and pass it through ultimately
  to dhcp_service::DhcpV4Service via macvlan_dhcp.get_dhcp_lease().

* Added "container_id" to proxy.proto so it can also be passed to
  dhcp_service::DhcpV4Service and used to set the client id.

* Added tests to test-dhcp/002-setup.bats to check that hostname is passed
  correctly.

Fixes: containers#676
Signed-off-by: George Joseph <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants