[Openvpn-devel,M] Change in openvpn[master]: win: match search domains when creating exclude rules

Message ID 2609ea882d125e4e86450ea68da0edcaa7823587-HTML@gerrit.openvpn.net
State New
Headers show
Series [Openvpn-devel,M] Change in openvpn[master]: win: match search domains when creating exclude rules | expand

Commit Message

d12fk (Code Review) March 7, 2025, 2:39 a.m. UTC
Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

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

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

to review the following change.


Change subject: win: match search domains when creating exclude rules
......................................................................

win: match search domains when creating exclude rules

Compare local domains for exclude rules to search domains and skip
matching ones. This prevents the creation of exclude rules when the
server indicates that the domain should be resolved via the VPN, by
pushing the search domain.

Change-Id: I4919af2b845a47787c08f454b108ef376ea5c0f6
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
---
M src/openvpnserv/interactive.c
1 file changed, 56 insertions(+), 24 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/05/905/1

Patch

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 6097cd0..f725199 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -2163,12 +2163,14 @@ 
  * In case of an error or if no domains are found for the interface
  * \p size is set to 0 and the contents of \p domains are invalid.
  * Note that the domains could have been set by DHCP or manually.
+ * Note that domains are ignored if they match a pushed search domain.
  *
- * @param  itf        HKEY of the interface to read from
- * @param  domains    PWSTR buffer to return the domain(s) in
- * @param  size       pointer to size of the domains buffer in bytes. Will be
- *                    set to the size of the string returned, including
- *                    the terminating zeros or 0.
+ * @param  itf             HKEY of the interface to read from
+ * @param  search_domains  optional list of search domains
+ * @param  domains         PWSTR buffer to return the domain(s) in
+ * @param  size            pointer to size of the domains buffer in bytes. Will be
+ *                         set to the size of the string returned, including
+ *                         the terminating zeros or 0.
  *
  * @return LSTATUS NO_ERROR if the domain suffix(es) were read successfully,
  *         ERROR_FILE_NOT_FOUND if no domain was found for the interface,
@@ -2176,7 +2178,7 @@ 
  *         any other error indicates an error while reading from the registry.
  */
 static LSTATUS
-GetItfDnsDomains(HKEY itf, PWSTR domains, PDWORD size)
+GetItfDnsDomains(HKEY itf, PCWSTR search_domains, PWSTR domains, PDWORD size)
 {
     if (domains == NULL || size == 0)
     {
@@ -2209,9 +2211,29 @@ 
                     *comma = '\0';
                 }
 
+                /* Ignore itf domains which match pushed search domains */
+                size_t domain_len = wcslen(pos);
+                PCWSTR match = wcsstr(search_domains, pos);
+                if (match && (match == search_domains || *(match - 1) == ',')
+                    && (*(match + domain_len) == ',' || *(match + domain_len) == '\0'))
+                {
+                    if (comma)
+                    {
+                        pos = comma + 1;
+                        continue;
+                    }
+                    else
+                    {
+                        /* This was the last domain */
+                        *pos = '\0';
+                        *size += 1;
+                        return wcslen(domains) ? NO_ERROR : ERROR_FILE_NOT_FOUND;
+                    }
+                }
+
                 /* Check for enough space to convert this domain */
+                domain_len += 1; /* leading dot */
                 size_t converted_size = pos - domains;
-                size_t domain_len = wcslen(pos) + 1;
                 size_t domain_size = domain_len * one_glyph;
                 size_t extra_size = 2 * one_glyph;
                 if (converted_size + domain_size + extra_size > buf_size)
@@ -2292,11 +2314,12 @@ 
  * needed so that local DNS keeps working even when a catch all NRPT rule is
  * installed by a VPN connection.
  *
- * @param  data       pointer to the data structures the values are returned in
- * @param  data_size  number of exclude data structures pointed to
+ * @param  search_domains  optional list of search domains
+ * @param  data            pointer to the data structures the values are returned in
+ * @param  data_size       number of exclude data structures pointed to
  */
 static void
-GetNrptExcludeData(nrpt_exclude_data_t *data, size_t data_size)
+GetNrptExcludeData(PCWSTR search_domains, nrpt_exclude_data_t *data, size_t data_size)
 {
     HKEY v4_itfs = INVALID_HANDLE_VALUE;
     HKEY v6_itfs = INVALID_HANDLE_VALUE;
@@ -2342,7 +2365,7 @@ 
         /* Get the DNS domain(s) for exclude routing */
         data[i].domains_size = sizeof(data[0].domains);
         memset(data[i].domains, 0, data[i].domains_size);
-        err = GetItfDnsDomains(v4_itf, data[i].domains, &data[i].domains_size);
+        err = GetItfDnsDomains(v4_itf, search_domains, data[i].domains, &data[i].domains_size);
         if (err)
         {
             if (err != ERROR_FILE_NOT_FOUND)
@@ -2504,15 +2527,16 @@ 
  * local resolution of names is not interfered with in case the VPN resolves
  * all names.
  *
- * @param  nrpt_key   the registry key to set the rules under
- * @param  ovpn_pid   the PID of the openvpn process
+ * @param  nrpt_key        the registry key to set the rules under
+ * @param  ovpn_pid        the PID of the openvpn process
+ * @param  search_domains  optional list of search domains
  */
 static void
-SetNrptExcludeRules(HKEY nrpt_key, DWORD ovpn_pid)
+SetNrptExcludeRules(HKEY nrpt_key, DWORD ovpn_pid, PCWSTR search_domains)
 {
     nrpt_exclude_data_t data[8]; /* data from up to 8 interfaces */
     memset(data, 0, sizeof(data));
-    GetNrptExcludeData(data, _countof(data));
+    GetNrptExcludeData(search_domains, data, _countof(data));
 
     unsigned n = 0;
     for (int i = 0; i < _countof(data); ++i)
@@ -2534,17 +2558,18 @@ 
 /**
  * Set NRPT rules for a openvpn process
  *
- * @param  nrpt_key   the registry key to set the rules under
- * @param  addresses  name server addresses
- * @param  domains    optional list of split routing domains
- * @param  dnssec     boolean whether DNSSEC is to be used
- * @param  ovpn_pid   the PID of the openvpn process
+ * @param  nrpt_key          the registry key to set the rules under
+ * @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  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, BOOL dnssec, DWORD ovpn_pid)
+SetNrptRules(HKEY nrpt_key, const nrpt_address_t *addresses, const char *domains,
+             const char *search_domains, BOOL dnssec, DWORD ovpn_pid)
 {
     DWORD err = NO_ERROR;
     PWSTR wide_domains = L".\0"; /* DNS route everything by default */
@@ -2573,7 +2598,14 @@ 
     }
     else
     {
-        SetNrptExcludeRules(nrpt_key, ovpn_pid);
+        PWSTR wide_search_domains;
+        wide_search_domains = utf8to16(search_domains);
+        if (!wide_search_domains)
+        {
+            return ERROR_OUTOFMEMORY;
+        }
+        SetNrptExcludeRules(nrpt_key, ovpn_pid, wide_search_domains);
+        free(wide_search_domains);
     }
 
     /* Create address string list */
@@ -2833,7 +2865,7 @@ 
 
     /* Set NRPT rules */
     BOOL dnssec = (msg->flags & nrpt_dnssec) != 0;
-    err = SetNrptRules(key, msg->addresses, msg->resolve_domains, dnssec, ovpn_pid);
+    err = SetNrptRules(key, msg->addresses, msg->resolve_domains, msg->search_domains, dnssec, ovpn_pid);
     if (err)
     {
         goto out;