dhcp offer ip by client id in dhcpv4

[nmreadelf]

I have configured a virtual machine that utilizes the host network on my physical machine. Upon examination, I discovered that the AdGuard Home server being hosted offers IP addresses based on the MAC address of the physical host machine rather than the virtual machine itself. Specifically, the actual MAC address of the virtual machine is present in the client-id field of DHCP requests. For proper functionality, the DHCP server should be offering IP addresses based on the client-id field, which contains the MAC address of the virtual machine. This would allow the DHCP server to correctly identify the virtual machine and provide it an IP address accordingly. The current behavior indicates a misconfiguration, where the DHCP server is assigning IP addresses using the physical host's MAC address instead of the virtual machine client's MAC address contained in the DHCP request.

resolve #6135

[EugeneOne1]

Hi @nmreadelf, sorry for the late reply and thanks for the contribution.

As far as I know, RFC-4361 Section 6.1, which updates the referenced RFC-2132, actually forbids using the hardware address in option 61 alone. However, a cursory internet search indicated that this feature may be somewhat useful, so could you please file a "feature request" issue so that we can see if it's actually the case?

[EugeneOne1]

See previous comment.

[nmreadelf]

Hi @nmreadelf, sorry for the late reply and thanks for the contribution.

As far as I'm know, RFC-4361 Section 6.1, which updates the referenced RFC-2132, actually forbids using the hardware address in option 61 alone. However, a cursory internet search indicated that this feature may be somewhat useful, so could you please file a "feature request" issue so that we can see if it's actually the case?

relate #6135

[nmreadelf]

@EugeneOne1 @ainar-g hello, i resolve merge conflicts, can you make approve again?

[nmreadelf]

@EugeneOne1 @ainar-g hello, Could you please assist in merging this pull request? It has been open for close to a year.

[EugeneOne1]

@nmreadelf, good afternoon and apologies for late review. The reactions to the issue proposing this change seem to not have demonstrated the need for it. In addition, you never responded to our concerns about the standard's recommendations. So, we are not ready to merge this change.
You can always keep this feature in your personal fork of AdGuard Home.

[Sil3ntVip3r]

I rechecked exact head 9ba9ba73 on the current Go toolchain. It no longer
compiles:

go test -race -count=1 ./internal/dhcpd
internal/dhcpd/v4_unix.go:25:2: slices redeclared in this block
internal/dhcpd/v4_unix.go:25:2: "golang.org/x/exp/slices" imported and not used
FAIL github.com/AdguardTeam/AdGuardHome/internal/dhcpd [build failed]

The branch imports both the standard-library slices package and
golang.org/x/exp/slices; git diff --check itself is clean.

More importantly, resolving that compile error would not make the
implementation safe to merge. The patch treats any seven-byte option 61 with
a nonzero type as an Ethernet MAC and overwrites req.ClientHWAddr. RFC 4361
section 6.1 defines the modern type-255 IAID+DUID form, while section 6.3
requires a conforming server to use option 61 as the client identity when
supplied. A future implementation should preserve option 61 as an opaque
lease identity (with explicit legacy type handling) instead of reinterpreting
it as chaddr.

The linked feature request #6135 has received no additional use-case
discussion since 2023, and the maintainer already concluded that the demand
and standards questions do not justify merging this approach. I therefore
recommend closing this conflicting PR. I did not run the full suite or
packet-level runtime tests, and I did not prepare a current-master port,
because the exact target package cannot build and the product direction
remains rejected.

[nmreadelf]

I rechecked exact head 9ba9ba73 on the current Go toolchain. It no longer compiles:

go test -race -count=1 ./internal/dhcpd
internal/dhcpd/v4_unix.go:25:2: slices redeclared in this block
internal/dhcpd/v4_unix.go:25:2: "golang.org/x/exp/slices" imported and not used
FAIL github.com/AdguardTeam/AdGuardHome/internal/dhcpd [build failed]

The branch imports both the standard-library slices package and golang.org/x/exp/slices; git diff --check itself is clean.

More importantly, resolving that compile error would not make the implementation safe to merge. The patch treats any seven-byte option 61 with a nonzero type as an Ethernet MAC and overwrites req.ClientHWAddr. RFC 4361 section 6.1 defines the modern type-255 IAID+DUID form, while section 6.3 requires a conforming server to use option 61 as the client identity when supplied. A future implementation should preserve option 61 as an opaque lease identity (with explicit legacy type handling) instead of reinterpreting it as chaddr.

The linked feature request #6135 has received no additional use-case discussion since 2023, and the maintainer already concluded that the demand and standards questions do not justify merging this approach. I therefore recommend closing this conflicting PR. I did not run the full suite or packet-level runtime tests, and I did not prepare a current-master port, because the exact target package cannot build and the product direction remains rejected.

compile problem fixed

[Sil3ntVip3r]

Re-reviewed exact head ced76bb47b4499c374131a48c3b2eb411696044d. The import/compile repair is good, but this still is not ready to merge.

1. The current implementation does not solve the scenario in the PR body. clientMAC uses option 61 only when chaddr is nil or all-zero, and the new valid_chaddr_is_preferred test makes that explicit. With a nonzero physical-host chaddr and a legacy type-1 option-61 containing the VM MAC, it still returns the physical-host MAC. All of handleDiscover, request handling, and release lookup then use that value, so the VM cannot receive a separate lease. I verified this with a focused disposable test of exactly that packet shape. Please either implement an explicit, end-to-end client-identity policy (including lease lookup/storage and reply-delivery implications) with a regression test for nonzero chaddr plus option 61, or close this PR as it cannot satisfy its stated use case. The current type-255 protection is good, but this fallback is not RFC-4361 client-identity support.

2. make go-check currently fails before tests because gofumpt requires a blank line before TestV4Server_clientMAC in internal/dhcpd/v4_unix_internal_test.go. Please fix that and rerun the gate.

Focused go test -race -count=1 ./internal/dhcpd and go vet ./internal/dhcpd pass on this head; the full gate fails only at the formatter described above.