From 12808651448837c611e4f6a262f7a1eb3deaf8da Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 14 Feb 2015 14:08:06 +0100 Subject: [PATCH] make dhcp-ddns qualifying-suffix mandatory in enabled configs --- ChangeLog | 7 ++ doc/guide/dhcp4-srv.xml | 87 ++++++++++++----- doc/guide/dhcp6-srv.xml | 93 ++++++++++++++----- src/bin/dhcp4/dhcp4.spec | 2 +- src/bin/dhcp6/dhcp6.spec | 2 +- src/lib/dhcpsrv/d2_client_cfg.cc | 5 +- src/lib/dhcpsrv/d2_client_cfg.h | 8 +- src/lib/dhcpsrv/parsers/dhcp_parsers.cc | 12 +-- src/lib/dhcpsrv/parsers/dhcp_parsers.h | 2 +- src/lib/dhcpsrv/tests/d2_client_unittest.cc | 4 +- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 21 +++-- 11 files changed, 173 insertions(+), 70 deletions(-) diff --git a/ChangeLog b/ChangeLog index 211e643374..ce61d0503b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +893. [func,bug] fdupont + Changed the qualifying-suffix parameter in the dhcp-ddns + configuration element to be mandatory with no default value when + updates are enabled (i.e., the enable-updates mandatory parameter + is true). + (Trac #3632, git ) + 892. [func] sar A class, LeaseFileStats, has been added to provide simple statistics for use with lease files. Also added logging diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index d22f52951f..8f11d86e7d 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -318,7 +318,7 @@ url="http://jsonviewer.stack.hu/"/>. database host name must also be specified (although it should be noted that this configuration may have a severe impact on server performance): -"Dhcp4": { "lease-database": { "host": remote-host-name", ... }, ... } +"Dhcp4": { "lease-database": { "host": remote-host-name, ... }, ... } The usual state of affairs will be to have the database on the same machine as the DHCPv4 server. In this case, set the value to the empty string: @@ -1270,7 +1270,7 @@ temporarily override a list of interface names and listen on all interfaces. "subnet4": [ { "subnet": "192.0.2.0/24" - "option-data": [ {" + "option-data": [ { "name": "domain-name-servers", "code": 6, "data": "192.0.2.200,192.0.2.201", @@ -1416,27 +1416,68 @@ temporarily override a list of interface names and listen on all interfaces. The parameters for controlling the generation of NCRs for submission to D2 are contained in the dhcp-ddns section of the kea-dhcp4 server - configuration. The default values for this section are as follows: + configuration. The mandatory parameters for the DHCP DDNS configuration + are enable-updates which is unconditionally + required, and qualifying-suffix which has no + default value and is required when enable-updates + is set to true. + + The two (disabled and enabled) minimal DHCP DDNS configurations are: + +"Dhcp4": { + "dhcp-ddns": { + "enable-updates": false + }, + ... +} + + and for example: "Dhcp4": { "dhcp-ddns": { "enable-updates": true, - "server-ip": "127.0.0.1", - "server-port": 53001, - "sender-ip": "", - "sender-port": 0, - "max-queue-size": 1024, - "ncr-protocol": "UDP", - "ncr-format": "JSON", - "override-no-update": false, - "override-client-update": false, - "replace-client-name": false, - "generated-prefix": "myhost", - "qualifying-suffix": "example.com" + "qualifying-suffix": "example." }, ... } + + The default values for the "dhcp-ddns" section are as follows: + + + "server-ip": "127.0.0.1" + + + "server-port": 53001 + + + "sender-ip": "" + + + "sender-port": 0 + + + "max-queue-size": 1024 + + + "ncr-protocol": "UDP" + + + "ncr-format": "JSON" + + + "override-no-update": false + + + "override-client-update": false + + + "replace-client-name": false + + + "generated-prefix": "myhost" + +
@@ -1712,9 +1753,12 @@ temporarily override a list of interface names and listen on all interfaces. } - The suffix used when generating a FQDN or when qualifying a partial name - is specified by the qualifying-suffix parameter. There - is no default value. To set its value simply set it to the desired string: + The suffix used when generating a FQDN or when qualifying a + partial name is specified by + the qualifying-suffix parameter. This + parameter has no default value, thus it is mandatory when + DDNS updates are enabled. + To set its value simply set it to the desired string: "Dhcp4": { @@ -1734,10 +1778,9 @@ temporarily override a list of interface names and listen on all interfaces. where address-text is simply the lease IP address converted to a - hyphenated string. For example, if the lease address is 172.16.1.10 and - default values are used for - generated-prefix and qualifying-suffix, the - generated FQDN would be: + hyphenated string. For example, if the lease address is 172.16.1.10, + the qualifying suffix "example.com", and the default value is used for + generated-prefix, the generated FQDN would be: myhost-172-16-1-10.example.com. diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 35031cca93..cceb16ded7 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -322,7 +322,7 @@ JSON validator is available at . database host name must also be specified (although it should be noted that this configuration may have a severe impact on server performance): -"Dhcp6": { "lease-database": { "host": remote-host-name", ... }, ... } +"Dhcp6": { "lease-database": { "host": remote-host-name, ... }, ... } The usual state of affairs will be to have the database on the same machine as the DHCPv6 server. In this case, set the value to the empty string: @@ -1029,10 +1029,10 @@ temporarily override a list of interface names and listen on all interfaces. "type": "ipv6-address". "record-types": "", "array": false, - "encapsulate "" + "encapsulate": "" }, { - "name": "subopt2", + "name": "subopt2", "code": 2, "space": "isc", "type": "string", @@ -1402,27 +1402,68 @@ should include options from the isc option space: The parameters controlling the generation of NCRs for submission to D2 are contained in the "dhcp-ddns" section of kea-dhcp6 - configuration. The default values for this section appears as follows: + configuration. The mandatory parameters for the DHCP DDNS configuration + are enable-updates which is unconditionally + required, and qualifying-suffix which has no + default value and is required when enable-updates + is set to true. + + The two (disabled and enabled) minimal DHCP DDNS configurations are: + +"Dhcp6": { + "dhcp-ddns": { + "enable-updates": false + }, + ... +} + + and for example: "Dhcp6": { "dhcp-ddns": { "enable-updates": true, - "server-ip": "127.0.0.1", - "server-port": 53001, - "sender-ip": "", - "sender-port": 0, - "max-queue-size": 1024, - "ncr-protocol": "UDP", - "ncr-format": "JSON", - "override-no-update": false, - "override-client-update": false, - "replace-client-name": false, - "generated-prefix": "myhost", - "qualifying-suffix": "example.com" + "qualifying-suffix": "example." }, ... } + + The default values for the "dhcp-ddns" section are as follows: + + + "server-ip": "127.0.0.1" + + + "server-port": 53001 + + + "sender-ip": "" + + + "sender-port": 0 + + + "max-queue-size": 1024 + + + "ncr-protocol": "UDP" + + + "ncr-format": "JSON" + + + "override-no-update": false + + + "override-client-update": false + + + "replace-client-name": false + + + "generated-prefix": "myhost" + + @@ -1694,9 +1735,12 @@ should include options from the isc option space: } - The suffix used when generating a FQDN or when qualifying a partial name - is specified by the qualifying-suffix parameter. There - is no default value. To set its value simply set it to the desired string: + The suffix used when generating a FQDN or when qualifying a + partial name is specified by + the qualifying-suffix parameter. This + parameter has no default value, thus it is mandatory when + DDNS updates are enabled. + To set its value simply set it to the desired string: "Dhcp6": { @@ -1717,8 +1761,8 @@ should include options from the isc option space: where candidate-name is the partial name supplied in the REQUEST. - For example, if FQDN domain name value was "some-computer" and assuming - the default value for qualifying-suffix, the generated FQDN would be: + For example, if FQDN domain name value was "some-computer" and + qualifying-suffix "example.com", the generated FQDN would be: some-computer.example.com. @@ -1732,10 +1776,9 @@ should include options from the isc option space: where address-text is simply the lease IP address converted to a - hyphenated string. For example, if lease address is 3001:1::70E and - default values are used for - generated-prefix and qualifying-suffix, the - generated FQDN would be: + hyphenated string. For example, if lease address is 3001:1::70E, + the qualifying suffix "example.com", and the default value is used for + generated-prefix, the generated FQDN would be: myhost-3001-1--70E.example.com. diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec index 56be067d4b..fd57881705 100644 --- a/src/bin/dhcp4/dhcp4.spec +++ b/src/bin/dhcp4/dhcp4.spec @@ -488,7 +488,7 @@ "item_name": "qualifying-suffix", "item_type": "string", "item_optional": true, - "item_default": "example.com", + "item_default": "", "item_description": "Fully qualified domain-name suffix if partial name provided by client" }, ] diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec index 3d8bebf634..90043b4cdc 100644 --- a/src/bin/dhcp6/dhcp6.spec +++ b/src/bin/dhcp6/dhcp6.spec @@ -554,7 +554,7 @@ "item_name": "qualifying-suffix", "item_type": "string", "item_optional": true, - "item_default": "example.com", + "item_default": "", "item_description": "Fully qualified domain-name suffix if partial name provided by client" }, ] diff --git a/src/lib/dhcpsrv/d2_client_cfg.cc b/src/lib/dhcpsrv/d2_client_cfg.cc index 49107dda60..c6fd77cef0 100644 --- a/src/lib/dhcpsrv/d2_client_cfg.cc +++ b/src/lib/dhcpsrv/d2_client_cfg.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -36,7 +36,6 @@ const bool D2ClientConfig::DFT_OVERRIDE_NO_UPDATE = false; const bool D2ClientConfig::DFT_OVERRIDE_CLIENT_UPDATE = false; const bool D2ClientConfig::DFT_REPLACE_CLIENT_NAME = false; const char *D2ClientConfig::DFT_GENERATED_PREFIX = "myhost"; -const char *D2ClientConfig::DFT_QUALIFYING_SUFFIX = "example.com"; D2ClientConfig::D2ClientConfig(const bool enable_updates, const isc::asiolink::IOAddress& server_ip, @@ -85,7 +84,7 @@ D2ClientConfig::D2ClientConfig() override_client_update_(DFT_OVERRIDE_CLIENT_UPDATE), replace_client_name_(DFT_REPLACE_CLIENT_NAME), generated_prefix_(DFT_GENERATED_PREFIX), - qualifying_suffix_(DFT_QUALIFYING_SUFFIX) { + qualifying_suffix_("") { validateContents(); } diff --git a/src/lib/dhcpsrv/d2_client_cfg.h b/src/lib/dhcpsrv/d2_client_cfg.h index 1a82942abf..0c8afb537a 100644 --- a/src/lib/dhcpsrv/d2_client_cfg.h +++ b/src/lib/dhcpsrv/d2_client_cfg.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -73,7 +73,6 @@ public: static const bool DFT_OVERRIDE_CLIENT_UPDATE; static const bool DFT_REPLACE_CLIENT_NAME; static const char *DFT_GENERATED_PREFIX; - static const char *DFT_QUALIFYING_SUFFIX; /// @brief Constructor /// @@ -96,7 +95,10 @@ public: /// @param replace_client_name enables replacement of the domain-name /// supplied by the client with a generated name. /// @param generated_prefix Prefix to use when generating domain-names. - /// @param qualifying_suffix Suffix to use to qualify partial domain-names. + /// @param qualifying_suffix Suffix to use to qualify partial domain-names. + /// + /// @c enable_updates is mandatory, @c qualifying_suffix is mandatory + /// when updates are enabled, other parameters are optional. /// /// @throw D2ClientError if given an invalid protocol or format. D2ClientConfig(const bool enable_updates, diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 068216fdbd..0a99038584 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -1238,6 +1238,11 @@ D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) { } // Get all parameters that are needed to create the D2ClientConfig. + + // The qualifying suffix is mandatory when updates are enabled + std::string qualifying_suffix = + string_values_->getParam("qualifying-suffix"); + IOAddress server_ip = IOAddress(string_values_->getOptionalParam("server-ip", D2ClientConfig:: @@ -1278,11 +1283,6 @@ D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) { string_values_->getOptionalParam("generated-prefix", D2ClientConfig:: DFT_GENERATED_PREFIX); - std::string qualifying_suffix = - string_values_->getOptionalParam("qualifying-suffix", - D2ClientConfig:: - DFT_QUALIFYING_SUFFIX); - bool always_include_fqdn = boolean_values_->getOptionalParam("always-include-fqdn", D2ClientConfig:: diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 53bf505ee3..9d7e55d45c 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -1120,6 +1120,7 @@ public: /// The elements currently supported are (see isc::dhcp::D2ClientConfig /// for details on each): /// -# enable-updates + /// -# qualifying-suffix /// -# server-ip /// -# server-port /// -# ncr-protocol @@ -1131,7 +1132,6 @@ public: /// -# override-client-update /// -# replace-client-name /// -# generated-prefix - /// -# qualifying-suffix /// /// @param config_id is the "item_name" for a specific member element of /// the "dns_server" specification. diff --git a/src/lib/dhcpsrv/tests/d2_client_unittest.cc b/src/lib/dhcpsrv/tests/d2_client_unittest.cc index 99f15e0b54..94338793d0 100644 --- a/src/lib/dhcpsrv/tests/d2_client_unittest.cc +++ b/src/lib/dhcpsrv/tests/d2_client_unittest.cc @@ -689,8 +689,8 @@ TEST(D2ClientMgr, generateFqdn) { ASSERT_NO_THROW(mgr.setD2ClientConfig(cfg)); // Verify names generate properly with a disabled configuration. - EXPECT_EQ("myhost-192-0-2-75.example.com.", mgr.generateFqdn(v4address,true)); - EXPECT_EQ("myhost-2001-db8--2.example.com.", mgr.generateFqdn(v6address,true)); + EXPECT_EQ("myhost-192-0-2-75.", mgr.generateFqdn(v4address,true)); + EXPECT_EQ("myhost-2001-db8--2.", mgr.generateFqdn(v6address,true)); } /// @brief Tests adjustDomainName template method with Option4ClientFqdn diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index cbd9034286..96c23b6e1e 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -1036,7 +1036,7 @@ TEST_F(ParseConfigTest, validD2Config) { /// false only. TEST_F(ParseConfigTest, validDisabledD2Config) { - // Configuration string. This contains a set of valid libraries. + // Configuration string. This defines a disabled D2 client config. std::string config_str = "{ \"dhcp-ddns\" :" " {" @@ -1062,11 +1062,14 @@ TEST_F(ParseConfigTest, validDisabledD2Config) { /// default values TEST_F(ParseConfigTest, parserDefaultsD2Config) { - // Configuration string. This contains a set of valid libraries. + // Configuration string. This defines an enabled D2 client config + // with the mandatory parameter in such a case, all other parameters + // are optional and their default values will be used. std::string config_str = "{ \"dhcp-ddns\" :" " {" - " \"enable-updates\" : true" + " \"enable-updates\" : true, " + " \"qualifying-suffix\" : \"test.suffix.\" " " }" "}"; @@ -1100,7 +1103,7 @@ TEST_F(ParseConfigTest, parserDefaultsD2Config) { d2_client_config->getReplaceClientName()); EXPECT_EQ(D2ClientConfig::DFT_GENERATED_PREFIX, d2_client_config->getGeneratedPrefix()); - EXPECT_EQ(D2ClientConfig::DFT_QUALIFYING_SUFFIX, + EXPECT_EQ("test.suffix.", d2_client_config->getQualifyingSuffix()); } @@ -1108,11 +1111,17 @@ TEST_F(ParseConfigTest, parserDefaultsD2Config) { /// @brief Check various invalid D2 client configurations. TEST_F(ParseConfigTest, invalidD2Config) { std::string invalid_configs[] = { - // Must supply at lease enable-updates + // Must supply at least enable-updates "{ \"dhcp-ddns\" :" " {" " }" "}", + // Must supply qualifying-suffix when updates are enabled + "{ \"dhcp-ddns\" :" + " {" + " \"enable-updates\" : true" + " }" + "}", // Invalid server ip value "{ \"dhcp-ddns\" :" " {" -- 2.47.3