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

Hostnames for DHCP static leases are not retrieved #3166

Closed
3 tasks done
agneevX opened this issue May 22, 2021 · 16 comments
Closed
3 tasks done

Hostnames for DHCP static leases are not retrieved #3166

agneevX opened this issue May 22, 2021 · 16 comments
Assignees
Milestone

Comments

@agneevX
Copy link
Contributor

agneevX commented May 22, 2021

Prerequisites

  • I am running the latest version
  • I checked the documentation and found no answer
  • I checked to make sure that this issue has not already been filed

Issue Details

  • Version of AdGuard Home server: 0.106.3
  • How did you install AdGuard Home: GH Releases
  • How did you setup DNS configuration: Router/WAN DNS
  • If it's a router or IoT, please write device model: N/A
  • CPU architecture: arm64
  • Operating system and version: Ubuntu 20.04 LTS

Expected Behavior

Hostnames should be retrieved for DHCP leased devices, just like the behavior with any other DHCP-connected, non-leased device.

Actual Behavior

It seems that the "filler hostname" feature that was introduced in v0.106.0 sets a hostname for clients which are added via DHCP leases, instead of actually querying with ARP.

Screenshots

image

Additional Information

@ameshkov
Copy link
Member

I don't think this is the right way to resolve this.

When you add a static lease you're supposed to supply a hostname as well. The hostname that the device sends in the DHCP request will be ignored in this case.

@rysuq
Copy link

rysuq commented May 23, 2021

The same issue after update to new version of AdGuardHome.

@agneevX
Copy link
Contributor Author

agneevX commented May 24, 2021

When you add a static lease you're supposed to supply a hostname as well.

That sounds ridiculous. How am I supposed to know what the hostname is? I don't want to put a fancy custom hostname if that's what you mean.

At best, this should be auto-generated.

@ameshkov
Copy link
Member

10-0-0-2 is auto-generated hostname.

@agneevX
Copy link
Contributor Author

agneevX commented May 24, 2021

Sorry, I will clarify that when I meant "auto-generated", I referred to the automated ARP scan just like for every DHCP device.

That should be used when the device does not have a host name. E.g. Pixel phones.

@ameshkov
Copy link
Member

ARP scan makes little sense for a DHCP server, we're receiving the hostname in a DHCP request anyway.

@ainar-g what do you think?

@agneevX
Copy link
Contributor Author

agneevX commented May 24, 2021

I was under the impression that all devices added via AGH acting as DHCP were scanned in via ARP, as denoted by the source column.

These devices are all added via DHCP (non-static leasing):

image

@ainar-g
Copy link
Contributor

ainar-g commented May 24, 2021

@ameshkov, @EugeneOne1, we should already get the hostname if it's not taken, so I'm not really sure what is happening here.

@agneevX, could you please collect verbose logs after restarting AGH and post them here or to our email? Thanks!

@agneevX
Copy link
Contributor Author

agneevX commented May 24, 2021

I've sent an email with the logs!

@agneevX
Copy link
Contributor Author

agneevX commented Jun 7, 2021

Sorry to interrupt but is there an update available here?

I've found that adding a DHCP client to a static lease instantly invalidates the hostname.

@ainar-g
Copy link
Contributor

ainar-g commented Jun 7, 2021

This issue is next in the queue, but I can't give you all any precise ETAs, sorry.

@EugeneOne1
Copy link
Member

EugeneOne1 commented Jun 9, 2021

@agneevX, unfortunatelly, we can't reproduce the issue. Could you please provide some details:

  1. How the static leases from the original message were added?
  2. What does the non-static leases table in "DHCP settings" contains? Are devices' names there (received from ARP) differ from the ones in the "Clients settings"?

Additionally, you could try the latest edge build and see, if the issue appears there.

@agneevX
Copy link
Contributor Author

agneevX commented Jun 9, 2021

How the static leases from the original message were added?

Not sure what you mean. Static leases were added via the UI with the hostname field being blank.

Note that the device was not connected via DHCP when it was added. Also AGH has been restarted since, which means that the hostname field was automatically populated with 10-0-0- pattern.

Are devices' names there (received from ARP) differ from the ones in the "Clients settings"?

