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] non of the options are transmitted #4705

Closed
RealKev79 opened this issue Jul 1, 2022 · 8 comments
Closed

[DHCP] non of the options are transmitted #4705

RealKev79 opened this issue Jul 1, 2022 · 8 comments
Assignees
Milestone

Comments

@RealKev79
Copy link

RealKev79 commented Jul 1, 2022

Issue Details

  • Version of AdGuard Home server:
    • v0.107.7
  • How did you install AdGuard Home:
    • Docker (adguard/adguardhome:latest)
  • How did you setup DNS configuration:
    • ?
  • If it's a router or IoT, please write device model:
    • Synology DS614!play
  • CPU architecture:
    • Intel
  • Operating system and version:
    • DSM 7.1-42661 Update 2

Expected Behavior

Entered DHCPv4-options are sended to the client even when not directly requested (like #4152)

Actual Behavior

None of the entered options in the AdGuardHome.yaml (dhcp/dhcpv4/options) are sended to the client.

Screenshots

Screenshot from the config:

image

Screenshot from responding answer to a discover packet from AGH

image

Screenshot from responding answer to a discover packet from parallel installed docker-image dhcpd with nearly the same config-options

image

As seen, dhcpd sends all available options when AGH only sends the absolut minimum

Additional Information

config-file is loaded correctly because in logfile are no errors, and when configured an extremly short lease time (60sec) it is loaded correctly and also is sended to the client

@EugeneOne1
Copy link
Member

@RealKev79, hello. Just to ensure, have you enabled DHCP by setting dhcp.enabled field in the config to true? Also, which ports are exposed in the AGH's container?

@EugeneOne1 EugeneOne1 added the waiting for data Waiting for users to provide more data. label Jul 2, 2022
@RealKev79
Copy link
Author

Hi @EugeneOne1 ,
sorry but this question could you answer by yourself ;-) When the DHCP weren´t enabled then it would not be responding with the "standard" options (51, 53, 54). And also the ports couldn´t be a problem, because the server is responding (and the image is configured with "host"-network ;-))

@EugeneOne1 EugeneOne1 added needs investigation Needs to be reproduced reliably. and removed waiting for data Waiting for users to provide more data. labels Jul 4, 2022
@ainar-g ainar-g added this to the v0.107.12 milestone Aug 30, 2022
@EugeneOne1
Copy link
Member

EugeneOne1 commented Aug 30, 2022

@RealKev79, hello again and sorry for leaving the issue with no attention, but we've came up with an assumption. The current implementation only overwrites the options that should be in the response (we've also described this in the wiki):

  1. the ones explicitly requested by client with the option 55;
  2. the ones required for host configuration, as per RFC 2131 (which points to RFC 1122, RFC 1123, RFC 2132).

Shortly, I suppose the device doesn't request the parameters, you're trying to rewrite. Could you please request any of those with some manual DHCP client? Thanks.

@RealKev79
Copy link
Author

Hi @EugeneOne1,
sorry to bother you, but did I get this topic right that the DHCP in AdGuardHome is only pushing the "standard" option as an offer and all other, in the config entered, options are ignored and only are pushed when the client is directly requesting them?

Is this right or did I get something wrong?

Yes my request is a "simple" dhcp-discover (with a dhcp-test-tool), and as you can see in my screenshots above, that my parallel installed DHCPD-Docker responses there directly with all possible options from the config.

How should I unterstand the milestone? Would you (the whole team ;-)) check it and "corrected" it in that version, so that all options from the config are then provided?

Greetings

@EugeneOne1
Copy link
Member

@RealKev79, could you please take a look at the RFC 2131 page 29, the paragraph after the table 3. Doesn't it state that only the explicitly requested options (called "parameters" there) should be returned to the client? However, the same list of statements also contains the item:

Parameters with non-default values on the client's subnet.

Perhaps it means exactly the desired feature. @ainar-g, what do you think?

@ainar-g ainar-g added bug P3: Medium and removed needs investigation Needs to be reproduced reliably. labels Aug 31, 2022
adguard pushed a commit that referenced this issue Sep 5, 2022
Merge in DNS/adguard-home from 4705-fix-opts to master

Updates #4705.

Squashed commit of the following:

commit d3924c4
Merge: e46198c e545f3b
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 5 16:57:38 2022 +0300

    Merge branch 'master' into 4705-fix-opts

commit e46198c
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 5 16:52:20 2022 +0300

    dhcpd: immp docs

commit 1c1caea
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Sat Sep 3 17:31:35 2022 +0300

    dhcpd: fix logic, imp docs

commit bc74e21
Merge: 280ad10 1fb0437
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Sep 2 18:58:52 2022 +0300

    Merge branch 'master' into 4705-fix-opts

commit 280ad10
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Sep 2 00:53:38 2022 +0300

    dhcpd: imp docs, tests

commit 600fa44
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 1 20:24:52 2022 +0300

    dhcpd: add new opts

commit caf0cc6
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 1 18:13:02 2022 +0300

    dhcpd: log changes

commit 3d2c61d
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 1 18:09:34 2022 +0300

    dhcpd: imp opts
@EugeneOne1
Copy link
Member

@RealKev79, hello again. We've fixed the DHCP options logic as well as added some new option types for convenient overriding the default ones. You may find the verbose description in the wiki page. The fix is already available in the latest edge build. Could you please check, if the desired options are returned now?

@RealKev79
Copy link
Author

Hi @EugeneOne1, I have tried the "edge-version" and there everything works fine, as you can see:
image

@EugeneOne1
Copy link
Member

Fine, then I'll close the issue. Thank you for reporting.

heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Merge in DNS/adguard-home from 4705-fix-opts to master

Updates AdguardTeam#4705.

Squashed commit of the following:

commit d3924c4
Merge: e46198c e545f3b
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 5 16:57:38 2022 +0300

    Merge branch 'master' into 4705-fix-opts

commit e46198c
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Sep 5 16:52:20 2022 +0300

    dhcpd: immp docs

commit 1c1caea
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Sat Sep 3 17:31:35 2022 +0300

    dhcpd: fix logic, imp docs

commit bc74e21
Merge: 280ad10 1fb0437
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Sep 2 18:58:52 2022 +0300

    Merge branch 'master' into 4705-fix-opts

commit 280ad10
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Sep 2 00:53:38 2022 +0300

    dhcpd: imp docs, tests

commit 600fa44
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 1 20:24:52 2022 +0300

    dhcpd: add new opts

commit caf0cc6
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 1 18:13:02 2022 +0300

    dhcpd: log changes

commit 3d2c61d
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Thu Sep 1 18:09:34 2022 +0300

    dhcpd: imp opts
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

3 participants