From 4f2a05a221ac2d129e59f765f611236cd79c73ab Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Tue, 7 Dec 2021 13:44:56 +0200 Subject: [PATCH 1/2] [#2228] backport gss-tsig hook library to Kea 2.0 --- configure.ac | 2 + doc/devel/unit-tests.dox | 11 + doc/examples/ddns/gss-tsig.json | 22 +- doc/sphinx/arm/ext-gss-tsig.rst | 443 ++++++++++++++++-- src/bin/d2/Makefile.am | 4 - src/bin/d2/d2_update_mgr.h | 2 +- src/bin/d2/nc_add.cc | 99 ++-- src/bin/d2/nc_add.h | 4 +- src/bin/d2/nc_remove.cc | 103 ++-- src/bin/d2/nc_remove.h | 4 +- src/bin/d2/simple_add.cc | 66 ++- src/bin/d2/simple_add.h | 4 +- src/bin/d2/simple_remove.cc | 69 ++- src/bin/d2/simple_remove.h | 4 +- src/bin/d2/tests/Makefile.am | 22 +- src/bin/d2/tests/d2_controller_unittests.cc | 2 +- src/bin/d2/tests/d2_process_unittests.cc | 4 +- src/bin/d2/tests/d2_queue_mgr_unittests.cc | 21 +- src/bin/d2/tests/d2_update_mgr_unittests.cc | 6 +- src/bin/d2/tests/nc_add_unittests.cc | 77 +-- src/bin/d2/tests/nc_remove_unittests.cc | 77 +-- src/bin/d2/tests/simple_add_unittests.cc | 62 +-- src/bin/d2/tests/simple_remove_unittests.cc | 62 +-- src/bin/d2/tests/stats_test_utils.h | 66 --- src/lib/asiodns/io_fetch.h | 3 + src/lib/d2srv/Makefile.am | 7 +- .../d2 => lib/d2srv}/d2_update_message.cc | 4 +- src/{bin/d2 => lib/d2srv}/d2_update_message.h | 4 +- src/{bin/d2 => lib/d2srv}/d2_zone.cc | 4 +- src/{bin/d2 => lib/d2srv}/d2_zone.h | 2 +- src/{bin/d2 => lib/d2srv}/dns_client.cc | 89 ++-- src/{bin/d2 => lib/d2srv}/dns_client.h | 9 +- src/{bin/d2 => lib/d2srv}/nc_trans.cc | 8 +- src/{bin/d2 => lib/d2srv}/nc_trans.h | 8 +- src/lib/d2srv/tests/Makefile.am | 6 + .../tests/d2_update_message_unittests.cc | 4 +- .../d2srv}/tests/d2_zone_unittests.cc | 4 +- .../d2srv}/tests/dns_client_unittests.cc | 295 +++++++----- .../d2srv}/tests/nc_trans_unittests.cc | 15 +- src/lib/d2srv/testutils/Makefile.am | 23 + .../d2srv/testutils}/nc_test_utils.cc | 80 ++-- .../d2srv/testutils}/nc_test_utils.h | 47 +- .../d2srv/testutils}/stats_test_utils.cc | 19 +- src/lib/d2srv/testutils/stats_test_utils.h | 48 ++ src/lib/dhcp_ddns/ncr_udp.cc | 6 +- src/lib/stats/Makefile.am | 2 +- src/lib/stats/testutils/Makefile.am | 1 + src/lib/stats/testutils/stats_test_utils.h | 64 +++ 48 files changed, 1144 insertions(+), 844 deletions(-) delete mode 100644 src/bin/d2/tests/stats_test_utils.h rename src/{bin/d2 => lib/d2srv}/d2_update_message.cc (98%) rename src/{bin/d2 => lib/d2srv}/d2_update_message.h (99%) rename src/{bin/d2 => lib/d2srv}/d2_zone.cc (87%) rename src/{bin/d2 => lib/d2srv}/d2_zone.h (98%) rename src/{bin/d2 => lib/d2srv}/dns_client.cc (75%) rename src/{bin/d2 => lib/d2srv}/dns_client.h (97%) rename src/{bin/d2 => lib/d2srv}/nc_trans.cc (99%) rename src/{bin/d2 => lib/d2srv}/nc_trans.h (99%) rename src/{bin/d2 => lib/d2srv}/tests/d2_update_message_unittests.cc (99%) rename src/{bin/d2 => lib/d2srv}/tests/d2_zone_unittests.cc (96%) rename src/{bin/d2 => lib/d2srv}/tests/dns_client_unittests.cc (69%) rename src/{bin/d2 => lib/d2srv}/tests/nc_trans_unittests.cc (99%) create mode 100644 src/lib/d2srv/testutils/Makefile.am rename src/{bin/d2/tests => lib/d2srv/testutils}/nc_test_utils.cc (94%) rename src/{bin/d2/tests => lib/d2srv/testutils}/nc_test_utils.h (95%) rename src/{bin/d2/tests => lib/d2srv/testutils}/stats_test_utils.cc (59%) create mode 100644 src/lib/d2srv/testutils/stats_test_utils.h create mode 100644 src/lib/stats/testutils/Makefile.am create mode 100644 src/lib/stats/testutils/stats_test_utils.h diff --git a/configure.ac b/configure.ac index 7feae35180..a576580b07 100644 --- a/configure.ac +++ b/configure.ac @@ -1660,6 +1660,7 @@ AC_CONFIG_FILES([src/lib/config_backend/tests/Makefile]) AC_CONFIG_FILES([src/lib/cryptolink/Makefile]) AC_CONFIG_FILES([src/lib/cryptolink/tests/Makefile]) AC_CONFIG_FILES([src/lib/d2srv/Makefile]) +AC_CONFIG_FILES([src/lib/d2srv/testutils/Makefile]) AC_CONFIG_FILES([src/lib/d2srv/tests/Makefile]) AC_CONFIG_FILES([src/lib/database/Makefile]) AC_CONFIG_FILES([src/lib/database/tests/Makefile]) @@ -1722,6 +1723,7 @@ AC_CONFIG_FILES([src/lib/process/tests/Makefile]) AC_CONFIG_FILES([src/lib/process/testutils/Makefile]) AC_CONFIG_FILES([src/lib/stats/Makefile]) AC_CONFIG_FILES([src/lib/stats/tests/Makefile]) +AC_CONFIG_FILES([src/lib/stats/testutils/Makefile]) AC_CONFIG_FILES([src/lib/testutils/Makefile]) AC_CONFIG_FILES([src/lib/testutils/dhcp_test_lib.sh], [chmod +x src/lib/testutils/dhcp_test_lib.sh]) diff --git a/doc/devel/unit-tests.dox b/doc/devel/unit-tests.dox index f49bb49934..942164a902 100644 --- a/doc/devel/unit-tests.dox +++ b/doc/devel/unit-tests.dox @@ -354,6 +354,17 @@ local all postgres trust Use HELP for help. cqlsh> @endverbatim\n +@section unitTestsKerberos Kerberos Configuration for Unit Tests + +The GSS-TSIG hook library uses the GSS-API with Kerberos. While there are +no doubts that the hook can be safely used with a valid Kerberos configuration +in production, unit tests reported problems on some systems. + +GSS-TSIG hook unit tests use a setup inherited from bind9 with old crypto +settings which are not allowed by default Kerberos system configuration. +A simple workaround is to set the KRB5_CONFIG environment variable to +a random value that doesn't match a file (e.g. KRB5_CONFIG=). + @section writingShellScriptsAndTests Writing shell scripts and tests Shell tests are `shellcheck`ed. But there are other writing practices that are diff --git a/doc/examples/ddns/gss-tsig.json b/doc/examples/ddns/gss-tsig.json index cad5024f58..0cc324df74 100644 --- a/doc/examples/ddns/gss-tsig.json +++ b/doc/examples/ddns/gss-tsig.json @@ -28,14 +28,15 @@ }, { // This server also has an entry there, so will // use GSS-TSIG, too. - "ip-address": "192.0.2.2" + "ip-address": "192.0.2.2", + "port": 5300 } ] } ] }, - // Reverse zone: we want to update the reverse zone "2.0.192.in-addr-arpa". + // Reverse zone: we want to update the reverse zone "2.0.192.in-addr.arpa". "reverse-ddns": { "ddns-domains": @@ -58,7 +59,7 @@ // Need to add gss-tsig hook here "hooks-libraries": [ { - "library": "/opt/lib/libdhcp_gss_tsig.so", + "library": "/opt/lib/libddns_gss_tsig.so", "parameters": { // This section governs the GSS-TSIG integration. Each server // mentioned in forward-ddns and/or reverse-ddns needs to have @@ -68,10 +69,13 @@ "server-principal": "DNS/server.example.org@EXAMPLE.ORG", "client-principal": "DHCP/admin.example.org@EXAMPLE.ORG", - "client-keytab": "FILE:/etc/krb5.keytab", // toplevel only + "client-keytab": "FILE:/etc/dhcp.keytab", // toplevel only "credentials-cache": "FILE:/etc/ccache", // toplevel only - "tkey-lifetime": 3600, + "tkey-lifetime": 3600, // 1 hour + "rekey-interval": 2700, // 45 minutes + "retry-interval": 120, // 2 minutes "tkey-protocol": "TCP", + "fallback": false, // The list of GSS-TSIG capable servers "servers": [ @@ -85,8 +89,12 @@ "port": 53, "server-principal": "DNS/server1.example.org@EXAMPLE.ORG", "client-principal": "DHCP/admin1.example.org@EXAMPLE.ORG", - "tkey-lifetime": 86400, // 24h - "tkey-protocol": "TCP" + "tkey-lifetime": 7200, // 2 hours + "rekey-interval": 5400, // 90 minutes + "retry-interval": 240, // 4 minutes + "tkey-protocol": "TCP", + "fallback": true // if no key is available fallback to the + // standard behavior (vs skip this server) }, { // The second server (it has most of the parameters missing diff --git a/doc/sphinx/arm/ext-gss-tsig.rst b/doc/sphinx/arm/ext-gss-tsig.rst index af769694f1..cf718a5589 100644 --- a/doc/sphinx/arm/ext-gss-tsig.rst +++ b/doc/sphinx/arm/ext-gss-tsig.rst @@ -10,7 +10,7 @@ GSS-TSIG .. note:: The GSS-TSIG feature is considered experimental. It is possible to perform - the key exchanges and sign the DNS updates using GSS-TSIG, but some error + the TKEY exchanges and sign the DNS updates using GSS-TSIG, but some error handling and fallback scenarios are not covered yet. Use with caution. GSS-TSIG Overview @@ -19,7 +19,7 @@ GSS-TSIG Overview Kea provides a support for DNS updates, which can be protected using Transaction Signatures (or TSIG). This protection is often adequate. However some systems, in particular Active Directory (AD) on Microsoft -Windows systems, chose to adopt more complex GSS-TSIG approach that offers +Windows servers, chose to adopt more complex GSS-TSIG approach that offers additional capabilities as using negotiated dynamic keys. Kea provides the support of GSS-TSIG to protect DNS updates sent by @@ -131,7 +131,7 @@ detection, similar to this: 7. After compilation, the gss_tsig hook is available in the ``premium/src/hooks/d2/gss_tsig`` directory. It can be loaded by - the DHCP-DDNS (D2) daemon. + the Kea DHCP-DDNS (D2) daemon. The gss_tsig was developed using the MIT Kerberos 5 implementation but @@ -250,7 +250,7 @@ You will be required to retype the password: Re-enter KDC database master key to verify: -If succesfully applied, the following message will be displayed: +If successfully applied, the following message will be displayed: .. code-block:: console @@ -269,7 +269,7 @@ If succesfully applied, the following message will be displayed: Next step consists in creating the principals for the Bind9 DNS server (the service protected by the GSS-TSIG TKEY) and for the DNS client -(the Kea DDNS server). +(the Kea DHCP-DDNS server). The Bind9 DNS server principal (used for authentication) is created the following way: @@ -278,7 +278,7 @@ following way: kadmin.local -q "addprinc -randkey DNS/server.example.org" -If succesfully created, the following message will be displayed: +If successfully created, the following message will be displayed: .. code-block:: console @@ -286,36 +286,44 @@ If succesfully created, the following message will be displayed: Authenticating as principal root/admin@EXAMPLE.ORG with password. Principal "DNS/server.example.org@EXAMPLE.ORG" created. -The DNS client principal (used by the Kea DDNS server) is created the -following way (please choose your own password here): +The DNS server principal must be exported so that it can be used by the Bind 9 +DNS server. Only this principal is required and is is exported to the keytab +file with the name ``dns.keytab``. .. code-block:: console - kadmin.local -q "addprinc -pw DHCP/admin.example.org" + kadmin.local -q "ktadd -k /tmp/dns.keytab DNS/server.example.org" -If succesfully created, the following message will be displayed: +If successfully exported, the following message will be displayed: .. code-block:: console - No policy specified for DHCP/admin.example.org@EXAMPLE.ORG; defaulting to no policy Authenticating as principal root/admin@EXAMPLE.ORG with password. - Principal "DHCP/admin.example.org@EXAMPLE.ORG" created. + Entry for principal DNS/server.example.org with kvno 2, encryption type aes256-cts-hmac-sha1-96 added to keytab WRFILE:/tmp/dns.keytab. + Entry for principal DNS/server.example.org with kvno 2, encryption type aes128-cts-hmac-sha1-96 added to keytab WRFILE:/tmp/dns.keytab. -The DNS server principal must be exported so that it can be used by the Bind 9 -DNS server. Only this principal is required and is is exported to the keytab -file with the name ``dns.keytab``. +The DHCP client principal (used by the Kea DHCP-DDNS server) is created the +following way: .. code-block:: console - kadmin.local -q "ktadd -k /tmp/dns.keytab DNS/server.example.org" + kadmin.local -q "addprinc -randkey DHCP/admin.example.org" -If succesfully exported, the following message will be displayed: +If successfully created, the following message will be displayed: .. code-block:: console + No policy specified for DHCP/admin.example.org@EXAMPLE.ORG; defaulting to no policy Authenticating as principal root/admin@EXAMPLE.ORG with password. - Entry for principal DNS/server.example.org with kvno 2, encryption type aes256-cts-hmac-sha1-96 added to keytab WRFILE:/tmp/dns.keytab. - Entry for principal DNS/server.example.org with kvno 2, encryption type aes128-cts-hmac-sha1-96 added to keytab WRFILE:/tmp/dns.keytab. + Principal "DHCP/admin.example.org@EXAMPLE.ORG" created. + +The DHCP client principal must be exported so that it can be used by the +Kea DHCP-DDNS server and GSS-TSIG hook library. It is exported to the client +keytab file with the name ```dhcp.keytab```. + +.. code-block:: console + + kadmin.local -q "ktadd -k /tmp/dhcp.keytab DHCP/admin.example.org" Finally, the krb5-admin-server must be restarted: @@ -336,6 +344,7 @@ Updating the ``named.conf`` file is required: ... directory "/var/cache/bind"; dnssec-validation auto; + listen-on-v6 { any; }; tkey-gssapi-keytab "/etc/bind/dns.keytab"; }; zone "example.org" { @@ -392,6 +401,74 @@ The ``/var/lib/bind/db.example.org`` file needs to be created or updated: kdc A ${KDC_IP_ADDR} server A ${BIND9_IP_ADDR} +After any configuration change the server must be reloaded or +restarted: + +.. code-block:: console + + systemctl restart named.service + +It is possible to get status or restart logs: + +.. code-block:: console + + systemctl status named.service + journalctl -u named | tail -n 30 + +Windows Active Directory Configuration +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This sub-section is based on an Amazon AWS provided Microsoft Windows Server +2016 with Active Directory pre-installed so describes only the steps used +for GSS-TSIG deployment (for complete configuration process please refer to +Microsoft documentation or other external resources. We found `this `__ tutorial very +useful during configuration of our internal QA testing systems. + +Two Active Directory (AD) user accounts are needed: + - the first account is used to download AD information, for instance + the client key table of Kea + - the second account will be mapped to the Kea DHCP client principal + +Kea needs to know: + - the server IP address + - the domain/realm name: the domain is in lower case, the realm in upper + case, both without a final dot + - the server name + +The second account (named ``kea`` below) is used to create a Service +Principal Name (SPN): + +.. code-block:: console + + setspn -S DHCP/kea. kea + +After a shared secret key is generated and put in a key table file: + +.. code-block:: console + + ktpass -princ DHCP/kea.@ -mapuser kea +rndpass -mapop set -ptype KRB5_NT_PRINCIPAL -out dhcp.keytab + +The ``dhcp.keytab`` takes the same usage as for Unix Kerberos. + + +GSS-TSIG Troubleshooting +~~~~~~~~~~~~~~~~~~~~~~~~ + +While testing GSS-TSIG integration with Active Directory we came across +one very cryptic error: + +.. code-block:: console + + INFO [kea-dhcp-ddns.gss-tsig-hooks/4678.139690935890624] GSS_TSIG_VERIFY_FAILED GSS-TSIG verify failed: gss_verify_mic failed with GSSAPI error: + Major = 'A token had an invalid Message Integrity Check (MIC)' (393216), Minor = 'Packet was replayed in wrong direction' (100002). + +In our case problem was that Kea DDNS was trying to perform update of reverse +DNS zone while it was not configured. Easy solution was to add reverse DNS +zone similar to the one configured in Kea. To do it open `DNS Manager` choose +DNS from the list, from drop down list choose `Reverse Lookup Zones` +click `Action` and `New Zone` then follow New Zone Wizard to add new zone. + + .. _gss-tsig-using: Using GSS-TSIG @@ -407,7 +484,8 @@ An excerpt from D2 server is provided below. More examples are available in the .. code-block:: javascript :linenos: - :emphasize-lines: 57-97 + :emphasize-lines: 57-107 + { "DhcpDdns": { @@ -437,7 +515,8 @@ An excerpt from D2 server is provided below. More examples are available in the }, { // This server also has an entry there, so will // use GSS-TSIG, too. - "ip-address": "192.0.2.2" + "ip-address": "192.0.2.2", + "port": 5300 } ] } @@ -467,7 +546,7 @@ An excerpt from D2 server is provided below. More examples are available in the // Need to add gss-tsig hook here "hooks-libraries": [ { - "library": "/opt/lib/libdhcp_gss_tsig.so", + "library": "/opt/lib/libddns_gss_tsig.so", "parameters": { // This section governs the GSS-TSIG integration. Each server // mentioned in forward-ddns and/or reverse-ddns needs to have @@ -477,10 +556,13 @@ An excerpt from D2 server is provided below. More examples are available in the "server-principal": "DNS/server.example.org@EXAMPLE.ORG", "client-principal": "DHCP/admin.example.org@EXAMPLE.ORG", - "client-keytab": "FILE:/etc/krb5.keytab", // toplevel only + "client-keytab": "FILE:/etc/dhcp.keytab", // toplevel only "credentials-cache": "FILE:/etc/ccache", // toplevel only - "tkey-lifetime": 3600, + "tkey-lifetime": 3600, // 1 hour + "rekey-interval": 2700, // 45 minutes + "retry-interval": 120, // 2 minutes "tkey-protocol": "TCP", + "fallback": false, // The list of GSS-TSIG capable servers "servers": [ @@ -494,8 +576,12 @@ An excerpt from D2 server is provided below. More examples are available in the "port": 53, "server-principal": "DNS/server1.example.org@EXAMPLE.ORG", "client-principal": "DHCP/admin1.example.org@EXAMPLE.ORG", - "tkey-lifetime": 86400, // 24h - "tkey-protocol": "TCP" + "tkey-lifetime": 7200, // 2 hours + "rekey-interval": 5400, // 90 minutes + "retry-interval": 240, // 4 minutes + "tkey-protocol": "TCP", + "fallback": true // if no key is available fallback to the + // standard behavior (vs skip this server) }, { // The second server (it has most of the parameters missing @@ -523,23 +609,89 @@ specified, the default of 53 is assumed. This is similar to basic mode with no authentication or authentication done using TSIG keys, with the exception that static TSIG keys are not referenced by name. -Second, the ``libdhcp_gss_tsig.so`` library has to be specified on the -``hooks-libraries`` list. This hook takes many parameters. The most -important one is `servers`, which is a list of GSS-TSIG capable -servers. If there are several servers and they share some -characteristics, the values can be specified in `parameters` scope as -defaults. In the example above, the defaults that apply to all servers -unless otherwise specified on per server scope, are defined in lines -63 through 68. The defaults can be skipped if there is only one server +Second, the ``libddns_gss_tsig.so`` library has to be specified on the +``hooks-libraries`` list. This hook takes many parameters. The most important +one is ``servers``, which is a list of GSS-TSIG capable servers. If there are +several servers and they share some characteristics, the values can be specified +in ``parameters`` scope as defaults. In the example above, the defaults that apply +to all servers unless otherwise specified on per server scope, are defined in +lines 63 through 68. The defaults can be skipped if there is only one server defined or all servers have different values. -The parameters have the following meaning: +.. table:: List of available parameters + + +-------------------+----------+---------+---------------------+--------------------------------+ + | Name | Scope | Type | Default value | Description | + | | | | | | + +===================+==========+=========+=====================+================================+ + | client-keytab | global / | string | empty | the Kerberos **client** key | + | | server | | | table | + +-------------------+----------+---------+---------------------+--------------------------------+ + | credentials-cache | global / | string | empty | the Kerberos credentials cache | + | | server | | | | + +-------------------+----------+---------+---------------------+--------------------------------+ + | server-principal | global / | string | empty | the Kerberos principal name of | + | | server | | | the DNS server that will | + | | | | | receive updates | + +-------------------+----------+---------+---------------------+--------------------------------+ + | client-principal | global / | string | empty | the Kerberos principal name of | + | | server | | | the Kea D2 service | + +-------------------+----------+---------+---------------------+--------------------------------+ + | tkey-protocol | global / | string | "TCP" | the protocol used to establish | + | | server | "TCP" / | | the security context with the | + | | | "UDP" | | DNS servers | + +-------------------+----------+---------+---------------------+--------------------------------+ + | tkey-lifetime | global / | uint32 | | 3600 seconds | the lifetime of GSS-TSIG keys | + | | server | | | ( 1 hour ) | | + +-------------------+----------+---------+---------------------+--------------------------------+ + | rekey-interval | global / | uint32 | | 2700 seconds | the time interval the keys are | + | | server | | | ( 45 minutes ) | checked for rekeying | + +-------------------+----------+---------+---------------------+--------------------------------+ + | retry-interval | global / | uint32 | | 120 seconds | the time interval to retry to | + | | server | | | ( 2 minutes ) | create a key if any error | + | | | | | occurred previously | + +-------------------+----------+---------+---------------------+--------------------------------+ + | fallback | global / | true / | false | the behavior to fallback to | + | | server | false | | non GSS-TSIG when GSS-TSIG | + | | | | | should be used but no GSS-TSIG | + | | | | | key is available. | + +-------------------+----------+---------+---------------------+--------------------------------+ + | exchange-timeout | global / | uint32 | | 3000 milliseconds | the time used to wait for the | + | | server | | | ( 3 seconds ) | GSS-TSIG TKEY exchange to | + | | | | | finish before it timeouts | + +-------------------+----------+---------+---------------------+--------------------------------+ + | user-context | global / | string | empty | the user provided data in JSON | + | | server | | | format (will not be used by | + | | | | | the GSS-TSIG hook) | + +-------------------+----------+---------+---------------------+--------------------------------+ + | comment | global / | string | empty | ignored | + | | server | | | | + +-------------------+----------+---------+---------------------+--------------------------------+ + | id | server | string | empty | identifier to a DNS server | + | | | | | (required) | + +-------------------+----------+---------+---------------------+--------------------------------+ + | domain-names | server | list of | empty | the many to one relationship | + | | | strings | | between D2 DNS servers and | + | | | | | GSS-TSIG DNS servers | + +-------------------+----------+---------+---------------------+--------------------------------+ + | ip-address | server | IPv4 / | empty | the IP address at which the | + | | | IPv6 | | GSS-TSIG DNS server listens | + | | | address | | for DDNS and TKEY requests | + | | | | | (required) | + +-------------------+----------+---------+---------------------+--------------------------------+ + | port | server | uint16 | 53 | the DNS transport port at | + | | | | | which the GSS-TSIG DNS server | + | | | | | listens for DDNS and TKEY | + | | | | | requests | + +-------------------+----------+---------+---------------------+--------------------------------+ + +The global parameters are described below: - ``client-keytab`` specifies the Kerberos **client** key table. For instance, ``FILE:`` can be used to point to a specific file. This parameter can be specified only once, in the parameters scope, and is the equivalent of setting the ``KRB5_CLIENT_KTNAME`` environment - variable. + variable. The empty value is silently ignored. - ``credentials-cache`` specifies the Kerberos credentials cache. For instance ``FILE:`` can be used to point to a file or @@ -547,7 +699,7 @@ The parameters have the following meaning: ``DIR:``. This parameter can be specified only once, in the parameters scope, and is the equivalent of setting the ``KRB5CCNAME`` environment - variable. + variable. The empty value is silently ignored. - ``server-principal`` is the Kerberos principal name of the DNS server that will receive updates. In plain words, this is the @@ -564,7 +716,34 @@ The parameters have the following meaning: values are TCP (the default) and UDP. - ``tkey-lifetime`` determines the lifetime of GSS-TSIG keys in the - TKEY protocol, expressed in seconds. Default value is 3600 (one hour). + TKEY protocol. The value must be greater than the ``rekey-interval`` + value. It is expressed in seconds and it default to 3600 seconds + (one hour) if not specified. + +- ``rekey-interval`` governs the time interval the keys for each configured + server are checked for rekeying, i.e. a new key is created to replace the + current usable one when its age is greater than the ``rekey-interval`` value. + The value must be smaller than the ``tkey-lifetime`` value (it is recommend + between 50% and 80% of the ``tkey-lifetime`` value). It is expressed in + seconds and it defaults to 2700 seconds (45 minutes, 75% of one hour) if not + specified. + +- ``retry-interval`` governs the time interval to retry to create a key if any + error occurred previously for any configured server. The value must be smaller + than the ``rekey-interval`` value, and should be at most 1/3 of the difference + between ``tkey-lifetime`` and ``rekey-interval``. It is expressed in seconds + and it defaults to 120 seconds (2 minutes) if not specified. + +- ``fallback`` governs the behavior when GSS-TSIG should be used (a + matching DNS server is configured) but no GSS-TSIG key is available. + If configured to false (the default) this server is skipped, if + configured to true the DNS server is ignored and the DNS update + is sent with the configured DHCP-DDNS protection e.g. TSIG key or + without any protection when none was configured. + +- ``exchange-timeout`` governs the time used to wait for the GSS-TSIG TKEY + exchange to finish before it timeouts. It is expressed in milliseconds and it + defaults to 3000 milliseconds (3 seconds) if not specified. - ``user-context`` is an optional parameter (see :ref:`user-context` for a general description of user contexts in Kea). @@ -573,7 +752,7 @@ The parameters have the following meaning: - ``servers`` specifies the list of DNS servers where GSS-TSIG is enabled. -The server map parameters are: +The server map parameters are described below: - ``id`` assigns an identifier to a DNS server. It is used for statistics and commands. It is required, must be not empty and unique. @@ -607,20 +786,139 @@ The server map parameters are: - ``tkey-lifetime`` determines the lifetime of GSS-TSIG keys in the TKEY protocol for the DNS server. The TKEY lifetime parameter per server - takes precedence. Default and supported values are the same as for - the global level parameter. + takes precedence. Default and supported values are the same as for the + global level parameter. + +- ``rekey-interval`` governs the time interval the keys for this particular + server are checked for rekeying, i.e. a new key is created to replace the + current usable one when its age is greater than the ``rekey-interval`` value. + The value must be smaller than the ``tkey-lifetime`` value (it is recommend + between 50% and 80% of the ``tkey-lifetime`` value). The rekey interval + parameter per server takes precedence. Default and supported values are the + same as for the global level parameter. + +- ``retry-interval`` governs the time interval to retry to create a key if any + error occurred previously for this particular server. The value must be + smaller than the ``rekey-interval`` value, and should be at most 1/3 of the + difference between ``tkey-lifetime`` and ``rekey-interval``. The retry + interval parameter per server takes precedence. Default and supported values + are the same as for the global level parameter. + +- ``fallback`` governs the behavior when GSS-TSIG should be used (a + matching DNS server is configured) but no GSS-TSIG key is available. + The fallback parameter per server takes precedence. Default and + supported values are the same as for the global level parameter. + +- ``exchange-timeout`` governs the time used to wait for the GSS-TSIG TKEY + exchange to finish before it timeouts. The exchange timeout parameter per + server takes precedence. Default and supported values are the same as for the + global level parameter. - ``user-context`` is an optional parameter (see :ref:`user-context` for a general description of user contexts in Kea). - ``comment`` is allowed but currently ignored. -.. _command-gss-tsig: + +GSS-TSIG Automatic Key Removal +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The server will periodically delete keys which expired more than 3 times the +maximum key lifetime (``tkey-lifetime`` parameter). +The user has the option to purge keys on demand by using ``gss-tsig-purge-all`` +command (see :ref:`command-gss-tsig-purge-all`) or ``gss-tsig-purge`` command +(see :ref:`command-gss-tsig-purge`). + + +GSS-TSIG Configuration for Deployment +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +When using the Kerberos 5 and Bind9 setup of :ref:`gss-tsig-deployment` +the local resolver must point to the Bind9 named server address and +local Kerberos be configured by putting in the ``krb5.conf`` file: + +.. code-block:: ini + + [libdefaults] + default_realm = EXAMPLE.ORG + kdc_timesync = 1 + ccache_type = 4 + forwardable = true + proxiable = true + [realms] + EXAMPLE.ORG = { + kdc = kdc.example.org + admin_server = kdc.example.org + } + +With Windows AD the DNS service is provided by AD. AD also provides +the Kerberos service and the ``krb5.conf`` file becomes: + +.. code-block:: ini + + [libdefaults] + default_realm = + kdc_timesync = 1 + ccache_type = 4 + forwardable = true + proxiable = true + [realms] + ${REALM} = { + kdc = + admin_server = + } + +Even when the GSS-API library can use the secret from the client key +table it is far better to get and cache credentials. + +This can be done manually by: + +.. code-block:: console + + kinit -k -t /tmp/dhcp.keytab DHCP/admin.example.org + +or when using AD: + +.. code-block:: console + + kinit -k -t /tmp/dhcp.keytab DHCP/kea. + +The credential cache can be displayed using ``klist``. + +In production it is better to rely on a Kerberos Credential Manager as +the System Security Services Daemon (``sssd``). + +The server principal will be "DNS/server.example.org@EXAMPLE.ORG¨ or +for AD "DNS/.@". + +.. _stats-gss-tsig: + +GSS-TSIG Statistics +------------------- + +The GSS-TSIG hook library introduces new statistics at global and +per DNS server levels: + +- ``gss-tsig-key-created`` - number of created GSS-TSIG keys +- ``tkey-sent`` - sent TKEY exchange initial requests +- ``tkey-success`` - TKEY exchanges which completed with a success +- ``tkey-timeout`` - TKEY exchanges which completed on timeout +- ``tkey-error`` - TKEY exchanges which completed with an error other than + timeout + +The relationship between keys and DNS servers are very different between +the D2 code and static TSIG keys, and GSS-TSIG keys and DNS servers: + + - a static TSIG key can be shared between many DNS servers + - a GSS-TSIG key is used only by one DNS server inside a dedicated + set of keys. + +.. _commands-gss-tsig: GSS-TSIG Commands ----------------- -The GSS-TSIG hook library supports some commands. +The GSS-TSIG hook library supports some commands which are described below. .. _command-gss-tsig-get-all: @@ -899,3 +1197,62 @@ An example response informing about 2 GSS-TSIG keys for server 'foo' being purge "result": 0, "text": "2 purged keys for GSS-TSIG server[foo]" } + +.. _command-gss-tsig-rekey-all: + +The gss-tsig-rekey-all Command +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The command rekeys i.e. unconditionally creates new GSS-TSIG keys for +all DNS servers. + +An example command invocation looks like this: + +.. code-block:: json + + { + "command": "gss-tsig-rekey-all" + } + +An example response informing that a rekey was scheduled: + +.. code-block:: json + + { + "result": 0, + "text": "rekeyed" + } + +This command should be use for instance when the DHCP-DDNS server is +reconnected to the network. + +.. _command-gss-tsig-rekey: + +The gss-tsig-rekey Command +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The command rekeys i.e. unconditionally creates new GSS-TSIG keys for +a specified DNS server. + +An example command invocation looks like this: + +.. code-block:: json + + { + "command": "gss-tsig-purge", + "arguments": { + "server-id": "foo" + } + } + +An example response informing that a rekey was scheduled: + +.. code-block:: json + + { + "result": 0, + "text": "GSS-TSIG server[foo] rekeyed" + } + +A typical usage of this command is when a DNS server was rebooted so +existing GSS-TSIG keys shared with this server can no longer be used. diff --git a/src/bin/d2/Makefile.am b/src/bin/d2/Makefile.am index 9a716e42e4..53282977f7 100644 --- a/src/bin/d2/Makefile.am +++ b/src/bin/d2/Makefile.am @@ -33,13 +33,9 @@ libd2_la_SOURCES += d2_process.cc d2_process.h libd2_la_SOURCES += d2_lexer.ll location.hh libd2_la_SOURCES += d2_parser.cc d2_parser.h libd2_la_SOURCES += d2_queue_mgr.cc d2_queue_mgr.h -libd2_la_SOURCES += d2_update_message.cc d2_update_message.h libd2_la_SOURCES += d2_update_mgr.cc d2_update_mgr.h -libd2_la_SOURCES += d2_zone.cc d2_zone.h -libd2_la_SOURCES += dns_client.cc dns_client.h libd2_la_SOURCES += nc_add.cc nc_add.h libd2_la_SOURCES += nc_remove.cc nc_remove.h -libd2_la_SOURCES += nc_trans.cc nc_trans.h libd2_la_SOURCES += d2_controller.cc d2_controller.h libd2_la_SOURCES += parser_context.cc parser_context.h parser_context_decl.h libd2_la_SOURCES += simple_add.cc simple_add.h diff --git a/src/bin/d2/d2_update_mgr.h b/src/bin/d2/d2_update_mgr.h index 2ff8765bcd..593cfd1167 100644 --- a/src/bin/d2/d2_update_mgr.h +++ b/src/bin/d2/d2_update_mgr.h @@ -11,7 +11,7 @@ #include #include -#include +#include #include #include #include diff --git a/src/bin/d2/nc_add.cc b/src/bin/d2/nc_add.cc index 4a1661ac77..0d517cb4d5 100644 --- a/src/bin/d2/nc_add.cc +++ b/src/bin/d2/nc_add.cc @@ -172,28 +172,25 @@ NameAddTransaction::selectingFwdServerHandler() { void NameAddTransaction::addingFwdAddrsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildAddFwdAddressRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_FORWARD_ADD_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildAddFwdAddressRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_FORWARD_ADD_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets @@ -287,29 +284,26 @@ NameAddTransaction::addingFwdAddrsHandler() { void NameAddTransaction::replacingFwdAddrsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case FQDN_IN_USE_EVT: case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildReplaceFwdAddressRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_FORWARD_REPLACE_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildReplaceFwdAddressRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_FORWARD_REPLACE_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets @@ -439,28 +433,25 @@ NameAddTransaction::selectingRevServerHandler() { void NameAddTransaction::replacingRevPtrsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildReplaceRevPtrsRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_REVERSE_REPLACE_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildReplaceRevPtrsRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_REVERSE_REPLACE_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets diff --git a/src/bin/d2/nc_add.h b/src/bin/d2/nc_add.h index 0f9929abd1..6bb8ce311f 100644 --- a/src/bin/d2/nc_add.h +++ b/src/bin/d2/nc_add.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -9,7 +9,7 @@ /// @file nc_add.h This file defines the class NameAddTransaction. -#include +#include #include namespace isc { diff --git a/src/bin/d2/nc_remove.cc b/src/bin/d2/nc_remove.cc index 8c158598d3..cd9c6794bf 100644 --- a/src/bin/d2/nc_remove.cc +++ b/src/bin/d2/nc_remove.cc @@ -176,29 +176,26 @@ NameRemoveTransaction::selectingFwdServerHandler() { void NameRemoveTransaction::removingFwdAddrsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildRemoveFwdAddressRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, - DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildRemoveFwdAddressRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, + DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets @@ -286,30 +283,27 @@ NameRemoveTransaction::removingFwdAddrsHandler() { void NameRemoveTransaction::removingFwdRRsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case UPDATE_OK_EVT: case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildRemoveFwdRRsRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, - DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildRemoveFwdRRsRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, + DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets @@ -446,28 +440,25 @@ NameRemoveTransaction::selectingRevServerHandler() { void NameRemoveTransaction::removingRevPtrsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildRemoveRevPtrsRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildRemoveRevPtrsRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets diff --git a/src/bin/d2/nc_remove.h b/src/bin/d2/nc_remove.h index 1584cb0025..672a3d9841 100644 --- a/src/bin/d2/nc_remove.h +++ b/src/bin/d2/nc_remove.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015,2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -9,7 +9,7 @@ /// @file nc_remove.h This file defines the class NameRemoveTransaction. -#include +#include namespace isc { namespace d2 { diff --git a/src/bin/d2/simple_add.cc b/src/bin/d2/simple_add.cc index 7c7b0cf2c4..c1814c77b4 100644 --- a/src/bin/d2/simple_add.cc +++ b/src/bin/d2/simple_add.cc @@ -167,28 +167,25 @@ SimpleAddTransaction::selectingFwdServerHandler() { void SimpleAddTransaction::replacingFwdAddrsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildReplaceFwdAddressRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_FORWARD_ADD_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildReplaceFwdAddressRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_FORWARD_ADD_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets @@ -306,28 +303,25 @@ SimpleAddTransaction::selectingRevServerHandler() { void SimpleAddTransaction::replacingRevPtrsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildReplaceRevPtrsRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_REVERSE_REPLACE_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildReplaceRevPtrsRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_REVERSE_REPLACE_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets diff --git a/src/bin/d2/simple_add.h b/src/bin/d2/simple_add.h index 7bf7b6c605..bf617a50a7 100644 --- a/src/bin/d2/simple_add.h +++ b/src/bin/d2/simple_add.h @@ -1,4 +1,4 @@ -// Copyright (C) 2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2020-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -9,7 +9,7 @@ /// @file nc_add.h This file defines the class SimpleAddTransaction. -#include +#include #include namespace isc { diff --git a/src/bin/d2/simple_remove.cc b/src/bin/d2/simple_remove.cc index e780080d71..457afe7125 100644 --- a/src/bin/d2/simple_remove.cc +++ b/src/bin/d2/simple_remove.cc @@ -170,30 +170,27 @@ SimpleRemoveTransaction::selectingFwdServerHandler() { void SimpleRemoveTransaction::removingFwdRRsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case UPDATE_OK_EVT: case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildRemoveFwdRRsRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, - DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildRemoveFwdRRsRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, + DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets @@ -286,7 +283,6 @@ SimpleRemoveTransaction::removingFwdRRsHandler() { } } - void SimpleRemoveTransaction::selectingRevServerHandler() { switch(getNextEvent()) { @@ -320,28 +316,25 @@ SimpleRemoveTransaction::selectingRevServerHandler() { void SimpleRemoveTransaction::removingRevPtrsHandler() { if (doOnEntry()) { - // Clear the request on initial transition. This allows us to reuse - // the request on retries if necessary. - clearDnsUpdateRequest(); + // Clear the update attempts count on initial transition. + clearUpdateAttempts(); } switch(getNextEvent()) { case SERVER_SELECTED_EVT: - if (!getDnsUpdateRequest()) { - // Request hasn't been constructed yet, so build it. - try { - buildRemoveRevPtrsRequest(); - } catch (const std::exception& ex) { - // While unlikely, the build might fail if we have invalid - // data. Should that be the case, we need to fail the - // transaction. - LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE) - .arg(getRequestId()) - .arg(getNcr()->toText()) - .arg(ex.what()); - transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); - break; - } + try { + clearDnsUpdateRequest(); + buildRemoveRevPtrsRequest(); + } catch (const std::exception& ex) { + // While unlikely, the build might fail if we have invalid + // data. Should that be the case, we need to fail the + // transaction. + LOG_ERROR(d2_to_dns_logger, DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE) + .arg(getRequestId()) + .arg(getNcr()->toText()) + .arg(ex.what()); + transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT); + break; } // Call sendUpdate() to initiate the async send. Note it also sets diff --git a/src/bin/d2/simple_remove.h b/src/bin/d2/simple_remove.h index ce6d525145..957cb138a7 100644 --- a/src/bin/d2/simple_remove.h +++ b/src/bin/d2/simple_remove.h @@ -1,4 +1,4 @@ -// Copyright (C) 2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2020-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -9,7 +9,7 @@ /// @file nc_remove.h This file defines the class SimpleRemoveTransaction. -#include +#include namespace isc { namespace d2 { diff --git a/src/bin/d2/tests/Makefile.am b/src/bin/d2/tests/Makefile.am index 820c4900c4..fe9d60f0f0 100644 --- a/src/bin/d2/tests/Makefile.am +++ b/src/bin/d2/tests/Makefile.am @@ -17,7 +17,11 @@ TESTS = $(SHTESTS) # As with every file generated by ./configure, clean them up when running # "make distclean", but not on "make clean". -DISTCLEANFILES = $(SHTESTS) +DISTCLEANFILES = $(SHTESTS) +DISTCLEANFILES += d2_process_tests.sh +DISTCLEANFILES += test_data_files_config.h +DISTCLEANFILES += test_callout_libraries.h +DISTCLEANFILES += test_configured_libraries.h # Don't install shell tests. noinst_SCRIPTS = $(SHTESTS) @@ -45,14 +49,9 @@ d2_unittests_SOURCES = d2_unittests.cc d2_unittests_SOURCES += d2_process_unittests.cc d2_unittests_SOURCES += d2_cfg_mgr_unittests.cc d2_unittests_SOURCES += d2_queue_mgr_unittests.cc -d2_unittests_SOURCES += d2_update_message_unittests.cc d2_unittests_SOURCES += d2_update_mgr_unittests.cc -d2_unittests_SOURCES += d2_zone_unittests.cc -d2_unittests_SOURCES += dns_client_unittests.cc d2_unittests_SOURCES += nc_add_unittests.cc d2_unittests_SOURCES += nc_remove_unittests.cc -d2_unittests_SOURCES += nc_test_utils.cc nc_test_utils.h -d2_unittests_SOURCES += nc_trans_unittests.cc d2_unittests_SOURCES += d2_controller_unittests.cc d2_unittests_SOURCES += d2_simple_parser_unittest.cc d2_unittests_SOURCES += parser_unittest.cc parser_unittest.h @@ -60,7 +59,6 @@ d2_unittests_SOURCES += get_config_unittest.cc d2_unittests_SOURCES += d2_command_unittest.cc d2_unittests_SOURCES += simple_add_unittests.cc d2_unittests_SOURCES += simple_remove_unittests.cc -d2_unittests_SOURCES += stats_test_utils.cc stats_test_utils.h d2_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) d2_unittests_LDFLAGS = $(AM_LDFLAGS) $(CRYPTO_LDFLAGS) @@ -75,7 +73,8 @@ d2_unittests_LDFLAGS += $(CQL_LIBS) endif d2_unittests_LDFLAGS += $(GTEST_LDFLAGS) -d2_unittests_LDADD = $(top_builddir)/src/bin/d2/libd2.la +d2_unittests_LDADD = $(top_builddir)/src/bin/d2/libd2.la +d2_unittests_LDADD += $(top_builddir)/src/lib/d2srv/testutils/libd2srvtest.la d2_unittests_LDADD += $(top_builddir)/src/lib/d2srv/libkea-d2srv.la d2_unittests_LDADD += $(top_builddir)/src/lib/process/testutils/libprocesstest.la d2_unittests_LDADD += $(top_builddir)/src/lib/cfgrpt/libcfgrpt.la @@ -132,13 +131,6 @@ nodist_d2_unittests_SOURCES += test_configured_libraries.h # Run C++ tests on "make check". TESTS += $(PROGRAM_TESTS) -# As with every file generated by ./configure, clean them up when running -# "make distclean", but not on "make clean". -DISTCLEANFILES += d2_process_tests.sh -DISTCLEANFILES += test_data_files_config.h -DISTCLEANFILES += test_callout_libraries.h -DISTCLEANFILES += test_configured_libraries.h - # Don't install C++ tests. noinst_PROGRAMS = $(PROGRAM_TESTS) diff --git a/src/bin/d2/tests/d2_controller_unittests.cc b/src/bin/d2/tests/d2_controller_unittests.cc index 2a4a7dd738..0c6e5ecf90 100644 --- a/src/bin/d2/tests/d2_controller_unittests.cc +++ b/src/bin/d2/tests/d2_controller_unittests.cc @@ -8,9 +8,9 @@ #include #include +#include #include #include -#include #include #include diff --git a/src/bin/d2/tests/d2_process_unittests.cc b/src/bin/d2/tests/d2_process_unittests.cc index 992c533fb6..4dda27551d 100644 --- a/src/bin/d2/tests/d2_process_unittests.cc +++ b/src/bin/d2/tests/d2_process_unittests.cc @@ -8,11 +8,11 @@ #include #include +#include #include +#include #include #include -#include -#include #include #include diff --git a/src/bin/d2/tests/d2_queue_mgr_unittests.cc b/src/bin/d2/tests/d2_queue_mgr_unittests.cc index a00de29f4b..42177e6f6a 100644 --- a/src/bin/d2/tests/d2_queue_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_queue_mgr_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -21,6 +22,7 @@ using namespace std; using namespace isc; using namespace isc::dhcp_ddns; using namespace isc::d2; +using namespace isc::d2::test; namespace { @@ -203,7 +205,7 @@ bool checkSendVsReceived(NameChangeRequestPtr sent_ncr, /// @brief Text fixture that allows testing a listener and sender together /// It derives from both the receive and send handler classes and contains /// and instance of UDP listener and UDP sender. -class QueueMgrUDPTest : public virtual ::testing::Test, +class QueueMgrUDPTest : public virtual ::testing::Test, public D2StatTest, NameChangeSender::RequestSendHandler { public: asiolink::IOServicePtr io_service_; @@ -384,6 +386,13 @@ TEST_F (QueueMgrUDPTest, liveFeed) { EXPECT_EQ(0, queue_mgr_->getQueueSize()); } + StatMap stats_ncr = { + { "ncr-received", 3}, + { "ncr-invalid", 0}, + { "ncr-error", 0} + }; + checkStats(stats_ncr); + // Iterate over the list of requests, sending and receiving // each one. Allow them to accumulate in the queue. for (int i = 0; i < VALID_MSG_CNT; i++) { @@ -397,6 +406,13 @@ TEST_F (QueueMgrUDPTest, liveFeed) { EXPECT_EQ(i+1, queue_mgr_->getQueueSize()); } + StatMap stats_ncr_new = { + { "ncr-received", 6}, + { "ncr-invalid", 0}, + { "ncr-error", 0} + }; + checkStats(stats_ncr_new); + // Verify that the queue is at max capacity. EXPECT_EQ(queue_mgr_->getMaxQueueSize(), queue_mgr_->getQueueSize()); @@ -428,7 +444,6 @@ TEST_F (QueueMgrUDPTest, liveFeed) { EXPECT_NO_THROW(queue_mgr_->clearQueue()); EXPECT_EQ(0, queue_mgr_->getQueueSize()); - // Verify that we can again receive requests. // Send should be fine. ASSERT_NO_THROW(sender_->sendRequest(send_ncr)); diff --git a/src/bin/d2/tests/d2_update_mgr_unittests.cc b/src/bin/d2/tests/d2_update_mgr_unittests.cc index a89de3e273..d79b4538f0 100644 --- a/src/bin/d2/tests/d2_update_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_update_mgr_unittests.cc @@ -7,8 +7,8 @@ #include #include +#include #include -#include #include #include #include @@ -232,8 +232,8 @@ public: // value). This is roughly ten times the number for the longest // test (currently, multiTransactionTimeout). if (passes > max_passes) { - ADD_FAILURE() << "processALL failed, too many passes: " - << passes << ", total handlers executed: " << handlers; + FAIL() << "processALL failed, too many passes: " + << passes << ", total handlers executed: " << handlers; } } } diff --git a/src/bin/d2/tests/nc_add_unittests.cc b/src/bin/d2/tests/nc_add_unittests.cc index 0ef94ddf6b..3f36b46737 100644 --- a/src/bin/d2/tests/nc_add_unittests.cc +++ b/src/bin/d2/tests/nc_add_unittests.cc @@ -9,8 +9,8 @@ #include #include #include +#include #include -#include #include @@ -714,24 +714,9 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run addingFwdAddrsHandler to send the request. EXPECT_NO_THROW(name_add->addingFwdAddrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_add->setDnsUpdateStatus(DNSClient::TIMEOUT); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1059,24 +1044,9 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run replacingFwdAddrsHandler to send the request. EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_add->setDnsUpdateStatus(DNSClient::TIMEOUT); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1127,24 +1097,9 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_CorruptResponse) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run replacingFwdAddrsHandler to send the request. EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a corrupt server response. name_add->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1371,24 +1326,9 @@ TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run replacingRevPtrsHandler to send the request. EXPECT_NO_THROW(name_add->replacingRevPtrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_add->setDnsUpdateStatus(DNSClient::TIMEOUT); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1438,24 +1378,9 @@ TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_CorruptResponse) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run replacingRevPtrsHandler to send the request. EXPECT_NO_THROW(name_add->replacingRevPtrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server corrupt response. name_add->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); diff --git a/src/bin/d2/tests/nc_remove_unittests.cc b/src/bin/d2/tests/nc_remove_unittests.cc index d335e101a5..4373811f17 100644 --- a/src/bin/d2/tests/nc_remove_unittests.cc +++ b/src/bin/d2/tests/nc_remove_unittests.cc @@ -9,8 +9,8 @@ #include #include #include +#include #include -#include #include @@ -692,24 +692,9 @@ TEST_F(NameRemoveTransactionTest, removingFwdAddrsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingFwdAddrsHandler to send the request. EXPECT_NO_THROW(name_remove->removingFwdAddrsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_remove->setDnsUpdateStatus(DNSClient::TIMEOUT); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1045,24 +1030,9 @@ TEST_F(NameRemoveTransactionTest, removingFwdRRsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingFwdRRsHandler to send the request. EXPECT_NO_THROW(name_remove->removingFwdRRsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_remove->setDnsUpdateStatus(DNSClient::TIMEOUT); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1115,24 +1085,9 @@ TEST_F(NameRemoveTransactionTest, removingFwdRRsHandler_InvalidResponse) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingFwdRRsHandler to send the request. EXPECT_NO_THROW(name_remove->removingFwdRRsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a corrupt server response. name_remove->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1419,24 +1374,9 @@ TEST_F(NameRemoveTransactionTest, removingRevPtrsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingRevPtrsHandler to send the request. EXPECT_NO_THROW(name_remove->removingRevPtrsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_remove->setDnsUpdateStatus(DNSClient::TIMEOUT); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1488,24 +1428,9 @@ TEST_F(NameRemoveTransactionTest, removingRevPtrsHandler_CorruptResponse) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingRevPtrsHandler to send the request. EXPECT_NO_THROW(name_remove->removingRevPtrsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server corrupt response. name_remove->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); diff --git a/src/bin/d2/tests/simple_add_unittests.cc b/src/bin/d2/tests/simple_add_unittests.cc index 5d8f4d5653..c47a2b6f4b 100644 --- a/src/bin/d2/tests/simple_add_unittests.cc +++ b/src/bin/d2/tests/simple_add_unittests.cc @@ -9,8 +9,8 @@ #include #include #include +#include #include -#include #include @@ -598,24 +598,9 @@ TEST_F(SimpleAddTransactionTest, replacingFwdAddrsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run replacingFwdAddrsHandler to send the request. EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_add->setDnsUpdateStatus(DNSClient::TIMEOUT); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -662,24 +647,9 @@ TEST_F(SimpleAddTransactionTest, replacingFwdAddrsHandler_CorruptResponse) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run replacingFwdAddrsHandler to send the request. EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a corrupt server response. name_add->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -886,24 +856,9 @@ TEST_F(SimpleAddTransactionTest, replacingRevPtrsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run replacingRevPtrsHandler to send the request. EXPECT_NO_THROW(name_add->replacingRevPtrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_add->setDnsUpdateStatus(DNSClient::TIMEOUT); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -949,24 +904,9 @@ TEST_F(SimpleAddTransactionTest, replacingRevPtrsHandler_CorruptResponse) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_add->getDnsUpdateRequest(); - // Run replacingRevPtrsHandler to send the request. EXPECT_NO_THROW(name_add->replacingRevPtrsHandler()); - const D2UpdateMessagePtr curr_msg = name_add->getDnsUpdateRequest(); - if (i == 1) { - // First time out we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server corrupt response. name_add->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE); name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); diff --git a/src/bin/d2/tests/simple_remove_unittests.cc b/src/bin/d2/tests/simple_remove_unittests.cc index 64ce75834a..1d53400fc9 100644 --- a/src/bin/d2/tests/simple_remove_unittests.cc +++ b/src/bin/d2/tests/simple_remove_unittests.cc @@ -9,8 +9,8 @@ #include #include #include +#include #include -#include #include @@ -605,24 +605,9 @@ TEST_F(SimpleRemoveTransactionTest, removingFwdRRsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingFwdRRsHandler to send the request. EXPECT_NO_THROW(name_remove->removingFwdRRsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_remove->setDnsUpdateStatus(DNSClient::TIMEOUT); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -669,24 +654,9 @@ TEST_F(SimpleRemoveTransactionTest, removingFwdRRsHandler_InvalidResponse) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingFwdRRsHandler to send the request. EXPECT_NO_THROW(name_remove->removingFwdRRsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a corrupt server response. name_remove->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -945,24 +915,9 @@ TEST_F(SimpleRemoveTransactionTest, removingRevPtrsHandler_Timeout) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingRevPtrsHandler to send the request. EXPECT_NO_THROW(name_remove->removingRevPtrsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server IO timeout. name_remove->setDnsUpdateStatus(DNSClient::TIMEOUT); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); @@ -1009,24 +964,9 @@ TEST_F(SimpleRemoveTransactionTest, removingRevPtrsHandler_CorruptResponse) { // and then transition to selecting a new server. int max_tries = NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; for (int i = 1; i <= max_tries; ++i) { - const D2UpdateMessagePtr prev_msg = name_remove->getDnsUpdateRequest(); - // Run removingRevPtrsHandler to send the request. EXPECT_NO_THROW(name_remove->removingRevPtrsHandler()); - const D2UpdateMessagePtr curr_msg = name_remove->getDnsUpdateRequest(); - if (i == 1) { - // First time around we should build the message. - EXPECT_FALSE(prev_msg); - EXPECT_TRUE(curr_msg); - } else { - // Subsequent passes should reuse the request. We are only - // looking to check that we have not replaced the pointer value - // with a new pointer. This tests the on_entry() logic which - // clears the request ONLY upon initial entry into the state. - EXPECT_TRUE(prev_msg == curr_msg); - } - // Simulate a server corrupt response. name_remove->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE); name_remove->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT); diff --git a/src/bin/d2/tests/stats_test_utils.h b/src/bin/d2/tests/stats_test_utils.h deleted file mode 100644 index d7e4ae1462..0000000000 --- a/src/bin/d2/tests/stats_test_utils.h +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright (C) 2020-2021 Internet Systems Consortium, Inc. ("ISC") -// -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#ifndef STATS_TEST_UTILS_H -#define STATS_TEST_UTILS_H - -#include -#include -#include -#include - -#include - -namespace isc { -namespace d2 { -namespace test { - -/// @brief Type of name x value for statistics. -typedef std::map StatMap; - -/// @brief Test fixture class with utility functions to test statistics. -class D2StatTest : public ::testing::Test { -public: - /// @brief Constructor. - D2StatTest(); - - /// @brief Destructor. - virtual ~D2StatTest(); - - /// @brief Compares a statistic to an expected value. - /// - /// Attempt to fetch the named statistic from the StatsMgr and if - /// found, compare its observed value to the given value. - /// Fails if the stat is not found or if the values do not match. - /// - /// @param name StatsMgr name for the statistic to check. - /// @param expected_value expected value of the statistic. - void checkStat(const std::string& name, const int64_t expected_value); - - /// @brief Compares StatsMgr statistics against expected values. - /// - /// Iterates over a list of statistic names and expected values, attempting - /// to fetch each from the StatsMgr and if found, compare its observed - /// value to the expected value. Fails if any of the expected stats are not - /// found or if the values do not match. - /// - /// @param expected_stats Map of expected static names and values. - void checkStats(const StatMap& expected_stats); - - /// @brief Compares StatsMgr key statistics against expected values. - /// - /// Prepend key part of names before calling checkStats simpler variant. - /// - /// @param key_name Name of the key. - /// @param expected_stats Map of expected static names and values. - void checkStats(const std::string& key_name, const StatMap& expected_stats); -}; - -} -} -} - -#endif // STATS_TEST_UTILS_H diff --git a/src/lib/asiodns/io_fetch.h b/src/lib/asiodns/io_fetch.h index c6e909f286..0b67e74d5d 100644 --- a/src/lib/asiodns/io_fetch.h +++ b/src/lib/asiodns/io_fetch.h @@ -237,6 +237,9 @@ private: }; +/// \brief Defines a pointer to an IOFetch. +typedef boost::shared_ptr IOFetchPtr; + } // namespace asiodns } // namespace isc diff --git a/src/lib/d2srv/Makefile.am b/src/lib/d2srv/Makefile.am index 5b79604203..6ece84cbc7 100644 --- a/src/lib/d2srv/Makefile.am +++ b/src/lib/d2srv/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = . tests +SUBDIRS = . testutils tests AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) @@ -15,9 +15,13 @@ libkea_d2srv_la_SOURCES += d2_cfg_mgr.cc d2_cfg_mgr.h libkea_d2srv_la_SOURCES += d2_config.cc d2_config.h libkea_d2srv_la_SOURCES += d2_log.cc d2_log.h libkea_d2srv_la_SOURCES += d2_messages.cc d2_messages.h +libkea_d2srv_la_SOURCES += d2_update_message.cc d2_update_message.h libkea_d2srv_la_SOURCES += d2_simple_parser.cc d2_simple_parser.h libkea_d2srv_la_SOURCES += d2_stats.cc d2_stats.h libkea_d2srv_la_SOURCES += d2_tsig_key.cc d2_tsig_key.h +libkea_d2srv_la_SOURCES += d2_zone.cc d2_zone.h +libkea_d2srv_la_SOURCES += dns_client.cc dns_client.h +libkea_d2srv_la_SOURCES += nc_trans.cc nc_trans.h EXTRA_DIST += d2_messages.mes libkea_d2srv_la_CXXFLAGS = $(AM_CXXFLAGS) @@ -27,6 +31,7 @@ libkea_d2srv_la_LIBADD = libkea_d2srv_la_LIBADD += $(top_builddir)/src/lib/process/libkea-process.la libkea_d2srv_la_LIBADD += $(top_builddir)/src/lib/cfgrpt/libcfgrpt.la libkea_d2srv_la_LIBADD += $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns.la +libkea_d2srv_la_LIBADD += $(top_builddir)/src/lib/asiodns/libkea-asiodns.la libkea_d2srv_la_LIBADD += $(top_builddir)/src/lib/stats/libkea-stats.la libkea_d2srv_la_LIBADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la libkea_d2srv_la_LIBADD += $(top_builddir)/src/lib/http/libkea-http.la diff --git a/src/bin/d2/d2_update_message.cc b/src/lib/d2srv/d2_update_message.cc similarity index 98% rename from src/bin/d2/d2_update_message.cc rename to src/lib/d2srv/d2_update_message.cc index 115663fa80..1917d6e339 100644 --- a/src/bin/d2/d2_update_message.cc +++ b/src/lib/d2srv/d2_update_message.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -6,7 +6,7 @@ #include -#include +#include #include #include #include diff --git a/src/bin/d2/d2_update_message.h b/src/lib/d2srv/d2_update_message.h similarity index 99% rename from src/bin/d2/d2_update_message.h rename to src/lib/d2srv/d2_update_message.h index ad01a9ce69..8365128d05 100644 --- a/src/bin/d2/d2_update_message.h +++ b/src/lib/d2srv/d2_update_message.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015,2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -7,7 +7,7 @@ #ifndef D2_UPDATE_MESSAGE_H #define D2_UPDATE_MESSAGE_H -#include +#include #include #include #include diff --git a/src/bin/d2/d2_zone.cc b/src/lib/d2srv/d2_zone.cc similarity index 87% rename from src/bin/d2/d2_zone.cc rename to src/lib/d2srv/d2_zone.cc index e029d17d01..c48b2c5fac 100644 --- a/src/bin/d2/d2_zone.cc +++ b/src/lib/d2srv/d2_zone.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -6,7 +6,7 @@ #include -#include +#include namespace isc { namespace d2 { diff --git a/src/bin/d2/d2_zone.h b/src/lib/d2srv/d2_zone.h similarity index 98% rename from src/bin/d2/d2_zone.h rename to src/lib/d2srv/d2_zone.h index ec13871094..077ad16b8b 100644 --- a/src/bin/d2/d2_zone.h +++ b/src/lib/d2srv/d2_zone.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this diff --git a/src/bin/d2/dns_client.cc b/src/lib/d2srv/dns_client.cc similarity index 75% rename from src/bin/d2/dns_client.cc rename to src/lib/d2srv/dns_client.cc index de929bc549..5798207670 100644 --- a/src/bin/d2/dns_client.cc +++ b/src/lib/d2srv/dns_client.cc @@ -5,8 +5,9 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include -#include + #include +#include #include #include #include @@ -37,42 +38,68 @@ using namespace isc::stats; // to this separation. class DNSClientImpl : public asiodns::IOFetch::Callback { public: - // A buffer holding response from a DNS. + /// @brief A buffer holding response from a DNS. util::OutputBufferPtr in_buf_; - // A caller-supplied object which will hold the parsed response from DNS. - // The response object is (or descends from) isc::dns::Message and is - // populated using Message::fromWire(). This method may only be called - // once in the lifetime of a Message instance. Therefore, response_ is a - // pointer reference thus allowing this class to replace the object - // pointed to with a new Message instance each time a message is - // received. This allows a single DNSClientImpl instance to be used for - // multiple, sequential IOFetch calls. (@todo Trac# 3286 has been opened - // against dns::Message::fromWire. Should the behavior of fromWire change - // the behavior here with could be reexamined). + + /// A caller-supplied object which will hold the parsed response from DNS. + /// The response object is (or descends from) isc::dns::Message and is + /// populated using Message::fromWire(). This method may only be called + /// once in the lifetime of a Message instance. Therefore, response_ is a + /// pointer reference thus allowing this class to replace the object + /// pointed to with a new Message instance each time a message is + /// received. This allows a single DNSClientImpl instance to be used for + /// multiple, sequential IOFetch calls. (@todo Trac# 3286 has been opened + /// against dns::Message::fromWire. Should the behavior of fromWire change + /// the behavior here with could be reexamined). D2UpdateMessagePtr& response_; - // A caller-supplied external callback which is invoked when DNS message - // exchange is complete or interrupted. + + /// @brief A caller-supplied external callback which is invoked when DNS + /// message exchange is complete or interrupted. DNSClient::Callback* callback_; - // A Transport Layer protocol used to communicate with a DNS. + + /// @brief A Transport Layer protocol used to communicate with a DNS. DNSClient::Protocol proto_; - // TSIG context used to sign outbound and verify inbound messages. + + /// @brief TSIG context used to sign outbound and verify inbound messages. dns::TSIGContextPtr tsig_context_; - // TSIG key name for stats. + + /// @brief TSIG key name for stats. std::string tsig_key_name_; - // Constructor and Destructor + /// @brief Constructor. + /// + /// @param response_placeholder Message object pointer which will be updated + /// with dynamically allocated object holding the DNS server's response. + /// @param callback Pointer to an object implementing @c DNSClient::Callback + /// class. This object will be called when DNS message exchange completes or + /// if an error occurs. NULL value disables callback invocation. + /// @param proto caller's preference regarding Transport layer protocol to + /// be used by DNS Client to communicate with a server. DNSClientImpl(D2UpdateMessagePtr& response_placeholder, DNSClient::Callback* callback, const DNSClient::Protocol proto); + + /// @brief Destructor. virtual ~DNSClientImpl(); - // This internal callback is called when the DNS update message exchange is - // complete. It further invokes the external callback provided by a caller. - // Before external callback is invoked, an object of the D2UpdateMessage - // type, representing a response from the server is set. + /// @brief This internal callback is called when the DNS update message + /// exchange is complete. It further invokes the external callback provided + /// by a caller. Before external callback is invoked, an object of the + /// D2UpdateMessage type, representing a response from the server is set. virtual void operator()(asiodns::IOFetch::Result result); - // Starts asynchronous DNS Update using TSIG. + /// @brief Starts asynchronous DNS Update using TSIG. + /// + /// @param io_service IO service to be used to run the message exchange. + /// @param ns_addr DNS server address. + /// @param ns_port DNS server port. + /// @param update A DNS Update message to be sent to the server. + /// @param wait A timeout (in milliseconds) for the response. If a response + /// is not received within the timeout, exchange is interrupted. This value + /// must not exceed maximal value for 'int' data type. + /// @param tsig_key A pointer to an @c D2TsigKeyPtr object that will + /// (if not null) be used to sign the DNS Update message and verify the + /// response. void doUpdate(asiolink::IOService& io_service, const asiolink::IOAddress& ns_addr, const uint16_t ns_port, @@ -80,10 +107,17 @@ public: const unsigned int wait, const D2TsigKeyPtr& tsig_key); - // This function maps the IO error to the DNSClient error. - DNSClient::Status getStatus(const asiodns::IOFetch::Result); - - // This function updates statistics. + /// @brief This function maps the IO error to the DNSClient error. + /// + /// @param result The IOFetch result to be converted to DNSClient status. + /// @return The DNSClient status corresponding to the IOFetch result. + DNSClient::Status getStatus(const asiodns::IOFetch::Result result); + + /// @brief This function updates statistics. + /// + /// @param stat The statistic name to be incremented. + /// @param update_key The flag indicating if the key statistics should also + /// be updated. void incrStats(const std::string& stat, bool update_key = true); }; @@ -268,7 +302,6 @@ DNSClient::DNSClient(D2UpdateMessagePtr& response_placeholder, } DNSClient::~DNSClient() { - delete (impl_); } unsigned int diff --git a/src/bin/d2/dns_client.h b/src/lib/d2srv/dns_client.h similarity index 97% rename from src/bin/d2/dns_client.h rename to src/lib/d2srv/dns_client.h index fb2412523d..c63a7a0c0a 100644 --- a/src/bin/d2/dns_client.h +++ b/src/lib/d2srv/dns_client.h @@ -7,13 +7,11 @@ #ifndef DNS_CLIENT_H #define DNS_CLIENT_H -#include - #include -#include - #include #include +#include +#include namespace isc { namespace d2 { @@ -149,7 +147,8 @@ public: const D2TsigKeyPtr& tsig_key = D2TsigKeyPtr()); private: - DNSClientImpl* impl_; ///< Pointer to DNSClient implementation. + /// @brief Pointer to DNSClient implementation. + std::unique_ptr impl_; }; } // namespace d2 diff --git a/src/bin/d2/nc_trans.cc b/src/lib/d2srv/nc_trans.cc similarity index 99% rename from src/bin/d2/nc_trans.cc rename to src/lib/d2srv/nc_trans.cc index 6f5c8ebb9f..e9534c256c 100644 --- a/src/bin/d2/nc_trans.cc +++ b/src/lib/d2srv/nc_trans.cc @@ -6,8 +6,8 @@ #include -#include #include +#include #include #include #include @@ -301,10 +301,14 @@ NameChangeTransaction::setDnsUpdateRequest(D2UpdateMessagePtr& request) { void NameChangeTransaction::clearDnsUpdateRequest() { - update_attempts_ = 0; dns_update_request_.reset(); } +void +NameChangeTransaction::clearUpdateAttempts() { + update_attempts_ = 0; +} + void NameChangeTransaction::setDnsUpdateStatus(const DNSClient::Status& status) { dns_update_status_ = status; diff --git a/src/bin/d2/nc_trans.h b/src/lib/d2srv/nc_trans.h similarity index 99% rename from src/bin/d2/nc_trans.h rename to src/lib/d2srv/nc_trans.h index 503d9a0f83..2aee7eccfb 100644 --- a/src/bin/d2/nc_trans.h +++ b/src/lib/d2srv/nc_trans.h @@ -10,7 +10,7 @@ /// @file nc_trans.h This file defines the class NameChangeTransaction. #include -#include +#include #include #include #include @@ -288,10 +288,12 @@ protected: /// @param request is the new request packet to assign. void setDnsUpdateRequest(D2UpdateMessagePtr& request); - /// @brief Destroys the current update request packet and resets - /// update attempts count. + /// @brief Destroys the current update request packet. void clearDnsUpdateRequest(); + /// @brief Resets the update attempts count. + void clearUpdateAttempts(); + /// @brief Sets the update status to the given status value. /// /// @param status is the new value for the update status. diff --git a/src/lib/d2srv/tests/Makefile.am b/src/lib/d2srv/tests/Makefile.am index caede036f9..436cb21ee1 100644 --- a/src/lib/d2srv/tests/Makefile.am +++ b/src/lib/d2srv/tests/Makefile.am @@ -20,13 +20,19 @@ TESTS += libd2srv_unittests libd2srv_unittests_SOURCES = run_unittests.cc libd2srv_unittests_SOURCES += d2_tsig_key_unittest.cc +libd2srv_unittests_SOURCES += d2_update_message_unittests.cc +libd2srv_unittests_SOURCES += d2_zone_unittests.cc +libd2srv_unittests_SOURCES += dns_client_unittests.cc +libd2srv_unittests_SOURCES += nc_trans_unittests.cc libd2srv_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) libd2srv_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) libd2srv_unittests_LDADD = $(top_builddir)/src/lib/d2srv/libkea-d2srv.la +libd2srv_unittests_LDADD += $(top_builddir)/src/lib/d2srv/testutils/libd2srvtest.la libd2srv_unittests_LDADD += $(top_builddir)/src/lib/process/libkea-process.la libd2srv_unittests_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns.la +libd2srv_unittests_LDADD += $(top_builddir)/src/lib/asiodns/libkea-asiodns.la libd2srv_unittests_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la libd2srv_unittests_LDADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la libd2srv_unittests_LDADD += $(top_builddir)/src/lib/http/libkea-http.la diff --git a/src/bin/d2/tests/d2_update_message_unittests.cc b/src/lib/d2srv/tests/d2_update_message_unittests.cc similarity index 99% rename from src/bin/d2/tests/d2_update_message_unittests.cc rename to src/lib/d2srv/tests/d2_update_message_unittests.cc index 092b23f9e6..6f0cfca7cf 100644 --- a/src/bin/d2/tests/d2_update_message_unittests.cc +++ b/src/lib/d2srv/tests/d2_update_message_unittests.cc @@ -6,9 +6,9 @@ #include -#include -#include #include +#include +#include #include #include #include diff --git a/src/bin/d2/tests/d2_zone_unittests.cc b/src/lib/d2srv/tests/d2_zone_unittests.cc similarity index 96% rename from src/bin/d2/tests/d2_zone_unittests.cc rename to src/lib/d2srv/tests/d2_zone_unittests.cc index 486bed4798..f252d07aa1 100644 --- a/src/bin/d2/tests/d2_zone_unittests.cc +++ b/src/lib/d2srv/tests/d2_zone_unittests.cc @@ -1,11 +1,11 @@ -// Copyright (C) 2013-2015,2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include -#include +#include #include #include diff --git a/src/bin/d2/tests/dns_client_unittests.cc b/src/lib/d2srv/tests/dns_client_unittests.cc similarity index 69% rename from src/bin/d2/tests/dns_client_unittests.cc rename to src/lib/d2srv/tests/dns_client_unittests.cc index 6d38cc3fac..a560546b0f 100644 --- a/src/bin/d2/tests/dns_client_unittests.cc +++ b/src/lib/d2srv/tests/dns_client_unittests.cc @@ -6,14 +6,14 @@ #include -#include +#include #include #include #include #include +#include +#include #include -#include -#include #include #include @@ -56,34 +56,61 @@ const long TEST_TIMEOUT = 5 * 1000; // properly handled a task may be hanging for a long time. In order to prevent // it, the asiolink::IntervalTimer is used to break a running test if test // timeout is hit. This will result in test failure. -class DNSClientTest : public virtual D2StatTest, DNSClient::Callback { +class DNSClientTest : public ::testing::Test, DNSClient::Callback, + public D2StatTest { public: + /// @brief The IOService which handles IO operations. IOService service_; + + /// @brief The UDP socket. + std::unique_ptr socket_; + + /// @brief The UDP socket endpoint. + std::unique_ptr endpoint_; + + /// @brief DNS client response. D2UpdateMessagePtr response_; + + /// @brief The status of the DNS client update callback. DNSClient::Status status_; + + /// @brief The receive buffer. uint8_t receive_buffer_[MAX_SIZE]; + + /// @brief The DNS client performing DNS update. DNSClientPtr dns_client_; + + /// @brief The flag which specifies if the response should be corrupted. bool corrupt_response_; + + /// @brief The flag which specifies if a response is expected. bool expect_response_; + + /// @brief The timeout timer. asiolink::IntervalTimer test_timer_; + + /// @brief The number of received DNS updates. int received_; + + /// @brief The number of expected DNS updates. int expected_; + /// @brief The flag which specifies if the server should continue with + /// receiving DNS updates. + bool go_on_; + /// @brief Constructor - // - // This constructor overrides the default logging level of asiodns logger to - // prevent it from emitting debug messages from IOFetch class. Such an error - // message can be emitted if timeout occurs when DNSClient class is - // waiting for a response. Some of the tests are checking DNSClient behavior - // in case when response from the server is not received. Tests output would - // become messy if such errors were logged. - DNSClientTest() - : service_(), - status_(DNSClient::SUCCESS), - corrupt_response_(false), - expect_response_(true), - test_timer_(service_), - received_(0), expected_(0) { + /// + /// This constructor overrides the default logging level of asiodns logger to + /// prevent it from emitting debug messages from IOFetch class. Such an error + /// message can be emitted if timeout occurs when DNSClient class is + /// waiting for a response. Some of the tests are checking DNSClient behavior + /// in case when response from the server is not received. Tests output would + /// become messy if such errors were logged. + DNSClientTest() : service_(), socket_(), endpoint_(), + status_(DNSClient::SUCCESS), corrupt_response_(false), + expect_response_(true), test_timer_(service_), + received_(0), expected_(0), go_on_(false) { asiodns::logger.setSeverity(isc::log::INFO); response_.reset(); dns_client_.reset(new DNSClient(response_, this)); @@ -94,22 +121,21 @@ public: } /// @brief Destructor - // - // Sets the asiodns logging level back to DEBUG. + /// + /// Sets the asiodns logging level back to DEBUG. virtual ~DNSClientTest() { asiodns::logger.setSeverity(isc::log::DEBUG); }; /// @brief Exchange completion callback - // - // This callback is called when the exchange with the DNS server is - // complete or an error occurred. This includes the occurrence of a timeout. - // - // @param status A status code returned by DNSClient. + /// + /// This callback is called when the exchange with the DNS server is + /// complete or an error occurred. This includes the occurrence of a timeout. + /// + /// @param status A status code returned by DNSClient. virtual void operator()(DNSClient::Status status) { status_ = status; - if (!expected_ || (expected_ == ++received_)) - { + if (!expected_ || (expected_ == ++received_)) { service_.stop(); } @@ -139,27 +165,27 @@ public: } /// @brief Handler invoked when test timeout is hit - // - // This callback stops all running (hanging) tasks on IO service. + /// + /// This callback stops all running (hanging) tasks on IO service. void testTimeoutHandler() { service_.stop(); FAIL() << "Test timeout hit."; } /// @brief Handler invoked when test request is received - // - // This callback handler is installed when performing async read on a - // socket to emulate reception of the DNS Update request by a server. - // As a result, this handler will send an appropriate DNS Update response - // message back to the address from which the request has come. - // - // @param socket A pointer to a socket used to receive a query and send a - // response. - // @param remote A pointer to an object which specifies the host (address - // and port) from which a request has come. - // @param receive_length A length (in bytes) of the received data. - // @param corrupt_response A bool value which indicates that the server's - // response should be invalid (true) or valid (false) + /// + /// This callback handler is installed when performing async read on a + /// socket to emulate reception of the DNS Update request by a server. + /// As a result, this handler will send an appropriate DNS Update response + /// message back to the address from which the request has come. + /// + /// @param socket A pointer to a socket used to receive a query and send a + /// response. + /// @param remote A pointer to an object which specifies the host (address + /// and port) from which a request has come. + /// @param receive_length A length (in bytes) of the received data. + /// @param corrupt_response A bool value which specifies if the server's + /// response should be invalid (true) or valid (false). void udpReceiveHandler(udp::socket* socket, udp::endpoint* remote, size_t receive_length, const bool corrupt_response) { // The easiest way to create a response message is to copy the entire @@ -183,31 +209,39 @@ public: socket->send_to(boost::asio::buffer(response_buf.getData(), response_buf.getLength()), *remote); + + if (go_on_) { + socket_->async_receive_from(boost::asio::buffer(receive_buffer_, + sizeof(receive_buffer_)), + *endpoint_, + std::bind(&DNSClientTest::udpReceiveHandler, + this, socket_.get(), + endpoint_.get(), ph::_2, + corrupt_response)); + } } /// @brief Request handler for testing clients using TSIG - // - // This callback handler is installed when performing async read on a - // socket to emulate reception of the DNS Update request with TSIG by a - // server. As a result, this handler will send an appropriate DNS Update - // response message back to the address from which the request has come. - // - // @param socket A pointer to a socket used to receive a query and send a - // response. - // @param remote A pointer to an object which specifies the host (address - // and port) from which a request has come. - // @param receive_length A length (in bytes) of the received data. - // @param corrupt_response A bool value which indicates that the server's - // response should be invalid (true) or valid (false) - // @param client_key TSIG key the server should use to verify the inbound - // request. If the pointer is NULL, the server will not attempt to - // verify the request. - // @param server_key TSIG key the server should use to sign the outbound - // request. If the pointer is NULL, the server will not sign the outbound - // response. If the pointer is not NULL and not the same value as the - // client_key, the server will use a new context to sign the response then - // the one used to verify it. This allows us to simulate the server - // signing with the wrong key. + /// + /// This callback handler is installed when performing async read on a + /// socket to emulate reception of the DNS Update request with TSIG by a + /// server. As a result, this handler will send an appropriate DNS Update + /// response message back to the address from which the request has come. + /// + /// @param socket A pointer to a socket used to receive a query and send a + /// response. + /// @param remote A pointer to an object which specifies the host (address + /// and port) from which a request has come. + /// @param receive_length A length (in bytes) of the received data. + /// @param client_key TSIG key the server should use to verify the inbound + /// request. If the pointer is NULL, the server will not attempt to + /// verify the request. + /// @param server_key TSIG key the server should use to sign the outbound + /// request. If the pointer is NULL, the server will not sign the outbound + /// response. If the pointer is not NULL and not the same value as the + /// client_key, the server will use a new context to sign the response then + /// the one used to verify it. This allows us to simulate the server + /// signing with the wrong key. void TSIGReceiveHandler(udp::socket* socket, udp::endpoint* remote, size_t receive_length, D2TsigKeyPtr client_key, @@ -255,14 +289,15 @@ public: response.toWire(renderer, context.get()); // A response message is now ready to send. Send it! - socket->send_to(boost::asio::buffer(renderer.getData(), renderer.getLength()), + socket->send_to(boost::asio::buffer(renderer.getData(), + renderer.getLength()), *remote); } - // This test verifies that when invalid response placeholder object is - // passed to a constructor, constructor throws the appropriate exception. - // It also verifies that the constructor will not throw if the supplied - // callback object is NULL. + /// @brief This test verifies that when invalid response placeholder object + /// is passed to a constructor which throws the appropriate exception. + /// It also verifies that the constructor will not throw if the supplied + /// callback object is NULL. void runConstructorTest() { EXPECT_NO_THROW(DNSClient(response_, NULL, DNSClient::UDP)); @@ -273,8 +308,8 @@ public: isc::NotImplemented); } - // This test verifies that it accepted timeout values belong to the range of - // <0, DNSClient::getMaxTimeout()>. + /// @brief This test verifies that it accepted timeout values belong to the + /// range of <0, DNSClient::getMaxTimeout()>. void runInvalidTimeoutTest() { expect_response_ = false; @@ -299,9 +334,9 @@ public: isc::BadValue); } - // This test verifies the DNSClient behavior when a server does not respond - // do the DNS Update message. In such case, the callback function is - // expected to be called and the TIME_OUT error code should be returned. + /// @brief This test verifies the DNSClient behavior when a server does not + /// respond do the DNS Update message. In such case, the callback function + /// is expected to be called and the TIME_OUT error code should be returned. void runSendNoReceiveTest() { // We expect no response from a server. expect_response_ = false; @@ -329,7 +364,7 @@ public: // completion callback will be triggered. The doUpdate function returns // immediately. EXPECT_NO_THROW(dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS), - TEST_PORT, message, timeout)); + TEST_PORT, message, timeout)); // This starts the execution of tasks posted to IOService. run() blocks // until stop() is called in the completion callback function. @@ -337,10 +372,11 @@ public: } - // This test verifies that DNSClient can send DNS Update and receive a - // corresponding response from a server. + /// @brief This test verifies that DNSClient can send DNS Update and receive + /// a corresponding response from a server. void runSendReceiveTest(const bool corrupt_response, const bool two_sends) { + go_on_ = two_sends; corrupt_response_ = corrupt_response; // Create a request DNS Update message. @@ -352,15 +388,16 @@ public: // and receives a response from the server, we have to emulate the // server's response in the test. A request will be sent via loopback // interface to 127.0.0.1 and known test port. Response must be sent - // to 127.0.0.1 and a source port which has been used to send the + // to 127.0.0.1 and the source port which has been used to send the // request. A new socket is created, specifically to handle sending // responses. The reuse address option is set so as both sockets can // use the same address. This new socket is bound to the test address // and port, where requests will be sent. - udp::socket udp_socket(service_.get_io_service(), boost::asio::ip::udp::v4()); - udp_socket.set_option(socket_base::reuse_address(true)); - udp_socket.bind(udp::endpoint(address::from_string(TEST_ADDRESS), - TEST_PORT)); + socket_.reset(new udp::socket(service_.get_io_service(), + boost::asio::ip::udp::v4())); + socket_->set_option(socket_base::reuse_address(true)); + socket_->bind(udp::endpoint(address::from_string(TEST_ADDRESS), + TEST_PORT)); // Once socket is created, we can post an IO request to receive some // packet from this socket. This is asynchronous operation and // nothing is received until another IO request to send a query is @@ -373,13 +410,14 @@ public: // Callback function will send a response to this address and port. // The last parameter holds a length of the received request. It is // required to construct a response. - udp::endpoint remote; - udp_socket.async_receive_from(boost::asio::buffer(receive_buffer_, - sizeof(receive_buffer_)), - remote, - std::bind(&DNSClientTest::udpReceiveHandler, - this, &udp_socket, &remote, ph::_2, - corrupt_response)); + endpoint_.reset(new udp::endpoint()); + socket_->async_receive_from(boost::asio::buffer(receive_buffer_, + sizeof(receive_buffer_)), + *endpoint_, + std::bind(&DNSClientTest::udpReceiveHandler, + this, socket_.get(), + endpoint_.get(), ph::_2, + corrupt_response)); // The socket is now ready to receive the data. Let's post some request // message then. Set timeout to some reasonable value to make sure that @@ -402,7 +440,7 @@ public: // "send" and "receive" operations. service_.run(); - udp_socket.close(); + socket_->close(); // Since the callback, operator(), calls stop() on the io_service, // we must reset it in order for subsequent calls to run() or @@ -410,14 +448,13 @@ public: service_.get_io_service().reset(); } - // Performs a single request-response exchange with or without TSIG - // - // @param client_key TSIG passed to dns_client and also used by the - // "server" to verify the request. - // request. - // @param server_key TSIG key the "server" should use to sign the response. - // If this is NULL, then client_key is used. - // @param should_pass indicates if the test should pass. + /// @brief Performs a single request-response exchange with or without TSIG. + /// + /// @param client_key TSIG passed to dns_client and also used by the + /// "server" to verify the request. + /// @param server_key TSIG key the "server" should use to sign the response. + /// If this is NULL, then client_key is used. + /// @param should_pass indicates if the test should pass. void runTSIGTest(D2TsigKeyPtr client_key, D2TsigKeyPtr server_key, bool should_pass = true) { // Tell operator() method if we expect an invalid response. @@ -468,6 +505,15 @@ public: // valid. Constructor should throw exceptions when parameters are invalid. TEST_F(DNSClientTest, constructor) { runConstructorTest(); + StatMap stats_upd = { + { "update-sent", 0}, + { "update-signed", 0}, + { "update-unsigned", 0}, + { "update-success", 0}, + { "update-timeout", 0}, + { "update-error", 0} + }; + checkStats(stats_upd); } // This test verifies that the maximal allowed timeout value is maximal int @@ -479,6 +525,15 @@ TEST_F(DNSClientTest, getMaxTimeout) { // Verify that timeout is reported when no response is received from DNS. TEST_F(DNSClientTest, timeout) { runSendNoReceiveTest(); + StatMap stats_upd = { + { "update-sent", 1}, + { "update-signed", 0}, + { "update-unsigned", 1}, + { "update-success", 0}, + { "update-timeout", 1}, + { "update-error", 0} + }; + checkStats(stats_upd); } // Verify that exception is thrown when invalid (too high) timeout value is @@ -553,6 +608,15 @@ TEST_F(DNSClientTest, runTSIGTest) { TEST_F(DNSClientTest, sendReceive) { // false means that server response is not corrupted. runSendReceiveTest(false, false); + StatMap stats_upd = { + { "update-sent", 1}, + { "update-signed", 0}, + { "update-unsigned", 1}, + { "update-success", 1}, + { "update-timeout", 0}, + { "update-error", 0} + }; + checkStats(stats_upd); } // Verify that the DNSClient reports an error when the response is received from @@ -560,6 +624,15 @@ TEST_F(DNSClientTest, sendReceive) { TEST_F(DNSClientTest, sendReceiveCorrupted) { // true means that server's response is corrupted. runSendReceiveTest(true, false); + StatMap stats_upd = { + { "update-sent", 1}, + { "update-signed", 0}, + { "update-unsigned", 1}, + { "update-success", 0}, + { "update-timeout", 0}, + { "update-error", 1} + }; + checkStats(stats_upd); } // Verify that it is possible to use the same DNSClient instance to @@ -572,6 +645,15 @@ TEST_F(DNSClientTest, sendReceiveTwice) { runSendReceiveTest(false, false); runSendReceiveTest(false, false); EXPECT_EQ(2, received_); + StatMap stats_upd = { + { "update-sent", 2}, + { "update-signed", 0}, + { "update-unsigned", 2}, + { "update-success", 2}, + { "update-timeout", 0}, + { "update-error", 0} + }; + checkStats(stats_upd); } // Verify that it is possible to use the DNSClient instance to perform the @@ -580,14 +662,17 @@ TEST_F(DNSClientTest, sendReceiveTwice) { // 2. send // 3. receive // 4. receive -// @todo THIS Test does not function. The method runSendReceive only -// schedules one "server" receive. In other words only one request is -// listened for and then received. Once it is received, the operator() -// method calls stop() on the io_service, which causes the second receive -// to be cancelled. It is also unclear, what the asio layer does with a -// second receive on the same socket. -TEST_F(DNSClientTest, DISABLED_concurrentSendReceive) { +TEST_F(DNSClientTest, concurrentSendReceive) { runSendReceiveTest(false, true); + StatMap stats_upd = { + { "update-sent", 2}, + { "update-signed", 0}, + { "update-unsigned", 2}, + { "update-success", 2}, + { "update-timeout", 0}, + { "update-error", 0} + }; + checkStats(stats_upd); } } // End of anonymous namespace diff --git a/src/bin/d2/tests/nc_trans_unittests.cc b/src/lib/d2srv/tests/nc_trans_unittests.cc similarity index 99% rename from src/bin/d2/tests/nc_trans_unittests.cc rename to src/lib/d2srv/tests/nc_trans_unittests.cc index 1471006a14..89a23b7806 100644 --- a/src/bin/d2/tests/nc_trans_unittests.cc +++ b/src/lib/d2srv/tests/nc_trans_unittests.cc @@ -8,13 +8,13 @@ #include #include #include -#include +#include +#include #include #include #include #include #include -#include #include #include @@ -44,6 +44,8 @@ public: // NameChangeStub events static const int SEND_UPDATE_EVT = NCT_DERIVED_EVENT_MIN + 2; + /// @brief Flag which specifies if the NameChangeStub's callback should be + /// used instead of the NameChangeTransaction's callback. bool use_stub_callback_; /// @brief Constructor @@ -235,6 +237,7 @@ public: using NameChangeTransaction::setNcrStatus; using NameChangeTransaction::setDnsUpdateRequest; using NameChangeTransaction::clearDnsUpdateRequest; + using NameChangeTransaction::clearUpdateAttempts; using NameChangeTransaction::setDnsUpdateStatus; using NameChangeTransaction::getDnsUpdateResponse; using NameChangeTransaction::setDnsUpdateResponse; @@ -663,7 +666,7 @@ TEST_F(NameChangeTransactionTest, serverSelectionTest) { // The server selection process determines the current server, // instantiates a new DNSClient, and a DNS response message buffer. - // We need to save the values before each selection, so we can verify + // We need to save the values before each selection, so we can verify // they are correct after each selection. DnsServerInfoPtr prev_server = name_change->getCurrentServer(); DNSClientPtr prev_client = name_change->getDNSClient(); @@ -871,6 +874,12 @@ TEST_F(NameChangeTransactionTest, updateAttempts) { // Verify that the value is as expected. EXPECT_EQ(5, name_change->getUpdateAttempts()); + + // Clear it. + name_change->clearUpdateAttempts(); + + // Verify that it was cleared as expected. + EXPECT_EQ(0, name_change->getUpdateAttempts()); } /// @brief Tests retryTransition method diff --git a/src/lib/d2srv/testutils/Makefile.am b/src/lib/d2srv/testutils/Makefile.am new file mode 100644 index 0000000000..c7ae2add5b --- /dev/null +++ b/src/lib/d2srv/testutils/Makefile.am @@ -0,0 +1,23 @@ +SUBDIRS = . + +AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib +AM_CPPFLAGS += $(BOOST_INCLUDES) + +AM_CXXFLAGS = $(KEA_CXXFLAGS) + +CLEANFILES = *.gcno *.gcda + +if HAVE_GTEST + +noinst_LTLIBRARIES = libd2srvtest.la + +libd2srvtest_la_SOURCES = nc_test_utils.cc nc_test_utils.h +libd2srvtest_la_SOURCES += stats_test_utils.cc stats_test_utils.h + +libd2srvtest_la_CXXFLAGS = $(AM_CXXFLAGS) +libd2srvtest_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) + +libd2srvtest_la_LIBADD = $(top_builddir)/src/lib/d2srv/libkea-d2srv.la +libd2srvtest_la_LIBADD += $(top_builddir)/src/lib/log/libkea-log.la + +endif diff --git a/src/bin/d2/tests/nc_test_utils.cc b/src/lib/d2srv/testutils/nc_test_utils.cc similarity index 94% rename from src/bin/d2/tests/nc_test_utils.cc rename to src/lib/d2srv/testutils/nc_test_utils.cc index 358387bdc7..39035352b3 100644 --- a/src/bin/d2/tests/nc_test_utils.cc +++ b/src/lib/d2srv/testutils/nc_test_utils.cc @@ -8,9 +8,9 @@ #include #include #include +#include #include #include -#include #include #include @@ -394,9 +394,9 @@ checkZone(const D2UpdateMessagePtr& request, const std::string& exp_zone_name) { void checkRR(dns::RRsetPtr rrset, const std::string& exp_name, - const dns::RRClass& exp_class, const dns::RRType& exp_type, - unsigned int exp_ttl, dhcp_ddns::NameChangeRequestPtr ncr, - bool has_rdata) { + const dns::RRClass& exp_class, const dns::RRType& exp_type, + unsigned int exp_ttl, dhcp_ddns::NameChangeRequestPtr ncr, + bool has_rdata) { // Verify the FQDN/DHCID RR fields. EXPECT_EQ(exp_name, rrset->getName().toText()); EXPECT_EQ(exp_class.getCode(), rrset->getClass().getCode()); @@ -417,7 +417,7 @@ checkRR(dns::RRsetPtr rrset, const std::string& exp_name, (exp_type == dns::RRType::AAAA())) { // should have lease rdata EXPECT_EQ(ncr->getIpAddress(), rdata_it->getCurrent().toText()); - } else if (exp_type == dns::RRType::PTR()) { + } else if (exp_type == dns::RRType::PTR()) { // should have PTR rdata EXPECT_EQ(ncr->getFqdn(), rdata_it->getCurrent().toText()); } else if (exp_type == dns::RRType::DHCID()) { @@ -447,19 +447,20 @@ getRRFromSection(const D2UpdateMessagePtr& request, return (*rrset_it); } -dhcp_ddns::NameChangeRequestPtr makeNcrFromString(const std::string& ncr_str) { +dhcp_ddns::NameChangeRequestPtr +makeNcrFromString(const std::string& ncr_str) { return (dhcp_ddns::NameChangeRequest::fromJSON(ncr_str)); } -DdnsDomainPtr makeDomain(const std::string& zone_name, - const std::string& key_name) { +DdnsDomainPtr +makeDomain(const std::string& zone_name, const std::string& key_name) { DnsServerInfoStoragePtr servers(new DnsServerInfoStorage()); DdnsDomainPtr domain(new DdnsDomain(zone_name, servers, key_name)); return (domain); } -DdnsDomainPtr makeDomain(const std::string& zone_name, - const TSIGKeyInfoPtr &tsig_key_info) { +DdnsDomainPtr +makeDomain(const std::string& zone_name, const TSIGKeyInfoPtr &tsig_key_info) { DdnsDomainPtr domain; DnsServerInfoStoragePtr servers(new DnsServerInfoStorage()); std::string key_name; @@ -470,9 +471,9 @@ DdnsDomainPtr makeDomain(const std::string& zone_name, return (domain); } -TSIGKeyInfoPtr makeTSIGKeyInfo(const std::string& key_name, - const std::string& secret, - const std::string& algorithm) { +TSIGKeyInfoPtr +makeTSIGKeyInfo(const std::string& key_name, const std::string& secret, + const std::string& algorithm) { TSIGKeyInfoPtr key_info; if (!key_name.empty()) { if (!secret.empty()) { @@ -493,20 +494,21 @@ TSIGKeyInfoPtr makeTSIGKeyInfo(const std::string& key_name, } return (key_info); - } -void addDomainServer(DdnsDomainPtr& domain, const std::string& name, - const std::string& ip, const size_t port, - const TSIGKeyInfoPtr &tsig_key_info) { +void +addDomainServer(DdnsDomainPtr& domain, const std::string& name, + const std::string& ip, const size_t port, + const TSIGKeyInfoPtr &tsig_key_info) { DnsServerInfoPtr server(new DnsServerInfo(name, asiolink::IOAddress(ip), port, true, tsig_key_info)); domain->getServers()->push_back(server); } -// Verifies that the contents of the given transaction's DNS update request +// Verifies that the contents of the given transaction's DNS update request // is correct for adding a forward DNS entry -void checkAddFwdAddressRequest(NameChangeTransaction& tran) { +void +checkAddFwdAddressRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -555,9 +557,10 @@ void checkAddFwdAddressRequest(NameChangeTransaction& tran) { ASSERT_NO_THROW(request->toWire(renderer)); } -// Verifies that the contents of the given transaction's DNS update request +// Verifies that the contents of the given transaction's DNS update request // is correct for replacing a forward DNS entry -void checkReplaceFwdAddressRequest(NameChangeTransaction& tran) { +void +checkReplaceFwdAddressRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -614,9 +617,10 @@ void checkReplaceFwdAddressRequest(NameChangeTransaction& tran) { ASSERT_NO_THROW(request->toWire(renderer)); } -// Verifies that the contents of the given transaction's DNS update request +// Verifies that the contents of the given transaction's DNS update request // is correct for replacing a reverse DNS entry -void checkReplaceRevPtrsRequest(NameChangeTransaction& tran) { +void +checkReplaceRevPtrsRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -677,7 +681,8 @@ void checkReplaceRevPtrsRequest(NameChangeTransaction& tran) { ASSERT_NO_THROW(request->toWire(renderer)); } -void checkRemoveFwdAddressRequest(NameChangeTransaction& tran) { +void +checkRemoveFwdAddressRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -716,7 +721,8 @@ void checkRemoveFwdAddressRequest(NameChangeTransaction& tran) { ASSERT_NO_THROW(request->toWire(renderer)); } -void checkRemoveFwdRRsRequest(NameChangeTransaction& tran) { +void +checkRemoveFwdRRsRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -766,7 +772,8 @@ void checkRemoveFwdRRsRequest(NameChangeTransaction& tran) { ASSERT_NO_THROW(request->toWire(renderer)); } -void checkRemoveRevPtrsRequest(NameChangeTransaction& tran) { +void +checkRemoveRevPtrsRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -804,7 +811,8 @@ void checkRemoveRevPtrsRequest(NameChangeTransaction& tran) { ASSERT_NO_THROW(request->toWire(renderer)); } -std::string toHexText(const uint8_t* data, size_t len) { +std::string +toHexText(const uint8_t* data, size_t len) { std::ostringstream stream; stream << "Data length is: " << len << std::endl; for (int i = 0; i < len; ++i) { @@ -819,10 +827,11 @@ std::string toHexText(const uint8_t* data, size_t len) { return (stream.str()); } -// Verifies that the contents of the given transaction's DNS update request +// Verifies that the contents of the given transaction's DNS update request // is correct for replacing a forward DNS entry when not using conflict // resolution. -void checkSimpleReplaceFwdAddressRequest(NameChangeTransaction& tran) { +void +checkSimpleReplaceFwdAddressRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -883,7 +892,8 @@ void checkSimpleReplaceFwdAddressRequest(NameChangeTransaction& tran) { ASSERT_NO_THROW(request->toWire(renderer)); } -void checkSimpleRemoveFwdRRsRequest(NameChangeTransaction& tran) { +void +checkSimpleRemoveFwdRRsRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -921,10 +931,11 @@ void checkSimpleRemoveFwdRRsRequest(NameChangeTransaction& tran) { ASSERT_NO_THROW(request->toWire(renderer)); } -// Verifies that the contents of the given transaction's DNS update request +// Verifies that the contents of the given transaction's DNS update request // is correct for removing a reverse DNS entry when not using conflict // resolution. -void checkSimpleRemoveRevPtrsRequest(NameChangeTransaction& tran) { +void +checkSimpleRemoveRevPtrsRequest(NameChangeTransaction& tran) { const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest(); ASSERT_TRUE(request); @@ -961,8 +972,9 @@ void checkSimpleRemoveRevPtrsRequest(NameChangeTransaction& tran) { } // Verifies the current state and next event in a transaction -void checkContext(NameChangeTransactionPtr trans, const int exp_state, - const int exp_evt, const std::string& file, int line) { +void +checkContext(NameChangeTransactionPtr trans, const int exp_state, + const int exp_evt, const std::string& file, int line) { ASSERT_TRUE(trans); ASSERT_TRUE(exp_state == trans->getCurrState() && exp_evt == trans->getNextEvent()) << "expected state: " << trans->getStateLabel(exp_state) diff --git a/src/bin/d2/tests/nc_test_utils.h b/src/lib/d2srv/testutils/nc_test_utils.h similarity index 95% rename from src/bin/d2/tests/nc_test_utils.h rename to src/lib/d2srv/testutils/nc_test_utils.h index 4cfff4cb92..73b6287624 100644 --- a/src/bin/d2/tests/nc_test_utils.h +++ b/src/lib/d2srv/testutils/nc_test_utils.h @@ -11,7 +11,8 @@ #include #include -#include +#include +#include #include #include @@ -34,33 +35,41 @@ typedef boost::shared_ptr SocketPtr; /// requests in a given manner. class FauxServer { public: - enum ResponseMode { + /// @brief The types of response generated by the server. + enum ResponseMode { USE_RCODE, // Generate a response with a given RCODE CORRUPT_RESP, // Generate a corrupt response INVALID_TSIG // Generate a response with the wrong TSIG key }; - // Reference to IOService to use for IO processing. + /// @brief Reference to IOService to use for IO processing. asiolink::IOService& io_service_; - // IP address at which to listen for requests. + + /// @brief IP address at which to listen for requests. const asiolink::IOAddress& address_; - // Port on which to listen for requests. + + /// @brief Port on which to listen for requests. size_t port_; - // Socket on which listening is done. + + /// @brief Socket on which listening is done. SocketPtr server_socket_; - // Stores the end point of requesting client. + + /// @brief Stores the end point of requesting client. boost::asio::ip::udp::endpoint remote_; - // Buffer in which received packets are stuffed. + + /// @brief Buffer in which received packets are stuffed. uint8_t receive_buffer_[TEST_MSG_MAX]; - // Flag which indicates if a receive has been initiated but - // not yet completed. + + /// @brief Flag which indicates if a receive has been initiated but not yet + /// completed. bool receive_pending_; - // Indicates if server is in perpetual receive mode. If true once - // a receive has been completed, a new one will be automatically - // initiated. + /// @brief Flag which indicates if server is in perpetual receive mode. + /// If true once a receive has been completed, a new one will be + /// automatically initiated. bool perpetual_receive_; - // TSIG Key to use to verify requests and sign responses. If its - // NULL TSIG is not used. + + /// @brief TSIG Key to use to verify requests and sign responses. If it is + /// NULL TSIG is not used. D2TsigKeyPtr tsig_key_; /// @brief Constructor @@ -137,10 +146,10 @@ public: asiolink::IntervalTimer timer_; int run_time_; - // Constructor + /// @brief Constructor TimedIO(); - // Destructor + /// @brief Destructor virtual ~TimedIO(); /// @brief IO Timer expiration handler @@ -177,7 +186,7 @@ public: DdnsDomainPtr reverse_domain_; D2CfgMgrPtr cfg_mgr_; - /// #brief constants used to specify change directions for a transaction. + /// @brief constants used to specify change directions for a transaction. static const unsigned int FORWARD_CHG; // Only forward change. static const unsigned int REVERSE_CHG; // Only reverse change. static const unsigned int FWD_AND_REV_CHG; // Both forward and reverse. @@ -252,7 +261,6 @@ public: /// be used. void setupForIPv6Transaction(dhcp_ddns::NameChangeType change_type, int change_mask, const std::string& key_name); - }; @@ -483,7 +491,6 @@ extern void checkContext(NameChangeTransactionPtr trans, const int exp_state, /// @brief Macro for calling checkContext() that supplies invocation location #define CHECK_CONTEXT(a,b,c) checkContext(a,b,c,__FILE__,__LINE__) - } // namespace isc::d2 } // namespace isc diff --git a/src/bin/d2/tests/stats_test_utils.cc b/src/lib/d2srv/testutils/stats_test_utils.cc similarity index 59% rename from src/bin/d2/tests/stats_test_utils.cc rename to src/lib/d2srv/testutils/stats_test_utils.cc index 7c8f6df6e3..262879b421 100644 --- a/src/bin/d2/tests/stats_test_utils.cc +++ b/src/lib/d2srv/testutils/stats_test_utils.cc @@ -6,7 +6,7 @@ #include #include -#include +#include using namespace isc::data; using namespace isc::stats; @@ -26,22 +26,7 @@ D2StatTest::~D2StatTest() { } void -D2StatTest::checkStat(const string& name, const int64_t expected_value) { - ObservationPtr obs = StatsMgr::instance().getObservation(name); - ASSERT_TRUE(obs) << " stat: " << name << " not found "; - ASSERT_EQ(expected_value, obs->getInteger().first) - << " stat: " << name << " value wrong"; -} - -void -D2StatTest::checkStats(const StatMap& expected_stats) { - for (const auto& it : expected_stats) { - checkStat(it.first, it.second); - } -} - -void -D2StatTest::checkStats(const string& key_name, const StatMap& expected_stats) { +checkStats(const string& key_name, const StatMap& expected_stats) { StatMap key_stats; for (const auto& it : expected_stats) { const string& stat_name = diff --git a/src/lib/d2srv/testutils/stats_test_utils.h b/src/lib/d2srv/testutils/stats_test_utils.h new file mode 100644 index 0000000000..ccdad02acb --- /dev/null +++ b/src/lib/d2srv/testutils/stats_test_utils.h @@ -0,0 +1,48 @@ +// Copyright (C) 2020-2021 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef D2_STATS_TEST_UTILS_H +#define D2_STATS_TEST_UTILS_H + +#include +#include +#include +#include + +#include + +namespace isc { +namespace d2 { +namespace test { + +/// @brief Import statistic test utils. +using isc::stats::test::StatMap; +using isc::stats::test::checkStat; +using isc::stats::test::checkStats; + +/// @brief Test class with utility functions to test statistics. +class D2StatTest { +public: + /// @brief Constructor. + D2StatTest(); + + /// @brief Destructor. + virtual ~D2StatTest(); +}; + +/// @brief Compares StatsMgr key statistics against expected values. +/// +/// Prepend key part of names before calling checkStats simpler variant. +/// +/// @param key_name Name of the key. +/// @param expected_stats Map of expected static names and values. +void checkStats(const std::string& key_name, const StatMap& expected_stats); + +} +} +} + +#endif // D2_STATS_TEST_UTILS_H diff --git a/src/lib/dhcp_ddns/ncr_udp.cc b/src/lib/dhcp_ddns/ncr_udp.cc index fc46f5b651..43629d1818 100644 --- a/src/lib/dhcp_ddns/ncr_udp.cc +++ b/src/lib/dhcp_ddns/ncr_udp.cc @@ -163,12 +163,12 @@ NameChangeUDPListener::receiveCompletionHandler(const bool successful, try { ncr = NameChangeRequest::fromFormat(format_, input_buffer); isc::stats::StatsMgr::instance().addValue("ncr-received", - static_cast(0)); + static_cast(1)); } catch (const NcrMessageError& ex) { // log it and go back to listening LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_INVALID_NCR).arg(ex.what()); isc::stats::StatsMgr::instance().addValue("ncr-invalid", - static_cast(0)); + static_cast(1)); // Queue up the next receive. // NOTE: We must call the base class, NEVER doReceive @@ -187,7 +187,7 @@ NameChangeUDPListener::receiveCompletionHandler(const bool successful, LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_NCR_UDP_RECV_ERROR) .arg(error_code.message()); isc::stats::StatsMgr::instance().addValue("ncr-error", - static_cast(0)); + static_cast(1)); result = ERROR; } } diff --git a/src/lib/stats/Makefile.am b/src/lib/stats/Makefile.am index aa7568a769..37f3e41f15 100644 --- a/src/lib/stats/Makefile.am +++ b/src/lib/stats/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = . tests +SUBDIRS = . tests testutils AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) diff --git a/src/lib/stats/testutils/Makefile.am b/src/lib/stats/testutils/Makefile.am new file mode 100644 index 0000000000..009257f6b6 --- /dev/null +++ b/src/lib/stats/testutils/Makefile.am @@ -0,0 +1 @@ +EXTRA_DIST = stats_test_utils.h diff --git a/src/lib/stats/testutils/stats_test_utils.h b/src/lib/stats/testutils/stats_test_utils.h new file mode 100644 index 0000000000..808dced39a --- /dev/null +++ b/src/lib/stats/testutils/stats_test_utils.h @@ -0,0 +1,64 @@ +// Copyright (C) 2020-2021 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef STATS_TEST_UTILS_H +#define STATS_TEST_UTILS_H + +#include +#include + +#include + +namespace isc { +namespace stats { +namespace test { + +/// @brief Type of name x value for statistics. +typedef std::map StatMap; + +/// @brief Compares a statistic to an expected value. +/// +/// Attempt to fetch the named statistic from the StatsMgr and if +/// found, compare its observed value to the given value. +/// Fails if the stat is not found or if the values do not match. +/// +/// @param name StatsMgr name for the statistic to check. +/// @param expected_value expected value of the statistic. +inline void checkStat(const std::string& name, const int64_t expected_value) { + using namespace isc::stats; + ObservationPtr obs = StatsMgr::instance().getObservation(name); + ASSERT_TRUE(obs) << " stat: " << name << " not found "; + ASSERT_EQ(expected_value, obs->getInteger().first) + << " stat: " << name << " value wrong"; +} + +/// @brief Check if a statistic does not exists. +/// +/// @param name StatsMgr name for the statistic to check. +inline void checkNoStat(const std::string& name) { + using namespace isc::stats; + EXPECT_FALSE(StatsMgr::instance().getObservation(name)); +} + +/// @brief Compares StatsMgr statistics against expected values. +/// +/// Iterates over a list of statistic names and expected values, attempting +/// to fetch each from the StatsMgr and if found, compare its observed +/// value to the expected value. Fails if any of the expected stats are not +/// found or if the values do not match. +/// +/// @param expected_stats Map of expected static names and values. +inline void checkStats(const StatMap& expected_stats) { + for (const auto& it : expected_stats) { + checkStat(it.first, it.second); + } +} + +} +} +} + +#endif // STATS_TEST_UTILS_H -- GitLab From 809c0a13aa1526f2f4a204dbf3025d8ffcd4ba33 Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Wed, 8 Dec 2021 20:50:44 +0200 Subject: [PATCH 2/2] [#2228] updated documentation --- doc/sphinx/arm/ext-gss-tsig.rst | 12 --- doc/sphinx/arm/hooks.rst | 4 + src/bin/d2/d2.dox | 147 +++++++++++++++++++++----------- src/lib/d2srv/d2_tsig_key.h | 2 +- src/lib/d2srv/d2srv.dox | 21 +++++ 5 files changed, 122 insertions(+), 64 deletions(-) create mode 100644 src/lib/d2srv/d2srv.dox diff --git a/doc/sphinx/arm/ext-gss-tsig.rst b/doc/sphinx/arm/ext-gss-tsig.rst index cf718a5589..7f296e72f4 100644 --- a/doc/sphinx/arm/ext-gss-tsig.rst +++ b/doc/sphinx/arm/ext-gss-tsig.rst @@ -6,13 +6,6 @@ GSS-TSIG .. _gss-tsig-overview: - -.. note:: - - The GSS-TSIG feature is considered experimental. It is possible to perform - the TKEY exchanges and sign the DNS updates using GSS-TSIG, but some error - handling and fallback scenarios are not covered yet. Use with caution. - GSS-TSIG Overview ----------------- @@ -25,11 +18,6 @@ additional capabilities as using negotiated dynamic keys. Kea provides the support of GSS-TSIG to protect DNS updates sent by the Kea DHCP-DDNS (aka D2) server in a premium hook, called `gss_tsig`. -.. note:: - - This library is still in the experimental phase and is not recommended - nor supported for use in production. Use with care! - The GSS-TSIG is defined in `RFC 3645 `__. The GSS-TSIG protocol itself is an implementation of generic GSS-API v2 services, defined in `RFC 2743 `__. diff --git a/doc/sphinx/arm/hooks.rst b/doc/sphinx/arm/hooks.rst index cc3c5b9ddb..989a3cd98e 100644 --- a/doc/sphinx/arm/hooks.rst +++ b/doc/sphinx/arm/hooks.rst @@ -468,6 +468,10 @@ loaded by the correct process per the table below. | | |are several exported environment variables available for | | | |the script. | +-----------------+---------------+------------------------------------------------------------+ + | GSS-TSIG | ISC support |This hook library adds support to the Kea D2 server | + | | customers |(kea-dhcp-ddns) for using GSS-TSIG to sign DNS updates. | + | | (since 2.0.1) | | + +-----------------+---------------+------------------------------------------------------------+ ISC hopes to see more hooks libraries become available as time progresses, developed both internally and externally. Since this list diff --git a/src/bin/d2/d2.dox b/src/bin/d2/d2.dox index 99694687e9..8a41e523dd 100644 --- a/src/bin/d2/d2.dox +++ b/src/bin/d2/d2.dox @@ -18,6 +18,19 @@ update the DNS data. The design documentation for D2 can be found here: D2 Design. +The implementation is split in several libraries which can be used separately +(linked only when required): +- src/lib/dns (libkea-dns++) - contains classes and enumerations which define +DNS update message, message content, EDNS, RData, RR Types, RCODEs, OPCODEs, +DNS Name, TSIG key and TSIG record, exception types, etc. +- src/lib/dhcp_ddns (libkea-dhcp_ddns) - contains classes to send and receive, +encapsulate and decapsulate DNS messages, etc. +- src/lib/d2srv (libkea-d2srv) - contains classes to handle NCR transactions, +the DNS client implementation, statistics, configuration parser and manager, +logger, DNS Zone, etc. +- src/bin/d2 (kea-dhcp-ddns) - contains classes which handle message queues, +D2 process and D2 controller internals, etc. + This document contains several UML diagrams, and a few conventions used within these diagrams are worth noting: @@ -37,15 +50,19 @@ in the diagram below: @image html d2_app_classes.svg "D2's CPL Derivations" - isc::d2::D2Controller - entry point for running D2, it processes command line -options, starts and controls the application process, @c D2Process. +options, starts and controls the application process, @c D2Process +(see @ref src/bin/d2/d2_controller.h). - isc::d2::D2Process - creates and manages D2's primary resources and -implements the main event loop described in @ref d2EventLoop. +implements the main event loop described in @ref d2EventLoop section +(see @ref src/bin/d2/d2_process.h). - isc::d2::D2CfgMgr - creates, updates, and provides access to D2's application -configuration which is embodied by @c D2CfgContext. +configuration which is embodied by @c D2CfgContext +(see @ref src/lib/d2srv/d2_cfg_mgr.h). -- isc::d2::D2CfgContext - warehouses D2's application configuration. +- isc::d2::D2CfgContext - warehouses D2's application configuration +(see @ref src/lib/d2srv/d2_cfg_mgr.h). @section d2ConfigMgt Configuration Management @@ -69,17 +86,24 @@ The runtime classes that embody this model are shown in the following diagram: @image html config_data_classes.svg "D2's Configuration Data Classes" - isc::d2::D2CfgContext - D2-specific derivation of @c DCfgContextBase. It -houses the "global" configuration for an instance of D2. +houses the "global" configuration for an instance of D2 +(see @ref src/lib/d2srv/d2_cfg_mgr.h). + - isc::d2::DdnsDomainListMgr - manages a list of domains. Currently there is one manager instance for the list of forward domains, and one for the list of reverse domains. In addition the domain list, it will may house other values - specific to that list of domains (e.g. enable flag) +specific to that list of domains (e.g. enable flag) +(see @ref src/lib/d2srv/d2_config.h). + - isc::d2::DdnsDomain - represents a DNS domain (really a zone). When requests -are received they are matched to a domain by comparing their FQDN to the domain's name. +are received they are matched to a domain by comparing their FQDN to the +domain's name (see @ref src/lib/d2srv/d2_config.h). + - isc::d2::DnsServerInfo - describes a DNS server which supports DDNS for a -given domain. -- isc::d2::TSIGKeyInfo - describes a TSIG key used for authenticated DDNS for -a given domain. +given domain (see @ref src/lib/d2srv/d2_config.h). + +- isc::d2::TSIGKeyInfo - describes a TSIG key used for authenticated DDNS for a +given domain (see @ref src/lib/d2srv/d2_config.h). The parsing classes, as one would expect, parallel the runtime classes quite closely. The parsers are named for the runtime class they instantiate and are @@ -89,23 +113,29 @@ class. The parser classes are shown in the diagram below: @image html config_parser_classes.svg "D2's Configuration Parsers" - isc::d2::DdnsDomainListMgrParser - parser for a domain list manager, it -creates a domain list parser +creates a domain list parser. + - isc::d2::DdnsDomainListParser - parser for a list of domains, it creates a -domain parser for domain described in a list domains +domain parser for domain described in a list domains. + - isc::d2::DdnsDomainParser - Parser for a domain, it creates a DNS server list -parser -- isc::d2::DnsServerInfoListParser - parser for a list of DNS servers, it -creates a DNS server parser for server described in a list of servers -- isc::d2::DnsServerInfoParser - parser for DNS server +parser. + +- isc::d2::DnsServerInfoListParser - parser for a list of DNS servers, it +creates a DNS server parser for server described in a list of servers. + +- isc::d2::DnsServerInfoParser - parser for DNS server. + - isc::d2::TSIGKeyInfoListParser - parser for a list of TSIG keys, it creates a -parser for key described in a list of keys -- isc::d2::TSIGKeyInfoParser +parser for key described in a list of keys. + +- isc::d2::TSIGKeyInfoParser - parser for TSIG key. + +For more details on the parsers see @ref src/lib/d2srv/d2_config.h. The underlying common libraries for configuration parsing support configuration input in JSON format, that complies with a fixed set of generic constructs that -may be described by a spec file (also JSON). This mechanism is subject to -change, but in the meantime, the spec file describe D2's configuration may be -found here: @c src/bin/d2/dhcp-ddns.spec +may be described by a spec file (also JSON). @section d2NCRReceipt Request Reception and Queuing @@ -115,23 +145,29 @@ shown in the diagram below: @image html request_mgt_classes.svg "Request Management Classes" -- isc::d2::D2QueueMgr - owned by @c D2Process, it listens for @c NameChangeRequests -and queues them for processing. It also provides services for adding, -finding, and removing queue entries. It owns the interface used to receive -requests and thus shields the remainder of D2 from any specific knowledge or -interaction with this interface. -- isc::d2::RequestQueue - storage container for the received requests. +- isc::d2::D2QueueMgr - owned by @c D2Process, it listens for +@c NameChangeRequests and queues them for processing. It also provides services +for adding, finding, and removing queue entries. It owns the interface used to +receive requests and thus shields the remainder of D2 from any specific +knowledge or interaction with this interface +(see @ref src/bin/d2/d2_queue_mgr.h). + +- isc::d2::RequestQueue - storage container for the received requests +(see @ref src/bin/d2/d2_queue_mgr.h). + - isc::dhcp_ddns::NameChangeListener - Abstract asynchronous interface for receiving requests which uses ASIO to process IO and invoke a callback upon -request receipt +request receipt (see @ref src/lib/dhcp_ddns/ncr_io.h). + - isc::dhcp_ddns::NameChangeUDPListener - Derivation of NameChangeListener which supports receiving request via UDP socket +(see @ref src/lib/dhcp_ddns/ncr_udp.h). + - isc::dhcp_ddns::NameChangeRequest - embodies a request to update DNS entries -based upon a DHCP lease change +based upon a DHCP lease change (see @ref src/lib/dhcp_ddns/ncr_msg.h). D2QueueMgr is state-driven, albeit with a very simple state model. The states -are defined by isc::d2::D2QueueMgr::State, and described in detail in in -@ref d2_queue_mgr.h. +are defined by isc::d2::D2QueueMgr::State (see @ref src/bin/d2/d2_queue_mgr.h). @section d2DDNSUpdateExecution Update Execution @@ -154,34 +190,39 @@ following diagram: - isc::d2::D2UpdateMgr owned by @c D2Process, orchestrates the fulfillment of each request by managing the execution of its transaction. Its high level method @ref isc::d2::D2UpdateMgr::sweep() is meant to be called whenever IO -events occur. The following steps are performed each time the method is called: +events occur (see @ref src/bin/d2/d2_update_mgr.h). The following steps are +performed each time the method is called: - Any transaction which has been completed, is logged and culled from the transaction list. - - Start a new transaction for the next queued request (if any) + - Start a new transaction for the next queued request (if any). - isc::d2::NameChangeTransaction - abstract state-driven class which carries out the steps necessary to fulfill a single request. Fulfilling a request is -achieved as IO events in response it DDNS requests drive the transaction -through its state model. The type of transaction is based on the nature of -the request, this is discussed further on @ref d2TransDetail +achieved as IO events in response it DDNS requests drive the transaction through +its state model (see @ref src/lib/d2srv/nc_trans.h). The type of transaction is +based on the nature of the request, this is discussed further on +@ref d2TransDetail section. - isc::d2::DNSClient - an asynchronous worker which atomically performs a single DDNS packet exchange with a given server, providing the response via a callback mechanism. Each time a transaction's state model calls for a packet -exchange with a DNS server, it uses an instance of this class to do it. +exchange with a DNS server, it uses an instance of this class to do it +(see @ref src/lib/d2srv/dns_client.h). - isc::d2::D2UpdateMessage - container for sending and receiving DDNS packets +(see @ref src/lib/d2srv/d2_update_message.h). @section d2EventLoop Main Event Loop Now that all of the primary components have been introduced it is worth while discussing D2's main event loop. As mentioned earlier D2 is constructed around -the CPL which is designed to be driven by asynchronous IO processed by a -common IO service thread (isc::asiolink::io_service). Any IO that needs to be +the CPL which is designed to be driven by asynchronous IO processed by a common +IO service thread (isc::asiolink::io_service). Any IO that needs to be performed by the application thread uses this service to do so. This organizes the IO event processing into a single event loop centered around the service. -(This does not preclude spinning off worker threads to conduct other tasks, -with their own io_service instances). D2's main event loop, implemented in @ref isc::d2::D2Process::run() may be paraphrased as follows: +(This does not preclude spinning off worker threads to conduct other tasks, with +their own io_service instances). D2's main event loop, implemented in +@ref isc::d2::D2Process::run() may be paraphrased as follows: @code As long as we should not shutdown repeat the following steps: @@ -213,9 +254,14 @@ The transaction classes are shown in the following diagram: @image html trans_classes.svg "NameChangeTransaction Derivations" -- isc::d2::NameAddTransaction - carries out a @c NameChangeRequest to add entries -- isc::d2::NameRemoveTransaction - carries out a @c NameChangeRequest to remove entries +- isc::d2::NameAddTransaction - carries out a @c NameChangeRequest to add +entries (see @ref src/bin/d2/nc_add.h). + +- isc::d2::NameRemoveTransaction - carries out a @c NameChangeRequest to remove +entries (see @ref src/bin/d2/nc_remove.h). + - isc::util::StateModel - abstract state model described in @ref d2StateModel +section (see @ref src/lib/util/state_model.h). The state models for these two transactions implement DDNS with conflict resolution as described in RFC 4703. @@ -246,20 +292,19 @@ a common library. - isc::util::StateModel - provides the mechanics for executing a state model described by a dictionary events and states. It provides methods to: - - initialize the model - constructs the dictionary of events and states + - initialize the model - constructs the dictionary of events and states. - start the model - sets the model to its initial state, posts a "start" - event and starts the model "running" - "start event" + event and starts the model "running". - run the model - process the posted event (if one) until the model reaches a wait state or reaches the end state. - - transition from one state to another -- isc::d2::Event - Defines an event used to stimulate the model + - transition from one state to another. +- isc::d2::Event - Defines an event used to stimulate the model. - isc::d2::State - Defines a state with the model, each state has a handler -method that is executed upon transition "into" that state from another +method that is executed upon transition "into" that state from another state. - isc::d2::LabeledValue - An abstract that associates a integer value with a -text label +text label. - isc::d2::LabeledValueSet - A collection of LabeledValues for which the integer -values must be unique +values must be unique. @subsection d2TransExecExample Transaction Execution Example diff --git a/src/lib/d2srv/d2_tsig_key.h b/src/lib/d2srv/d2_tsig_key.h index 0101b4b2ef..4cdb0361c6 100644 --- a/src/lib/d2srv/d2_tsig_key.h +++ b/src/lib/d2srv/d2_tsig_key.h @@ -56,7 +56,7 @@ public: /// /// @note Derived classes can implement their own specific context. /// - /// @return The specific @ref TSIGContext of the @ref TSIGKey. + /// @return The specific @ref dns::TSIGContext of the @ref dns::TSIGKey. virtual dns::TSIGContextPtr createContext(); private: diff --git a/src/lib/d2srv/d2srv.dox b/src/lib/d2srv/d2srv.dox new file mode 100644 index 0000000000..b2e338d1ce --- /dev/null +++ b/src/lib/d2srv/d2srv.dox @@ -0,0 +1,21 @@ +// Copyright (C) 2020-2021 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +/** + @page libd2srv libkea-d2srv - Kea D2 Server Library + +This library contains most of the DNS classes used by the Kea D2 server +(kea-dhcp-ddns) but is organized as an independent library so that it +can be reused as needed by other components. + +For a more detaliled documentation about the design and class separation +see @ref src/bin/d2/d2.dox. + +@section d2srvMTConsiderations Multi-Threading Consideration for D2 Server Library + +By default this library is not thread safe. + +*/ -- GitLab