Page MenuHomeVyOS Platform
Create Task
Maniphest T1129

Lack of quotes in DHCP's shared-network-parameters option, stops isc-dhcp-server from loading
Closed, ResolvedPublicBUG
Actions

Description

From the forum

In the following config, the shared-network-parameters breaks the config.

shared-network-name Management {
     shared-network-parameters 'option root-path 10.0.1.4:/var/tmp/rootfs'
     subnet 10.50.1.0/24 {
         default-router 10.50.1.1
         dns-server 1.1.1.1
         dns-server 8.8.8.8
         domain-name management.domain.io
         lease 86399
         range 0 {
             start 10.50.1.100
             stop 10.50.1.254
         }
     }
 }

But it would be a valid config if it was allowed to be: shared-network-parameters ‘option root-path “10.0.1.4:/var/tmp/rootfs"’. The validator prevents this (maybe for good reason).

Details

Difficulty level
Unknown (require assessment)
Version
1.2.0-rc11
Why the issue appeared?
Implementation mistake

Related Objects

Event Timeline

kroy created this task.Dec 20 2018, 3:47 PM
kroy updated the task description. (Show Details)
pasik added a subscriber: pasik.Dec 20 2018, 5:34 PM
kroy updated the task description. (Show Details)Dec 20 2018, 6:21 PM
c-po added a subscriber: c-po.Dec 20 2018, 7:05 PM

The shared-network-parameters is a real pain and should be used "without warranty" we should rather deprecate it and add proper CLI interface.

syncer triaged this task as Low priority.Dec 21 2018, 10:17 AM
syncer edited projects, added VyOS 1.3 Equuleus; removed VyOS 1.2 Crux.
patanne added a subscriber: patanne.EditedJan 2 2019, 8:51 PM

I agree that this should become part of the CLI. The current method is too dangerous and has always been too loose for good syntax checking. It always used to be that you had to use " to embed quotes. That has not worked of late.

I made a little patch against rc11 to restore the ability to use ". the diff looks like this..

*** /tmp/dhcp_server.py 2019-01-02 16:57:20.815113797 -0500
--- /usr/libexec/vyos/conf_mode/dhcp_server.py  2019-01-02 16:57:47.436205526 -0500
***************
*** 767,772 ****
--- 767,773 ----

      tmpl = jinja2.Template(config_tmpl)
      config_text = tmpl.render(dhcp)
+     config_text = config_text.replace(""",'"')
      with open(config_file, 'w') as f:
          f.write(config_text)

your changes would now look like this...

shared-network-name Management {
     shared-network-parameters option root-path "10.0.1.4:/var/tmp/rootfs"'
     subnet 10.50.1.0/24 {
         default-router 10.50.1.1
         dns-server 1.1.1.1
         dns-server 8.8.8.8
         domain-name management.domain.io
         lease 86399
         range 0 {
             start 10.50.1.100
             stop 10.50.1.254
         }
     }
 }
patanne added a comment.EditedJan 6 2019, 3:05 AM

I found another bug in the dhcp generator. Here is the full fix.

In this thread and in the code comments you are mulling deprecating the dhcp custom options feature, like dns forwarding. These options are crucial for provisioning phones of all different brands and anything in the Ubiquiti Unifi line of products. Do not pull the feature.

dhcp_server.patch2 KBDownload

*** ~/dhcp_server.py    2019-01-03 02:47:12.246518015 -0500
--- /usr/libexec/vyos/conf_mode/dhcp_server.py  2019-01-03 06:26:41.756600189 -0500
***************
*** 150,155 ****
--- 150,161 ----
          {%- if subnet.domain_name %}
          option domain-name "{{ subnet.domain_name }}";
          {%- endif -%}
+         {%- if subnet.subnet_parameters %}
+         # The following {{ subnet.subnet_parameters | length }} line(s) were added as subnet-parameters in the CLI and have not been validated
+         {%- for param in subnet.subnet_parameters %}
+         {{ param }}
+         {%- endfor -%}
+         {%- endif %}
          {%- if subnet.tftp_server %}
          option tftp-server-name "{{ subnet.tftp_server }}";
          {%- endif -%}
***************
*** 570,576 ****
                      #
                      # deprecate this and issue a warning like we do for DNS forwarding?
                      if conf.exists('subnet-parameters'):
!                         config['subnet_parameters'] = conf.return_values('subnet-parameters')

                      # This option is used to identify a TFTP server and, if supported by the client, should have
                      # the same effect as the server-name declaration. BOOTP clients are unlikely to support this
--- 576,582 ----
                      #
                      # deprecate this and issue a warning like we do for DNS forwarding?
                      if conf.exists('subnet-parameters'):
!                         subnet['subnet_parameters'] = conf.return_values('subnet-parameters')

                      # This option is used to identify a TFTP server and, if supported by the client, should have
                      # the same effect as the server-name declaration. BOOTP clients are unlikely to support this
***************
*** 767,772 ****
--- 773,779 ----

      tmpl = jinja2.Template(config_tmpl)
      config_text = tmpl.render(dhcp)
+     config_text = config_text.replace(""",'"')
      with open(config_file, 'w') as f:
          f.write(config_text)
c-po added a comment.Jan 6 2019, 9:59 AM

Thanks for your contribution. It will be included in the next EPA and rolling images.

c-po closed this task as Resolved.Jan 6 2019, 9:59 AM
c-po claimed this task.
c-po edited projects, added VyOS 1.2 Crux (VyOS 1.2.0-EPA3); removed VyOS 1.3 Equuleus.
c-po changed Why the issue appeared? from Will be filled on close to Implementation mistake.
syncer added a project: VyOS-1.2.0-GA.Jan 6 2019, 3:03 PM
syncer moved this task from Needs Triage to Finished on the VyOS 1.2 Crux (VyOS 1.2.0-EPA3) board.
c-po mentioned this in T1213: ddclient not functional.Feb 3 2019, 3:49 PM
c-po mentioned this in T1632: OpenVPN 'push' options with quotes.Sep 4 2019, 6:31 PM
jjakob mentioned this in T1806: Add missing documentation on how to use quote characters inside dhcp-server raw parameters.Dec 15 2019, 3:45 PM