No they do not. They all have the 10-0-0- pattern (even though these devices have valid hostnames).

we can't reproduce the issue.

Try this:

Scenario 1:

  1. Connect a device via AGH DHCP, without creating a lease
  2. (Note the hostname displayed)
  3. Reset AGH
  4. Set up a static lease (hostname field blank) for that device, while leaving it disconnected
  5. Connect device
  6. Note that the device is not reachable via the hostname you noted down.

Scenario 2:

Hostname disappears when static lease is set

  1. Note down the hostname of a particular device
  2. Confirm with a nslookup query
  3. Add a static lease for a device that's connected, again leaving the hostname field blank
  4. Try doing the nslookup again: NXDOMAIN probably
  5. Try doing a nslookup for the pattern hostname.

These could be related or you could open an issue for this one.

adguard pushed a commit that referenced this issue Jun 10, 2021
Updates #3166.

Squashed commit of the following:

commit cac62f8
Author: Ildar Kamalov <ik@adguard.com>
Date:   Thu Jun 10 14:35:40 2021 +0300

    client: get dhcp status on static lease submit

commit 2ae4e3a
Merge: 1e0f9c1 299ea88
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 14:09:05 2021 +0300

    Merge branch 'master' into 3166-fix-dhcp

commit 1e0f9c1
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 14:03:58 2021 +0300

    dhcpd: imp code

commit e4b9626
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 13:03:47 2021 +0300

    dhcpd: fix static lease load

commit dc449f2
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Jun 1 17:21:10 2021 +0300

    home: change clients sources priority
@EugeneOne1
Copy link
Member

@agneevX, thanks for your detailed report.

We've just published the new edge build. It should prevent generating "filler hostname" for static leases loaded from database. The hostname field of all such leases now should remain empty. Note also, that the empty hostname is a feature, not a bug. If you want to set the hostname for a static lease, you should set it manually.

Could you please check if it fixes the issue for you?

@agneevX
Copy link
Contributor Author

agneevX commented Jun 10, 2021

The hostname field of all such leases now should remain empty. Note also, that the empty hostname is a feature, not a bug.

I'm sorry but this is not the way this feature generally works.

A static lease is simply a static lease. If a user chooses to specify a custom hostname, that should be honored.

However, if the hostname is left empty, and AGH is able to retrieve the hostname, it should use it rather than discard it, which serves no purpose at all. I'm very curious as to why this is.

Also, am I correct in assuming that static leased clients that have an empty hostname field do not have a hostname at all?

We've just published the new edge build

I'm sorry, but my environment does not allow me to test beta builds, especially since I've run into major issues with stable builds.

@ainar-g
Copy link
Contributor

ainar-g commented Jun 10, 2021

A static lease is simply a static lease. If a user chooses to specify a custom hostname, that should be honored.

The way it currently works is that if you add a static lease, you also decide its hostname, including having the ability to leave it empty. We try to not modify static leases in any way. Perhaps we could a separate setting that would mean “this is a static lease, but accept any hostname that the client will send”, but even that would significantly complicate an already complicated logic. We will probably be able to properly return to this only in the v0.108.0 cycle.

I'm sorry, but my environment does not allow me to test beta builds, especially since I've run into major issues with stable builds.

I see. Hopefully, v0.107.0 will be an exception.

We'll close this issue for now. Apologies for any inconveniences.

@ainar-g ainar-g closed this as completed Jun 10, 2021
heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Updates AdguardTeam#3166.

Squashed commit of the following:

commit cac62f8
Author: Ildar Kamalov <ik@adguard.com>
Date:   Thu Jun 10 14:35:40 2021 +0300

    client: get dhcp status on static lease submit

commit 2ae4e3a
Merge: 1e0f9c1 299ea88
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 14:09:05 2021 +0300

    Merge branch 'master' into 3166-fix-dhcp

commit 1e0f9c1
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 14:03:58 2021 +0300

    dhcpd: imp code

commit e4b9626
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 13:03:47 2021 +0300

    dhcpd: fix static lease load

commit dc449f2
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Jun 1 17:21:10 2021 +0300

    home: change clients sources priority
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants