[S] Change in openvpn[master]: dns: correctly handle dnssec settings

"selvanair (Code Review)" <[email protected]>
Newsgroups gmane.network.openvpn.devel
Message-ID <[email protected]>
Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1643?usp=email

to review the following change.


Change subject: dns: correctly handle dnssec settings
......................................................................

dns: correctly handle dnssec settings

Change-Id: Id514b06223cb55295c92b1fa6727f03d6e06befe
Signed-off-by: Selva Nair <[email protected]>
---
M doc/man-sections/client-options.rst
M include/openvpn-msg.h
M src/openvpn/dns.c
M src/openvpn/dns.h
M src/openvpnserv/interactive.c
5 files changed, 29 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/43/1643/1

diff --git a/doc/man-sections/client-options.rst b/doc/man-sections/client-options.rst
index 1664eed..abd5ec7 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -231,7 +231,7 @@
   The ``dnssec`` option is used to configure validation of DNSSEC records.
   While the exact semantics may differ for resolvers on different systems,
   ``yes`` likely makes validation mandatory, ``no`` disables it, and ``optional``
-  uses it opportunistically.
+  uses it opportunistically. The default is ``optional``.
 
   The ``transport`` option enables DNS-over-HTTPS (``DoH``) or DNS-over-TLS (``DoT``)
   for a DNS server. The ``sni`` option can be used with them to specify the
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index ca3267e..3e1f99d 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -110,7 +110,8 @@
 
 typedef enum
 {
-    nrpt_dnssec = 1 << 0,
+    nrpt_dnssec_enabled = 1 << 0,
+    nrpt_dnssec_required = 1 << 1,
 } nrpt_flags_t;
 
 #define NRPT_ADDR_NUM  8  /* Max. number of addresses */
diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index ce23f1f..256aac9 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -442,8 +442,25 @@
         .header = { (add ? msg_add_nrpt_cfg : msg_del_nrpt_cfg), sizeof(nrpt_dns_cfg_message_t),
                     0 },
         .iface = { .index = tt->adapter_index, .name = "" },
-        .flags = server->dnssec == DNS_SECURITY_NO ? 0 : nrpt_dnssec,
+        .flags = 0,
     };
+
+    switch (server->dnssec)
+    {
+        case DNS_SECURITY_YES:
+            nrpt.flags |= nrpt_dnssec_required;
+            /* fall through */
+        case DNS_SECURITY_OPTIONAL:
+            nrpt.flags |= nrpt_dnssec_enabled;
+            break;
+        case DNS_SECURITY_NO:
+            nrpt.flags = 0;
+            break;
+        default:
+            ASSERT(0);
+            break;
+    }
+
     strncpynt(nrpt.iface.name, tt->actual_name, sizeof(nrpt.iface.name));
 
     for (size_t i = 0; i < NRPT_ADDR_NUM; ++i)
diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 51bc2de..7786452 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -29,10 +29,9 @@
 
 enum dns_security
 {
-    DNS_SECURITY_UNSET,
+    DNS_SECURITY_OPTIONAL,
     DNS_SECURITY_NO,
     DNS_SECURITY_YES,
-    DNS_SECURITY_OPTIONAL
 };
 
 enum dns_server_transport
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 473a8d3..f1dd794 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -2463,13 +2463,13 @@
  * @param  address    name server address string
  * @param  domains    domains to resolve by this server as MULTI_SZ
  * @param  dom_size   size of domains in bytes including the terminators
- * @param  dnssec     boolean to determine if DNSSEC is to be enabled
+ * @param  dnssec     flag to determine DNSSEC setting
  *
  * @return NO_ERROR on success, or Windows error code
  */
 static DWORD
 SetNrptRule(HKEY nrpt_key, PCWSTR subkey, PCSTR address, PCWSTR domains, DWORD dom_size,
-            BOOL dnssec)
+            nrpt_flags_t dnssec)
 {
     /* Create rule subkey */
     DWORD err = NO_ERROR;
@@ -2497,7 +2497,7 @@
 
     DWORD reg_val;
     /* Set DNSSEC if required */
-    if (dnssec)
+    if (dnssec & nrpt_dnssec_required)
     {
         reg_val = 1;
         err = RegSetValueExA(rule_key, "DNSSECValidationRequired", 0, REG_DWORD, (PBYTE)&reg_val,
@@ -2506,7 +2506,6 @@
         {
             goto out;
         }
-
         reg_val = 0;
         err = RegSetValueExA(rule_key, "DNSSECQueryIPSECRequired", 0, REG_DWORD, (PBYTE)&reg_val,
                              sizeof(reg_val));
@@ -2525,7 +2524,7 @@
     }
 
     /* Set NRPT config options */
-    reg_val = dnssec ? 0x0000000A : 0x00000008;
+    reg_val = (dnssec & nrpt_dnssec_enabled) ? 0x0000000A : 0x00000008;
     err = RegSetValueExA(rule_key, "ConfigOptions", 0, REG_DWORD, (const PBYTE)&reg_val,
                          sizeof(reg_val));
     if (err)
@@ -2593,14 +2592,14 @@
  * @param  addresses         name server addresses
  * @param  domains           optional list of split routing domains
  * @param  search_domains    optional list of search domains
- * @param  dnssec            boolean whether DNSSEC is to be used
+ * @param  dnssec            flag to determine DNSSEC setting
  * @param  ovpn_pid          the PID of the openvpn process
  *
  * @return NO_ERROR on success, or a Windows error code
  */
 static DWORD
 SetNrptRules(HKEY nrpt_key, const nrpt_address_t *addresses, const char *domains,
-             const char *search_domains, BOOL dnssec, DWORD ovpn_pid)
+             const char *search_domains, nrpt_flags_t dnssec, DWORD ovpn_pid)
 {
     DWORD err = NO_ERROR;
     PWSTR wide_domains = L".\0"; /* DNS route everything by default */
@@ -2896,8 +2895,7 @@
     }
 
     /* Set NRPT rules */
-    BOOL dnssec = (msg->flags & nrpt_dnssec) != 0;
-    err = SetNrptRules(key, msg->addresses, msg->resolve_domains, msg->search_domains, dnssec,
+    err = SetNrptRules(key, msg->addresses, msg->resolve_domains, msg->search_domains, msg->flags,
                        ovpn_pid);
     if (err)
     {

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1643?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id514b06223cb55295c92b1fa6727f03d6e06befe
Gerrit-Change-Number: 1643
Gerrit-PatchSet: 1
Gerrit-Owner: selvanair <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.